| Message ID | 20190628213921.16879-1-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/2] drm/i915: Add support for retrying hotplug | expand |
On Fri, Jun 28, 2019 at 02:39:20PM -0700, José Roberto de Souza wrote: > From: Imre Deak <imre.deak@intel.com> > > There is some scenarios that we are aware that sink probe can fail, > so lets add the infrastructure to let hotplug() hook to request > another probe after some time. > > v2: Handle shared HPD pins (Imre) > v3: Rebased > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 12 ++-- > drivers/gpu/drm/i915/display/intel_dp.c | 12 ++-- > drivers/gpu/drm/i915/display/intel_hotplug.c | 59 +++++++++++++++----- > drivers/gpu/drm/i915/display/intel_hotplug.h | 5 +- > drivers/gpu/drm/i915/display/intel_sdvo.c | 8 ++- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 3 +- > drivers/gpu/drm/i915/intel_drv.h | 11 +++- > 8 files changed, 80 insertions(+), 32 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > index 7925a176f900..53009984e046 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -4081,14 +4081,16 @@ static int intel_hdmi_reset_link(struct intel_encoder *encoder, > return modeset_pipe(&crtc->base, ctx); > } > > -static bool intel_ddi_hotplug(struct intel_encoder *encoder, > - struct intel_connector *connector) > +static enum intel_hotplug_state > +intel_ddi_hotplug(struct intel_encoder *encoder, > + struct intel_connector *connector, > + bool irq_received) > { > struct drm_modeset_acquire_ctx ctx; > - bool changed; > + enum intel_hotplug_state state; > int ret; > > - changed = intel_encoder_hotplug(encoder, connector); > + state = intel_encoder_hotplug(encoder, connector, irq_received); > > drm_modeset_acquire_init(&ctx, 0); > > @@ -4110,7 +4112,7 @@ static bool intel_ddi_hotplug(struct intel_encoder *encoder, > drm_modeset_acquire_fini(&ctx); > WARN(ret, "Acquiring modeset locks failed with %i\n", ret); > > - return changed; > + return state; > } > > static struct intel_connector * > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 4336df46fe78..95d0da9d1bac 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -4878,14 +4878,16 @@ int intel_dp_retrain_link(struct intel_encoder *encoder, > * retrain the link to get a picture. That's in case no > * userspace component reacted to intermittent HPD dip. > */ > -static bool intel_dp_hotplug(struct intel_encoder *encoder, > - struct intel_connector *connector) > +static enum intel_hotplug_state > +intel_dp_hotplug(struct intel_encoder *encoder, > + struct intel_connector *connector, > + bool irq_received) > { > struct drm_modeset_acquire_ctx ctx; > - bool changed; > + enum intel_hotplug_state state; > int ret; > > - changed = intel_encoder_hotplug(encoder, connector); > + state = intel_encoder_hotplug(encoder, connector, irq_received); > > drm_modeset_acquire_init(&ctx, 0); > > @@ -4904,7 +4906,7 @@ static bool intel_dp_hotplug(struct intel_encoder *encoder, > drm_modeset_acquire_fini(&ctx); > WARN(ret, "Acquiring modeset locks failed with %i\n", ret); > > - return changed; > + return state; > } > > static void intel_dp_check_service_irq(struct intel_dp *intel_dp) > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c > index ea3de4acc850..3662966d366e 100644 > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c > @@ -112,6 +112,7 @@ enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv, > > #define HPD_STORM_DETECT_PERIOD 1000 > #define HPD_STORM_REENABLE_DELAY (2 * 60 * 1000) > +#define HPD_RETRY_DELAY 1000 > > /** > * intel_hpd_irq_storm_detect - gather stats and detect HPD IRQ storm on a pin > @@ -266,8 +267,10 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work) > intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref); > } > > -bool intel_encoder_hotplug(struct intel_encoder *encoder, > - struct intel_connector *connector) > +enum intel_hotplug_state > +intel_encoder_hotplug(struct intel_encoder *encoder, > + struct intel_connector *connector, > + bool irq_received) > { > struct drm_device *dev = connector->base.dev; > enum drm_connector_status old_status; > @@ -279,7 +282,7 @@ bool intel_encoder_hotplug(struct intel_encoder *encoder, > drm_helper_probe_detect(&connector->base, NULL, false); > > if (old_status == connector->base.status) > - return false; > + return INTEL_HOTPLUG_NOCHANGE; > > DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n", > connector->base.base.id, > @@ -287,7 +290,7 @@ bool intel_encoder_hotplug(struct intel_encoder *encoder, > drm_get_connector_status_name(old_status), > drm_get_connector_status_name(connector->base.status)); > > - return true; > + return INTEL_HOTPLUG_CHANGED; > } > > static bool intel_encoder_has_hpd_pulse(struct intel_encoder *encoder) > @@ -339,7 +342,7 @@ static void i915_digport_work_func(struct work_struct *work) > spin_lock_irq(&dev_priv->irq_lock); > dev_priv->hotplug.event_bits |= old_bits; > spin_unlock_irq(&dev_priv->irq_lock); > - schedule_work(&dev_priv->hotplug.hotplug_work); > + schedule_delayed_work(&dev_priv->hotplug.hotplug_work, 0); > } > } > > @@ -349,14 +352,16 @@ static void i915_digport_work_func(struct work_struct *work) > static void i915_hotplug_work_func(struct work_struct *work) > { > struct drm_i915_private *dev_priv = > - container_of(work, struct drm_i915_private, hotplug.hotplug_work); > + container_of(work, struct drm_i915_private, > + hotplug.hotplug_work.work); > struct drm_device *dev = &dev_priv->drm; > struct intel_connector *intel_connector; > struct intel_encoder *intel_encoder; > struct drm_connector *connector; > struct drm_connector_list_iter conn_iter; > - bool changed = false; > + u32 changed = 0, retry = 0; > u32 hpd_event_bits; > + u32 hpd_retry_bits; > > mutex_lock(&dev->mode_config.mutex); > DRM_DEBUG_KMS("running encoder hotplug functions\n"); > @@ -365,6 +370,8 @@ static void i915_hotplug_work_func(struct work_struct *work) > > hpd_event_bits = dev_priv->hotplug.event_bits; > dev_priv->hotplug.event_bits = 0; > + hpd_retry_bits = dev_priv->hotplug.retry_bits; > + dev_priv->hotplug.retry_bits = 0; > > /* Enable polling for connectors which had HPD IRQ storms */ > intel_hpd_irq_storm_switch_to_polling(dev_priv); > @@ -373,16 +380,29 @@ static void i915_hotplug_work_func(struct work_struct *work) > > drm_connector_list_iter_begin(dev, &conn_iter); > drm_for_each_connector_iter(connector, &conn_iter) { > + u32 hpd_bit; > + > intel_connector = to_intel_connector(connector); > if (!intel_connector->encoder) > continue; > intel_encoder = intel_connector->encoder; > - if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) { > + hpd_bit = BIT(intel_encoder->hpd_pin); > + if ((hpd_event_bits | hpd_retry_bits) & hpd_bit) { > DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n", > connector->name, intel_encoder->hpd_pin); > > - changed |= intel_encoder->hotplug(intel_encoder, > - intel_connector); > + switch (intel_encoder->hotplug(intel_encoder, > + intel_connector, > + hpd_event_bits & hpd_bit)) { > + case INTEL_HOTPLUG_NOCHANGE: > + break; > + case INTEL_HOTPLUG_CHANGED: > + changed |= hpd_bit; > + break; > + case INTEL_HOTPLUG_RETRY: > + retry |= hpd_bit; > + break; > + } > } > } > drm_connector_list_iter_end(&conn_iter); > @@ -390,6 +410,17 @@ static void i915_hotplug_work_func(struct work_struct *work) > > if (changed) > drm_kms_helper_hotplug_event(dev); > + > + /* Remove shared HPD pins that have changed */ > + retry &= ~changed; > + if (retry) { > + spin_lock_irq(&dev_priv->irq_lock); > + dev_priv->hotplug.retry_bits |= retry; > + spin_unlock_irq(&dev_priv->irq_lock); > + > + mod_delayed_work(system_wq, &dev_priv->hotplug.hotplug_work, > + msecs_to_jiffies(HPD_RETRY_DELAY)); > + } > } > > > @@ -516,7 +547,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv, > if (queue_dig) > queue_work(dev_priv->hotplug.dp_wq, &dev_priv->hotplug.dig_port_work); > if (queue_hp) > - schedule_work(&dev_priv->hotplug.hotplug_work); > + schedule_delayed_work(&dev_priv->hotplug.hotplug_work, 0); > } > > /** > @@ -636,7 +667,8 @@ void intel_hpd_poll_init(struct drm_i915_private *dev_priv) > > void intel_hpd_init_work(struct drm_i915_private *dev_priv) > { > - INIT_WORK(&dev_priv->hotplug.hotplug_work, i915_hotplug_work_func); > + INIT_DELAYED_WORK(&dev_priv->hotplug.hotplug_work, > + i915_hotplug_work_func); > INIT_WORK(&dev_priv->hotplug.dig_port_work, i915_digport_work_func); > INIT_WORK(&dev_priv->hotplug.poll_init_work, i915_hpd_poll_init_work); > INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work, > @@ -650,11 +682,12 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv) > dev_priv->hotplug.long_port_mask = 0; > dev_priv->hotplug.short_port_mask = 0; > dev_priv->hotplug.event_bits = 0; > + dev_priv->hotplug.retry_bits = 0; > > spin_unlock_irq(&dev_priv->irq_lock); > > cancel_work_sync(&dev_priv->hotplug.dig_port_work); > - cancel_work_sync(&dev_priv->hotplug.hotplug_work); > + cancel_delayed_work_sync(&dev_priv->hotplug.hotplug_work); > cancel_work_sync(&dev_priv->hotplug.poll_init_work); > cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work); > } > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h > index 805f897dbb7a..b0cd447b7fbc 100644 > --- a/drivers/gpu/drm/i915/display/intel_hotplug.h > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h > @@ -15,8 +15,9 @@ struct intel_connector; > struct intel_encoder; > > void intel_hpd_poll_init(struct drm_i915_private *dev_priv); > -bool intel_encoder_hotplug(struct intel_encoder *encoder, > - struct intel_connector *connector); > +enum intel_hotplug_state intel_encoder_hotplug(struct intel_encoder *encoder, > + struct intel_connector *connector, > + bool irq_received); > void intel_hpd_irq_handler(struct drm_i915_private *dev_priv, > u32 pin_mask, u32 long_mask); > void intel_hpd_init(struct drm_i915_private *dev_priv); > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c > index ceda03e5a3d4..2855f14f5746 100644 > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c > @@ -1893,12 +1893,14 @@ static void intel_sdvo_enable_hotplug(struct intel_encoder *encoder) > &intel_sdvo->hotplug_active, 2); > } > > -static bool intel_sdvo_hotplug(struct intel_encoder *encoder, > - struct intel_connector *connector) > +static enum intel_hotplug_state > +intel_sdvo_hotplug(struct intel_encoder *encoder, > + struct intel_connector *connector, > + bool irq_received) > { > intel_sdvo_enable_hotplug(encoder); > > - return intel_encoder_hotplug(encoder, connector); > + return intel_encoder_hotplug(encoder, connector, irq_received); > } > > static bool > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index eeecdad0e3ca..836709ee9ff7 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -4083,7 +4083,7 @@ static int i915_hpd_storm_ctl_show(struct seq_file *m, void *data) > */ > synchronize_irq(dev_priv->drm.irq); > flush_work(&dev_priv->hotplug.dig_port_work); > - flush_work(&dev_priv->hotplug.hotplug_work); > + flush_delayed_work(&dev_priv->hotplug.hotplug_work); > > seq_printf(m, "Threshold: %d\n", hotplug->hpd_storm_threshold); > seq_printf(m, "Detected: %s\n", > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 7e981b03face..d240997912d0 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -163,7 +163,7 @@ enum hpd_pin { > #define HPD_STORM_DEFAULT_THRESHOLD 50 > > struct i915_hotplug { > - struct work_struct hotplug_work; > + struct delayed_work hotplug_work; > > struct { > unsigned long last_jiffies; > @@ -175,6 +175,7 @@ struct i915_hotplug { > } state; > } stats[HPD_NUM_PINS]; > u32 event_bits; > + u32 retry_bits; > struct delayed_work reenable_work; > > u32 long_port_mask; > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 1d58f7ec5d84..0088eb2f510e 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -101,14 +101,21 @@ struct intel_fbdev { > struct mutex hpd_lock; > }; > > +enum intel_hotplug_state { > + INTEL_HOTPLUG_NOCHANGE, > + INTEL_HOTPLUG_CHANGED, my only bikeshed would be to keep this symmetric... no-change change or preferably unchanged changed. But everything seems right so let's move Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > + INTEL_HOTPLUG_RETRY, > +}; > + > struct intel_encoder { > struct drm_encoder base; > > enum intel_output_type type; > enum port port; > unsigned int cloneable; > - bool (*hotplug)(struct intel_encoder *encoder, > - struct intel_connector *connector); > + enum intel_hotplug_state (*hotplug)(struct intel_encoder *encoder, > + struct intel_connector *connector, > + bool irq_received); > enum intel_output_type (*compute_output_type)(struct intel_encoder *, > struct intel_crtc_state *, > struct drm_connector_state *); > -- > 2.22.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, 2019-07-10 at 04:20 -0700, Rodrigo Vivi wrote: > On Fri, Jun 28, 2019 at 02:39:20PM -0700, José Roberto de Souza > wrote: > > From: Imre Deak <imre.deak@intel.com> > > > > There is some scenarios that we are aware that sink probe can fail, > > so lets add the infrastructure to let hotplug() hook to request > > another probe after some time. > > > > v2: Handle shared HPD pins (Imre) > > v3: Rebased > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_ddi.c | 12 ++-- > > drivers/gpu/drm/i915/display/intel_dp.c | 12 ++-- > > drivers/gpu/drm/i915/display/intel_hotplug.c | 59 +++++++++++++++- > > ---- > > drivers/gpu/drm/i915/display/intel_hotplug.h | 5 +- > > drivers/gpu/drm/i915/display/intel_sdvo.c | 8 ++- > > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > > drivers/gpu/drm/i915/i915_drv.h | 3 +- > > drivers/gpu/drm/i915/intel_drv.h | 11 +++- > > 8 files changed, 80 insertions(+), 32 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > > b/drivers/gpu/drm/i915/display/intel_ddi.c > > index 7925a176f900..53009984e046 100644 > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > @@ -4081,14 +4081,16 @@ static int intel_hdmi_reset_link(struct > > intel_encoder *encoder, > > return modeset_pipe(&crtc->base, ctx); > > } > > > > -static bool intel_ddi_hotplug(struct intel_encoder *encoder, > > - struct intel_connector *connector) > > +static enum intel_hotplug_state > > +intel_ddi_hotplug(struct intel_encoder *encoder, > > + struct intel_connector *connector, > > + bool irq_received) > > { > > struct drm_modeset_acquire_ctx ctx; > > - bool changed; > > + enum intel_hotplug_state state; > > int ret; > > > > - changed = intel_encoder_hotplug(encoder, connector); > > + state = intel_encoder_hotplug(encoder, connector, > > irq_received); > > > > drm_modeset_acquire_init(&ctx, 0); > > > > @@ -4110,7 +4112,7 @@ static bool intel_ddi_hotplug(struct > > intel_encoder *encoder, > > drm_modeset_acquire_fini(&ctx); > > WARN(ret, "Acquiring modeset locks failed with %i\n", ret); > > > > - return changed; > > + return state; > > } > > > > static struct intel_connector * > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > b/drivers/gpu/drm/i915/display/intel_dp.c > > index 4336df46fe78..95d0da9d1bac 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -4878,14 +4878,16 @@ int intel_dp_retrain_link(struct > > intel_encoder *encoder, > > * retrain the link to get a picture. That's in case no > > * userspace component reacted to intermittent HPD dip. > > */ > > -static bool intel_dp_hotplug(struct intel_encoder *encoder, > > - struct intel_connector *connector) > > +static enum intel_hotplug_state > > +intel_dp_hotplug(struct intel_encoder *encoder, > > + struct intel_connector *connector, > > + bool irq_received) > > { > > struct drm_modeset_acquire_ctx ctx; > > - bool changed; > > + enum intel_hotplug_state state; > > int ret; > > > > - changed = intel_encoder_hotplug(encoder, connector); > > + state = intel_encoder_hotplug(encoder, connector, > > irq_received); > > > > drm_modeset_acquire_init(&ctx, 0); > > > > @@ -4904,7 +4906,7 @@ static bool intel_dp_hotplug(struct > > intel_encoder *encoder, > > drm_modeset_acquire_fini(&ctx); > > WARN(ret, "Acquiring modeset locks failed with %i\n", ret); > > > > - return changed; > > + return state; > > } > > > > static void intel_dp_check_service_irq(struct intel_dp *intel_dp) > > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c > > b/drivers/gpu/drm/i915/display/intel_hotplug.c > > index ea3de4acc850..3662966d366e 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c > > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c > > @@ -112,6 +112,7 @@ enum hpd_pin intel_hpd_pin_default(struct > > drm_i915_private *dev_priv, > > > > #define HPD_STORM_DETECT_PERIOD 1000 > > #define HPD_STORM_REENABLE_DELAY (2 * 60 * 1000) > > +#define HPD_RETRY_DELAY 1000 > > > > /** > > * intel_hpd_irq_storm_detect - gather stats and detect HPD IRQ > > storm on a pin > > @@ -266,8 +267,10 @@ static void > > intel_hpd_irq_storm_reenable_work(struct work_struct *work) > > intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref); > > } > > > > -bool intel_encoder_hotplug(struct intel_encoder *encoder, > > - struct intel_connector *connector) > > +enum intel_hotplug_state > > +intel_encoder_hotplug(struct intel_encoder *encoder, > > + struct intel_connector *connector, > > + bool irq_received) > > { > > struct drm_device *dev = connector->base.dev; > > enum drm_connector_status old_status; > > @@ -279,7 +282,7 @@ bool intel_encoder_hotplug(struct intel_encoder > > *encoder, > > drm_helper_probe_detect(&connector->base, NULL, false); > > > > if (old_status == connector->base.status) > > - return false; > > + return INTEL_HOTPLUG_NOCHANGE; > > > > DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to > > %s\n", > > connector->base.base.id, > > @@ -287,7 +290,7 @@ bool intel_encoder_hotplug(struct intel_encoder > > *encoder, > > drm_get_connector_status_name(old_status), > > drm_get_connector_status_name(connector- > > >base.status)); > > > > - return true; > > + return INTEL_HOTPLUG_CHANGED; > > } > > > > static bool intel_encoder_has_hpd_pulse(struct intel_encoder > > *encoder) > > @@ -339,7 +342,7 @@ static void i915_digport_work_func(struct > > work_struct *work) > > spin_lock_irq(&dev_priv->irq_lock); > > dev_priv->hotplug.event_bits |= old_bits; > > spin_unlock_irq(&dev_priv->irq_lock); > > - schedule_work(&dev_priv->hotplug.hotplug_work); > > + schedule_delayed_work(&dev_priv->hotplug.hotplug_work, > > 0); > > } > > } > > > > @@ -349,14 +352,16 @@ static void i915_digport_work_func(struct > > work_struct *work) > > static void i915_hotplug_work_func(struct work_struct *work) > > { > > struct drm_i915_private *dev_priv = > > - container_of(work, struct drm_i915_private, > > hotplug.hotplug_work); > > + container_of(work, struct drm_i915_private, > > + hotplug.hotplug_work.work); > > struct drm_device *dev = &dev_priv->drm; > > struct intel_connector *intel_connector; > > struct intel_encoder *intel_encoder; > > struct drm_connector *connector; > > struct drm_connector_list_iter conn_iter; > > - bool changed = false; > > + u32 changed = 0, retry = 0; > > u32 hpd_event_bits; > > + u32 hpd_retry_bits; > > > > mutex_lock(&dev->mode_config.mutex); > > DRM_DEBUG_KMS("running encoder hotplug functions\n"); > > @@ -365,6 +370,8 @@ static void i915_hotplug_work_func(struct > > work_struct *work) > > > > hpd_event_bits = dev_priv->hotplug.event_bits; > > dev_priv->hotplug.event_bits = 0; > > + hpd_retry_bits = dev_priv->hotplug.retry_bits; > > + dev_priv->hotplug.retry_bits = 0; > > > > /* Enable polling for connectors which had HPD IRQ storms */ > > intel_hpd_irq_storm_switch_to_polling(dev_priv); > > @@ -373,16 +380,29 @@ static void i915_hotplug_work_func(struct > > work_struct *work) > > > > drm_connector_list_iter_begin(dev, &conn_iter); > > drm_for_each_connector_iter(connector, &conn_iter) { > > + u32 hpd_bit; > > + > > intel_connector = to_intel_connector(connector); > > if (!intel_connector->encoder) > > continue; > > intel_encoder = intel_connector->encoder; > > - if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) { > > + hpd_bit = BIT(intel_encoder->hpd_pin); > > + if ((hpd_event_bits | hpd_retry_bits) & hpd_bit) { > > DRM_DEBUG_KMS("Connector %s (pin %i) received > > hotplug event.\n", > > connector->name, intel_encoder- > > >hpd_pin); > > > > - changed |= intel_encoder- > > >hotplug(intel_encoder, > > - intel_connect > > or); > > + switch (intel_encoder->hotplug(intel_encoder, > > + intel_connector, > > + hpd_event_bits & > > hpd_bit)) { > > + case INTEL_HOTPLUG_NOCHANGE: > > + break; > > + case INTEL_HOTPLUG_CHANGED: > > + changed |= hpd_bit; > > + break; > > + case INTEL_HOTPLUG_RETRY: > > + retry |= hpd_bit; > > + break; > > + } > > } > > } > > drm_connector_list_iter_end(&conn_iter); > > @@ -390,6 +410,17 @@ static void i915_hotplug_work_func(struct > > work_struct *work) > > > > if (changed) > > drm_kms_helper_hotplug_event(dev); > > + > > + /* Remove shared HPD pins that have changed */ > > + retry &= ~changed; > > + if (retry) { > > + spin_lock_irq(&dev_priv->irq_lock); > > + dev_priv->hotplug.retry_bits |= retry; > > + spin_unlock_irq(&dev_priv->irq_lock); > > + > > + mod_delayed_work(system_wq, &dev_priv- > > >hotplug.hotplug_work, > > + msecs_to_jiffies(HPD_RETRY_DELAY)); > > + } > > } > > > > > > @@ -516,7 +547,7 @@ void intel_hpd_irq_handler(struct > > drm_i915_private *dev_priv, > > if (queue_dig) > > queue_work(dev_priv->hotplug.dp_wq, &dev_priv- > > >hotplug.dig_port_work); > > if (queue_hp) > > - schedule_work(&dev_priv->hotplug.hotplug_work); > > + schedule_delayed_work(&dev_priv->hotplug.hotplug_work, > > 0); > > } > > > > /** > > @@ -636,7 +667,8 @@ void intel_hpd_poll_init(struct > > drm_i915_private *dev_priv) > > > > void intel_hpd_init_work(struct drm_i915_private *dev_priv) > > { > > - INIT_WORK(&dev_priv->hotplug.hotplug_work, > > i915_hotplug_work_func); > > + INIT_DELAYED_WORK(&dev_priv->hotplug.hotplug_work, > > + i915_hotplug_work_func); > > INIT_WORK(&dev_priv->hotplug.dig_port_work, > > i915_digport_work_func); > > INIT_WORK(&dev_priv->hotplug.poll_init_work, > > i915_hpd_poll_init_work); > > INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work, > > @@ -650,11 +682,12 @@ void intel_hpd_cancel_work(struct > > drm_i915_private *dev_priv) > > dev_priv->hotplug.long_port_mask = 0; > > dev_priv->hotplug.short_port_mask = 0; > > dev_priv->hotplug.event_bits = 0; > > + dev_priv->hotplug.retry_bits = 0; > > > > spin_unlock_irq(&dev_priv->irq_lock); > > > > cancel_work_sync(&dev_priv->hotplug.dig_port_work); > > - cancel_work_sync(&dev_priv->hotplug.hotplug_work); > > + cancel_delayed_work_sync(&dev_priv->hotplug.hotplug_work); > > cancel_work_sync(&dev_priv->hotplug.poll_init_work); > > cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work); > > } > > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h > > b/drivers/gpu/drm/i915/display/intel_hotplug.h > > index 805f897dbb7a..b0cd447b7fbc 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hotplug.h > > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h > > @@ -15,8 +15,9 @@ struct intel_connector; > > struct intel_encoder; > > > > void intel_hpd_poll_init(struct drm_i915_private *dev_priv); > > -bool intel_encoder_hotplug(struct intel_encoder *encoder, > > - struct intel_connector *connector); > > +enum intel_hotplug_state intel_encoder_hotplug(struct > > intel_encoder *encoder, > > + struct intel_connector > > *connector, > > + bool irq_received); > > void intel_hpd_irq_handler(struct drm_i915_private *dev_priv, > > u32 pin_mask, u32 long_mask); > > void intel_hpd_init(struct drm_i915_private *dev_priv); > > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c > > b/drivers/gpu/drm/i915/display/intel_sdvo.c > > index ceda03e5a3d4..2855f14f5746 100644 > > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c > > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c > > @@ -1893,12 +1893,14 @@ static void > > intel_sdvo_enable_hotplug(struct intel_encoder *encoder) > > &intel_sdvo->hotplug_active, 2); > > } > > > > -static bool intel_sdvo_hotplug(struct intel_encoder *encoder, > > - struct intel_connector *connector) > > +static enum intel_hotplug_state > > +intel_sdvo_hotplug(struct intel_encoder *encoder, > > + struct intel_connector *connector, > > + bool irq_received) > > { > > intel_sdvo_enable_hotplug(encoder); > > > > - return intel_encoder_hotplug(encoder, connector); > > + return intel_encoder_hotplug(encoder, connector, irq_received); > > } > > > > static bool > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > b/drivers/gpu/drm/i915/i915_debugfs.c > > index eeecdad0e3ca..836709ee9ff7 100644 > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > @@ -4083,7 +4083,7 @@ static int i915_hpd_storm_ctl_show(struct > > seq_file *m, void *data) > > */ > > synchronize_irq(dev_priv->drm.irq); > > flush_work(&dev_priv->hotplug.dig_port_work); > > - flush_work(&dev_priv->hotplug.hotplug_work); > > + flush_delayed_work(&dev_priv->hotplug.hotplug_work); > > > > seq_printf(m, "Threshold: %d\n", hotplug->hpd_storm_threshold); > > seq_printf(m, "Detected: %s\n", > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 7e981b03face..d240997912d0 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -163,7 +163,7 @@ enum hpd_pin { > > #define HPD_STORM_DEFAULT_THRESHOLD 50 > > > > struct i915_hotplug { > > - struct work_struct hotplug_work; > > + struct delayed_work hotplug_work; > > > > struct { > > unsigned long last_jiffies; > > @@ -175,6 +175,7 @@ struct i915_hotplug { > > } state; > > } stats[HPD_NUM_PINS]; > > u32 event_bits; > > + u32 retry_bits; > > struct delayed_work reenable_work; > > > > u32 long_port_mask; > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > b/drivers/gpu/drm/i915/intel_drv.h > > index 1d58f7ec5d84..0088eb2f510e 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -101,14 +101,21 @@ struct intel_fbdev { > > struct mutex hpd_lock; > > }; > > > > +enum intel_hotplug_state { > > + INTEL_HOTPLUG_NOCHANGE, > > + INTEL_HOTPLUG_CHANGED, > > my only bikeshed would be to keep this symmetric... > no-change change or preferably unchanged changed. > > But everything seems right so let's move Just sent a new version replacing nochange to unchanged and added your r-v-b. Thanks > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > + INTEL_HOTPLUG_RETRY, > > +}; > > + > > struct intel_encoder { > > struct drm_encoder base; > > > > enum intel_output_type type; > > enum port port; > > unsigned int cloneable; > > - bool (*hotplug)(struct intel_encoder *encoder, > > - struct intel_connector *connector); > > + enum intel_hotplug_state (*hotplug)(struct intel_encoder > > *encoder, > > + struct intel_connector > > *connector, > > + bool irq_received); > > enum intel_output_type (*compute_output_type)(struct > > intel_encoder *, > > struct > > intel_crtc_state *, > > struct > > drm_connector_state *); > > -- > > 2.22.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 7925a176f900..53009984e046 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -4081,14 +4081,16 @@ static int intel_hdmi_reset_link(struct intel_encoder *encoder, return modeset_pipe(&crtc->base, ctx); } -static bool intel_ddi_hotplug(struct intel_encoder *encoder, - struct intel_connector *connector) +static enum intel_hotplug_state +intel_ddi_hotplug(struct intel_encoder *encoder, + struct intel_connector *connector, + bool irq_received) { struct drm_modeset_acquire_ctx ctx; - bool changed; + enum intel_hotplug_state state; int ret; - changed = intel_encoder_hotplug(encoder, connector); + state = intel_encoder_hotplug(encoder, connector, irq_received); drm_modeset_acquire_init(&ctx, 0); @@ -4110,7 +4112,7 @@ static bool intel_ddi_hotplug(struct intel_encoder *encoder, drm_modeset_acquire_fini(&ctx); WARN(ret, "Acquiring modeset locks failed with %i\n", ret); - return changed; + return state; } static struct intel_connector * diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 4336df46fe78..95d0da9d1bac 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -4878,14 +4878,16 @@ int intel_dp_retrain_link(struct intel_encoder *encoder, * retrain the link to get a picture. That's in case no * userspace component reacted to intermittent HPD dip. */ -static bool intel_dp_hotplug(struct intel_encoder *encoder, - struct intel_connector *connector) +static enum intel_hotplug_state +intel_dp_hotplug(struct intel_encoder *encoder, + struct intel_connector *connector, + bool irq_received) { struct drm_modeset_acquire_ctx ctx; - bool changed; + enum intel_hotplug_state state; int ret; - changed = intel_encoder_hotplug(encoder, connector); + state = intel_encoder_hotplug(encoder, connector, irq_received); drm_modeset_acquire_init(&ctx, 0); @@ -4904,7 +4906,7 @@ static bool intel_dp_hotplug(struct intel_encoder *encoder, drm_modeset_acquire_fini(&ctx); WARN(ret, "Acquiring modeset locks failed with %i\n", ret); - return changed; + return state; } static void intel_dp_check_service_irq(struct intel_dp *intel_dp) diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c index ea3de4acc850..3662966d366e 100644 --- a/drivers/gpu/drm/i915/display/intel_hotplug.c +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c @@ -112,6 +112,7 @@ enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv, #define HPD_STORM_DETECT_PERIOD 1000 #define HPD_STORM_REENABLE_DELAY (2 * 60 * 1000) +#define HPD_RETRY_DELAY 1000 /** * intel_hpd_irq_storm_detect - gather stats and detect HPD IRQ storm on a pin @@ -266,8 +267,10 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work) intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref); } -bool intel_encoder_hotplug(struct intel_encoder *encoder, - struct intel_connector *connector) +enum intel_hotplug_state +intel_encoder_hotplug(struct intel_encoder *encoder, + struct intel_connector *connector, + bool irq_received) { struct drm_device *dev = connector->base.dev; enum drm_connector_status old_status; @@ -279,7 +282,7 @@ bool intel_encoder_hotplug(struct intel_encoder *encoder, drm_helper_probe_detect(&connector->base, NULL, false); if (old_status == connector->base.status) - return false; + return INTEL_HOTPLUG_NOCHANGE; DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n", connector->base.base.id, @@ -287,7 +290,7 @@ bool intel_encoder_hotplug(struct intel_encoder *encoder, drm_get_connector_status_name(old_status), drm_get_connector_status_name(connector->base.status)); - return true; + return INTEL_HOTPLUG_CHANGED; } static bool intel_encoder_has_hpd_pulse(struct intel_encoder *encoder) @@ -339,7 +342,7 @@ static void i915_digport_work_func(struct work_struct *work) spin_lock_irq(&dev_priv->irq_lock); dev_priv->hotplug.event_bits |= old_bits; spin_unlock_irq(&dev_priv->irq_lock); - schedule_work(&dev_priv->hotplug.hotplug_work); + schedule_delayed_work(&dev_priv->hotplug.hotplug_work, 0); } } @@ -349,14 +352,16 @@ static void i915_digport_work_func(struct work_struct *work) static void i915_hotplug_work_func(struct work_struct *work) { struct drm_i915_private *dev_priv = - container_of(work, struct drm_i915_private, hotplug.hotplug_work); + container_of(work, struct drm_i915_private, + hotplug.hotplug_work.work); struct drm_device *dev = &dev_priv->drm; struct intel_connector *intel_connector; struct intel_encoder *intel_encoder; struct drm_connector *connector; struct drm_connector_list_iter conn_iter; - bool changed = false; + u32 changed = 0, retry = 0; u32 hpd_event_bits; + u32 hpd_retry_bits; mutex_lock(&dev->mode_config.mutex); DRM_DEBUG_KMS("running encoder hotplug functions\n"); @@ -365,6 +370,8 @@ static void i915_hotplug_work_func(struct work_struct *work) hpd_event_bits = dev_priv->hotplug.event_bits; dev_priv->hotplug.event_bits = 0; + hpd_retry_bits = dev_priv->hotplug.retry_bits; + dev_priv->hotplug.retry_bits = 0; /* Enable polling for connectors which had HPD IRQ storms */ intel_hpd_irq_storm_switch_to_polling(dev_priv); @@ -373,16 +380,29 @@ static void i915_hotplug_work_func(struct work_struct *work) drm_connector_list_iter_begin(dev, &conn_iter); drm_for_each_connector_iter(connector, &conn_iter) { + u32 hpd_bit; + intel_connector = to_intel_connector(connector); if (!intel_connector->encoder) continue; intel_encoder = intel_connector->encoder; - if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) { + hpd_bit = BIT(intel_encoder->hpd_pin); + if ((hpd_event_bits | hpd_retry_bits) & hpd_bit) { DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n", connector->name, intel_encoder->hpd_pin); - changed |= intel_encoder->hotplug(intel_encoder, - intel_connector); + switch (intel_encoder->hotplug(intel_encoder, + intel_connector, + hpd_event_bits & hpd_bit)) { + case INTEL_HOTPLUG_NOCHANGE: + break; + case INTEL_HOTPLUG_CHANGED: + changed |= hpd_bit; + break; + case INTEL_HOTPLUG_RETRY: + retry |= hpd_bit; + break; + } } } drm_connector_list_iter_end(&conn_iter); @@ -390,6 +410,17 @@ static void i915_hotplug_work_func(struct work_struct *work) if (changed) drm_kms_helper_hotplug_event(dev); + + /* Remove shared HPD pins that have changed */ + retry &= ~changed; + if (retry) { + spin_lock_irq(&dev_priv->irq_lock); + dev_priv->hotplug.retry_bits |= retry; + spin_unlock_irq(&dev_priv->irq_lock); + + mod_delayed_work(system_wq, &dev_priv->hotplug.hotplug_work, + msecs_to_jiffies(HPD_RETRY_DELAY)); + } } @@ -516,7 +547,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv, if (queue_dig) queue_work(dev_priv->hotplug.dp_wq, &dev_priv->hotplug.dig_port_work); if (queue_hp) - schedule_work(&dev_priv->hotplug.hotplug_work); + schedule_delayed_work(&dev_priv->hotplug.hotplug_work, 0); } /** @@ -636,7 +667,8 @@ void intel_hpd_poll_init(struct drm_i915_private *dev_priv) void intel_hpd_init_work(struct drm_i915_private *dev_priv) { - INIT_WORK(&dev_priv->hotplug.hotplug_work, i915_hotplug_work_func); + INIT_DELAYED_WORK(&dev_priv->hotplug.hotplug_work, + i915_hotplug_work_func); INIT_WORK(&dev_priv->hotplug.dig_port_work, i915_digport_work_func); INIT_WORK(&dev_priv->hotplug.poll_init_work, i915_hpd_poll_init_work); INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work, @@ -650,11 +682,12 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv) dev_priv->hotplug.long_port_mask = 0; dev_priv->hotplug.short_port_mask = 0; dev_priv->hotplug.event_bits = 0; + dev_priv->hotplug.retry_bits = 0; spin_unlock_irq(&dev_priv->irq_lock); cancel_work_sync(&dev_priv->hotplug.dig_port_work); - cancel_work_sync(&dev_priv->hotplug.hotplug_work); + cancel_delayed_work_sync(&dev_priv->hotplug.hotplug_work); cancel_work_sync(&dev_priv->hotplug.poll_init_work); cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work); } diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h index 805f897dbb7a..b0cd447b7fbc 100644 --- a/drivers/gpu/drm/i915/display/intel_hotplug.h +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h @@ -15,8 +15,9 @@ struct intel_connector; struct intel_encoder; void intel_hpd_poll_init(struct drm_i915_private *dev_priv); -bool intel_encoder_hotplug(struct intel_encoder *encoder, - struct intel_connector *connector); +enum intel_hotplug_state intel_encoder_hotplug(struct intel_encoder *encoder, + struct intel_connector *connector, + bool irq_received); void intel_hpd_irq_handler(struct drm_i915_private *dev_priv, u32 pin_mask, u32 long_mask); void intel_hpd_init(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c index ceda03e5a3d4..2855f14f5746 100644 --- a/drivers/gpu/drm/i915/display/intel_sdvo.c +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c @@ -1893,12 +1893,14 @@ static void intel_sdvo_enable_hotplug(struct intel_encoder *encoder) &intel_sdvo->hotplug_active, 2); } -static bool intel_sdvo_hotplug(struct intel_encoder *encoder, - struct intel_connector *connector) +static enum intel_hotplug_state +intel_sdvo_hotplug(struct intel_encoder *encoder, + struct intel_connector *connector, + bool irq_received) { intel_sdvo_enable_hotplug(encoder); - return intel_encoder_hotplug(encoder, connector); + return intel_encoder_hotplug(encoder, connector, irq_received); } static bool diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index eeecdad0e3ca..836709ee9ff7 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -4083,7 +4083,7 @@ static int i915_hpd_storm_ctl_show(struct seq_file *m, void *data) */ synchronize_irq(dev_priv->drm.irq); flush_work(&dev_priv->hotplug.dig_port_work); - flush_work(&dev_priv->hotplug.hotplug_work); + flush_delayed_work(&dev_priv->hotplug.hotplug_work); seq_printf(m, "Threshold: %d\n", hotplug->hpd_storm_threshold); seq_printf(m, "Detected: %s\n", diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 7e981b03face..d240997912d0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -163,7 +163,7 @@ enum hpd_pin { #define HPD_STORM_DEFAULT_THRESHOLD 50 struct i915_hotplug { - struct work_struct hotplug_work; + struct delayed_work hotplug_work; struct { unsigned long last_jiffies; @@ -175,6 +175,7 @@ struct i915_hotplug { } state; } stats[HPD_NUM_PINS]; u32 event_bits; + u32 retry_bits; struct delayed_work reenable_work; u32 long_port_mask; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 1d58f7ec5d84..0088eb2f510e 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -101,14 +101,21 @@ struct intel_fbdev { struct mutex hpd_lock; }; +enum intel_hotplug_state { + INTEL_HOTPLUG_NOCHANGE, + INTEL_HOTPLUG_CHANGED, + INTEL_HOTPLUG_RETRY, +}; + struct intel_encoder { struct drm_encoder base; enum intel_output_type type; enum port port; unsigned int cloneable; - bool (*hotplug)(struct intel_encoder *encoder, - struct intel_connector *connector); + enum intel_hotplug_state (*hotplug)(struct intel_encoder *encoder, + struct intel_connector *connector, + bool irq_received); enum intel_output_type (*compute_output_type)(struct intel_encoder *, struct intel_crtc_state *, struct drm_connector_state *);