Re: [PATCH 1/6] drm/xe/migrate: Handle clear ccs logic for xe2 dgfx

["Jahagirdar, Akshata" <akshata.jahagirdar@intel.com>]

On 7/11/2024 5:09 AM, Matthew Auld wrote:
> On 11/07/2024 10:18, Akshata Jahagirdar wrote:
>> For Xe2 dGPU, we clear the bo by modifying the VRAM using an
>> uncompressed pat index which then indirectly updates the
>> compression status as uncompressed i.e zeroed CCS.
>> So xe_migrate_clear() should be updated for BMG to not
>> emit CCS surf copy commands.
>>
>> Signed-off-by: Akshata Jahagirdar <akshata.jahagirdar@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_device.h  | 5 +++++
>>   drivers/gpu/drm/xe/xe_migrate.c | 6 +++---
>>   2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_device.h 
>> b/drivers/gpu/drm/xe/xe_device.h
>> index 0a2a3e7fd402..c3093506c28c 100644
>> --- a/drivers/gpu/drm/xe/xe_device.h
>> +++ b/drivers/gpu/drm/xe/xe_device.h
>> @@ -144,6 +144,11 @@ static inline bool xe_device_has_flat_ccs(struct 
>> xe_device *xe)
>>       return xe->info.has_flat_ccs;
>>   }
>>   +static inline bool xe_device_needs_ccs_emit(struct xe_device *xe)
>> +{
>> +    return xe_device_has_flat_ccs(xe) && !(GRAPHICS_VER(xe) >= 20 && 
>> IS_DGFX(xe));
>> +}
>> +
>
> This should in theory be hyper specific to the migration code 
> implementation. I think best keep in xe_migrate.c, instead of 
> exporting (if possible).
>
> With that,
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>
Thank you for your review.
Should I move this change in internal as well?

-Akshata
>
>>   static inline bool xe_device_has_sriov(struct xe_device *xe)
>>   {
>>       return xe->info.has_sriov;
>> diff --git a/drivers/gpu/drm/xe/xe_migrate.c 
>> b/drivers/gpu/drm/xe/xe_migrate.c
>> index fa23a7e7ec43..2fc2cf375b1e 100644
>> --- a/drivers/gpu/drm/xe/xe_migrate.c
>> +++ b/drivers/gpu/drm/xe/xe_migrate.c
>> @@ -420,7 +420,7 @@ struct xe_migrate *xe_migrate_init(struct xe_tile 
>> *tile)
>>           return ERR_PTR(err);
>>         if (IS_DGFX(xe)) {
>> -        if (xe_device_has_flat_ccs(xe))
>> +        if (xe_device_needs_ccs_emit(xe))
>>               /* min chunk size corresponds to 4K of CCS Metadata */
>>               m->min_chunk_size = SZ_4K * SZ_64K /
>>                   xe_device_ccs_bytes(xe, SZ_64K);
>> @@ -1034,7 +1034,7 @@ struct dma_fence *xe_migrate_clear(struct 
>> xe_migrate *m,
>>                       clear_system_ccs ? 0 : emit_clear_cmd_len(gt), 0,
>>                       avail_pts);
>>   -        if (xe_device_has_flat_ccs(xe))
>> +        if (xe_device_needs_ccs_emit(xe))
>>               batch_size += EMIT_COPY_CCS_DW;
>>             /* Clear commands */
>> @@ -1062,7 +1062,7 @@ struct dma_fence *xe_migrate_clear(struct 
>> xe_migrate *m,
>>           if (!clear_system_ccs)
>>               emit_clear(gt, bb, clear_L0_ofs, clear_L0, 
>> XE_PAGE_SIZE, clear_vram);
>>   -        if (xe_device_has_flat_ccs(xe)) {
>> +        if (xe_device_needs_ccs_emit(xe)) {
>>               emit_copy_ccs(gt, bb, clear_L0_ofs, true,
>>                         m->cleared_mem_ofs, false, clear_L0);
>>               flush_flags = MI_FLUSH_DW_CCS;