* [PATCH 1/5] drm/i915: trivial: kill-agp collateral cleanups
2013-01-26 0:41 [PATCH 0/5] [REPOST] GTT cleanups, rebased Ben Widawsky
@ 2013-01-26 0:41 ` Ben Widawsky
2013-01-29 12:49 ` Daniel Vetter
2013-01-26 0:41 ` [PATCH 2/5] drm/i915: Reclaim GTT space for failed PPGTT Ben Widawsky
` (4 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Ben Widawsky @ 2013-01-26 0:41 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
- i915_gem_init_aliasing_ppgtt should now be static
- move i915_gem_init_ppgtt declaration to the right place
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_drv.h | 3 +--
drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 09753e5..1af8e73 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1598,7 +1598,6 @@ int __must_check i915_gem_init(struct drm_device *dev);
int __must_check i915_gem_init_hw(struct drm_device *dev);
void i915_gem_l3_remap(struct drm_device *dev);
void i915_gem_init_swizzling(struct drm_device *dev);
-void i915_gem_init_ppgtt(struct drm_device *dev);
void i915_gem_cleanup_ringbuffer(struct drm_device *dev);
int __must_check i915_gpu_idle(struct drm_device *dev);
int __must_check i915_gem_idle(struct drm_device *dev);
@@ -1653,7 +1652,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
/* i915_gem_gtt.c */
-int __must_check i915_gem_init_aliasing_ppgtt(struct drm_device *dev);
+void i915_gem_init_ppgtt(struct drm_device *dev);
void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev);
void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a0ba4a9..462dec9 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -108,7 +108,7 @@ static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
}
}
-int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
+static int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct i915_hw_ppgtt *ppgtt;
--
1.8.1.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 1/5] drm/i915: trivial: kill-agp collateral cleanups
2013-01-26 0:41 ` [PATCH 1/5] drm/i915: trivial: kill-agp collateral cleanups Ben Widawsky
@ 2013-01-29 12:49 ` Daniel Vetter
2013-01-29 18:58 ` Ben Widawsky
0 siblings, 1 reply; 20+ messages in thread
From: Daniel Vetter @ 2013-01-29 12:49 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Fri, Jan 25, 2013 at 04:41:03PM -0800, Ben Widawsky wrote:
> - i915_gem_init_aliasing_ppgtt should now be static
> - move i915_gem_init_ppgtt declaration to the right place
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
I've tried to apply this patch on top of latest dinq, and there's nothing
left after resolving conflicts. Have I screwed something up?
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +--
> drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 09753e5..1af8e73 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1598,7 +1598,6 @@ int __must_check i915_gem_init(struct drm_device *dev);
> int __must_check i915_gem_init_hw(struct drm_device *dev);
> void i915_gem_l3_remap(struct drm_device *dev);
> void i915_gem_init_swizzling(struct drm_device *dev);
> -void i915_gem_init_ppgtt(struct drm_device *dev);
> void i915_gem_cleanup_ringbuffer(struct drm_device *dev);
> int __must_check i915_gpu_idle(struct drm_device *dev);
> int __must_check i915_gem_idle(struct drm_device *dev);
> @@ -1653,7 +1652,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file);
>
> /* i915_gem_gtt.c */
> -int __must_check i915_gem_init_aliasing_ppgtt(struct drm_device *dev);
> +void i915_gem_init_ppgtt(struct drm_device *dev);
> void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev);
> void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
> struct drm_i915_gem_object *obj,
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index a0ba4a9..462dec9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -108,7 +108,7 @@ static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
> }
> }
>
> -int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
> +static int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct i915_hw_ppgtt *ppgtt;
> --
> 1.8.1.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 1/5] drm/i915: trivial: kill-agp collateral cleanups
2013-01-29 12:49 ` Daniel Vetter
@ 2013-01-29 18:58 ` Ben Widawsky
0 siblings, 0 replies; 20+ messages in thread
From: Ben Widawsky @ 2013-01-29 18:58 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Tue, Jan 29, 2013 at 01:49:22PM +0100, Daniel Vetter wrote:
> On Fri, Jan 25, 2013 at 04:41:03PM -0800, Ben Widawsky wrote:
> > - i915_gem_init_aliasing_ppgtt should now be static
> > - move i915_gem_init_ppgtt declaration to the right place
> >
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
>
> I've tried to apply this patch on top of latest dinq, and there's nothing
> left after resolving conflicts. Have I screwed something up?
> -Daniel
No, I think you're right. I think it no longer applies.
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/5] drm/i915: Reclaim GTT space for failed PPGTT
2013-01-26 0:41 [PATCH 0/5] [REPOST] GTT cleanups, rebased Ben Widawsky
2013-01-26 0:41 ` [PATCH 1/5] drm/i915: trivial: kill-agp collateral cleanups Ben Widawsky
@ 2013-01-26 0:41 ` Ben Widawsky
2013-01-29 12:51 ` Daniel Vetter
2013-01-26 0:41 ` [PATCH 3/5] drm/i915: Extract gen6 aliasing ppgtt code Ben Widawsky
` (3 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Ben Widawsky @ 2013-01-26 0:41 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
When the PPGTT init fails, we may as well reuse the space that we were
reserving for the PPGTT PDEs.
This also fixes an extraneous mutex_unlock.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 33 ++++++++++++++++-----------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 462dec9..31c490e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -531,12 +531,20 @@ static void i915_gtt_color_adjust(struct drm_mm_node *node,
*end -= 4096;
}
}
-
void i915_gem_setup_global_gtt(struct drm_device *dev,
unsigned long start,
unsigned long mappable_end,
unsigned long end)
{
+ /* Let GEM Manage all of the aperture.
+ *
+ * However, leave one page at the end still bound to the scratch page.
+ * There are a number of places where the hardware apparently prefetches
+ * past the end of the object, and we've seen multiple hangs with the
+ * GPU head pointer stuck in a batchbuffer bound at the last page of the
+ * aperture. One page should be enough to keep any prefetching inside
+ * of the aperture.
+ */
drm_i915_private_t *dev_priv = dev->dev_private;
struct drm_mm_node *entry;
struct drm_i915_gem_object *obj;
@@ -598,12 +606,12 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long gtt_size, mappable_size;
- int ret;
gtt_size = dev_priv->mm.gtt->gtt_total_entries << PAGE_SHIFT;
mappable_size = dev_priv->gtt.mappable_end;
if (intel_enable_ppgtt(dev) && HAS_ALIASING_PPGTT(dev)) {
+ int ret;
/* PPGTT pdes are stolen from global gtt ptes, so shrink the
* aperture accordingly when using aliasing ppgtt. */
gtt_size -= I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
@@ -611,23 +619,14 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
ret = i915_gem_init_aliasing_ppgtt(dev);
- if (ret) {
- mutex_unlock(&dev->struct_mutex);
+ if (!ret)
return;
- }
- } else {
- /* Let GEM Manage all of the aperture.
- *
- * However, leave one page at the end still bound to the scratch
- * page. There are a number of places where the hardware
- * apparently prefetches past the end of the object, and we've
- * seen multiple hangs with the GPU head pointer stuck in a
- * batchbuffer bound at the last page of the aperture. One page
- * should be enough to keep any prefetching inside of the
- * aperture.
- */
- i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
+
+ DRM_ERROR("Aliased PPGTT setup failed %d\n", ret);
+ drm_mm_takedown(&dev_priv->mm.gtt_space);
+ gtt_size += I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
}
+ i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
}
static int setup_scratch_page(struct drm_device *dev)
--
1.8.1.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 3/5] drm/i915: Extract gen6 aliasing ppgtt code
2013-01-26 0:41 [PATCH 0/5] [REPOST] GTT cleanups, rebased Ben Widawsky
2013-01-26 0:41 ` [PATCH 1/5] drm/i915: trivial: kill-agp collateral cleanups Ben Widawsky
2013-01-26 0:41 ` [PATCH 2/5] drm/i915: Reclaim GTT space for failed PPGTT Ben Widawsky
@ 2013-01-26 0:41 ` Ben Widawsky
2013-01-29 12:55 ` Daniel Vetter
2013-01-26 0:41 ` [PATCH 4/5] drm/i915: Aliased PPGTT size abstraction Ben Widawsky
` (2 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Ben Widawsky @ 2013-01-26 0:41 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
This refactor clearly identifies the GEN specific portion of the aliased
ppgtt code. Aside from some of the gen specific parts being extracted,
it also preps us for an upcoming patch that pulls out the size, which
has some nice benefits.
v2: Fix wonky error handling (Chris)
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 42 ++++++++++++++++++++++++-------------
1 file changed, 28 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 31c490e..f79f427 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -108,10 +108,10 @@ static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
}
}
-static int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
+static int gen6_init_aliasing_ppgtt(struct i915_hw_ppgtt *ppgtt)
{
+ struct drm_device *dev = ppgtt->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct i915_hw_ppgtt *ppgtt;
unsigned first_pd_entry_in_global_pt;
int i;
int ret = -ENOMEM;
@@ -121,16 +121,12 @@ static int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
* now. */
first_pd_entry_in_global_pt = dev_priv->mm.gtt->gtt_total_entries - I915_PPGTT_PD_ENTRIES;
- ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL);
- if (!ppgtt)
- return ret;
-
- ppgtt->dev = dev;
ppgtt->num_pd_entries = I915_PPGTT_PD_ENTRIES;
ppgtt->pt_pages = kzalloc(sizeof(struct page *)*ppgtt->num_pd_entries,
GFP_KERNEL);
+
if (!ppgtt->pt_pages)
- goto err_ppgtt;
+ return -ENOMEM;
for (i = 0; i < ppgtt->num_pd_entries; i++) {
ppgtt->pt_pages[i] = alloc_page(GFP_KERNEL);
@@ -157,15 +153,11 @@ static int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
ppgtt->pt_dma_addr[i] = pt_addr;
}
- ppgtt->scratch_page_dma_addr = dev_priv->gtt.scratch_page_dma;
-
i915_ppgtt_clear_range(ppgtt, 0,
ppgtt->num_pd_entries*I915_PPGTT_PT_ENTRIES);
ppgtt->pd_offset = (first_pd_entry_in_global_pt)*sizeof(gtt_pte_t);
- dev_priv->mm.aliasing_ppgtt = ppgtt;
-
return 0;
err_pd_pin:
@@ -180,9 +172,31 @@ err_pt_alloc:
if (ppgtt->pt_pages[i])
__free_page(ppgtt->pt_pages[i]);
}
+
kfree(ppgtt->pt_pages);
-err_ppgtt:
- kfree(ppgtt);
+
+ return ret;
+}
+
+
+static int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct i915_hw_ppgtt *ppgtt;
+ int ret = -ENOMEM;
+
+ ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL);
+ if (!ppgtt)
+ return ret;
+
+ ppgtt->dev = dev;
+ ppgtt->scratch_page_dma_addr = dev_priv->gtt.scratch_page_dma;
+
+ ret = gen6_init_aliasing_ppgtt(ppgtt);
+ if (ret)
+ kfree(ppgtt);
+ else
+ dev_priv->mm.aliasing_ppgtt = ppgtt;
return ret;
}
--
1.8.1.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 3/5] drm/i915: Extract gen6 aliasing ppgtt code
2013-01-26 0:41 ` [PATCH 3/5] drm/i915: Extract gen6 aliasing ppgtt code Ben Widawsky
@ 2013-01-29 12:55 ` Daniel Vetter
2013-01-29 19:00 ` Ben Widawsky
0 siblings, 1 reply; 20+ messages in thread
From: Daniel Vetter @ 2013-01-29 12:55 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Fri, Jan 25, 2013 at 04:41:05PM -0800, Ben Widawsky wrote:
> This refactor clearly identifies the GEN specific portion of the aliased
> ppgtt code. Aside from some of the gen specific parts being extracted,
> it also preps us for an upcoming patch that pulls out the size, which
> has some nice benefits.
>
> v2: Fix wonky error handling (Chris)
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Functionally we seem to have this patch already with
commit e9ccfd7b94f1ca9b46b992a19d2e8c5af70e4c69
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Thu Jan 24 13:49:56 2013 -0800
drm/i915: extract hw ppgtt setup/cleanup code
Yeah, you're allowed to hate me with the heat of a thousand suns now ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 3/5] drm/i915: Extract gen6 aliasing ppgtt code
2013-01-29 12:55 ` Daniel Vetter
@ 2013-01-29 19:00 ` Ben Widawsky
0 siblings, 0 replies; 20+ messages in thread
From: Ben Widawsky @ 2013-01-29 19:00 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Tue, Jan 29, 2013 at 01:55:46PM +0100, Daniel Vetter wrote:
> On Fri, Jan 25, 2013 at 04:41:05PM -0800, Ben Widawsky wrote:
> > This refactor clearly identifies the GEN specific portion of the aliased
> > ppgtt code. Aside from some of the gen specific parts being extracted,
> > it also preps us for an upcoming patch that pulls out the size, which
> > has some nice benefits.
> >
> > v2: Fix wonky error handling (Chris)
> >
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
>
> Functionally we seem to have this patch already with
>
> commit e9ccfd7b94f1ca9b46b992a19d2e8c5af70e4c69
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date: Thu Jan 24 13:49:56 2013 -0800
>
> drm/i915: extract hw ppgtt setup/cleanup code
>
> Yeah, you're allowed to hate me with the heat of a thousand suns now ;-)
> -Daniel
I already did. I guess when I rebased the series, you hadn't merged
those yet.
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 4/5] drm/i915: Aliased PPGTT size abstraction
2013-01-26 0:41 [PATCH 0/5] [REPOST] GTT cleanups, rebased Ben Widawsky
` (2 preceding siblings ...)
2013-01-26 0:41 ` [PATCH 3/5] drm/i915: Extract gen6 aliasing ppgtt code Ben Widawsky
@ 2013-01-26 0:41 ` Ben Widawsky
2013-01-28 12:19 ` Jani Nikula
2013-01-28 20:35 ` [PATCH 4/5 v2] " Ben Widawsky
2013-01-26 0:41 ` [PATCH 5/5] drm/i915: Dynamically calculate dclv Ben Widawsky
2013-01-28 12:24 ` [PATCH 0/5] [REPOST] GTT cleanups, rebased Jani Nikula
5 siblings, 2 replies; 20+ messages in thread
From: Ben Widawsky @ 2013-01-26 0:41 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
Aside from being a nice prep for some work I'm doing in PPGTT, this also
leaves us with a more accurate size in our gtt_total_entries member.
Since we can't actually use the PPGTT reserved part of the GGTT.
This will help some of the existing assertions find issues which would
overwrite the PPGTT PDEs.
Another benefit is for platforms which don't use the entire 2GB GTT,
this will save GTT space. For example if a platform has 1 1GB GTT, this
patch should save 1MB of GTT space (512MB has an even greater savings).
I don't have a platform to easily test this however.
If/when we ever move to real PPGTT, I think allocating the PDEs as
objects will make the most sense.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
drivers/gpu/drm/i915/i915_drv.h | 3 ++-
drivers/gpu/drm/i915/i915_gem_gtt.c | 35 ++++++++++++++++++++++++++---------
3 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 384f193..4545357 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1605,6 +1605,8 @@ static int i915_ppgtt_info(struct seq_file *m, void *data)
seq_printf(m, "aliasing PPGTT:\n");
seq_printf(m, "pd gtt offset: 0x%08x\n", ppgtt->pd_offset);
+ seq_printf(m, "mapped size: %lldM\n",
+ ppgtt->mapped_size>>20);
}
seq_printf(m, "ECOCHK: 0x%08x\n", I915_READ(GAM_ECOCHK));
mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1af8e73..60ba4ab 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -388,10 +388,11 @@ struct i915_gtt {
struct page *scratch_page;
};
-#define I915_PPGTT_PD_ENTRIES 512
+#define GEN6_MAX_PD_ENTRIES 512
#define I915_PPGTT_PT_ENTRIES 1024
struct i915_hw_ppgtt {
struct drm_device *dev;
+ uint64_t mapped_size;
unsigned num_pd_entries;
struct page **pt_pages;
uint32_t pd_offset;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index f79f427..d9af815 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -108,7 +108,7 @@ static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
}
}
-static int gen6_init_aliasing_ppgtt(struct i915_hw_ppgtt *ppgtt)
+static int gen6_init_ppgtt(struct i915_hw_ppgtt *ppgtt, uint64_t size)
{
struct drm_device *dev = ppgtt->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -116,15 +116,22 @@ static int gen6_init_aliasing_ppgtt(struct i915_hw_ppgtt *ppgtt)
int i;
int ret = -ENOMEM;
+ if (dev_priv->mm.aliasing_ppgtt)
+ return -EBUSY;
+
+ /* Each PDE maps 4MB (1k PAGE_SIZE pages)*/
+ ppgtt->mapped_size = round_up(size, 1<<22);
+ ppgtt->num_pd_entries = ppgtt->mapped_size >> 22;
+ BUG_ON(ppgtt->num_pd_entries > GEN6_MAX_PD_ENTRIES);
+
/* ppgtt PDEs reside in the global gtt pagetable, which has 512*1024
* entries. For aliasing ppgtt support we just steal them at the end for
* now. */
- first_pd_entry_in_global_pt = dev_priv->mm.gtt->gtt_total_entries - I915_PPGTT_PD_ENTRIES;
+ first_pd_entry_in_global_pt = dev_priv->mm.gtt->gtt_total_entries -
+ ppgtt->num_pd_entries;
- ppgtt->num_pd_entries = I915_PPGTT_PD_ENTRIES;
ppgtt->pt_pages = kzalloc(sizeof(struct page *)*ppgtt->num_pd_entries,
GFP_KERNEL);
-
if (!ppgtt->pt_pages)
return -ENOMEM;
@@ -156,7 +163,10 @@ static int gen6_init_aliasing_ppgtt(struct i915_hw_ppgtt *ppgtt)
i915_ppgtt_clear_range(ppgtt, 0,
ppgtt->num_pd_entries*I915_PPGTT_PT_ENTRIES);
- ppgtt->pd_offset = (first_pd_entry_in_global_pt)*sizeof(gtt_pte_t);
+ ppgtt->pd_offset = first_pd_entry_in_global_pt * sizeof(gtt_pte_t);
+
+ DRM_INFO("Allocated %d PDEs for PPGTT\n", ppgtt->num_pd_entries);
+ DRM_DEBUG_DRIVER("First PDE: %d\n", first_pd_entry_in_global_pt);
return 0;
@@ -192,7 +202,7 @@ static int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
ppgtt->dev = dev;
ppgtt->scratch_page_dma_addr = dev_priv->gtt.scratch_page_dma;
- ret = gen6_init_aliasing_ppgtt(ppgtt);
+ ret = gen6_init_ppgtt(ppgtt, dev_priv->gtt.total);
if (ret)
kfree(ppgtt);
else
@@ -625,20 +635,27 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
mappable_size = dev_priv->gtt.mappable_end;
if (intel_enable_ppgtt(dev) && HAS_ALIASING_PPGTT(dev)) {
+ /* Each PDE represents 1k PAGE_SIZE objects */
+ const int reserve_pd_space = gtt_size >> 10;
int ret;
+
+ BUG_ON(!reserve_pd_space);
+
/* PPGTT pdes are stolen from global gtt ptes, so shrink the
* aperture accordingly when using aliasing ppgtt. */
- gtt_size -= I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
+ gtt_size -= reserve_pd_space;
i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
ret = i915_gem_init_aliasing_ppgtt(dev);
- if (!ret)
+ if (!ret) {
+ dev_priv->mm.gtt->gtt_total_entries = gtt_size >> PAGE_SHIFT;
return;
+ }
DRM_ERROR("Aliased PPGTT setup failed %d\n", ret);
drm_mm_takedown(&dev_priv->mm.gtt_space);
- gtt_size += I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
+ gtt_size += reserve_pd_space;
}
i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
}
--
1.8.1.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 4/5] drm/i915: Aliased PPGTT size abstraction
2013-01-26 0:41 ` [PATCH 4/5] drm/i915: Aliased PPGTT size abstraction Ben Widawsky
@ 2013-01-28 12:19 ` Jani Nikula
2013-01-28 18:26 ` Ben Widawsky
2013-01-28 20:35 ` [PATCH 4/5 v2] " Ben Widawsky
1 sibling, 1 reply; 20+ messages in thread
From: Jani Nikula @ 2013-01-28 12:19 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
On Sat, 26 Jan 2013, Ben Widawsky <ben@bwidawsk.net> wrote:
> Aside from being a nice prep for some work I'm doing in PPGTT, this also
> leaves us with a more accurate size in our gtt_total_entries member.
> Since we can't actually use the PPGTT reserved part of the GGTT.
>
> This will help some of the existing assertions find issues which would
> overwrite the PPGTT PDEs.
>
> Another benefit is for platforms which don't use the entire 2GB GTT,
> this will save GTT space. For example if a platform has 1 1GB GTT, this
> patch should save 1MB of GTT space (512MB has an even greater savings).
> I don't have a platform to easily test this however.
>
> If/when we ever move to real PPGTT, I think allocating the PDEs as
> objects will make the most sense.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> drivers/gpu/drm/i915/i915_gem_gtt.c | 35 ++++++++++++++++++++++++++---------
> 3 files changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 384f193..4545357 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1605,6 +1605,8 @@ static int i915_ppgtt_info(struct seq_file *m, void *data)
>
> seq_printf(m, "aliasing PPGTT:\n");
> seq_printf(m, "pd gtt offset: 0x%08x\n", ppgtt->pd_offset);
> + seq_printf(m, "mapped size: %lldM\n",
> + ppgtt->mapped_size>>20);
> }
> seq_printf(m, "ECOCHK: 0x%08x\n", I915_READ(GAM_ECOCHK));
> mutex_unlock(&dev->struct_mutex);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1af8e73..60ba4ab 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -388,10 +388,11 @@ struct i915_gtt {
> struct page *scratch_page;
> };
>
> -#define I915_PPGTT_PD_ENTRIES 512
> +#define GEN6_MAX_PD_ENTRIES 512
> #define I915_PPGTT_PT_ENTRIES 1024
> struct i915_hw_ppgtt {
> struct drm_device *dev;
> + uint64_t mapped_size;
> unsigned num_pd_entries;
> struct page **pt_pages;
> uint32_t pd_offset;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index f79f427..d9af815 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -108,7 +108,7 @@ static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
> }
> }
>
> -static int gen6_init_aliasing_ppgtt(struct i915_hw_ppgtt *ppgtt)
> +static int gen6_init_ppgtt(struct i915_hw_ppgtt *ppgtt, uint64_t size)
> {
> struct drm_device *dev = ppgtt->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -116,15 +116,22 @@ static int gen6_init_aliasing_ppgtt(struct i915_hw_ppgtt *ppgtt)
> int i;
> int ret = -ENOMEM;
>
> + if (dev_priv->mm.aliasing_ppgtt)
> + return -EBUSY;
> +
> + /* Each PDE maps 4MB (1k PAGE_SIZE pages)*/
> + ppgtt->mapped_size = round_up(size, 1<<22);
> + ppgtt->num_pd_entries = ppgtt->mapped_size >> 22;
> + BUG_ON(ppgtt->num_pd_entries > GEN6_MAX_PD_ENTRIES);
> +
> /* ppgtt PDEs reside in the global gtt pagetable, which has 512*1024
> * entries. For aliasing ppgtt support we just steal them at the end for
> * now. */
> - first_pd_entry_in_global_pt = dev_priv->mm.gtt->gtt_total_entries - I915_PPGTT_PD_ENTRIES;
> + first_pd_entry_in_global_pt = dev_priv->mm.gtt->gtt_total_entries -
> + ppgtt->num_pd_entries;
>
> - ppgtt->num_pd_entries = I915_PPGTT_PD_ENTRIES;
> ppgtt->pt_pages = kzalloc(sizeof(struct page *)*ppgtt->num_pd_entries,
> GFP_KERNEL);
> -
> if (!ppgtt->pt_pages)
> return -ENOMEM;
>
> @@ -156,7 +163,10 @@ static int gen6_init_aliasing_ppgtt(struct i915_hw_ppgtt *ppgtt)
> i915_ppgtt_clear_range(ppgtt, 0,
> ppgtt->num_pd_entries*I915_PPGTT_PT_ENTRIES);
>
> - ppgtt->pd_offset = (first_pd_entry_in_global_pt)*sizeof(gtt_pte_t);
> + ppgtt->pd_offset = first_pd_entry_in_global_pt * sizeof(gtt_pte_t);
> +
> + DRM_INFO("Allocated %d PDEs for PPGTT\n", ppgtt->num_pd_entries);
> + DRM_DEBUG_DRIVER("First PDE: %d\n", first_pd_entry_in_global_pt);
>
> return 0;
>
> @@ -192,7 +202,7 @@ static int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
> ppgtt->dev = dev;
> ppgtt->scratch_page_dma_addr = dev_priv->gtt.scratch_page_dma;
>
> - ret = gen6_init_aliasing_ppgtt(ppgtt);
> + ret = gen6_init_ppgtt(ppgtt, dev_priv->gtt.total);
> if (ret)
> kfree(ppgtt);
> else
> @@ -625,20 +635,27 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
> mappable_size = dev_priv->gtt.mappable_end;
>
> if (intel_enable_ppgtt(dev) && HAS_ALIASING_PPGTT(dev)) {
> + /* Each PDE represents 1k PAGE_SIZE objects */
> + const int reserve_pd_space = gtt_size >> 10;
Maybe (likely) I'm being dense, but care to explain this shift a bit?
Should it match ppgtt->num_pd_entries * PAGE_SIZE after
i915_gem_init_aliasing_ppgtt()?
BR,
Jani.
> int ret;
> +
> + BUG_ON(!reserve_pd_space);
> +
> /* PPGTT pdes are stolen from global gtt ptes, so shrink the
> * aperture accordingly when using aliasing ppgtt. */
> - gtt_size -= I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
> + gtt_size -= reserve_pd_space;
>
> i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
>
> ret = i915_gem_init_aliasing_ppgtt(dev);
> - if (!ret)
> + if (!ret) {
> + dev_priv->mm.gtt->gtt_total_entries = gtt_size >> PAGE_SHIFT;
> return;
> + }
>
> DRM_ERROR("Aliased PPGTT setup failed %d\n", ret);
> drm_mm_takedown(&dev_priv->mm.gtt_space);
> - gtt_size += I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
> + gtt_size += reserve_pd_space;
> }
> i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
> }
> --
> 1.8.1.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 4/5] drm/i915: Aliased PPGTT size abstraction
2013-01-28 12:19 ` Jani Nikula
@ 2013-01-28 18:26 ` Ben Widawsky
2013-01-28 18:31 ` Daniel Vetter
0 siblings, 1 reply; 20+ messages in thread
From: Ben Widawsky @ 2013-01-28 18:26 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Mon, Jan 28, 2013 at 02:19:15PM +0200, Jani Nikula wrote:
> On Sat, 26 Jan 2013, Ben Widawsky <ben@bwidawsk.net> wrote:
> > Aside from being a nice prep for some work I'm doing in PPGTT, this also
> > leaves us with a more accurate size in our gtt_total_entries member.
> > Since we can't actually use the PPGTT reserved part of the GGTT.
> >
> > This will help some of the existing assertions find issues which would
> > overwrite the PPGTT PDEs.
> >
> > Another benefit is for platforms which don't use the entire 2GB GTT,
> > this will save GTT space. For example if a platform has 1 1GB GTT, this
> > patch should save 1MB of GTT space (512MB has an even greater savings).
> > I don't have a platform to easily test this however.
> >
> > If/when we ever move to real PPGTT, I think allocating the PDEs as
> > objects will make the most sense.
> >
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
> > drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> > drivers/gpu/drm/i915/i915_gem_gtt.c | 35 ++++++++++++++++++++++++++---------
> > 3 files changed, 30 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 384f193..4545357 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -1605,6 +1605,8 @@ static int i915_ppgtt_info(struct seq_file *m, void *data)
> >
> > seq_printf(m, "aliasing PPGTT:\n");
> > seq_printf(m, "pd gtt offset: 0x%08x\n", ppgtt->pd_offset);
> > + seq_printf(m, "mapped size: %lldM\n",
> > + ppgtt->mapped_size>>20);
> > }
> > seq_printf(m, "ECOCHK: 0x%08x\n", I915_READ(GAM_ECOCHK));
> > mutex_unlock(&dev->struct_mutex);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 1af8e73..60ba4ab 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -388,10 +388,11 @@ struct i915_gtt {
> > struct page *scratch_page;
> > };
> >
> > -#define I915_PPGTT_PD_ENTRIES 512
> > +#define GEN6_MAX_PD_ENTRIES 512
> > #define I915_PPGTT_PT_ENTRIES 1024
> > struct i915_hw_ppgtt {
> > struct drm_device *dev;
> > + uint64_t mapped_size;
> > unsigned num_pd_entries;
> > struct page **pt_pages;
> > uint32_t pd_offset;
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index f79f427..d9af815 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -108,7 +108,7 @@ static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
> > }
> > }
> >
> > -static int gen6_init_aliasing_ppgtt(struct i915_hw_ppgtt *ppgtt)
> > +static int gen6_init_ppgtt(struct i915_hw_ppgtt *ppgtt, uint64_t size)
> > {
> > struct drm_device *dev = ppgtt->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -116,15 +116,22 @@ static int gen6_init_aliasing_ppgtt(struct i915_hw_ppgtt *ppgtt)
> > int i;
> > int ret = -ENOMEM;
> >
> > + if (dev_priv->mm.aliasing_ppgtt)
> > + return -EBUSY;
> > +
> > + /* Each PDE maps 4MB (1k PAGE_SIZE pages)*/
> > + ppgtt->mapped_size = round_up(size, 1<<22);
> > + ppgtt->num_pd_entries = ppgtt->mapped_size >> 22;
> > + BUG_ON(ppgtt->num_pd_entries > GEN6_MAX_PD_ENTRIES);
> > +
> > /* ppgtt PDEs reside in the global gtt pagetable, which has 512*1024
> > * entries. For aliasing ppgtt support we just steal them at the end for
> > * now. */
> > - first_pd_entry_in_global_pt = dev_priv->mm.gtt->gtt_total_entries - I915_PPGTT_PD_ENTRIES;
> > + first_pd_entry_in_global_pt = dev_priv->mm.gtt->gtt_total_entries -
> > + ppgtt->num_pd_entries;
> >
> > - ppgtt->num_pd_entries = I915_PPGTT_PD_ENTRIES;
> > ppgtt->pt_pages = kzalloc(sizeof(struct page *)*ppgtt->num_pd_entries,
> > GFP_KERNEL);
> > -
> > if (!ppgtt->pt_pages)
> > return -ENOMEM;
> >
> > @@ -156,7 +163,10 @@ static int gen6_init_aliasing_ppgtt(struct i915_hw_ppgtt *ppgtt)
> > i915_ppgtt_clear_range(ppgtt, 0,
> > ppgtt->num_pd_entries*I915_PPGTT_PT_ENTRIES);
> >
> > - ppgtt->pd_offset = (first_pd_entry_in_global_pt)*sizeof(gtt_pte_t);
> > + ppgtt->pd_offset = first_pd_entry_in_global_pt * sizeof(gtt_pte_t);
> > +
> > + DRM_INFO("Allocated %d PDEs for PPGTT\n", ppgtt->num_pd_entries);
> > + DRM_DEBUG_DRIVER("First PDE: %d\n", first_pd_entry_in_global_pt);
> >
> > return 0;
> >
> > @@ -192,7 +202,7 @@ static int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
> > ppgtt->dev = dev;
> > ppgtt->scratch_page_dma_addr = dev_priv->gtt.scratch_page_dma;
> >
> > - ret = gen6_init_aliasing_ppgtt(ppgtt);
> > + ret = gen6_init_ppgtt(ppgtt, dev_priv->gtt.total);
> > if (ret)
> > kfree(ppgtt);
> > else
> > @@ -625,20 +635,27 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
> > mappable_size = dev_priv->gtt.mappable_end;
> >
> > if (intel_enable_ppgtt(dev) && HAS_ALIASING_PPGTT(dev)) {
> > + /* Each PDE represents 1k PAGE_SIZE objects */
> > + const int reserve_pd_space = gtt_size >> 10;
>
> Maybe (likely) I'm being dense, but care to explain this shift a bit?
> Should it match ppgtt->num_pd_entries * PAGE_SIZE after
> i915_gem_init_aliasing_ppgtt()?
>
> BR,
> Jani.
>
yeah, it's horribly confusing until you actually need to write code
yourself. I actually think Daniel should have reviewed this patch, but I
guess he is lazy.
In this context, gtt_size refers to the amount of GPU mappable memory.
(I actually had another patch which renamed all of this stuff, but
whatever).
We want to be able to map the entire space the gtt maps with the ppgtt.
Each page directory holds 1024 PTEs, which each map a 4k pfn. Therefore,
each PDEs is effecitvely mapping (1 << (10 + 12)). Since the PDs are
stored in the GGTT (suck), we're trying to carve out the necessary
memory from the GGTT. The PDEs are special, see the DCLV stuff.
The number of PDEs required is:
gtt_size >> 22
But as I said above, we really need to calculate how much PD space we
need. Each PD has PAGE_SIZE of space required
gtt_size >> 22 >> PAGE_SIZE = gtt_size >> 10
In hindsight, maybe doing that calculation in the code would have been
better.
Anyway, to answer the question, I think yes, it should match
ppgtt->num_pd_entries * PAGE_SIZE
> > int ret;
> > +
> > + BUG_ON(!reserve_pd_space);
> > +
> > /* PPGTT pdes are stolen from global gtt ptes, so shrink the
> > * aperture accordingly when using aliasing ppgtt. */
> > - gtt_size -= I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
> > + gtt_size -= reserve_pd_space;
> >
> > i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
> >
> > ret = i915_gem_init_aliasing_ppgtt(dev);
> > - if (!ret)
> > + if (!ret) {
> > + dev_priv->mm.gtt->gtt_total_entries = gtt_size >> PAGE_SHIFT;
> > return;
> > + }
> >
> > DRM_ERROR("Aliased PPGTT setup failed %d\n", ret);
> > drm_mm_takedown(&dev_priv->mm.gtt_space);
> > - gtt_size += I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
> > + gtt_size += reserve_pd_space;
> > }
> > i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
> > }
> > --
> > 1.8.1.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 4/5] drm/i915: Aliased PPGTT size abstraction
2013-01-28 18:26 ` Ben Widawsky
@ 2013-01-28 18:31 ` Daniel Vetter
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2013-01-28 18:31 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Mon, Jan 28, 2013 at 7:26 PM, Ben Widawsky <ben@bwidawsk.net> wrote:
> But as I said above, we really need to calculate how much PD space we
> need. Each PD has PAGE_SIZE of space required
> gtt_size >> 22 >> PAGE_SIZE = gtt_size >> 10
gtt_size >> 22 * PAGE_SIZE
> In hindsight, maybe doing that calculation in the code would have been
> better.
>
> Anyway, to answer the question, I think yes, it should match
> ppgtt->num_pd_entries * PAGE_SIZE
That looks much clearer imo and spells out what happens much better
(we still a pte = full page in the address space for each ppgtt pd).
Can you please apply this bikeshed to the patch?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 4/5 v2] drm/i915: Aliased PPGTT size abstraction
2013-01-26 0:41 ` [PATCH 4/5] drm/i915: Aliased PPGTT size abstraction Ben Widawsky
2013-01-28 12:19 ` Jani Nikula
@ 2013-01-28 20:35 ` Ben Widawsky
2013-01-29 12:58 ` Daniel Vetter
1 sibling, 1 reply; 20+ messages in thread
From: Ben Widawsky @ 2013-01-28 20:35 UTC (permalink / raw)
To: Intel GFX; +Cc: Daniel Vetter, Ben Widawsky
Aside from being a nice prep for some work I'm doing in PPGTT, this also
leaves us with a more accurate size in our gtt_total_entries member.
Since we can't actually use the PPGTT reserved part of the GGTT.
This will help some of the existing assertions find issues which would
overwrite the PPGTT PDEs.
Another benefit is for platforms which don't use the entire 2GB GTT,
this will save GTT space. For example if a platform has 1 1GB GTT, this
patch should save 1MB of GTT space (512MB has an even greater savings).
I don't have a platform to easily test this however.
If/when we ever move to real PPGTT, I think allocating the PDEs as
objects will make the most sense.
v2: Change the size calculation for the PDs carved out of the ggtt to be
more explicit. (Daniel)
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
drivers/gpu/drm/i915/i915_drv.h | 3 ++-
drivers/gpu/drm/i915/i915_gem_gtt.c | 35 ++++++++++++++++++++++++++---------
3 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 384f193..4545357 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1605,6 +1605,8 @@ static int i915_ppgtt_info(struct seq_file *m, void *data)
seq_printf(m, "aliasing PPGTT:\n");
seq_printf(m, "pd gtt offset: 0x%08x\n", ppgtt->pd_offset);
+ seq_printf(m, "mapped size: %lldM\n",
+ ppgtt->mapped_size>>20);
}
seq_printf(m, "ECOCHK: 0x%08x\n", I915_READ(GAM_ECOCHK));
mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1af8e73..60ba4ab 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -388,10 +388,11 @@ struct i915_gtt {
struct page *scratch_page;
};
-#define I915_PPGTT_PD_ENTRIES 512
+#define GEN6_MAX_PD_ENTRIES 512
#define I915_PPGTT_PT_ENTRIES 1024
struct i915_hw_ppgtt {
struct drm_device *dev;
+ uint64_t mapped_size;
unsigned num_pd_entries;
struct page **pt_pages;
uint32_t pd_offset;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index f79f427..7d8c2e5 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -108,7 +108,7 @@ static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
}
}
-static int gen6_init_aliasing_ppgtt(struct i915_hw_ppgtt *ppgtt)
+static int gen6_init_ppgtt(struct i915_hw_ppgtt *ppgtt, uint64_t size)
{
struct drm_device *dev = ppgtt->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -116,15 +116,22 @@ static int gen6_init_aliasing_ppgtt(struct i915_hw_ppgtt *ppgtt)
int i;
int ret = -ENOMEM;
+ if (dev_priv->mm.aliasing_ppgtt)
+ return -EBUSY;
+
+ /* Each PDE maps 4MB (1k PAGE_SIZE pages)*/
+ ppgtt->mapped_size = round_up(size, 1<<22);
+ ppgtt->num_pd_entries = ppgtt->mapped_size >> 22;
+ BUG_ON(ppgtt->num_pd_entries > GEN6_MAX_PD_ENTRIES);
+
/* ppgtt PDEs reside in the global gtt pagetable, which has 512*1024
* entries. For aliasing ppgtt support we just steal them at the end for
* now. */
- first_pd_entry_in_global_pt = dev_priv->mm.gtt->gtt_total_entries - I915_PPGTT_PD_ENTRIES;
+ first_pd_entry_in_global_pt = dev_priv->mm.gtt->gtt_total_entries -
+ ppgtt->num_pd_entries;
- ppgtt->num_pd_entries = I915_PPGTT_PD_ENTRIES;
ppgtt->pt_pages = kzalloc(sizeof(struct page *)*ppgtt->num_pd_entries,
GFP_KERNEL);
-
if (!ppgtt->pt_pages)
return -ENOMEM;
@@ -156,7 +163,10 @@ static int gen6_init_aliasing_ppgtt(struct i915_hw_ppgtt *ppgtt)
i915_ppgtt_clear_range(ppgtt, 0,
ppgtt->num_pd_entries*I915_PPGTT_PT_ENTRIES);
- ppgtt->pd_offset = (first_pd_entry_in_global_pt)*sizeof(gtt_pte_t);
+ ppgtt->pd_offset = first_pd_entry_in_global_pt * sizeof(gtt_pte_t);
+
+ DRM_INFO("Allocated %d PDEs for PPGTT\n", ppgtt->num_pd_entries);
+ DRM_DEBUG_DRIVER("First PDE: %d\n", first_pd_entry_in_global_pt);
return 0;
@@ -192,7 +202,7 @@ static int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
ppgtt->dev = dev;
ppgtt->scratch_page_dma_addr = dev_priv->gtt.scratch_page_dma;
- ret = gen6_init_aliasing_ppgtt(ppgtt);
+ ret = gen6_init_ppgtt(ppgtt, dev_priv->gtt.total);
if (ret)
kfree(ppgtt);
else
@@ -625,20 +635,27 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
mappable_size = dev_priv->gtt.mappable_end;
if (intel_enable_ppgtt(dev) && HAS_ALIASING_PPGTT(dev)) {
+ /* Each PDE represents 1k PAGE_SIZE objects */
+ const int reserve_pd_space = (gtt_size >> 22) * PAGE_SIZE;
int ret;
+
+ BUG_ON(!reserve_pd_space);
+
/* PPGTT pdes are stolen from global gtt ptes, so shrink the
* aperture accordingly when using aliasing ppgtt. */
- gtt_size -= I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
+ gtt_size -= reserve_pd_space;
i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
ret = i915_gem_init_aliasing_ppgtt(dev);
- if (!ret)
+ if (!ret) {
+ dev_priv->mm.gtt->gtt_total_entries = gtt_size >> PAGE_SHIFT;
return;
+ }
DRM_ERROR("Aliased PPGTT setup failed %d\n", ret);
drm_mm_takedown(&dev_priv->mm.gtt_space);
- gtt_size += I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
+ gtt_size += reserve_pd_space;
}
i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
}
--
1.8.1.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 4/5 v2] drm/i915: Aliased PPGTT size abstraction
2013-01-28 20:35 ` [PATCH 4/5 v2] " Ben Widawsky
@ 2013-01-29 12:58 ` Daniel Vetter
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2013-01-29 12:58 UTC (permalink / raw)
To: Ben Widawsky; +Cc: Daniel Vetter, Intel GFX
On Mon, Jan 28, 2013 at 12:35:55PM -0800, Ben Widawsky wrote:
> Aside from being a nice prep for some work I'm doing in PPGTT, this also
> leaves us with a more accurate size in our gtt_total_entries member.
> Since we can't actually use the PPGTT reserved part of the GGTT.
>
> This will help some of the existing assertions find issues which would
> overwrite the PPGTT PDEs.
>
> Another benefit is for platforms which don't use the entire 2GB GTT,
> this will save GTT space. For example if a platform has 1 1GB GTT, this
> patch should save 1MB of GTT space (512MB has an even greater savings).
> I don't have a platform to easily test this however.
>
> If/when we ever move to real PPGTT, I think allocating the PDEs as
> objects will make the most sense.
>
> v2: Change the size calculation for the PDs carved out of the ggtt to be
> more explicit. (Daniel)
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
I'm not sold yet on whether this is the interface we want. Furthermore I
have some fading nightmares that maybe the pdes should be naturally
aligned in the GSM. So trying to safe a few of those sounds like a risky
idea. I'll punt on this one for now.
For patch 5 I'd prefer the clearer math over adding a comment (comments
tend to get stale, fast), otherwise all patches of this series merged (or
nothing left after resolving conflicts).
Thanks, Daniel
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> drivers/gpu/drm/i915/i915_gem_gtt.c | 35 ++++++++++++++++++++++++++---------
> 3 files changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 384f193..4545357 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1605,6 +1605,8 @@ static int i915_ppgtt_info(struct seq_file *m, void *data)
>
> seq_printf(m, "aliasing PPGTT:\n");
> seq_printf(m, "pd gtt offset: 0x%08x\n", ppgtt->pd_offset);
> + seq_printf(m, "mapped size: %lldM\n",
> + ppgtt->mapped_size>>20);
> }
> seq_printf(m, "ECOCHK: 0x%08x\n", I915_READ(GAM_ECOCHK));
> mutex_unlock(&dev->struct_mutex);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1af8e73..60ba4ab 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -388,10 +388,11 @@ struct i915_gtt {
> struct page *scratch_page;
> };
>
> -#define I915_PPGTT_PD_ENTRIES 512
> +#define GEN6_MAX_PD_ENTRIES 512
> #define I915_PPGTT_PT_ENTRIES 1024
> struct i915_hw_ppgtt {
> struct drm_device *dev;
> + uint64_t mapped_size;
> unsigned num_pd_entries;
> struct page **pt_pages;
> uint32_t pd_offset;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index f79f427..7d8c2e5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -108,7 +108,7 @@ static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
> }
> }
>
> -static int gen6_init_aliasing_ppgtt(struct i915_hw_ppgtt *ppgtt)
> +static int gen6_init_ppgtt(struct i915_hw_ppgtt *ppgtt, uint64_t size)
> {
> struct drm_device *dev = ppgtt->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -116,15 +116,22 @@ static int gen6_init_aliasing_ppgtt(struct i915_hw_ppgtt *ppgtt)
> int i;
> int ret = -ENOMEM;
>
> + if (dev_priv->mm.aliasing_ppgtt)
> + return -EBUSY;
> +
> + /* Each PDE maps 4MB (1k PAGE_SIZE pages)*/
> + ppgtt->mapped_size = round_up(size, 1<<22);
> + ppgtt->num_pd_entries = ppgtt->mapped_size >> 22;
> + BUG_ON(ppgtt->num_pd_entries > GEN6_MAX_PD_ENTRIES);
> +
> /* ppgtt PDEs reside in the global gtt pagetable, which has 512*1024
> * entries. For aliasing ppgtt support we just steal them at the end for
> * now. */
> - first_pd_entry_in_global_pt = dev_priv->mm.gtt->gtt_total_entries - I915_PPGTT_PD_ENTRIES;
> + first_pd_entry_in_global_pt = dev_priv->mm.gtt->gtt_total_entries -
> + ppgtt->num_pd_entries;
>
> - ppgtt->num_pd_entries = I915_PPGTT_PD_ENTRIES;
> ppgtt->pt_pages = kzalloc(sizeof(struct page *)*ppgtt->num_pd_entries,
> GFP_KERNEL);
> -
> if (!ppgtt->pt_pages)
> return -ENOMEM;
>
> @@ -156,7 +163,10 @@ static int gen6_init_aliasing_ppgtt(struct i915_hw_ppgtt *ppgtt)
> i915_ppgtt_clear_range(ppgtt, 0,
> ppgtt->num_pd_entries*I915_PPGTT_PT_ENTRIES);
>
> - ppgtt->pd_offset = (first_pd_entry_in_global_pt)*sizeof(gtt_pte_t);
> + ppgtt->pd_offset = first_pd_entry_in_global_pt * sizeof(gtt_pte_t);
> +
> + DRM_INFO("Allocated %d PDEs for PPGTT\n", ppgtt->num_pd_entries);
> + DRM_DEBUG_DRIVER("First PDE: %d\n", first_pd_entry_in_global_pt);
>
> return 0;
>
> @@ -192,7 +202,7 @@ static int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
> ppgtt->dev = dev;
> ppgtt->scratch_page_dma_addr = dev_priv->gtt.scratch_page_dma;
>
> - ret = gen6_init_aliasing_ppgtt(ppgtt);
> + ret = gen6_init_ppgtt(ppgtt, dev_priv->gtt.total);
> if (ret)
> kfree(ppgtt);
> else
> @@ -625,20 +635,27 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
> mappable_size = dev_priv->gtt.mappable_end;
>
> if (intel_enable_ppgtt(dev) && HAS_ALIASING_PPGTT(dev)) {
> + /* Each PDE represents 1k PAGE_SIZE objects */
> + const int reserve_pd_space = (gtt_size >> 22) * PAGE_SIZE;
> int ret;
> +
> + BUG_ON(!reserve_pd_space);
> +
> /* PPGTT pdes are stolen from global gtt ptes, so shrink the
> * aperture accordingly when using aliasing ppgtt. */
> - gtt_size -= I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
> + gtt_size -= reserve_pd_space;
>
> i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
>
> ret = i915_gem_init_aliasing_ppgtt(dev);
> - if (!ret)
> + if (!ret) {
> + dev_priv->mm.gtt->gtt_total_entries = gtt_size >> PAGE_SHIFT;
> return;
> + }
>
> DRM_ERROR("Aliased PPGTT setup failed %d\n", ret);
> drm_mm_takedown(&dev_priv->mm.gtt_space);
> - gtt_size += I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
> + gtt_size += reserve_pd_space;
> }
> i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
> }
> --
> 1.8.1.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 5/5] drm/i915: Dynamically calculate dclv
2013-01-26 0:41 [PATCH 0/5] [REPOST] GTT cleanups, rebased Ben Widawsky
` (3 preceding siblings ...)
2013-01-26 0:41 ` [PATCH 4/5] drm/i915: Aliased PPGTT size abstraction Ben Widawsky
@ 2013-01-26 0:41 ` Ben Widawsky
2013-01-28 12:20 ` Jani Nikula
` (2 more replies)
2013-01-28 12:24 ` [PATCH 0/5] [REPOST] GTT cleanups, rebased Jani Nikula
5 siblings, 3 replies; 20+ messages in thread
From: Ben Widawsky @ 2013-01-26 0:41 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
Directory CacheLine Valid controls which PDEs are held in the directory
cache. Each bit represents 16 PDEs (16 PDEs at 4 bytes per entry is 1
cacheline, and therefore, almost makes sense).
Since we can now have an aliasing PPGTT which isn't 2GB, theoretically,
we should also modify the DCLV value to fit accordingly.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
drivers/gpu/drm/i915/i915_reg.h | 1 -
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index d9af815..168583b 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -344,11 +344,12 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
}
for_each_ring(ring, dev_priv, i) {
+ uint64_t dclv = ppgtt->mapped_size >> 26;
+ dclv = (1ULL << dclv) - 1;
if (INTEL_INFO(dev)->gen >= 7)
I915_WRITE(RING_MODE_GEN7(ring),
_MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
-
- I915_WRITE(RING_PP_DIR_DCLV(ring), PP_DIR_DCLV_2G);
+ I915_WRITE(RING_PP_DIR_DCLV(ring), dclv);
I915_WRITE(RING_PP_DIR_BASE(ring), pd_offset);
}
}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 94a08b9..780a715 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -117,7 +117,6 @@
#define RING_PP_DIR_BASE(ring) ((ring)->mmio_base+0x228)
#define RING_PP_DIR_BASE_READ(ring) ((ring)->mmio_base+0x518)
#define RING_PP_DIR_DCLV(ring) ((ring)->mmio_base+0x220)
-#define PP_DIR_DCLV_2G 0xffffffff
#define GAM_ECOCHK 0x4090
#define ECOCHK_SNB_BIT (1<<10)
--
1.8.1.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 5/5] drm/i915: Dynamically calculate dclv
2013-01-26 0:41 ` [PATCH 5/5] drm/i915: Dynamically calculate dclv Ben Widawsky
@ 2013-01-28 12:20 ` Jani Nikula
2013-01-28 20:36 ` Ben Widawsky
2013-01-28 21:08 ` Daniel Vetter
2 siblings, 0 replies; 20+ messages in thread
From: Jani Nikula @ 2013-01-28 12:20 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
On Sat, 26 Jan 2013, Ben Widawsky <ben@bwidawsk.net> wrote:
> Directory CacheLine Valid controls which PDEs are held in the directory
> cache. Each bit represents 16 PDEs (16 PDEs at 4 bytes per entry is 1
> cacheline, and therefore, almost makes sense).
>
> Since we can now have an aliasing PPGTT which isn't 2GB, theoretically,
> we should also modify the DCLV value to fit accordingly.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
> drivers/gpu/drm/i915/i915_reg.h | 1 -
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index d9af815..168583b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -344,11 +344,12 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
> }
>
> for_each_ring(ring, dev_priv, i) {
> + uint64_t dclv = ppgtt->mapped_size >> 26;
> + dclv = (1ULL << dclv) - 1;
Maybe add a comment with some of the info from the commit message here?
BR,
Jani.
> if (INTEL_INFO(dev)->gen >= 7)
> I915_WRITE(RING_MODE_GEN7(ring),
> _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
> -
> - I915_WRITE(RING_PP_DIR_DCLV(ring), PP_DIR_DCLV_2G);
> + I915_WRITE(RING_PP_DIR_DCLV(ring), dclv);
> I915_WRITE(RING_PP_DIR_BASE(ring), pd_offset);
> }
> }
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 94a08b9..780a715 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -117,7 +117,6 @@
> #define RING_PP_DIR_BASE(ring) ((ring)->mmio_base+0x228)
> #define RING_PP_DIR_BASE_READ(ring) ((ring)->mmio_base+0x518)
> #define RING_PP_DIR_DCLV(ring) ((ring)->mmio_base+0x220)
> -#define PP_DIR_DCLV_2G 0xffffffff
>
> #define GAM_ECOCHK 0x4090
> #define ECOCHK_SNB_BIT (1<<10)
> --
> 1.8.1.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH 5/5] drm/i915: Dynamically calculate dclv
2013-01-26 0:41 ` [PATCH 5/5] drm/i915: Dynamically calculate dclv Ben Widawsky
2013-01-28 12:20 ` Jani Nikula
@ 2013-01-28 20:36 ` Ben Widawsky
2013-01-28 21:08 ` Daniel Vetter
2 siblings, 0 replies; 20+ messages in thread
From: Ben Widawsky @ 2013-01-28 20:36 UTC (permalink / raw)
To: Intel GFX; +Cc: Ben Widawsky
Directory CacheLine Valid controls which PDEs are held in the directory
cache. Each bit represents 16 PDEs (16 PDEs at 4 bytes per entry is 1
cacheline, and therefore, almost makes sense).
Since we can now have an aliasing PPGTT which isn't 2GB, theoretically,
we should also modify the DCLV value to fit accordingly.
v2: Add part of this commit message to the code as a comment (Jani)
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++++--
drivers/gpu/drm/i915/i915_reg.h | 1 -
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 7d8c2e5..2fc866a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -344,11 +344,14 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
}
for_each_ring(ring, dev_priv, i) {
+ /* Directory CacheLine Valid controls which PDEs are held in the
+ * directory cache. Each bit represents 16 PDEs */
+ uint64_t dclv = ppgtt->mapped_size >> 26;
+ dclv = (1ULL << dclv) - 1;
if (INTEL_INFO(dev)->gen >= 7)
I915_WRITE(RING_MODE_GEN7(ring),
_MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
-
- I915_WRITE(RING_PP_DIR_DCLV(ring), PP_DIR_DCLV_2G);
+ I915_WRITE(RING_PP_DIR_DCLV(ring), dclv);
I915_WRITE(RING_PP_DIR_BASE(ring), pd_offset);
}
}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0c89cf5..c5ac375 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -117,7 +117,6 @@
#define RING_PP_DIR_BASE(ring) ((ring)->mmio_base+0x228)
#define RING_PP_DIR_BASE_READ(ring) ((ring)->mmio_base+0x518)
#define RING_PP_DIR_DCLV(ring) ((ring)->mmio_base+0x220)
-#define PP_DIR_DCLV_2G 0xffffffff
#define GAM_ECOCHK 0x4090
#define ECOCHK_SNB_BIT (1<<10)
--
1.8.1.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 5/5] drm/i915: Dynamically calculate dclv
2013-01-26 0:41 ` [PATCH 5/5] drm/i915: Dynamically calculate dclv Ben Widawsky
2013-01-28 12:20 ` Jani Nikula
2013-01-28 20:36 ` Ben Widawsky
@ 2013-01-28 21:08 ` Daniel Vetter
2 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2013-01-28 21:08 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Fri, Jan 25, 2013 at 04:41:07PM -0800, Ben Widawsky wrote:
> Directory CacheLine Valid controls which PDEs are held in the directory
> cache. Each bit represents 16 PDEs (16 PDEs at 4 bytes per entry is 1
> cacheline, and therefore, almost makes sense).
>
> Since we can now have an aliasing PPGTT which isn't 2GB, theoretically,
> we should also modify the DCLV value to fit accordingly.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
> drivers/gpu/drm/i915/i915_reg.h | 1 -
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index d9af815..168583b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -344,11 +344,12 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
> }
>
> for_each_ring(ring, dev_priv, i) {
> + uint64_t dclv = ppgtt->mapped_size >> 26;
ppgtt->num_pd_entries / 16? Would be much more self-documenting than the
almost magic >> 26 ...
-Daniel
> + dclv = (1ULL << dclv) - 1;
> if (INTEL_INFO(dev)->gen >= 7)
> I915_WRITE(RING_MODE_GEN7(ring),
> _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
> -
> - I915_WRITE(RING_PP_DIR_DCLV(ring), PP_DIR_DCLV_2G);
> + I915_WRITE(RING_PP_DIR_DCLV(ring), dclv);
> I915_WRITE(RING_PP_DIR_BASE(ring), pd_offset);
> }
> }
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 94a08b9..780a715 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -117,7 +117,6 @@
> #define RING_PP_DIR_BASE(ring) ((ring)->mmio_base+0x228)
> #define RING_PP_DIR_BASE_READ(ring) ((ring)->mmio_base+0x518)
> #define RING_PP_DIR_DCLV(ring) ((ring)->mmio_base+0x220)
> -#define PP_DIR_DCLV_2G 0xffffffff
>
> #define GAM_ECOCHK 0x4090
> #define ECOCHK_SNB_BIT (1<<10)
> --
> 1.8.1.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] [REPOST] GTT cleanups, rebased
2013-01-26 0:41 [PATCH 0/5] [REPOST] GTT cleanups, rebased Ben Widawsky
` (4 preceding siblings ...)
2013-01-26 0:41 ` [PATCH 5/5] drm/i915: Dynamically calculate dclv Ben Widawsky
@ 2013-01-28 12:24 ` Jani Nikula
5 siblings, 0 replies; 20+ messages in thread
From: Jani Nikula @ 2013-01-28 12:24 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
On Sat, 26 Jan 2013, Ben Widawsky <ben@bwidawsk.net> wrote:
> This work sort of led me on the gtt vtable distraction which ended in Daniel
> doing his own version. I posted these originally about a month ago. I've
> rebased them on dinq. I'd like to see them get merged since they still help
> with some other work I'm doing on PPGTT.
>
> Some of this work does conflict a bit with the gtt vtable stuff.
>
> I did a decent amount of testing on the original post, less so on this rebased
> version.
On the series, with my limited exposure to (pp)gtt,
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
> Ben Widawsky (5):
> drm/i915: trivial: kill-agp collateral cleanups
> drm/i915: Reclaim GTT space for failed PPGTT
> drm/i915: Extract gen6 aliasing ppgtt code
> drm/i915: Aliased PPGTT size abstraction
> drm/i915: Dynamically calculate dclv
>
> drivers/gpu/drm/i915/i915_debugfs.c | 2 +
> drivers/gpu/drm/i915/i915_drv.h | 6 +--
> drivers/gpu/drm/i915/i915_gem_gtt.c | 101 +++++++++++++++++++++++-------------
> drivers/gpu/drm/i915/i915_reg.h | 1 -
> 4 files changed, 71 insertions(+), 39 deletions(-)
>
> --
> 1.8.1.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread