Re: [PATCH 4/6] drm/xe/xe_migrate: Handle migration logic for xe2+ dgfx

[Nirmoy Das <nirmoy.das@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3896 bytes --]

Hi Akshata,

On 7/15/2024 10:56 PM, Jahagirdar, Akshata wrote:
>
>
> On 7/12/2024 5:55 AM, Nirmoy Das wrote:
>>
>> Hi Akshata,
>>
>> On 7/12/2024 8:39 AM, Akshata Jahagirdar wrote:
>>> From: "Jahagirdar, Akshata"<akshata.jahagirdar@intel.com>
>>>
>>> During eviction (vram->sysmem), we use the mapping from compressed -> uncompressed.
>>> During restore (sysmem->vram), we need the mapping from uncompressed -> uncompressed.
>>> Handle logic for selecting the compressed identity map for eviction,
>>> and selecting uncompressed map for restore operations.
>>>
>>> Signed-off-by: Akshata Jahagirdar<akshata.jahagirdar@intel.com>
>>> Reviewed-by: Matthew Auld<matthew.auld@intel.com>
>>> Reviewed-by: Himal Prasad Ghimiray<himal.prasad.ghimiray@intel.com>
>>> ---
>>>   drivers/gpu/drm/xe/xe_migrate.c | 7 ++++---
>>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
>>> index 3b8a334fe08f..1eee6be24423 100644
>>> --- a/drivers/gpu/drm/xe/xe_migrate.c
>>> +++ b/drivers/gpu/drm/xe/xe_migrate.c
>>> @@ -715,7 +715,7 @@ static u32 xe_migrate_ccs_copy(struct xe_migrate *m,
>>>   	struct xe_gt *gt = m->tile->primary_gt;
>>>   	u32 flush_flags = 0;
>>>   
>>> -	if (xe_device_has_flat_ccs(gt_to_xe(gt)) && !copy_ccs && dst_is_indirect) {
>>> +	if (xe_device_needs_ccs_emit(gt_to_xe(gt)) && !copy_ccs && dst_is_indirect) {
>>
>>
>> Do we need to call xe_migrate_ccs_copy() when 
>> xe_device_needs_ccs_emit() is true ? If not then move the check for 
>> xe_device_needs_ccs_emit() to xe_migrate_copy()
>>

I see that I've confused you here. I meant "Do we need to call 
xe_migrate_ccs_copy() when xe_device_needs_ccs_emit() is *not* true ?"

>> to ensure that xe_migrate_ccs_copy() is not even called on such 
>> platforms.
>>
>> Regards,
>> Nirmoy
>
> xe_device_needs_ccs_emit() is true for any non-BMG platform that has 
> flat-ccs enabled.
> In such cases, we do need to call xe_migrate_ccs_copy() while clearing 
> any ccs metadata in vram.
>

So let's use xe_device_needs_ccs_emit() in the upper layer to decide 
whether to call xe_migrate_ccs_copy(), instead of checking 
xe_device_needs_ccs_emit() within an if-else block of xe_migrate_ccs_copy()

Either:

if (xe_device_needs_ccs_emit())

xe_migrate_ccs_copy(...)


Or even*"
*


xe_migrate_ccs_copy() {

     if ( !xe_device_needs_ccs_emit() )

     return;

..........

}



Regards,

Nirmoy

>
> BR,
> Akshata
>
>>>   		/*
>>>   		 * If the src is already in vram, then it should already
>>>   		 * have been cleared by us, or has been populated by the
>>> @@ -791,6 +791,7 @@ struct dma_fence *xe_migrate_copy(struct xe_migrate *m,
>>>   	bool copy_ccs = xe_device_has_flat_ccs(xe) &&
>>>   		xe_bo_needs_ccs_pages(src_bo) && xe_bo_needs_ccs_pages(dst_bo);
>>>   	bool copy_system_ccs = copy_ccs && (!src_is_vram || !dst_is_vram);
>>> +	bool use_comp_pat = GRAPHICS_VER(xe) >= 20 && IS_DGFX(xe) && src_is_vram && !dst_is_vram;
>>>   
>>>   	/* Copying CCS between two different BOs is not supported yet. */
>>>   	if (XE_WARN_ON(copy_ccs && src_bo != dst_bo))
>>> @@ -833,7 +834,7 @@ struct dma_fence *xe_migrate_copy(struct xe_migrate *m,
>>>   
>>>   		src_L0 = min(src_L0, dst_L0);
>>>   
>>> -		batch_size += pte_update_size(m, src_is_vram, false, src, &src_it, &src_L0,
>>> +		batch_size += pte_update_size(m, src_is_vram, use_comp_pat, src, &src_it, &src_L0,
>>>   					      &src_L0_ofs, &src_L0_pt, 0, 0,
>>>   					      avail_pts);
>>>   
>>> @@ -852,7 +853,7 @@ struct dma_fence *xe_migrate_copy(struct xe_migrate *m,
>>>   
>>>   		/* Add copy commands size here */
>>>   		batch_size += ((copy_only_ccs) ? 0 : EMIT_COPY_DW) +
>>> -			((xe_device_has_flat_ccs(xe) ? EMIT_COPY_CCS_DW : 0));
>>> +			((xe_device_needs_ccs_emit(xe) ? EMIT_COPY_CCS_DW : 0));
>>>   
>>>   		bb = xe_bb_new(gt, batch_size, usm);
>>>   		if (IS_ERR(bb)) {

[-- Attachment #2: Type: text/html, Size: 6563 bytes --]