Re: [Intel-gfx] [CI v4 12/13] drm/i915/ttm: disallow CPU fallback mode for ccs pages

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

Hi Matt,

On 29/06/2022 18:43, Matthew Auld wrote:
> Falling back to memcpy/memset shouldn't be allowed if we know we have
> CCS state to manage using the blitter. Otherwise we are potentially
> leaving the aux CCS state in an unknown state, which smells like an info
> leak.
> 
> Fixes: 48760ffe923a ("drm/i915/gt: Clear compress metadata for Flat-ccs objects")

This is marking the patch for 5.19-rc, but it not apply since the code 
seems a bit different. There is no i915_ttm_memcpy_allowed to start 
with, which only comes in bfe53be268af ("drm/i915/ttm: handle blitter 
failure on DG2"), which is for 5.20.

Do you think a version of this patch for 5.19 is needed and if so could 
you, or someone in the know, cook one up today or tomorrow at the latest?

Regards,

Tvrtko

> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Cc: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
> Cc: Ramalingam C <ramalingam.c@intel.com>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_object.c   | 26 ++++++++++++++++++++
>   drivers/gpu/drm/i915/gem/i915_gem_object.h   |  2 ++
>   drivers/gpu/drm/i915/gem/i915_gem_ttm.c      | 18 --------------
>   drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c |  3 +++
>   4 files changed, 31 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> index 642a5d59ce26..ccec4055fde3 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> @@ -717,6 +717,32 @@ bool i915_gem_object_placement_possible(struct drm_i915_gem_object *obj,
>   	return false;
>   }
>   
> +/**
> + * i915_gem_object_needs_ccs_pages - Check whether the object requires extra
> + * pages when placed in system-memory, in order to save and later restore the
> + * flat-CCS aux state when the object is moved between local-memory and
> + * system-memory
> + * @obj: Pointer to the object
> + *
> + * Return: True if the object needs extra ccs pages. False otherwise.
> + */
> +bool i915_gem_object_needs_ccs_pages(struct drm_i915_gem_object *obj)
> +{
> +	bool lmem_placement = false;
> +	int i;
> +
> +	for (i = 0; i < obj->mm.n_placements; i++) {
> +		/* Compression is not allowed for the objects with smem placement */
> +		if (obj->mm.placements[i]->type == INTEL_MEMORY_SYSTEM)
> +			return false;
> +		if (!lmem_placement &&
> +		    obj->mm.placements[i]->type == INTEL_MEMORY_LOCAL)
> +			lmem_placement = true;
> +	}
> +
> +	return lmem_placement;
> +}
> +
>   void i915_gem_init__objects(struct drm_i915_private *i915)
>   {
>   	INIT_DELAYED_WORK(&i915->mm.free_work, __i915_gem_free_work);
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> index 0bf3ee27a2a8..6f0a3ce35567 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> @@ -618,6 +618,8 @@ int i915_gem_object_wait_migration(struct drm_i915_gem_object *obj,
>   bool i915_gem_object_placement_possible(struct drm_i915_gem_object *obj,
>   					enum intel_memory_type type);
>   
> +bool i915_gem_object_needs_ccs_pages(struct drm_i915_gem_object *obj);
> +
>   int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
>   			 size_t size, struct intel_memory_region *mr,
>   			 struct address_space *mapping,
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> index 098409a33e10..7e1f8b83077f 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> @@ -266,24 +266,6 @@ static const struct i915_refct_sgt_ops tt_rsgt_ops = {
>   	.release = i915_ttm_tt_release
>   };
>   
> -static inline bool
> -i915_gem_object_needs_ccs_pages(struct drm_i915_gem_object *obj)
> -{
> -	bool lmem_placement = false;
> -	int i;
> -
> -	for (i = 0; i < obj->mm.n_placements; i++) {
> -		/* Compression is not allowed for the objects with smem placement */
> -		if (obj->mm.placements[i]->type == INTEL_MEMORY_SYSTEM)
> -			return false;
> -		if (!lmem_placement &&
> -		    obj->mm.placements[i]->type == INTEL_MEMORY_LOCAL)
> -			lmem_placement = true;
> -	}
> -
> -	return lmem_placement;
> -}
> -
>   static struct ttm_tt *i915_ttm_tt_create(struct ttm_buffer_object *bo,
>   					 uint32_t page_flags)
>   {
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> index df14ac81c128..9a7e50534b84 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> @@ -435,6 +435,9 @@ i915_ttm_memcpy_work_arm(struct i915_ttm_memcpy_work *work,
>   static bool i915_ttm_memcpy_allowed(struct ttm_buffer_object *bo,
>   				    struct ttm_resource *dst_mem)
>   {
> +	if (i915_gem_object_needs_ccs_pages(i915_ttm_to_gem(bo)))
> +		return false;
> +
>   	if (!(i915_ttm_resource_mappable(bo->resource) &&
>   	      i915_ttm_resource_mappable(dst_mem)))
>   		return false;