| Message ID | 20200121171100.4370-3-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Hotplug cleanups | expand |
On Tue, 21 Jan 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Simplify the hotplug code connector->encoder->hpd_pin handling > by introducing a helper for exactly this purpose. > > In the helper we can neatly deal with the potential lack of an > attached encoder on fresh MST connectors leaving the rest of the > hpd code oblivious to such details. I might want to see a WARN_ON(connector->mst_port && encoder->hpd_pin != HPD_NONE) added somewhere. But not necessarily in this patch. Reviewed-by: Jani Nikula <jani.nikula@intel.com> > > Cc: Lyude Paul <lyude@redhat.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_hotplug.c | 50 +++++++++++--------- > 1 file changed, 28 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c > index 042d98bae1ea..8a3e9e901cf7 100644 > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c > @@ -120,6 +120,20 @@ enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv, > #define HPD_STORM_REENABLE_DELAY (2 * 60 * 1000) > #define HPD_RETRY_DELAY 1000 > > +static enum hpd_pin > +intel_connector_hpd_pin(struct intel_connector *connector) > +{ > + struct intel_encoder *encoder = intel_attached_encoder(connector); > + > + /* > + * MST connectors get their encoder attached dynamically > + * so need to make sure we have an encoder here. But since > + * MST encoders have their hpd_pin set to HPD_NONE we don't > + * have to special case them beyond that. > + */ > + return encoder ? encoder->hpd_pin : HPD_NONE; > +} > + > /** > * intel_hpd_irq_storm_detect - gather stats and detect HPD IRQ storm on a pin > * @dev_priv: private driver data pointer > @@ -193,17 +207,12 @@ intel_hpd_irq_storm_switch_to_polling(struct drm_i915_private *dev_priv) > > drm_connector_list_iter_begin(dev, &conn_iter); > for_each_intel_connector_iter(connector, &conn_iter) { > - struct intel_encoder *encoder; > enum hpd_pin pin; > > if (connector->base.polled != DRM_CONNECTOR_POLL_HPD) > continue; > > - encoder = intel_attached_encoder(connector); > - if (!encoder) > - continue; > - > - pin = encoder->hpd_pin; > + pin = intel_connector_hpd_pin(connector); > if (pin == HPD_NONE || > dev_priv->hotplug.stats[pin].state != HPD_MARK_DISABLED) > continue; > @@ -250,9 +259,7 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work) > > drm_connector_list_iter_begin(dev, &conn_iter); > for_each_intel_connector_iter(connector, &conn_iter) { > - /* Don't check MST ports, they don't have pins */ > - if (!connector->mst_port && > - intel_attached_encoder(connector)->hpd_pin == pin) { > + if (intel_connector_hpd_pin(connector) == pin) { > if (connector->base.polled != connector->polled) > DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n", > connector->base.name); > @@ -381,17 +388,20 @@ static void i915_hotplug_work_func(struct work_struct *work) > > drm_connector_list_iter_begin(dev, &conn_iter); > for_each_intel_connector_iter(connector, &conn_iter) { > - struct intel_encoder *encoder = > - intel_attached_encoder(connector); > + enum hpd_pin pin; > u32 hpd_bit; > > - if (!encoder) > + pin = intel_connector_hpd_pin(connector); > + if (pin == HPD_NONE) > continue; > > - hpd_bit = BIT(encoder->hpd_pin); > + hpd_bit = BIT(pin); > if ((hpd_event_bits | hpd_retry_bits) & hpd_bit) { > + struct intel_encoder *encoder = > + intel_attached_encoder(connector); > + > DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n", > - connector->base.name, encoder->hpd_pin); > + connector->base.name, pin); > > switch (encoder->hotplug(encoder, connector, > hpd_event_bits & hpd_bit)) { > @@ -606,20 +616,16 @@ static void i915_hpd_poll_init_work(struct work_struct *work) > > drm_connector_list_iter_begin(dev, &conn_iter); > for_each_intel_connector_iter(connector, &conn_iter) { > + enum hpd_pin pin = intel_connector_hpd_pin(connector); > + > connector->base.polled = connector->polled; > > - /* MST has a dynamic intel_connector->encoder and it's reprobing > - * is all handled by the MST helpers. */ > - if (connector->mst_port) > - continue; > - > - if (!connector->base.polled && I915_HAS_HOTPLUG(dev_priv) && > - intel_attached_encoder(connector)->hpd_pin > HPD_NONE) { > + if (pin != HPD_NONE && I915_HAS_HOTPLUG(dev_priv) && > + !connector->base.polled) > connector->base.polled = enabled ? > DRM_CONNECTOR_POLL_CONNECT | > DRM_CONNECTOR_POLL_DISCONNECT : > DRM_CONNECTOR_POLL_HPD; > - } > } > drm_connector_list_iter_end(&conn_iter);
diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c index 042d98bae1ea..8a3e9e901cf7 100644 --- a/drivers/gpu/drm/i915/display/intel_hotplug.c +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c @@ -120,6 +120,20 @@ enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv, #define HPD_STORM_REENABLE_DELAY (2 * 60 * 1000) #define HPD_RETRY_DELAY 1000 +static enum hpd_pin +intel_connector_hpd_pin(struct intel_connector *connector) +{ + struct intel_encoder *encoder = intel_attached_encoder(connector); + + /* + * MST connectors get their encoder attached dynamically + * so need to make sure we have an encoder here. But since + * MST encoders have their hpd_pin set to HPD_NONE we don't + * have to special case them beyond that. + */ + return encoder ? encoder->hpd_pin : HPD_NONE; +} + /** * intel_hpd_irq_storm_detect - gather stats and detect HPD IRQ storm on a pin * @dev_priv: private driver data pointer @@ -193,17 +207,12 @@ intel_hpd_irq_storm_switch_to_polling(struct drm_i915_private *dev_priv) drm_connector_list_iter_begin(dev, &conn_iter); for_each_intel_connector_iter(connector, &conn_iter) { - struct intel_encoder *encoder; enum hpd_pin pin; if (connector->base.polled != DRM_CONNECTOR_POLL_HPD) continue; - encoder = intel_attached_encoder(connector); - if (!encoder) - continue; - - pin = encoder->hpd_pin; + pin = intel_connector_hpd_pin(connector); if (pin == HPD_NONE || dev_priv->hotplug.stats[pin].state != HPD_MARK_DISABLED) continue; @@ -250,9 +259,7 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work) drm_connector_list_iter_begin(dev, &conn_iter); for_each_intel_connector_iter(connector, &conn_iter) { - /* Don't check MST ports, they don't have pins */ - if (!connector->mst_port && - intel_attached_encoder(connector)->hpd_pin == pin) { + if (intel_connector_hpd_pin(connector) == pin) { if (connector->base.polled != connector->polled) DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n", connector->base.name); @@ -381,17 +388,20 @@ static void i915_hotplug_work_func(struct work_struct *work) drm_connector_list_iter_begin(dev, &conn_iter); for_each_intel_connector_iter(connector, &conn_iter) { - struct intel_encoder *encoder = - intel_attached_encoder(connector); + enum hpd_pin pin; u32 hpd_bit; - if (!encoder) + pin = intel_connector_hpd_pin(connector); + if (pin == HPD_NONE) continue; - hpd_bit = BIT(encoder->hpd_pin); + hpd_bit = BIT(pin); if ((hpd_event_bits | hpd_retry_bits) & hpd_bit) { + struct intel_encoder *encoder = + intel_attached_encoder(connector); + DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n", - connector->base.name, encoder->hpd_pin); + connector->base.name, pin); switch (encoder->hotplug(encoder, connector, hpd_event_bits & hpd_bit)) { @@ -606,20 +616,16 @@ static void i915_hpd_poll_init_work(struct work_struct *work) drm_connector_list_iter_begin(dev, &conn_iter); for_each_intel_connector_iter(connector, &conn_iter) { + enum hpd_pin pin = intel_connector_hpd_pin(connector); + connector->base.polled = connector->polled; - /* MST has a dynamic intel_connector->encoder and it's reprobing - * is all handled by the MST helpers. */ - if (connector->mst_port) - continue; - - if (!connector->base.polled && I915_HAS_HOTPLUG(dev_priv) && - intel_attached_encoder(connector)->hpd_pin > HPD_NONE) { + if (pin != HPD_NONE && I915_HAS_HOTPLUG(dev_priv) && + !connector->base.polled) connector->base.polled = enabled ? DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT : DRM_CONNECTOR_POLL_HPD; - } } drm_connector_list_iter_end(&conn_iter);