[PATCH i-g-t v1 0/3] Add check to ensure platform supports compression

[PATCH i-g-t v1 0/3] Add check to ensure platform supports compression

[Akshata Jahagirdar]

Currently, the igt testcases don't have any check to ensure the platform
supports compression (has flat ccs enabled) before running the test.
Add this check by reading the flat_ccs value received from DRM_IOCTL_XE_DEVICE_QUERY 
ioctl and proceed accordingly.

KMD-changes patch: https://patchwork.freedesktop.org/series/131595/
Fixes : VLK-56136

Akshata Jahagirdar (2):
  lib/intel_blt: Add helper function to ensure platform supports
    compression
  tests/xe_ccs: Check if platform supports compression
  drm-uapi/xe_drm: Read flat ccs value from drm_xe_query_config

 include/drm-uapi/xe_drm.h |  3 +++
 lib/intel_blt.c           | 18 ++++++++++++++++++
 lib/intel_blt.h           |  1 +
 tests/intel/xe_ccs.c      |  1 +
 4 files changed, 23 insertions(+)

-- 
2.34.1

✗ Fi.CI.BUILD: failure for Add check to ensure platform supports compression

[Patchwork]

== Series Details ==

Series: Add check to ensure platform supports compression
URL   : https://patchwork.freedesktop.org/series/131607/
State : failure

== Summary ==

Applying: drm-uapi/xe_drm: Read flat ccs value from drm_xe_query_config
Applying: lib/intel_blt: Add helper function to ensure platform supports compression
Applying: tests/xe_ccs: Check if platform supports compression
Patch failed at 0003 tests/xe_ccs: Check if platform supports compression
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

[PATCH i-g-t v1 1/3] drm-uapi/xe_drm: Read flat ccs value from drm_xe_query_config

[Akshata Jahagirdar]

Add an additional drm_xe_query_config_flag for reading the value
of flat_ccs of the platform.
This can be used in respective igt testcases.

Signed-off-by: Akshata Jahagirdar <akshata.jahagirdar@intel.com>
---
 include/drm-uapi/xe_drm.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/drm-uapi/xe_drm.h b/include/drm-uapi/xe_drm.h
