public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Add BXT A0/A1 WA for HPD swap in generic way
@ 2015-08-10  5:05 Sonika Jindal
  2015-08-10  5:05 ` [PATCH 1/3] drm/i915: Dont enable hpd for eDP Sonika Jindal
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Sonika Jindal @ 2015-08-10  5:05 UTC (permalink / raw)
  To: intel-gfx

This series adds BXT HPD pin swap WA for A0/A1 by setting right hpd_pin
and irq_port for PORT_B.

Sonika Jindal (3):
  drm/i915: Dont enable hpd for eDP
  drm/i915/bxt: Add HPD support for DDIA
  drm/i915/bxt: WA for swapped HPD pins in A stepping

 drivers/gpu/drm/i915/i915_irq.c   |   10 +++-------
 drivers/gpu/drm/i915/intel_ddi.c  |   10 +++++++++-
 drivers/gpu/drm/i915/intel_dp.c   |    4 +++-
 drivers/gpu/drm/i915/intel_hdmi.c |    9 ++++++++-
 4 files changed, 23 insertions(+), 10 deletions(-)

-- 
1.7.10.4

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/3] drm/i915: Dont enable hpd for eDP
  2015-08-10  5:05 [PATCH 0/3] Add BXT A0/A1 WA for HPD swap in generic way Sonika Jindal
@ 2015-08-10  5:05 ` Sonika Jindal
  2015-08-10  5:24   ` Sivakumar Thulasimani
  2015-08-10  5:05 ` [PATCH 2/3] drm/i915/bxt: Add HPD support for DDIA Sonika Jindal
  2015-08-10  5:05 ` [PATCH 3/3] drm/i915/bxt: WA for swapped HPD pins in A stepping Sonika Jindal
  2 siblings, 1 reply; 17+ messages in thread
From: Sonika Jindal @ 2015-08-10  5:05 UTC (permalink / raw)
  To: intel-gfx

With HPD support added for all ports including PORT_A, setting hpd_pin will
result in enabling of hpd to edp as well. There is no need to enable HPD on
PORT_A hence this patch removes hpd_pin update for PORT_A, where edp will
be connected. it can be added back when required

Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fcc64e5..5a614c9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5784,7 +5784,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	/* Set up the hotplug pin. */
 	switch (port) {
 	case PORT_A:
-		intel_encoder->hpd_pin = HPD_PORT_A;
+		/* Not enabling edp interrupts */
 		break;
 	case PORT_B:
 		intel_encoder->hpd_pin = HPD_PORT_B;
-- 
1.7.10.4

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/3] drm/i915/bxt: Add HPD support for DDIA
  2015-08-10  5:05 [PATCH 0/3] Add BXT A0/A1 WA for HPD swap in generic way Sonika Jindal
  2015-08-10  5:05 ` [PATCH 1/3] drm/i915: Dont enable hpd for eDP Sonika Jindal
@ 2015-08-10  5:05 ` Sonika Jindal
  2015-08-10  5:27   ` Sivakumar Thulasimani
  2015-08-10  5:05 ` [PATCH 3/3] drm/i915/bxt: WA for swapped HPD pins in A stepping Sonika Jindal
  2 siblings, 1 reply; 17+ messages in thread
From: Sonika Jindal @ 2015-08-10  5:05 UTC (permalink / raw)
  To: intel-gfx

Also remove redundant comments.

Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c |   10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 02b9e73..9b9533a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -90,6 +90,7 @@ static const u32 hpd_status_i915[HPD_NUM_PINS] = {
 
 /* BXT hpd list */
 static const u32 hpd_bxt[HPD_NUM_PINS] = {
+	[HPD_PORT_A] = BXT_DE_PORT_HP_DDIA,
 	[HPD_PORT_B] = BXT_DE_PORT_HP_DDIB,
 	[HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
 };
@@ -3018,30 +3019,25 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
 	u32 hotplug_port = 0;
 	u32 hotplug_ctrl;
 
-	/* Now, enable HPD */
 	for_each_intel_encoder(dev, intel_encoder) {
 		if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state
 				== HPD_ENABLED)
 			hotplug_port |= hpd_bxt[intel_encoder->hpd_pin];
 	}
 
-	/* Mask all HPD control bits */
 	hotplug_ctrl = I915_READ(BXT_HOTPLUG_CTL) & ~BXT_HOTPLUG_CTL_MASK;
 
-	/* Enable requested port in hotplug control */
-	/* TODO: implement (short) HPD support on port A */
-	WARN_ON_ONCE(hotplug_port & BXT_DE_PORT_HP_DDIA);
+	if (hotplug_port & BXT_DE_PORT_HP_DDIA)
+		hotplug_ctrl |= BXT_DDIA_HPD_ENABLE;
 	if (hotplug_port & BXT_DE_PORT_HP_DDIB)
 		hotplug_ctrl |= BXT_DDIB_HPD_ENABLE;
 	if (hotplug_port & BXT_DE_PORT_HP_DDIC)
 		hotplug_ctrl |= BXT_DDIC_HPD_ENABLE;
 	I915_WRITE(BXT_HOTPLUG_CTL, hotplug_ctrl);
 
-	/* Unmask DDI hotplug in IMR */
 	hotplug_ctrl = I915_READ(GEN8_DE_PORT_IMR) & ~hotplug_port;
 	I915_WRITE(GEN8_DE_PORT_IMR, hotplug_ctrl);
 
-	/* Enable DDI hotplug in IER */
 	hotplug_ctrl = I915_READ(GEN8_DE_PORT_IER) | hotplug_port;
 	I915_WRITE(GEN8_DE_PORT_IER, hotplug_ctrl);
 	POSTING_READ(GEN8_DE_PORT_IER);
-- 
1.7.10.4

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/3] drm/i915/bxt: WA for swapped HPD pins in A stepping
  2015-08-10  5:05 [PATCH 0/3] Add BXT A0/A1 WA for HPD swap in generic way Sonika Jindal
  2015-08-10  5:05 ` [PATCH 1/3] drm/i915: Dont enable hpd for eDP Sonika Jindal
  2015-08-10  5:05 ` [PATCH 2/3] drm/i915/bxt: Add HPD support for DDIA Sonika Jindal
@ 2015-08-10  5:05 ` Sonika Jindal
  2015-08-10  5:28   ` Sivakumar Thulasimani
  2015-08-13 12:48   ` shuang.he
  2 siblings, 2 replies; 17+ messages in thread
From: Sonika Jindal @ 2015-08-10  5:05 UTC (permalink / raw)
  To: intel-gfx

WA for BXT A0/A1, where DDIB's HPD pin is swapped to DDIA, so enabling
DDIA HPD pin in place of DDIB.

v2: For DP, irq_port is used to determine the encoder instead of
hpd_pin and removing the edp HPD logic because port A HPD is not
present(Imre)
v3: Rebased on top of Imre's patchset for enabling HPD on PORT A.
Added hpd_pin swapping for intel_dp_init_connector, setting encoder
for PORT_A as per the WA in irq_port (Imre)
v4: Dont enable interrupt for edp, also reframe the description (Siva)
v5: Don’t check for PORT_A in intel_ddi_init to update dig_port,
instead avoid setting hpd_pin itself (Imre)

Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c  |   10 +++++++++-
 drivers/gpu/drm/i915/intel_dp.c   |    2 ++
 drivers/gpu/drm/i915/intel_hdmi.c |    9 ++++++++-
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e2c6f73..777e3a3 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3225,7 +3225,15 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
 			goto err;
 
 		intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
-		dev_priv->hotplug.irq_port[port] = intel_dig_port;
+		/*
+		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
+		 * interrupts to check the external panel connection.
+		 */
+		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
+					 && port == PORT_B)
+			dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
+		else
+			dev_priv->hotplug.irq_port[port] = intel_dig_port;
 	}
 
 	/* In theory we don't need the encoder->type check, but leave it just in
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5a614c9..7fab3e5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5788,6 +5788,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		break;
 	case PORT_B:
 		intel_encoder->hpd_pin = HPD_PORT_B;
+		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
+			intel_encoder->hpd_pin = HPD_PORT_A;
 		break;
 	case PORT_C:
 		intel_encoder->hpd_pin = HPD_PORT_C;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 70bad5b..94fa716 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 			intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
 		else
 			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
-		intel_encoder->hpd_pin = HPD_PORT_B;
+		/*
+		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
+		 * interrupts to check the external panel connection.
+		 */
+		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
+			intel_encoder->hpd_pin = HPD_PORT_A;
+		else
+			intel_encoder->hpd_pin = HPD_PORT_B;
 		break;
 	case PORT_C:
 		if (IS_BROXTON(dev_priv))
-- 
1.7.10.4

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] drm/i915: Dont enable hpd for eDP
  2015-08-10  5:05 ` [PATCH 1/3] drm/i915: Dont enable hpd for eDP Sonika Jindal
@ 2015-08-10  5:24   ` Sivakumar Thulasimani
  2015-08-10 11:37     ` Jani Nikula
  0 siblings, 1 reply; 17+ messages in thread
From: Sivakumar Thulasimani @ 2015-08-10  5:24 UTC (permalink / raw)
  To: Sonika Jindal, intel-gfx

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

On 8/10/2015 10:35 AM, Sonika Jindal wrote:
> With HPD support added for all ports including PORT_A, setting hpd_pin will
> result in enabling of hpd to edp as well. There is no need to enable HPD on
> PORT_A hence this patch removes hpd_pin update for PORT_A, where edp will
> be connected. it can be added back when required
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index fcc64e5..5a614c9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5784,7 +5784,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>   	/* Set up the hotplug pin. */
>   	switch (port) {
>   	case PORT_A:
> -		intel_encoder->hpd_pin = HPD_PORT_A;
> +		/* Not enabling edp interrupts */
>   		break;
>   	case PORT_B:
>   		intel_encoder->hpd_pin = HPD_PORT_B;

-- 
regards,
Sivakumar

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] drm/i915/bxt: Add HPD support for DDIA
  2015-08-10  5:05 ` [PATCH 2/3] drm/i915/bxt: Add HPD support for DDIA Sonika Jindal
@ 2015-08-10  5:27   ` Sivakumar Thulasimani
  0 siblings, 0 replies; 17+ messages in thread
From: Sivakumar Thulasimani @ 2015-08-10  5:27 UTC (permalink / raw)
  To: Sonika Jindal, intel-gfx

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

On 8/10/2015 10:35 AM, Sonika Jindal wrote:
> Also remove redundant comments.
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_irq.c |   10 +++-------
>   1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 02b9e73..9b9533a 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -90,6 +90,7 @@ static const u32 hpd_status_i915[HPD_NUM_PINS] = {
>   
>   /* BXT hpd list */
>   static const u32 hpd_bxt[HPD_NUM_PINS] = {
> +	[HPD_PORT_A] = BXT_DE_PORT_HP_DDIA,
>   	[HPD_PORT_B] = BXT_DE_PORT_HP_DDIB,
>   	[HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
>   };
> @@ -3018,30 +3019,25 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
>   	u32 hotplug_port = 0;
>   	u32 hotplug_ctrl;
>   
> -	/* Now, enable HPD */
>   	for_each_intel_encoder(dev, intel_encoder) {
>   		if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state
>   				== HPD_ENABLED)
>   			hotplug_port |= hpd_bxt[intel_encoder->hpd_pin];
>   	}
>   
> -	/* Mask all HPD control bits */
>   	hotplug_ctrl = I915_READ(BXT_HOTPLUG_CTL) & ~BXT_HOTPLUG_CTL_MASK;
>   
> -	/* Enable requested port in hotplug control */
> -	/* TODO: implement (short) HPD support on port A */
> -	WARN_ON_ONCE(hotplug_port & BXT_DE_PORT_HP_DDIA);
> +	if (hotplug_port & BXT_DE_PORT_HP_DDIA)
> +		hotplug_ctrl |= BXT_DDIA_HPD_ENABLE;
>   	if (hotplug_port & BXT_DE_PORT_HP_DDIB)
>   		hotplug_ctrl |= BXT_DDIB_HPD_ENABLE;
>   	if (hotplug_port & BXT_DE_PORT_HP_DDIC)
>   		hotplug_ctrl |= BXT_DDIC_HPD_ENABLE;
>   	I915_WRITE(BXT_HOTPLUG_CTL, hotplug_ctrl);
>   
> -	/* Unmask DDI hotplug in IMR */
>   	hotplug_ctrl = I915_READ(GEN8_DE_PORT_IMR) & ~hotplug_port;
>   	I915_WRITE(GEN8_DE_PORT_IMR, hotplug_ctrl);
>   
> -	/* Enable DDI hotplug in IER */
>   	hotplug_ctrl = I915_READ(GEN8_DE_PORT_IER) | hotplug_port;
>   	I915_WRITE(GEN8_DE_PORT_IER, hotplug_ctrl);
>   	POSTING_READ(GEN8_DE_PORT_IER);

-- 
regards,
Sivakumar

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] drm/i915/bxt: WA for swapped HPD pins in A stepping
  2015-08-10  5:05 ` [PATCH 3/3] drm/i915/bxt: WA for swapped HPD pins in A stepping Sonika Jindal
@ 2015-08-10  5:28   ` Sivakumar Thulasimani
  2015-08-12 12:58     ` Daniel Vetter
  2015-08-13 12:48   ` shuang.he
  1 sibling, 1 reply; 17+ messages in thread
From: Sivakumar Thulasimani @ 2015-08-10  5:28 UTC (permalink / raw)
  To: Sonika Jindal, intel-gfx

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

On 8/10/2015 10:35 AM, Sonika Jindal wrote:
> WA for BXT A0/A1, where DDIB's HPD pin is swapped to DDIA, so enabling
> DDIA HPD pin in place of DDIB.
>
> v2: For DP, irq_port is used to determine the encoder instead of
> hpd_pin and removing the edp HPD logic because port A HPD is not
> present(Imre)
> v3: Rebased on top of Imre's patchset for enabling HPD on PORT A.
> Added hpd_pin swapping for intel_dp_init_connector, setting encoder
> for PORT_A as per the WA in irq_port (Imre)
> v4: Dont enable interrupt for edp, also reframe the description (Siva)
> v5: Don’t check for PORT_A in intel_ddi_init to update dig_port,
> instead avoid setting hpd_pin itself (Imre)
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_ddi.c  |   10 +++++++++-
>   drivers/gpu/drm/i915/intel_dp.c   |    2 ++
>   drivers/gpu/drm/i915/intel_hdmi.c |    9 ++++++++-
>   3 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e2c6f73..777e3a3 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3225,7 +3225,15 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>   			goto err;
>   
>   		intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> -		dev_priv->hotplug.irq_port[port] = intel_dig_port;
> +		/*
> +		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> +		 * interrupts to check the external panel connection.
> +		 */
> +		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
> +					 && port == PORT_B)
> +			dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> +		else
> +			dev_priv->hotplug.irq_port[port] = intel_dig_port;
>   	}
>   
>   	/* In theory we don't need the encoder->type check, but leave it just in
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 5a614c9..7fab3e5 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5788,6 +5788,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>   		break;
>   	case PORT_B:
>   		intel_encoder->hpd_pin = HPD_PORT_B;
> +		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> +			intel_encoder->hpd_pin = HPD_PORT_A;
>   		break;
>   	case PORT_C:
>   		intel_encoder->hpd_pin = HPD_PORT_C;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 70bad5b..94fa716 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>   			intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
>   		else
>   			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> -		intel_encoder->hpd_pin = HPD_PORT_B;
> +		/*
> +		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> +		 * interrupts to check the external panel connection.
> +		 */
> +		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> +			intel_encoder->hpd_pin = HPD_PORT_A;
> +		else
> +			intel_encoder->hpd_pin = HPD_PORT_B;
>   		break;
>   	case PORT_C:
>   		if (IS_BROXTON(dev_priv))

-- 
regards,
Sivakumar

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] drm/i915: Dont enable hpd for eDP
  2015-08-10  5:24   ` Sivakumar Thulasimani
