drm/i915/bxt: WA for swapped HPD pins in A stepping
diff mbox

Message ID 1437975128-30182-1-git-send-email-sonika.jindal@intel.com
State New
Headers show

Commit Message

sonika.jindal@intel.com July 27, 2015, 5:32 a.m. UTC
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)

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

Comments

Sivakumar Thulasimani July 27, 2015, 11:46 a.m. UTC | #1
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>


On 7/27/2015 11:02 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)
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_ddi.c  |   11 ++++++++++-
>   drivers/gpu/drm/i915/intel_dp.c   |    4 ++++
>   drivers/gpu/drm/i915/intel_hdmi.c |    9 ++++++++-
>   3 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e2c6f73..8d7ffe0 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3225,7 +3225,16 @@ 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;
> +		/* Dont enable interrupts for edp*/
> +		else if (port != PORT_A)
> +			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 fcc64e5..71679ef 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5785,9 +5785,13 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>   	switch (port) {
>   	case PORT_A:
>   		intel_encoder->hpd_pin = HPD_PORT_A;
> +		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> +			intel_encoder->hpd_pin = HPD_PORT_C;
>   		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))
Shuang He July 28, 2015, 8:42 a.m. UTC | #2
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6870
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                 -1              296/296              295/296
SNB                 -1              316/316              315/316
IVB                 -1              343/343              342/343
BYT                 -3              286/286              283/286
HSW                 -1              379/379              378/379
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*ILK  igt@gem_reg_read      PASS(1)      NRUN(1)
*SNB  igt@gem_reg_read      PASS(1)      INIT(1)
*IVB  igt@gem_reg_read      PASS(1)      NRUN(1)
*BYT  igt@drm_read@short-buffer-block      PASS(1)      FAIL(1)
*BYT  igt@gem_partial_pwrite_pread@reads-uncached      PASS(1)      FAIL(1)
*BYT  igt@gem_reg_read      PASS(1)      INIT(1)
*HSW  igt@gem_reg_read      PASS(1)      INIT(1)
Note: You need to pay more attention to line start with '*'
Imre Deak Aug. 5, 2015, 9:53 a.m. UTC | #3
On Mon, 2015-07-27 at 11:02 +0530, 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)
> 
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>

This patch is a new version of 2/2 of your patchset at
http://lists.freedesktop.org/archives/intel-gfx/2015-July/071580.html

so this should've been sent either as a reply to the individual v1 patch
there, or just by resending the whole patchset. For now you could just
resend the whole patchset. Also please add the version info to the
subject with git format-patch --subject-prefix.


> ---
>  drivers/gpu/drm/i915/intel_ddi.c  |   11 ++++++++++-
>  drivers/gpu/drm/i915/intel_dp.c   |    4 ++++
>  drivers/gpu/drm/i915/intel_hdmi.c |    9 ++++++++-
>  3 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e2c6f73..8d7ffe0 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3225,7 +3225,16 @@ 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;
> +		/* Dont enable interrupts for edp*/
> +		else if (port != PORT_A)
> +			dev_priv->hotplug.irq_port[port] = intel_dig_port;

Not enabling HPD on eDP is not a BXT specific change (even though it's
disabled now everywhere), so that part should be a separate patch and
ordered before patch 1/2. Also this isn't the proper place to disable
HPD handling; as long as the corresponding intel_encoder->hpd_pin is set
the relevant interrupt will get unmasked and hotplug_work will be
called. See bxt_hpd_irq_setup() and intel_hpd_irq_handler().

>  	}
>  
>  	/* 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 fcc64e5..71679ef 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5785,9 +5785,13 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  	switch (port) {
>  	case PORT_A:
>  		intel_encoder->hpd_pin = HPD_PORT_A;
> +		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> +			intel_encoder->hpd_pin = HPD_PORT_C;
>  		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))
Sivakumar Thulasimani Aug. 5, 2015, 10:11 a.m. UTC | #4
On 8/5/2015 3:23 PM, Imre Deak wrote:
> On Mon, 2015-07-27 at 11:02 +0530, 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)
>>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> This patch is a new version of 2/2 of your patchset at
> http://lists.freedesktop.org/archives/intel-gfx/2015-July/071580.html
>
> so this should've been sent either as a reply to the individual v1 patch
> there, or just by resending the whole patchset. For now you could just
> resend the whole patchset. Also please add the version info to the
> subject with git format-patch --subject-prefix.
>
>
>> ---
>>   drivers/gpu/drm/i915/intel_ddi.c  |   11 ++++++++++-
>>   drivers/gpu/drm/i915/intel_dp.c   |    4 ++++
>>   drivers/gpu/drm/i915/intel_hdmi.c |    9 ++++++++-
>>   3 files changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index e2c6f73..8d7ffe0 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -3225,7 +3225,16 @@ 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;
>> +		/* Dont enable interrupts for edp*/
>> +		else if (port != PORT_A)
>> +			dev_priv->hotplug.irq_port[port] = intel_dig_port;
> Not enabling HPD on eDP is not a BXT specific change (even though it's
> disabled now everywhere), so that part should be a separate patch and
> ordered before patch 1/2. Also this isn't the proper place to disable
> HPD handling; as long as the corresponding intel_encoder->hpd_pin is set
> the relevant interrupt will get unmasked and hotplug_work will be
> called. See bxt_hpd_irq_setup() and intel_hpd_irq_handler().
>
hmm , ok Sonika, please remove HPD_PORT_A being set for PORT_A. here 
else if can
be modified to simple else
>>   	}
>>   
>>   	/* 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 fcc64e5..71679ef 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -5785,9 +5785,13 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>>   	switch (port) {
>>   	case PORT_A:
>>   		intel_encoder->hpd_pin = HPD_PORT_A;
>> +		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>> +			intel_encoder->hpd_pin = HPD_PORT_C;
>>   		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))
>
>
>

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e2c6f73..8d7ffe0 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3225,7 +3225,16 @@  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;
+		/* Dont enable interrupts for edp*/
+		else if (port != PORT_A)
+			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 fcc64e5..71679ef 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5785,9 +5785,13 @@  intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	switch (port) {
 	case PORT_A:
 		intel_encoder->hpd_pin = HPD_PORT_A;
+		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
+			intel_encoder->hpd_pin = HPD_PORT_C;
 		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))