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

["Jindal, Sonika" <sonika.jindal@intel.com>]

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.jindal@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