@ 2015-08-10 11:37     ` Jani Nikula
  2015-08-10 11:38       ` Sivakumar Thulasimani
  0 siblings, 1 reply; 17+ messages in thread
From: Jani Nikula @ 2015-08-10 11:37 UTC (permalink / raw)
  To: Sivakumar Thulasimani, Sonika Jindal, intel-gfx

On Mon, 10 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>
> On 8/10/2015 10:35 AM, Sonika Jindal wrote:
>> With HPD support added for all ports including PORT_A, setting hpd_pin will
>> result in enabling of hpd to edp as well. There is no need to enable HPD on
>> PORT_A hence this patch removes hpd_pin update for PORT_A, where edp will
>> be connected. it can be added back when required

What? You can't just go ahead and remove HPD from eDP sinks.

BR,
Jani.


>>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dp.c |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index fcc64e5..5a614c9 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -5784,7 +5784,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>>   	/* Set up the hotplug pin. */
>>   	switch (port) {
>>   	case PORT_A:
>> -		intel_encoder->hpd_pin = HPD_PORT_A;
>> +		/* Not enabling edp interrupts */
>>   		break;
>>   	case PORT_B:
>>   		intel_encoder->hpd_pin = HPD_PORT_B;
>
> -- 
> regards,
> Sivakumar
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] drm/i915: Dont enable hpd for eDP
  2015-08-10 11:37     ` Jani Nikula
@ 2015-08-10 11:38       ` Sivakumar Thulasimani
  2015-08-10 12:14         ` Jani Nikula
  0 siblings, 1 reply; 17+ messages in thread
From: Sivakumar Thulasimani @ 2015-08-10 11:38 UTC (permalink / raw)
  To: Jani Nikula, Sonika Jindal, intel-gfx



On 8/10/2015 5:07 PM, Jani Nikula wrote:
> On Mon, 10 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
>> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>>
>> On 8/10/2015 10:35 AM, Sonika Jindal wrote:
>>> With HPD support added for all ports including PORT_A, setting hpd_pin will
>>> result in enabling of hpd to edp as well. There is no need to enable HPD on
>>> PORT_A hence this patch removes hpd_pin update for PORT_A, where edp will
>>> be connected. it can be added back when required
> What? You can't just go ahead and remove HPD from eDP sinks.
>
> BR,
> Jani.
Nope, we are not removing HPD for edp sinks, it was never there in the 
first place. It was
enabled for CHV (even there by mistake since PORT B/C was both DP and 
eDP) but it was
never there for any other plaforms nor is it used for any purpose (PSR 
must use it, but i
dont see code for it as well).
>
>>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/intel_dp.c |    2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index fcc64e5..5a614c9 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -5784,7 +5784,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>>>    	/* Set up the hotplug pin. */
>>>    	switch (port) {
>>>    	case PORT_A:
>>> -		intel_encoder->hpd_pin = HPD_PORT_A;
>>> +		/* Not enabling edp interrupts */
>>>    		break;
>>>    	case PORT_B:
>>>    		intel_encoder->hpd_pin = HPD_PORT_B;
>> -- 
>> regards,
>> Sivakumar
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
regards,
Sivakumar

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] drm/i915: Dont enable hpd for eDP
  2015-08-10 11:38       ` Sivakumar Thulasimani
@ 2015-08-10 12:14         ` Jani Nikula
  2015-08-10 12:16           ` Ville Syrjälä
  2015-08-10 12:21           ` Sivakumar Thulasimani
  0 siblings, 2 replies; 17+ messages in thread
From: Jani Nikula @ 2015-08-10 12:14 UTC (permalink / raw)
  To: Sivakumar Thulasimani, Sonika Jindal, intel-gfx

On Mon, 10 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
> On 8/10/2015 5:07 PM, Jani Nikula wrote:
>> On Mon, 10 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
>>> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>>>
>>> On 8/10/2015 10:35 AM, Sonika Jindal wrote:
>>>> With HPD support added for all ports including PORT_A, setting hpd_pin will
>>>> result in enabling of hpd to edp as well. There is no need to enable HPD on
>>>> PORT_A hence this patch removes hpd_pin update for PORT_A, where edp will
>>>> be connected. it can be added back when required
>> What? You can't just go ahead and remove HPD from eDP sinks.
>>
>> BR,
>> Jani.
> Nope, we are not removing HPD for edp sinks, it was never there in the 
> first place. It was
> enabled for CHV (even there by mistake since PORT B/C was both DP and 
> eDP) but it was
> never there for any other plaforms nor is it used for any purpose (PSR 
> must use it, but i
> dont see code for it as well).

Are you saying there's no HPD enabled in our *hardware* for eDP? Or
driver?

My point is, is this patch making it harder to enable eDP hpd handling
(e.g. for PSR or DP link re-training) in the future? We currently take
it into account in a few places, and if we start removing that, it will
be a loss of effort to first remove and then add it back.

BR,
Jani.



>>
>>>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/intel_dp.c |    2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>>> index fcc64e5..5a614c9 100644
>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>> @@ -5784,7 +5784,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>>>>    	/* Set up the hotplug pin. */
>>>>    	switch (port) {
>>>>    	case PORT_A:
>>>> -		intel_encoder->hpd_pin = HPD_PORT_A;
>>>> +		/* Not enabling edp interrupts */
>>>>    		break;
>>>>    	case PORT_B:
>>>>    		intel_encoder->hpd_pin = HPD_PORT_B;
>>> -- 
>>> regards,
>>> Sivakumar
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> regards,
> Sivakumar
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] drm/i915: Dont enable hpd for eDP
  2015-08-10 12:14         ` Jani Nikula
