On Wed, Jul 10, 2019 at 03:14:59PM -0700, José Roberto de Souza wrote:
> From: Imre Deak <imre.d...@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
> v4: Renamed INTEL_HOTPLUG_NOCHANGE to INTEL_HOTPLUG_UNCHANGED to keep
> it consistent(Rodrigo)
> 
> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.v...@intel.com>
> Signed-off-by: José Roberto de Souza <jose.so...@intel.com>
> Signed-off-by: Jani Nikula <jani.nik...@intel.com>
> Signed-off-by: Imre Deak <imre.d...@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 ad638e7f27bb..734c004800f8 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4047,14 +4047,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);
>  
> @@ -4076,7 +4078,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 0bdb7ecc5a81..4423abbc7907 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4853,14 +4853,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);
>  
> @@ -4879,7 +4881,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..2ca92780c659 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_UNCHANGED;
>  
>       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_UNCHANGED:
> +                             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 3fe8eaef6bd8..639317b84e51 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -1915,12 +1915,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 3e4f58f19362..6d4bc983915f 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4090,7 +4090,7 @@ static int i915_hpd_storm_ctl_show(struct seq_file *m, 
> void *data)
>        */
>       intel_synchronize_irq(dev_priv);
>       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 f9878cbef4d9..f471307aa704 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 24c63ed45c6f..e58d3751ed43 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_UNCHANGED,
> +     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 *);
> -- 
> 2.22.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to