Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <8f25394fb9fa71eeec2f6184ea854a391b415d22.camel@linux.intel.com>
Subject: Re: [PATCH v2 3/5] drm/i915: ttm move/clear logic fix
From:   Thomas =?ISO-8859-1?Q?Hellstr=F6m?= 
        <thomas.hellstrom@linux.intel.com>
To:     Robert Beckett <bob.beckett@collabora.com>,
        dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
        Jani Nikula <jani.nikula@linux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>
Cc:     Matthew Auld <matthew.auld@intel.com>, linux-kernel@vger.kernel.org
Date:   Thu, 14 Apr 2022 16:00:06 +0200
In-Reply-To: <20220412151838.1298956-4-bob.beckett@collabora.com>
References: <20220412151838.1298956-1-bob.beckett@collabora.com>
         <20220412151838.1298956-4-bob.beckett@collabora.com>
Organization: Intel Sweden AB, Registration Number: 556189-6027
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.40.4 (3.40.4-3.fc34) 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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@collabora.com>

Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>


> ---
>  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);