[PATCH 0/3] drm/i915/bxt: HPD long/short pulse detect support on port A

[PATCH 0/3] drm/i915/bxt: HPD long/short pulse detect support on port A

[Imre Deak]

This patchset is a dependency for enabling generic HPD support on the
port A pin.

Imre Deak (3):
  drm/i915: combine i9xx_get_hpd_pins and pch_get_hpd_pins
  drm/i915: don't use HPD_PORT_A as an alias for HPD_NONE
  drm/i915/bxt: add support for HPD long/short pulse detection on
    HPD_PORT_A pin

 drivers/gpu/drm/i915/i915_drv.h      |  4 +-
 drivers/gpu/drm/i915/i915_irq.c      | 72 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/i915_reg.h      |  5 +++
 drivers/gpu/drm/i915/intel_hotplug.c | 20 ++++++----
 4 files changed, 55 insertions(+), 46 deletions(-)

-- 
2.1.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/3] drm/i915: combine i9xx_get_hpd_pins and pch_get_hpd_pins

[Imre Deak]

These functions are quite similar, so combine them with the use of a new
argument for a function that detects long pulses. This will be also
needed by an upcoming patch adding support for BXT long pulse detection.

No functional change.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 54 ++++++++++++++---------------------------
 1 file changed, 18 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a6fbe64..306de78 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1256,9 +1256,10 @@ static bool i9xx_port_hotplug_long_detect(enum port port, u32 val)
 }
 
 /* Get a bit mask of pins that have triggered, and which ones may be long. */
-static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
+static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
 			     u32 hotplug_trigger, u32 dig_hotplug_reg,
-			     const u32 hpd[HPD_NUM_PINS])
+			     const u32 hpd[HPD_NUM_PINS],
+			     bool long_pulse_detect(enum port port, u32 val))
 {
 	enum port port;
 	int i;
@@ -1276,7 +1277,7 @@ static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
 		*pin_mask |= BIT(i);
 
 		port = intel_hpd_pin_to_port(i);
-		if (pch_port_hotplug_long_detect(port, dig_hotplug_reg))
+		if (long_pulse_detect(port, dig_hotplug_reg))
 			*long_mask |= BIT(i);
 	}
 
@@ -1285,34 +1286,6 @@ static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
 
 }
 
-/* Get a bit mask of pins that have triggered, and which ones may be long. */
-static void i9xx_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
-			      u32 hotplug_trigger, const u32 hpd[HPD_NUM_PINS])
-{
-	enum port port;
-	int i;
-
-	*pin_mask = 0;
-	*long_mask = 0;
-
-	if (!hotplug_trigger)
-		return;
-
-	for_each_hpd_pin(i) {
-		if ((hpd[i] & hotplug_trigger) == 0)
-			continue;
-
-		*pin_mask |= BIT(i);
-
-		port = intel_hpd_pin_to_port(i);
-		if (i9xx_port_hotplug_long_detect(port, hotplug_trigger))
-			*long_mask |= BIT(i);
-	}
-
-	DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x, pins 0x%08x\n",
-			 hotplug_trigger, *pin_mask);
-}
-
 static void gmbus_irq_handler(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1550,7 +1523,9 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)
 	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
 		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
 
-		i9xx_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, hpd_status_g4x);
+		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
+				   hotplug_trigger, hpd_status_g4x,
+				   i9xx_port_hotplug_long_detect);
 		intel_hpd_irq_handler(dev, pin_mask, long_mask);
 
 		if (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
@@ -1558,7 +1533,9 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)
 	} else {
 		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
 
-		i9xx_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, hpd_status_i915);
+		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
+				   hotplug_trigger, hpd_status_g4x,
+				   i9xx_port_hotplug_long_detect);
 		intel_hpd_irq_handler(dev, pin_mask, long_mask);
 	}
 }
@@ -1664,7 +1641,9 @@ static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir)
 	dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
 	I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
 
-	pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, dig_hotplug_reg, hpd_ibx);
+	intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
+			   dig_hotplug_reg, hpd_ibx,
+			   pch_port_hotplug_long_detect);
 	intel_hpd_irq_handler(dev, pin_mask, long_mask);
 
 	if (pch_iir & SDE_AUDIO_POWER_MASK) {
@@ -1763,7 +1742,9 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
 	dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
 	I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
 
-	pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, dig_hotplug_reg, hpd_cpt);
+	intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
+			   dig_hotplug_reg, hpd_cpt,
+			   pch_port_hotplug_long_detect);
 	intel_hpd_irq_handler(dev, pin_mask, long_mask);
 
 	if (pch_iir & SDE_AUDIO_POWER_MASK_CPT) {
@@ -1979,7 +1960,8 @@ static void bxt_hpd_handler(struct drm_device *dev, uint32_t iir_status)
 	/* Clear sticky bits in hpd status */
 	I915_WRITE(BXT_HOTPLUG_CTL, hp_control);
 
-	pch_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control, hpd_bxt);
+	intel_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control,
+			   hpd_bxt, pch_port_hotplug_long_detect);
 	intel_hpd_irq_handler(dev, pin_mask, long_mask);
 }
 
-- 
2.1.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915: don't use HPD_PORT_A as an alias for HPD_NONE

[Imre Deak]

Currently HPD_PORT_A is used as an alias for HPD_NONE to mean that the
given port doesn't support long/short HPD pulse detection. SDVO and CRT
ports are like this and for these ports we only want to know whether an
hot plug event was detected on the corresponding pin. Since at least on
BXT we need long/short pulse detection on PORT A as well (added by the
next patch) remove this aliasing of HPD_PORT_A/HPD_NONE and let the
return value of intel_hpd_pin_to_port() show whether long/short pulse
detection is supported on the passed in pin.