@ 2015-08-10 12:16           ` Ville Syrjälä
  2015-08-10 12:21           ` Sivakumar Thulasimani
  1 sibling, 0 replies; 17+ messages in thread
From: Ville Syrjälä @ 2015-08-10 12:16 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Mon, Aug 10, 2015 at 03:14:36PM +0300, Jani Nikula wrote:
> On Mon, 10 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
> > On 8/10/2015 5:07 PM, Jani Nikula wrote:
> >> On Mon, 10 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
> >>> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> >>>
> >>> On 8/10/2015 10:35 AM, Sonika Jindal wrote:
> >>>> With HPD support added for all ports including PORT_A, setting hpd_pin will
> >>>> result in enabling of hpd to edp as well. There is no need to enable HPD on
> >>>> PORT_A hence this patch removes hpd_pin update for PORT_A, where edp will
> >>>> be connected. it can be added back when required
> >> What? You can't just go ahead and remove HPD from eDP sinks.
> >>
> >> BR,
> >> Jani.
> > Nope, we are not removing HPD for edp sinks, it was never there in the 
> > first place. It was
> > enabled for CHV (even there by mistake since PORT B/C was both DP and 
> > eDP) but it was
> > never there for any other plaforms nor is it used for any purpose (PSR 
> > must use it, but i
> > dont see code for it as well).
> 
> Are you saying there's no HPD enabled in our *hardware* for eDP? Or
> driver?
> 
> My point is, is this patch making it harder to enable eDP hpd handling
> (e.g. for PSR or DP link re-training) in the future? We currently take
> it into account in a few places, and if we start removing that, it will
> be a loss of effort to first remove and then add it back.

I have a branch with (untested) port A/E HPD support (+ a bunch of SPT+
irq handler reorganization). I guess I should post that in case people
are interested in this stuff.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] drm/i915: Dont enable hpd for eDP
  2015-08-10 12:14         ` Jani Nikula
  2015-08-10 12:16           ` Ville Syrjälä
@ 2015-08-10 12:21           ` Sivakumar Thulasimani
  2015-08-12 12:56             ` Daniel Vetter
  1 sibling, 1 reply; 17+ messages in thread
From: Sivakumar Thulasimani @ 2015-08-10 12:21 UTC (permalink / raw)
  To: Jani Nikula, Sonika Jindal, intel-gfx



On 8/10/2015 5:44 PM, Jani Nikula wrote:
> On Mon, 10 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
>> On 8/10/2015 5:07 PM, Jani Nikula wrote:
>>> On Mon, 10 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
>>>> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>>>>
>>>> On 8/10/2015 10:35 AM, Sonika Jindal wrote:
>>>>> With HPD support added for all ports including PORT_A, setting hpd_pin will
>>>>> result in enabling of hpd to edp as well. There is no need to enable HPD on
>>>>> PORT_A hence this patch removes hpd_pin update for PORT_A, where edp will
>>>>> be connected. it can be added back when required
>>> What? You can't just go ahead and remove HPD from eDP sinks.
>>>
>>> BR,
>>> Jani.
>> Nope, we are not removing HPD for edp sinks, it was never there in the
>> first place. It was
>> enabled for CHV (even there by mistake since PORT B/C was both DP and
>> eDP) but it was
>> never there for any other plaforms nor is it used for any purpose (PSR
>> must use it, but i
>> dont see code for it as well).
> Are you saying there's no HPD enabled in our *hardware* for eDP? Or
> driver?
>
> My point is, is this patch making it harder to enable eDP hpd handling
> (e.g. for PSR or DP link re-training) in the future? We currently take
> it into account in a few places, and if we start removing that, it will
> be a loss of effort to first remove and then add it back.
>
> BR,
> Jani.
i was referring to our driver only.

Our VLV/CHV code already receives hpd for every pps on and off which is
later ignored. if we dont disable HPD on eDPs this behavior will be extended
for all platforms which i feel is too costly to keep enabled when there 
is no
purpose for it right now.

