Hi, On Wed, Mar 30, 2022 at 9:03 AM Sankeerth Billakanti <quic_sbillaka@xxxxxxxxxxx> 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@xxxxxxxxxxx> > --- > > 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. */ > @@ -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. > 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. Things are pretty much nits, so FWIW: Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>