No functional change.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  4 ++--
 drivers/gpu/drm/i915/i915_irq.c      |  4 ++--
 drivers/gpu/drm/i915/intel_hotplug.c | 20 +++++++++++++-------
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 78d6c7a..ebfd5a5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -206,11 +206,11 @@ enum intel_display_power_domain {
 
 enum hpd_pin {
 	HPD_NONE = 0,
-	HPD_PORT_A = HPD_NONE, /* PORT_A is internal */
 	HPD_TV = HPD_NONE,     /* TV is known to be unreliable */
 	HPD_CRT,
 	HPD_SDVO_B,
 	HPD_SDVO_C,
+	HPD_PORT_A,
 	HPD_PORT_B,
 	HPD_PORT_C,
 	HPD_PORT_D,
@@ -2627,7 +2627,7 @@ void intel_hpd_irq_handler(struct drm_device *dev, u32 pin_mask, u32 long_mask);
 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(enum hpd_pin pin);
+bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port);
 
 /* i915_irq.c */
 void i915_queue_hangcheck(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 306de78..4ad7a31 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1276,8 +1276,8 @@ static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
 
 		*pin_mask |= BIT(i);
 
-		port = intel_hpd_pin_to_port(i);
-		if (long_pulse_detect(port, dig_hotplug_reg))
+		if (intel_hpd_pin_to_port(i, &port) &&
+		    long_pulse_detect(port, dig_hotplug_reg))
 			*long_mask |= BIT(i);
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index 3c53aac..8cda7b9 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -29,17 +29,23 @@
 #include "i915_drv.h"
 #include "intel_drv.h"
 
-enum port intel_hpd_pin_to_port(enum hpd_pin pin)
+bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port)
 {
 	switch (pin) {
+	case HPD_PORT_A:
+		*port = PORT_A;
+		return true;
 	case HPD_PORT_B:
-		return PORT_B;
+		*port = PORT_B;
+		return true;
 	case HPD_PORT_C:
-		return PORT_C;
+		*port = PORT_C;
+		return true;
 	case HPD_PORT_D:
-		return PORT_D;
+		*port = PORT_D;
+		return true;
 	default:
-		return PORT_A; /* no hpd */
+		return false;	/* no hpd */
 	}
 }
 
@@ -322,8 +328,8 @@ void intel_hpd_irq_handler(struct drm_device *dev,
 		if (!(BIT(i) & pin_mask))
 			continue;
 
-		port = intel_hpd_pin_to_port(i);
-		is_dig_port = port && dev_priv->hotplug.irq_port[port];
+		is_dig_port = intel_hpd_pin_to_port(i, &port) &&
+			      dev_priv->hotplug.irq_port[port];
 
 		if (is_dig_port) {
 			bool long_hpd = long_mask & BIT(i);
-- 
2.1.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915/bxt: add support for HPD long/short pulse detection on HPD_PORT_A pin

[Imre Deak]

This is a requirement for enabling display port HPD support on the port
A HPD pin. This support is to be added by follow-up patches.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 18 +++++++++++++++++-
 drivers/gpu/drm/i915/i915_reg.h |  5 +++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 4ad7a31..02b9e73 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1227,6 +1227,22 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_i915_private *dev_priv,
 	return ret;
 }
 
+static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
+{
+	switch (port) {
+	case PORT_A:
+		return val & BXT_PORTA_HOTPLUG_LONG_DETECT;
+	case PORT_B:
+		return val & PORTB_HOTPLUG_LONG_DETECT;
+	case PORT_C:
+		return val & PORTC_HOTPLUG_LONG_DETECT;
+	case PORT_D:
+		return val & PORTD_HOTPLUG_LONG_DETECT;
+	default:
+		return false;
+	}
+}
+
 static bool pch_port_hotplug_long_detect(enum port port, u32 val)
 {
 	switch (port) {
@@ -1961,7 +1977,7 @@ static void bxt_hpd_handler(struct drm_device *dev, uint32_t iir_status)
 	I915_WRITE(BXT_HOTPLUG_CTL, hp_control);
 
 	intel_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control,
-			   hpd_bxt, pch_port_hotplug_long_detect);
+			   hpd_bxt, bxt_port_hotplug_long_detect);
 	intel_hpd_irq_handler(dev, pin_mask, long_mask);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index fc70035..be166b3 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5942,6 +5942,11 @@ enum skl_disp_power_wells {
 
 /* digital port hotplug */
 #define PCH_PORT_HOTPLUG        0xc4030		/* SHOTPLUG_CTL */
+#define BXT_PORTA_HOTPLUG_ENABLE	(1 << 28)
+#define BXT_PORTA_HOTPLUG_STATUS_MASK	(0x3 << 24)
+#define  BXT_PORTA_HOTPLUG_NO_DETECT	(0 << 24)
+#define  BXT_PORTA_HOTPLUG_SHORT_DETECT	(1 << 24)
+#define  BXT_PORTA_HOTPLUG_LONG_DETECT	(2 << 24)
 #define PORTD_HOTPLUG_ENABLE            (1 << 20)
 #define PORTD_PULSE_DURATION_2ms        (0)
 #define PORTD_PULSE_DURATION_4_5ms      (1 << 18)
-- 
2.1.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] drm/i915/bxt: HPD long/short pulse detect support on port A

[Jindal, Sonika]

For the patch 3, the commit message can be changed to only "long pulse 
detection" instead of "long/short" because you are not adding support 
for short pulse detection.

Otherwise, this series looks good to me.

Reviewed-by: Sonika Jindal <sonika.jindal@intel.com>

On 7/21/2015 3:13 AM, Imre Deak wrote:
> This patchset is a dependency for enabling generic HPD support on the
> port A pin.
>
> Imre Deak (3):
>    drm/i915: combine i9xx_get_hpd_pins and pch_get_hpd_pins
>    drm/i915: don't use HPD_PORT_A as an alias for HPD_NONE
>    drm/i915/bxt: add support for HPD long/short pulse detection on
>      HPD_PORT_A pin
>
>   drivers/gpu/drm/i915/i915_drv.h      |  4 +-
>   drivers/gpu/drm/i915/i915_irq.c      | 72 ++++++++++++++++++------------------
>   drivers/gpu/drm/i915/i915_reg.h      |  5 +++
>   drivers/gpu/drm/i915/intel_hotplug.c | 20 ++++++----
>   4 files changed, 55 insertions(+), 46 deletions(-)
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: combine i9xx_get_hpd_pins and pch_get_hpd_pins

[Sivakumar Thulasimani]

On 7/21/2015 3:13 AM, Imre Deak wrote:
> These functions are quite similar, so combine them with the use of a new
> argument for a function that detects long pulses. This will be also
> needed by an upcoming patch adding support for BXT long pulse detection.
>
> No functional change.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_irq.c | 54 ++++++++++++++---------------------------
>   1 file changed, 18 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index a6fbe64..306de78 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1256,9 +1256,10 @@ static bool i9xx_port_hotplug_long_detect(enum port port, u32 val)
>   }
>   
>   /* Get a bit mask of pins that have triggered, and which ones may be long. */
> -static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
> +static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
>   			     u32 hotplug_trigger, u32 dig_hotplug_reg,
> -			     const u32 hpd[HPD_NUM_PINS])
> +			     const u32 hpd[HPD_NUM_PINS],
> +			     bool long_pulse_detect(enum port port, u32 val))
>   {
>   	enum port port;
>   	int i;
> @@ -1276,7 +1277,7 @@ static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
>   		*pin_mask |= BIT(i);
>   
>   		port = intel_hpd_pin_to_port(i);
> -		if (pch_port_hotplug_long_detect(port, dig_hotplug_reg))
> +		if (long_pulse_detect(port, dig_hotplug_reg))
>   			*long_mask |= BIT(i);
>   	}
>   
> @@ -1285,34 +1286,6 @@ static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
>   
>   }
>   
> -/* Get a bit mask of pins that have triggered, and which ones may be long. */
> -static void i9xx_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
> -			      u32 hotplug_trigger, const u32 hpd[HPD_NUM_PINS])
> -{
> -	enum port port;
> -	int i;
> -
> -	*pin_mask = 0;
> -	*long_mask = 0;
> -
> -	if (!hotplug_trigger)
> -		return;
> -
> -	for_each_hpd_pin(i) {
> -		if ((hpd[i] & hotplug_trigger) == 0)
> -			continue;
> -
> -		*pin_mask |= BIT(i);
> -
> -		port = intel_hpd_pin_to_port(i);
> -		if (i9xx_port_hotplug_long_detect(port, hotplug_trigger))
> -			*long_mask |= BIT(i);
> -	}
> -
> -	DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x, pins 0x%08x\n",
> -			 hotplug_trigger, *pin_mask);
> -}
> -
>   static void gmbus_irq_handler(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1550,7 +1523,9 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)
>   	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
>   		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
>   
> -		i9xx_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, hpd_status_g4x);
> +		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> +				   hotplug_trigger, hpd_status_g4x,
> +				   i9xx_port_hotplug_long_detect);
>   		intel_hpd_irq_handler(dev, pin_mask, long_mask);
>   
>   		if (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
> @@ -1558,7 +1533,9 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)
>   	} else {
>   		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
>   
> -		i9xx_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, hpd_status_i915);
> +		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> +				   hotplug_trigger, hpd_status_g4x,
> +				   i9xx_port_hotplug_long_detect);
>   		intel_hpd_irq_handler(dev, pin_mask, long_mask);
>   	}
>   }
> @@ -1664,7 +1641,9 @@ static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir)
>   	dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
>   	I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
>   
> -	pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, dig_hotplug_reg, hpd_ibx);
> +	intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> +			   dig_hotplug_reg, hpd_ibx,
> +			   pch_port_hotplug_long_detect);
>   	intel_hpd_irq_handler(dev, pin_mask, long_mask);
>   
>   	if (pch_iir & SDE_AUDIO_POWER_MASK) {
> @@ -1763,7 +1742,9 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
>   	dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
>   	I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
>   
> -	pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, dig_hotplug_reg, hpd_cpt);
> +	intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> +			   dig_hotplug_reg, hpd_cpt,
> +			   pch_port_hotplug_long_detect);
>   	intel_hpd_irq_handler(dev, pin_mask, long_mask);
>   
>   	if (pch_iir & SDE_AUDIO_POWER_MASK_CPT) {
> @@ -1979,7 +1960,8 @@ static void bxt_hpd_handler(struct drm_device *dev, uint32_t iir_status)
>   	/* Clear sticky bits in hpd status */
>   	I915_WRITE(BXT_HOTPLUG_CTL, hp_control);
>   
> -	pch_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control, hpd_bxt);
> +	intel_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control,
> +			   hpd_bxt, pch_port_hotplug_long_detect);
>   	intel_hpd_irq_handler(dev, pin_mask, long_mask);
>   }
>   
a what if question :), before this change only intel_hpd_irq_handler was 
platform independent and
it was in intel_hotplug.c file. Now we have one more function which is 
(i am assuming) platform independent
and is always called before intel_hpd_irq_handler.

can we move the intel_get_hpd_pins to intel_hotplug.c by renmaing it as 
intel_hpd_get_pins & call the
intel_hpd_irq_handler function at the end of this new function ?

-- 
regards,
Sivakumar

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: don't use HPD_PORT_A as an alias for HPD_NONE

[Sivakumar Thulasimani]

[-- Attachment #1.1: Type: text/plain, Size: 4050 bytes --]



On 7/21/2015 3:13 AM, Imre Deak wrote:
> Currently HPD_PORT_A is used as an alias for HPD_NONE to mean that the
> given port doesn't support long/short HPD pulse detection. SDVO and CRT
> ports are like this and for these ports we only want to know whether an
> hot plug event was detected on the corresponding pin. Since at least on
> BXT we need long/short pulse detection on PORT A as well (added by the
> next patch) remove this aliasing of HPD_PORT_A/HPD_NONE and let the
> return value of intel_hpd_pin_to_port() show whether long/short pulse
> detection is supported on the passed in pin.
>
> No functional change.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h      |  4 ++--
>   drivers/gpu/drm/i915/i915_irq.c      |  4 ++--
>   drivers/gpu/drm/i915/intel_hotplug.c | 20 +++++++++++++-------
>   3 files changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 78d6c7a..ebfd5a5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -206,11 +206,11 @@ enum intel_display_power_domain {
>   
>   enum hpd_pin {
>   	HPD_NONE = 0,
> -	HPD_PORT_A = HPD_NONE, /* PORT_A is internal */
>   	HPD_TV = HPD_NONE,     /* TV is known to be unreliable */
>   	HPD_CRT,
>   	HPD_SDVO_B,
>   	HPD_SDVO_C,
> +	HPD_PORT_A,
>   	HPD_PORT_B,
>   	HPD_PORT_C,
>   	HPD_PORT_D,
> @@ -2627,7 +2627,7 @@ void intel_hpd_irq_handler(struct drm_device *dev, u32 pin_mask, u32 long_mask);
>   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(enum hpd_pin pin);
> +bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port);
>   
>   /* i915_irq.c */
>   void i915_queue_hangcheck(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 306de78..4ad7a31 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1276,8 +1276,8 @@ static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
>   
>   		*pin_mask |= BIT(i);
>   
> -		port = intel_hpd_pin_to_port(i);
> -		if (long_pulse_detect(port, dig_hotplug_reg))
> +		if (intel_hpd_pin_to_port(i, &port) &&
> +		    long_pulse_detect(port, dig_hotplug_reg))
>   			*long_mask |= BIT(i);
>   	}
feel that this could be more readable when split it to two conditions ?
if (intel_hpd_pin_to_port(i, &port) == false)
     continue;
if (long_pulse_detect(port, dig_hotplug_reg)
     *long_mask |= BIT(i);

other than this, this change looks good.

Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>


>   
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index 3c53aac..8cda7b9 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -29,17 +29,23 @@
>   #include "i915_drv.h"
>   #include "intel_drv.h"
>   
> -enum port intel_hpd_pin_to_port(enum hpd_pin pin)
> +bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port)
>   {
>   	switch (pin) {
> +	case HPD_PORT_A:
> +		*port = PORT_A;
> +		return true;
>   	case HPD_PORT_B:
> -		return PORT_B;
> +		*port = PORT_B;
> +		return true;
>   	case HPD_PORT_C:
> -		return PORT_C;
> +		*port = PORT_C;
> +		return true;
>   	case HPD_PORT_D:
> -		return PORT_D;
> +		*port = PORT_D;
> +		return true;
>   	default:
> -		return PORT_A; /* no hpd */
> +		return false;	/* no hpd */
>   	}
>   }
>   
> @@ -322,8 +328,8 @@ void intel_hpd_irq_handler(struct drm_device *dev,
>   		if (!(BIT(i) & pin_mask))
>   			continue;
>   
> -		port = intel_hpd_pin_to_port(i);
> -		is_dig_port = port && dev_priv->hotplug.irq_port[port];
> +		is_dig_port = intel_hpd_pin_to_port(i, &port) &&
> +			      dev_priv->hotplug.irq_port[port];
>   
>   		if (is_dig_port) {
>   			bool long_hpd = long_mask & BIT(i);

-- 
regards,
Sivakumar


[-- Attachment #1.2: Type: text/html, Size: 5062 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] drm/i915/bxt: HPD long/short pulse detect support on port A

[Imre Deak]

On Tue, 2015-07-21 at 11:40 +0530, Jindal, Sonika wrote:
> For the patch 3, the commit message can be changed to only "long pulse 
> detection" instead of "long/short" because you are not adding support 
> for short pulse detection.

Well it is support for differentiating between long and short pulses. We
don't check for the short pulse HW flag, but we do imply it in
intel_hpd_irq_handler() if the long pulse flag is not set.

> 
> Otherwise, this series looks good to me.
> 
> Reviewed-by: Sonika Jindal <sonika.jindal@intel.com>
> 
> On 7/21/2015 3:13 AM, Imre Deak wrote:
> > This patchset is a dependency for enabling generic HPD support on the
> > port A pin.
> >
> > Imre Deak (3):
> >    drm/i915: combine i9xx_get_hpd_pins and pch_get_hpd_pins
> >    drm/i915: don't use HPD_PORT_A as an alias for HPD_NONE
> >    drm/i915/bxt: add support for HPD long/short pulse detection on
> >      HPD_PORT_A pin
> >
> >   drivers/gpu/drm/i915/i915_drv.h      |  4 +-
> >   drivers/gpu/drm/i915/i915_irq.c      | 72 ++++++++++++++++++------------------
> >   drivers/gpu/drm/i915/i915_reg.h      |  5 +++
> >   drivers/gpu/drm/i915/intel_hotplug.c | 20 ++++++----
> >   4 files changed, 55 insertions(+), 46 deletions(-)
> >


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: combine i9xx_get_hpd_pins and pch_get_hpd_pins

[Imre Deak]

On Tue, 2015-07-21 at 13:50 +0530, Sivakumar Thulasimani wrote:
> 
> On 7/21/2015 3:13 AM, Imre Deak wrote:
> > These functions are quite similar, so combine them with the use of a new
> > argument for a function that detects long pulses. This will be also
> > needed by an upcoming patch adding support for BXT long pulse detection.
> >
> > No functional change.
> >
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_irq.c | 54 ++++++++++++++---------------------------
> >   1 file changed, 18 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index a6fbe64..306de78 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1256,9 +1256,10 @@ static bool i9xx_port_hotplug_long_detect(enum port port, u32 val)
> >   }
> >   
> >   /* Get a bit mask of pins that have triggered, and which ones may be long. */
> > -static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
> > +static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
> >   			     u32 hotplug_trigger, u32 dig_hotplug_reg,
> > -			     const u32 hpd[HPD_NUM_PINS])
> > +			     const u32 hpd[HPD_NUM_PINS],
> > +			     bool long_pulse_detect(enum port port, u32 val))
> >   {
> >   	enum port port;
> >   	int i;
> > @@ -1276,7 +1277,7 @@ static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
> >   		*pin_mask |= BIT(i);
> >   
> >   		port = intel_hpd_pin_to_port(i);
> > -		if (pch_port_hotplug_long_detect(port, dig_hotplug_reg))
> > +		if (long_pulse_detect(port, dig_hotplug_reg))
> >   			*long_mask |= BIT(i);
> >   	}
> >   
> > @@ -1285,34 +1286,6 @@ static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
> >   
> >   }
> >   
> > -/* Get a bit mask of pins that have triggered, and which ones may be long. */
> > -static void i9xx_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
> > -			      u32 hotplug_trigger, const u32 hpd[HPD_NUM_PINS])
> > -{
> > -	enum port port;
> > -	int i;
> > -
> > -	*pin_mask = 0;
> > -	*long_mask = 0;
> > -
> > -	if (!hotplug_trigger)
> > -		return;
> > -
> > -	for_each_hpd_pin(i) {
> > -		if ((hpd[i] & hotplug_trigger) == 0)
> > -			continue;
> > -
> > -		*pin_mask |= BIT(i);
> > -
> > -		port = intel_hpd_pin_to_port(i);
> > -		if (i9xx_port_hotplug_long_detect(port, hotplug_trigger))
> > -			*long_mask |= BIT(i);
> > -	}
> > -
> > -	DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x, pins 0x%08x\n",
> > -			 hotplug_trigger, *pin_mask);
> > -}
> > -
> >   static void gmbus_irq_handler(struct drm_device *dev)
> >   {
> >   	struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -1550,7 +1523,9 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)
> >   	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
> >   		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
> >   
> > -		i9xx_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, hpd_status_g4x);
> > +		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> > +				   hotplug_trigger, hpd_status_g4x,
> > +				   i9xx_port_hotplug_long_detect);
> >   		intel_hpd_irq_handler(dev, pin_mask, long_mask);
> >   
> >   		if (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
> > @@ -1558,7 +1533,9 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)
> >   	} else {
> >   		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
> >   
> > -		i9xx_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, hpd_status_i915);
> > +		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> > +				   hotplug_trigger, hpd_status_g4x,
> > +				   i9xx_port_hotplug_long_detect);
> >   		intel_hpd_irq_handler(dev, pin_mask, long_mask);
> >   	}
> >   }
> > @@ -1664,7 +1641,9 @@ static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir)
> >   	dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
> >   	I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
> >   
> > -	pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, dig_hotplug_reg, hpd_ibx);
> > +	intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> > +			   dig_hotplug_reg, hpd_ibx,
> > +			   pch_port_hotplug_long_detect);
> >   	intel_hpd_irq_handler(dev, pin_mask, long_mask);
> >   
> >   	if (pch_iir & SDE_AUDIO_POWER_MASK) {
> > @@ -1763,7 +1742,9 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
> >   	dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
> >   	I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
> >   
> > -	pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, dig_hotplug_reg, hpd_cpt);
> > +	intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> > +			   dig_hotplug_reg, hpd_cpt,
> > +			   pch_port_hotplug_long_detect);
> >   	intel_hpd_irq_handler(dev, pin_mask, long_mask);
> >   
> >   	if (pch_iir & SDE_AUDIO_POWER_MASK_CPT) {
> > @@ -1979,7 +1960,8 @@ static void bxt_hpd_handler(struct drm_device *dev, uint32_t iir_status)
> >   	/* Clear sticky bits in hpd status */
> >   	I915_WRITE(BXT_HOTPLUG_CTL, hp_control);
> >   
> > -	pch_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control, hpd_bxt);
> > +	intel_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control,
> > +			   hpd_bxt, pch_port_hotplug_long_detect);
> >   	intel_hpd_irq_handler(dev, pin_mask, long_mask);
> >   }
> >   
> a what if question :), before this change only intel_hpd_irq_handler was 
> platform independent and
> it was in intel_hotplug.c file. Now we have one more function which is 
> (i am assuming) platform independent
> and is always called before intel_hpd_irq_handler.
> 
> can we move the intel_get_hpd_pins to intel_hotplug.c by renmaing it as 
> intel_hpd_get_pins & call the
> intel_hpd_irq_handler function at the end of this new function ?

Yes, factoring out intel_get_hpd_pins() and intel_hpd_irq_handler() into
a new function is a good idea. I think it's best done as follow-up
though, so maybe you could do this after this patchset is merged?

--Imre

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: don't use HPD_PORT_A as an alias for HPD_NONE

[Imre Deak]

On Tue, 2015-07-21 at 13:51 +0530, Sivakumar Thulasimani wrote:
> 
> 
> On 7/21/2015 3:13 AM, Imre Deak wrote:
> 
> > Currently HPD_PORT_A is used as an alias for HPD_NONE to mean that the
> > given port doesn't support long/short HPD pulse detection. SDVO and CRT
> > ports are like this and for these ports we only want to know whether an
> > hot plug event was detected on the corresponding pin. Since at least on
> > BXT we need long/short pulse detection on PORT A as well (added by the
> > next patch) remove this aliasing of HPD_PORT_A/HPD_NONE and let the
> > return value of intel_hpd_pin_to_port() show whether long/short pulse
> > detection is supported on the passed in pin.
> > 
> > No functional change.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      |  4 ++--
> >  drivers/gpu/drm/i915/i915_irq.c      |  4 ++--
> >  drivers/gpu/drm/i915/intel_hotplug.c | 20 +++++++++++++-------
> >  3 files changed, 17 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 78d6c7a..ebfd5a5 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -206,11 +206,11 @@ enum intel_display_power_domain {
> >  
> >  enum hpd_pin {
> >  	HPD_NONE = 0,
> > -	HPD_PORT_A = HPD_NONE, /* PORT_A is internal */
> >  	HPD_TV = HPD_NONE,     /* TV is known to be unreliable */
> >  	HPD_CRT,
> >  	HPD_SDVO_B,
> >  	HPD_SDVO_C,
> > +	HPD_PORT_A,
> >  	HPD_PORT_B,
> >  	HPD_PORT_C,
> >  	HPD_PORT_D,
> > @@ -2627,7 +2627,7 @@ void intel_hpd_irq_handler(struct drm_device *dev, u32 pin_mask, u32 long_mask);
> >  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(enum hpd_pin pin);
> > +bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port);
> >  
> >  /* i915_irq.c */
> >  void i915_queue_hangcheck(struct drm_device *dev);
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 306de78..4ad7a31 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1276,8 +1276,8 @@ static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
> >  
> >  		*pin_mask |= BIT(i);
> >  
> > -		port = intel_hpd_pin_to_port(i);
> > -		if (long_pulse_detect(port, dig_hotplug_reg))
> > +		if (intel_hpd_pin_to_port(i, &port) &&
> > +		    long_pulse_detect(port, dig_hotplug_reg))
> >  			*long_mask |= BIT(i);
> >  	}
> feel that this could be more readable when split it to two
> conditions ? 
> if (intel_hpd_pin_to_port(i, &port) == false)
>     continue;
> if (long_pulse_detect(port, dig_hotplug_reg)
>     *long_mask |= BIT(i);

Ok, will change it.

> 
> other than this, this change looks good. 
> 
> 
> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> 
> 
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> > index 3c53aac..8cda7b9 100644
> > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > @@ -29,17 +29,23 @@
> >  #include "i915_drv.h"
> >  #include "intel_drv.h"
> >  
> > -enum port intel_hpd_pin_to_port(enum hpd_pin pin)
> > +bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port)
> >  {
> >  	switch (pin) {
> > +	case HPD_PORT_A:
> > +		*port = PORT_A;
> > +		return true;
> >  	case HPD_PORT_B:
> > -		return PORT_B;
> > +		*port = PORT_B;
> > +		return true;
> >  	case HPD_PORT_C:
> > -		return PORT_C;
> > +		*port = PORT_C;
> > +		return true;
> >  	case HPD_PORT_D:
> > -		return PORT_D;
> > +		*port = PORT_D;
> > +		return true;
> >  	default:
> > -		return PORT_A; /* no hpd */
> > +		return false;	/* no hpd */
> >  	}
> >  }
> >  
> > @@ -322,8 +328,8 @@ void intel_hpd_irq_handler(struct drm_device *dev,
> >  		if (!(BIT(i) & pin_mask))
> >  			continue;
> >  
> > -		port = intel_hpd_pin_to_port(i);
> > -		is_dig_port = port && dev_priv->hotplug.irq_port[port];
> > +		is_dig_port = intel_hpd_pin_to_port(i, &port) &&
> > +			      dev_priv->hotplug.irq_port[port];
> >  
> >  		if (is_dig_port) {
> >  			bool long_hpd = long_mask & BIT(i);
> 
> -- 
> regards,
> Sivakumar


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 1/3] drm/i915: combine i9xx_get_hpd_pins and pch_get_hpd_pins

[Imre Deak]

These functions are quite similar, so combine them with the use of a new
argument for a function that detects long pulses. This will be also
needed by an upcoming patch adding support for BXT long pulse detection.

No functional change.

v2:
- rebase on top -nightly (Daniel)

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Sonika Jindal <sonika.jindal@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 57 ++++++++++++++---------------------------
 1 file changed, 19 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d87f173..9410aab 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1256,9 +1256,10 @@ static bool i9xx_port_hotplug_long_detect(enum port port, u32 val)
 }
 
 /* Get a bit mask of pins that have triggered, and which ones may be long. */
-static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
+static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
 			     u32 hotplug_trigger, u32 dig_hotplug_reg,
-			     const u32 hpd[HPD_NUM_PINS])
+			     const u32 hpd[HPD_NUM_PINS],
+			     bool long_pulse_detect(enum port port, u32 val))
 {
 	enum port port;
 	int i;
@@ -1273,7 +1274,7 @@ static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
 		*pin_mask |= BIT(i);
 
 		port = intel_hpd_pin_to_port(i);
-		if (pch_port_hotplug_long_detect(port, dig_hotplug_reg))
+		if (long_pulse_detect(port, dig_hotplug_reg))
 			*long_mask |= BIT(i);
 	}
 
@@ -1282,34 +1283,6 @@ static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
 
 }
 
-/* Get a bit mask of pins that have triggered, and which ones may be long. */
-static void i9xx_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
-			      u32 hotplug_trigger, const u32 hpd[HPD_NUM_PINS])
-{
-	enum port port;
-	int i;
-
-	*pin_mask = 0;
-	*long_mask = 0;
-
-	if (!hotplug_trigger)
-		return;
-
-	for_each_hpd_pin(i) {
-		if ((hpd[i] & hotplug_trigger) == 0)
-			continue;
-
-		*pin_mask |= BIT(i);
-
-		port = intel_hpd_pin_to_port(i);
-		if (i9xx_port_hotplug_long_detect(port, hotplug_trigger))
-			*long_mask |= BIT(i);
-	}
-
-	DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x, pins 0x%08x\n",
-			 hotplug_trigger, *pin_mask);
-}
-
 static void gmbus_irq_handler(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1547,7 +1520,9 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)
 	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
 		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
 
-		i9xx_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, hpd_status_g4x);
+		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
+				   hotplug_trigger, hpd_status_g4x,
+				   i9xx_port_hotplug_long_detect);
 		intel_hpd_irq_handler(dev, pin_mask, long_mask);
 
 		if (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
@@ -1555,7 +1530,9 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)
 	} else {
 		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
 
-		i9xx_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, hpd_status_i915);
+		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
+				   hotplug_trigger, hpd_status_g4x,
+				   i9xx_port_hotplug_long_detect);
 		intel_hpd_irq_handler(dev, pin_mask, long_mask);
 	}
 }
@@ -1662,8 +1639,9 @@ static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir)
 		dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
 		I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
 
-		pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
-				 dig_hotplug_reg, hpd_ibx);
+		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
+				   dig_hotplug_reg, hpd_ibx,
+				   pch_port_hotplug_long_detect);
 		intel_hpd_irq_handler(dev, pin_mask, long_mask);
 	}
 
@@ -1763,8 +1741,10 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
 
 		dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
 		I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
-		pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
-				 dig_hotplug_reg, hpd_cpt);
+
+		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
+				   dig_hotplug_reg, hpd_cpt,
+				   pch_port_hotplug_long_detect);
 		intel_hpd_irq_handler(dev, pin_mask, long_mask);
 	}
 
@@ -1981,7 +1961,8 @@ static void bxt_hpd_handler(struct drm_device *dev, uint32_t iir_status)
 	/* Clear sticky bits in hpd status */
 	I915_WRITE(BXT_HOTPLUG_CTL, hp_control);
 
-	pch_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control, hpd_bxt);
+	intel_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control,
+			   hpd_bxt, pch_port_hotplug_long_detect);
 	intel_hpd_irq_handler(dev, pin_mask, long_mask);
 }
 
-- 
2.1.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/3] drm/i915: don't use HPD_PORT_A as an alias for HPD_NONE

[Imre Deak]

Currently HPD_PORT_A is used as an alias for HPD_NONE to mean that the
given port doesn't support long/short HPD pulse detection. SDVO and CRT
ports are like this and for these ports we only want to know whether an
hot plug event was detected on the corresponding pin. Since at least on
BXT we need long/short pulse detection on PORT A as well (added by the
next patch) remove this aliasing of HPD_PORT_A/HPD_NONE and let the
return value of intel_hpd_pin_to_port() show whether long/short pulse
detection is supported on the passed in pin.

No functional change.

