Quoting Douglas Anderson (2022-04-18 10:17:54) > Sometimes it's useful for users of the DP AUX bus (like panels) to be > able to poll HPD. Let's add a callback that allows DP AUX busses > drivers to provide this. > > Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx> > --- > Left Dmitry's Reviewed-by tag off since patch changed enough. > > (no changes since v2) > > Changes in v2: > - Change is_hpd_asserted() to wait_hpd_asserted() > > include/drm/dp/drm_dp_helper.h | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/include/drm/dp/drm_dp_helper.h b/include/drm/dp/drm_dp_helper.h > index 53d1e722f4de..0940c415db8c 100644 > --- a/include/drm/dp/drm_dp_helper.h > +++ b/include/drm/dp/drm_dp_helper.h > @@ -2035,6 +2035,32 @@ struct drm_dp_aux { > ssize_t (*transfer)(struct drm_dp_aux *aux, > struct drm_dp_aux_msg *msg); > > + /** > + * @wait_hpd_asserted: wait for HPD to be asserted > + * > + * This is mainly useful for eDP panels drivers to wait for an eDP > + * panel to finish powering on. This is an optional function. Is there any use for the opposite direction? For example, does anything care that HPD is deasserted? > + * > + * This function will efficiently wait for up to `wait_us` microseconds > + * for HPD to be asserted and might sleep. > + * > + * This function returns 0 if HPD was asserted or -ETIMEDOUT if time > + * expired and HPD wasn't asserted. This function should not print > + * timeout errors to the log. > + * > + * The semantics of this function are designed to match the > + * readx_poll_timeout() function. That means a `wait_us` of 0 means > + * to wait forever. If you want to do a quick poll you could pass 1 > + * for `wait_us`. It would also make sense to have a drm_dp_wait_hpd_asserted() API int drm_dp_wait_hpd_asserted(struct drm_dp_aux *aux, unsigned long wait_us); and then this aux function could be implemented in various ways. The API could poll if the aux can only read immediate state of HPD, or it could sleep (is sleeping allowed? that isn't clear) and wake up the process once HPD goes high. Or if this op isn't implemented maybe there's a fixed timeout member that is non-zero which means "sleep this long". Either way, making each drm_dp_aux implement that logic seems error prone vs. having the drm_dp_aux implement some function for get_immediate_hpd(struct drm_dp_aux *aux) or notify_on_hpd(struct drm_dp_aux *auxstruct completion *comp) > + * > + * NOTE: this function specifically reports the state of the HPD pin > + * that's associated with the DP AUX channel. This is different from > + * the HPD concept in much of the rest of DRM which is more about > + * physical presence of a display. For eDP, for instance, a display is > + * assumed always present even if the HPD pin is deasserted. > + */ > + int (*wait_hpd_asserted)(struct drm_dp_aux *aux, unsigned long wait_us); > + > /** > * @i2c_nack_count: Counts I2C NACKs, used for DP validation. > */