2022-06-15 15:32:20

by Mauro Carvalho Chehab

Subject: [PATCH 4/6] drm/i915/gt: Only invalidate TLBs exposed to user manipulation

From: Chris Wilson <[email protected]>

Don't flush TLBs when the buffer is only used in the GGTT under full
control of the kernel, as there's no risk of of concurrent access
and stale access from prefetch.

We only need to invalidate the TLB if they are accessible by the user.

Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store")

Signed-off-by: Chris Wilson <[email protected]>
Cc: Fei Yang <[email protected]>
Cc: Andi Shyti <[email protected]>
Cc: [email protected]
Acked-by: Thomas Hellström <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---

See [PATCH 0/6] at: https://lore.kernel.org/all/[email protected]/

drivers/gpu/drm/i915/i915_vma.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 0bffb70b3c5f..7989986161e8 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -537,7 +537,8 @@ int i915_vma_bind(struct i915_vma *vma,
bind_flags);
}

- set_bit(I915_BO_WAS_BOUND_BIT, &vma->obj->flags);
+ if (bind_flags & I915_VMA_LOCAL_BIND)
+ set_bit(I915_BO_WAS_BOUND_BIT, &vma->obj->flags);

atomic_or(bind_flags, &vma->flags);
return 0;
--
2.36.1

2022-06-16 07:34:51

by Tvrtko Ursulin

Subject: Re: [PATCH 4/6] drm/i915/gt: Only invalidate TLBs exposed to user manipulation

On 15/06/2022 16:27, Mauro Carvalho Chehab wrote:
> From: Chris Wilson <[email protected]>
>
> Don't flush TLBs when the buffer is only used in the GGTT under full
> control of the kernel, as there's no risk of of concurrent access
> and stale access from prefetch.
>
> We only need to invalidate the TLB if they are accessible by the user.
>
> Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store")

Same question as against the other patch - fix or optimisation?

Regards,

Tvrtko

> Signed-off-by: Chris Wilson <[email protected]>
> Cc: Fei Yang <[email protected]>
> Cc: Andi Shyti <[email protected]>
> Cc: [email protected]
> Acked-by: Thomas Hellström <[email protected]>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> ---
>
> See [PATCH 0/6] at: https://lore.kernel.org/all/[email protected]/
>
> drivers/gpu/drm/i915/i915_vma.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 0bffb70b3c5f..7989986161e8 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -537,7 +537,8 @@ int i915_vma_bind(struct i915_vma *vma,
> bind_flags);
> }
>
> - set_bit(I915_BO_WAS_BOUND_BIT, &vma->obj->flags);
> + if (bind_flags & I915_VMA_LOCAL_BIND)
> + set_bit(I915_BO_WAS_BOUND_BIT, &vma->obj->flags);
>
> atomic_or(bind_flags, &vma->flags);
> return 0;

2022-06-23 11:20:57

by Andi Shyti

Subject: Re: [PATCH 4/6] drm/i915/gt: Only invalidate TLBs exposed to user manipulation

Hi Mauro,

On Wed, Jun 15, 2022 at 04:27:38PM +0100, Mauro Carvalho Chehab wrote:
> From: Chris Wilson <[email protected]>
>
> Don't flush TLBs when the buffer is only used in the GGTT under full
> control of the kernel, as there's no risk of of concurrent access
> and stale access from prefetch.
>
> We only need to invalidate the TLB if they are accessible by the user.
>
> Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store")
>
> Signed-off-by: Chris Wilson <[email protected]>
> Cc: Fei Yang <[email protected]>
> Cc: Andi Shyti <[email protected]>
> Cc: [email protected]
> Acked-by: Thomas Hellstr?m <[email protected]>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>

Reviewed-by: Andi Shyti <[email protected]>

Thanks,
Andi