Re: [Intel-gfx] [PATCH 1/2] drm: Add HPD state to drm_connector_oob_hotplug_event()

Bjorn Andersson
2022-02-15T14:17:26Z

On Mon 14 Feb 09:59 PST 2022, Imre Deak wrote:

> On Mon, Feb 07, 2022 at 08:43:27PM -0800, Bjorn Andersson wrote:
> > In some implementations, such as the Qualcomm platforms, the display
> > driver has no way to query the current HPD state and as such it's
> > impossible to distinguish between disconnect and attention events.
> >
> > Add a parameter to drm_connector_oob_hotplug_event() to pass the HPD
> > state.
> >
> > Also push the test for unchanged state in the displayport altmode driver
> > into the i915 driver, to allow other drivers to act upon each update.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >
> > Note that the Intel driver has only been compile tested with this patch.
> >
> > drivers/gpu/drm/drm_connector.c | 6 ++++--
> > drivers/gpu/drm/i915/display/intel_dp.c | 14 +++++++++++---
> > drivers/gpu/drm/i915/i915_drv.h | 3 +++
> > drivers/usb/typec/altmodes/displayport.c | 9 ++-------
> > include/drm/drm_connector.h | 5 +++--
> > 5 files changed, 23 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index a50c82bc2b2f..ad7295597c0f 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -2825,6 +2825,7 @@ struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
> > /**
> > * drm_connector_oob_hotplug_event - Report out-of-band hotplug event to connector
> > * @connector_fwnode: fwnode_handle to report the event on
> > + * @hpd_state: number of data lanes available
> > *
> > * On some hardware a hotplug event notification may come from outside the display
> > * driver / device. An example of this is some USB Type-C setups where the hardware
> > @@ -2834,7 +2835,8 @@ struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
> > * This function can be used to report these out-of-band events after obtaining
> > * a drm_connector reference through calling drm_connector_find_by_fwnode().
> > */
> > -void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode)
> > +void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode,
> > + bool hpd_state)
> > {
> > struct drm_connector *connector;
> >
> > @@ -2843,7 +2845,7 @@ void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode)
> > return;
> >
> > if (connector->funcs->oob_hotplug_event)
> > - connector->funcs->oob_hotplug_event(connector);
> > + connector->funcs->oob_hotplug_event(connector, hpd_state);
> >
> > drm_connector_put(connector);
> > }
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 146b83916005..00520867d37b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -4816,15 +4816,23 @@ static int intel_dp_connector_atomic_check(struct drm_connector *conn,
> > return intel_modeset_synced_crtcs(state, conn);
> > }
> >
> > -static void intel_dp_oob_hotplug_event(struct drm_connector *connector)
> > +static void intel_dp_oob_hotplug_event(struct drm_connector *connector, bool hpd_state)
> > {
> > struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
> > struct drm_i915_private *i915 = to_i915(connector->dev);
> > + bool need_work = false;
> >
> > spin_lock_irq(&i915->irq_lock);
> > - i915->hotplug.event_bits |= BIT(encoder->hpd_pin);
> > + if (hpd_state != i915->hotplug.oob_hotplug_state) {
>
> hpd_state is speific to the encoder (pin) so similarly to event_bits
> oob_hotplug_state should be a bitmask as well.
>

That makes sense, thanks for point it out!

Regards,
Bjorn