Hi Doug,
On Wed, Mar 30, 2022 at 9:03 AM Sankeerth Billakanti quic_sbillaka@quicinc.com wrote:
The source device should ensure the sink is ready before proceeding to read the sink capability or performing any aux transactions. The sink
s/performing/perform
will indicate its readiness by asserting the HPD line. The controller driver needs to wait for the hpd line to be asserted by the sink before performing any aux transactions.
The eDP sink is assumed to be always connected. It needs power from the source and its HPD line will be asserted only after the panel is powered on. The panel power will be enabled from the panel-edp driver and only after that, the hpd line will be asserted.
Whereas for DP, the sink can be hotplugged and unplugged anytime. The hpd line gets asserted to indicate the sink is connected and ready. Hence there is no need to wait for the hpd line to be asserted for a DP sink.
Signed-off-by: Sankeerth Billakanti quic_sbillaka@quicinc.com
Changes in v6:
- Wait for hpd high only for eDP
- Split into smaller patches
drivers/gpu/drm/msm/dp/dp_aux.c | 13 ++++++++++++- drivers/gpu/drm/msm/dp/dp_aux.h | 3 ++- drivers/gpu/drm/msm/dp/dp_catalog.c | 13 +++++++++++++ drivers/gpu/drm/msm/dp/dp_catalog.h | 1 + drivers/gpu/drm/msm/dp/dp_display.c | 3 ++- 5 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c index 6d36f63..a217c80 100644 --- a/drivers/gpu/drm/msm/dp/dp_aux.c +++ b/drivers/gpu/drm/msm/dp/dp_aux.c @@ -36,6 +36,7 @@ struct dp_aux_private { bool initted; u32 offset; u32 segment;
bool is_edp; struct drm_dp_aux dp_aux;
}; @@ -337,6 +338,14 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux
*dp_aux,
goto exit; }
if (aux->is_edp) {
Adding a comment about _why_ you're doing this just for eDP would probably be a good idea. Like maybe:
/*
- For eDP it's important to give a reasonably long wait here for HPD
- to be asserted. This is because the panel driver may have _just_
- turned on the panel and then tried to do an AUX transfer. The panel
- driver has no way of knowing when the panel is ready, so it's up
- to us to wait. For DP we never get into this situation so let's
- avoid ever doing the extra long wait for DP.
*/
Okay. Will add it
@@ -491,7 +500,8 @@ void dp_aux_unregister(struct drm_dp_aux
*dp_aux)
drm_dp_aux_unregister(dp_aux); }
-struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog) +struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog
*catalog,
bool is_edp)
nit: I think your indentation of the 2nd line isn't quite right.
I moved bool is_edp into the next line. In vim , it was sowing fine. I'll check
diff --git a/drivers/gpu/drm/msm/dp/dp_aux.h b/drivers/gpu/drm/msm/dp/dp_aux.h index 82afc8d..c99aeec 100644 --- a/drivers/gpu/drm/msm/dp/dp_aux.h +++ b/drivers/gpu/drm/msm/dp/dp_aux.h @@ -16,7 +16,8 @@ void dp_aux_init(struct drm_dp_aux *dp_aux); void dp_aux_deinit(struct drm_dp_aux *dp_aux); void dp_aux_reconfig(struct drm_dp_aux *dp_aux);
-struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog); +struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog
*catalog,
bool is_edp);
nit: I think your indentation of the 2nd line isn't quite right.
I'll check
Things are pretty much nits, so FWIW:
Reviewed-by: Douglas Anderson dianders@chromium.org
Thank you, Sankeerth