index 4353595a4..b43c8cf76 100644
--- a/include/drm-uapi/xe_drm.h
+++ b/include/drm-uapi/xe_drm.h
@@ -389,6 +389,8 @@ struct drm_xe_query_mem_regions {
  *
  *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM - Flag is set if the device
  *      has usable VRAM
+ *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_FLAT_CCS - Flag is set if the device
+ *      has flat ccs enabled
  *  - %DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT - Minimal memory alignment
  *    required by this device, typically SZ_4K or SZ_64K
  *  - %DRM_XE_QUERY_CONFIG_VA_BITS - Maximum bits of a virtual address
@@ -405,6 +407,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
-- 
2.34.1

Re: [PATCH i-g-t v1 1/3] drm-uapi/xe_drm: Read flat ccs value from drm_xe_query_config

[Zbigniew Kempczyński]

On Tue, Mar 26, 2024 at 02:54:36AM -0700, Akshata Jahagirdar wrote:
> Add an additional drm_xe_query_config_flag for reading the value
> of flat_ccs of the platform.
> This can be used in respective igt testcases.
> 
> Signed-off-by: Akshata Jahagirdar <akshata.jahagirdar@intel.com>
> ---
>  include/drm-uapi/xe_drm.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/drm-uapi/xe_drm.h b/include/drm-uapi/xe_drm.h
> index 4353595a4..b43c8cf76 100644
> --- a/include/drm-uapi/xe_drm.h
> +++ b/include/drm-uapi/xe_drm.h
> @@ -389,6 +389,8 @@ struct drm_xe_query_mem_regions {
>   *
>   *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM - Flag is set if the device
>   *      has usable VRAM
> + *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_FLAT_CCS - Flag is set if the device
> + *      has flat ccs enabled
>   *  - %DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT - Minimal memory alignment
>   *    required by this device, typically SZ_4K or SZ_64K
>   *  - %DRM_XE_QUERY_CONFIG_VA_BITS - Maximum bits of a virtual address
> @@ -405,6 +407,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)

May you point me out which kernel patch includes this flag into
xe_query.c?

--
Zbigniew

>  #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
> -- 
> 2.34.1
> 

Re: [PATCH i-g-t v1 1/3] drm-uapi/xe_drm: Read flat ccs value from drm_xe_query_config

[Jahagirdar, Akshata]

On 3/25/2024 10:19 PM, Zbigniew Kempczyński wrote:
> On Tue, Mar 26, 2024 at 02:54:36AM -0700, Akshata Jahagirdar wrote:
>> Add an additional drm_xe_query_config_flag for reading the value
>> of flat_ccs of the platform.
>> This can be used in respective igt testcases.
>>
>> Signed-off-by: Akshata Jahagirdar <akshata.jahagirdar@intel.com>
>> ---
>>   include/drm-uapi/xe_drm.h | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/include/drm-uapi/xe_drm.h b/include/drm-uapi/xe_drm.h
>> index 4353595a4..b43c8cf76 100644
>> --- a/include/drm-uapi/xe_drm.h
>> +++ b/include/drm-uapi/xe_drm.h
>> @@ -389,6 +389,8 @@ struct drm_xe_query_mem_regions {
>>    *
>>    *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM - Flag is set if the device
>>    *      has usable VRAM
>> + *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_FLAT_CCS - Flag is set if the device
>> + *      has flat ccs enabled
>>    *  - %DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT - Minimal memory alignment
>>    *    required by this device, typically SZ_4K or SZ_64K
>>    *  - %DRM_XE_QUERY_CONFIG_VA_BITS - Maximum bits of a virtual address
>> @@ -405,6 +407,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)
> May you point me out which kernel patch includes this flag into
> xe_query.c?
>
> --
> Zbigniew

https://patchwork.freedesktop.org/series/131609/

-- Akshata

>>   #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
>> -- 
>> 2.34.1
>>

Re: [PATCH i-g-t v1 1/3] drm-uapi/xe_drm: Read flat ccs value from drm_xe_query_config

[Zbigniew Kempczyński]

On Mon, Mar 25, 2024 at 10:29:26PM -0700, Jahagirdar, Akshata wrote:
> 
> On 3/25/2024 10:19 PM, Zbigniew Kempczyński wrote:
> > On Tue, Mar 26, 2024 at 02:54:36AM -0700, Akshata Jahagirdar wrote:
> > > Add an additional drm_xe_query_config_flag for reading the value
> > > of flat_ccs of the platform.
> > > This can be used in respective igt testcases.
> > > 
> > > Signed-off-by: Akshata Jahagirdar <akshata.jahagirdar@intel.com>
> > > ---
> > >   include/drm-uapi/xe_drm.h | 3 +++
> > >   1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/include/drm-uapi/xe_drm.h b/include/drm-uapi/xe_drm.h
> > > index 4353595a4..b43c8cf76 100644
> > > --- a/include/drm-uapi/xe_drm.h
> > > +++ b/include/drm-uapi/xe_drm.h
> > > @@ -389,6 +389,8 @@ struct drm_xe_query_mem_regions {
> > >    *
> > >    *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM - Flag is set if the device
> > >    *      has usable VRAM
> > > + *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_FLAT_CCS - Flag is set if the device
> > > + *      has flat ccs enabled
> > >    *  - %DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT - Minimal memory alignment
> > >    *    required by this device, typically SZ_4K or SZ_64K
> > >    *  - %DRM_XE_QUERY_CONFIG_VA_BITS - Maximum bits of a virtual address
> > > @@ -405,6 +407,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)
> > May you point me out which kernel patch includes this flag into
> > xe_query.c?
> > 
> > --
> > Zbigniew
> 
> https://patchwork.freedesktop.org/series/131609/

Ok, I've found it already and replied.

Anyway your igt changes won't compile until above change will be
merged. UAPI requires some userspace user, but according to possibility
of turning off flat-ccs in BIOS I think UMDs will be interested to
examine of this flag. Generally this is chicken-and-egg problem -
igts require changes in the kernel which in turn requires userspace
user (which at the moment doesn't exist as this uapi is new).
We introduced some auxiliary files in igt (see i915_drm_local.h or
i915_pciids_local.h) to put necessary definitions there allowing
igts to compile. And when kernel change is merged updating uapi
along with removal of this temporary definitions from "local" files.

But to avoid double work I would just wait for Xe maintainers
opinion about new UAPI. I guess they will ack it and this flag
will be exposed.

--
Zbigniew

> 
> -- Akshata
> 
> > >   #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
> > > -- 
> > > 2.34.1
> > > 

[PATCH i-g-t v1 2/3] lib/intel_blt: Add helper function to ensure platform supports compression

[Akshata Jahagirdar]

Add helper function to check if platform supports compression by
reading the flat_ccs value from struct drm_xe_query_config.

Signed-off-by: Akshata Jahagirdar <akshata.jahagirdar@intel.com>
---
 lib/intel_blt.c | 18 ++++++++++++++++++
 lib/intel_blt.h |  1 +
 2 files changed, 19 insertions(+)

diff --git a/lib/intel_blt.c b/lib/intel_blt.c
index fe0a45cb8..076b065c0 100644
--- a/lib/intel_blt.c
+++ b/lib/intel_blt.c
@@ -464,6 +464,24 @@ bool blt_block_copy_supports_compression(int fd)
 				    BLT_CMD_SUPPORTS_COMPRESSION);
 }
 
+/**
+ * blt_platform_supports_compression
+ * @fd: drm fd
+ *
+ * Check if platform provided by @fd device supports compression.
+ *
+ * Returns:
+ * true if it does, false otherwise.
+ */
+bool blt_platform_supports_compression(int fd)
+{
+	struct drm_xe_query_config *config = xe_config(fd);
+
+	igt_assert(config);
+	return  config->info[DRM_XE_QUERY_CONFIG_FLAGS] &
+		DRM_XE_QUERY_CONFIG_FLAG_HAS_FLAT_CCS;
+}
+
 /**
  * blt_uses_extended_block_copy
  * @fd: drm fd
diff --git a/lib/intel_blt.h b/lib/intel_blt.h
index 1f6c71359..a2c4a4adb 100644
--- a/lib/intel_blt.h
+++ b/lib/intel_blt.h
@@ -209,6 +209,7 @@ bool blt_fast_copy_supports_tiling(int fd, enum blt_tiling_type tiling);
 bool blt_block_copy_supports_tiling(int fd, enum blt_tiling_type tiling);
 bool blt_xy_src_copy_supports_tiling(int fd, enum blt_tiling_type tiling);
 bool blt_block_copy_supports_compression(int fd);
+bool blt_platform_supports_compression(int fd);
 bool blt_uses_extended_block_copy(int fd);
 
 const char *blt_tiling_name(enum blt_tiling_type tiling);
-- 
2.34.1

[PATCH i-g-t v1 3/3] tests/xe_ccs: Check if platform supports compression

[Akshata Jahagirdar]

Add check before running the test to ensure the platform supports
compression and has flat ccs enabled.

Signed-off-by: Akshata Jahagirdar <akshata.jahagirdar@intel.com>
---
 tests/intel/xe_ccs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/intel/xe_ccs.c b/tests/intel/xe_ccs.c
index 8fe1f62eb..b424a1d6d 100644
--- a/tests/intel/xe_ccs.c
+++ b/tests/intel/xe_ccs.c
@@ -718,6 +718,7 @@ igt_main_args("bf:pst:W:H:", NULL, help_str, opt_handler, NULL)
 	igt_fixture {
 		xe = drm_open_driver(DRIVER_XE);
 		igt_require(blt_has_block_copy(xe));
+		igt_require(blt_platform_supports_compression(xe));
 		if (AT_LEAST_GEN(xe, 20))
 			igt_require(HAS_FLATCCS(xe));
 
-- 
2.34.1

Re: [PATCH i-g-t v1 3/3] tests/xe_ccs: Check if platform supports compression

[Matthew Auld]

On 26/03/2024 09:54, Akshata Jahagirdar wrote:
> Add check before running the test to ensure the platform supports
> compression and has flat ccs enabled.
> 
> Signed-off-by: Akshata Jahagirdar <akshata.jahagirdar@intel.com>
> ---
>   tests/intel/xe_ccs.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/tests/intel/xe_ccs.c b/tests/intel/xe_ccs.c
> index 8fe1f62eb..b424a1d6d 100644
> --- a/tests/intel/xe_ccs.c
> +++ b/tests/intel/xe_ccs.c
> @@ -718,6 +718,7 @@ igt_main_args("bf:pst:W:H:", NULL, help_str, opt_handler, NULL)
>   	igt_fixture {
>   		xe = drm_open_driver(DRIVER_XE);
>   		igt_require(blt_has_block_copy(xe));
> +		igt_require(blt_platform_supports_compression(xe));

At least on Xe2 I think we still need some kind validation here, since 
this is a valid user config. IIRC HW claims that we shouldn't really 
care that flat-ccs is for example disabled by BIOS on LNL. Everything 
should "just work", given that the CCS state is always zero. IMO test 
should be updated to account for xe2 && compression_disabled to verify 
that CCS state is indeed always zero on such a config.

>   		if (AT_LEAST_GEN(xe, 20))
>   			igt_require(HAS_FLATCCS(xe));
>