[4/6] drm/i915: Skip CS verification of L3 bank registers
diff mbox series

Message ID 20190717180624.20354-5-tvrtko.ursulin@linux.intel.com
State New
Headers show
Series
  • MCR fixes and more
Related show

Commit Message

Tvrtko Ursulin July 17, 2019, 6:06 p.m. UTC
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Access to 0xb100 - 0xb3ff mmio range is controlled by the MCR selector
which only affects CPU MMIO. Therefore these registers cannot be realiably
read with MI_SRM from the command streamer so skip their verification.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 38 ++++++++++++++++++---
 1 file changed, 33 insertions(+), 5 deletions(-)

Comments

Chris Wilson July 17, 2019, 7:37 p.m. UTC | #1
Quoting Tvrtko Ursulin (2019-07-17 19:06:22)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Access to 0xb100 - 0xb3ff mmio range is controlled by the MCR selector
> which only affects CPU MMIO. Therefore these registers cannot be realiably
> read with MI_SRM from the command streamer so skip their verification.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 38 ++++++++++++++++++---
>  1 file changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index c2325b7ecf8d..619d42a2b81b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1436,26 +1436,50 @@ create_scratch(struct i915_address_space *vm, int count)
>         return ERR_PTR(err);
>  }
>  
> +static bool mcr_range(struct drm_i915_private *i915, u32 offset)
> +{
> +       /*
> +        * Registers in this range are affected by the MCR selector
> +        * which only controls CPU initiated MMIO. Routing does not
> +        * work for CS access so we cannot verify them on this path.
> +        */
> +       if (INTEL_GEN(i915) >= 8 && (offset >= 0xb100 && offset <= 0xb3ff))
> +               return true;

Bonus (). I was thinking maybe
	if (INTEL_GEN() < 8)
		return false;
	return offset >= 0xb100 && offset <= 0xb3ff;
to remove the apparent need. But structuring the checks with a
per-gen if-ladder will probably be easier to extend should the need
arise.

> +       return false;
> +}
> +
>  static int
>  wa_list_srm(struct i915_request *rq,
>             const struct i915_wa_list *wal,
>             struct i915_vma *vma)
>  {
> +       struct drm_i915_private *i915 = rq->i915;
> +       unsigned int i, count = 0;
>         const struct i915_wa *wa;
> -       unsigned int i;
>         u32 srm, *cs;
>  
>         srm = MI_STORE_REGISTER_MEM | MI_SRM_LRM_GLOBAL_GTT;
> -       if (INTEL_GEN(rq->i915) >= 8)
> +       if (INTEL_GEN(i915) >= 8)
>                 srm++;
>  
> -       cs = intel_ring_begin(rq, 4 * wal->count);
> +       for (i = 0, wa = wal->list; i < wal->count; i++, wa++) {
> +               if (!mcr_range(i915, i915_mmio_reg_offset(wa->reg)))
> +                       count++;
> +       }
> +
> +       cs = intel_ring_begin(rq, 4 * count);
>         if (IS_ERR(cs))
>                 return PTR_ERR(cs);
>  
>         for (i = 0, wa = wal->list; i < wal->count; i++, wa++) {
> +               u32 offset = i915_mmio_reg_offset(wa->reg);
> +
> +               if (mcr_range(i915, offset))
> +                       continue;
> +

I would have just done the reg read and ignored the result, rather than
add another loop to fixup the count.

>                 *cs++ = srm;
> -               *cs++ = i915_mmio_reg_offset(wa->reg);
> +               *cs++ = offset;
>                 *cs++ = i915_ggtt_offset(vma) + sizeof(u32) * i;
>                 *cs++ = 0;
>         }
> @@ -1505,9 +1529,13 @@ static int engine_wa_list_verify(struct intel_context *ce,
>         }
>  
>         err = 0;
> -       for (i = 0, wa = wal->list; i < wal->count; i++, wa++)
> +       for (i = 0, wa = wal->list; i < wal->count; i++, wa++) {
> +               if (mcr_range(rq->i915, i915_mmio_reg_offset(wa->reg)))
> +                       continue;
> +
>                 if (!wa_verify(wa, results[i], wal->name, from))
>                         err = -ENXIO;
> +       }

Looks fine though
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index c2325b7ecf8d..619d42a2b81b 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1436,26 +1436,50 @@  create_scratch(struct i915_address_space *vm, int count)
 	return ERR_PTR(err);
 }
 
+static bool mcr_range(struct drm_i915_private *i915, u32 offset)
+{
+	/*
+	 * Registers in this range are affected by the MCR selector
+	 * which only controls CPU initiated MMIO. Routing does not
+	 * work for CS access so we cannot verify them on this path.
+	 */
+	if (INTEL_GEN(i915) >= 8 && (offset >= 0xb100 && offset <= 0xb3ff))
+		return true;
+
+	return false;
+}
+
 static int
 wa_list_srm(struct i915_request *rq,
 	    const struct i915_wa_list *wal,
 	    struct i915_vma *vma)
 {
+	struct drm_i915_private *i915 = rq->i915;
+	unsigned int i, count = 0;
 	const struct i915_wa *wa;
-	unsigned int i;
 	u32 srm, *cs;
 
 	srm = MI_STORE_REGISTER_MEM | MI_SRM_LRM_GLOBAL_GTT;
-	if (INTEL_GEN(rq->i915) >= 8)
+	if (INTEL_GEN(i915) >= 8)
 		srm++;
 
-	cs = intel_ring_begin(rq, 4 * wal->count);
+	for (i = 0, wa = wal->list; i < wal->count; i++, wa++) {
+		if (!mcr_range(i915, i915_mmio_reg_offset(wa->reg)))
+			count++;
+	}
+
+	cs = intel_ring_begin(rq, 4 * count);
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
 	for (i = 0, wa = wal->list; i < wal->count; i++, wa++) {
+		u32 offset = i915_mmio_reg_offset(wa->reg);
+
+		if (mcr_range(i915, offset))
+			continue;
+
 		*cs++ = srm;
-		*cs++ = i915_mmio_reg_offset(wa->reg);
+		*cs++ = offset;
 		*cs++ = i915_ggtt_offset(vma) + sizeof(u32) * i;
 		*cs++ = 0;
 	}
@@ -1505,9 +1529,13 @@  static int engine_wa_list_verify(struct intel_context *ce,
 	}
 
 	err = 0;
-	for (i = 0, wa = wal->list; i < wal->count; i++, wa++)
+	for (i = 0, wa = wal->list; i < wal->count; i++, wa++) {
+		if (mcr_range(rq->i915, i915_mmio_reg_offset(wa->reg)))
+			continue;
+
 		if (!wa_verify(wa, results[i], wal->name, from))
 			err = -ENXIO;
+	}
 
 	i915_gem_object_unpin_map(vma->obj);