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

2017-08-11 Thread Rodrigo Vivi
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ä 
Signed-off-by: Rodrigo Vivi 
Reviewed-by: Dhinakaran Pandiyan 
---
 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 907603cba447..68ec47b378ac 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3194,7 +3194,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_pin_to_port(enum hpd_pin pin);
 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..58262380dcb8 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_pin_to_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 76c8a0bd17f9..eeede2037931 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_pin_to_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..d442d9f012d6 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_pin_to_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_pin_to_port(i);
+   is_dig_port = port != PORT_NONE &&
+   dev_priv->hotplug.irq_port[port];
 
if (is_dig_port) {
bool long_hpd = long_mask & BIT(i);
-- 
2.13.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2017-08-09 Thread 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ä 
> Signed-off-by: Rodrigo Vivi 
> ---
>  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 


>  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

>  
>

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

2017-08-02 Thread Rodrigo Vivi
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ä 
Signed-off-by: Rodrigo Vivi 
---
 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);
 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];
 
if (is_dig_port) {
bool long_hpd = long_mask & BIT(i);
-- 
2.13.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx