Re: [Intel-xe] [RFC v2 3/6] drm/xe/xe2: Allocate extra pages for ccs during bo create.

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Tue, 2023-11-21 at 15:39 +0530, Himal Prasad Ghimiray wrote:
> Each byte of CCS data now represents 512 bytes of main memory data.
> Allocate extra pages to handle ccs region for igfx too.
> 
> Bspec:58796
> 
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Signed-off-by: Himal Prasad Ghimiray
> <himal.prasad.ghimiray@intel.com>
> ---
>  drivers/gpu/drm/xe/regs/xe_gpu_commands.h |  2 +-
>  drivers/gpu/drm/xe/xe_bo.c                | 15 ++++++---------
>  drivers/gpu/drm/xe/xe_device.c            |  2 +-
>  3 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/regs/xe_gpu_commands.h
> b/drivers/gpu/drm/xe/regs/xe_gpu_commands.h
> index 4402f72481dc..7f74592f99ce 100644
> --- a/drivers/gpu/drm/xe/regs/xe_gpu_commands.h
> +++ b/drivers/gpu/drm/xe/regs/xe_gpu_commands.h
> @@ -16,7 +16,7 @@
>  #define   XY_CTRL_SURF_MOCS_MASK       GENMASK(31, 26)
>  #define   XE2_XY_CTRL_SURF_MOCS_INDEX_MASK     GENMASK(31, 28)
>  #define   NUM_CCS_BYTES_PER_BLOCK      256
> -#define   NUM_BYTES_PER_CCS_BYTE       256
> +#define   NUM_BYTES_PER_CCS_BYTE(_xe)  (GRAPHICS_VER(_xe) >= 20 ?
> 512 : 256)
>  #define   NUM_CCS_BLKS_PER_XFER                1024
>  
>  #define XY_FAST_COLOR_BLT_CMD          (2 << 29 | 0x44 << 22)
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 4305f5cbc2ab..4730ee3c1012 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -2074,19 +2074,16 @@ int xe_bo_evict(struct xe_bo *bo, bool
> force_alloc)
>   * placed in system memory.
>   * @bo: The xe_bo
>   *
> - * If a bo has an allowable placement in XE_PL_TT memory, it can't
> use
> - * flat CCS compression, because the GPU then has no way to access
> the
> - * CCS metadata using relevant commands. For the opposite case, we
> need to
> - * allocate storage for the CCS metadata when the BO is not resident
> in
> - * VRAM memory.

Please extend modify this comment rather than deleting it


> - *
>   * Return: true if extra pages need to be allocated, false
> otherwise.
>   */
>  bool xe_bo_needs_ccs_pages(struct xe_bo *bo)
>  {
> -       return bo->ttm.type == ttm_bo_type_device &&
> -               !(bo->flags & XE_BO_CREATE_SYSTEM_BIT) &&
> -               (bo->flags & XE_BO_CREATE_VRAM_MASK);
> +       struct xe_device *xe = xe_bo_device(bo);
> +
> +       return (xe_device_has_flat_ccs(xe) &&
> +               bo->ttm.type == ttm_bo_type_device &&
> +               ((IS_DGFX(xe) && (bo->flags &
> XE_BO_CREATE_VRAM_MASK)) ||

It looks like you have removed a restriction for DGFX here: If the BO
has SYSTEM set, then ccs pages are not needed.

> +               (!IS_DGFX(xe) && (bo->flags &
> XE_BO_CREATE_SYSTEM_BIT))));
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/xe/xe_device.c
> b/drivers/gpu/drm/xe/xe_device.c
> index 07a3e4cf48d1..265f9ffc5323 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -551,7 +551,7 @@ void xe_device_wmb(struct xe_device *xe)
>  u32 xe_device_ccs_bytes(struct xe_device *xe, u64 size)
>  {
>         return xe_device_has_flat_ccs(xe) ?
> -               DIV_ROUND_UP(size, NUM_BYTES_PER_CCS_BYTE) : 0;
> +               DIV_ROUND_UP(size, NUM_BYTES_PER_CCS_BYTE(xe)) : 0;
>  }
>  
>  bool xe_device_mem_access_ongoing(struct xe_device *xe)