Re: [PATCH] drm/i915: Rename to GEN8_LEGACY_PDPES

From: Daniel Vetter <daniel@ffwll.ch>
To: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Intel GFX <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/i915: Rename to GEN8_LEGACY_PDPES
Date: Wed, 9 Apr 2014 14:56:54 +0200	[thread overview]
Message-ID: <20140409125654.GK9262@phenom.ffwll.local> (raw)
In-Reply-To: <1396895956-11584-1-git-send-email-benjamin.widawsky@intel.com>

On Mon, Apr 07, 2014 at 11:39:16AM -0700, Ben Widawsky wrote:
> In gen8, 32b PPGTT has always had one "pdp" (it doesn't actually have
> one, but it resembles having one). The #define was confusing as is, and
> using "PDPE" is a much better description.
> 
> sed -i 's/GEN8_LEGACY_PDPS/GEN8_LEGACY_PDPES/' drivers/gpu/drm/i915/*.[ch]
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

I have to admit that I still don't really see the finer disdinction here.
Call me dense, but can you please supply expansions of pdp and pdpe?

Thus far I've simply looked at what the code does when reviewing ppgtt
code ;-)
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 6 +++---
>  drivers/gpu/drm/i915/i915_gem_gtt.h | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 584e24b..12fa8a5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -301,7 +301,7 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
>  	pt_vaddr = NULL;
>  
>  	for_each_sg_page(pages->sgl, &sg_iter, pages->nents, 0) {
> -		if (WARN_ON(pdpe >= GEN8_LEGACY_PDPS))
> +		if (WARN_ON(pdpe >= GEN8_LEGACY_PDPES))
>  			break;
>  
>  		if (pt_vaddr == NULL)
> @@ -410,7 +410,7 @@ bail:
>  static int gen8_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt,
>  					   const int max_pdp)
>  {
> -	struct page **pt_pages[GEN8_LEGACY_PDPS];
> +	struct page **pt_pages[GEN8_LEGACY_PDPES];
>  	int i, ret;
>  
>  	for (i = 0; i < max_pdp; i++) {
> @@ -462,7 +462,7 @@ static int gen8_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt,
>  		return -ENOMEM;
>  
>  	ppgtt->num_pd_pages = 1 << get_order(max_pdp << PAGE_SHIFT);
> -	BUG_ON(ppgtt->num_pd_pages > GEN8_LEGACY_PDPS);
> +	BUG_ON(ppgtt->num_pd_pages > GEN8_LEGACY_PDPES);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index b5e8ac0..220e9de 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -86,7 +86,7 @@ typedef gen8_gtt_pte_t gen8_ppgtt_pde_t;
>  #define GEN8_PDE_MASK			0x1ff
>  #define GEN8_PTE_SHIFT			12
>  #define GEN8_PTE_MASK			0x1ff
> -#define GEN8_LEGACY_PDPS		4
> +#define GEN8_LEGACY_PDPES		4
>  #define GEN8_PTES_PER_PAGE		(PAGE_SIZE / sizeof(gen8_gtt_pte_t))
>  #define GEN8_PDES_PER_PAGE		(PAGE_SIZE / sizeof(gen8_ppgtt_pde_t))
>  
> @@ -244,12 +244,12 @@ struct i915_hw_ppgtt {
>  	unsigned num_pd_pages; /* gen8+ */
>  	union {
>  		struct page **pt_pages;
> -		struct page **gen8_pt_pages[GEN8_LEGACY_PDPS];
> +		struct page **gen8_pt_pages[GEN8_LEGACY_PDPES];
>  	};
>  	struct page *pd_pages;
>  	union {
>  		uint32_t pd_offset;
> -		dma_addr_t pd_dma_addr[GEN8_LEGACY_PDPS];
> +		dma_addr_t pd_dma_addr[GEN8_LEGACY_PDPES];
>  	};
>  	union {
>  		dma_addr_t *pt_dma_addr;
> -- 
> 1.9.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