[PATCH] drm/i915: Fix context size calculation on SNB/IVB/VLV

[PATCH] drm/i915: Fix context size calculation on SNB/IVB/VLV

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

All the different context sizes reported in the CXT_SIZE register
aren't meant to be simply added together.

While BSpec is somewhat unclear on the topic of the actual context
size, empirical tests have now revealed the truth. So let's add a
big fat comment to remind people how it all works.

As a result of correctly interpreting CXT_SIZE, the IVB context
size is reduced from three pages to two, while SNB context size
remains at two pages.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 53d0e70..ee0660d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1707,15 +1707,26 @@
  */
 #define CCID			0x2180
 #define   CCID_EN		(1<<0)
+/*
+ * Notes on SNB/IVB/VLV context size:
+ * - Power context is saved elsewhere (LLC or stolen)
+ * - Ring/execlist context is saved on SNB, not on IVB
+ * - Extended context size already includes render context size
+ * - We always need to follow the extended context size.
+ *   SNB BSpec has comments indicating that we should use the
+ *   render context size instead if execlists are disabled, but
+ *   based on empirical testing that's just nonsense.
+ * - Pipelined/VF state is saved on SNB/IVB respectively
+ * - GT1 size just indicates how much of render context
+ *   doesn't need saving on GT1
+ */
 #define CXT_SIZE		0x21a0
 #define GEN6_CXT_POWER_SIZE(cxt_reg)	((cxt_reg >> 24) & 0x3f)
 #define GEN6_CXT_RING_SIZE(cxt_reg)	((cxt_reg >> 18) & 0x3f)
 #define GEN6_CXT_RENDER_SIZE(cxt_reg)	((cxt_reg >> 12) & 0x3f)
 #define GEN6_CXT_EXTENDED_SIZE(cxt_reg)	((cxt_reg >> 6) & 0x3f)
 #define GEN6_CXT_PIPELINE_SIZE(cxt_reg)	((cxt_reg >> 0) & 0x3f)
-#define GEN6_CXT_TOTAL_SIZE(cxt_reg)	(GEN6_CXT_POWER_SIZE(cxt_reg) + \
-					GEN6_CXT_RING_SIZE(cxt_reg) + \
-					GEN6_CXT_RENDER_SIZE(cxt_reg) + \
+#define GEN6_CXT_TOTAL_SIZE(cxt_reg)	(GEN6_CXT_RING_SIZE(cxt_reg) + \
 					GEN6_CXT_EXTENDED_SIZE(cxt_reg) + \
 					GEN6_CXT_PIPELINE_SIZE(cxt_reg))
 #define GEN7_CXT_SIZE		0x21a8
@@ -1725,11 +1736,7 @@
 #define GEN7_CXT_EXTENDED_SIZE(ctx_reg)	((ctx_reg >> 9) & 0x7f)
 #define GEN7_CXT_GT1_SIZE(ctx_reg)	((ctx_reg >> 6) & 0x7)
 #define GEN7_CXT_VFSTATE_SIZE(ctx_reg)	((ctx_reg >> 0) & 0x3f)
-#define GEN7_CXT_TOTAL_SIZE(ctx_reg)	(GEN7_CXT_POWER_SIZE(ctx_reg) + \
-					 GEN7_CXT_RING_SIZE(ctx_reg) + \
-					 GEN7_CXT_RENDER_SIZE(ctx_reg) + \
-					 GEN7_CXT_EXTENDED_SIZE(ctx_reg) + \
-					 GEN7_CXT_GT1_SIZE(ctx_reg) + \
+#define GEN7_CXT_TOTAL_SIZE(ctx_reg)	(GEN7_CXT_EXTENDED_SIZE(ctx_reg) + \
 					 GEN7_CXT_VFSTATE_SIZE(ctx_reg))
 /* Haswell does have the CXT_SIZE register however it does not appear to be
  * valid. Now, docs explain in dwords what is in the context object. The full
-- 
1.8.1.5

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

Re: [PATCH] drm/i915: Fix context size calculation on SNB/IVB/VLV

[Ben Widawsky]

On Thu, Aug 22, 2013 at 07:23:13PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> All the different context sizes reported in the CXT_SIZE register
> aren't meant to be simply added together.
> 
> While BSpec is somewhat unclear on the topic of the actual context
> size, empirical tests have now revealed the truth. So let's add a
> big fat comment to remind people how it all works.

By the way. I've done some digging. I believe (75% certain) pre-HSW,
every context save writes the entire data. So if you wanted to set some
pattern and see what HW actually overwrites, it should be doable. HSW+
though we can't do that.

[snip]
-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix context size calculation on SNB/IVB/VLV

[Ville Syrjälä]

On Thu, Aug 22, 2013 at 11:30:55AM -0700, Ben Widawsky wrote:
> On Thu, Aug 22, 2013 at 07:23:13PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > All the different context sizes reported in the CXT_SIZE register
> > aren't meant to be simply added together.
> > 
> > While BSpec is somewhat unclear on the topic of the actual context
> > size, empirical tests have now revealed the truth. So let's add a
> > big fat comment to remind people how it all works.
> 
> By the way. I've done some digging. I believe (75% certain) pre-HSW,
> every context save writes the entire data. So if you wanted to set some
> pattern and see what HW actually overwrites, it should be doable. HSW+
> though we can't do that.

That's actually what I did. I dumped the context to disk and looked for
magic numbers and checked their offsets against the context layout
information in BSpec.

Now sure how HSW works. If it just jumps over some parts of the context
that don't need saving, then we could certainly use the same method.
Just identify some magic numbers in some high offsets, make sure to use
that part of the hardware to force it to save it, and then look for
those magic numbers in the dump. That'd easily tell us which parts of
the context it never saves (eg. the execlist context).

But if it dynamically rearranges the contents of the context image
every time, then it won't work obviously.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: Fix context size calculation on SNB/IVB/VLV

[Ville Syrjälä]

On Thu, Aug 22, 2013 at 11:30:55AM -0700, Ben Widawsky wrote:
> On Thu, Aug 22, 2013 at 07:23:13PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > All the different context sizes reported in the CXT_SIZE register
> > aren't meant to be simply added together.
> > 
> > While BSpec is somewhat unclear on the topic of the actual context
> > size, empirical tests have now revealed the truth. So let's add a
> > big fat comment to remind people how it all works.
> 
> By the way. I've done some digging. I believe (75% certain) pre-HSW,
> every context save writes the entire data. So if you wanted to set some
> pattern and see what HW actually overwrites, it should be doable. HSW+
> though we can't do that.

Ah right, so you were thinking of initializing the context w/ some
poison, and then see how much is left after the HW has saved. That I
didnb't actually do. Should be reasonable trivial though, so I guess
I'll try it. The same method could also be used to identify whether
HSW skips parts or rearranges the context dynamically.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: Fix context size calculation on SNB/IVB/VLV

[Ben Widawsky]

On Thu, Aug 22, 2013 at 09:52:53PM +0300, Ville Syrjälä wrote:
> On Thu, Aug 22, 2013 at 11:30:55AM -0700, Ben Widawsky wrote:
> > On Thu, Aug 22, 2013 at 07:23:13PM +0300, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > All the different context sizes reported in the CXT_SIZE register
> > > aren't meant to be simply added together.
> > > 
> > > While BSpec is somewhat unclear on the topic of the actual context
> > > size, empirical tests have now revealed the truth. So let's add a
> > > big fat comment to remind people how it all works.
> > 
> > By the way. I've done some digging. I believe (75% certain) pre-HSW,
> > every context save writes the entire data. So if you wanted to set some
> > pattern and see what HW actually overwrites, it should be doable. HSW+
> > though we can't do that.
> 
> Ah right, so you were thinking of initializing the context w/ some
> poison, and then see how much is left after the HW has saved. That I
> didnb't actually do. Should be reasonable trivial though, so I guess
> I'll try it. The same method could also be used to identify whether
> HSW skips parts or rearranges the context dynamically.

Yes, the only problem is it's not always trivial to invoke writes of
some parts of the context (I fear).

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

Re: [PATCH] drm/i915: Fix context size calculation on SNB/IVB/VLV

[Ville Syrjälä]

On Thu, Aug 22, 2013 at 11:30:55AM -0700, Ben Widawsky wrote:
> On Thu, Aug 22, 2013 at 07:23:13PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > All the different context sizes reported in the CXT_SIZE register
> > aren't meant to be simply added together.
> > 
> > While BSpec is somewhat unclear on the topic of the actual context
> > size, empirical tests have now revealed the truth. So let's add a
> > big fat comment to remind people how it all works.
> 
> By the way. I've done some digging. I believe (75% certain) pre-HSW,
> every context save writes the entire data. So if you wanted to set some
> pattern and see what HW actually overwrites, it should be doable. HSW+
> though we can't do that.

So I did this test on SNB and IVB and it confirms my earlier findings.
This experiment also showed that while SNB leaves room for the ring
context, it doesn't actually write to it.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: Fix context size calculation on SNB/IVB/VLV

[Ben Widawsky]

On Thu, Aug 22, 2013 at 07:23:13PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> All the different context sizes reported in the CXT_SIZE register
> aren't meant to be simply added together.
> 
> While BSpec is somewhat unclear on the topic of the actual context
> size, empirical tests have now revealed the truth. So let's add a
> big fat comment to remind people how it all works.
> 
> As a result of correctly interpreting CXT_SIZE, the IVB context
> size is reduced from three pages to two, while SNB context size
> remains at two pages.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Ben Widawsky <ben@bwidawsk.net>

[snip]

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

Re: [PATCH] drm/i915: Fix context size calculation on SNB/IVB/VLV

[Daniel Vetter]

On Thu, Aug 22, 2013 at 12:05:31PM -0700, Ben Widawsky wrote:
> On Thu, Aug 22, 2013 at 07:23:13PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > All the different context sizes reported in the CXT_SIZE register
> > aren't meant to be simply added together.
> > 
> > While BSpec is somewhat unclear on the topic of the actual context
> > size, empirical tests have now revealed the truth. So let's add a
> > big fat comment to remind people how it all works.
> > 
> > As a result of correctly interpreting CXT_SIZE, the IVB context
> > size is reduced from three pages to two, while SNB context size
> > remains at two pages.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Acked-by: Ben Widawsky <ben@bwidawsk.net>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch