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

[Ghimiray, Himal Prasad]

On 27-11-2023 15:11, Thomas Hellström wrote:
>
>
> On 11/27/23 04:19, Ghimiray, Himal Prasad wrote:
>>
>>
>> On 24-11-2023 16:39, Thomas Hellström wrote:
>>> 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 at linux.intel.com>
>>>> Signed-off-by: Himal Prasad Ghimiray
>>>> <himal.prasad.ghimiray at 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
>> Sure. Will address in next patch.
>>>> - *
>>>>    * 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 ((IS_DGFX(xe) && (bo->flags & XE_BO_CREATE_VRAM_MASK)) not suffice 
>> to ensure
>>
>> that for DGFX BO is only in VRAM region ?
>>
> No. It can also has the the CREATE_SYSTEM_BIT set, and then the 
> content can't be compressed.
>
> /Thomas
>
Thanks for the clarification. Will address this is next version.

BR

Himal

>
>
>>>> +               (!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)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-xe/attachments/20231129/9107b02f/attachment.htm>