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

From: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
To: "Jindal, Sonika" <sonika.jindal@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
Date: Wed, 22 Jul 2015 17:50:31 +0530	[thread overview]
Message-ID: <55AF8A8F.3030902@intel.com> (raw)
In-Reply-To: <55AF866A.3020706@intel.com>


On 7/22/2015 5:32 PM, Jindal, Sonika wrote:
>
>
> On 7/22/2015 5:01 PM, Sivakumar Thulasimani wrote:
>>
>>
>> On 7/22/2015 4:39 PM, Jindal, Sonika wrote:
>>>
>>>
>>> On 7/22/2015 4:03 PM, Sivakumar Thulasimani wrote:
>>>>
>>>>
>>>> On 7/22/2015 3:37 PM, 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.
>>>>>
>>>>> 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)
>>>>>
>>>>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>>>>> ---
>>>>>   drivers/gpu/drm/i915/intel_ddi.c  |   12 +++++++++++-
>>>>>   drivers/gpu/drm/i915/intel_dp.c   |    4 ++++
>>>>>   drivers/gpu/drm/i915/intel_hdmi.c |    9 ++++++++-
>>>>>   3 files changed, 23 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>>>>> b/drivers/gpu/drm/i915/intel_ddi.c
>>>>> index e2c6f73..d5745e2 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>>>> @@ -3225,7 +3225,17 @@ 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)) {
>>>>> +            if (port == PORT_B)
>>>>> +                dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
>>>>> +            else if (port == PORT_A)
>>>>> +                dev_priv->hotplug.irq_port[PORT_C] = intel_dig_port;
Please don't enable interrupt for PORT_A :) otherwise we might have to 
handle HPD
for each pps on/off like in CHV.
>>>>> +        } else
>>>>> +            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))
>>>> bxt_hpd_irq_setup is not touched here without this being updated i 
>>>> dont
>>>> think HPD can be enabled.
>>> Now, we swap the hpd_pin, which is being used to enable HPD in
>>> bxt_hpd_irq_setup.
>>>
>> yes, but the hpd_pin which is updated to HPD_PORT_A when enters
>> bxt_hpd_setup_irq we are enabling
>> only HPD B & C. so it will not enable HPDA. unless i am missing some
>> patch i am not seeing code to
>> enable HPD A with latest nightly code.
>>
> Yes, you are missing the patch 1 of this series :)
> [PATCH 1/2] drm/i915/bxt: Add HPD support for DDIA
>
>>> Regards,
>>> Sonika
>>>>
>>
ok missed that in the thread list :).

-- 
regards,
Sivakumar

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