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

[Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>]

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.

>   	/* 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.
> +	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 */
>
>


-- 
regards,
Sivakumar


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