Re: [PATCH 1/2] drm/i915: Add support for retrying hotplug

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 22 Feb 2019, José Roberto de Souza <jose.souza@xxxxxxxxx> wrote:
> From: Imre Deak <imre.deak@xxxxxxxxx>
>
> 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.

See also

http://patchwork.freedesktop.org/patch/msgid/20180925071836.24711-1-jani.nikula@xxxxxxxxx

BR,
Jani.

>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h      |  3 +-
>  drivers/gpu/drm/i915/intel_ddi.c     | 12 +++---
>  drivers/gpu/drm/i915/intel_dp.c      | 12 +++---
>  drivers/gpu/drm/i915/intel_drv.h     | 17 +++++++--
>  drivers/gpu/drm/i915/intel_hotplug.c | 56 ++++++++++++++++++++++------
>  drivers/gpu/drm/i915/intel_sdvo.c    |  8 ++--
>  7 files changed, 79 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 37175414ce89..8149be62a630 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4296,7 +4296,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 bb0e75e43987..5703526bddab 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -156,7 +156,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;
> @@ -168,6 +168,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_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index ea83071a22c4..1676a87f18cb 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -4051,14 +4051,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);
>  
> @@ -4080,7 +4082,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/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e1a051c0fbfe..0bb43eaa154c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4734,14 +4734,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);
>  
> @@ -4760,7 +4762,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/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 81ec73e4a083..de62af970dc1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -226,14 +226,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 *);
> @@ -2003,8 +2010,10 @@ int intel_dsi_dcs_init_backlight_funcs(struct intel_connector *intel_connector);
>  void intel_dvo_init(struct drm_i915_private *dev_priv);
>  /* intel_hotplug.c */
>  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);
>  
>  /* legacy fbdev emulation in intel_fbdev.c */
>  #ifdef CONFIG_DRM_FBDEV_EMULATION
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index b8937c788f03..3eaa06526f6b 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -111,6 +111,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
> @@ -265,8 +266,10 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
>  	intel_runtime_pm_put(dev_priv, 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;
> @@ -278,7 +281,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,
> @@ -286,7 +289,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)
> @@ -338,7 +341,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);
>  	}
>  }
>  
> @@ -348,14 +351,17 @@ 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 retry = 0;
>  	u32 hpd_event_bits;
> +	u32 hpd_retry_bits;
>  
>  	mutex_lock(&dev->mode_config.mutex);
>  	DRM_DEBUG_KMS("running encoder hotplug functions\n");
> @@ -364,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);
> @@ -372,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 = true;
> +				break;
> +			case INTEL_HOTPLUG_RETRY:
> +				retry |= hpd_bit;
> +				break;
> +			}
>  		}
>  	}
>  	drm_connector_list_iter_end(&conn_iter);
> @@ -389,6 +410,15 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  
>  	if (changed)
>  		drm_kms_helper_hotplug_event(dev);
> +
> +	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));
> +	}
>  }
>  
>  
> @@ -515,7 +545,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);
>  }
>  
>  /**
> @@ -635,7 +665,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,
> @@ -649,11 +680,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/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index e7b0884ba5a5..92793475b2b9 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1722,12 +1722,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

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux