Re: [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT

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

On 2/4/2016 5:59 PM, Jani Nikula wrote:
> On Thu, 04 Feb 2016, Shubhangi Shrivastava <shubhangi.shrivastava@intel.com> wrote:
>> This patch sets the invert bit for hpd detection for each port
>> based on vbt configuration. since each AOB can be designed to
>> depend on invert bit or not, it is expected if an AOB requires
>> invert bit, the user will set respective bit in VBT.
>>
>> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
>> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_irq.c | 49 +++++++++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/i915_reg.h |  9 ++++++++
>>   2 files changed, 58 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 25a8937..305e6dd 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -3424,6 +3424,54 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
>>   	I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
>>   }
>>   
>> +/*
>> + * For BXT invert bit has to be set based on AOB design
>> + * for HPD detection logic, update it based on VBT fields.
>> + */
>> +static void bxt_hpd_set_invert(struct drm_device *dev, u32 hotplug_port)
>> +{
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	int i, reg_val, val = 0;
>> +
>> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> +
>> +		/* Proceed only if invert bit is set */
>> +		if (dev_priv->vbt.child_dev[i].common.hpd_invert == 0)
>> +			continue;
>> +
>> +		/*
>> +		 * Convert dvo_port to PORT_X and set appropriate bit
>> +		 * only if hotplug is enabled on that port
>> +		 */
>> +		switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
>> +		case DVO_PORT_DPA:
>> +		case DVO_PORT_HDMIA:
>> +			if (hotplug_port & BXT_DE_PORT_HP_DDIA)
>> +				val |= BXT_DDIA_HPD_INVERT;
>> +			break;
>> +		case DVO_PORT_DPB:
>> +		case DVO_PORT_HDMIB:
>> +			if (hotplug_port & BXT_DE_PORT_HP_DDIB)
>> +				val |= BXT_DDIB_HPD_INVERT;
>> +			break;
>> +		case DVO_PORT_DPC:
>> +		case DVO_PORT_HDMIC:
>> +			if (hotplug_port & BXT_DE_PORT_HP_DDIC)
>> +				val |= BXT_DDIC_HPD_INVERT;
>> +			break;
>> +		default:
>> +			DRM_ERROR("HPD invert set for invalid dvo port %d\n",
>> +				   dev_priv->vbt.child_dev[i].common.dvo_port);
>> +			break;
>> +		}
>> +	}
>> +	reg_val = I915_READ(BXT_HOTPLUG_CTL);
>> +	DRM_DEBUG_KMS("Invert bit setting: hp_ctl:%x hp_port:%x val:%x\n",
>> +				reg_val, hotplug_port, val);
>> +	reg_val &= ~BXT_DDI_HPD_INVERT_MASK;
>> +	I915_WRITE(BXT_HOTPLUG_CTL, reg_val | val);
>> +}
> No, we don't want this here. Separate VBT parsing from the rest of the
> logic. See [1] for some directions where I want to take this type of
> things.
hmm understood, will add intel_bios_requires_invert(dev, port)
and change the logic above to
if (intel_bios_requires_invert(dev,port)
     val |= port;
hope this should be fine.
> BR,
> Jani.
>
> [1] http://mid.gmane.org/cover.1452541881.git.jani.nikula@intel.com
>
>
>
>> +
>>   static void spt_hpd_irq_setup(struct drm_device *dev)
>>   {
>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -3494,6 +3542,7 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
>>   	hotplug |= PORTC_HOTPLUG_ENABLE | PORTB_HOTPLUG_ENABLE |
>>   		PORTA_HOTPLUG_ENABLE;
>>   	I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
>> +	bxt_hpd_set_invert(dev, enabled_irqs);
>>   }
>>   
>>   static void ibx_irq_postinstall(struct drm_device *dev)
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 0a98889..01bd3c5 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -5936,6 +5936,15 @@ enum skl_disp_power_wells {
>>   #define GEN8_PCU_IIR _MMIO(0x444e8)
>>   #define GEN8_PCU_IER _MMIO(0x444ec)
>>   
>> +/* BXT hotplug control */
>> +#define BXT_HOTPLUG_CTL			_MMIO(0xC4030)
>> +#define BXT_DDIA_HPD_INVERT		(1 << 27)
>> +#define BXT_DDIC_HPD_INVERT		(1 << 11)
>> +#define BXT_DDIB_HPD_INVERT		(1 << 3)
>> +#define BXT_DDI_HPD_INVERT_MASK		(BXT_DDIA_HPD_INVERT | \
>> +					 BXT_DDIB_HPD_INVERT | \
>> +					 BXT_DDIC_HPD_INVERT)
>> +
>>   #define ILK_DISPLAY_CHICKEN2	_MMIO(0x42004)
>>   /* Required on all Ironlake and Sandybridge according to the B-Spec. */
>>   #define  ILK_ELPIN_409_SELECT	(1 << 25)

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