intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-02-04  8:58 [PATCH 1/2] drm/i915: Update VBT fields for child devices Shubhangi Shrivastava
@ 2016-02-04  8:58 ` Shubhangi Shrivastava
  2016-02-04 12:29   ` Jani Nikula
  0 siblings, 1 reply; 24+ messages in thread
From: Shubhangi Shrivastava @ 2016-02-04  8:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shubhangi Shrivastava

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

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-02-04  8:58 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
@ 2016-02-04 12:29   ` Jani Nikula
  2016-02-05  1:06     ` Thulasimani, Sivakumar
  0 siblings, 1 reply; 24+ messages in thread
From: Jani Nikula @ 2016-02-04 12:29 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shubhangi Shrivastava

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)

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-02-04 12:29   ` Jani Nikula
@ 2016-02-05  1:06     ` Thulasimani, Sivakumar
  2016-02-05  8:27       ` Jani Nikula
  0 siblings, 1 reply; 24+ messages in thread
From: Thulasimani, Sivakumar @ 2016-02-05  1:06 UTC (permalink / raw)
  To: Jani Nikula, Shubhangi Shrivastava, intel-gfx



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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-02-05  1:06     ` Thulasimani, Sivakumar
@ 2016-02-05  8:27       ` Jani Nikula
  0 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2016-02-05  8:27 UTC (permalink / raw)
  To: Thulasimani, Sivakumar, Shubhangi Shrivastava, intel-gfx

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

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-02-12 13:09 [PATCH 1/2] drm/i915: Update VBT fields for child devices Shubhangi Shrivastava
@ 2016-02-12 13:09 ` Shubhangi Shrivastava
  2016-02-12 13:37   ` kbuild test robot
  2016-02-12 14:08   ` Ville Syrjälä
  0 siblings, 2 replies; 24+ messages in thread
From: Shubhangi Shrivastava @ 2016-02-12 13:09 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shubhangi Shrivastava

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.

v2: Separated VBT parsing from the rest of the logic. (Jani)

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_drv.h   |  1 +
 drivers/gpu/drm/i915/i915_irq.c   | 43 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h   |  9 ++++++++
 drivers/gpu/drm/i915/intel_bios.c | 42 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 95 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8216665..457f175 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3393,6 +3393,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
 /* intel_bios.c */
 int intel_bios_init(struct drm_i915_private *dev_priv);
 bool intel_bios_is_valid_vbt(const void *buf, size_t size);
+bool intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port);
 
 /* intel_opregion.c */
 #ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 25a8937..fb95fb0 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -36,6 +36,7 @@
 #include "i915_drv.h"
 #include "i915_trace.h"
 #include "intel_drv.h"
+#include "intel_bios.h"
 
 /**
  * DOC: interrupt handling
@@ -3424,6 +3425,47 @@ 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 reg_val, val = 0;
+	enum port port;
+
+	for (port = PORT_A; port <= PORT_C; port++) {
+
+		/* Proceed only if invert bit is set */
+		if (intel_bios_is_port_hpd_inverted(dev, port)) {
+			switch (port) {
+			case PORT_A:
+				if (hotplug_port & BXT_DE_PORT_HP_DDIA)
+					val |= BXT_DDIA_HPD_INVERT;
+				break;
+			case PORT_B:
+				if (hotplug_port & BXT_DE_PORT_HP_DDIB)
+					val |= BXT_DDIB_HPD_INVERT;
+				break;
+			case PORT_C:
+				if (hotplug_port & BXT_DE_PORT_HP_DDIC)
+					val |= BXT_DDIC_HPD_INVERT;
+				break;
+			default:
+				DRM_ERROR("HPD invert set for invalid port %d\n",
+						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 +3536,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 6732fc1..66cf92e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5940,6 +5940,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 --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index a26d4b4..24d0077 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -105,6 +105,48 @@ find_section(const void *_bdb, int section_id)
 	return NULL;
 }
 
+bool
+intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int i;
+
+	if (!IS_BROXTON(dev)) {
+		DRM_ERROR("Bit inversion is not required in this platform\n");
+		return false;
+	}
+
+	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+
+		if (dev_priv->vbt.child_dev[i].common.hpd_invert == 1) {
+
+			switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
+			case DVO_PORT_DPA:
+			case DVO_PORT_HDMIA:
+				if (port == PORT_A)
+					return true;
+				break;
+			case DVO_PORT_DPB:
+			case DVO_PORT_HDMIB:
+				if (port == PORT_B)
+					return true;
+				break;
+			case DVO_PORT_DPC:
+			case DVO_PORT_HDMIC:
+				if (port == PORT_C)
+					return true;
+				break;
+			default:
+				DRM_DEBUG_KMS("This dvo port %d doesn't need invert\n",
+					dev_priv->vbt.child_dev[i].common.dvo_port);
+				break;
+			}
+		}
+	}
+
+	return false;
+}
+
 static void
 fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
 			const struct lvds_dvo_timing *dvo_timing)
-- 
2.6.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-02-12 13:09 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
@ 2016-02-12 13:37   ` kbuild test robot
  2016-02-12 14:08   ` Ville Syrjälä
  1 sibling, 0 replies; 24+ messages in thread
From: kbuild test robot @ 2016-02-12 13:37 UTC (permalink / raw)
  Cc: intel-gfx, Shubhangi Shrivastava, kbuild-all

