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.syrj...@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.v...@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.pandi...@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);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to