| Message ID | 1366784140-2670-4-git-send-email-ben@bwidawsk.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 23 Apr 2013 23:15:31 -0700 Ben Widawsky <ben@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 <ben@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 <jbarnes@virtuousgeek.org>
On Thu, May 02, 2013 at 02:26:17PM -0700, Jesse Barnes wrote: > On Tue, 23 Apr 2013 23:15:31 -0700 > Ben Widawsky <ben@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 <ben@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 <jbarnes@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
On Thu, 2 May 2013 15:49:17 -0700 Ben Widawsky <ben@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 <ben@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 <ben@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 <jbarnes@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.
On Thu, May 02, 2013 at 02:26:17PM -0700, Jesse Barnes wrote: > On Tue, 23 Apr 2013 23:15:31 -0700 > Ben Widawsky <ben@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 <ben@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 <jbarnes@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
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 <ben@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 <ben@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 <jbarnes@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
On Wed, May 8, 2013 at 6:49 PM, Ben Widawsky <ben@bwidawsk.net> wrote: >> 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? Yeah, maybe call it GEN6_ instead of I915_ while at it ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
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); }
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. 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 <ben@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(-)