Re: [PATCH v2 3/5] drm/i915: ttm move/clear logic fix

[Thomas Hellström]

On Tue, 2022-04-12 at 15:18 +0000, Robert Beckett wrote:
> ttm managed buffers start off with system resource definitions and
> ttm_tt
> tracking structures allocated (though unpopulated).
> currently this prevents clearing of buffers on first move to desired
> placements.
> 
> The desired behaviour is to clear user allocated buffers and any
> kernel
> buffers that specifically requests it only.
> Make the logic match the desired behaviour.
> 
> Signed-off-by: Robert Beckett <bob.beckett@xxxxxxxxxxxxx>

Reviewed-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>


> ---
>  drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 22
> +++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> 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 9fe8132de3b2..9cf85f91edb5 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> @@ -3,6 +3,7 @@
>   * Copyright © 2021 Intel Corporation
>   */
>  
> +#include "drm/ttm/ttm_tt.h"
>  #include <drm/ttm/ttm_bo_driver.h>
>  
>  #include "i915_deps.h"
> @@ -470,6 +471,25 @@ __i915_ttm_move(struct ttm_buffer_object *bo,
>         return fence;
>  }
>  
> +static bool
> +allow_clear(struct drm_i915_gem_object *obj, struct ttm_tt *ttm,
> struct ttm_resource *dst_mem)
> +{
> +       /* never clear stolen */
> +       if (dst_mem->mem_type == I915_PL_STOLEN)
> +               return false;
> +       /*
> +        * we want to clear user buffers and any kernel buffers
> +        * that specifically request clearing.
> +        */
> +       if (obj->flags & I915_BO_ALLOC_USER)
> +               return true;
> +
> +       if (ttm && ttm->page_flags & TTM_TT_FLAG_ZERO_ALLOC)
> +               return true;
> +
> +       return false;
> +}
> +
>  /**
>   * i915_ttm_move - The TTM move callback used by i915.
>   * @bo: The buffer object.
> @@ -520,7 +540,7 @@ int i915_ttm_move(struct ttm_buffer_object *bo,
> bool evict,
>                 return PTR_ERR(dst_rsgt);
>  
>         clear = !i915_ttm_cpu_maps_iomem(bo->resource) && (!ttm ||
> !ttm_tt_is_populated(ttm));
> -       if (!(clear && ttm && !(ttm->page_flags &
> TTM_TT_FLAG_ZERO_ALLOC))) {
> +       if (!clear || allow_clear(obj, ttm, dst_mem)) {
>                 struct i915_deps deps;
>  
>                 i915_deps_init(&deps, GFP_KERNEL | __GFP_NORETRY |
> __GFP_NOWARN);