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

[Zbigniew Kempczyński]

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?

--
Zbigniew
> 
> > Fixes: VLK-56136
> > 
> > Signed-off-by: Akshata Jahagirdar <akshata.jahagirdar at 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