[Intel-gfx] [RFC 21/28] drm/i915/gtt: Reduce source verbosity by caching repeated dereferences

Chris Wilson chris at chris-wilson.co.uk
Thu Jun 13 14:12:28 UTC 2019


Quoting Tvrtko Ursulin (2019-06-13 14:35:32)
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> There is a lot of code in i915_gem_gtt.c which repeatadly dereferences
> either ggtt or ppgtt in order to get to the vm. Cache those accesses in
> local variables for better readability.

There isn't a dereference though, it's just using the base struct. Meh.

I don't really mind, but I chose to write it the other way, specifically
using vm to keep it short.

At the end of the day, the compiler *should* eliminate the redundant
local, so it is merely a matter of which readers prefer. I think I still
have a slight preference to using ppgtt throughout rather than mixing
ppgtt and vm for the same object.

> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 254 +++++++++++++++-------------
>  1 file changed, 134 insertions(+), 120 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 516ffc4a521a..d09a4d9b71da 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1004,10 +1004,11 @@ gen8_ppgtt_insert_pte_entries(struct i915_ppgtt *ppgtt,
>  {
>         struct i915_page_directory *pd;
>         const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags);
> +       struct i915_address_space *vm = &ppgtt->vm;
>         gen8_pte_t *vaddr;
>         bool ret;
>  
> -       GEM_BUG_ON(idx->pdpe >= i915_pdpes_per_pdp(&ppgtt->vm));
> +       GEM_BUG_ON(idx->pdpe >= i915_pdpes_per_pdp(vm));
>         pd = pdp->page_directory[idx->pdpe];
>         vaddr = kmap_atomic_px(pd->page_table[idx->pde]);
>         do {
> @@ -1038,7 +1039,7 @@ gen8_ppgtt_insert_pte_entries(struct i915_ppgtt *ppgtt,
>                                         break;
>                                 }
>  
> -                               GEM_BUG_ON(idx->pdpe >= i915_pdpes_per_pdp(&ppgtt->vm));
> +                               GEM_BUG_ON(idx->pdpe >= i915_pdpes_per_pdp(vm));

I don't see any code here. :-p
-Chris


More information about the Intel-gfx mailing list