Re: [PATCH 20/62] drm/i915/bdw: Add GTT functions

Bloomfield, Jon
2013-11-06T08:28:35Z

> -----Original Message-----
> From: intel-gfx-bounces@lists.freedesktop.org [mailto:intel-gfx-
> bounces@lists.freedesktop.org] On Behalf Of Ben Widawsky
> Sent: Sunday, November 03, 2013 4:07 AM
> To: Intel GFX
> Cc: Daniel Vetter; Ben Widawsky; Widawsky, Benjamin
> Subject: [Intel-gfx] [PATCH 20/62] drm/i915/bdw: Add GTT functions
>
> With the PTE clarifications, the bind and clear functions can now be added for
> gen8.
>
> v2: Use for_each_sg_pages in gen8_ggtt_insert_entries.
>
> v3: Drop dev argument to pte encode functions, upstream lost it. Also rebase
> on top of the scratch page movement.
>
> v4: Rebase on top of the new address space vfuncs.
>
> v5: Add the bool use_scratch argument to clear_range and the bool valid
> argument to the PTE encode function to follow upstream changes.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> (v1)
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 88
> +++++++++++++++++++++++++++++++++++--
> 1 file changed, 85 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 8bf2184..df992dc 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -58,6 +58,15 @@ typedef uint64_t gen8_gtt_pte_t;
> #define HSW_WB_ELLC_LLC_AGE0
> HSW_CACHEABILITY_CONTROL(0xb)
> #define HSW_WT_ELLC_LLC_AGE0
> HSW_CACHEABILITY_CONTROL(0x6)
>
> +static inline gen8_gtt_pte_t gen8_pte_encode(dma_addr_t addr,
> + enum i915_cache_level level,
> + bool valid)
> +{
> + gen8_gtt_pte_t pte = valid ? _PAGE_PRESENT | _PAGE_RW : 0;
> + pte |= addr;
> + return pte;
> +}
> +
> static gen6_gtt_pte_t snb_pte_encode(dma_addr_t addr,
> enum i915_cache_level level,
> bool valid)
> @@ -576,6 +585,56 @@ int i915_gem_gtt_prepare_object(struct
> drm_i915_gem_object *obj)
> return 0;
> }
>
> +static inline void gen8_set_pte(void __iomem *addr, gen8_gtt_pte_t pte)
> +{ #ifdef writeq
> + writeq(pte, addr);
> +#else
> + iowrite32((u32)pte, addr);
> + iowrite32(pte >> 32, addr + 4);
> +#endif
> +}
> +
> +static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
> + struct sg_table *st,
> + unsigned int first_entry,
> + enum i915_cache_level level)
> +{
> + struct drm_i915_private *dev_priv = vm->dev->dev_private;
> + gen8_gtt_pte_t __iomem *gtt_entries =
> + (gen8_gtt_pte_t __iomem *)dev_priv->gtt.gsm +
> first_entry;
> + int i = 0;
> + struct sg_page_iter sg_iter;
> + dma_addr_t addr;
> +
> + for_each_sg_page(st->sgl, &sg_iter, st->nents, 0) {
> + addr = sg_dma_address(sg_iter.sg) +
> + (sg_iter.sg_pgoffset << PAGE_SHIFT);
> + gen8_set_pte(&gtt_entries[i],
> + gen8_pte_encode(addr, level, true));
> + i++;
> + }
> +
> + /* XXX: This serves as a posting read to make sure that the PTE has
> + * actually been updated. There is some concern that even though
> + * registers and PTEs are within the same BAR that they are
> potentially
> + * of NUMA access patterns. Therefore, even with the way we
> assume
> + * hardware should work, we must keep this posting read for
> paranoia.
> + */
> + if (i != 0)
> + WARN_ON(readl(&gtt_entries[i-1])
> + != gen8_pte_encode(addr, level, true));
Comparing a u32 with a 64-bit page-table entry ?