[02/11] drm/i915: Introduce intel_connector_hpd_pin()
diff mbox series

Message ID 20200121171100.4370-3-ville.syrjala@linux.intel.com
State New
Headers show
Series
  • drm/i915: Hotplug cleanups
Related show

Commit Message

Ville Syrjälä Jan. 21, 2020, 5:10 p.m. UTC
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.

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(-)

Comments

Jani Nikula Jan. 28, 2020, 8:22 a.m. UTC | #1
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);

Patch
diff mbox series

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);