if anyone later requires HPD , they might need it for PSR panels only and
since that code is not available as of now, this can be added along with 
those
changes so i feel this is the better tradeoff :)
>
>
>>>>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>>>>> ---
>>>>>     drivers/gpu/drm/i915/intel_dp.c |    2 +-
>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>>>> index fcc64e5..5a614c9 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>>> @@ -5784,7 +5784,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>>>>>     	/* Set up the hotplug pin. */
>>>>>     	switch (port) {
>>>>>     	case PORT_A:
>>>>> -		intel_encoder->hpd_pin = HPD_PORT_A;
>>>>> +		/* Not enabling edp interrupts */
>>>>>     		break;
>>>>>     	case PORT_B:
>>>>>     		intel_encoder->hpd_pin = HPD_PORT_B;
>>>> -- 
>>>> regards,
>>>> Sivakumar
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> -- 
>> regards,
>> Sivakumar
>>

-- 
regards,
Sivakumar

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] drm/i915: Dont enable hpd for eDP
  2015-08-10 12:21           ` Sivakumar Thulasimani
@ 2015-08-12 12:56             ` Daniel Vetter
  2015-08-13  4:05               ` Sivakumar Thulasimani
  0 siblings, 1 reply; 17+ messages in thread
From: Daniel Vetter @ 2015-08-12 12:56 UTC (permalink / raw)
  To: Sivakumar Thulasimani; +Cc: intel-gfx

On Mon, Aug 10, 2015 at 05:51:48PM +0530, Sivakumar Thulasimani wrote:
> 
> 
> On 8/10/2015 5:44 PM, Jani Nikula wrote:
> >On Mon, 10 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
> >>On 8/10/2015 5:07 PM, Jani Nikula wrote:
> >>>On Mon, 10 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
> >>>>Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> >>>>
> >>>>On 8/10/2015 10:35 AM, Sonika Jindal wrote:
> >>>>>With HPD support added for all ports including PORT_A, setting hpd_pin will
> >>>>>result in enabling of hpd to edp as well. There is no need to enable HPD on
> >>>>>PORT_A hence this patch removes hpd_pin update for PORT_A, where edp will
> >>>>>be connected. it can be added back when required
> >>>What? You can't just go ahead and remove HPD from eDP sinks.
> >>>
> >>>BR,
> >>>Jani.
> >>Nope, we are not removing HPD for edp sinks, it was never there in the
> >>first place. It was
> >>enabled for CHV (even there by mistake since PORT B/C was both DP and
> >>eDP) but it was
> >>never there for any other plaforms nor is it used for any purpose (PSR
> >>must use it, but i
> >>dont see code for it as well).
> >Are you saying there's no HPD enabled in our *hardware* for eDP? Or
> >driver?
> >
> >My point is, is this patch making it harder to enable eDP hpd handling
> >(e.g. for PSR or DP link re-training) in the future? We currently take
> >it into account in a few places, and if we start removing that, it will
> >be a loss of effort to first remove and then add it back.
> >
> >BR,
> >Jani.
> i was referring to our driver only.
> 
> Our VLV/CHV code already receives hpd for every pps on and off which is
> later ignored. if we dont disable HPD on eDPs this behavior will be extended
> for all platforms which i feel is too costly to keep enabled when there is
> no
> purpose for it right now.

don't optimize code because you "feel it's costly", only do it when you
have hard numbers. One interrupt per pps on or off transition won't be
measurable at all.
-Daniel
-- 
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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] drm/i915/bxt: WA for swapped HPD pins in A stepping
  2015-08-10  5:28   ` Sivakumar Thulasimani
@ 2015-08-12 12:58     ` Daniel Vetter
  0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2015-08-12 12:58 UTC (permalink / raw)
  To: Sivakumar Thulasimani; +Cc: intel-gfx

On Mon, Aug 10, 2015 at 10:58:55AM +0530, Sivakumar Thulasimani wrote:
> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> 
> On 8/10/2015 10:35 AM, Sonika Jindal wrote:
> >WA for BXT A0/A1, where DDIB's HPD pin is swapped to DDIA, so enabling
> >DDIA HPD pin in place of DDIB.
> >
> >v2: For DP, irq_port is used to determine the encoder instead of
> >hpd_pin and removing the edp HPD logic because port A HPD is not
> >present(Imre)
> >v3: Rebased on top of Imre's patchset for enabling HPD on PORT A.
> >Added hpd_pin swapping for intel_dp_init_connector, setting encoder
> >for PORT_A as per the WA in irq_port (Imre)
> >v4: Dont enable interrupt for edp, also reframe the description (Siva)
> >v5: Don’t check for PORT_A in intel_ddi_init to update dig_port,
> >instead avoid setting hpd_pin itself (Imre)
> >
> >Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>

Merged patches 2&3 from this series, thanks.
-Daniel

> >---
> >  drivers/gpu/drm/i915/intel_ddi.c  |   10 +++++++++-
> >  drivers/gpu/drm/i915/intel_dp.c   |    2 ++
> >  drivers/gpu/drm/i915/intel_hdmi.c |    9 ++++++++-
> >  3 files changed, 19 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >index e2c6f73..777e3a3 100644
> >--- a/drivers/gpu/drm/i915/intel_ddi.c
> >+++ b/drivers/gpu/drm/i915/intel_ddi.c
> >@@ -3225,7 +3225,15 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
> >  			goto err;
> >  		intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> >-		dev_priv->hotplug.irq_port[port] = intel_dig_port;
> >+		/*
> >+		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> >+		 * interrupts to check the external panel connection.
> >+		 */
> >+		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
> >+					 && port == PORT_B)
> >+			dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> >+		else
> >+			dev_priv->hotplug.irq_port[port] = intel_dig_port;
> >  	}
> >  	/* In theory we don't need the encoder->type check, but leave it just in
> >diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >index 5a614c9..7fab3e5 100644
> >--- a/drivers/gpu/drm/i915/intel_dp.c
> >+++ b/drivers/gpu/drm/i915/intel_dp.c
> >@@ -5788,6 +5788,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> >  		break;
> >  	case PORT_B:
> >  		intel_encoder->hpd_pin = HPD_PORT_B;
> >+		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> >+			intel_encoder->hpd_pin = HPD_PORT_A;
> >  		break;
> >  	case PORT_C:
> >  		intel_encoder->hpd_pin = HPD_PORT_C;
> >diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> >index 70bad5b..94fa716 100644
> >--- a/drivers/gpu/drm/i915/intel_hdmi.c
> >+++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >@@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >  			intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> >  		else
> >  			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> >-		intel_encoder->hpd_pin = HPD_PORT_B;
> >+		/*
> >+		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> >+		 * interrupts to check the external panel connection.
> >+		 */
> >+		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> >+			intel_encoder->hpd_pin = HPD_PORT_A;
> >+		else
> >+			intel_encoder->hpd_pin = HPD_PORT_B;
> >  		break;
> >  	case PORT_C:
> >  		if (IS_BROXTON(dev_priv))
> 
> -- 
> regards,
> Sivakumar
> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] drm/i915: Dont enable hpd for eDP
  2015-08-12 12:56             ` Daniel Vetter