[-- Attachment #1: Type: text/plain, Size: 6506 bytes --]

Hi Shubhangi,

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on next-20160212]
[cannot apply to v4.5-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Shubhangi-Shrivastava/drm-i915-Set-invert-bit-for-hpd-based-on-VBT/20160212-203937
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-x000-201606 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/uapi/linux/stddef.h:1:0,
                    from include/linux/stddef.h:4,
                    from include/uapi/linux/posix_types.h:4,
                    from include/uapi/linux/types.h:13,
                    from include/linux/types.h:5,
                    from include/drm/drm_dp_helper.h:26,
                    from drivers/gpu/drm/i915/intel_bios.c:28:
   drivers/gpu/drm/i915/intel_bios.c: In function 'intel_bios_is_port_hpd_inverted':
   drivers/gpu/drm/i915/intel_bios.c:121:40: error: 'struct common_child_dev_config' has no member named 'hpd_invert'
      if (dev_priv->vbt.child_dev[i].common.hpd_invert == 1) {
                                           ^
   include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
     if (__builtin_constant_p((cond)) ? !!(cond) :   \
                               ^
>> drivers/gpu/drm/i915/intel_bios.c:121:3: note: in expansion of macro 'if'
      if (dev_priv->vbt.child_dev[i].common.hpd_invert == 1) {
      ^
   drivers/gpu/drm/i915/intel_bios.c:121:40: error: 'struct common_child_dev_config' has no member named 'hpd_invert'
      if (dev_priv->vbt.child_dev[i].common.hpd_invert == 1) {
                                           ^
   include/linux/compiler.h:147:40: note: in definition of macro '__trace_if'
     if (__builtin_constant_p((cond)) ? !!(cond) :   \
                                           ^
>> drivers/gpu/drm/i915/intel_bios.c:121:3: note: in expansion of macro 'if'
      if (dev_priv->vbt.child_dev[i].common.hpd_invert == 1) {
      ^
   drivers/gpu/drm/i915/intel_bios.c:121:40: error: 'struct common_child_dev_config' has no member named 'hpd_invert'
      if (dev_priv->vbt.child_dev[i].common.hpd_invert == 1) {
                                           ^
   include/linux/compiler.h:158:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^
>> drivers/gpu/drm/i915/intel_bios.c:121:3: note: in expansion of macro 'if'
      if (dev_priv->vbt.child_dev[i].common.hpd_invert == 1) {
      ^

vim +/if +121 drivers/gpu/drm/i915/intel_bios.c

    22	 *
    23	 * Authors:
    24	 *    Eric Anholt <eric@anholt.net>
    25	 *
    26	 */
    27	
  > 28	#include <drm/drm_dp_helper.h>
    29	#include <drm/drmP.h>
    30	#include <drm/i915_drm.h>
    31	#include "i915_drv.h"
    32	#include "intel_bios.h"
    33	
    34	/**
    35	 * DOC: Video BIOS Table (VBT)
    36	 *
    37	 * The Video BIOS Table, or VBT, provides platform and board specific
    38	 * configuration information to the driver that is not discoverable or available
    39	 * through other means. The configuration is mostly related to display
    40	 * hardware. The VBT is available via the ACPI OpRegion or, on older systems, in
    41	 * the PCI ROM.
    42	 *
    43	 * The VBT consists of a VBT Header (defined as &struct vbt_header), a BDB
    44	 * Header (&struct bdb_header), and a number of BIOS Data Blocks (BDB) that
    45	 * contain the actual configuration information. The VBT Header, and thus the
    46	 * VBT, begins with "$VBT" signature. The VBT Header contains the offset of the
    47	 * BDB Header. The data blocks are concatenated after the BDB Header. The data
    48	 * blocks have a 1-byte Block ID, 2-byte Block Size, and Block Size bytes of
    49	 * data. (Block 53, the MIPI Sequence Block is an exception.)
    50	 *
    51	 * The driver parses the VBT during load. The relevant information is stored in
    52	 * driver private data for ease of use, and the actual VBT is not read after
    53	 * that.
    54	 */
    55	
    56	#define	SLAVE_ADDR1	0x70
    57	#define	SLAVE_ADDR2	0x72
    58	
    59	static int panel_type;
    60	
    61	/* Get BDB block size given a pointer to Block ID. */
    62	static u32 _get_blocksize(const u8 *block_base)
    63	{
    64		/* The MIPI Sequence Block v3+ has a separate size field. */
    65		if (*block_base == BDB_MIPI_SEQUENCE && *(block_base + 3) >= 3)
    66			return *((const u32 *)(block_base + 4));
    67		else
    68			return *((const u16 *)(block_base + 1));
    69	}
    70	
    71	/* Get BDB block size give a pointer to data after Block ID and Block Size. */
    72	static u32 get_blocksize(const void *block_data)
    73	{
    74		return _get_blocksize(block_data - 3);
    75	}
    76	
    77	static const void *
    78	find_section(const void *_bdb, int section_id)
    79	{
    80		const struct bdb_header *bdb = _bdb;
    81		const u8 *base = _bdb;
    82		int index = 0;
    83		u32 total, current_size;
    84		u8 current_id;
    85	
    86		/* skip to first section */
    87		index += bdb->header_size;
    88		total = bdb->bdb_size;
    89	
    90		/* walk the sections looking for section_id */
    91		while (index + 3 < total) {
    92			current_id = *(base + index);
    93			current_size = _get_blocksize(base + index);
    94			index += 3;
    95	
    96			if (index + current_size > total)
    97				return NULL;
    98	
    99			if (current_id == section_id)
   100				return base + index;
   101	
   102			index += current_size;
   103		}
   104	
   105		return NULL;
   106	}
   107	
   108	bool
   109	intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port)
   110	{
   111		struct drm_i915_private *dev_priv = dev->dev_private;
   112		int i;
   113	
   114		if (!IS_BROXTON(dev)) {
   115			DRM_ERROR("Bit inversion is not required in this platform\n");
   116			return false;
   117		}
   118	
   119		for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
   120	
 > 121			if (dev_priv->vbt.child_dev[i].common.hpd_invert == 1) {
   122	
   123				switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
   124				case DVO_PORT_DPA:

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 24181 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-02-12 13:09 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
  2016-02-12 13:37   ` kbuild test robot
@ 2016-02-12 14:08   ` Ville Syrjälä
  2016-02-16  1:59     ` Thulasimani, Sivakumar
  1 sibling, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2016-02-12 14:08 UTC (permalink / raw)
  To: Shubhangi Shrivastava; +Cc: intel-gfx

On Fri, Feb 12, 2016 at 06:39:44PM +0530, Shubhangi Shrivastava 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.
> 
> v2: Separated VBT parsing from the rest of the logic. (Jani)
> 
> 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_drv.h   |  1 +
>  drivers/gpu/drm/i915/i915_irq.c   | 43 +++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h   |  9 ++++++++
>  drivers/gpu/drm/i915/intel_bios.c | 42 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 95 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8216665..457f175 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3393,6 +3393,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
>  /* intel_bios.c */
>  int intel_bios_init(struct drm_i915_private *dev_priv);
>  bool intel_bios_is_valid_vbt(const void *buf, size_t size);
> +bool intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port);
>  
>  /* intel_opregion.c */
>  #ifdef CONFIG_ACPI
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 25a8937..fb95fb0 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -36,6 +36,7 @@
>  #include "i915_drv.h"
>  #include "i915_trace.h"
>  #include "intel_drv.h"
> +#include "intel_bios.h"
>  
>  /**
>   * DOC: interrupt handling
> @@ -3424,6 +3425,47 @@ 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 reg_val, val = 0;
> +	enum port port;
> +
> +	for (port = PORT_A; port <= PORT_C; port++) {
> +
> +		/* Proceed only if invert bit is set */
> +		if (intel_bios_is_port_hpd_inverted(dev, port)) {
> +			switch (port) {
> +			case PORT_A:
> +				if (hotplug_port & BXT_DE_PORT_HP_DDIA)
> +					val |= BXT_DDIA_HPD_INVERT;
> +				break;
> +			case PORT_B:
> +				if (hotplug_port & BXT_DE_PORT_HP_DDIB)
> +					val |= BXT_DDIB_HPD_INVERT;
> +				break;
> +			case PORT_C:
> +				if (hotplug_port & BXT_DE_PORT_HP_DDIC)
> +					val |= BXT_DDIC_HPD_INVERT;
> +				break;
> +			default:
> +				DRM_ERROR("HPD invert set for invalid port %d\n",
> +						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 RMW stuff please. Just set up the bits appropriately in bxt_hpd_irq_setup().

> +
>  static void spt_hpd_irq_setup(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -3494,6 +3536,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 6732fc1..66cf92e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5940,6 +5940,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)

We already have a name for that register.

> +#define BXT_DDIA_HPD_INVERT		(1 << 27)
> +#define BXT_DDIC_HPD_INVERT		(1 << 11)
> +#define BXT_DDIB_HPD_INVERT		(1 << 3)

I was going to suggest parametrizing this stuff, but the bits are in a
fairly nasty order so not so easy, and we haven't parametrized the rest
if the hpd bits either, so doing it just for these would be a bit out of
place perhaps.

We should perhaps try to parametrize all the hpd bits though, since that
could result in some neater looking code. But that sort of stuff is
better left for a separate patch. After a bit more though it's not
actually hard at all: (((port) * 8 + 24 + whatever) & 31)

> +#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 --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index a26d4b4..24d0077 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -105,6 +105,48 @@ find_section(const void *_bdb, int section_id)
>  	return NULL;
>  }
>  
> +bool
> +intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int i;
> +
> +	if (!IS_BROXTON(dev)) {
> +		DRM_ERROR("Bit inversion is not required in this platform\n");
> +		return false;
> +	}
> +
> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> +
> +		if (dev_priv->vbt.child_dev[i].common.hpd_invert == 1) {
> +
> +			switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
> +			case DVO_PORT_DPA:
> +			case DVO_PORT_HDMIA:
> +				if (port == PORT_A)
> +					return true;
> +				break;
> +			case DVO_PORT_DPB:
> +			case DVO_PORT_HDMIB:
> +				if (port == PORT_B)
> +					return true;
> +				break;
> +			case DVO_PORT_DPC:
> +			case DVO_PORT_HDMIC:
> +				if (port == PORT_C)
> +					return true;
> +				break;
> +			default:
> +				DRM_DEBUG_KMS("This dvo port %d doesn't need invert\n",
> +					dev_priv->vbt.child_dev[i].common.dvo_port);

Why do we need a debug message for the "no invert" case if we don't
need one for the invert case?

The code structure feels a bit wonky. It's sort of expecting multiple
child devs for each port. Can that actually happen?

> +				break;
> +			}
> +		}
> +	}
> +
> +	return false;
> +}
> +
>  static void
>  fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
>  			const struct lvds_dvo_timing *dvo_timing)
> -- 
> 2.6.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-02-12 14:08   ` Ville Syrjälä
@ 2016-02-16  1:59     ` Thulasimani, Sivakumar
  2016-02-16  9:41       ` Ville Syrjälä
  0 siblings, 1 reply; 24+ messages in thread
From: Thulasimani, Sivakumar @ 2016-02-16  1:59 UTC (permalink / raw)
  To: Ville Syrjälä, Shubhangi Shrivastava; +Cc: intel-gfx



On 2/12/2016 7:38 PM, Ville Syrjälä wrote:
> On Fri, Feb 12, 2016 at 06:39:44PM +0530, Shubhangi Shrivastava 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.
>>
>> v2: Separated VBT parsing from the rest of the logic. (Jani)
>>
>> 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_drv.h   |  1 +
>>   drivers/gpu/drm/i915/i915_irq.c   | 43 +++++++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/i915_reg.h   |  9 ++++++++
>>   drivers/gpu/drm/i915/intel_bios.c | 42 ++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 95 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 8216665..457f175 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3393,6 +3393,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
>>   /* intel_bios.c */
>>   int intel_bios_init(struct drm_i915_private *dev_priv);
>>   bool intel_bios_is_valid_vbt(const void *buf, size_t size);
>> +bool intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port);
>>   
>>   /* intel_opregion.c */
>>   #ifdef CONFIG_ACPI
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 25a8937..fb95fb0 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -36,6 +36,7 @@
>>   #include "i915_drv.h"
>>   #include "i915_trace.h"
>>   #include "intel_drv.h"
>> +#include "intel_bios.h"
>>   
>>   /**
>>    * DOC: interrupt handling
>> @@ -3424,6 +3425,47 @@ 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 reg_val, val = 0;
>> +	enum port port;
>> +
>> +	for (port = PORT_A; port <= PORT_C; port++) {
>> +
>> +		/* Proceed only if invert bit is set */
>> +		if (intel_bios_is_port_hpd_inverted(dev, port)) {
>> +			switch (port) {
>> +			case PORT_A:
>> +				if (hotplug_port & BXT_DE_PORT_HP_DDIA)
>> +					val |= BXT_DDIA_HPD_INVERT;
>> +				break;
>> +			case PORT_B:
>> +				if (hotplug_port & BXT_DE_PORT_HP_DDIB)
>> +					val |= BXT_DDIB_HPD_INVERT;
>> +				break;
>> +			case PORT_C:
>> +				if (hotplug_port & BXT_DE_PORT_HP_DDIC)
>> +					val |= BXT_DDIC_HPD_INVERT;
>> +				break;
>> +			default:
>> +				DRM_ERROR("HPD invert set for invalid port %d\n",
>> +						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 RMW stuff please. Just set up the bits appropriately in bxt_hpd_irq_setup().
the problem is we need to query vbt for setting invert bit. so not sure 
having this logic
inside bxt_hpd_irq_setup is good. if we want to avoid read/write to 
registers we can
modify the input to be values written on register instead of 
enabled_irqs. that way
we can write the value to register post call to this function and we 
need not
worry about current values in register. is that fine ?
>
>> +
>>   static void spt_hpd_irq_setup(struct drm_device *dev)
>>   {
>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -3494,6 +3536,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 6732fc1..66cf92e 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -5940,6 +5940,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)
> We already have a name for that register.
yes, but as mentioned by you below the register bits are different.
we did not want to confuse by using an old register define when the
bits are different.
>> +#define BXT_DDIA_HPD_INVERT		(1 << 27)
>> +#define BXT_DDIC_HPD_INVERT		(1 << 11)
>> +#define BXT_DDIB_HPD_INVERT		(1 << 3)
> I was going to suggest parametrizing this stuff, but the bits are in a
> fairly nasty order so not so easy, and we haven't parametrized the rest
> if the hpd bits either, so doing it just for these would be a bit out of
> place perhaps.
>
> We should perhaps try to parametrize all the hpd bits though, since that
> could result in some neater looking code. But that sort of stuff is
> better left for a separate patch. After a bit more though it's not
> actually hard at all: (((port) * 8 + 24 + whatever) & 31)
hmmm will check and get back on if this can be done.
>> +#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 --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index a26d4b4..24d0077 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -105,6 +105,48 @@ find_section(const void *_bdb, int section_id)
>>   	return NULL;
>>   }
>>   
>> +bool
>> +intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port)
>> +{
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	int i;
>> +
>> +	if (!IS_BROXTON(dev)) {
>> +		DRM_ERROR("Bit inversion is not required in this platform\n");
>> +		return false;
>> +	}
>> +
>> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> +
>> +		if (dev_priv->vbt.child_dev[i].common.hpd_invert == 1) {
>> +
>> +			switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
>> +			case DVO_PORT_DPA:
>> +			case DVO_PORT_HDMIA:
>> +				if (port == PORT_A)
>> +					return true;
>> +				break;
>> +			case DVO_PORT_DPB:
>> +			case DVO_PORT_HDMIB:
>> +				if (port == PORT_B)
>> +					return true;
>> +				break;
>> +			case DVO_PORT_DPC:
>> +			case DVO_PORT_HDMIC:
>> +				if (port == PORT_C)
>> +					return true;
>> +				break;
>> +			default:
>> +				DRM_DEBUG_KMS("This dvo port %d doesn't need invert\n",
>> +					dev_priv->vbt.child_dev[i].common.dvo_port);
> Why do we need a debug message for the "no invert" case if we don't
> need one for the invert case?
>
> The code structure feels a bit wonky. It's sort of expecting multiple
> child devs for each port. Can that actually happen?
the debug message is just a fail safe to let know if VBT programming is 
incorrect.
it can be removed. but we already found and fixed a incorrect programming in
default VBT with this so i would recommend keeping it :).

regarding child devs, the invert bit can be set for HDMI or DP based on 
the board
design so we need to handle this for both port types. hence we are 
checking for
both here. i too felt it odd having to loop twice once here and again in

bxt_hpd_set_invert.

if we can come up with a simpler design then i would favor it as well 
but for now
this seems to be required.

regards,
Sivakumar
>
>> +				break;
>> +			}
>> +		}
>> +	}
>> +
>> +	return false;
>> +}
>> +
>>   static void
>>   fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
>>   			const struct lvds_dvo_timing *dvo_timing)
>> -- 
>> 2.6.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-02-16  1:59     ` Thulasimani, Sivakumar
@ 2016-02-16  9:41       ` Ville Syrjälä
  0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2016-02-16  9:41 UTC (permalink / raw)
  To: Thulasimani, Sivakumar; +Cc: intel-gfx, Shubhangi Shrivastava

On Tue, Feb 16, 2016 at 07:29:03AM +0530, Thulasimani, Sivakumar wrote:
> 
> 
> On 2/12/2016 7:38 PM, Ville Syrjälä wrote:
> > On Fri, Feb 12, 2016 at 06:39:44PM +0530, Shubhangi Shrivastava 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.
> >>
> >> v2: Separated VBT parsing from the rest of the logic. (Jani)
> >>
> >> 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_drv.h   |  1 +
> >>   drivers/gpu/drm/i915/i915_irq.c   | 43 +++++++++++++++++++++++++++++++++++++++
> >>   drivers/gpu/drm/i915/i915_reg.h   |  9 ++++++++
> >>   drivers/gpu/drm/i915/intel_bios.c | 42 ++++++++++++++++++++++++++++++++++++++
> >>   4 files changed, 95 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index 8216665..457f175 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -3393,6 +3393,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
> >>   /* intel_bios.c */
> >>   int intel_bios_init(struct drm_i915_private *dev_priv);
> >>   bool intel_bios_is_valid_vbt(const void *buf, size_t size);
> >> +bool intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port);
> >>   
> >>   /* intel_opregion.c */
> >>   #ifdef CONFIG_ACPI
> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >> index 25a8937..fb95fb0 100644
> >> --- a/drivers/gpu/drm/i915/i915_irq.c
> >> +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> @@ -36,6 +36,7 @@
> >>   #include "i915_drv.h"
> >>   #include "i915_trace.h"
> >>   #include "intel_drv.h"
> >> +#include "intel_bios.h"
> >>   
> >>   /**
> >>    * DOC: interrupt handling
> >> @@ -3424,6 +3425,47 @@ 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 reg_val, val = 0;
> >> +	enum port port;
> >> +
> >> +	for (port = PORT_A; port <= PORT_C; port++) {
> >> +
> >> +		/* Proceed only if invert bit is set */
> >> +		if (intel_bios_is_port_hpd_inverted(dev, port)) {
> >> +			switch (port) {
> >> +			case PORT_A:
> >> +				if (hotplug_port & BXT_DE_PORT_HP_DDIA)
> >> +					val |= BXT_DDIA_HPD_INVERT;
> >> +				break;
> >> +			case PORT_B:
> >> +				if (hotplug_port & BXT_DE_PORT_HP_DDIB)
> >> +					val |= BXT_DDIB_HPD_INVERT;
> >> +				break;
> >> +			case PORT_C:
> >> +				if (hotplug_port & BXT_DE_PORT_HP_DDIC)
> >> +					val |= BXT_DDIC_HPD_INVERT;
> >> +				break;
> >> +			default:
> >> +				DRM_ERROR("HPD invert set for invalid port %d\n",
> >> +						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 RMW stuff please. Just set up the bits appropriately in bxt_hpd_irq_setup().
> the problem is we need to query vbt for setting invert bit. so not sure 
> having this logic
> inside bxt_hpd_irq_setup is good.

I think it is. We don't want to spread this stuff around all over the
place.

> if we want to avoid read/write to 
> registers we can
> modify the input to be values written on register instead of 
> enabled_irqs. that way
> we can write the value to register post call to this function and we 
> need not
> worry about current values in register. is that fine ?

I don't really see any point in doing it outside bxt_hpd_irq_setup().

> >
> >> +
> >>   static void spt_hpd_irq_setup(struct drm_device *dev)
> >>   {
> >>   	struct drm_i915_private *dev_priv = dev->dev_private;
> >> @@ -3494,6 +3536,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 6732fc1..66cf92e 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -5940,6 +5940,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)
> > We already have a name for that register.
> yes, but as mentioned by you below the register bits are different.
> we did not want to confuse by using an old register define when the
> bits are different.

That's not how we roll generally. Besides we're alredy using the other
bits in that register on BXT.

> >> +#define BXT_DDIA_HPD_INVERT		(1 << 27)
> >> +#define BXT_DDIC_HPD_INVERT		(1 << 11)
> >> +#define BXT_DDIB_HPD_INVERT		(1 << 3)
> > I was going to suggest parametrizing this stuff, but the bits are in a
> > fairly nasty order so not so easy, and we haven't parametrized the rest
> > if the hpd bits either, so doing it just for these would be a bit out of
> > place perhaps.
> >
> > We should perhaps try to parametrize all the hpd bits though, since that
> > could result in some neater looking code. But that sort of stuff is
> > better left for a separate patch. After a bit more though it's not
> > actually hard at all: (((port) * 8 + 24 + whatever) & 31)
> hmmm will check and get back on if this can be done.

I tried to do it globally, and while most of it turned out OK, there
were enough exceptions that I don't think it's necessarily worthwile.
It'll just make some of the bits parametrized and some not, so feels
a bit inconsistent.

> >> +#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 --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> >> index a26d4b4..24d0077 100644
> >> --- a/drivers/gpu/drm/i915/intel_bios.c
> >> +++ b/drivers/gpu/drm/i915/intel_bios.c
> >> @@ -105,6 +105,48 @@ find_section(const void *_bdb, int section_id)
> >>   	return NULL;
> >>   }
> >>   
> >> +bool
> >> +intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port)
> >> +{
> >> +	struct drm_i915_private *dev_priv = dev->dev_private;
> >> +	int i;
> >> +
> >> +	if (!IS_BROXTON(dev)) {
> >> +		DRM_ERROR("Bit inversion is not required in this platform\n");
> >> +		return false;
> >> +	}
> >> +
> >> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> >> +
> >> +		if (dev_priv->vbt.child_dev[i].common.hpd_invert == 1) {
> >> +
> >> +			switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
> >> +			case DVO_PORT_DPA:
> >> +			case DVO_PORT_HDMIA:
> >> +				if (port == PORT_A)
> >> +					return true;
> >> +				break;
> >> +			case DVO_PORT_DPB:
> >> +			case DVO_PORT_HDMIB:
> >> +				if (port == PORT_B)
> >> +					return true;
> >> +				break;
> >> +			case DVO_PORT_DPC:
> >> +			case DVO_PORT_HDMIC:
> >> +				if (port == PORT_C)
> >> +					return true;
> >> +				break;
> >> +			default:
> >> +				DRM_DEBUG_KMS("This dvo port %d doesn't need invert\n",
> >> +					dev_priv->vbt.child_dev[i].common.dvo_port);
> > Why do we need a debug message for the "no invert" case if we don't
> > need one for the invert case?
> >
> > The code structure feels a bit wonky. It's sort of expecting multiple
> > child devs for each port. Can that actually happen?
> the debug message is just a fail safe to let know if VBT programming is 
> incorrect.

Well, it'll tell you there's a bogus port in the VBT, but only if it
doesn't have invert==1, but it won't tell you about a bogus port with
invert==0. So seems like a somewhat half hearted attempt at verifying
the VBT to me.

> it can be removed. but we already found and fixed a incorrect programming in
> default VBT with this so i would recommend keeping it :).
> 
> regarding child devs, the invert bit can be set for HDMI or DP based on 
> the board
> design so we need to handle this for both port types. hence we are 
> checking for
> both here. i too felt it odd having to loop twice once here and again in
> 
> bxt_hpd_set_invert.
> 
> if we can come up with a simpler design then i would favor it as well 
> but for now
> this seems to be required.

I tried to think how to do this more neatly and didn't really come up
with anything really elegant.

I suppose one thing we could try is to have this construct a bitmask
so it wouldn't have to loop through the child devs more than once.
And then the register setup could maybe do just something like:

if (invert_hpd & PORT_A)
	something |= BXT_DDIA_HPD_INVERT;
if (invert_hpd& PORT_B)
	something |= BXT_DDIB_HPD_INVERT;
if (invert_hpd & PORT_C)
	something |= BXT_DDIC_HPD_INVERT;

without looping since we don't loop there for the other bits either.

> 
> regards,
> Sivakumar
> >
> >> +				break;
> >> +			}
> >> +		}
> >> +	}
> >> +
> >> +	return false;
> >> +}
> >> +
> >>   static void
> >>   fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
> >>   			const struct lvds_dvo_timing *dvo_timing)
> >> -- 
> >> 2.6.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-02-25  9:57 [PATCH 1/2] drm/i915: Update VBT fields for child devices Shubhangi Shrivastava
@ 2016-02-25  9:57 ` Shubhangi Shrivastava
  2016-02-25 13:57   ` Ville Syrjälä
  0 siblings, 1 reply; 24+ messages in thread
From: Shubhangi Shrivastava @ 2016-02-25  9:57 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shubhangi Shrivastava

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.

v2: Separated VBT parsing from the rest of the logic. (Jani)

v3: Moved setting invert bit logic to bxt_hpd_irq_setup()
    and changed its logic to avoid looping twice. (Ville)

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_drv.h   |  1 +
 drivers/gpu/drm/i915/i915_irq.c   | 22 +++++++++++++++++++-
 drivers/gpu/drm/i915/i915_reg.h   |  8 ++++++++
 drivers/gpu/drm/i915/intel_bios.c | 42 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8216665..457f175 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3393,6 +3393,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
 /* intel_bios.c */
 int intel_bios_init(struct drm_i915_private *dev_priv);
 bool intel_bios_is_valid_vbt(const void *buf, size_t size);
+bool intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port);
 
 /* intel_opregion.c */
 #ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 25a8937..525da56 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -36,6 +36,7 @@
 #include "i915_drv.h"
 #include "i915_trace.h"
 #include "intel_drv.h"
+#include "intel_bios.h"
 
 /**
  * DOC: interrupt handling
@@ -3484,6 +3485,7 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 hotplug_irqs, hotplug, enabled_irqs;
+	int val = 0;
 
 	enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_bxt);
 	hotplug_irqs = BXT_DE_PORT_HOTPLUG_MASK;
@@ -3493,7 +3495,25 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
 	hotplug = I915_READ(PCH_PORT_HOTPLUG);
 	hotplug |= PORTC_HOTPLUG_ENABLE | PORTB_HOTPLUG_ENABLE |
 		PORTA_HOTPLUG_ENABLE;
-	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.
+	 */
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIA)
+		&& intel_bios_is_port_hpd_inverted(dev, PORT_A))
+			val |= BXT_DDIA_HPD_INVERT;
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIB)
+		&& intel_bios_is_port_hpd_inverted(dev, PORT_B))
+			val |= BXT_DDIB_HPD_INVERT;
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIC)
+		&& intel_bios_is_port_hpd_inverted(dev, PORT_C))
+			val |= BXT_DDIC_HPD_INVERT;
+
+	DRM_DEBUG_KMS("Invert bit setting: hp_ctl:%x hp_port:%x val:%x\n",
+				hotplug, enabled_irqs, val);
+	hotplug &= ~BXT_DDI_HPD_INVERT_MASK;
+	I915_WRITE(PCH_PORT_HOTPLUG, hotplug | val);
 }
 
 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 6732fc1..9ed42bb 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5940,6 +5940,14 @@ enum skl_disp_power_wells {
 #define GEN8_PCU_IIR _MMIO(0x444e8)
 #define GEN8_PCU_IER _MMIO(0x444ec)
 
+/* BXT hotplug control */
+#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 --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index a26d4b4..24d0077 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -105,6 +105,48 @@ find_section(const void *_bdb, int section_id)
 	return NULL;
 }
 
