* [PATCH] drm/i915: Fix context sizes on HSW
@ 2013-06-24 17:01 Ben Widawsky
2013-06-24 17:01 ` [PATCH] drm/i915: PPGTT dump for debug Ben Widawsky
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Ben Widawsky @ 2013-06-24 17:01 UTC (permalink / raw)
To: Intel GFX; +Cc: Ben Widawsky, stable
With updates to the spec, we can actually see the context layout, and
how many dwords are allocated. That table suggests we need 70720 bytes
per HW context. Rounded up, this is 18 pages. Looking at what lives
after the current 4 pages we use, I can't see too much important (mostly
it's d3d related), but there are a couple of things which look scary. I
am hopeful this can explain some of our odd HSW failures.
NOTE: There is some discrepancy in our internal docs about whether or
not this should be 17, or 18 pages. Since for stable, and currently we
don't use many contexts, just use 18 to be safe. We can trim it later.
Reported-by: "Azad, Vinit" <vinit.azad@intel.com>
Cc: stable@vger.kernel.org
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/i915_reg.h | 9 +--------
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index ff47145..51b7a21 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -113,7 +113,7 @@ static int get_context_size(struct drm_device *dev)
case 7:
reg = I915_READ(GEN7_CXT_SIZE);
if (IS_HASWELL(dev))
- ret = HSW_CXT_TOTAL_SIZE(reg) * 64;
+ ret = HSW_CXT_TOTAL_SIZE;
else
ret = GEN7_CXT_TOTAL_SIZE(reg) * 64;
break;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2102ff3..4344b77 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1718,14 +1718,7 @@
GEN7_CXT_EXTENDED_SIZE(ctx_reg) + \
GEN7_CXT_GT1_SIZE(ctx_reg) + \
GEN7_CXT_VFSTATE_SIZE(ctx_reg))
-#define HSW_CXT_POWER_SIZE(ctx_reg) ((ctx_reg >> 26) & 0x3f)
-#define HSW_CXT_RING_SIZE(ctx_reg) ((ctx_reg >> 23) & 0x7)
-#define HSW_CXT_RENDER_SIZE(ctx_reg) ((ctx_reg >> 15) & 0xff)
-#define HSW_CXT_TOTAL_SIZE(ctx_reg) (HSW_CXT_POWER_SIZE(ctx_reg) + \
- HSW_CXT_RING_SIZE(ctx_reg) + \
- HSW_CXT_RENDER_SIZE(ctx_reg) + \
- GEN7_CXT_VFSTATE_SIZE(ctx_reg))
-
+#define HSW_CXT_TOTAL_SIZE (18 * PAGE_SIZE)
/*
* Overlay regs
--
1.8.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH] drm/i915: PPGTT dump for debug
2013-06-24 17:01 [PATCH] drm/i915: Fix context sizes on HSW Ben Widawsky
@ 2013-06-24 17:01 ` Ben Widawsky
2013-06-24 16:59 ` Ben Widawsky
2013-06-24 22:17 ` [PATCH] [v2] drm/i915: Fix context sizes on HSW Ben Widawsky
2013-06-26 4:53 ` [PATCH] [v3] " Ben Widawsky
2 siblings, 1 reply; 7+ messages in thread
From: Ben Widawsky @ 2013-06-24 17:01 UTC (permalink / raw)
To: Intel GFX; +Cc: Ben Widawsky
No users yet
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 893007a..005358f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -99,6 +99,37 @@ static gen6_gtt_pte_t hsw_pte_encode(dma_addr_t addr,
return pte;
}
+void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt)
+{
+ struct drm_i915_private *dev_priv = ppgtt->base.dev->dev_private;
+ gen6_gtt_pte_t __iomem *pd_addr;
+ uint32_t pd_entry;
+ int i, j;
+
+ pd_addr = (gen6_gtt_pte_t __iomem*)dev_priv->gtt.gsm +
+ ppgtt->pd_offset / sizeof(gen6_gtt_pte_t);
+
+ for (i = 0; i < ppgtt->num_pd_entries; i++) {
+ gen6_gtt_pte_t *pt_vaddr;
+ dma_addr_t pt_addr = ppgtt->pt_dma_addr[i];
+ pd_entry = readl(pd_addr + i);
+
+ if (pd_entry != (u32)(GEN6_PDE_ADDR_ENCODE(pt_addr) | GEN6_PDE_VALID))
+ DRM_ERROR("Actual PDE: %x Expected PDE: %x",
+ pd_entry,
+ (u32)(GEN6_PDE_ADDR_ENCODE(pt_addr) | GEN6_PDE_VALID));
+
+ pt_vaddr = kmap_atomic(ppgtt->pt_pages[i]);
+ for (j = 0; j < I915_PPGTT_PT_ENTRIES; j++) {
+ gen6_gtt_pte_t scratch_pte =
+ ppgtt->base.pte_encode(ppgtt->base.scratch.addr,
+ I915_CACHE_LLC);
+ if (pt_vaddr[j] != scratch_pte)
+ DRM_ERROR("\tPTE = %x (%d %d)", pt_vaddr[j], i,j);
+ }
+ kunmap_atomic(pt_vaddr);
+ }
+}
static void gen6_write_pdes(struct i915_hw_ppgtt *ppgtt)
{
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH] [v2] drm/i915: Fix context sizes on HSW
2013-06-24 17:01 [PATCH] drm/i915: Fix context sizes on HSW Ben Widawsky
2013-06-24 17:01 ` [PATCH] drm/i915: PPGTT dump for debug Ben Widawsky
@ 2013-06-24 22:17 ` Ben Widawsky
2013-06-25 21:59 ` [Intel-gfx] " Jesse Barnes
2013-06-26 4:53 ` [PATCH] [v3] " Ben Widawsky
2 siblings, 1 reply; 7+ messages in thread
From: Ben Widawsky @ 2013-06-24 22:17 UTC (permalink / raw)
To: Intel GFX; +Cc: Vinit Azad, Ben Widawsky, stable
With updates to the spec, we can actually see the context layout, and
how many dwords are allocated. That table suggests we need 70720 bytes
per HW context. Rounded up, this is 18 pages. Looking at what lives
after the current 4 pages we use, I can't see too much important (mostly
it's d3d related), but there are a couple of things which look scary. I
am hopeful this can explain some of our odd HSW failures.
v2: Make the context only 17 pages. The power context space isn't used
ever, and execlists aren't used in our driver, making the actual total
66944 bytes.
Reported-by: "Azad, Vinit" <vinit.azad@intel.com>
Cc: stable@vger.kernel.org
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/i915_reg.h | 9 +--------
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index ff47145..51b7a21 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -113,7 +113,7 @@ static int get_context_size(struct drm_device *dev)
case 7:
reg = I915_READ(GEN7_CXT_SIZE);
if (IS_HASWELL(dev))
- ret = HSW_CXT_TOTAL_SIZE(reg) * 64;
+ ret = HSW_CXT_TOTAL_SIZE;
else
ret = GEN7_CXT_TOTAL_SIZE(reg) * 64;
break;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2102ff3..f061c67 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1718,14 +1718,7 @@
GEN7_CXT_EXTENDED_SIZE(ctx_reg) + \
GEN7_CXT_GT1_SIZE(ctx_reg) + \
GEN7_CXT_VFSTATE_SIZE(ctx_reg))
-#define HSW_CXT_POWER_SIZE(ctx_reg) ((ctx_reg >> 26) & 0x3f)
-#define HSW_CXT_RING_SIZE(ctx_reg) ((ctx_reg >> 23) & 0x7)
-#define HSW_CXT_RENDER_SIZE(ctx_reg) ((ctx_reg >> 15) & 0xff)
-#define HSW_CXT_TOTAL_SIZE(ctx_reg) (HSW_CXT_POWER_SIZE(ctx_reg) + \
- HSW_CXT_RING_SIZE(ctx_reg) + \
- HSW_CXT_RENDER_SIZE(ctx_reg) + \
- GEN7_CXT_VFSTATE_SIZE(ctx_reg))
-
+#define HSW_CXT_TOTAL_SIZE (17 * PAGE_SIZE)
/*
* Overlay regs
--
1.8.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [PATCH] [v2] drm/i915: Fix context sizes on HSW
2013-06-24 22:17 ` [PATCH] [v2] drm/i915: Fix context sizes on HSW Ben Widawsky
@ 2013-06-25 21:59 ` Jesse Barnes
0 siblings, 0 replies; 7+ messages in thread
From: Jesse Barnes @ 2013-06-25 21:59 UTC (permalink / raw)
To: Ben Widawsky; +Cc: Intel GFX, stable
On Mon, 24 Jun 2013 15:17:09 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> With updates to the spec, we can actually see the context layout, and
> how many dwords are allocated. That table suggests we need 70720 bytes
> per HW context. Rounded up, this is 18 pages. Looking at what lives
> after the current 4 pages we use, I can't see too much important (mostly
> it's d3d related), but there are a couple of things which look scary. I
> am hopeful this can explain some of our odd HSW failures.
>
> v2: Make the context only 17 pages. The power context space isn't used
> ever, and execlists aren't used in our driver, making the actual total
> 66944 bytes.
>
> Reported-by: "Azad, Vinit" <vinit.azad@intel.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
> drivers/gpu/drm/i915/i915_reg.h | 9 +--------
> 2 files changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index ff47145..51b7a21 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -113,7 +113,7 @@ static int get_context_size(struct drm_device *dev)
> case 7:
> reg = I915_READ(GEN7_CXT_SIZE);
> if (IS_HASWELL(dev))
> - ret = HSW_CXT_TOTAL_SIZE(reg) * 64;
> + ret = HSW_CXT_TOTAL_SIZE;
> else
> ret = GEN7_CXT_TOTAL_SIZE(reg) * 64;
> break;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2102ff3..f061c67 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1718,14 +1718,7 @@
> GEN7_CXT_EXTENDED_SIZE(ctx_reg) + \
> GEN7_CXT_GT1_SIZE(ctx_reg) + \
> GEN7_CXT_VFSTATE_SIZE(ctx_reg))
> -#define HSW_CXT_POWER_SIZE(ctx_reg) ((ctx_reg >> 26) & 0x3f)
> -#define HSW_CXT_RING_SIZE(ctx_reg) ((ctx_reg >> 23) & 0x7)
> -#define HSW_CXT_RENDER_SIZE(ctx_reg) ((ctx_reg >> 15) & 0xff)
> -#define HSW_CXT_TOTAL_SIZE(ctx_reg) (HSW_CXT_POWER_SIZE(ctx_reg) + \
> - HSW_CXT_RING_SIZE(ctx_reg) + \
> - HSW_CXT_RENDER_SIZE(ctx_reg) + \
> - GEN7_CXT_VFSTATE_SIZE(ctx_reg))
> -
> +#define HSW_CXT_TOTAL_SIZE (17 * PAGE_SIZE)
>
> /*
> * Overlay regs
Yeah, we can't rely on the reg I guess, and this matches the docs
(though they don't make it easy to figure it out)... I'd like to see a
reference to the bspec where this information is kept, but other than
that:
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] [v3] drm/i915: Fix context sizes on HSW
2013-06-24 17:01 [PATCH] drm/i915: Fix context sizes on HSW Ben Widawsky
2013-06-24 17:01 ` [PATCH] drm/i915: PPGTT dump for debug Ben Widawsky
2013-06-24 22:17 ` [PATCH] [v2] drm/i915: Fix context sizes on HSW Ben Widawsky
@ 2013-06-26 4:53 ` Ben Widawsky
2013-06-27 17:40 ` [Intel-gfx] " Daniel Vetter
2 siblings, 1 reply; 7+ messages in thread
From: Ben Widawsky @ 2013-06-26 4:53 UTC (permalink / raw)
To: Intel GFX; +Cc: Ben Widawsky, stable
With updates to the spec, we can actually see the context layout, and
how many dwords are allocated. That table suggests we need 70720 bytes
per HW context. Rounded up, this is 18 pages. Looking at what lives
after the current 4 pages we use, I can't see too much important (mostly
it's d3d related), but there are a couple of things which look scary. I
am hopeful this can explain some of our odd HSW failures.
v2: Make the context only 17 pages. The power context space isn't used
ever, and execlists aren't used in our driver, making the actual total
66944 bytes.
v3: Add a comment to the code. (Jesse & Paulo)
Reported-by: "Azad, Vinit" <vinit.azad@intel.com>
Cc: stable@vger.kernel.org
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/i915_reg.h | 15 +++++++--------
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index ff47145..51b7a21 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -113,7 +113,7 @@ static int get_context_size(struct drm_device *dev)
case 7:
reg = I915_READ(GEN7_CXT_SIZE);
if (IS_HASWELL(dev))
- ret = HSW_CXT_TOTAL_SIZE(reg) * 64;
+ ret = HSW_CXT_TOTAL_SIZE;
else
ret = GEN7_CXT_TOTAL_SIZE(reg) * 64;
break;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2102ff3..a426168 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1718,14 +1718,13 @@
GEN7_CXT_EXTENDED_SIZE(ctx_reg) + \
GEN7_CXT_GT1_SIZE(ctx_reg) + \
GEN7_CXT_VFSTATE_SIZE(ctx_reg))
-#define HSW_CXT_POWER_SIZE(ctx_reg) ((ctx_reg >> 26) & 0x3f)
-#define HSW_CXT_RING_SIZE(ctx_reg) ((ctx_reg >> 23) & 0x7)
-#define HSW_CXT_RENDER_SIZE(ctx_reg) ((ctx_reg >> 15) & 0xff)
-#define HSW_CXT_TOTAL_SIZE(ctx_reg) (HSW_CXT_POWER_SIZE(ctx_reg) + \
- HSW_CXT_RING_SIZE(ctx_reg) + \
- HSW_CXT_RENDER_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
+ * size is 70720 bytes, however, the power context and execlist context will
+ * never be saved (power context is stored elsewhere, and execlists don't work
+ * on HSW) - so the final size is 66944 bytes, which rounds to 17 pages.
+ */
+#define HSW_CXT_TOTAL_SIZE (17 * PAGE_SIZE)
/*
* Overlay regs
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [PATCH] [v3] drm/i915: Fix context sizes on HSW
2013-06-26 4:53 ` [PATCH] [v3] " Ben Widawsky
@ 2013-06-27 17:40 ` Daniel Vetter
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2013-06-27 17:40 UTC (permalink / raw)
To: Ben Widawsky; +Cc: Intel GFX, stable
On Tue, Jun 25, 2013 at 09:53:40PM -0700, Ben Widawsky wrote:
> With updates to the spec, we can actually see the context layout, and
> how many dwords are allocated. That table suggests we need 70720 bytes
> per HW context. Rounded up, this is 18 pages. Looking at what lives
> after the current 4 pages we use, I can't see too much important (mostly
> it's d3d related), but there are a couple of things which look scary. I
> am hopeful this can explain some of our odd HSW failures.
>
> v2: Make the context only 17 pages. The power context space isn't used
> ever, and execlists aren't used in our driver, making the actual total
> 66944 bytes.
>
> v3: Add a comment to the code. (Jesse & Paulo)
>
> Reported-by: "Azad, Vinit" <vinit.azad@intel.com>
> Cc: stable@vger.kernel.org
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-06-27 17:40 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-24 17:01 [PATCH] drm/i915: Fix context sizes on HSW Ben Widawsky
2013-06-24 17:01 ` [PATCH] drm/i915: PPGTT dump for debug Ben Widawsky
2013-06-24 16:59 ` Ben Widawsky
2013-06-24 22:17 ` [PATCH] [v2] drm/i915: Fix context sizes on HSW Ben Widawsky
2013-06-25 21:59 ` [Intel-gfx] " Jesse Barnes
2013-06-26 4:53 ` [PATCH] [v3] " Ben Widawsky
2013-06-27 17:40 ` [Intel-gfx] " Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox