[Intel-gfx] [PATCH] drm/i915: Add ppgtt->kunmap_page_dma vfunc
Daniele Ceraolo Spurio
daniele.ceraolospurio at intel.com
Wed May 18 11:53:29 UTC 2016
On 17/05/16 14:34, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>
> Rather than asking itself "am I a Broadwell, am I a Cherryview,
> or am I neither of the two" on on low level page table operations,
> like inserting and clearing PTEs; add a new vfunc kunmap_page_dma
> and set it to appropriate flavour at ppgtt init time.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 51 +++++++++++++++++++++++--------------
> drivers/gpu/drm/i915/i915_gem_gtt.h | 1 +
> 2 files changed, 33 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 7eab619a3eb2..dc7e128d7483 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -361,29 +361,30 @@ static void *kmap_page_dma(struct i915_page_dma *p)
> return kmap_atomic(p->page);
> }
>
> +static void kunmap_page_dma(void *vaddr)
> +{
> + kunmap_atomic(vaddr);
> +}
> +
> /* We use the flushing unmap only with ppgtt structures:
> * page directories, page tables and scratch pages.
> */
> -static void kunmap_page_dma(struct drm_device *dev, void *vaddr)
> +static void kunmap_page_dma_flush(void *vaddr)
> {
> - /* There are only few exceptions for gen >=6. chv and bxt.
> - * And we are not sure about the latter so play safe for now.
> - */
> - if (IS_CHERRYVIEW(dev) || IS_BROXTON(dev))
> - drm_clflush_virt_range(vaddr, PAGE_SIZE);
> + drm_clflush_virt_range(vaddr, PAGE_SIZE);
>
> kunmap_atomic(vaddr);
> }
>
> #define kmap_px(px) kmap_page_dma(px_base(px))
> -#define kunmap_px(ppgtt, vaddr) kunmap_page_dma((ppgtt)->base.dev, (vaddr))
> +#define kunmap_px(ppgtt, vaddr) (ppgtt)->kunmap_page_dma((vaddr))
>
> #define setup_px(dev, px) setup_page_dma((dev), px_base(px))
> #define cleanup_px(dev, px) cleanup_page_dma((dev), px_base(px))
> -#define fill_px(dev, px, v) fill_page_dma((dev), px_base(px), (v))
> -#define fill32_px(dev, px, v) fill_page_dma_32((dev), px_base(px), (v))
> +#define fill_px(ppgtt, px, v) fill_page_dma((ppgtt), px_base(px), (v))
> +#define fill32_px(ppgtt, px, v) fill_page_dma_32((ppgtt), px_base(px), (v))
>
This feels a bit asymmetric, because some of the functions expect the
ppgtt pointer while others expect the dev pointer. Nothing clean comes
to mind to solve this immediately without adding extra pointer
indirections, but I guess we could come back to standardize it in the
future if we start passing the ppgtt pointer around more (e.g. like in
https://patchwork.freedesktop.org/patch/83777/). Not a blocker for this
patch anyway.
> -static void fill_page_dma(struct drm_device *dev, struct i915_page_dma *p,
> +static void fill_page_dma(struct i915_hw_ppgtt *ppgtt, struct i915_page_dma *p,
> const uint64_t val)
> {
> int i;
> @@ -392,17 +393,17 @@ static void fill_page_dma(struct drm_device *dev, struct i915_page_dma *p,
> for (i = 0; i < 512; i++)
> vaddr[i] = val;
>
> - kunmap_page_dma(dev, vaddr);
> + ppgtt->kunmap_page_dma(vaddr);
> }
>
> -static void fill_page_dma_32(struct drm_device *dev, struct i915_page_dma *p,
> - const uint32_t val32)
> +static void fill_page_dma_32(struct i915_hw_ppgtt *ppgtt,
> + struct i915_page_dma *p, const uint32_t val32)
> {
> uint64_t v = val32;
>
> v = v << 32 | val32;
>
> - fill_page_dma(dev, p, v);
> + fill_page_dma(ppgtt, p, v);
> }
>
> static struct i915_page_scratch *alloc_scratch_page(struct drm_device *dev)
> @@ -480,7 +481,7 @@ static void gen8_initialize_pt(struct i915_address_space *vm,
> scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
> I915_CACHE_LLC, true);
>
> - fill_px(vm->dev, pt, scratch_pte);
> + fill_px(i915_vm_to_ppgtt(vm), pt, scratch_pte);
> }
>
> static void gen6_initialize_pt(struct i915_address_space *vm,
> @@ -493,7 +494,7 @@ static void gen6_initialize_pt(struct i915_address_space *vm,
> scratch_pte = vm->pte_encode(px_dma(vm->scratch_page),
> I915_CACHE_LLC, true, 0);
>
> - fill32_px(vm->dev, pt, scratch_pte);
> + fill32_px(i915_vm_to_ppgtt(vm), pt, scratch_pte);
> }
>
> static struct i915_page_directory *alloc_pd(struct drm_device *dev)
> @@ -540,7 +541,7 @@ static void gen8_initialize_pd(struct i915_address_space *vm,
>
> scratch_pde = gen8_pde_encode(px_dma(vm->scratch_pt), I915_CACHE_LLC);
>
> - fill_px(vm->dev, pd, scratch_pde);
> + fill_px(i915_vm_to_ppgtt(vm), pd, scratch_pde);
> }
>
> static int __pdp_init(struct drm_device *dev,
> @@ -621,7 +622,7 @@ static void gen8_initialize_pdp(struct i915_address_space *vm,
>
> scratch_pdpe = gen8_pdpe_encode(px_dma(vm->scratch_pd), I915_CACHE_LLC);
>
> - fill_px(vm->dev, pdp, scratch_pdpe);
> + fill_px(i915_vm_to_ppgtt(vm), pdp, scratch_pdpe);
> }
>
> static void gen8_initialize_pml4(struct i915_address_space *vm,
> @@ -632,7 +633,7 @@ static void gen8_initialize_pml4(struct i915_address_space *vm,
> scratch_pml4e = gen8_pml4e_encode(px_dma(vm->scratch_pdp),
> I915_CACHE_LLC);
>
> - fill_px(vm->dev, pml4, scratch_pml4e);
> + fill_px(i915_vm_to_ppgtt(vm), pml4, scratch_pml4e);
> }
>
> static void
> @@ -1512,8 +1513,17 @@ static int gen8_preallocate_top_level_pdps(struct i915_hw_ppgtt *ppgtt)
> */
> static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
> {
> + struct drm_i915_private *dev_priv = to_i915(ppgtt->base.dev);
> int ret;
>
> + /* There are only few exceptions for gen >=6. chv and bxt.
> + * And we are not sure about the latter so play safe for now.
> + */
> + if (IS_BROADWELL(dev_priv) || IS_CHERRYVIEW(dev_priv))
This should be IS_BROXTON instead of IS_BROADWELL.
> + ppgtt->kunmap_page_dma = kunmap_page_dma_flush;
> + else
> + ppgtt->kunmap_page_dma = kunmap_page_dma;
> +
This virtual function assignment comes before the gen8_init_scratch call
while all the others are after that. To keep all of them together we
could move the call to gen8_init_scratch further down.
Regards,
Daniele
> ret = gen8_init_scratch(&ppgtt->base);
> if (ret)
> return ret;
> @@ -2073,6 +2083,9 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
> int ret;
>
> ppgtt->base.pte_encode = ggtt->base.pte_encode;
> +
> + ppgtt->kunmap_page_dma = kunmap_page_dma;
> +
> if (IS_GEN6(dev)) {
> ppgtt->switch_mm = gen6_mm_switch;
> } else if (IS_HASWELL(dev)) {
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 62be77cac5cd..b36b997406c6 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -379,6 +379,7 @@ struct i915_hw_ppgtt {
>
> gen6_pte_t __iomem *pd_addr;
>
> + void (*kunmap_page_dma)(void *vaddr);
> int (*enable)(struct i915_hw_ppgtt *ppgtt);
> int (*switch_mm)(struct i915_hw_ppgtt *ppgtt,
> struct drm_i915_gem_request *req);
More information about the Intel-gfx
mailing list