diff mbox

[v3,08/22] drm/i915: Create a new category of display WAs

Message ID 1507928056-6966-9-git-send-email-oscar.mateo@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

oscar.mateo@intel.com Oct. 13, 2017, 8:54 p.m. UTC 
Display workarounds do not need to be re-applied on a GPU reset
(this is, in Ville's words: "at the very least wasted effort [...]
and could even be actively harmful in case we end up clobbering
something the current display configuration depends on"). Therefore,
they have to be applied in a different place that GT ones so they
deserve their own category.

Actually populating this is left for future patches: we have to
start moving WAs from init_clock_gating into either GT or
Display functions, and this requires a good deal of careful code
reviewing.

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c          |   2 +-
 drivers/gpu/drm/i915/i915_drv.h          |   3 +
 drivers/gpu/drm/i915/intel_pm.c          |   2 +
 drivers/gpu/drm/i915/intel_workarounds.c | 154 +++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_workarounds.h |   3 +-
 5 files changed, 155 insertions(+), 9 deletions(-)

Comments

Chris Wilson Oct. 17, 2017, 12:42 p.m. UTC | #1 
Quoting Oscar Mateo (2017-10-13 21:54:02)
> Display workarounds do not need to be re-applied on a GPU reset
> (this is, in Ville's words: "at the very least wasted effort [...]
> and could even be actively harmful in case we end up clobbering
> something the current display configuration depends on"). Therefore,
> they have to be applied in a different place that GT ones so they
> deserve their own category.
> 
> Actually populating this is left for future patches: we have to
> start moving WAs from init_clock_gating into either GT or
> Display functions, and this requires a good deal of careful code
> reviewing.
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c          |   2 +-
>  drivers/gpu/drm/i915/i915_drv.h          |   3 +
>  drivers/gpu/drm/i915/intel_pm.c          |   2 +
>  drivers/gpu/drm/i915/intel_workarounds.c | 154 +++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_workarounds.h |   3 +-
>  5 files changed, 155 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 27993eb..6e9a0da 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -887,7 +887,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
>         BUG_ON(device_info->gen > sizeof(device_info->gen_mask) * BITS_PER_BYTE);
>         device_info->gen_mask = BIT(device_info->gen - 1);
>  
> -       ret = intel_gt_workarounds_init(dev_priv);
> +       ret = intel_mmio_workarounds_init(dev_priv);

Momentary panic over. I'm going to suggest those that we carry the
init_early nomenclature over. It's very important that we stress that we
are *only* building the lists at this time and not touching the hw.

With that change backported to intel_gt_workarounds_init_early()
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 27993eb..6e9a0da 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -887,7 +887,7 @@  static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 	BUG_ON(device_info->gen > sizeof(device_info->gen_mask) * BITS_PER_BYTE);
 	device_info->gen_mask = BIT(device_info->gen - 1);
 
-	ret = intel_gt_workarounds_init(dev_priv);
+	ret = intel_mmio_workarounds_init(dev_priv);
 	if (ret < 0)
 		return ret;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b8ce7c1..1810e26 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1970,6 +1970,9 @@  struct i915_workarounds {
 	struct i915_wa_reg gt_wa_reg[I915_MAX_MMIO_WA_REGS];
 	u32 gt_wa_count;
 
+	struct i915_wa_reg display_wa_reg[I915_MAX_MMIO_WA_REGS];
+	u32 display_wa_count;
+
 	struct i915_wa_reg whitelist_wa_reg[I915_NUM_ENGINES][RING_MAX_NONPRIV_SLOTS];
 	u32 whitelist_wa_count[I915_NUM_ENGINES];
 };
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6c2c8d8..e4b3904 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -29,6 +29,7 @@ 
 #include <drm/drm_plane_helper.h>
 #include "i915_drv.h"
 #include "intel_drv.h"
+#include "intel_workarounds.h"
 #include "../../../platform/x86/intel_ips.h"
 #include <linux/module.h>
 #include <drm/drm_atomic_helper.h>
@@ -9003,6 +9004,7 @@  static void i830_init_clock_gating(struct drm_i915_private *dev_priv)
 void intel_init_clock_gating(struct drm_i915_private *dev_priv)
 {
 	dev_priv->display.init_clock_gating(dev_priv);
+	intel_display_workarounds_apply(dev_priv);
 }
 
 void intel_suspend_hw(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index 3a7f404..d1cafe4 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -513,6 +513,18 @@  int intel_ctx_workarounds_emit(struct drm_i915_gem_request *req)
 	return 0;
 }
 
+static void mmio_workarounds_apply(struct drm_i915_private *dev_priv,
+				   struct i915_wa_reg *wa,
+				   u32 count)
+{
+	int i;
+
+	for (i = 0; i < count; i++) {
+		u32 value = I915_READ(wa[i].addr);
+		I915_WRITE(wa[i].addr, (value & ~wa[i].mask) | wa[i].value);
+	}
+}
+
 static int gt_wa_add(struct drm_i915_private *dev_priv,
 		     i915_reg_t addr,
 		     const u32 mask, const u32 val)
@@ -728,7 +740,7 @@  static int cnl_gt_workarounds_init(struct drm_i915_private *dev_priv)
 	return 0;
 }
 
-int intel_gt_workarounds_init(struct drm_i915_private *dev_priv)
+static int gt_workarounds_init(struct drm_i915_private *dev_priv)
 {
 	int err;
 
@@ -767,15 +779,143 @@  int intel_gt_workarounds_init(struct drm_i915_private *dev_priv)
 void intel_gt_workarounds_apply(struct drm_i915_private *dev_priv)
 {
 	struct i915_workarounds *w = &dev_priv->workarounds;
-	int i;
 
-	for (i = 0; i < w->gt_wa_count; i++) {
-		i915_reg_t addr = w->gt_wa_reg[i].addr;
-		u32 value = w->gt_wa_reg[i].value;
-		u32 mask = w->gt_wa_reg[i].mask;
+	mmio_workarounds_apply(dev_priv, w->gt_wa_reg, w->gt_wa_count);
+}
+
+static int display_wa_add(struct drm_i915_private *dev_priv,
+			  i915_reg_t addr,
+			  const u32 mask, const u32 val)
+{
+	const u32 idx = dev_priv->workarounds.display_wa_count;
+
+	if (WARN_ON(idx >= I915_MAX_MMIO_WA_REGS))
+		return -ENOSPC;
+
+	dev_priv->workarounds.display_wa_reg[idx].addr = addr;
+	dev_priv->workarounds.display_wa_reg[idx].value = val;
+	dev_priv->workarounds.display_wa_reg[idx].mask = mask;
+
+	dev_priv->workarounds.display_wa_count++;
+
+	return 0;
+}
+
+#define DISPLAY_WA_REG(addr, mask, val) do { \
+		const int r = display_wa_add(dev_priv, (addr), (mask), (val)); \
+		if (r) \
+			return r; \
+	} while (0)
+
+#define DISPLAY_WA_SET_BIT(addr, mask) \
+	DISPLAY_WA_REG(addr, (mask), (mask))
+
+#define DISPLAY_WA_SET_BIT_MASKED(addr, mask) \
+	DISPLAY_WA_REG(addr, (mask), _MASKED_BIT_ENABLE(mask))
+
+#define DISPLAY_WA_CLR_BIT(addr, mask) \
+	DISPLAY_WA_REG(addr, (mask), 0)
+
+#define DISPLAY_WA_SET_FIELD(addr, mask, value) \
+	DISPLAY_WA_REG(addr, (mask), (value))
+
+static int bdw_display_workarounds_init(struct drm_i915_private *dev_priv)
+{
+	return 0;
+}
+
+static int chv_display_workarounds_init(struct drm_i915_private *dev_priv)
+{
+	return 0;
+}
+
+static int skl_display_workarounds_init(struct drm_i915_private *dev_priv)
+{
+	return 0;
+}
+
+static int bxt_display_workarounds_init(struct drm_i915_private *dev_priv)
+{
+	return 0;
+}
+
+static int kbl_display_workarounds_init(struct drm_i915_private *dev_priv)
+{
+	return 0;
+}
+
+static int glk_display_workarounds_init(struct drm_i915_private *dev_priv)
+{
+	return 0;
+}
+
+static int cfl_display_workarounds_init(struct drm_i915_private *dev_priv)
+{
+	return 0;
+}
+
+static int cnl_display_workarounds_init(struct drm_i915_private *dev_priv)
+{
+	return 0;
+}
+
+static int display_workarounds_init(struct drm_i915_private *dev_priv)
+{
+	int err;
+
+	DISPLAY_WA_SET_BIT(_MMIO(0), 0); /* shut up gcc temporarily */
+	dev_priv->workarounds.display_wa_count = 0;
 
-		I915_WRITE(addr, (I915_READ(addr) & ~mask) | value);
+	if (INTEL_GEN(dev_priv) < 8)
+		err = 0;
+	else if (IS_BROADWELL(dev_priv))
+		err = bdw_display_workarounds_init(dev_priv);
+	else if (IS_CHERRYVIEW(dev_priv))
+		err = chv_display_workarounds_init(dev_priv);
+	else if (IS_SKYLAKE(dev_priv))
+		err = skl_display_workarounds_init(dev_priv);
+	else if (IS_BROXTON(dev_priv))
+		err = bxt_display_workarounds_init(dev_priv);
+	else if (IS_KABYLAKE(dev_priv))
+		err = kbl_display_workarounds_init(dev_priv);
+	else if (IS_GEMINILAKE(dev_priv))
+		err = glk_display_workarounds_init(dev_priv);
+	else if (IS_COFFEELAKE(dev_priv))
+		err = cfl_display_workarounds_init(dev_priv);
+	else if (IS_CANNONLAKE(dev_priv))
+		err = cnl_display_workarounds_init(dev_priv);
+	else {
+		MISSING_CASE(INTEL_GEN(dev_priv));
+		err = 0;
 	}
+	if (err)
+		return err;
+
+	DRM_DEBUG_DRIVER("Number of display w/a: %d\n",
+			 dev_priv->workarounds.display_wa_count);
+	return 0;
+}
+
+void intel_display_workarounds_apply(struct drm_i915_private *dev_priv)
+{
+	struct i915_workarounds *w = &dev_priv->workarounds;
+
+	mmio_workarounds_apply(dev_priv, w->display_wa_reg, w->display_wa_count);
+}
+
+int intel_mmio_workarounds_init(struct drm_i915_private *dev_priv)
+{
+	int ret;
+
+	ret = gt_workarounds_init(dev_priv);
+	if (ret)
+		return ret;
+
+	ret = display_workarounds_init(dev_priv);
+	if (ret)
+		return ret;
+
+	return 0;
 }
 
 static int whitelist_wa_add(struct intel_engine_cs *engine,
diff --git a/drivers/gpu/drm/i915/intel_workarounds.h b/drivers/gpu/drm/i915/intel_workarounds.h
index fc3b12d..44d3941 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.h
+++ b/drivers/gpu/drm/i915/intel_workarounds.h
@@ -28,8 +28,9 @@ 
 int intel_ctx_workarounds_init(struct drm_i915_private *dev_priv);
 int intel_ctx_workarounds_emit(struct drm_i915_gem_request *req);
 
-int intel_gt_workarounds_init(struct drm_i915_private *dev_priv);
+int intel_mmio_workarounds_init(struct drm_i915_private *dev_priv);
 void intel_gt_workarounds_apply(struct drm_i915_private *dev_priv);
+void intel_display_workarounds_apply(struct drm_i915_private *dev_priv);
 
 int intel_whitelist_workarounds_init(struct intel_engine_cs *engine);
 void intel_whitelist_workarounds_apply(struct intel_engine_cs *engine);