@ 2015-08-13  4:05               ` Sivakumar Thulasimani
  2015-08-13 15:07                 ` Ville Syrjälä
  0 siblings, 1 reply; 17+ messages in thread
From: Sivakumar Thulasimani @ 2015-08-13  4:05 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx



On 8/12/2015 6:26 PM, Daniel Vetter wrote:
> On Mon, Aug 10, 2015 at 05:51:48PM +0530, Sivakumar Thulasimani wrote:
>>
>> On 8/10/2015 5:44 PM, Jani Nikula wrote:
>>> On Mon, 10 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
>>>> On 8/10/2015 5:07 PM, Jani Nikula wrote:
>>>>> On Mon, 10 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
>>>>>> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>>>>>>
>>>>>> On 8/10/2015 10:35 AM, Sonika Jindal wrote:
>>>>>>> With HPD support added for all ports including PORT_A, setting hpd_pin will
>>>>>>> result in enabling of hpd to edp as well. There is no need to enable HPD on
>>>>>>> PORT_A hence this patch removes hpd_pin update for PORT_A, where edp will
>>>>>>> be connected. it can be added back when required
>>>>> What? You can't just go ahead and remove HPD from eDP sinks.
>>>>>
>>>>> BR,
>>>>> Jani.
>>>> Nope, we are not removing HPD for edp sinks, it was never there in the
>>>> first place. It was
>>>> enabled for CHV (even there by mistake since PORT B/C was both DP and
>>>> eDP) but it was
>>>> never there for any other plaforms nor is it used for any purpose (PSR
>>>> must use it, but i
>>>> dont see code for it as well).
>>> Are you saying there's no HPD enabled in our *hardware* for eDP? Or
>>> driver?
>>>
>>> My point is, is this patch making it harder to enable eDP hpd handling
>>> (e.g. for PSR or DP link re-training) in the future? We currently take
>>> it into account in a few places, and if we start removing that, it will
>>> be a loss of effort to first remove and then add it back.
>>>
>>> BR,
>>> Jani.
>> i was referring to our driver only.
>>
>> Our VLV/CHV code already receives hpd for every pps on and off which is
>> later ignored. if we dont disable HPD on eDPs this behavior will be extended
>> for all platforms which i feel is too costly to keep enabled when there is
>> no
>> purpose for it right now.
> don't optimize code because you "feel it's costly", only do it when you
> have hard numbers. One interrupt per pps on or off transition won't be
> measurable at all.
> -Daniel
let me rephrase my concern then :)
a) HPD was never enabled before this patch for edp
b) this patch series will enable hpd for edp
so why should we allow hpd for edp when no one is using it and will 
cause problems
unless ignored explicitly ?

-- 
regards,
Sivakumar

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] drm/i915/bxt: WA for swapped HPD pins in A stepping
  2015-08-10  5:05 ` [PATCH 3/3] drm/i915/bxt: WA for swapped HPD pins in A stepping Sonika Jindal
  2015-08-10  5:28   ` Sivakumar Thulasimani
@ 2015-08-13 12:48   ` shuang.he
  1 sibling, 0 replies; 17+ messages in thread
From: shuang.he @ 2015-08-13 12:48 UTC (permalink / raw)
  To: shuang.he, julianx.dumez, christophe.sureau, lei.a.liu, intel-gfx,
	sonika.jindal

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 7125
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                 -1              302/302              301/302
SNB                                  315/315              315/315
IVB                                  336/336              336/336
BYT                                  283/283              283/283
HSW                                  378/378              378/378
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*ILK  igt@kms_flip@flip-vs-dpms-interruptible      PASS(1)      DMESG_WARN(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] drm/i915: Dont enable hpd for eDP
  2015-08-13  4:05               ` Sivakumar Thulasimani
