Re: [PATCH v2 1/1] drm/xe/xe2: Send flat_ccs value in struct drm_xe_query_config

[Matthew Auld <matthew.auld@intel.com>]

On 27/03/2024 04:54, Zbigniew Kempczyński wrote:
> On Tue, Mar 26, 2024 at 08:25:56AM +0000, Matthew Auld wrote:
>> On 26/03/2024 10:10, Akshata Jahagirdar wrote:
>>> Currently, we don't have any check in IGT testcases if platform supports
>>> compression or not. Send this flat_ccs value as part of the struct
>>> drm_xe_query_config so that we can read this value in the igt testcases
>>> and proceed accordingly.
>>
>> AFAIK it's not valid to add uAPI just for an IGT. uAPI normally needs a real
>> open source user (like Mesa etc) with UMD acks. However if this is going to
>> be used by Mesa etc, how do they tell the difference between "this device
>> doesn't have flat-CCS" and "this KMD version doesn't support
>> FLAG_HAS_FLAT_CCS"?
>>
>> If userspace doesn't need this, then other option could be debugfs. Or if
>> this just a single test perhaps we can just dynamically check if the CCS
>> state is zero for compressed buffer at the start of the test?
> 
> Just for making sure - is ctrl-surf-copy behavior defined if there's no
> flat ccs?

Yeah, claim is that HW will never compress anything, writes to CCS are 
dropped and reads from CCS return zero. KMD should also skip any CCS 
related stuff.

> 
> --
> Zbigniew
>>
>>> Fixes: VLK-56136
>>>
>>> Signed-off-by: Akshata Jahagirdar <akshata.jahagirdar@intel.com>
>>> ---
>>>    drivers/gpu/drm/xe/xe_query.c | 3 +++
>>>    include/uapi/drm/xe_drm.h     | 1 +
>>>    2 files changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
>>> index e80321b34918..8a3a3117491c 100644
>>> --- a/drivers/gpu/drm/xe/xe_query.c
>>> +++ b/drivers/gpu/drm/xe/xe_query.c
>>> @@ -332,6 +332,9 @@ static int query_config(struct xe_device *xe, struct drm_xe_device_query *query)
>>>    	config->num_params = num_params;
>>>    	config->info[DRM_XE_QUERY_CONFIG_REV_AND_DEVICE_ID] =
>>>    		xe->info.devid | (xe->info.revid << 16);
>>> +	if (xe->info.has_flat_ccs)
>>> +		config->info[DRM_XE_QUERY_CONFIG_FLAGS] =
>>> +			DRM_XE_QUERY_CONFIG_FLAG_HAS_FLAT_CCS;
>>>    	if (xe_device_get_root_tile(xe)->mem.vram.usable_size)
>>>    		config->info[DRM_XE_QUERY_CONFIG_FLAGS] =
>>
>> This tramples the previous value AFAICT, like when we have VRAM + CCS on
>> DG2. I think we need |= here.
>>
>>>    			DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM;
>>> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
>>> index f44a19ae9333..08783234bae1 100644
>>> --- a/include/uapi/drm/xe_drm.h
>>> +++ b/include/uapi/drm/xe_drm.h
>>> @@ -405,6 +405,7 @@ struct drm_xe_query_config {
>>>    #define DRM_XE_QUERY_CONFIG_REV_AND_DEVICE_ID	0
>>>    #define DRM_XE_QUERY_CONFIG_FLAGS			1
>>>    	#define DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM	(1 << 0)
>>> +	#define DRM_XE_QUERY_CONFIG_FLAG_HAS_FLAT_CCS	(1 << 1)
>>>    #define DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT		2
>>>    #define DRM_XE_QUERY_CONFIG_VA_BITS			3
>>>    #define DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY	4