+bool
+intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int i;
+
+	if (!IS_BROXTON(dev)) {
+		DRM_ERROR("Bit inversion is not required in this platform\n");
+		return false;
+	}
+
+	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+
+		if (dev_priv->vbt.child_dev[i].common.hpd_invert == 1) {
+
+			switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
+			case DVO_PORT_DPA:
+			case DVO_PORT_HDMIA:
+				if (port == PORT_A)
+					return true;
+				break;
+			case DVO_PORT_DPB:
+			case DVO_PORT_HDMIB:
+				if (port == PORT_B)
+					return true;
+				break;
+			case DVO_PORT_DPC:
+			case DVO_PORT_HDMIC:
+				if (port == PORT_C)
+					return true;
+				break;
+			default:
+				DRM_DEBUG_KMS("This dvo port %d doesn't need invert\n",
+					dev_priv->vbt.child_dev[i].common.dvo_port);
+				break;
+			}
+		}
+	}
+
+	return false;
+}
+
 static void
 fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
 			const struct lvds_dvo_timing *dvo_timing)
-- 
2.6.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-02-25  9:57 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
@ 2016-02-25 13:57   ` Ville Syrjälä
  0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2016-02-25 13:57 UTC (permalink / raw)
  To: Shubhangi Shrivastava; +Cc: intel-gfx

On Thu, Feb 25, 2016 at 03:27:26PM +0530, Shubhangi Shrivastava 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.
> 
> v2: Separated VBT parsing from the rest of the logic. (Jani)
> 
> v3: Moved setting invert bit logic to bxt_hpd_irq_setup()
>     and changed its logic to avoid looping twice. (Ville)
> 
> 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_drv.h   |  1 +
>  drivers/gpu/drm/i915/i915_irq.c   | 22 +++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_reg.h   |  8 ++++++++
>  drivers/gpu/drm/i915/intel_bios.c | 42 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 72 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8216665..457f175 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3393,6 +3393,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
>  /* intel_bios.c */
>  int intel_bios_init(struct drm_i915_private *dev_priv);
>  bool intel_bios_is_valid_vbt(const void *buf, size_t size);
> +bool intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port);
>  
>  /* intel_opregion.c */
>  #ifdef CONFIG_ACPI
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 25a8937..525da56 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -36,6 +36,7 @@
>  #include "i915_drv.h"
>  #include "i915_trace.h"
>  #include "intel_drv.h"
> +#include "intel_bios.h"
>  
>  /**
>   * DOC: interrupt handling
> @@ -3484,6 +3485,7 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 hotplug_irqs, hotplug, enabled_irqs;
> +	int val = 0;

u32

or if you change the logic below to mask out the bits first and then set
them as needed, then you wouldn't need another temporary variable.

>  
>  	enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_bxt);
>  	hotplug_irqs = BXT_DE_PORT_HOTPLUG_MASK;
> @@ -3493,7 +3495,25 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
>  	hotplug = I915_READ(PCH_PORT_HOTPLUG);
>  	hotplug |= PORTC_HOTPLUG_ENABLE | PORTB_HOTPLUG_ENABLE |
>  		PORTA_HOTPLUG_ENABLE;
> -	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.
> +	 */
> +	if ((enabled_irqs & BXT_DE_PORT_HP_DDIA)
> +		&& intel_bios_is_port_hpd_inverted(dev, PORT_A))

&& usually goes on the previous line, and indentation should be so that
the things line up after the opening '('.

> +			val |= BXT_DDIA_HPD_INVERT;
> +	if ((enabled_irqs & BXT_DE_PORT_HP_DDIB)
> +		&& intel_bios_is_port_hpd_inverted(dev, PORT_B))
> +			val |= BXT_DDIB_HPD_INVERT;
> +	if ((enabled_irqs & BXT_DE_PORT_HP_DDIC)
> +		&& intel_bios_is_port_hpd_inverted(dev, PORT_C))
> +			val |= BXT_DDIC_HPD_INVERT;
> +
> +	DRM_DEBUG_KMS("Invert bit setting: hp_ctl:%x hp_port:%x val:%x\n",
> +				hotplug, enabled_irqs, val);
> +	hotplug &= ~BXT_DDI_HPD_INVERT_MASK;
> +	I915_WRITE(PCH_PORT_HOTPLUG, hotplug | val);
>  }
>  
>  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 6732fc1..9ed42bb 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5940,6 +5940,14 @@ enum skl_disp_power_wells {
>  #define GEN8_PCU_IIR _MMIO(0x444e8)
>  #define GEN8_PCU_IER _MMIO(0x444ec)
>  
> +/* BXT hotplug control */
> +#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)

These should be moved to where the other bits for this register are.

> +
>  #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 --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index a26d4b4..24d0077 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -105,6 +105,48 @@ find_section(const void *_bdb, int section_id)
>  	return NULL;
>  }
>  
> +bool
> +intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int i;
> +
> +	if (!IS_BROXTON(dev)) {
> +		DRM_ERROR("Bit inversion is not required in this platform\n");
> +		return false;
> +	}
> +
> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> +
> +		if (dev_priv->vbt.child_dev[i].common.hpd_invert == 1) {
> +
> +			switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
> +			case DVO_PORT_DPA:
> +			case DVO_PORT_HDMIA:
> +				if (port == PORT_A)
> +					return true;
> +				break;
> +			case DVO_PORT_DPB:
> +			case DVO_PORT_HDMIB:
> +				if (port == PORT_B)
> +					return true;
> +				break;
> +			case DVO_PORT_DPC:
> +			case DVO_PORT_HDMIC:
> +				if (port == PORT_C)
> +					return true;
> +				break;
> +			default:
> +				DRM_DEBUG_KMS("This dvo port %d doesn't need invert\n",
> +					dev_priv->vbt.child_dev[i].common.dvo_port);

I still find this debug message confusing. In fact BXT only has ports
A,B,C, so if we end up here it would mean the VBT is broken or something,
no?

> +				break;
> +			}
> +		}
> +	}
> +
> +	return false;
> +}
> +
>  static void
>  fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
>  			const struct lvds_dvo_timing *dvo_timing)
> -- 
> 2.6.1

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-03-11 12:53 [PATCH 1/2] drm/i915: Update VBT fields for child devices Shubhangi Shrivastava
@ 2016-03-11 12:53 ` Shubhangi Shrivastava
  2016-03-11 13:20   ` kbuild test robot
  0 siblings, 1 reply; 24+ messages in thread
From: Shubhangi Shrivastava @ 2016-03-11 12:53 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shubhangi Shrivastava

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.

v2: Separated VBT parsing from the rest of the logic. (Jani)

v3: Moved setting invert bit logic to bxt_hpd_irq_setup()
    and changed its logic to avoid looping twice. (Ville)

v4: Changed the logic to mask out the bits first and then
    set them to remove need of temporary variable. (Ville)

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_drv.h   |  1 +
 drivers/gpu/drm/i915/i915_irq.c   | 21 ++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h   |  8 ++++++++
 drivers/gpu/drm/i915/intel_bios.c | 40 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 70 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9e76bfc..15b6a54 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3398,6 +3398,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
 /* intel_bios.c */
 int intel_bios_init(struct drm_i915_private *dev_priv);
 bool intel_bios_is_valid_vbt(const void *buf, size_t size);
+bool intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port);
 
 /* intel_opregion.c */
 #ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d1a46ef..ba6e844 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -36,6 +36,7 @@
 #include "i915_drv.h"
 #include "i915_trace.h"
 #include "intel_drv.h"
+#include "intel_bios.h"
 
 /**
  * DOC: interrupt handling
@@ -3506,6 +3507,26 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
 	hotplug = I915_READ(PCH_PORT_HOTPLUG);
 	hotplug |= PORTC_HOTPLUG_ENABLE | PORTB_HOTPLUG_ENABLE |
 		PORTA_HOTPLUG_ENABLE;
+
+	DRM_DEBUG_KMS("Invert bit setting: hp_ctl:%x hp_port:%x val:%x\n",
+			hotplug, enabled_irqs, val);
+	hotplug &= ~BXT_DDI_HPD_INVERT_MASK;
+
+	/*
+	 * For BXT invert bit has to be set based on AOB design
+	 * for HPD detection logic, update it based on VBT fields.
+	 */
+
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIA) &&
+	     intel_bios_is_port_hpd_inverted(dev, PORT_A))
+		hotplug |= BXT_DDIA_HPD_INVERT;
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIB) &&
+	     intel_bios_is_port_hpd_inverted(dev, PORT_B))
+		hotplug |= BXT_DDIB_HPD_INVERT;
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIC) &&
+	     intel_bios_is_port_hpd_inverted(dev, PORT_C))
+		hotplug |= BXT_DDIC_HPD_INVERT;
+
 	I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f76cbf3..f994e16 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6212,6 +6212,14 @@ enum skl_disp_power_wells {
 #define  PORTB_HOTPLUG_SHORT_DETECT	(1 << 0)
 #define  PORTB_HOTPLUG_LONG_DETECT	(2 << 0)
 
+/* BXT hotplug control */
+#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 PCH_PORT_HOTPLUG2		_MMIO(0xc403C)	/* SHOTPLUG_CTL2 SPT+ */
 #define  PORTE_HOTPLUG_ENABLE		(1 << 4)
 #define  PORTE_HOTPLUG_STATUS_MASK	(3 << 0)
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index a26d4b4..d809700 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -105,6 +105,46 @@ find_section(const void *_bdb, int section_id)
 	return NULL;
 }
 
+bool
+intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int i;
+
+	if (!IS_BROXTON(dev)) {
+		DRM_ERROR("Bit inversion is not required in this platform\n");
+		return false;
+	}
+
+	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+
+		if (dev_priv->vbt.child_dev[i].common.hpd_invert == 1) {
+
+			switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
+			case DVO_PORT_DPA:
+			case DVO_PORT_HDMIA:
+				if (port == PORT_A)
+					return true;
+				break;
+			case DVO_PORT_DPB:
+			case DVO_PORT_HDMIB:
+				if (port == PORT_B)
+					return true;
+				break;
+			case DVO_PORT_DPC:
+			case DVO_PORT_HDMIC:
+				if (port == PORT_C)
+					return true;
+				break;
+			default:
+				break;
+			}
+		}
+	}
+
+	return false;
+}
+
 static void
 fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
 			const struct lvds_dvo_timing *dvo_timing)
-- 
2.6.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-03-11 12:53 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
@ 2016-03-11 13:20   ` kbuild test robot
  0 siblings, 0 replies; 24+ messages in thread
From: kbuild test robot @ 2016-03-11 13:20 UTC (permalink / raw)
  Cc: intel-gfx, Shubhangi Shrivastava, kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2007 bytes --]

Hi Shubhangi,

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on next-20160311]
[cannot apply to v4.5-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Shubhangi-Shrivastava/drm-i915-Update-VBT-fields-for-child-devices/20160311-205215
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-allmodconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   In file included from drivers/gpu/drm/i915/i915_irq.c:34:0:
   drivers/gpu/drm/i915/i915_irq.c: In function 'bxt_hpd_irq_setup':
>> drivers/gpu/drm/i915/i915_irq.c:3509:27: error: 'val' undeclared (first use in this function)
       hotplug, enabled_irqs, val);
                              ^
   include/drm/drmP.h:212:41: note: in definition of macro 'DRM_DEBUG_KMS'
       drm_ut_debug_printk(__func__, fmt, ##args); \
                                            ^
   drivers/gpu/drm/i915/i915_irq.c:3509:27: note: each undeclared identifier is reported only once for each function it appears in
       hotplug, enabled_irqs, val);
                              ^
   include/drm/drmP.h:212:41: note: in definition of macro 'DRM_DEBUG_KMS'
       drm_ut_debug_printk(__func__, fmt, ##args); \
                                            ^

vim +/val +3509 drivers/gpu/drm/i915/i915_irq.c

  3503	
  3504		hotplug = I915_READ(PCH_PORT_HOTPLUG);
  3505		hotplug |= PORTC_HOTPLUG_ENABLE | PORTB_HOTPLUG_ENABLE |
  3506			PORTA_HOTPLUG_ENABLE;
  3507	
  3508		DRM_DEBUG_KMS("Invert bit setting: hp_ctl:%x hp_port:%x val:%x\n",
> 3509				hotplug, enabled_irqs, val);
  3510		hotplug &= ~BXT_DDI_HPD_INVERT_MASK;
  3511	
  3512		/*

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 53129 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-03-11 13:43 [PATCH 1/2] drm/i915: Update VBT fields for child devices Shubhangi Shrivastava
@ 2016-03-11 13:43 ` Shubhangi Shrivastava
  2016-03-11 14:17   ` Jani Nikula
  0 siblings, 1 reply; 24+ messages in thread
From: Shubhangi Shrivastava @ 2016-03-11 13:43 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shubhangi Shrivastava

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.

v2: Separated VBT parsing from the rest of the logic. (Jani)

v3: Moved setting invert bit logic to bxt_hpd_irq_setup()
    and changed its logic to avoid looping twice. (Ville)

v4: Changed the logic to mask out the bits first and then
    set them to remove need of temporary variable. (Ville)

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_drv.h   |  1 +
 drivers/gpu/drm/i915/i915_irq.c   | 21 ++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h   |  8 ++++++++
 drivers/gpu/drm/i915/intel_bios.c | 40 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 70 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1557d65..91963d24 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3334,6 +3334,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
 /* intel_bios.c */
 int intel_bios_init(struct drm_i915_private *dev_priv);
 bool intel_bios_is_valid_vbt(const void *buf, size_t size);
+bool intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port);
 
 /* intel_opregion.c */
 #ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 53e5104..764d8f8 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -36,6 +36,7 @@
 #include "i915_drv.h"
 #include "i915_trace.h"
 #include "intel_drv.h"
+#include "intel_bios.h"
 
 /**
  * DOC: interrupt handling
@@ -3503,6 +3504,26 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
 	hotplug = I915_READ(PCH_PORT_HOTPLUG);
 	hotplug |= PORTC_HOTPLUG_ENABLE | PORTB_HOTPLUG_ENABLE |
 		PORTA_HOTPLUG_ENABLE;
+
+	DRM_DEBUG_KMS("Invert bit setting: hp_ctl:%x hp_port:%x\n",
+			hotplug, enabled_irqs);
+	hotplug &= ~BXT_DDI_HPD_INVERT_MASK;
+
+	/*
+	 * For BXT invert bit has to be set based on AOB design
+	 * for HPD detection logic, update it based on VBT fields.
+	 */
+
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIA) &&
+	     intel_bios_is_port_hpd_inverted(dev, PORT_A))
+		hotplug |= BXT_DDIA_HPD_INVERT;
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIB) &&
+	     intel_bios_is_port_hpd_inverted(dev, PORT_B))
+		hotplug |= BXT_DDIB_HPD_INVERT;
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIC) &&
+	     intel_bios_is_port_hpd_inverted(dev, PORT_C))
+		hotplug |= BXT_DDIC_HPD_INVERT;
+
 	I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7dfc400..f80b870 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6213,6 +6213,14 @@ enum skl_disp_power_wells {
 #define  PORTB_HOTPLUG_SHORT_DETECT	(1 << 0)
 #define  PORTB_HOTPLUG_LONG_DETECT	(2 << 0)
 
+/* BXT hotplug control */
+#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 PCH_PORT_HOTPLUG2		_MMIO(0xc403C)	/* SHOTPLUG_CTL2 SPT+ */
 #define  PORTE_HOTPLUG_ENABLE		(1 << 4)
 #define  PORTE_HOTPLUG_STATUS_MASK	(3 << 0)
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index a26d4b4..d809700 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -105,6 +105,46 @@ find_section(const void *_bdb, int section_id)
 	return NULL;
 }
 
+bool
+intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int i;
+
+	if (!IS_BROXTON(dev)) {
+		DRM_ERROR("Bit inversion is not required in this platform\n");
+		return false;
+	}
+
+	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+
+		if (dev_priv->vbt.child_dev[i].common.hpd_invert == 1) {
+
+			switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
+			case DVO_PORT_DPA:
+			case DVO_PORT_HDMIA:
+				if (port == PORT_A)
+					return true;
+				break;
+			case DVO_PORT_DPB:
+			case DVO_PORT_HDMIB:
+				if (port == PORT_B)
+					return true;
+				break;
+			case DVO_PORT_DPC:
+			case DVO_PORT_HDMIC:
+				if (port == PORT_C)
+					return true;
+				break;
+			default:
+				break;
+			}
+		}
+	}
+
+	return false;
+}
+
 static void
 fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
 			const struct lvds_dvo_timing *dvo_timing)
-- 
2.6.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-03-11 13:43 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
@ 2016-03-11 14:17   ` Jani Nikula
  0 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2016-03-11 14:17 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shubhangi Shrivastava

On Fri, 11 Mar 2016, Shubhangi Shrivastava <shubhangi.shrivastava@intel.com> wrote:
> [ text/plain ]
> 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.
>
> v2: Separated VBT parsing from the rest of the logic. (Jani)
>
> v3: Moved setting invert bit logic to bxt_hpd_irq_setup()
>     and changed its logic to avoid looping twice. (Ville)
>
> v4: Changed the logic to mask out the bits first and then
>     set them to remove need of temporary variable. (Ville)
>
> 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_drv.h   |  1 +
>  drivers/gpu/drm/i915/i915_irq.c   | 21 ++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h   |  8 ++++++++
>  drivers/gpu/drm/i915/intel_bios.c | 40 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 70 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1557d65..91963d24 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3334,6 +3334,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
>  /* intel_bios.c */
>  int intel_bios_init(struct drm_i915_private *dev_priv);
>  bool intel_bios_is_valid_vbt(const void *buf, size_t size);
> +bool intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port);
>  
>  /* intel_opregion.c */
>  #ifdef CONFIG_ACPI
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 53e5104..764d8f8 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -36,6 +36,7 @@
>  #include "i915_drv.h"
>  #include "i915_trace.h"
>  #include "intel_drv.h"
> +#include "intel_bios.h"

What for? I don't think you need this.

>  
>  /**
>   * DOC: interrupt handling
> @@ -3503,6 +3504,26 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
>  	hotplug = I915_READ(PCH_PORT_HOTPLUG);
>  	hotplug |= PORTC_HOTPLUG_ENABLE | PORTB_HOTPLUG_ENABLE |
>  		PORTA_HOTPLUG_ENABLE;
> +
> +	DRM_DEBUG_KMS("Invert bit setting: hp_ctl:%x hp_port:%x\n",
> +			hotplug, enabled_irqs);
> +	hotplug &= ~BXT_DDI_HPD_INVERT_MASK;
> +
> +	/*
> +	 * For BXT invert bit has to be set based on AOB design
> +	 * for HPD detection logic, update it based on VBT fields.
> +	 */
> +
> +	if ((enabled_irqs & BXT_DE_PORT_HP_DDIA) &&
> +	     intel_bios_is_port_hpd_inverted(dev, PORT_A))
> +		hotplug |= BXT_DDIA_HPD_INVERT;
> +	if ((enabled_irqs & BXT_DE_PORT_HP_DDIB) &&
> +	     intel_bios_is_port_hpd_inverted(dev, PORT_B))
> +		hotplug |= BXT_DDIB_HPD_INVERT;
> +	if ((enabled_irqs & BXT_DE_PORT_HP_DDIC) &&
> +	     intel_bios_is_port_hpd_inverted(dev, PORT_C))
> +		hotplug |= BXT_DDIC_HPD_INVERT;
> +
>  	I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7dfc400..f80b870 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6213,6 +6213,14 @@ enum skl_disp_power_wells {
>  #define  PORTB_HOTPLUG_SHORT_DETECT	(1 << 0)
>  #define  PORTB_HOTPLUG_LONG_DETECT	(2 << 0)
>  
> +/* BXT hotplug control */
> +#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 PCH_PORT_HOTPLUG2		_MMIO(0xc403C)	/* SHOTPLUG_CTL2 SPT+ */
>  #define  PORTE_HOTPLUG_ENABLE		(1 << 4)
>  #define  PORTE_HOTPLUG_STATUS_MASK	(3 << 0)
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index a26d4b4..d809700 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -105,6 +105,46 @@ find_section(const void *_bdb, int section_id)
>  	return NULL;
>  }
>  
> +bool
> +intel_bios_is_port_hpd_inverted(struct drm_device *dev, enum port port)

You should pass in dev_priv directly, nothing here needs dev.

> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int i;
> +
> +	if (!IS_BROXTON(dev)) {
> +		DRM_ERROR("Bit inversion is not required in this platform\n");

If this happens we screwed up. If that shows up in dmesg out of context,
it's pretty ambiguous.

Please just do

	if (WARN_ON_ONCE(!IS_BROXTON(dev_priv)))
		return false;

> +		return false;
> +	}
> +
> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> +
> +		if (dev_priv->vbt.child_dev[i].common.hpd_invert == 1) {

I think this reads better and reduces indent as:

	if (!dev_priv->vbt.child_dev[i].common.hpd_invert)
        	continue;

> +
> +			switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
> +			case DVO_PORT_DPA:
> +			case DVO_PORT_HDMIA:
> +				if (port == PORT_A)
> +					return true;
> +				break;
> +			case DVO_PORT_DPB:
> +			case DVO_PORT_HDMIB:
> +				if (port == PORT_B)
> +					return true;
> +				break;
> +			case DVO_PORT_DPC:
> +			case DVO_PORT_HDMIC:
> +				if (port == PORT_C)
> +					return true;
> +				break;
> +			default:
> +				break;
> +			}
> +		}
> +	}
> +
> +	return false;
> +}
> +
>  static void
>  fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
>  			const struct lvds_dvo_timing *dvo_timing)

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-03-15 13:13 [PATCH 1/2] drm/i915: Update VBT fields for child devices Shubhangi Shrivastava
@ 2016-03-15 13:13 ` Shubhangi Shrivastava
  2016-03-15 14:51   ` Ville Syrjälä
  0 siblings, 1 reply; 24+ messages in thread
From: Shubhangi Shrivastava @ 2016-03-15 13:13 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shubhangi Shrivastava

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.

v2: Separated VBT parsing from the rest of the logic. (Jani)

v3: Moved setting invert bit logic to bxt_hpd_irq_setup()
    and changed its logic to avoid looping twice. (Ville)

v4: Changed the logic to mask out the bits first and then
    set them to remove need of temporary variable. (Ville)

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_drv.h   |  2 ++
 drivers/gpu/drm/i915/i915_irq.c   | 20 ++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h   |  8 ++++++++
 drivers/gpu/drm/i915/intel_bios.c | 35 +++++++++++++++++++++++++++++++++++
 4 files changed, 65 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1557d65..7a00966 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3334,6 +3334,8 @@ extern void intel_i2c_reset(struct drm_device *dev);
 /* intel_bios.c */
 int intel_bios_init(struct drm_i915_private *dev_priv);
 bool intel_bios_is_valid_vbt(const void *buf, size_t size);
+bool intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
+				     enum port port);
 
 /* intel_opregion.c */
 #ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 53e5104..0a1e378 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3503,6 +3503,26 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
 	hotplug = I915_READ(PCH_PORT_HOTPLUG);
 	hotplug |= PORTC_HOTPLUG_ENABLE | PORTB_HOTPLUG_ENABLE |
 		PORTA_HOTPLUG_ENABLE;
+
+	DRM_DEBUG_KMS("Invert bit setting: hp_ctl:%x hp_port:%x\n",
+			hotplug, enabled_irqs);
+	hotplug &= ~BXT_DDI_HPD_INVERT_MASK;
+
+	/*
+	 * For BXT invert bit has to be set based on AOB design
+	 * for HPD detection logic, update it based on VBT fields.
+	 */
+
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIA) &&
+	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_A))
+		hotplug |= BXT_DDIA_HPD_INVERT;
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIB) &&
+	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_B))
+		hotplug |= BXT_DDIB_HPD_INVERT;
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIC) &&
+	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_C))
+		hotplug |= BXT_DDIC_HPD_INVERT;
+
 	I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7dfc400..f80b870 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6213,6 +6213,14 @@ enum skl_disp_power_wells {
 #define  PORTB_HOTPLUG_SHORT_DETECT	(1 << 0)
 #define  PORTB_HOTPLUG_LONG_DETECT	(2 << 0)
 
+/* BXT hotplug control */
+#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 PCH_PORT_HOTPLUG2		_MMIO(0xc403C)	/* SHOTPLUG_CTL2 SPT+ */
 #define  PORTE_HOTPLUG_ENABLE		(1 << 4)
 #define  PORTE_HOTPLUG_STATUS_MASK	(3 << 0)
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index a26d4b4..71d62f8 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -105,6 +105,41 @@ find_section(const void *_bdb, int section_id)
 	return NULL;
 }
 
+bool
+intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
+				enum port port)
+{
+	int i;
+
+	if (WARN_ON_ONCE(!IS_BROXTON(dev_priv)))
+		return false;
+
+	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+
+		if (!dev_priv->vbt.child_dev[i].common.hpd_invert)
+			continue;
+
+		switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
+		case DVO_PORT_DPA:
+		case DVO_PORT_HDMIA:
+			if (port == PORT_A)
+				return true;
+		case DVO_PORT_DPB:
+		case DVO_PORT_HDMIB:
+			if (port == PORT_B)
+				return true;
+		case DVO_PORT_DPC:
+		case DVO_PORT_HDMIC:
+			if (port == PORT_C)
+				return true;
+		default:
+			break;
+		}
+	}
+
+	return false;
+}
+
 static void
 fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
 			const struct lvds_dvo_timing *dvo_timing)
-- 
2.6.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-03-15 13:13 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
@ 2016-03-15 14:51   ` Ville Syrjälä
  0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2016-03-15 14:51 UTC (permalink / raw)
  To: Shubhangi Shrivastava; +Cc: intel-gfx

On Tue, Mar 15, 2016 at 06:43:54PM +0530, Shubhangi Shrivastava 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.
> 
> v2: Separated VBT parsing from the rest of the logic. (Jani)
> 
> v3: Moved setting invert bit logic to bxt_hpd_irq_setup()
>     and changed its logic to avoid looping twice. (Ville)
> 
> v4: Changed the logic to mask out the bits first and then
>     set them to remove need of temporary variable. (Ville)
> 
> 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_drv.h   |  2 ++
>  drivers/gpu/drm/i915/i915_irq.c   | 20 ++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h   |  8 ++++++++
>  drivers/gpu/drm/i915/intel_bios.c | 35 +++++++++++++++++++++++++++++++++++
>  4 files changed, 65 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1557d65..7a00966 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3334,6 +3334,8 @@ extern void intel_i2c_reset(struct drm_device *dev);
>  /* intel_bios.c */
>  int intel_bios_init(struct drm_i915_private *dev_priv);
>  bool intel_bios_is_valid_vbt(const void *buf, size_t size);
> +bool intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
> +				     enum port port);
>  
>  /* intel_opregion.c */
>  #ifdef CONFIG_ACPI
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 53e5104..0a1e378 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3503,6 +3503,26 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
>  	hotplug = I915_READ(PCH_PORT_HOTPLUG);
>  	hotplug |= PORTC_HOTPLUG_ENABLE | PORTB_HOTPLUG_ENABLE |
>  		PORTA_HOTPLUG_ENABLE;
> +
> +	DRM_DEBUG_KMS("Invert bit setting: hp_ctl:%x hp_port:%x\n",
> +			hotplug, enabled_irqs);
> +	hotplug &= ~BXT_DDI_HPD_INVERT_MASK;
> +
> +	/*
> +	 * For BXT invert bit has to be set based on AOB design
> +	 * for HPD detection logic, update it based on VBT fields.
> +	 */
> +
> +	if ((enabled_irqs & BXT_DE_PORT_HP_DDIA) &&
> +	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_A))
> +		hotplug |= BXT_DDIA_HPD_INVERT;
> +	if ((enabled_irqs & BXT_DE_PORT_HP_DDIB) &&
> +	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_B))
> +		hotplug |= BXT_DDIB_HPD_INVERT;
> +	if ((enabled_irqs & BXT_DE_PORT_HP_DDIC) &&
> +	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_C))
> +		hotplug |= BXT_DDIC_HPD_INVERT;
> +
>  	I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7dfc400..f80b870 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6213,6 +6213,14 @@ enum skl_disp_power_wells {
>  #define  PORTB_HOTPLUG_SHORT_DETECT	(1 << 0)
>  #define  PORTB_HOTPLUG_LONG_DETECT	(2 << 0)
>  
> +/* BXT hotplug control */
> +#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)

The bits defines should be inserted to the proper place in the already
existing set of defines for this register.

> +
>  #define PCH_PORT_HOTPLUG2		_MMIO(0xc403C)	/* SHOTPLUG_CTL2 SPT+ */
>  #define  PORTE_HOTPLUG_ENABLE		(1 << 4)
>  #define  PORTE_HOTPLUG_STATUS_MASK	(3 << 0)
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index a26d4b4..71d62f8 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -105,6 +105,41 @@ find_section(const void *_bdb, int section_id)
>  	return NULL;
>  }
>  
> +bool
> +intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
> +				enum port port)
> +{
> +	int i;
> +
> +	if (WARN_ON_ONCE(!IS_BROXTON(dev_priv)))
> +		return false;
> +
> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> +
> +		if (!dev_priv->vbt.child_dev[i].common.hpd_invert)
> +			continue;
> +
> +		switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
> +		case DVO_PORT_DPA:
> +		case DVO_PORT_HDMIA:
> +			if (port == PORT_A)
> +				return true;

break;

> +		case DVO_PORT_DPB:
> +		case DVO_PORT_HDMIB:
> +			if (port == PORT_B)
> +				return true;

break;

> +		case DVO_PORT_DPC:
> +		case DVO_PORT_HDMIC:
> +			if (port == PORT_C)
> +				return true;

break;

With the missing breaks added and the defines moved to the right
place this is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +		default:
> +			break;
> +		}
> +	}
> +
> +	return false;
> +}
> +
>  static void
>  fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
>  			const struct lvds_dvo_timing *dvo_timing)
> -- 
> 2.6.1

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/2] drm/i915: Update VBT fields for child devices
@ 2016-03-24 12:10 Shubhangi Shrivastava
  2016-03-24 12:10 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
  2016-03-24 13:36 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Update VBT fields for child devices Patchwork
  0 siblings, 2 replies; 24+ messages in thread
From: Shubhangi Shrivastava @ 2016-03-24 12:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shubhangi Shrivastava

This patch adds new fields that are not yet added in drm code
in child devices struct

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>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c     | 15 ++++++++++++++-
 drivers/gpu/drm/i915/intel_vbt_defs.h | 16 +++++++++++-----
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 083003b..e2f636c 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1126,7 +1126,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
 	}
 
 	/* Parse the I_boost config for SKL and above */
-	if (bdb->version >= 196 && (child->common.flags_1 & IBOOST_ENABLE)) {
+	if (bdb->version >= 196 && child->common.iboost) {
 		info->dp_boost_level = translate_iboost(child->common.iboost_level & 0xF);
 		DRM_DEBUG_KMS("VBT (e)DP boost level for port %c: %d\n",
 			      port_name(port), info->dp_boost_level);
@@ -1244,6 +1244,19 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
 		 */
 		memcpy(child_dev_ptr, p_child,
 		       min_t(size_t, p_defs->child_dev_size, sizeof(*p_child)));
+
+		/*
+		 * copied full block, now init values when they are not
+		 * available in current version
+		 */
+		if (bdb->version < 196) {
+			/* Set default values for bits added from v196 */
+			child_dev_ptr->common.iboost = 0;
+			child_dev_ptr->common.hpd_invert = 0;
+		}
+
+		if (bdb->version < 192)
+			child_dev_ptr->common.lspcon = 0;
 	}
 	return;
 }
diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h
index 749dcea..2da7be8 100644
--- a/drivers/gpu/drm/i915/intel_vbt_defs.h
+++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
@@ -261,9 +261,6 @@ struct old_child_dev_config {
  * versions. Notice that the meaning of the contents contents may still change,
  * but at least the offsets are consistent. */
 
-/* Definitions for flags_1 */
-#define IBOOST_ENABLE (1<<3)
-
 struct common_child_dev_config {
 	u16 handle;
 	u16 device_type;
@@ -272,8 +269,17 @@ struct common_child_dev_config {
 	u8 not_common2[2];
 	u8 ddc_pin;
 	u16 edid_ptr;
-	u8 obsolete;
-	u8 flags_1;
+	u8 dvo_cfg; /* See DEVICE_CFG_* above */
+	u8 efp_routed:1;
+	u8 lane_reversal:1;
+	u8 lspcon:1;
+	u8 iboost:1;
+	u8 hpd_invert:1;
+	u8 flag_reserved:3;
+	u8 hdmi_support:1;
+	u8 dp_support:1;
+	u8 tmds_support:1;
+	u8 support_reserved:5;
 	u8 not_common3[13];
 	u8 iboost_level;
 } __packed;
-- 
2.6.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-03-24 12:10 [PATCH 1/2] drm/i915: Update VBT fields for child devices Shubhangi Shrivastava
@ 2016-03-24 12:10 ` Shubhangi Shrivastava
  2016-03-24 14:39   ` Ville Syrjälä
  2016-03-24 13:36 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Update VBT fields for child devices Patchwork
  1 sibling, 1 reply; 24+ messages in thread
From: Shubhangi Shrivastava @ 2016-03-24 12:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shubhangi Shrivastava

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.

v2: Separated VBT parsing from the rest of the logic. (Jani)

v3: Moved setting invert bit logic to bxt_hpd_irq_setup()
    and changed its logic to avoid looping twice. (Ville)

v4: Changed the logic to mask out the bits first and then
    set them to remove need of temporary variable. (Ville)

v5: Moved defines to existing set of defines for the register
    Changed logic to incorporate required breaks. (Ville)

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>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  2 ++
 drivers/gpu/drm/i915/i915_irq.c   | 20 ++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h   |  6 ++++++
 drivers/gpu/drm/i915/intel_bios.c | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 60 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9d29ab0..86fb5cb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3396,6 +3396,8 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
 bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
 bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
 bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
+bool intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
+				     enum port port);
 
 /* intel_opregion.c */
 #ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a55a7cc..8fbec3e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3504,6 +3504,26 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
 	hotplug = I915_READ(PCH_PORT_HOTPLUG);
 	hotplug |= PORTC_HOTPLUG_ENABLE | PORTB_HOTPLUG_ENABLE |
 		PORTA_HOTPLUG_ENABLE;
+
+	DRM_DEBUG_KMS("Invert bit setting: hp_ctl:%x hp_port:%x\n",
+			hotplug, enabled_irqs);
+	hotplug &= ~BXT_DDI_HPD_INVERT_MASK;
+
+	/*
+	 * For BXT invert bit has to be set based on AOB design
+	 * for HPD detection logic, update it based on VBT fields.
+	 */
+
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIA) &&
+	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_A))
+		hotplug |= BXT_DDIA_HPD_INVERT;
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIB) &&
+	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_B))
+		hotplug |= BXT_DDIB_HPD_INVERT;
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIC) &&
+	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_C))
+		hotplug |= BXT_DDIC_HPD_INVERT;
+
 	I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f3ba43c..73a806c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6185,6 +6185,7 @@ enum skl_disp_power_wells {
 /* digital port hotplug */
 #define PCH_PORT_HOTPLUG		_MMIO(0xc4030)	/* SHOTPLUG_CTL */
 #define  PORTA_HOTPLUG_ENABLE		(1 << 28) /* LPT:LP+ & BXT */
+#define  BXT_DDIA_HPD_INVERT            (1 << 27)
 #define  PORTA_HOTPLUG_STATUS_MASK	(3 << 24) /* SPT+ & BXT */
 #define  PORTA_HOTPLUG_NO_DETECT	(0 << 24) /* SPT+ & BXT */
 #define  PORTA_HOTPLUG_SHORT_DETECT	(1 << 24) /* SPT+ & BXT */
@@ -6200,6 +6201,7 @@ enum skl_disp_power_wells {
 #define  PORTD_HOTPLUG_SHORT_DETECT	(1 << 16)
 #define  PORTD_HOTPLUG_LONG_DETECT	(2 << 16)
 #define  PORTC_HOTPLUG_ENABLE		(1 << 12)
+#define  BXT_DDIC_HPD_INVERT            (1 << 11)
 #define  PORTC_PULSE_DURATION_2ms	(0 << 10) /* pre-LPT */
 #define  PORTC_PULSE_DURATION_4_5ms	(1 << 10) /* pre-LPT */
 #define  PORTC_PULSE_DURATION_6ms	(2 << 10) /* pre-LPT */
@@ -6210,6 +6212,7 @@ enum skl_disp_power_wells {
 #define  PORTC_HOTPLUG_SHORT_DETECT	(1 << 8)
 #define  PORTC_HOTPLUG_LONG_DETECT	(2 << 8)
 #define  PORTB_HOTPLUG_ENABLE		(1 << 4)
+#define  BXT_DDIB_HPD_INVERT            (1 << 3)
 #define  PORTB_PULSE_DURATION_2ms	(0 << 2) /* pre-LPT */
 #define  PORTB_PULSE_DURATION_4_5ms	(1 << 2) /* pre-LPT */
 #define  PORTB_PULSE_DURATION_6ms	(2 << 2) /* pre-LPT */
@@ -6219,6 +6222,9 @@ enum skl_disp_power_wells {
 #define  PORTB_HOTPLUG_NO_DETECT	(0 << 0)
 #define  PORTB_HOTPLUG_SHORT_DETECT	(1 << 0)
 #define  PORTB_HOTPLUG_LONG_DETECT	(2 << 0)
+#define  BXT_DDI_HPD_INVERT_MASK	(BXT_DDIA_HPD_INVERT | \
+					BXT_DDIB_HPD_INVERT | \
+					BXT_DDIC_HPD_INVERT)
 
 #define PCH_PORT_HOTPLUG2		_MMIO(0xc403C)	/* SHOTPLUG_CTL2 SPT+ */
 #define  PORTE_HOTPLUG_ENABLE		(1 << 4)
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index e2f636c..e015a95 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -107,6 +107,38 @@ find_section(const void *_bdb, int section_id)
 	return NULL;
 }
 
+bool
+intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
+				enum port port)
+{
+	int i;
+
+	if (WARN_ON_ONCE(!IS_BROXTON(dev_priv)))
+		return false;
+
+	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+
+		if (!dev_priv->vbt.child_dev[i].common.hpd_invert)
+			continue;
+
+		switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
+		case DVO_PORT_DPA:
+		case DVO_PORT_HDMIA:
+			return port == PORT_A;
+		case DVO_PORT_DPB:
+		case DVO_PORT_HDMIB:
+			return port == PORT_B;
+		case DVO_PORT_DPC:
+		case DVO_PORT_HDMIC:
+			return port == PORT_C;
+		default:
+			break;
+		}
+	}
+
+	return false;
+}
+
 static void
 fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
 			const struct lvds_dvo_timing *dvo_timing)
-- 
2.6.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Update VBT fields for child devices
  2016-03-24 12:10 [PATCH 1/2] drm/i915: Update VBT fields for child devices Shubhangi Shrivastava
  2016-03-24 12:10 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
@ 2016-03-24 13:36 ` Patchwork
  1 sibling, 0 replies; 24+ messages in thread
From: Patchwork @ 2016-03-24 13:36 UTC (permalink / raw)
  To: Shubhangi Shrivastava; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Update VBT fields for child devices
URL   : https://patchwork.freedesktop.org/series/4858/
State : success

== Summary ==

Series 4858v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/4858/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s3:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                dmesg-warn -> PASS       (bsw-nuc-2)
        Subgroup basic-rte:
                dmesg-warn -> PASS       (byt-nuc) UNSTABLE

bdw-nuci7        total:192  pass:179  dwarn:0   dfail:0   fail:1   skip:12 
bdw-ultra        total:192  pass:170  dwarn:0   dfail:0   fail:1   skip:21 
bsw-nuc-2        total:192  pass:155  dwarn:0   dfail:0   fail:0   skip:37 
byt-nuc          total:192  pass:157  dwarn:0   dfail:0   fail:0   skip:35 
hsw-brixbox      total:192  pass:170  dwarn:0   dfail:0   fail:0   skip:22 
hsw-gt2          total:192  pass:175  dwarn:0   dfail:0   fail:0   skip:17 
ivb-t430s        total:192  pass:167  dwarn:0   dfail:0   fail:0   skip:25 
skl-i7k-2        total:192  pass:169  dwarn:0   dfail:0   fail:0   skip:23 
skl-nuci5        total:192  pass:181  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:192  pass:158  dwarn:0   dfail:0   fail:0   skip:34 
snb-x220t        total:192  pass:158  dwarn:0   dfail:0   fail:1   skip:33 

Results at /archive/results/CI_IGT_test/Patchwork_1706/

83ec122b900baae1aca2bc11eedc28f2d9ea5060 drm-intel-nightly: 2016y-03m-24d-12h-48m-43s UTC integration manifest
031e9198b73186de00abd4fe545ac0a6cef01446 drm/i915: Set invert bit for hpd based on VBT
3a12d0a0a5a7c274d631b3e767008035139c6e99 drm/i915: Update VBT fields for child devices

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-03-24 12:10 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
@ 2016-03-24 14:39   ` Ville Syrjälä
  0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2016-03-24 14:39 UTC (permalink / raw)
  To: Shubhangi Shrivastava; +Cc: intel-gfx

On Thu, Mar 24, 2016 at 05:40:51PM +0530, Shubhangi Shrivastava 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.
> 
> v2: Separated VBT parsing from the rest of the logic. (Jani)
> 
> v3: Moved setting invert bit logic to bxt_hpd_irq_setup()
>     and changed its logic to avoid looping twice. (Ville)
> 
> v4: Changed the logic to mask out the bits first and then
>     set them to remove need of temporary variable. (Ville)
> 
> v5: Moved defines to existing set of defines for the register
>     Changed logic to incorporate required breaks. (Ville)
> 
> 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>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  2 ++
>  drivers/gpu/drm/i915/i915_irq.c   | 20 ++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h   |  6 ++++++
>  drivers/gpu/drm/i915/intel_bios.c | 32 ++++++++++++++++++++++++++++++++
>  4 files changed, 60 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9d29ab0..86fb5cb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3396,6 +3396,8 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
>  bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
>  bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
>  bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
> +bool intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
> +				     enum port port);
>  
>  /* intel_opregion.c */
>  #ifdef CONFIG_ACPI
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index a55a7cc..8fbec3e 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3504,6 +3504,26 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
>  	hotplug = I915_READ(PCH_PORT_HOTPLUG);
>  	hotplug |= PORTC_HOTPLUG_ENABLE | PORTB_HOTPLUG_ENABLE |
>  		PORTA_HOTPLUG_ENABLE;
> +
> +	DRM_DEBUG_KMS("Invert bit setting: hp_ctl:%x hp_port:%x\n",
> +			hotplug, enabled_irqs);
> +	hotplug &= ~BXT_DDI_HPD_INVERT_MASK;
> +
> +	/*
> +	 * For BXT invert bit has to be set based on AOB design
> +	 * for HPD detection logic, update it based on VBT fields.
> +	 */
> +
> +	if ((enabled_irqs & BXT_DE_PORT_HP_DDIA) &&
> +	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_A))
> +		hotplug |= BXT_DDIA_HPD_INVERT;
> +	if ((enabled_irqs & BXT_DE_PORT_HP_DDIB) &&
> +	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_B))
> +		hotplug |= BXT_DDIB_HPD_INVERT;
> +	if ((enabled_irqs & BXT_DE_PORT_HP_DDIC) &&
> +	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_C))
> +		hotplug |= BXT_DDIC_HPD_INVERT;
> +
>  	I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f3ba43c..73a806c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6185,6 +6185,7 @@ enum skl_disp_power_wells {
>  /* digital port hotplug */
>  #define PCH_PORT_HOTPLUG		_MMIO(0xc4030)	/* SHOTPLUG_CTL */
>  #define  PORTA_HOTPLUG_ENABLE		(1 << 28) /* LPT:LP+ & BXT */
> +#define  BXT_DDIA_HPD_INVERT            (1 << 27)
>  #define  PORTA_HOTPLUG_STATUS_MASK	(3 << 24) /* SPT+ & BXT */
>  #define  PORTA_HOTPLUG_NO_DETECT	(0 << 24) /* SPT+ & BXT */
>  #define  PORTA_HOTPLUG_SHORT_DETECT	(1 << 24) /* SPT+ & BXT */
> @@ -6200,6 +6201,7 @@ enum skl_disp_power_wells {
>  #define  PORTD_HOTPLUG_SHORT_DETECT	(1 << 16)
>  #define  PORTD_HOTPLUG_LONG_DETECT	(2 << 16)
>  #define  PORTC_HOTPLUG_ENABLE		(1 << 12)
> +#define  BXT_DDIC_HPD_INVERT            (1 << 11)
>  #define  PORTC_PULSE_DURATION_2ms	(0 << 10) /* pre-LPT */
>  #define  PORTC_PULSE_DURATION_4_5ms	(1 << 10) /* pre-LPT */
>  #define  PORTC_PULSE_DURATION_6ms	(2 << 10) /* pre-LPT */
> @@ -6210,6 +6212,7 @@ enum skl_disp_power_wells {
>  #define  PORTC_HOTPLUG_SHORT_DETECT	(1 << 8)
>  #define  PORTC_HOTPLUG_LONG_DETECT	(2 << 8)
>  #define  PORTB_HOTPLUG_ENABLE		(1 << 4)
> +#define  BXT_DDIB_HPD_INVERT            (1 << 3)
>  #define  PORTB_PULSE_DURATION_2ms	(0 << 2) /* pre-LPT */
>  #define  PORTB_PULSE_DURATION_4_5ms	(1 << 2) /* pre-LPT */
>  #define  PORTB_PULSE_DURATION_6ms	(2 << 2) /* pre-LPT */
> @@ -6219,6 +6222,9 @@ enum skl_disp_power_wells {
>  #define  PORTB_HOTPLUG_NO_DETECT	(0 << 0)
>  #define  PORTB_HOTPLUG_SHORT_DETECT	(1 << 0)
>  #define  PORTB_HOTPLUG_LONG_DETECT	(2 << 0)
> +#define  BXT_DDI_HPD_INVERT_MASK	(BXT_DDIA_HPD_INVERT | \
> +					BXT_DDIB_HPD_INVERT | \
> +					BXT_DDIC_HPD_INVERT)
>  
>  #define PCH_PORT_HOTPLUG2		_MMIO(0xc403C)	/* SHOTPLUG_CTL2 SPT+ */
>  #define  PORTE_HOTPLUG_ENABLE		(1 << 4)
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index e2f636c..e015a95 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -107,6 +107,38 @@ find_section(const void *_bdb, int section_id)
>  	return NULL;
>  }
>  
> +bool
> +intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
> +				enum port port)
> +{
> +	int i;
> +
> +	if (WARN_ON_ONCE(!IS_BROXTON(dev_priv)))
> +		return false;
> +
> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> +
> +		if (!dev_priv->vbt.child_dev[i].common.hpd_invert)
> +			continue;
> +
> +		switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
> +		case DVO_PORT_DPA:
> +		case DVO_PORT_HDMIA:
> +			return port == PORT_A;
> +		case DVO_PORT_DPB:
> +		case DVO_PORT_HDMIB:
> +			return port == PORT_B;
> +		case DVO_PORT_DPC:
> +		case DVO_PORT_HDMIC:
> +			return port == PORT_C;

That doesn't actually work. Eg. if called with port==PORT_B and there's
a port A listed first in the VBT with hpd_invert==true, we'll end up
returning false always for PORT_B regardless of what VBT says we should
return for port B.

> +		default:
> +			break;
> +		}
> +	}
> +
> +	return false;
> +}
> +
>  static void
>  fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
>  			const struct lvds_dvo_timing *dvo_timing)
> -- 
> 2.6.1

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-03-25 17:55 [PATCH 1/2] drm/i915: Update VBT fields for child devices Shubhangi Shrivastava
@ 2016-03-25 17:55 ` Shubhangi Shrivastava
  0 siblings, 0 replies; 24+ messages in thread
From: Shubhangi Shrivastava @ 2016-03-25 17:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shubhangi Shrivastava

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.

v2: Separated VBT parsing from the rest of the logic. (Jani)

v3: Moved setting invert bit logic to bxt_hpd_irq_setup()
    and changed its logic to avoid looping twice. (Ville)

v4: Changed the logic to mask out the bits first and then
    set them to remove need of temporary variable. (Ville)

v5: Moved defines to existing set of defines for the register
    and added required breaks. (Ville)

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>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  2 ++
 drivers/gpu/drm/i915/i915_irq.c   | 20 ++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h   |  6 ++++++
 drivers/gpu/drm/i915/intel_bios.c | 38 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 66 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 08b88c0..4e8d78a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3378,6 +3378,8 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
 bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
 bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
 bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
+bool intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
+				     enum port port);
 
 /* intel_opregion.c */
 #ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a55a7cc..8fbec3e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3504,6 +3504,26 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
 	hotplug = I915_READ(PCH_PORT_HOTPLUG);
 	hotplug |= PORTC_HOTPLUG_ENABLE | PORTB_HOTPLUG_ENABLE |
 		PORTA_HOTPLUG_ENABLE;
+
+	DRM_DEBUG_KMS("Invert bit setting: hp_ctl:%x hp_port:%x\n",
+			hotplug, enabled_irqs);
+	hotplug &= ~BXT_DDI_HPD_INVERT_MASK;
+
+	/*
+	 * For BXT invert bit has to be set based on AOB design
+	 * for HPD detection logic, update it based on VBT fields.
+	 */
+
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIA) &&
+	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_A))
+		hotplug |= BXT_DDIA_HPD_INVERT;
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIB) &&
+	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_B))
+		hotplug |= BXT_DDIB_HPD_INVERT;
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIC) &&
+	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_C))
+		hotplug |= BXT_DDIC_HPD_INVERT;
+
 	I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f3ba43c..73a806c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6185,6 +6185,7 @@ enum skl_disp_power_wells {
 /* digital port hotplug */
 #define PCH_PORT_HOTPLUG		_MMIO(0xc4030)	/* SHOTPLUG_CTL */
 #define  PORTA_HOTPLUG_ENABLE		(1 << 28) /* LPT:LP+ & BXT */
+#define  BXT_DDIA_HPD_INVERT            (1 << 27)
 #define  PORTA_HOTPLUG_STATUS_MASK	(3 << 24) /* SPT+ & BXT */
 #define  PORTA_HOTPLUG_NO_DETECT	(0 << 24) /* SPT+ & BXT */
 #define  PORTA_HOTPLUG_SHORT_DETECT	(1 << 24) /* SPT+ & BXT */
@@ -6200,6 +6201,7 @@ enum skl_disp_power_wells {
 #define  PORTD_HOTPLUG_SHORT_DETECT	(1 << 16)
 #define  PORTD_HOTPLUG_LONG_DETECT	(2 << 16)
 #define  PORTC_HOTPLUG_ENABLE		(1 << 12)
+#define  BXT_DDIC_HPD_INVERT            (1 << 11)
 #define  PORTC_PULSE_DURATION_2ms	(0 << 10) /* pre-LPT */
 #define  PORTC_PULSE_DURATION_4_5ms	(1 << 10) /* pre-LPT */
 #define  PORTC_PULSE_DURATION_6ms	(2 << 10) /* pre-LPT */
@@ -6210,6 +6212,7 @@ enum skl_disp_power_wells {
 #define  PORTC_HOTPLUG_SHORT_DETECT	(1 << 8)
 #define  PORTC_HOTPLUG_LONG_DETECT	(2 << 8)
 #define  PORTB_HOTPLUG_ENABLE		(1 << 4)
+#define  BXT_DDIB_HPD_INVERT            (1 << 3)
 #define  PORTB_PULSE_DURATION_2ms	(0 << 2) /* pre-LPT */
 #define  PORTB_PULSE_DURATION_4_5ms	(1 << 2) /* pre-LPT */
 #define  PORTB_PULSE_DURATION_6ms	(2 << 2) /* pre-LPT */
@@ -6219,6 +6222,9 @@ enum skl_disp_power_wells {
 #define  PORTB_HOTPLUG_NO_DETECT	(0 << 0)
 #define  PORTB_HOTPLUG_SHORT_DETECT	(1 << 0)
 #define  PORTB_HOTPLUG_LONG_DETECT	(2 << 0)
+#define  BXT_DDI_HPD_INVERT_MASK	(BXT_DDIA_HPD_INVERT | \
+					BXT_DDIB_HPD_INVERT | \
+					BXT_DDIC_HPD_INVERT)
 
 #define PCH_PORT_HOTPLUG2		_MMIO(0xc403C)	/* SHOTPLUG_CTL2 SPT+ */
 #define  PORTE_HOTPLUG_ENABLE		(1 << 4)
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index e2f636c..5e78406 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -107,6 +107,44 @@ find_section(const void *_bdb, int section_id)
 	return NULL;
 }
 
+bool
+intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
+				enum port port)
+{
+	int i;
+
+	if (WARN_ON_ONCE(!IS_BROXTON(dev_priv)))
+		return false;
+
+	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+
+		if (!dev_priv->vbt.child_dev[i].common.hpd_invert)
+			continue;
+
+		switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
+		case DVO_PORT_DPA:
+		case DVO_PORT_HDMIA:
+			if (port == PORT_A)
+				return true;
+			break;
+		case DVO_PORT_DPB:
+		case DVO_PORT_HDMIB:
+			if (port == PORT_B)
+				return true;
+			break;
+		case DVO_PORT_DPC:
+		case DVO_PORT_HDMIC:
+			if (port == PORT_C)
+				return true;
+			break;
+		default:
+			break;
+		}
+	}
+
+	return false;
+}
+
 static void
 fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
 			const struct lvds_dvo_timing *dvo_timing)
-- 
2.6.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-03-31 10:41 [PATCH 1/2] " Shubhangi Shrivastava
@ 2016-03-31 10:41 ` Shubhangi Shrivastava
  2016-04-06 11:26   ` Jani Nikula
  0 siblings, 1 reply; 24+ messages in thread
From: Shubhangi Shrivastava @ 2016-03-31 10:41 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shubhangi Shrivastava

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.

v2: Separated VBT parsing from the rest of the logic. (Jani)

v3: Moved setting invert bit logic to bxt_hpd_irq_setup()
    and changed its logic to avoid looping twice. (Ville)

v4: Changed the logic to mask out the bits first and then
    set them to remove need of temporary variable. (Ville)

v5: Moved defines to existing set of defines for the register
    and added required breaks. (Ville)

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>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  2 ++
 drivers/gpu/drm/i915/i915_irq.c   | 20 ++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h   |  6 ++++++
 drivers/gpu/drm/i915/intel_bios.c | 38 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 66 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 08b88c0..4e8d78a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3378,6 +3378,8 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
 bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
 bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
 bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
+bool intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
+				     enum port port);
 
 /* intel_opregion.c */
 #ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a55a7cc..8fbec3e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3504,6 +3504,26 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
 	hotplug = I915_READ(PCH_PORT_HOTPLUG);
 	hotplug |= PORTC_HOTPLUG_ENABLE | PORTB_HOTPLUG_ENABLE |
 		PORTA_HOTPLUG_ENABLE;
+
+	DRM_DEBUG_KMS("Invert bit setting: hp_ctl:%x hp_port:%x\n",
+			hotplug, enabled_irqs);
+	hotplug &= ~BXT_DDI_HPD_INVERT_MASK;
+
+	/*
+	 * For BXT invert bit has to be set based on AOB design
+	 * for HPD detection logic, update it based on VBT fields.
+	 */
+
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIA) &&
+	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_A))
+		hotplug |= BXT_DDIA_HPD_INVERT;
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIB) &&
+	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_B))
+		hotplug |= BXT_DDIB_HPD_INVERT;
+	if ((enabled_irqs & BXT_DE_PORT_HP_DDIC) &&
+	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_C))
+		hotplug |= BXT_DDIC_HPD_INVERT;
+
 	I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f3ba43c..73a806c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6185,6 +6185,7 @@ enum skl_disp_power_wells {
 /* digital port hotplug */
 #define PCH_PORT_HOTPLUG		_MMIO(0xc4030)	/* SHOTPLUG_CTL */
 #define  PORTA_HOTPLUG_ENABLE		(1 << 28) /* LPT:LP+ & BXT */
+#define  BXT_DDIA_HPD_INVERT            (1 << 27)
 #define  PORTA_HOTPLUG_STATUS_MASK	(3 << 24) /* SPT+ & BXT */
 #define  PORTA_HOTPLUG_NO_DETECT	(0 << 24) /* SPT+ & BXT */
 #define  PORTA_HOTPLUG_SHORT_DETECT	(1 << 24) /* SPT+ & BXT */
@@ -6200,6 +6201,7 @@ enum skl_disp_power_wells {
 #define  PORTD_HOTPLUG_SHORT_DETECT	(1 << 16)
 #define  PORTD_HOTPLUG_LONG_DETECT	(2 << 16)
 #define  PORTC_HOTPLUG_ENABLE		(1 << 12)
+#define  BXT_DDIC_HPD_INVERT            (1 << 11)
 #define  PORTC_PULSE_DURATION_2ms	(0 << 10) /* pre-LPT */
 #define  PORTC_PULSE_DURATION_4_5ms	(1 << 10) /* pre-LPT */
 #define  PORTC_PULSE_DURATION_6ms	(2 << 10) /* pre-LPT */
@@ -6210,6 +6212,7 @@ enum skl_disp_power_wells {
 #define  PORTC_HOTPLUG_SHORT_DETECT	(1 << 8)
 #define  PORTC_HOTPLUG_LONG_DETECT	(2 << 8)
 #define  PORTB_HOTPLUG_ENABLE		(1 << 4)
+#define  BXT_DDIB_HPD_INVERT            (1 << 3)
 #define  PORTB_PULSE_DURATION_2ms	(0 << 2) /* pre-LPT */
 #define  PORTB_PULSE_DURATION_4_5ms	(1 << 2) /* pre-LPT */
 #define  PORTB_PULSE_DURATION_6ms	(2 << 2) /* pre-LPT */
@@ -6219,6 +6222,9 @@ enum skl_disp_power_wells {
 #define  PORTB_HOTPLUG_NO_DETECT	(0 << 0)
 #define  PORTB_HOTPLUG_SHORT_DETECT	(1 << 0)
 #define  PORTB_HOTPLUG_LONG_DETECT	(2 << 0)
+#define  BXT_DDI_HPD_INVERT_MASK	(BXT_DDIA_HPD_INVERT | \
+					BXT_DDIB_HPD_INVERT | \
+					BXT_DDIC_HPD_INVERT)
 
 #define PCH_PORT_HOTPLUG2		_MMIO(0xc403C)	/* SHOTPLUG_CTL2 SPT+ */
 #define  PORTE_HOTPLUG_ENABLE		(1 << 4)
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index e2f636c..5e78406 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -107,6 +107,44 @@ find_section(const void *_bdb, int section_id)
 	return NULL;
 }
 
+bool
+intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
+				enum port port)
+{
+	int i;
+
+	if (WARN_ON_ONCE(!IS_BROXTON(dev_priv)))
+		return false;
+
+	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+
+		if (!dev_priv->vbt.child_dev[i].common.hpd_invert)
+			continue;
+
+		switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
+		case DVO_PORT_DPA:
+		case DVO_PORT_HDMIA:
+			if (port == PORT_A)
+				return true;
+			break;
+		case DVO_PORT_DPB:
+		case DVO_PORT_HDMIB:
+			if (port == PORT_B)
+				return true;
+			break;
+		case DVO_PORT_DPC:
+		case DVO_PORT_HDMIC:
+			if (port == PORT_C)
+				return true;
+			break;
+		default:
+			break;
+		}
+	}
+
+	return false;
+}
+
 static void
 fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
 			const struct lvds_dvo_timing *dvo_timing)
-- 
2.6.1

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT
  2016-03-31 10:41 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
@ 2016-04-06 11:26   ` Jani Nikula
  0 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2016-04-06 11:26 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shubhangi Shrivastava, Syrjala, Ville

