diff mbox

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

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

Commit Message

Shubhangi Shrivastava Feb. 25, 2016, 9:57 a.m. UTC
This patch sets the invert bit for hpd detection for each port
based on VBT configuration. Since each AOB can be designed to
depend on invert bit or not, it is expected if an AOB requires
invert bit, the user will set respective bit in VBT.

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

Comments

Ville Syrjala Feb. 25, 2016, 1:57 p.m. UTC | #1
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
diff mbox

Patch

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)