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

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx