diff mbox series

drm/i915/selftests: Check a few more fixed locations within the context image

Message ID 20191027150313.10802-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series drm/i915/selftests: Check a few more fixed locations within the context image | expand

Commit Message

Chris Wilson Oct. 27, 2019, 3:03 p.m. UTC
As we use hard coded offsets for a few locations within the context
image, include those in the selftests to assert that they are valid.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_lrc.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Joonas Lahtinen Oct. 28, 2019, 12:13 p.m. UTC | #1
Quoting Chris Wilson (2019-10-27 17:03:13)
> As we use hard coded offsets for a few locations within the context
> image, include those in the selftests to assert that they are valid.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>

<SNIP>

> +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> @@ -3194,6 +3194,26 @@ static int live_lrc_fixed(void *arg)
>                         u32 offset;
>                         const char *name;
>                 } tbl[] = {
> +                       {
> +                               i915_mmio_reg_offset(RING_START(engine->mmio_base)),
> +                               CTX_RING_BUFFER_START - 1,
> +                               "RING_START"
> +                       },
> +                       {
> +                               i915_mmio_reg_offset(RING_CTL(engine->mmio_base)),
> +                               CTX_RING_BUFFER_CONTROL - 1,
> +                               "RING_CTL"
> +                       },
> +                       {
> +                               i915_mmio_reg_offset(RING_HEAD(engine->mmio_base)),
> +                               CTX_RING_HEAD - 1,
> +                               "RING_HEAD"
> +                       },
> +                       {
> +                               i915_mmio_reg_offset(RING_TAIL(engine->mmio_base)),
> +                               CTX_RING_TAIL - 1,
> +                               "RING_TAIL"
> +                       },

Quite repetitive. If you are not looking to add dissimilar patterns,
could use a #define, but up to you.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
Chris Wilson Oct. 28, 2019, 12:16 p.m. UTC | #2
Quoting Joonas Lahtinen (2019-10-28 12:13:05)
> Quoting Chris Wilson (2019-10-27 17:03:13)
> > As we use hard coded offsets for a few locations within the context
> > image, include those in the selftests to assert that they are valid.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> 
> <SNIP>
> 
> > +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> > @@ -3194,6 +3194,26 @@ static int live_lrc_fixed(void *arg)
> >                         u32 offset;
> >                         const char *name;
> >                 } tbl[] = {
> > +                       {
> > +                               i915_mmio_reg_offset(RING_START(engine->mmio_base)),
> > +                               CTX_RING_BUFFER_START - 1,
> > +                               "RING_START"
> > +                       },
> > +                       {
> > +                               i915_mmio_reg_offset(RING_CTL(engine->mmio_base)),
> > +                               CTX_RING_BUFFER_CONTROL - 1,
> > +                               "RING_CTL"
> > +                       },
> > +                       {
> > +                               i915_mmio_reg_offset(RING_HEAD(engine->mmio_base)),
> > +                               CTX_RING_HEAD - 1,
> > +                               "RING_HEAD"
> > +                       },
> > +                       {
> > +                               i915_mmio_reg_offset(RING_TAIL(engine->mmio_base)),
> > +                               CTX_RING_TAIL - 1,
> > +                               "RING_TAIL"
> > +                       },
> 
> Quite repetitive. If you are not looking to add dissimilar patterns,
> could use a #define, but up to you.

The tables spares us even more repetition :)
I didn't look too hard at reducing the repetitiveness as I was expecting
a few more registers to break the mould. We shall see what the future
brings.

There's a few more registers we use hardcoded locations for, mostly in
perf. Hmm, I should probably make that a local test for selftest_perf.c
so that the logic and test are close together.
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index ba7fc4397bd9..5e90fc127be6 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -3194,6 +3194,26 @@  static int live_lrc_fixed(void *arg)
 			u32 offset;
 			const char *name;
 		} tbl[] = {
+			{
+				i915_mmio_reg_offset(RING_START(engine->mmio_base)),
+				CTX_RING_BUFFER_START - 1,
+				"RING_START"
+			},
+			{
+				i915_mmio_reg_offset(RING_CTL(engine->mmio_base)),
+				CTX_RING_BUFFER_CONTROL - 1,
+				"RING_CTL"
+			},
+			{
+				i915_mmio_reg_offset(RING_HEAD(engine->mmio_base)),
+				CTX_RING_HEAD - 1,
+				"RING_HEAD"
+			},
+			{
+				i915_mmio_reg_offset(RING_TAIL(engine->mmio_base)),
+				CTX_RING_TAIL - 1,
+				"RING_TAIL"
+			},
 			{
 				i915_mmio_reg_offset(RING_MI_MODE(engine->mmio_base)),
 				lrc_ring_mi_mode(engine),