[Intel-gfx] [PATCH 1/2] drm/i915: Simplify hpd pin to port

[Pandiyan, Dhinakaran]

On Wed, 2017-08-02 at 10:20 -0700, Rodrigo Vivi wrote:
> We will soon need to make that pin port association per
> platform, so let's try to simplify it beforehand.
> 
> Also we are moving the backwards port to pin
> here as well so let's use a standardized way.
> 
> One extra possibility here would be to add a
> MISSING_CASE along with PORT_NONE, but I don't want
> to change this behaviour for now.
> 
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 +-
>  drivers/gpu/drm/i915/i915_irq.c      |  3 ++-
>  drivers/gpu/drm/i915/intel_dp.c      |  2 +-
>  drivers/gpu/drm/i915/intel_hotplug.c | 31 +++++++++++++++++--------------
>  4 files changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d63645a521c4..5c2c7a677e96 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3147,7 +3147,7 @@ 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);
> -bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port);
> +enum port intel_hpd_port(enum hpd_pin pin);

bikeshed: I feel hpd_pin_to_port reads better, conveys the conversion
from enum hpd_pin to enum port. Secondly, I haven't seen any references
to "hpd_port" in the driver.

It is not entirely obvious to me how changing the function signature
will help per-platform mapping, but returning port looks cleaner. 

With the function name left intact
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>


>  bool intel_hpd_disable(struct drm_i915_private *dev_priv, enum hpd_pin pin);
>  void intel_hpd_enable(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 196caa463edf..b115ab584b5e 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1501,7 +1501,8 @@ static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
>  
>  		*pin_mask |= BIT(i);
>  
> -		if (!intel_hpd_pin_to_port(i, &port))
> +		port = intel_hpd_port(i);
> +		if (port == PORT_NONE)
>  			continue;
>  
>  		if (long_pulse_detect(port, dig_hotplug_reg))
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 2d42d09428c9..d4e6128f3b3a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4566,7 +4566,7 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
>  	enum port port;
>  	u32 bit;
>  
> -	intel_hpd_pin_to_port(intel_encoder->hpd_pin, &port);
> +	port = intel_hpd_port(intel_encoder->hpd_pin);
>  	switch (port) {
>  	case PORT_A:
>  		bit = BXT_DE_PORT_HP_DDIA;
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index f1200272a699..5982c47586e7 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -76,26 +76,28 @@
>   * it will use i915_hotplug_work_func where this logic is handled.
>   */
>  
> -bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port)
> +/**
> + * intel_hpd_port - return port hard associated with certain pin.
> + * @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_port(enum hpd_pin pin)
>  {
>  	switch (pin) {
>  	case HPD_PORT_A:
> -		*port = PORT_A;
> -		return true;
> +		return PORT_A;
>  	case HPD_PORT_B:
> -		*port = PORT_B;
> -		return true;
> +		return PORT_B;
>  	case HPD_PORT_C:
> -		*port = PORT_C;
> -		return true;
> +		return PORT_C;
>  	case HPD_PORT_D:
> -		*port = PORT_D;
> -		return true;
> +		return PORT_D;
>  	case HPD_PORT_E:
> -		*port = PORT_E;
> -		return true;
> +		return PORT_E;
>  	default:
> -		return false;	/* no hpd */
> +		return PORT_NONE; /* no port for this pin */
>  	}
>  }
>  
> @@ -389,8 +391,9 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  		if (!(BIT(i) & pin_mask))
>  			continue;
>  
> -		is_dig_port = intel_hpd_pin_to_port(i, &port) &&
> -			      dev_priv->hotplug.irq_port[port];
> +		port = intel_hpd_port(i);
> +		is_dig_port = port != PORT_NONE &&
> +			dev_priv->hotplug.irq_port[port];

nit: I find
= (port != PORT_NONE) && dev_priv->hotplug.irq_port[port]
easier to read

>  
>  		if (is_dig_port) {
>  			bool long_hpd = long_mask & BIT(i);