Quoting Tvrtko Ursulin (2019-06-13 14:35:32)
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> ---
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