v2:
- rebase on top of -nightly (Daniel)
- make the check for intel_hpd_pin_to_port() return value more readable
  (Sivakumar)

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Sonika Jindal <sonika.jindal@intel.com>
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  4 ++--
 drivers/gpu/drm/i915/i915_irq.c      |  4 +++-
 drivers/gpu/drm/i915/intel_hotplug.c | 20 +++++++++++++-------
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b876e78..b94ada9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -206,11 +206,11 @@ enum intel_display_power_domain {
 
 enum hpd_pin {
 	HPD_NONE = 0,
-	HPD_PORT_A = HPD_NONE, /* PORT_A is internal */
 	HPD_TV = HPD_NONE,     /* TV is known to be unreliable */
 	HPD_CRT,
 	HPD_SDVO_B,
 	HPD_SDVO_C,
+	HPD_PORT_A,
 	HPD_PORT_B,
 	HPD_PORT_C,
 	HPD_PORT_D,
@@ -2648,7 +2648,7 @@ void intel_hpd_irq_handler(struct drm_device *dev, u32 pin_mask, u32 long_mask);
 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(enum hpd_pin pin);
+bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port);
 
 /* i915_irq.c */
 void i915_queue_hangcheck(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9410aab..f08ec9c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1273,7 +1273,9 @@ static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
 
 		*pin_mask |= BIT(i);
 
-		port = intel_hpd_pin_to_port(i);
+		if (!intel_hpd_pin_to_port(i, &port))
+			continue;
+
 		if (long_pulse_detect(port, dig_hotplug_reg))
 			*long_mask |= BIT(i);
 	}
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index 3c9171f..032a0bf 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -76,17 +76,23 @@
  * it will use i915_hotplug_work_func where this logic is handled.
  */
 
-enum port intel_hpd_pin_to_port(enum hpd_pin pin)
+bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port)
 {
 	switch (pin) {
+	case HPD_PORT_A:
+		*port = PORT_A;
+		return true;
 	case HPD_PORT_B:
-		return PORT_B;
+		*port = PORT_B;
+		return true;
 	case HPD_PORT_C:
-		return PORT_C;
+		*port = PORT_C;
+		return true;
 	case HPD_PORT_D:
-		return PORT_D;
+		*port = PORT_D;
+		return true;
 	default:
-		return PORT_A; /* no hpd */
+		return false;	/* no hpd */
 	}
 }
 
@@ -369,8 +375,8 @@ void intel_hpd_irq_handler(struct drm_device *dev,
 		if (!(BIT(i) & pin_mask))
 			continue;
 
-		port = intel_hpd_pin_to_port(i);
-		is_dig_port = port && dev_priv->hotplug.irq_port[port];
+		is_dig_port = intel_hpd_pin_to_port(i, &port) &&
+			      dev_priv->hotplug.irq_port[port];
 
 		if (is_dig_port) {
 			bool long_hpd = long_mask & BIT(i);
-- 
2.1.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: combine i9xx_get_hpd_pins and pch_get_hpd_pins

[Sivakumar Thulasimani]

On 7/21/2015 11:28 PM, Imre Deak wrote:
> On Tue, 2015-07-21 at 13:50 +0530, Sivakumar Thulasimani wrote:
>> On 7/21/2015 3:13 AM, Imre Deak wrote:
>>> These functions are quite similar, so combine them with the use of a new
>>> argument for a function that detects long pulses. This will be also
>>> needed by an upcoming patch adding support for BXT long pulse detection.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/i915_irq.c | 54 ++++++++++++++---------------------------
>>>    1 file changed, 18 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>>> index a6fbe64..306de78 100644
>>> --- a/drivers/gpu/drm/i915/i915_irq.c
>>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>>> @@ -1256,9 +1256,10 @@ static bool i9xx_port_hotplug_long_detect(enum port port, u32 val)
>>>    }
>>>    
>>>    /* Get a bit mask of pins that have triggered, and which ones may be long. */
>>> -static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
>>> +static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
>>>    			     u32 hotplug_trigger, u32 dig_hotplug_reg,
>>> -			     const u32 hpd[HPD_NUM_PINS])
>>> +			     const u32 hpd[HPD_NUM_PINS],
>>> +			     bool long_pulse_detect(enum port port, u32 val))
>>>    {
>>>    	enum port port;
>>>    	int i;
>>> @@ -1276,7 +1277,7 @@ static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
>>>    		*pin_mask |= BIT(i);
>>>    
>>>    		port = intel_hpd_pin_to_port(i);
>>> -		if (pch_port_hotplug_long_detect(port, dig_hotplug_reg))
>>> +		if (long_pulse_detect(port, dig_hotplug_reg))
>>>    			*long_mask |= BIT(i);
>>>    	}
>>>    
>>> @@ -1285,34 +1286,6 @@ static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
>>>    
>>>    }
>>>    
>>> -/* Get a bit mask of pins that have triggered, and which ones may be long. */
>>> -static void i9xx_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
>>> -			      u32 hotplug_trigger, const u32 hpd[HPD_NUM_PINS])
>>> -{
>>> -	enum port port;
>>> -	int i;
>>> -
>>> -	*pin_mask = 0;
>>> -	*long_mask = 0;
>>> -
>>> -	if (!hotplug_trigger)
>>> -		return;
>>> -
>>> -	for_each_hpd_pin(i) {
>>> -		if ((hpd[i] & hotplug_trigger) == 0)
>>> -			continue;
>>> -
>>> -		*pin_mask |= BIT(i);
>>> -
>>> -		port = intel_hpd_pin_to_port(i);
>>> -		if (i9xx_port_hotplug_long_detect(port, hotplug_trigger))
>>> -			*long_mask |= BIT(i);
>>> -	}
>>> -
>>> -	DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x, pins 0x%08x\n",
>>> -			 hotplug_trigger, *pin_mask);
>>> -}
>>> -
>>>    static void gmbus_irq_handler(struct drm_device *dev)
>>>    {
>>>    	struct drm_i915_private *dev_priv = dev->dev_private;
>>> @@ -1550,7 +1523,9 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)
>>>    	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
>>>    		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
>>>    
>>> -		i9xx_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, hpd_status_g4x);
>>> +		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
>>> +				   hotplug_trigger, hpd_status_g4x,
>>> +				   i9xx_port_hotplug_long_detect);
>>>    		intel_hpd_irq_handler(dev, pin_mask, long_mask);
>>>    
>>>    		if (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
>>> @@ -1558,7 +1533,9 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)
>>>    	} else {
>>>    		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
>>>    
>>> -		i9xx_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, hpd_status_i915);
>>> +		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
>>> +				   hotplug_trigger, hpd_status_g4x,
>>> +				   i9xx_port_hotplug_long_detect);
>>>    		intel_hpd_irq_handler(dev, pin_mask, long_mask);
>>>    	}
>>>    }
>>> @@ -1664,7 +1641,9 @@ static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir)
>>>    	dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
>>>    	I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
>>>    
>>> -	pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, dig_hotplug_reg, hpd_ibx);
>>> +	intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
>>> +			   dig_hotplug_reg, hpd_ibx,
>>> +			   pch_port_hotplug_long_detect);
>>>    	intel_hpd_irq_handler(dev, pin_mask, long_mask);
>>>    
>>>    	if (pch_iir & SDE_AUDIO_POWER_MASK) {
>>> @@ -1763,7 +1742,9 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
>>>    	dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
>>>    	I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
>>>    
>>> -	pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, dig_hotplug_reg, hpd_cpt);
>>> +	intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
>>> +			   dig_hotplug_reg, hpd_cpt,
>>> +			   pch_port_hotplug_long_detect);
>>>    	intel_hpd_irq_handler(dev, pin_mask, long_mask);
>>>    
>>>    	if (pch_iir & SDE_AUDIO_POWER_MASK_CPT) {
>>> @@ -1979,7 +1960,8 @@ static void bxt_hpd_handler(struct drm_device *dev, uint32_t iir_status)
>>>    	/* Clear sticky bits in hpd status */
>>>    	I915_WRITE(BXT_HOTPLUG_CTL, hp_control);
>>>    
>>> -	pch_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control, hpd_bxt);
>>> +	intel_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control,
>>> +			   hpd_bxt, pch_port_hotplug_long_detect);
>>>    	intel_hpd_irq_handler(dev, pin_mask, long_mask);
>>>    }
>>>    
>> a what if question :), before this change only intel_hpd_irq_handler was
>> platform independent and
>> it was in intel_hotplug.c file. Now we have one more function which is
>> (i am assuming) platform independent
>> and is always called before intel_hpd_irq_handler.
>>
>> can we move the intel_get_hpd_pins to intel_hotplug.c by renmaing it as
>> intel_hpd_get_pins & call the
>> intel_hpd_irq_handler function at the end of this new function ?
> Yes, factoring out intel_get_hpd_pins() and intel_hpd_irq_handler() into
> a new function is a good idea. I think it's best done as follow-up
> though, so maybe you could do this after this patchset is merged?
>
> --Imre
>
Sure :) i'll take it up post this gets merged.

