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

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

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