Re: [Intel-gfx] [PATCH 03/12] drm/i915: make PDE|PTE platform specific
On Mon, May 06, 2013 at 11:47:42AM +0200, Daniel Vetter wrote:
On Thu, May 02, 2013 at 02:26:17PM -0700, Jesse Barnes wrote:
On Tue, 23 Apr 2013 23:15:31 -0700
Ben Widawsky b...@bwidawsk.net wrote:
Accomplish this be removing the PDE count define which is (and has
always been) part of the PPGTT structure anyway. With the addition of
the gen specific init function, we can nicely tuck away the magic number
in there.
In this vain, make the PTE define less of a magic number.
vein
The remaining code in the global gtt setup is a bit messy, but an
upcoming patch will clean that one up.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_drv.h | 2 --
drivers/gpu/drm/i915/i915_gem_gtt.c | 10 +-
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index d5dcf7f..0a9c7cd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -431,8 +431,6 @@ struct i915_gtt {
};
#define gtt_total_entries(gtt) ((gtt).total PAGE_SHIFT)
-#define I915_PPGTT_PD_ENTRIES 512
-#define I915_PPGTT_PT_ENTRIES 1024
struct i915_hw_ppgtt {
struct drm_device *dev;
unsigned num_pd_entries;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0503f09..11a50cf 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -29,6 +29,7 @@
#include intel_drv.h
typedef uint32_t gen6_gtt_pte_t;
+#define I915_PPGTT_PT_ENTRIES (PAGE_SIZE / sizeof(gen6_gtt_pte_t))
/* PPGTT stuff */
#define GEN6_GTT_ADDR_ENCODE(addr) ((addr) | (((addr) 28)
0xff0))
@@ -235,10 +236,9 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt
*ppgtt)
/* 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 =
- gtt_total_entries(dev_priv-gtt) - I915_PPGTT_PD_ENTRIES;
+ first_pd_entry_in_global_pt = gtt_total_entries(dev_priv-gtt) - 512;
- ppgtt-num_pd_entries = I915_PPGTT_PD_ENTRIES;
+ ppgtt-num_pd_entries = 512;
ppgtt-enable = gen6_ppgtt_enable;
ppgtt-clear_range = gen6_ppgtt_clear_range;
ppgtt-insert_entries = gen6_ppgtt_insert_entries;
@@ -646,7 +646,7 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
if (INTEL_INFO(dev)-gen = 7) {
/* 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 -= 512 * PAGE_SIZE;
}
i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
@@ -657,7 +657,7 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
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 += 512 * PAGE_SIZE;
}
i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
}
I thought we could have 1024 PDEs? Either way, this just drops the
macro, which is fine, unless we want to increase our PDE count.
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
I've written this code, so I know what the magic 512 is all about. But
since Jesse is already confused about them I don't like removing the
#define that much. Heck, we even tend to use PAGE_SIZE instead of 4096.
What about a GEN6_ prefix instead? Or if you want to make this dynamic
(iirc we can allocated PDEs in groups of 32) call it _MAX or so?
I like the PT entries part though.
-Daniel
If I just bring back #define I915_PPGTT_PD_ENTRIES 512, does this make
you sufficient happy to accept the patch?
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
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: [Intel-gfx] [PATCH 03/12] drm/i915: make PDE|PTE platform specific
On Tue, 23 Apr 2013 23:15:31 -0700
Ben Widawsky b...@bwidawsk.net wrote:
Accomplish this be removing the PDE count define which is (and has
always been) part of the PPGTT structure anyway. With the addition of
the gen specific init function, we can nicely tuck away the magic number
in there.
In this vain, make the PTE define less of a magic number.
vein
The remaining code in the global gtt setup is a bit messy, but an
upcoming patch will clean that one up.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_drv.h | 2 --
drivers/gpu/drm/i915/i915_gem_gtt.c | 10 +-
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d5dcf7f..0a9c7cd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -431,8 +431,6 @@ struct i915_gtt {
};
#define gtt_total_entries(gtt) ((gtt).total PAGE_SHIFT)
-#define I915_PPGTT_PD_ENTRIES 512
-#define I915_PPGTT_PT_ENTRIES 1024
struct i915_hw_ppgtt {
struct drm_device *dev;
unsigned num_pd_entries;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0503f09..11a50cf 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -29,6 +29,7 @@
#include intel_drv.h
typedef uint32_t gen6_gtt_pte_t;
+#define I915_PPGTT_PT_ENTRIES (PAGE_SIZE / sizeof(gen6_gtt_pte_t))
/* PPGTT stuff */
#define GEN6_GTT_ADDR_ENCODE(addr) ((addr) | (((addr) 28) 0xff0))
@@ -235,10 +236,9 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
/* 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 =
- gtt_total_entries(dev_priv-gtt) - I915_PPGTT_PD_ENTRIES;
+ first_pd_entry_in_global_pt = gtt_total_entries(dev_priv-gtt) - 512;
- ppgtt-num_pd_entries = I915_PPGTT_PD_ENTRIES;
+ ppgtt-num_pd_entries = 512;
ppgtt-enable = gen6_ppgtt_enable;
ppgtt-clear_range = gen6_ppgtt_clear_range;
ppgtt-insert_entries = gen6_ppgtt_insert_entries;
@@ -646,7 +646,7 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
if (INTEL_INFO(dev)-gen = 7) {
/* 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 -= 512 * PAGE_SIZE;
}
i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
@@ -657,7 +657,7 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
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 += 512 * PAGE_SIZE;
}
i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
}
I thought we could have 1024 PDEs? Either way, this just drops the
macro, which is fine, unless we want to increase our PDE count.
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/12] drm/i915: make PDE|PTE platform specific
On Thu, May 02, 2013 at 02:26:17PM -0700, Jesse Barnes wrote:
On Tue, 23 Apr 2013 23:15:31 -0700
Ben Widawsky b...@bwidawsk.net wrote:
Accomplish this be removing the PDE count define which is (and has
always been) part of the PPGTT structure anyway. With the addition of
the gen specific init function, we can nicely tuck away the magic number
in there.
In this vain, make the PTE define less of a magic number.
vein
The remaining code in the global gtt setup is a bit messy, but an
upcoming patch will clean that one up.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_drv.h | 2 --
drivers/gpu/drm/i915/i915_gem_gtt.c | 10 +-
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index d5dcf7f..0a9c7cd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -431,8 +431,6 @@ struct i915_gtt {
};
#define gtt_total_entries(gtt) ((gtt).total PAGE_SHIFT)
-#define I915_PPGTT_PD_ENTRIES 512
-#define I915_PPGTT_PT_ENTRIES 1024
struct i915_hw_ppgtt {
struct drm_device *dev;
unsigned num_pd_entries;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0503f09..11a50cf 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -29,6 +29,7 @@
#include intel_drv.h
typedef uint32_t gen6_gtt_pte_t;
+#define I915_PPGTT_PT_ENTRIES (PAGE_SIZE / sizeof(gen6_gtt_pte_t))
/* PPGTT stuff */
#define GEN6_GTT_ADDR_ENCODE(addr) ((addr) | (((addr) 28) 0xff0))
@@ -235,10 +236,9 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
/* 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 =
- gtt_total_entries(dev_priv-gtt) - I915_PPGTT_PD_ENTRIES;
+ first_pd_entry_in_global_pt = gtt_total_entries(dev_priv-gtt) - 512;
- ppgtt-num_pd_entries = I915_PPGTT_PD_ENTRIES;
+ ppgtt-num_pd_entries = 512;
ppgtt-enable = gen6_ppgtt_enable;
ppgtt-clear_range = gen6_ppgtt_clear_range;
ppgtt-insert_entries = gen6_ppgtt_insert_entries;
@@ -646,7 +646,7 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
if (INTEL_INFO(dev)-gen = 7) {
/* 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 -= 512 * PAGE_SIZE;
}
i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
@@ -657,7 +657,7 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
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 += 512 * PAGE_SIZE;
}
i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
}
I thought we could have 1024 PDEs? Either way, this just drops the
macro, which is fine, unless we want to increase our PDE count.
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
Thanks for the review.
I think the docs must be wrong if they say this. 512PDEs are all you
need to map a 2GB address space (which is the max), and furthermore, the
DCLV which is used to indicate the size of the PDEs can only allow up to 2GB.
--
Jesse Barnes, Intel Open Source Technology Center
--
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: [Intel-gfx] [PATCH 03/12] drm/i915: make PDE|PTE platform specific
On Thu, 2 May 2013 15:49:17 -0700
Ben Widawsky b...@bwidawsk.net wrote:
On Thu, May 02, 2013 at 02:26:17PM -0700, Jesse Barnes wrote:
On Tue, 23 Apr 2013 23:15:31 -0700
Ben Widawsky b...@bwidawsk.net wrote:
Accomplish this be removing the PDE count define which is (and has
always been) part of the PPGTT structure anyway. With the addition of
the gen specific init function, we can nicely tuck away the magic number
in there.
In this vain, make the PTE define less of a magic number.
vein
The remaining code in the global gtt setup is a bit messy, but an
upcoming patch will clean that one up.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_drv.h | 2 --
drivers/gpu/drm/i915/i915_gem_gtt.c | 10 +-
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index d5dcf7f..0a9c7cd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -431,8 +431,6 @@ struct i915_gtt {
};
#define gtt_total_entries(gtt) ((gtt).total PAGE_SHIFT)
-#define I915_PPGTT_PD_ENTRIES 512
-#define I915_PPGTT_PT_ENTRIES 1024
struct i915_hw_ppgtt {
struct drm_device *dev;
unsigned num_pd_entries;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0503f09..11a50cf 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -29,6 +29,7 @@
#include intel_drv.h
typedef uint32_t gen6_gtt_pte_t;
+#define I915_PPGTT_PT_ENTRIES (PAGE_SIZE / sizeof(gen6_gtt_pte_t))
/* PPGTT stuff */
#define GEN6_GTT_ADDR_ENCODE(addr) ((addr) | (((addr) 28)
0xff0))
@@ -235,10 +236,9 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt
*ppgtt)
/* 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 =
- gtt_total_entries(dev_priv-gtt) - I915_PPGTT_PD_ENTRIES;
+ first_pd_entry_in_global_pt = gtt_total_entries(dev_priv-gtt) - 512;
- ppgtt-num_pd_entries = I915_PPGTT_PD_ENTRIES;
+ ppgtt-num_pd_entries = 512;
ppgtt-enable = gen6_ppgtt_enable;
ppgtt-clear_range = gen6_ppgtt_clear_range;
ppgtt-insert_entries = gen6_ppgtt_insert_entries;
@@ -646,7 +646,7 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
if (INTEL_INFO(dev)-gen = 7) {
/* 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 -= 512 * PAGE_SIZE;
}
i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
@@ -657,7 +657,7 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
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 += 512 * PAGE_SIZE;
}
i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
}
I thought we could have 1024 PDEs? Either way, this just drops the
macro, which is fine, unless we want to increase our PDE count.
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
Thanks for the review.
I think the docs must be wrong if they say this. 512PDEs are all you
need to map a 2GB address space (which is the max), and furthermore, the
DCLV which is used to indicate the size of the PDEs can only allow up to 2GB.
Agreed, one of the diagrams indicates a 4k set of PDEs, but the DCLV
reg says 32 * 16 entries. So the 512 is fine.
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
