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@xxxxxxxxxxxxxxx>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
> 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@xxxxxxxxx>
> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