On Thu, 31 Mar 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.
>
> v2: Separated VBT parsing from the rest of the logic. (Jani)
>
> v3: Moved setting invert bit logic to bxt_hpd_irq_setup()
>     and changed its logic to avoid looping twice. (Ville)
>
> v4: Changed the logic to mask out the bits first and then
>     set them to remove need of temporary variable. (Ville)
>
> v5: Moved defines to existing set of defines for the register
>     and added required breaks. (Ville)
>
> 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>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Both pushed to drm-intel-next-queued, thanks for the patch and review.

Although I got a bugzilla reference [1], I managed to not add it to the
commit message. :(


BR,
Jani.

[1] https://bugs.freedesktop.org/show_bug.cgi?id=93915


> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  2 ++
>  drivers/gpu/drm/i915/i915_irq.c   | 20 ++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h   |  6 ++++++
>  drivers/gpu/drm/i915/intel_bios.c | 38 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 66 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 08b88c0..4e8d78a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3378,6 +3378,8 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
>  bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
>  bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
>  bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
> +bool intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
> +				     enum port port);
>  
>  /* intel_opregion.c */
>  #ifdef CONFIG_ACPI
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index a55a7cc..8fbec3e 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3504,6 +3504,26 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
>  	hotplug = I915_READ(PCH_PORT_HOTPLUG);
>  	hotplug |= PORTC_HOTPLUG_ENABLE | PORTB_HOTPLUG_ENABLE |
>  		PORTA_HOTPLUG_ENABLE;
> +
> +	DRM_DEBUG_KMS("Invert bit setting: hp_ctl:%x hp_port:%x\n",
> +			hotplug, enabled_irqs);
> +	hotplug &= ~BXT_DDI_HPD_INVERT_MASK;
> +
> +	/*
> +	 * For BXT invert bit has to be set based on AOB design
> +	 * for HPD detection logic, update it based on VBT fields.
> +	 */
> +
> +	if ((enabled_irqs & BXT_DE_PORT_HP_DDIA) &&
> +	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_A))
> +		hotplug |= BXT_DDIA_HPD_INVERT;
> +	if ((enabled_irqs & BXT_DE_PORT_HP_DDIB) &&
> +	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_B))
> +		hotplug |= BXT_DDIB_HPD_INVERT;
> +	if ((enabled_irqs & BXT_DE_PORT_HP_DDIC) &&
> +	     intel_bios_is_port_hpd_inverted(dev_priv, PORT_C))
> +		hotplug |= BXT_DDIC_HPD_INVERT;
> +
>  	I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f3ba43c..73a806c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6185,6 +6185,7 @@ enum skl_disp_power_wells {
>  /* digital port hotplug */
>  #define PCH_PORT_HOTPLUG		_MMIO(0xc4030)	/* SHOTPLUG_CTL */
>  #define  PORTA_HOTPLUG_ENABLE		(1 << 28) /* LPT:LP+ & BXT */
> +#define  BXT_DDIA_HPD_INVERT            (1 << 27)
>  #define  PORTA_HOTPLUG_STATUS_MASK	(3 << 24) /* SPT+ & BXT */
>  #define  PORTA_HOTPLUG_NO_DETECT	(0 << 24) /* SPT+ & BXT */
>  #define  PORTA_HOTPLUG_SHORT_DETECT	(1 << 24) /* SPT+ & BXT */
> @@ -6200,6 +6201,7 @@ enum skl_disp_power_wells {
>  #define  PORTD_HOTPLUG_SHORT_DETECT	(1 << 16)
>  #define  PORTD_HOTPLUG_LONG_DETECT	(2 << 16)
>  #define  PORTC_HOTPLUG_ENABLE		(1 << 12)
> +#define  BXT_DDIC_HPD_INVERT            (1 << 11)
>  #define  PORTC_PULSE_DURATION_2ms	(0 << 10) /* pre-LPT */
>  #define  PORTC_PULSE_DURATION_4_5ms	(1 << 10) /* pre-LPT */
>  #define  PORTC_PULSE_DURATION_6ms	(2 << 10) /* pre-LPT */
> @@ -6210,6 +6212,7 @@ enum skl_disp_power_wells {
>  #define  PORTC_HOTPLUG_SHORT_DETECT	(1 << 8)
>  #define  PORTC_HOTPLUG_LONG_DETECT	(2 << 8)
>  #define  PORTB_HOTPLUG_ENABLE		(1 << 4)
> +#define  BXT_DDIB_HPD_INVERT            (1 << 3)
>  #define  PORTB_PULSE_DURATION_2ms	(0 << 2) /* pre-LPT */
>  #define  PORTB_PULSE_DURATION_4_5ms	(1 << 2) /* pre-LPT */
>  #define  PORTB_PULSE_DURATION_6ms	(2 << 2) /* pre-LPT */
> @@ -6219,6 +6222,9 @@ enum skl_disp_power_wells {
>  #define  PORTB_HOTPLUG_NO_DETECT	(0 << 0)
>  #define  PORTB_HOTPLUG_SHORT_DETECT	(1 << 0)
>  #define  PORTB_HOTPLUG_LONG_DETECT	(2 << 0)
> +#define  BXT_DDI_HPD_INVERT_MASK	(BXT_DDIA_HPD_INVERT | \
> +					BXT_DDIB_HPD_INVERT | \
> +					BXT_DDIC_HPD_INVERT)
>  
>  #define PCH_PORT_HOTPLUG2		_MMIO(0xc403C)	/* SHOTPLUG_CTL2 SPT+ */
>  #define  PORTE_HOTPLUG_ENABLE		(1 << 4)
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index e2f636c..5e78406 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -107,6 +107,44 @@ find_section(const void *_bdb, int section_id)
>  	return NULL;
>  }
>  
> +bool
> +intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
> +				enum port port)
> +{
> +	int i;
> +
> +	if (WARN_ON_ONCE(!IS_BROXTON(dev_priv)))
> +		return false;
> +
> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> +
> +		if (!dev_priv->vbt.child_dev[i].common.hpd_invert)
> +			continue;
> +
> +		switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
> +		case DVO_PORT_DPA:
> +		case DVO_PORT_HDMIA:
> +			if (port == PORT_A)
> +				return true;
> +			break;
> +		case DVO_PORT_DPB:
> +		case DVO_PORT_HDMIB:
> +			if (port == PORT_B)
> +				return true;
> +			break;
> +		case DVO_PORT_DPC:
> +		case DVO_PORT_HDMIC:
> +			if (port == PORT_C)
> +				return true;
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +
> +	return false;
> +}
> +
>  static void
>  fill_detail_timing_data(struct drm_display_mode *panel_fixed_mode,
>  			const struct lvds_dvo_timing *dvo_timing)

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2016-04-06 11:26 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-24 12:10 [PATCH 1/2] drm/i915: Update VBT fields for child devices Shubhangi Shrivastava
2016-03-24 12:10 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
2016-03-24 14:39   ` Ville Syrjälä
2016-03-24 13:36 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Update VBT fields for child devices Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2016-03-31 10:41 [PATCH 1/2] " Shubhangi Shrivastava
2016-03-31 10:41 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
2016-04-06 11:26   ` Jani Nikula
2016-03-25 17:55 [PATCH 1/2] drm/i915: Update VBT fields for child devices Shubhangi Shrivastava
2016-03-25 17:55 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
2016-03-15 13:13 [PATCH 1/2] drm/i915: Update VBT fields for child devices Shubhangi Shrivastava
2016-03-15 13:13 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
2016-03-15 14:51   ` Ville Syrjälä
2016-03-11 13:43 [PATCH 1/2] drm/i915: Update VBT fields for child devices Shubhangi Shrivastava
2016-03-11 13:43 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
2016-03-11 14:17   ` Jani Nikula
2016-03-11 12:53 [PATCH 1/2] drm/i915: Update VBT fields for child devices Shubhangi Shrivastava
2016-03-11 12:53 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
2016-03-11 13:20   ` kbuild test robot
2016-02-25  9:57 [PATCH 1/2] drm/i915: Update VBT fields for child devices Shubhangi Shrivastava
2016-02-25  9:57 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
2016-02-25 13:57   ` Ville Syrjälä
2016-02-12 13:09 [PATCH 1/2] drm/i915: Update VBT fields for child devices Shubhangi Shrivastava
2016-02-12 13:09 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
2016-02-12 13:37   ` kbuild test robot
2016-02-12 14:08   ` Ville Syrjälä
2016-02-16  1:59     ` Thulasimani, Sivakumar
2016-02-16  9:41       ` Ville Syrjälä
2016-02-04  8:58 [PATCH 1/2] drm/i915: Update VBT fields for child devices Shubhangi Shrivastava
2016-02-04  8:58 ` [PATCH 2/2] drm/i915: Set invert bit for hpd based on VBT Shubhangi Shrivastava
2016-02-04 12:29   ` Jani Nikula
2016-02-05  1:06     ` Thulasimani, Sivakumar
2016-02-05  8:27       ` Jani Nikula

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).