diff mbox

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

Message ID 1454576310-8727-2-git-send-email-shubhangi.shrivastava@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Shubhangi Shrivastava Feb. 4, 2016, 8:58 a.m. UTC 
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(+)

Comments

Jani Nikula Feb. 4, 2016, 12:29 p.m. UTC | #1 
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.

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)
Sivakumar Thulasimani Feb. 5, 2016, 1:06 a.m. UTC | #2 
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)
Jani Nikula Feb. 5, 2016, 8:27 a.m. UTC | #3 
On Fri, 05 Feb 2016, "Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com> wrote:
> 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.

I'd make it

bool intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv, enum port port);

BR,
Jani.



>> 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)
>
diff mbox

Patch

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);
+}
+
 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)