@ 2015-08-13 15:07                 ` Ville Syrjälä
  0 siblings, 0 replies; 17+ messages in thread
From: Ville Syrjälä @ 2015-08-13 15:07 UTC (permalink / raw)
  To: Sivakumar Thulasimani; +Cc: intel-gfx

On Thu, Aug 13, 2015 at 09:35:21AM +0530, Sivakumar Thulasimani wrote:
> 
> 
> On 8/12/2015 6:26 PM, Daniel Vetter wrote:
> > On Mon, Aug 10, 2015 at 05:51:48PM +0530, Sivakumar Thulasimani wrote:
> >>
> >> On 8/10/2015 5:44 PM, Jani Nikula wrote:
> >>> On Mon, 10 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
> >>>> On 8/10/2015 5:07 PM, Jani Nikula wrote:
> >>>>> On Mon, 10 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
> >>>>>> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> >>>>>>
> >>>>>> On 8/10/2015 10:35 AM, Sonika Jindal wrote:
> >>>>>>> With HPD support added for all ports including PORT_A, setting hpd_pin will
> >>>>>>> result in enabling of hpd to edp as well. There is no need to enable HPD on
> >>>>>>> PORT_A hence this patch removes hpd_pin update for PORT_A, where edp will
> >>>>>>> be connected. it can be added back when required
> >>>>> What? You can't just go ahead and remove HPD from eDP sinks.
> >>>>>
> >>>>> BR,
> >>>>> Jani.
> >>>> Nope, we are not removing HPD for edp sinks, it was never there in the
> >>>> first place. It was
> >>>> enabled for CHV (even there by mistake since PORT B/C was both DP and
> >>>> eDP) but it was
> >>>> never there for any other plaforms nor is it used for any purpose (PSR
> >>>> must use it, but i
> >>>> dont see code for it as well).
> >>> Are you saying there's no HPD enabled in our *hardware* for eDP? Or
> >>> driver?
> >>>
> >>> My point is, is this patch making it harder to enable eDP hpd handling
> >>> (e.g. for PSR or DP link re-training) in the future? We currently take
> >>> it into account in a few places, and if we start removing that, it will
> >>> be a loss of effort to first remove and then add it back.
> >>>
> >>> BR,
> >>> Jani.
> >> i was referring to our driver only.
> >>
> >> Our VLV/CHV code already receives hpd for every pps on and off which is
> >> later ignored. if we dont disable HPD on eDPs this behavior will be extended
> >> for all platforms which i feel is too costly to keep enabled when there is
> >> no
> >> purpose for it right now.
> > don't optimize code because you "feel it's costly", only do it when you
> > have hard numbers. One interrupt per pps on or off transition won't be
> > measurable at all.
> > -Daniel
> let me rephrase my concern then :)
> a) HPD was never enabled before this patch for edp

It was for port!=A, which means all VLV/CHV + some other machines with
eDP on port D.

> b) this patch series will enable hpd for edp
> so why should we allow hpd for edp when no one is using it and will 
> cause problems
> unless ignored explicitly ?

It's not clear it would cause problems. My ILK still seems happy now that
I enabled port A HPD on it [1]. As are the VLV/CHV machines. On BSW we
had problems but that turned out to be misconfigured pullups on the HPD
pins. Before that got fixed there was a lot of noise on the line
whenever the panel was off, and that spurred me to write the patch
series to keep the HPD disabled while the port is disabled [2].

[1] http://lists.freedesktop.org/archives/intel-gfx/2015-August/073559.html
[2] http://lists.freedesktop.org/archives/intel-gfx/2015-January/058173.html

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2015-08-13 15:07 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-10  5:05 [PATCH 0/3] Add BXT A0/A1 WA for HPD swap in generic way Sonika Jindal
2015-08-10  5:05 ` [PATCH 1/3] drm/i915: Dont enable hpd for eDP Sonika Jindal
2015-08-10  5:24   ` Sivakumar Thulasimani
2015-08-10 11:37     ` Jani Nikula
2015-08-10 11:38       ` Sivakumar Thulasimani
2015-08-10 12:14         ` Jani Nikula
2015-08-10 12:16           ` Ville Syrjälä
2015-08-10 12:21           ` Sivakumar Thulasimani
2015-08-12 12:56             ` Daniel Vetter
2015-08-13  4:05               ` Sivakumar Thulasimani
2015-08-13 15:07                 ` Ville Syrjälä
2015-08-10  5:05 ` [PATCH 2/3] drm/i915/bxt: Add HPD support for DDIA Sonika Jindal
2015-08-10  5:27   ` Sivakumar Thulasimani
2015-08-10  5:05 ` [PATCH 3/3] drm/i915/bxt: WA for swapped HPD pins in A stepping Sonika Jindal
2015-08-10  5:28   ` Sivakumar Thulasimani
2015-08-12 12:58     ` Daniel Vetter
2015-08-13 12:48   ` shuang.he

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox