From: Chris Wilson chris.p.wilson@intel.com
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 concurrent access and stale access from prefetch.
We only need to invalidate the TLB if they are accessible by the user. That helps to reduce the performance regression introduced by TLB invalidate logic.
Cc: stable@vger.kernel.org Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store") Signed-off-by: Chris Wilson chris.p.wilson@intel.com Cc: Fei Yang fei.yang@intel.com Cc: Andi Shyti andi.shyti@linux.intel.com Acked-by: Thomas Hellström thomas.hellstrom@linux.intel.com Signed-off-by: Mauro Carvalho Chehab mchehab@kernel.org ---
To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover. See [PATCH v2 00/21] at: https://lore.kernel.org/all/cover.1657800199.git.mchehab@kernel.org/
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 ef3b04c7e153..646f419b2035 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -538,7 +538,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;
On 14/07/2022 13:06, Mauro Carvalho Chehab wrote:
From: Chris Wilson chris.p.wilson@intel.com
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 concurrent access and stale access from prefetch.
We only need to invalidate the TLB if they are accessible by the user. That helps to reduce the performance regression introduced by TLB invalidate logic.
Cc: stable@vger.kernel.org Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store")
Do we really need or want stable and fixes on this one?
What do we think the performance improvement is, given there's very little in GGTT, which is not mapped via PPGTT as well?
I think it is safe, but part of me would ideally not even want to think about whether it is safe, if the performance improvement is non-existent. Which I can't imagine how there would be?
Regards,
Tvrtko
Signed-off-by: Chris Wilson chris.p.wilson@intel.com Cc: Fei Yang fei.yang@intel.com Cc: Andi Shyti andi.shyti@linux.intel.com Acked-by: Thomas Hellström thomas.hellstrom@linux.intel.com Signed-off-by: Mauro Carvalho Chehab mchehab@kernel.org
To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover. See [PATCH v2 00/21] at: https://lore.kernel.org/all/cover.1657800199.git.mchehab@kernel.org/
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 ef3b04c7e153..646f419b2035 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -538,7 +538,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;
On Mon, 18 Jul 2022 14:39:17 +0100 Tvrtko Ursulin tvrtko.ursulin@linux.intel.com wrote:
On 14/07/2022 13:06, Mauro Carvalho Chehab wrote:
From: Chris Wilson chris.p.wilson@intel.com
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 concurrent access and stale access from prefetch.
We only need to invalidate the TLB if they are accessible by the user. That helps to reduce the performance regression introduced by TLB invalidate logic.
Cc: stable@vger.kernel.org Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store")
Do we really need or want stable and fixes on this one?
What do we think the performance improvement is, given there's very little in GGTT, which is not mapped via PPGTT as well?
I think it is safe, but part of me would ideally not even want to think about whether it is safe, if the performance improvement is non-existent. Which I can't imagine how there would be?
Makes sense. Patch 6 actually ends removing the code doing that, so I'll just fold this patch with patch 6, in order to avoid adding something that will later be removed.
Regards, Mauro
Hi Mauro,
On Thu, Jul 14, 2022 at 01:06:09PM +0100, Mauro Carvalho Chehab wrote:
From: Chris Wilson chris.p.wilson@intel.com
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 concurrent access and stale access from prefetch.
We only need to invalidate the TLB if they are accessible by the user. That helps to reduce the performance regression introduced by TLB invalidate logic.
Cc: stable@vger.kernel.org Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store") Signed-off-by: Chris Wilson chris.p.wilson@intel.com Cc: Fei Yang fei.yang@intel.com Cc: Andi Shyti andi.shyti@linux.intel.com Acked-by: Thomas Hellström thomas.hellstrom@linux.intel.com Signed-off-by: Mauro Carvalho Chehab mchehab@kernel.org
Please, once you have sorted out Tvrtko's question you can add:
Reviewed-by: Andi Shyti andi.shyti@linux.intel.com
Thanks, Andi
linux-stable-mirror@lists.linaro.org
-
Andi Shyti -
Mauro Carvalho Chehab -
Mauro Carvalho Chehab -
Tvrtko Ursulin