Re: [Intel-gfx] [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping

2015-07-16 Thread Jindal, Sonika 



On 7/15/2015 2:37 PM, Daniel Vetter wrote:

On Wed, Jul 15, 2015 at 01:34:29PM +0530, Jindal, Sonika wrote:



On 7/15/2015 12:05 PM, Jindal, Sonika wrote:



On 7/14/2015 7:52 PM, Imre Deak wrote:

On ti, 2015-07-14 at 11:18 +0530, Sonika Jindal wrote:

As per bspec, on BXT A0/A1, sw needs to activate DDIA HPD logic
and interrupts to check the external panel connection and DDIC HPD
logic for edp panel.

Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
   drivers/gpu/drm/i915/intel_dp.c   |   18 --
   drivers/gpu/drm/i915/intel_hdmi.c |9 -
   2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7b54b9d..c32f3d3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5869,10 +5869,24 @@ 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;
+   /*
+* On BXT A0/A1, sw needs to activate DDIC HPD logic and
+* interrupts to check the eDP panel connection.
+*/
+   if (IS_BROXTON(dev_priv)  (INTEL_REVID(dev)  BXT_REVID_B0))
+   intel_encoder-hpd_pin = HPD_PORT_C;
+   else
+   intel_encoder-hpd_pin = HPD_PORT_A;
break;
case PORT_B:
-   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:
intel_encoder-hpd_pin = HPD_PORT_C;


This won't work for a couple of reasons: atm i915_digport_work_func()
assumes a fixed pin-port mapping, for example it'll call the HPD
handler for the port A encoder for a short/long pulse event triggered
via the HPD_PORT_A pin, whereas after the above patch you want to select
port B's encoder on BXT A0/1. This could be fixed by setting up
hotplug.irq_port in intel_ddi_init() according to the above change, or
not using irq_port at all, but instead looking up the encoder the same
way i915_hotplug_work_func() does using intel_encoder-hpd_pin.


Yeah that's kinda a bug in digport_work_func, but that part is also a
mess. Simplest would be to rename hotplug.irq_port to irq_pin and change
the assignment for that too.


Hmm, i didnt realize that.
Moving this check to intel_ddi_init, will again make the checks spread
all over which we wanted to avoid since hpd_pin was in place.
But looks like hpd_pin is only for i915_hotplug_work_func.
I feel we can move back to the older approach itself
What do you suggest?


But then we can remove these checks from here, and have it only in
intel_ddi_init, right? So it should look fine.

For HPD_PORT_A, I think we can skip that part as of now.

Please suggest..


I still think that fake-handling hpd A in the low-level irq handler is
massively confusing. And we need to change all the same parts anyway (i.e.
you'd have to change the digport_work_func too with the old approach).
-Daniel


So, for now, I will just correct it in intel_ddi_init and leave the 
handling of HPD for port A untouched. I will only change the irq_port 
for external DP (port B). And the other part with hpd_pin to handle hdmi 
hotplug will remain as is.

Please let me know if you see any issue in this.

Regards,
Sonika



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

Re: [Intel-gfx] [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping

2015-07-15 Thread Daniel Vetter 
On Wed, Jul 15, 2015 at 01:34:29PM +0530, Jindal, Sonika wrote:
 
 
 On 7/15/2015 12:05 PM, Jindal, Sonika wrote:
 
 
 On 7/14/2015 7:52 PM, Imre Deak wrote:
 On ti, 2015-07-14 at 11:18 +0530, Sonika Jindal wrote:
 As per bspec, on BXT A0/A1, sw needs to activate DDIA HPD logic
 and interrupts to check the external panel connection and DDIC HPD
 logic for edp panel.
 
 Signed-off-by: Sonika Jindal sonika.jin...@intel.com
 ---
drivers/gpu/drm/i915/intel_dp.c   |   18 --
drivers/gpu/drm/i915/intel_hdmi.c |9 -
2 files changed, 24 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_dp.c 
 b/drivers/gpu/drm/i915/intel_dp.c
 index 7b54b9d..c32f3d3 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -5869,10 +5869,24 @@ 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;
 +  /*
 +   * On BXT A0/A1, sw needs to activate DDIC HPD logic and
 +   * interrupts to check the eDP panel connection.
 +   */
 +  if (IS_BROXTON(dev_priv)  (INTEL_REVID(dev)  BXT_REVID_B0))
 +  intel_encoder-hpd_pin = HPD_PORT_C;
 +  else
 +  intel_encoder-hpd_pin = HPD_PORT_A;
break;
case PORT_B:
 -  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:
intel_encoder-hpd_pin = HPD_PORT_C;
 
 This won't work for a couple of reasons: atm i915_digport_work_func()
 assumes a fixed pin-port mapping, for example it'll call the HPD
 handler for the port A encoder for a short/long pulse event triggered
 via the HPD_PORT_A pin, whereas after the above patch you want to select
 port B's encoder on BXT A0/1. This could be fixed by setting up
 hotplug.irq_port in intel_ddi_init() according to the above change, or
 not using irq_port at all, but instead looking up the encoder the same
 way i915_hotplug_work_func() does using intel_encoder-hpd_pin.

Yeah that's kinda a bug in digport_work_func, but that part is also a
mess. Simplest would be to rename hotplug.irq_port to irq_pin and change
the assignment for that too.

 Hmm, i didnt realize that.
 Moving this check to intel_ddi_init, will again make the checks spread
 all over which we wanted to avoid since hpd_pin was in place.
 But looks like hpd_pin is only for i915_hotplug_work_func.
 I feel we can move back to the older approach itself
 What do you suggest?
 
 But then we can remove these checks from here, and have it only in
 intel_ddi_init, right? So it should look fine.
 
 For HPD_PORT_A, I think we can skip that part as of now.
 
 Please suggest..

I still think that fake-handling hpd A in the low-level irq handler is
massively confusing. And we need to change all the same parts anyway (i.e.
you'd have to change the digport_work_func too with the old approach).
-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

Re: [Intel-gfx] [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping

2015-07-15 Thread Jindal, Sonika 



On 7/15/2015 12:05 PM, Jindal, Sonika wrote:



On 7/14/2015 7:52 PM, Imre Deak wrote:

On ti, 2015-07-14 at 11:18 +0530, Sonika Jindal wrote:

As per bspec, on BXT A0/A1, sw needs to activate DDIA HPD logic
and interrupts to check the external panel connection and DDIC HPD
logic for edp panel.

Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
   drivers/gpu/drm/i915/intel_dp.c   |   18 --
   drivers/gpu/drm/i915/intel_hdmi.c |9 -
   2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7b54b9d..c32f3d3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5869,10 +5869,24 @@ 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;
+   /*
+* On BXT A0/A1, sw needs to activate DDIC HPD logic and
+* interrupts to check the eDP panel connection.
+*/
+   if (IS_BROXTON(dev_priv)  (INTEL_REVID(dev)  BXT_REVID_B0))
+   intel_encoder-hpd_pin = HPD_PORT_C;
+   else
+   intel_encoder-hpd_pin = HPD_PORT_A;
break;
case PORT_B:
-   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:
intel_encoder-hpd_pin = HPD_PORT_C;


This won't work for a couple of reasons: atm i915_digport_work_func()
assumes a fixed pin-port mapping, for example it'll call the HPD
handler for the port A encoder for a short/long pulse event triggered
via the HPD_PORT_A pin, whereas after the above patch you want to select
port B's encoder on BXT A0/1. This could be fixed by setting up
hotplug.irq_port in intel_ddi_init() according to the above change, or
not using irq_port at all, but instead looking up the encoder the same
way i915_hotplug_work_func() does using intel_encoder-hpd_pin.


Hmm, i didnt realize that.
Moving this check to intel_ddi_init, will again make the checks spread
all over which we wanted to avoid since hpd_pin was in place.
But looks like hpd_pin is only for i915_hotplug_work_func.
I feel we can move back to the older approach itself
What do you suggest?

But then we can remove these checks from here, and have it only in 
intel_ddi_init, right? So it should look fine.


For HPD_PORT_A, I think we can skip that part as of now.

Please suggest..


Daniel, any comments?


The HPD_PORT_A HPD handling is missing in general, see
for_each_hpd_pin() and intel_hpd_irq_handler()/is_dig_port.




So if going with the above way, these issues need to be addressed first.

--Imre



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


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

Re: [Intel-gfx] [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping

2015-07-15 Thread Jindal, Sonika 



On 7/14/2015 7:52 PM, Imre Deak wrote:

On ti, 2015-07-14 at 11:18 +0530, Sonika Jindal wrote:

As per bspec, on BXT A0/A1, sw needs to activate DDIA HPD logic
and interrupts to check the external panel connection and DDIC HPD
logic for edp panel.

Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
  drivers/gpu/drm/i915/intel_dp.c   |   18 --
  drivers/gpu/drm/i915/intel_hdmi.c |9 -
  2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7b54b9d..c32f3d3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5869,10 +5869,24 @@ 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;
+   /*
+* On BXT A0/A1, sw needs to activate DDIC HPD logic and
+* interrupts to check the eDP panel connection.
+*/
+   if (IS_BROXTON(dev_priv)  (INTEL_REVID(dev)  BXT_REVID_B0))
+   intel_encoder-hpd_pin = HPD_PORT_C;
+   else
+   intel_encoder-hpd_pin = HPD_PORT_A;
break;
case PORT_B:
-   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:
intel_encoder-hpd_pin = HPD_PORT_C;


This won't work for a couple of reasons: atm i915_digport_work_func()
assumes a fixed pin-port mapping, for example it'll call the HPD
handler for the port A encoder for a short/long pulse event triggered
via the HPD_PORT_A pin, whereas after the above patch you want to select
port B's encoder on BXT A0/1. This could be fixed by setting up
hotplug.irq_port in intel_ddi_init() according to the above change, or
not using irq_port at all, but instead looking up the encoder the same
way i915_hotplug_work_func() does using intel_encoder-hpd_pin.


Hmm, i didnt realize that.
Moving this check to intel_ddi_init, will again make the checks spread 
all over which we wanted to avoid since hpd_pin was in place.

But looks like hpd_pin is only for i915_hotplug_work_func.
I feel we can move back to the older approach itself
What do you suggest?

Daniel, any comments?


The HPD_PORT_A HPD handling is missing in general, see
for_each_hpd_pin() and intel_hpd_irq_handler()/is_dig_port.




So if going with the above way, these issues need to be addressed first.

--Imre



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

Re: [Intel-gfx] [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping

2015-07-14 Thread Daniel Vetter 
On Tue, Jul 14, 2015 at 11:18:50AM +0530, Sonika Jindal wrote:
 As per bspec, on BXT A0/A1, sw needs to activate DDIA HPD logic
 and interrupts to check the external panel connection and DDIC HPD
 logic for edp panel.
 
 Signed-off-by: Sonika Jindal sonika.jin...@intel.com

Yeah I think this is much clearer. Will pull in as soon as there's an r-b
on these two.

Thanks, Daniel

 ---
  drivers/gpu/drm/i915/intel_dp.c   |   18 --
  drivers/gpu/drm/i915/intel_hdmi.c |9 -
  2 files changed, 24 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index 7b54b9d..c32f3d3 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -5869,10 +5869,24 @@ 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;
 + /*
 +  * On BXT A0/A1, sw needs to activate DDIC HPD logic and
 +  * interrupts to check the eDP panel connection.
 +  */
 + if (IS_BROXTON(dev_priv)  (INTEL_REVID(dev)  BXT_REVID_B0))
 + intel_encoder-hpd_pin = HPD_PORT_C;
 + else
 + intel_encoder-hpd_pin = HPD_PORT_A;
   break;
   case PORT_B:
 - 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:
   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 44e7160..121e626 100644
 --- a/drivers/gpu/drm/i915/intel_hdmi.c
 +++ b/drivers/gpu/drm/i915/intel_hdmi.c
 @@ -2011,7 +2011,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

-- 
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

Re: [Intel-gfx] [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping

2015-07-14 Thread Imre Deak 
On ti, 2015-07-14 at 11:18 +0530, Sonika Jindal wrote:
 As per bspec, on BXT A0/A1, sw needs to activate DDIA HPD logic
 and interrupts to check the external panel connection and DDIC HPD
 logic for edp panel.
 
 Signed-off-by: Sonika Jindal sonika.jin...@intel.com
 ---
  drivers/gpu/drm/i915/intel_dp.c   |   18 --
  drivers/gpu/drm/i915/intel_hdmi.c |9 -
  2 files changed, 24 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index 7b54b9d..c32f3d3 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -5869,10 +5869,24 @@ 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;
 + /*
 +  * On BXT A0/A1, sw needs to activate DDIC HPD logic and
 +  * interrupts to check the eDP panel connection.
 +  */
 + if (IS_BROXTON(dev_priv)  (INTEL_REVID(dev)  BXT_REVID_B0))
 + intel_encoder-hpd_pin = HPD_PORT_C;
 + else
 + intel_encoder-hpd_pin = HPD_PORT_A;
   break;
   case PORT_B:
 - 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:
   intel_encoder-hpd_pin = HPD_PORT_C;

This won't work for a couple of reasons: atm i915_digport_work_func()
assumes a fixed pin-port mapping, for example it'll call the HPD
handler for the port A encoder for a short/long pulse event triggered
via the HPD_PORT_A pin, whereas after the above patch you want to select
port B's encoder on BXT A0/1. This could be fixed by setting up
hotplug.irq_port in intel_ddi_init() according to the above change, or
not using irq_port at all, but instead looking up the encoder the same
way i915_hotplug_work_func() does using intel_encoder-hpd_pin.

The HPD_PORT_A HPD handling is missing in general, see
for_each_hpd_pin() and intel_hpd_irq_handler()/is_dig_port. 

So if going with the above way, these issues need to be addressed first.

--Imre

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