diff mbox series

[RFC,1/8] drm/i915: Skip clflush after GPU writes on Meteorlake

Message ID 20230727145504.1919316-2-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series Another take on PAT/object cache mode refactoring | expand

Commit Message

Tvrtko Ursulin July 27, 2023, 2:54 p.m. UTC 
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

On Meteorlake CPU cache will not contain stale data after GPU access since
write-invalidate protocol is used, which means there is no need to flush
before potentially transitioning the buffer to a non-coherent domain.

Use the opportunity to documet the situation on discrete too.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Fei Yang <fei.yang@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_domain.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Matt Roper July 27, 2023, 10:19 p.m. UTC | #1 
On Thu, Jul 27, 2023 at 03:54:57PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> On Meteorlake CPU cache will not contain stale data after GPU access since
> write-invalidate protocol is used, which means there is no need to flush
> before potentially transitioning the buffer to a non-coherent domain.
> 
> Use the opportunity to documet the situation on discrete too.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Fei Yang <fei.yang@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> ---
>  drivers/gpu/drm/i915/gem/i915_gem_domain.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> index ffddec1d2a76..57db9c581bf6 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> @@ -24,9 +24,22 @@ static bool gpu_write_needs_clflush(struct drm_i915_gem_object *obj)
>  {
>  	struct drm_i915_private *i915 = to_i915(obj->base.dev);
>  
> +	/*
> +	 * Discrete GPUs never dirty the CPU cache.
> +	 */
>  	if (IS_DGFX(i915))
>  		return false;
>  
> +	/*
> +	 * Cache snooping on Meteorlake is using write-invalidate so GPU writes
> +	 * never end up in the CPU cache.
> +	 *
> +	 * QQQ: Do other snooping platforms behave identicaly and could we
> +	 *      therefore write this as "if !HAS_LLC(i915) && HAS_SNOOP(i915)"?
> +	 */
> +	if (IS_METEORLAKE(i915))
> +		return false;
> +
>  	/*
>  	 * For objects created by userspace through GEM_CREATE with pat_index
>  	 * set by set_pat extension, i915_gem_object_has_cache_level() will
> -- 
> 2.39.2
>
Yang, Fei July 28, 2023, 5:50 a.m. UTC | #2 
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> On Meteorlake CPU cache will not contain stale data after GPU
> access since write-invalidate protocol is used, which means
> there is no need to flush before potentially transitioning the
> buffer to a non-coherent domain.
>
> Use the opportunity to documet the situation on discrete too.

LGTM.
Reviewed-by: Fei Yang <fei.yang@intel.com>

> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Fei Yang <fei.yang@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_domain.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> index ffddec1d2a76..57db9c581bf6 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> @@ -24,9 +24,22 @@ static bool gpu_write_needs_clflush(struct drm_i915_gem_object *obj)  {
>       struct drm_i915_private *i915 = to_i915(obj->base.dev);
>
> +     /*
> +      * Discrete GPUs never dirty the CPU cache.
> +      */
>       if (IS_DGFX(i915))
>               return false;
>
> +     /*
> +      * Cache snooping on Meteorlake is using write-invalidate so GPU writes
> +      * never end up in the CPU cache.
> +      *
> +      * QQQ: Do other snooping platforms behave identicaly and could we
> +      *      therefore write this as "if !HAS_LLC(i915) && HAS_SNOOP(i915)"?
> +      */
> +     if (IS_METEORLAKE(i915))
> +             return false;
> +
>       /*
>        * For objects created by userspace through GEM_CREATE with pat_index
>        * set by set_pat extension, i915_gem_object_has_cache_level() will
> --
> 2.39.2
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index ffddec1d2a76..57db9c581bf6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -24,9 +24,22 @@  static bool gpu_write_needs_clflush(struct drm_i915_gem_object *obj)
 {
 	struct drm_i915_private *i915 = to_i915(obj->base.dev);
 
+	/*
+	 * Discrete GPUs never dirty the CPU cache.
+	 */
 	if (IS_DGFX(i915))
 		return false;
 
+	/*
+	 * Cache snooping on Meteorlake is using write-invalidate so GPU writes
+	 * never end up in the CPU cache.
+	 *
+	 * QQQ: Do other snooping platforms behave identicaly and could we
+	 *      therefore write this as "if !HAS_LLC(i915) && HAS_SNOOP(i915)"?
+	 */
+	if (IS_METEORLAKE(i915))
+		return false;
+
 	/*
 	 * For objects created by userspace through GEM_CREATE with pat_index
 	 * set by set_pat extension, i915_gem_object_has_cache_level() will