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

From: "Jindal, Sonika" <sonika.jindal@intel.com>
To: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
Date: Mon, 13 Jul 2015 13:41:36 +0530	[thread overview]
Message-ID: <55A372B8.9060607@intel.com> (raw)
In-Reply-To: <55A35B39.5010505@intel.com>


On 7/13/2015 12:01 PM, Sivakumar Thulasimani wrote:
>
>
> On 7/13/2015 9:47 AM, 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 remove the redundant comment.
>>
>> v2: Remove redundant IS_BROXTON check, Add comment about port C not
>> connected, and rephrase the commit message to include only what we
>> are doing here (Imre)
>>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_irq.c |   38
>> +++++++++++++++++++++++++++++---------
>>   1 file changed, 29 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c
>> b/drivers/gpu/drm/i915/i915_irq.c
>> index eb52a03..760539c 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -88,7 +88,11 @@ static const u32 hpd_status_i915[HPD_NUM_PINS] = {
>>       [HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
>>   };
>> -/* BXT hpd list */
>> +/* Port C is not connected on bxt A0/A1 */
>> +static const u32 hpd_bxt_a0[HPD_NUM_PINS] = {
>> +    [HPD_PORT_B] = BXT_DE_PORT_HP_DDIA
>> +};
>> +
>>   static const u32 hpd_bxt[HPD_NUM_PINS] = {
>>       [HPD_PORT_B] = BXT_DE_PORT_HP_DDIB,
>>       [HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
>> @@ -2257,6 +2261,7 @@ static void bxt_hpd_handler(struct drm_device
>> *dev, uint32_t iir_status)
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>>       u32 hp_control, hp_trigger;
>>       u32 pin_mask, long_mask;
>> +    const u32 *hpd;
>>       /* Get the status */
>>       hp_trigger = iir_status & BXT_DE_PORT_HOTPLUG_MASK;
>> @@ -2271,7 +2276,12 @@ static void bxt_hpd_handler(struct drm_device
>> *dev, uint32_t iir_status)
>>       /* Clear sticky bits in hpd status */
>>       I915_WRITE(BXT_HOTPLUG_CTL, hp_control);
>> -    pch_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control,
>> hpd_bxt);
>> +    if (INTEL_REVID(dev) < BXT_REVID_B0)
>> +        hpd = hpd_bxt_a0;
>> +    else
>> +        hpd = hpd_bxt;
>> +
>> +    pch_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control,
>> hpd);
>>       intel_hpd_irq_handler(dev, pin_mask, long_mask);
>>   }
>> @@ -3315,8 +3325,15 @@ static void bxt_hpd_irq_setup(struct drm_device
>> *dev)
>>       /* Now, enable HPD */
>>       for_each_intel_encoder(dev, intel_encoder) {
>>           if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state
>> -                == HPD_ENABLED)
>> -            hotplug_port |= hpd_bxt[intel_encoder->hpd_pin];
>> +                == HPD_ENABLED) {
>> +            const u32 *hpd;
>> +
>> +            if (INTEL_REVID(dev) < BXT_REVID_B0)
>> +                hpd = hpd_bxt_a0;
>> +            else
>> +                hpd = hpd_bxt;
>> +            hotplug_port |= hpd[intel_encoder->hpd_pin];
>> +        }
>>       }
> hpd initialization can be moved out so it is done once instead of being
> repeated for each encoder.
Sure.

>
>>       /* Mask all HPD control bits */
>> @@ -3324,11 +3341,14 @@ static void bxt_hpd_irq_setup(struct
>> drm_device *dev)
>>       /* Enable requested port in hotplug control */
>>       /* TODO: implement (short) HPD support on port A */
>> -    WARN_ON_ONCE(hotplug_port & BXT_DE_PORT_HP_DDIA);
>> -    if (hotplug_port & BXT_DE_PORT_HP_DDIB)
>> -        hotplug_ctrl |= BXT_DDIB_HPD_ENABLE;
>> -    if (hotplug_port & BXT_DE_PORT_HP_DDIC)
>> -        hotplug_ctrl |= BXT_DDIC_HPD_ENABLE;
>> +    if (INTEL_REVID(dev) < BXT_REVID_B0 && (hotplug_port &
>> BXT_DE_PORT_HP_DDIA))
>> +            hotplug_ctrl |= BXT_DDIA_HPD_ENABLE;
> can you add a comment here stating the swap in hpd pins ?
> as i am not sure not everyone will first check the commit message for
> the change here.
Ok, I'l add

>> +    else {
>> +        if (hotplug_port & BXT_DE_PORT_HP_DDIB)
>> +            hotplug_ctrl |= BXT_DDIB_HPD_ENABLE;
>> +        if (hotplug_port & BXT_DE_PORT_HP_DDIC)
>> +            hotplug_ctrl |= BXT_DDIC_HPD_ENABLE;
>> +    }
>>       I915_WRITE(BXT_HOTPLUG_CTL, hotplug_ctrl);
>>       /* Unmask DDI hotplug in IMR */
>>
>>
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx