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

From: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
To: imre.deak@intel.com
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
Date: Wed, 05 Aug 2015 15:41:03 +0530	[thread overview]
Message-ID: <55C1E137.9070100@intel.com> (raw)
In-Reply-To: <1438768411.3935.23.camel@intel.com>


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


-- 
regards,
Sivakumar


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