Re: [PATCH 04/10] drm/i915: Extract PPGTT pte encoding

From: Jesse Barnes <jbarnes@virtuousgeek.org>
To: Ben Widawsky <ben@bwidawsk.net>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 04/10] drm/i915: Extract PPGTT pte encoding
Date: Thu, 25 Oct 2012 13:50:31 -0700	[thread overview]
Message-ID: <20121025135031.1cbba393@jbarnes-desktop> (raw)
In-Reply-To: <1350956055-3224-5-git-send-email-ben@bwidawsk.net>

On Mon, 22 Oct 2012 18:34:09 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:

> HSW will change the PTE encoding, and laying this out now will be
> helpful when we're ready to implement that. More importantly, GGTT and
> PPGTT PTE encoding is quite similar, so moving this out into a helper
> function will enable us to lance the AGP layer.
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index a769b3c..da9c1fa 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -31,6 +31,17 @@
>  
>  typedef uint32_t gtt_pte_t;
>  
> +static inline gtt_pte_t pte_encode(struct drm_device *dev,
> +				   dma_addr_t addr,
> +				   gtt_pte_t cache_bits)
> +{
> +	gtt_pte_t pte = GEN6_PTE_VALID;
> +	pte |= GEN6_PTE_ADDR_ENCODE(addr);
> +	pte |= cache_bits;
> +
> +	return pte;
> +}
> +
>  /* PPGTT support for Sandybdrige/Gen6 and later */
>  static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
>  				   unsigned first_entry,
> @@ -42,8 +53,8 @@ static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
>  	unsigned first_pte = first_entry % I915_PPGTT_PT_ENTRIES;
>  	unsigned last_pte, i;
>  
> -	scratch_pte = GEN6_PTE_ADDR_ENCODE(ppgtt->scratch_page_dma_addr);
> -	scratch_pte |= GEN6_PTE_VALID | GEN6_PTE_CACHE_LLC;
> +	scratch_pte = pte_encode(ppgtt->dev, ppgtt->scratch_page_dma_addr,
> +				 GEN6_PTE_CACHE_LLC);
>  
>  	while (num_entries) {
>  		last_pte = first_pte + num_entries;
> @@ -174,7 +185,7 @@ static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
>  					 unsigned first_entry,
>  					 gtt_pte_t pte_flags)
>  {
> -	gtt_pte_t *pt_vaddr, pte;
> +	gtt_pte_t *pt_vaddr;
>  	unsigned act_pd = first_entry / I915_PPGTT_PT_ENTRIES;
>  	unsigned first_pte = first_entry % I915_PPGTT_PT_ENTRIES;
>  	unsigned i, j, m, segment_len;
> @@ -192,8 +203,8 @@ static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
>  
>  		for (j = first_pte; j < I915_PPGTT_PT_ENTRIES; j++) {
>  			page_addr = sg_dma_address(sg) + (m << PAGE_SHIFT);
> -			pte = GEN6_PTE_ADDR_ENCODE(page_addr);
> -			pt_vaddr[j] = pte | pte_flags;
> +			pt_vaddr[j] = pte_encode(ppgtt->dev, page_addr,
> +						 pte_flags);
>  
>  			/* grab the next page */
>  			if (++m == segment_len) {

Does this mean we can remove the GEN6_PTE_ADDR_ENCODE macro too?

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center