Re: [Intel-gfx] [PATCH v2 3/4] drm/i915/gem: Extra pages in ttm_tt for ccs data

Thomas Hellström
2022-03-02T12:58:48Z

On Wed, 2022-03-02 at 03:23 +0530, Ramalingam C wrote:
> On Xe-HP and later devices, we use dedicated compression control
> state (CCS) stored in local memory for each surface, to support the
> 3D and media compression formats.
>
> The memory required for the CCS of the entire local memory is 1/256
> of
> the local memory size. So before the kernel boot, the required memory
> is reserved for the CCS data and a secure register will be programmed
> with the CCS base address
>
> So when we allocate a object in local memory we dont need to
> explicitly
> allocate the space for ccs data. But when we evict the obj into the
> smem to hold the compression related data along with the obj we need
> smem space of obj_size + (obj_size/256).
>
> Hence when we create smem for an obj with lmem placement possibility
> we
> create with the extra space.

Nit: Again imperative wording,

>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> cc: Christian Koenig <christian.koenig@amd.com>
> cc: Hellstrom Thomas <thomas.hellstrom@intel.com>

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

> ---
>  drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> index 1a8262f5f692..c7a36861c38d 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> @@ -20,6 +20,7 @@
>  #include "gem/i915_gem_ttm.h"
>  #include "gem/i915_gem_ttm_move.h"
>  #include "gem/i915_gem_ttm_pm.h"
> +#include "gt/intel_gpu_commands.h"
>
>  #define I915_TTM_PRIO_PURGE     0
>  #define I915_TTM_PRIO_NO_PAGES  1
> @@ -255,12 +256,27 @@ static const struct i915_refct_sgt_ops
> tt_rsgt_ops = {
>         .release = i915_ttm_tt_release
>  };
>
> +static inline bool
> +i915_gem_object_has_lmem_placement(struct drm_i915_gem_object *obj)
> +{
> +       int i;
> +
> +       for (i = 0; i < obj->mm.n_placements; i++)
> +               if (obj->mm.placements[i]->type ==
> INTEL_MEMORY_LOCAL)
> +                       return true;
> +
> +       return false;
> +}
> +
>  static struct ttm_tt *i915_ttm_tt_create(struct ttm_buffer_object
> *bo,
>                                          uint32_t page_flags)
>  {
> +       struct drm_i915_private *i915 = container_of(bo->bdev,
> typeof(*i915),
> +                                                    bdev);
>         struct ttm_resource_manager *man =
>                 ttm_manager_type(bo->bdev, bo->resource->mem_type);
>         struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
> +       unsigned long ccs_pages = 0;
>         enum ttm_caching caching;
>         struct i915_ttm_tt *i915_tt;
>         int ret;
> @@ -283,7 +299,12 @@ static struct ttm_tt *i915_ttm_tt_create(struct
> ttm_buffer_object *bo,
>                 i915_tt->is_shmem = true;
>         }
>
> -       ret = ttm_tt_init(&i915_tt->ttm, bo, page_flags, caching, 0);
> +       if (HAS_FLAT_CCS(i915) &&
> i915_gem_object_has_lmem_placement(obj))
> +               ccs_pages = DIV_ROUND_UP(DIV_ROUND_UP(bo->base.size,
> +
> NUM_BYTES_PER_CCS_BYTE),
> +                                        PAGE_SIZE);
> +
> +       ret = ttm_tt_init(&i915_tt->ttm, bo, page_flags, caching,
> ccs_pages);
>         if (ret)
>                 goto err_free;
>