-- 
regards,
Sivakumar

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/3] drm/i915: combine i9xx_get_hpd_pins and pch_get_hpd_pins

[Sivakumar Thulasimani]

[-- Attachment #1.1: Type: text/plain, Size: 5148 bytes --]



On 7/22/2015 4:02 AM, Imre Deak wrote:
> These functions are quite similar, so combine them with the use of a new
> argument for a function that detects long pulses. This will be also
> needed by an upcoming patch adding support for BXT long pulse detection.
>
> No functional change.
>
> v2:
> - rebase on top -nightly (Daniel)
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> Reviewed-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_irq.c | 57 ++++++++++++++---------------------------
>   1 file changed, 19 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d87f173..9410aab 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1256,9 +1256,10 @@ static bool i9xx_port_hotplug_long_detect(enum port port, u32 val)
>   }
>   
>   /* Get a bit mask of pins that have triggered, and which ones may be long. */
> -static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
> +static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
>   			     u32 hotplug_trigger, u32 dig_hotplug_reg,
> -			     const u32 hpd[HPD_NUM_PINS])
> +			     const u32 hpd[HPD_NUM_PINS],
> +			     bool long_pulse_detect(enum port port, u32 val))
>   {
>   	enum port port;
>   	int i;
> @@ -1273,7 +1274,7 @@ static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
>   		*pin_mask |= BIT(i);
>   
>   		port = intel_hpd_pin_to_port(i);
> -		if (pch_port_hotplug_long_detect(port, dig_hotplug_reg))
> +		if (long_pulse_detect(port, dig_hotplug_reg))
>   			*long_mask |= BIT(i);
>   	}
>   
> @@ -1282,34 +1283,6 @@ static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
>   
>   }
>   
> -/* Get a bit mask of pins that have triggered, and which ones may be long. */
> -static void i9xx_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
> -			      u32 hotplug_trigger, const u32 hpd[HPD_NUM_PINS])
> -{
> -	enum port port;
> -	int i;
> -
> -	*pin_mask = 0;
> -	*long_mask = 0;
> -
> -	if (!hotplug_trigger)
> -		return;
> -
> -	for_each_hpd_pin(i) {
> -		if ((hpd[i] & hotplug_trigger) == 0)
> -			continue;
> -
> -		*pin_mask |= BIT(i);
> -
> -		port = intel_hpd_pin_to_port(i);
> -		if (i9xx_port_hotplug_long_detect(port, hotplug_trigger))
> -			*long_mask |= BIT(i);
> -	}
> -
> -	DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x, pins 0x%08x\n",
> -			 hotplug_trigger, *pin_mask);
> -}
> -
>   static void gmbus_irq_handler(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1547,7 +1520,9 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)
>   	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
>   		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
>   
> -		i9xx_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, hpd_status_g4x);
> +		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> +				   hotplug_trigger, hpd_status_g4x,
> +				   i9xx_port_hotplug_long_detect);
>   		intel_hpd_irq_handler(dev, pin_mask, long_mask);
>   
>   		if (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
> @@ -1555,7 +1530,9 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)
>   	} else {
>   		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
>   
> -		i9xx_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, hpd_status_i915);
> +		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> +				   hotplug_trigger, hpd_status_g4x,
> +				   i9xx_port_hotplug_long_detect);
>   		intel_hpd_irq_handler(dev, pin_mask, long_mask);
>   	}
>   }
> @@ -1662,8 +1639,9 @@ static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir)
>   		dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
>   		I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
>   
> -		pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> -				 dig_hotplug_reg, hpd_ibx);
> +		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> +				   dig_hotplug_reg, hpd_ibx,
> +				   pch_port_hotplug_long_detect);
>   		intel_hpd_irq_handler(dev, pin_mask, long_mask);
>   	}
>   
> @@ -1763,8 +1741,10 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
>   
>   		dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
>   		I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
> -		pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> -				 dig_hotplug_reg, hpd_cpt);
> +
> +		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> +				   dig_hotplug_reg, hpd_cpt,
> +				   pch_port_hotplug_long_detect);
>   		intel_hpd_irq_handler(dev, pin_mask, long_mask);
>   	}
>   
> @@ -1981,7 +1961,8 @@ static void bxt_hpd_handler(struct drm_device *dev, uint32_t iir_status)
>   	/* Clear sticky bits in hpd status */
>   	I915_WRITE(BXT_HOTPLUG_CTL, hp_control);
>   
> -	pch_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control, hpd_bxt);
> +	intel_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control,
> +			   hpd_bxt, pch_port_hotplug_long_detect);
>   	intel_hpd_irq_handler(dev, pin_mask, long_mask);
>   }
>   

Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>

-- 
regards,
Sivakumar


[-- Attachment #1.2: Type: text/html, Size: 5874 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/3] drm/i915: don't use HPD_PORT_A as an alias for HPD_NONE

[Sivakumar Thulasimani]

[-- Attachment #1.1: Type: text/plain, Size: 4058 bytes --]



On 7/22/2015 4:02 AM, Imre Deak wrote:
> Curren tly HPD_PORT_A is used as an alias for HPD_NONE to mean that the
> given port doesn't support long/short HPD pulse detection. SDVO and CRT
> ports are like this and for these ports we only want to know whether an
> hot plug event was detected on the corresponding pin. Since at least on
> BXT we need long/short pulse detection on PORT A as well (added by the
> next patch) remove this aliasing of HPD_PORT_A/HPD_NONE and let the
> return value of intel_hpd_pin_to_port() show whether long/short pulse
> detection is supported on the passed in
> pin.
>
> No functional change.
>
> v2:
> - rebase on top of -nightly (Daniel)
> - make the check for intel_hpd_pin_to_port() return value more readable
>    (Sivakumar)
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> Reviewed-by: Sonika Jindal <sonika.jindal@intel.com>
> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h      |  4 ++--
>   drivers/gpu/drm/i915/i915_irq.c      |  4 +++-
>   drivers/gpu/drm/i915/intel_hotplug.c | 20 +++++++++++++-------
>   3 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b876e78..b94ada9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -206,11 +206,11 @@ enum intel_display_power_domain {
>   
>   enum hpd_pin {
>   	HPD_NONE = 0,
> -	HPD_PORT_A = HPD_NONE, /* PORT_A is internal */
>   	HPD_TV = HPD_NONE,     /* TV is known to be unreliable */
>   	HPD_CRT,
>   	HPD_SDVO_B,
>   	HPD_SDVO_C,
> +	HPD_PORT_A,
>   	HPD_PORT_B,
>   	HPD_PORT_C,
>   	HPD_PORT_D,
> @@ -2648,7 +2648,7 @@ void intel_hpd_irq_handler(struct drm_device *dev, u32 pin_mask, u32 long_mask);
>   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(enum hpd_pin pin);
> +bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port);
>   
>   /* i915_irq.c */
>   void i915_queue_hangcheck(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 9410aab..f08ec9c 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1273,7 +1273,9 @@ static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
>   
>   		*pin_mask |= BIT(i);
>   
> -		port = intel_hpd_pin_to_port(i);
> +		if (!intel_hpd_pin_to_port(i, &port))
> +			continue;
> +
>   		if (long_pulse_detect(port, dig_hotplug_reg))
>   			*long_mask |= BIT(i);
>   	}
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index 3c9171f..032a0bf 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -76,17 +76,23 @@
>    * it will use i915_hotplug_work_func where this logic is handled.
>    */
>   
> -enum port intel_hpd_pin_to_port(enum hpd_pin pin)
> +bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port)
>   {
>   	switch (pin) {
> +	case HPD_PORT_A:
> +		*port = PORT_A;
> +		return true;
>   	case HPD_PORT_B:
> -		return PORT_B;
> +		*port = PORT_B;
> +		return true;
>   	case HPD_PORT_C:
> -		return PORT_C;
> +		*port = PORT_C;
> +		return true;
>   	case HPD_PORT_D:
> -		return PORT_D;
> +		*port = PORT_D;
> +		return true;
>   	default:
> -		return PORT_A; /* no hpd */
> +		return false;	/* no hpd */
>   	}
>   }
>   
> @@ -369,8 +375,8 @@ void intel_hpd_irq_handler(struct drm_device *dev,
>   		if (!(BIT(i) & pin_mask))
>   			continue;
>   
> -		port = intel_hpd_pin_to_port(i);
> -		is_dig_port = port && dev_priv->hotplug.irq_port[port];
> +		is_dig_port = intel_hpd_pin_to_port(i, &port) &&
> +			      dev_priv->hotplug.irq_port[port];
>   
>   		if (is_dig_port) {
>   			bool long_hpd = long_mask & BIT(i);

Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>

-- 
regards,
Sivakumar


[-- Attachment #1.2: Type: text/html, Size: 4862 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/bxt: add support for HPD long/short pulse detection on HPD_PORT_A pin

[Sivakumar Thulasimani]

[-- Attachment #1.1: Type: text/plain, Size: 2440 bytes --]



On 7/21/2015 3:13 AM, Imre Deak wrote:
> This is a requirement for enabling display port HPD support on the port
> A HPD pin. This support is to be added by follow-up patches.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_irq.c | 18 +++++++++++++++++-
>   drivers/gpu/drm/i915/i915_reg.h |  5 +++++
>   2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 4ad7a31..02b9e73 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1227,6 +1227,22 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_i915_private *dev_priv,
>   	return ret;
>   }
>   
> +static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
> +{
> +	switch (port) {
> +	case PORT_A:
> +		return val & BXT_PORTA_HOTPLUG_LONG_DETECT;
> +	case PORT_B:
> +		return val & PORTB_HOTPLUG_LONG_DETECT;
> +	case PORT_C:
> +		return val & PORTC_HOTPLUG_LONG_DETECT;
> +	case PORT_D:
> +		return val & PORTD_HOTPLUG_LONG_DETECT;
> +	default:
> +		return false;
> +	}
> +}
> +
>   static bool pch_port_hotplug_long_detect(enum port port, u32 val)
>   {
>   	switch (port) {
> @@ -1961,7 +1977,7 @@ static void bxt_hpd_handler(struct drm_device *dev, uint32_t iir_status)
>   	I915_WRITE(BXT_HOTPLUG_CTL, hp_control);
>   
>   	intel_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control,
> -			   hpd_bxt, pch_port_hotplug_long_detect);
> +			   hpd_bxt, bxt_port_hotplug_long_detect);
>   	intel_hpd_irq_handler(dev, pin_mask, long_mask);
>   }
>   
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index fc70035..be166b3 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5942,6 +5942,11 @@ enum skl_disp_power_wells {
>   
>   /* digital port hotplug */
>   #define PCH_PORT_HOTPLUG        0xc4030		/* SHOTPLUG_CTL */
> +#define BXT_PORTA_HOTPLUG_ENABLE	(1 << 28)
> +#define BXT_PORTA_HOTPLUG_STATUS_MASK	(0x3 << 24)
> +#define  BXT_PORTA_HOTPLUG_NO_DETECT	(0 << 24)
> +#define  BXT_PORTA_HOTPLUG_SHORT_DETECT	(1 << 24)
> +#define  BXT_PORTA_HOTPLUG_LONG_DETECT	(2 << 24)
>   #define PORTD_HOTPLUG_ENABLE            (1 << 20)
>   #define PORTD_PULSE_DURATION_2ms        (0)
>   #define PORTD_PULSE_DURATION_4_5ms      (1 << 18)

Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>

-- 
regards,
Sivakumar


[-- Attachment #1.2: Type: text/html, Size: 3199 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] drm/i915/bxt: HPD long/short pulse detect support on port A

[Daniel Vetter]

On Tue, Jul 21, 2015 at 10:42:11AM -0700, Imre Deak wrote:
> On Tue, 2015-07-21 at 11:40 +0530, Jindal, Sonika wrote:
> > For the patch 3, the commit message can be changed to only "long pulse 
> > detection" instead of "long/short" because you are not adding support 
> > for short pulse detection.
> 
> Well it is support for differentiating between long and short pulses. We
> don't check for the short pulse HW flag, but we do imply it in
> intel_hpd_irq_handler() if the long pulse flag is not set.

Entire series merged, thanks.
-Daniel

> 
> > 
> > Otherwise, this series looks good to me.
> > 
> > Reviewed-by: Sonika Jindal <sonika.jindal@intel.com>
> > 
> > On 7/21/2015 3:13 AM, Imre Deak wrote:
> > > This patchset is a dependency for enabling generic HPD support on the
> > > port A pin.
> > >
> > > Imre Deak (3):
> > >    drm/i915: combine i9xx_get_hpd_pins and pch_get_hpd_pins
> > >    drm/i915: don't use HPD_PORT_A as an alias for HPD_NONE
> > >    drm/i915/bxt: add support for HPD long/short pulse detection on
> > >      HPD_PORT_A pin
> > >
> > >   drivers/gpu/drm/i915/i915_drv.h      |  4 +-
> > >   drivers/gpu/drm/i915/i915_irq.c      | 72 ++++++++++++++++++------------------
> > >   drivers/gpu/drm/i915/i915_reg.h      |  5 +++
> > >   drivers/gpu/drm/i915/intel_hotplug.c | 20 ++++++----
> > >   4 files changed, 55 insertions(+), 46 deletions(-)
> > >
> 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx