diff mbox

[6/8] drm/i915: Pass hpd_pin to long_pulse_detect()

Message ID 20180705164357.28512-7-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä July 5, 2018, 4:43 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We're doing a pointless translation from hpd_pin to port simply for
passing the thing to long_pulse_detect(). Let's pass the hpd_pin
directly instead.

This removes the assumption that the hpd_pin and port always
match. The only other place where we make that assumption anymore
is intel_hpd_pin_default() and that's fine as it's what determines
the relationship between the two. If we ever get hardware where
the hpd pins are wired in more interesting ways it should be
trivial to handle from now on.

This should also fix the IS_CNL_WITH_PORT_F() case as that mapped
pin E back to port F and passed that to
spt_port_hotplug2_long_detect() which would always return false
for port F. Now that we pass in pin E directly it'll actually
do the right thing.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Fixes: cf53902f48c3 ("drm/i915/cnl: Add HPD support for Port F.")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  2 -
 drivers/gpu/drm/i915/i915_irq.c      | 95 +++++++++++++++++-------------------
 drivers/gpu/drm/i915/intel_hotplug.c | 31 ------------
 3 files changed, 45 insertions(+), 83 deletions(-)

Comments

Rodrigo Vivi July 5, 2018, 9:14 p.m. UTC | #1 
On Thu, Jul 05, 2018 at 07:43:55PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We're doing a pointless translation from hpd_pin to port simply for
> passing the thing to long_pulse_detect(). Let's pass the hpd_pin
> directly instead.
> 
> This removes the assumption that the hpd_pin and port always
> match. The only other place where we make that assumption anymore
> is intel_hpd_pin_default() and that's fine as it's what determines
> the relationship between the two. If we ever get hardware where
> the hpd pins are wired in more interesting ways it should be
> trivial to handle from now on.
> 
> This should also fix the IS_CNL_WITH_PORT_F() case as that mapped
> pin E back to port F and passed that to
> spt_port_hotplug2_long_detect() which would always return false
> for port F. Now that we pass in pin E directly it'll actually
> do the right thing.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Fixes: cf53902f48c3 ("drm/i915/cnl: Add HPD support for Port F.")

Thanks!

I could swear that I had a version similar to this somewhere,
but anyways this is much better...

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 -
>  drivers/gpu/drm/i915/i915_irq.c      | 95 +++++++++++++++++-------------------
>  drivers/gpu/drm/i915/intel_hotplug.c | 31 ------------
>  3 files changed, 45 insertions(+), 83 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a808bb8aa4d8..5afadc897e76 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2746,8 +2746,6 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  void intel_hpd_init(struct drm_i915_private *dev_priv);
>  void intel_hpd_init_work(struct drm_i915_private *dev_priv);
>  void intel_hpd_cancel_work(struct drm_i915_private *dev_priv);
> -enum port intel_hpd_pin_to_port(struct drm_i915_private *dev_priv,
> -				enum hpd_pin pin);
>  enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
>  				   enum port port);
>  bool intel_hpd_disable(struct drm_i915_private *dev_priv, enum hpd_pin pin);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 1c3ff07d9f2d..bb7c754979f8 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1576,122 +1576,122 @@ static void gen8_gt_irq_handler(struct drm_i915_private *i915,
>  	}
>  }
>  
> -static bool gen11_port_hotplug_long_detect(enum port port, u32 val)
> +static bool gen11_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_C:
> +	switch (pin) {
> +	case HPD_PORT_C:
>  		return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC1);
> -	case PORT_D:
> +	case HPD_PORT_D:
>  		return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC2);
> -	case PORT_E:
> +	case HPD_PORT_E:
>  		return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC3);
> -	case PORT_F:
> +	case HPD_PORT_F:
>  		return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC4);
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
> +static bool bxt_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_A:
> +	switch (pin) {
> +	case HPD_PORT_A:
>  		return val & PORTA_HOTPLUG_LONG_DETECT;
> -	case PORT_B:
> +	case HPD_PORT_B:
>  		return val & PORTB_HOTPLUG_LONG_DETECT;
> -	case PORT_C:
> +	case HPD_PORT_C:
>  		return val & PORTC_HOTPLUG_LONG_DETECT;
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool icp_ddi_port_hotplug_long_detect(enum port port, u32 val)
> +static bool icp_ddi_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_A:
> +	switch (pin) {
> +	case HPD_PORT_A:
>  		return val & ICP_DDIA_HPD_LONG_DETECT;
> -	case PORT_B:
> +	case HPD_PORT_B:
>  		return val & ICP_DDIB_HPD_LONG_DETECT;
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool icp_tc_port_hotplug_long_detect(enum port port, u32 val)
> +static bool icp_tc_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_C:
> +	switch (pin) {
> +	case HPD_PORT_C:
>  		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC1);
> -	case PORT_D:
> +	case HPD_PORT_D:
>  		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC2);
> -	case PORT_E:
> +	case HPD_PORT_E:
>  		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC3);
> -	case PORT_F:
> +	case HPD_PORT_F:
>  		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC4);
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
> +static bool spt_port_hotplug2_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_E:
> +	switch (pin) {
> +	case HPD_PORT_E:
>  		return val & PORTE_HOTPLUG_LONG_DETECT;
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool spt_port_hotplug_long_detect(enum port port, u32 val)
> +static bool spt_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_A:
> +	switch (pin) {
> +	case HPD_PORT_A:
>  		return val & PORTA_HOTPLUG_LONG_DETECT;
> -	case PORT_B:
> +	case HPD_PORT_B:
>  		return val & PORTB_HOTPLUG_LONG_DETECT;
> -	case PORT_C:
> +	case HPD_PORT_C:
>  		return val & PORTC_HOTPLUG_LONG_DETECT;
> -	case PORT_D:
> +	case HPD_PORT_D:
>  		return val & PORTD_HOTPLUG_LONG_DETECT;
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool ilk_port_hotplug_long_detect(enum port port, u32 val)
> +static bool ilk_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_A:
> +	switch (pin) {
> +	case HPD_PORT_A:
>  		return val & DIGITAL_PORTA_HOTPLUG_LONG_DETECT;
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool pch_port_hotplug_long_detect(enum port port, u32 val)
> +static bool pch_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_B:
> +	switch (pin) {
> +	case HPD_PORT_B:
>  		return val & PORTB_HOTPLUG_LONG_DETECT;
> -	case PORT_C:
> +	case HPD_PORT_C:
>  		return val & PORTC_HOTPLUG_LONG_DETECT;
> -	case PORT_D:
> +	case HPD_PORT_D:
>  		return val & PORTD_HOTPLUG_LONG_DETECT;
>  	default:
>  		return false;
>  	}
>  }
>  
> -static bool i9xx_port_hotplug_long_detect(enum port port, u32 val)
> +static bool i9xx_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
> -	switch (port) {
> -	case PORT_B:
> +	switch (pin) {
> +	case HPD_PORT_B:
>  		return val & PORTB_HOTPLUG_INT_LONG_PULSE;
> -	case PORT_C:
> +	case HPD_PORT_C:
>  		return val & PORTC_HOTPLUG_INT_LONG_PULSE;
> -	case PORT_D:
> +	case HPD_PORT_D:
>  		return val & PORTD_HOTPLUG_INT_LONG_PULSE;
>  	default:
>  		return false;
> @@ -1709,9 +1709,8 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
>  			       u32 *pin_mask, u32 *long_mask,
>  			       u32 hotplug_trigger, u32 dig_hotplug_reg,
>  			       const u32 hpd[HPD_NUM_PINS],
> -			       bool long_pulse_detect(enum port port, u32 val))
> +			       bool long_pulse_detect(enum hpd_pin pin, u32 val))
>  {
> -	enum port port;
>  	enum hpd_pin pin;
>  
>  	for_each_hpd_pin(pin) {
> @@ -1720,11 +1719,7 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
>  
>  		*pin_mask |= BIT(pin);
>  
> -		port = intel_hpd_pin_to_port(dev_priv, pin);
> -		if (port == PORT_NONE)
> -			continue;
> -
> -		if (long_pulse_detect(port, dig_hotplug_reg))
> +		if (long_pulse_detect(pin, dig_hotplug_reg))
>  			*long_mask |= BIT(pin);
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index d9d61d11dd2b..648a13c6043c 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -76,37 +76,6 @@
>   * it will use i915_hotplug_work_func where this logic is handled.
>   */
>  
> -/**
> - * intel_hpd_port - return port hard associated with certain pin.
> - * @dev_priv: private driver data pointer
> - * @pin: the hpd pin to get associated port
> - *
> - * Return port that is associatade with @pin and PORT_NONE if no port is
> - * hard associated with that @pin.
> - */
> -enum port intel_hpd_pin_to_port(struct drm_i915_private *dev_priv,
> -				enum hpd_pin pin)
> -{
> -	switch (pin) {
> -	case HPD_PORT_A:
> -		return PORT_A;
> -	case HPD_PORT_B:
> -		return PORT_B;
> -	case HPD_PORT_C:
> -		return PORT_C;
> -	case HPD_PORT_D:
> -		return PORT_D;
> -	case HPD_PORT_E:
> -		if (IS_CNL_WITH_PORT_F(dev_priv))
> -			return PORT_F;
> -		return PORT_E;
> -	case HPD_PORT_F:
> -		return PORT_F;
> -	default:
> -		return PORT_NONE; /* no port for this pin */
> -	}
> -}
> -
>  /**
>   * intel_hpd_pin_default - return default pin associated with certain port.
>   * @dev_priv: private driver data pointer
> -- 
> 2.16.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a808bb8aa4d8..5afadc897e76 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2746,8 +2746,6 @@  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
 void intel_hpd_init(struct drm_i915_private *dev_priv);
 void intel_hpd_init_work(struct drm_i915_private *dev_priv);
 void intel_hpd_cancel_work(struct drm_i915_private *dev_priv);
-enum port intel_hpd_pin_to_port(struct drm_i915_private *dev_priv,
-				enum hpd_pin pin);
 enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
 				   enum port port);
 bool intel_hpd_disable(struct drm_i915_private *dev_priv, enum hpd_pin pin);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1c3ff07d9f2d..bb7c754979f8 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1576,122 +1576,122 @@  static void gen8_gt_irq_handler(struct drm_i915_private *i915,
 	}
 }
 
-static bool gen11_port_hotplug_long_detect(enum port port, u32 val)
+static bool gen11_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_C:
+	switch (pin) {
+	case HPD_PORT_C:
 		return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC1);
-	case PORT_D:
+	case HPD_PORT_D:
 		return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC2);
-	case PORT_E:
+	case HPD_PORT_E:
 		return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC3);
-	case PORT_F:
+	case HPD_PORT_F:
 		return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC4);
 	default:
 		return false;
 	}
 }
 
-static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
+static bool bxt_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_A:
+	switch (pin) {
+	case HPD_PORT_A:
 		return val & PORTA_HOTPLUG_LONG_DETECT;
-	case PORT_B:
+	case HPD_PORT_B:
 		return val & PORTB_HOTPLUG_LONG_DETECT;
-	case PORT_C:
+	case HPD_PORT_C:
 		return val & PORTC_HOTPLUG_LONG_DETECT;
 	default:
 		return false;
 	}
 }
 
-static bool icp_ddi_port_hotplug_long_detect(enum port port, u32 val)
+static bool icp_ddi_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_A:
+	switch (pin) {
+	case HPD_PORT_A:
 		return val & ICP_DDIA_HPD_LONG_DETECT;
-	case PORT_B:
+	case HPD_PORT_B:
 		return val & ICP_DDIB_HPD_LONG_DETECT;
 	default:
 		return false;
 	}
 }
 
-static bool icp_tc_port_hotplug_long_detect(enum port port, u32 val)
+static bool icp_tc_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_C:
+	switch (pin) {
+	case HPD_PORT_C:
 		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC1);
-	case PORT_D:
+	case HPD_PORT_D:
 		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC2);
-	case PORT_E:
+	case HPD_PORT_E:
 		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC3);
-	case PORT_F:
+	case HPD_PORT_F:
 		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC4);
 	default:
 		return false;
 	}
 }
 
-static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
+static bool spt_port_hotplug2_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_E:
+	switch (pin) {
+	case HPD_PORT_E:
 		return val & PORTE_HOTPLUG_LONG_DETECT;
 	default:
 		return false;
 	}
 }
 
-static bool spt_port_hotplug_long_detect(enum port port, u32 val)
+static bool spt_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_A:
+	switch (pin) {
+	case HPD_PORT_A:
 		return val & PORTA_HOTPLUG_LONG_DETECT;
-	case PORT_B:
+	case HPD_PORT_B:
 		return val & PORTB_HOTPLUG_LONG_DETECT;
-	case PORT_C:
+	case HPD_PORT_C:
 		return val & PORTC_HOTPLUG_LONG_DETECT;
-	case PORT_D:
+	case HPD_PORT_D:
 		return val & PORTD_HOTPLUG_LONG_DETECT;
 	default:
 		return false;
 	}
 }
 
-static bool ilk_port_hotplug_long_detect(enum port port, u32 val)
+static bool ilk_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_A:
+	switch (pin) {
+	case HPD_PORT_A:
 		return val & DIGITAL_PORTA_HOTPLUG_LONG_DETECT;
 	default:
 		return false;
 	}
 }
 
-static bool pch_port_hotplug_long_detect(enum port port, u32 val)
+static bool pch_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_B:
+	switch (pin) {
+	case HPD_PORT_B:
 		return val & PORTB_HOTPLUG_LONG_DETECT;
-	case PORT_C:
+	case HPD_PORT_C:
 		return val & PORTC_HOTPLUG_LONG_DETECT;
-	case PORT_D:
+	case HPD_PORT_D:
 		return val & PORTD_HOTPLUG_LONG_DETECT;
 	default:
 		return false;
 	}
 }
 
-static bool i9xx_port_hotplug_long_detect(enum port port, u32 val)
+static bool i9xx_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
 {
-	switch (port) {
-	case PORT_B:
+	switch (pin) {
+	case HPD_PORT_B:
 		return val & PORTB_HOTPLUG_INT_LONG_PULSE;
-	case PORT_C:
+	case HPD_PORT_C:
 		return val & PORTC_HOTPLUG_INT_LONG_PULSE;
-	case PORT_D:
+	case HPD_PORT_D:
 		return val & PORTD_HOTPLUG_INT_LONG_PULSE;
 	default:
 		return false;
@@ -1709,9 +1709,8 @@  static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
 			       u32 *pin_mask, u32 *long_mask,
 			       u32 hotplug_trigger, u32 dig_hotplug_reg,
 			       const u32 hpd[HPD_NUM_PINS],
-			       bool long_pulse_detect(enum port port, u32 val))
+			       bool long_pulse_detect(enum hpd_pin pin, u32 val))
 {
-	enum port port;
 	enum hpd_pin pin;
 
 	for_each_hpd_pin(pin) {
@@ -1720,11 +1719,7 @@  static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
 
 		*pin_mask |= BIT(pin);
 
-		port = intel_hpd_pin_to_port(dev_priv, pin);
-		if (port == PORT_NONE)
-			continue;
-
-		if (long_pulse_detect(port, dig_hotplug_reg))
+		if (long_pulse_detect(pin, dig_hotplug_reg))
 			*long_mask |= BIT(pin);
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index d9d61d11dd2b..648a13c6043c 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -76,37 +76,6 @@ 
  * it will use i915_hotplug_work_func where this logic is handled.
  */
 
-/**
- * intel_hpd_port - return port hard associated with certain pin.
- * @dev_priv: private driver data pointer
- * @pin: the hpd pin to get associated port
- *
- * Return port that is associatade with @pin and PORT_NONE if no port is
- * hard associated with that @pin.
- */
-enum port intel_hpd_pin_to_port(struct drm_i915_private *dev_priv,
-				enum hpd_pin pin)
-{
-	switch (pin) {
-	case HPD_PORT_A:
-		return PORT_A;
-	case HPD_PORT_B:
-		return PORT_B;
-	case HPD_PORT_C:
-		return PORT_C;
-	case HPD_PORT_D:
-		return PORT_D;
-	case HPD_PORT_E:
-		if (IS_CNL_WITH_PORT_F(dev_priv))
-			return PORT_F;
-		return PORT_E;
-	case HPD_PORT_F:
-		return PORT_F;
-	default:
-		return PORT_NONE; /* no port for this pin */
-	}
-}
-
 /**
  * intel_hpd_pin_default - return default pin associated with certain port.
  * @dev_priv: private driver data pointer