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 at intel.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Signed-off-by: Ben Widawsky <ben at 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
