[igt-dev] [PATCH i-g-t 0/2] This patch series enables the compression feature for XE2 Lunarlake

[igt-dev] [PATCH i-g-t 0/2] This patch series enables the compression feature for XE2 Lunarlake

[Jahagirdar, Akshata]

*** BLURB HERE ***

Jahagirdar, Akshata (2):
  XE2: LNL: adding Blt command support for Lunarlake
  XE2: xe_ccs IGT changes to enable  xe2 igfx compression

 lib/intel_blt.c         | 18 ++++++++++--------
 lib/intel_blt.h         |  3 ++-
 lib/intel_chipset.h     |  1 +
 lib/intel_cmds_info.c   | 24 ++++++++++++++++++++++++
 lib/intel_cmds_info.h   |  1 +
 lib/intel_device_info.c |  2 +-
 lib/intel_pat.c         | 12 +++++++++++-
 lib/intel_pat.h         |  1 +
 tests/intel/gem_ccs.c   |  2 +-
 tests/intel/xe_ccs.c    | 37 ++++++++++++++++++-------------------
 10 files changed, 70 insertions(+), 31 deletions(-)

-- 
2.34.1

[igt-dev] ✗ Fi.CI.BUILD: failure for This patch series enables the compression feature for XE2 Lunarlake

[Patchwork]

== Series Details ==

Series: This patch series enables the compression feature for XE2 Lunarlake
URL   : https://patchwork.freedesktop.org/series/126124/
State : failure

== Summary ==

Applying: XE2: LNL: adding Blt command support for Lunarlake
Applying: XE2: xe_ccs IGT changes to enable xe2 igfx compression
Using index info to reconstruct a base tree...
M	lib/intel_blt.c
M	lib/intel_blt.h
A	lib/intel_pat.c
A	lib/intel_pat.h
M	tests/intel/gem_ccs.c
M	tests/intel/xe_ccs.c
Falling back to patching base and 3-way merge...
Auto-merging tests/intel/xe_ccs.c
CONFLICT (content): Merge conflict in tests/intel/xe_ccs.c
Auto-merging tests/intel/gem_ccs.c
CONFLICT (modify/delete): lib/intel_pat.h deleted in HEAD and modified in XE2: xe_ccs IGT changes to enable xe2 igfx compression. Version XE2: xe_ccs IGT changes to enable xe2 igfx compression of lib/intel_pat.h left in tree.
CONFLICT (modify/delete): lib/intel_pat.c deleted in HEAD and modified in XE2: xe_ccs IGT changes to enable xe2 igfx compression. Version XE2: xe_ccs IGT changes to enable xe2 igfx compression of lib/intel_pat.c left in tree.
Auto-merging lib/intel_blt.h
Auto-merging lib/intel_blt.c
CONFLICT (content): Merge conflict in lib/intel_blt.c
Patch failed at 0002 XE2: xe_ccs IGT changes to enable xe2 igfx 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".

[igt-dev] [PATCH i-g-t 1/2] XE2: LNL: adding Blt command support for Lunarlake

[Jahagirdar, Akshata]

---
 lib/intel_cmds_info.c   | 24 ++++++++++++++++++++++++
 lib/intel_cmds_info.h   |  1 +
 lib/intel_device_info.c |  2 +-
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/lib/intel_cmds_info.c b/lib/intel_cmds_info.c
index 2e51ec081..fc3554271 100644
--- a/lib/intel_cmds_info.c
+++ b/lib/intel_cmds_info.c
@@ -67,6 +67,22 @@ static const struct blt_cmd_info
 						 BLT_CMD_EXTENDED |
 						 BLT_CMD_SUPPORTS_COMPRESSION);
 
+static const struct blt_cmd_info				
+		xe2_xy_block_copy = BLT_INFO_EXT(XY_BLOCK_COPY,
+						 BIT(T_LINEAR) |
+						 BIT(T_XMAJOR) |
+						 BIT(T_TILE4)  |
+						 BIT(T_TILE64),
+						 BLT_CMD_EXTENDED |
+						 BLT_CMD_SUPPORTS_COMPRESSION);
+
+static const struct blt_cmd_info
+		xe2_xy_fast_copy = BLT_INFO(XY_FAST_COPY,
+					    BIT(T_LINEAR) |
+					    BIT(T_XMAJOR) |
+					    BIT(T_TILE4)  |
+					    BIT(T_TILE64));
+
 static const struct blt_cmd_info
 		mtl_xy_block_copy = BLT_INFO_EXT(XY_BLOCK_COPY,
 						 BIT(T_LINEAR) |
@@ -169,6 +185,14 @@ const struct intel_cmds_info gen12_pvc_cmds_info = {
 	}
 };
 
+
+const struct intel_cmds_info xe2_cmds_info  = {
+	.blt_cmds = {
+		[XY_FAST_COPY] = &xe2_xy_fast_copy,
+		[XY_BLOCK_COPY] = &xe2_xy_block_copy,
+	}
+};
+
 const struct blt_cmd_info *blt_get_cmd_info(const struct intel_cmds_info *cmds_info,
 					    enum blt_cmd_type cmd)
 {
diff --git a/lib/intel_cmds_info.h b/lib/intel_cmds_info.h
index f9e3932d1..0a83b6a44 100644
--- a/lib/intel_cmds_info.h
+++ b/lib/intel_cmds_info.h
@@ -55,6 +55,7 @@ extern const struct intel_cmds_info gen12_cmds_info;
 extern const struct intel_cmds_info gen12_dg2_cmds_info;
 extern const struct intel_cmds_info gen12_mtl_cmds_info;
 extern const struct intel_cmds_info gen12_pvc_cmds_info;
+extern const struct intel_cmds_info xe2_cmds_info;
 
 #define for_each_tiling(__tiling) \
 	for (__tiling = T_LINEAR; __tiling < __BLT_MAX_TILING; __tiling++)
diff --git a/lib/intel_device_info.c b/lib/intel_device_info.c
index 34817f7b6..a669797c3 100644
--- a/lib/intel_device_info.c
+++ b/lib/intel_device_info.c
@@ -511,7 +511,7 @@ static const struct intel_device_info intel_lunarlake_info = {
 	.has_4tile = true,
 	.is_lunarlake = true,
 	.codename = "lunarlake",
-	.cmds_info = &gen12_pvc_cmds_info,
+	.cmds_info = &xe2_cmds_info,
 };
 
 static const struct pci_id_match intel_device_match[] = {
-- 
2.34.1

Re: [igt-dev] [PATCH i-g-t 1/2] XE2: LNL: adding Blt command support for Lunarlake

[Kamil Konieczny]

Hi,

please correct subject of you mail/patch, now it reads:

[PATCH i-g-t 1/2] XE2: LNL: adding Blt command support for Lunarlake

but it should be like:

[PATCH i-g-t 1/2] lib: adding Blt command support for Lunarlake

Add (short) description here what did you added, for example
what command did you add.

Add proper From: and s-o-b:, now I see:

Author: Jahagirdar, Akshata <akshata.jahagirdar@intel.com>

it should be:

Author: Akshata Jahagirdar <akshata.jahagirdar@intel.com>

At the end of patch add s-o-b like:

Signed-off-by: Akshata Jahagirdar <akshata.jahagirdar@intel.com>

Regards,
Kamil

On 2023-11-08 at 11:57:05 -0800, Jahagirdar, Akshata wrote:
> ---
>  lib/intel_cmds_info.c   | 24 ++++++++++++++++++++++++
>  lib/intel_cmds_info.h   |  1 +
>  lib/intel_device_info.c |  2 +-
>  3 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/intel_cmds_info.c b/lib/intel_cmds_info.c
> index 2e51ec081..fc3554271 100644
> --- a/lib/intel_cmds_info.c
> +++ b/lib/intel_cmds_info.c
> @@ -67,6 +67,22 @@ static const struct blt_cmd_info
>  						 BLT_CMD_EXTENDED |
>  						 BLT_CMD_SUPPORTS_COMPRESSION);
>  
> +static const struct blt_cmd_info				
> +		xe2_xy_block_copy = BLT_INFO_EXT(XY_BLOCK_COPY,
> +						 BIT(T_LINEAR) |
> +						 BIT(T_XMAJOR) |
> +						 BIT(T_TILE4)  |
> +						 BIT(T_TILE64),
> +						 BLT_CMD_EXTENDED |
> +						 BLT_CMD_SUPPORTS_COMPRESSION);
> +
> +static const struct blt_cmd_info
> +		xe2_xy_fast_copy = BLT_INFO(XY_FAST_COPY,
> +					    BIT(T_LINEAR) |
> +					    BIT(T_XMAJOR) |
> +					    BIT(T_TILE4)  |
> +					    BIT(T_TILE64));
> +
>  static const struct blt_cmd_info
>  		mtl_xy_block_copy = BLT_INFO_EXT(XY_BLOCK_COPY,
>  						 BIT(T_LINEAR) |
> @@ -169,6 +185,14 @@ const struct intel_cmds_info gen12_pvc_cmds_info = {
>  	}
>  };
>  
> +
> +const struct intel_cmds_info xe2_cmds_info  = {
> +	.blt_cmds = {
> +		[XY_FAST_COPY] = &xe2_xy_fast_copy,
> +		[XY_BLOCK_COPY] = &xe2_xy_block_copy,
> +	}
> +};
> +
>  const struct blt_cmd_info *blt_get_cmd_info(const struct intel_cmds_info *cmds_info,
>  					    enum blt_cmd_type cmd)
>  {
> diff --git a/lib/intel_cmds_info.h b/lib/intel_cmds_info.h
> index f9e3932d1..0a83b6a44 100644
> --- a/lib/intel_cmds_info.h
> +++ b/lib/intel_cmds_info.h
> @@ -55,6 +55,7 @@ extern const struct intel_cmds_info gen12_cmds_info;
>  extern const struct intel_cmds_info gen12_dg2_cmds_info;
>  extern const struct intel_cmds_info gen12_mtl_cmds_info;
>  extern const struct intel_cmds_info gen12_pvc_cmds_info;
> +extern const struct intel_cmds_info xe2_cmds_info;
>  
>  #define for_each_tiling(__tiling) \
>  	for (__tiling = T_LINEAR; __tiling < __BLT_MAX_TILING; __tiling++)
> diff --git a/lib/intel_device_info.c b/lib/intel_device_info.c
> index 34817f7b6..a669797c3 100644
> --- a/lib/intel_device_info.c
> +++ b/lib/intel_device_info.c
> @@ -511,7 +511,7 @@ static const struct intel_device_info intel_lunarlake_info = {
>  	.has_4tile = true,
>  	.is_lunarlake = true,
>  	.codename = "lunarlake",
> -	.cmds_info = &gen12_pvc_cmds_info,
> +	.cmds_info = &xe2_cmds_info,
>  };
>  
>  static const struct pci_id_match intel_device_match[] = {
> -- 
> 2.34.1
> 

Re: [igt-dev] [PATCH i-g-t 1/2] XE2: LNL: adding Blt command support for Lunarlake

[Jahagirdar, Akshata]

Will fix this,
Thanks

-----Original Message-----
From: Kamil Konieczny <kamil.konieczny@linux.intel.com> 
Sent: Wednesday, November 8, 2023 6:37 AM
To: Jahagirdar, Akshata <akshata.jahagirdar@intel.com>
Cc: igt-dev@lists.freedesktop.org; Siddiqui, Ayaz A <ayaz.siddiqui@intel.com>
Subject: Re: [igt-dev] [PATCH i-g-t 1/2] XE2: LNL: adding Blt command support for Lunarlake

Hi,

please correct subject of you mail/patch, now it reads:

[PATCH i-g-t 1/2] XE2: LNL: adding Blt command support for Lunarlake

but it should be like:

[PATCH i-g-t 1/2] lib: adding Blt command support for Lunarlake

Add (short) description here what did you added, for example what command did you add.

Add proper From: and s-o-b:, now I see:

Author: Jahagirdar, Akshata <akshata.jahagirdar@intel.com>

it should be:

Author: Akshata Jahagirdar <akshata.jahagirdar@intel.com>

At the end of patch add s-o-b like:

Signed-off-by: Akshata Jahagirdar <akshata.jahagirdar@intel.com>

Regards,
Kamil

On 2023-11-08 at 11:57:05 -0800, Jahagirdar, Akshata wrote:
> ---
>  lib/intel_cmds_info.c   | 24 ++++++++++++++++++++++++
>  lib/intel_cmds_info.h   |  1 +
>  lib/intel_device_info.c |  2 +-
>  3 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/intel_cmds_info.c b/lib/intel_cmds_info.c index 
> 2e51ec081..fc3554271 100644
> --- a/lib/intel_cmds_info.c
> +++ b/lib/intel_cmds_info.c
> @@ -67,6 +67,22 @@ static const struct blt_cmd_info
>  						 BLT_CMD_EXTENDED |
>  						 BLT_CMD_SUPPORTS_COMPRESSION);
>  
> +static const struct blt_cmd_info				
> +		xe2_xy_block_copy = BLT_INFO_EXT(XY_BLOCK_COPY,
> +						 BIT(T_LINEAR) |
> +						 BIT(T_XMAJOR) |
> +						 BIT(T_TILE4)  |
> +						 BIT(T_TILE64),
> +						 BLT_CMD_EXTENDED |
> +						 BLT_CMD_SUPPORTS_COMPRESSION);
> +
> +static const struct blt_cmd_info
> +		xe2_xy_fast_copy = BLT_INFO(XY_FAST_COPY,
> +					    BIT(T_LINEAR) |
> +					    BIT(T_XMAJOR) |
> +					    BIT(T_TILE4)  |
> +					    BIT(T_TILE64));
> +
>  static const struct blt_cmd_info
>  		mtl_xy_block_copy = BLT_INFO_EXT(XY_BLOCK_COPY,
>  						 BIT(T_LINEAR) |
> @@ -169,6 +185,14 @@ const struct intel_cmds_info gen12_pvc_cmds_info = {
>  	}
>  };
>  
> +
> +const struct intel_cmds_info xe2_cmds_info  = {
> +	.blt_cmds = {
> +		[XY_FAST_COPY] = &xe2_xy_fast_copy,
> +		[XY_BLOCK_COPY] = &xe2_xy_block_copy,
> +	}
> +};
> +
>  const struct blt_cmd_info *blt_get_cmd_info(const struct intel_cmds_info *cmds_info,
>  					    enum blt_cmd_type cmd)
>  {
> diff --git a/lib/intel_cmds_info.h b/lib/intel_cmds_info.h index 
> f9e3932d1..0a83b6a44 100644
> --- a/lib/intel_cmds_info.h
> +++ b/lib/intel_cmds_info.h
> @@ -55,6 +55,7 @@ extern const struct intel_cmds_info gen12_cmds_info;  
> extern const struct intel_cmds_info gen12_dg2_cmds_info;  extern const 
> struct intel_cmds_info gen12_mtl_cmds_info;  extern const struct 
> intel_cmds_info gen12_pvc_cmds_info;
> +extern const struct intel_cmds_info xe2_cmds_info;
>  
>  #define for_each_tiling(__tiling) \
>  	for (__tiling = T_LINEAR; __tiling < __BLT_MAX_TILING; __tiling++) 
> diff --git a/lib/intel_device_info.c b/lib/intel_device_info.c index 
> 34817f7b6..a669797c3 100644
> --- a/lib/intel_device_info.c
> +++ b/lib/intel_device_info.c
> @@ -511,7 +511,7 @@ static const struct intel_device_info intel_lunarlake_info = {
>  	.has_4tile = true,
>  	.is_lunarlake = true,
>  	.codename = "lunarlake",
> -	.cmds_info = &gen12_pvc_cmds_info,
> +	.cmds_info = &xe2_cmds_info,
>  };
>  
>  static const struct pci_id_match intel_device_match[] = {
> --
> 2.34.1
> 

[igt-dev] [PATCH i-g-t 2/2] XE2: xe_ccs IGT changes to enable xe2 igfx compression

[Jahagirdar, Akshata]

---
 lib/intel_blt.c       | 18 ++++++++++--------
 lib/intel_blt.h       |  3 ++-
 lib/intel_chipset.h   |  1 +
 lib/intel_pat.c       | 12 +++++++++++-
 lib/intel_pat.h       |  1 +
 tests/intel/gem_ccs.c |  2 +-
 tests/intel/xe_ccs.c  | 37 ++++++++++++++++++-------------------
 7 files changed, 44 insertions(+), 30 deletions(-)

diff --git a/lib/intel_blt.c b/lib/intel_blt.c
index c0593930c..c60d9fb1e 100644
--- a/lib/intel_blt.c
+++ b/lib/intel_blt.c
@@ -948,15 +948,16 @@ int blt_block_copy(int fd,
 	return ret;
 }
 
-static uint16_t __ccs_size(const struct blt_ctrl_surf_copy_data *surf)
+static uint16_t __ccs_size(int fd, const struct blt_ctrl_surf_copy_data *surf)
 {
 	uint32_t src_size, dst_size;
+	uint16_t ccsratio = CCS_RATIO(fd);
 
 	src_size = surf->src.access_type == DIRECT_ACCESS ?
-				surf->src.size : surf->src.size / CCS_RATIO;
+				surf->src.size : surf->src.size / ccsratio;
 
 	dst_size = surf->dst.access_type == DIRECT_ACCESS ?
-				surf->dst.size : surf->dst.size / CCS_RATIO;
+				surf->dst.size : surf->dst.size / ccsratio;
 
 	igt_assert_f(src_size <= dst_size, "dst size must be >= src size for CCS copy\n");
 
@@ -1136,7 +1137,7 @@ uint64_t emit_blt_ctrl_surf_copy(int fd,
 		data.xe2.dw00.dst_access_type = surf->dst.access_type;
 
 		/* Ensure dst has size capable to keep src ccs aux */
-		data.xe2.dw00.size_of_ctrl_copy = __ccs_size(surf) / CCS_RATIO - 1;
+		data.xe2.dw00.size_of_ctrl_copy = __ccs_size(fd, surf) / NUM_4KPAGES_OF_CCS - 1;
 		data.xe2.dw00.length = 0x3;
 
 		data.xe2.dw01.src_address_lo = src_offset;
@@ -1155,7 +1156,7 @@ uint64_t emit_blt_ctrl_surf_copy(int fd,
 		data.gen12.dw00.dst_access_type = surf->dst.access_type;
 
 		/* Ensure dst has size capable to keep src ccs aux */
-		data.gen12.dw00.size_of_ctrl_copy = __ccs_size(surf) / CCS_RATIO - 1;
+		data.gen12.dw00.size_of_ctrl_copy = __ccs_size(fd,surf) / (CCS_RATIO(fd)) - 1;
 		data.gen12.dw00.length = 0x3;
 
 		data.gen12.dw01.src_address_lo = src_offset;
@@ -1794,7 +1795,8 @@ blt_create_object(const struct blt_copy_data *blt, uint32_t region,
 	uint64_t size = width * height * bpp / 8;
 	uint32_t stride = tiling == T_LINEAR ? width * 4 : width;
 	uint32_t handle;
-
+	uint8_t pat_index = compression ? intel_get_pat_idx_uc_comp(blt->fd) : intel_get_pat_idx_uc(blt->fd);
+	uint16_t cpu_caching = DRM_XE_GEM_CPU_CACHING_WC;
 	igt_assert_f(blt->driver, "Driver isn't set, have you called blt_copy_init()?\n");
 
 	obj = calloc(1, sizeof(*obj));
@@ -1808,13 +1810,13 @@ blt_create_object(const struct blt_copy_data *blt, uint32_t region,
 			flags |= XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM;
 
 		size = ALIGN(size, xe_get_default_alignment(blt->fd));
-		handle = xe_bo_create_flags(blt->fd, 0, size, flags);
+		handle = xe_bo_create_caching(blt->fd, 0, size, region, cpu_caching);
 	} else {
 		igt_assert(__gem_create_in_memory_regions(blt->fd, &handle,
 							  &size, region) == 0);
 	}
 
-	blt_set_object(obj, handle, size, region, mocs_index, DEFAULT_PAT_INDEX, tiling,
+	blt_set_object(obj, handle, size, region, mocs_index, pat_index, tiling,
 		       compression, compression_type);
 	blt_set_geom(obj, stride, 0, 0, width, height, 0, 0);
 
diff --git a/lib/intel_blt.h b/lib/intel_blt.h
index 5934ccd67..222c44074 100644
--- a/lib/intel_blt.h
+++ b/lib/intel_blt.h
@@ -52,7 +52,8 @@
 #include "igt.h"
 #include "intel_cmds_info.h"
 
-#define CCS_RATIO 256
+#define CCS_RATIO(xe) IS_XE2(intel_get_drm_devid(xe)) ? 512 : 256
+#define NUM_4KPAGES_OF_CCS 8
 
 enum blt_color_depth {
 	CD_8bit,
diff --git a/lib/intel_chipset.h b/lib/intel_chipset.h
index 1edc8e92c..aca429a97 100644
--- a/lib/intel_chipset.h
+++ b/lib/intel_chipset.h
@@ -205,6 +205,7 @@ void intel_check_pch(void);
 #define IS_METEORLAKE(devid)	(intel_get_device_info(devid)->is_meteorlake)
 #define IS_PONTEVECCHIO(devid)	(intel_get_device_info(devid)->is_pontevecchio)
 #define IS_LUNARLAKE(devid)	(intel_get_device_info(devid)->is_lunarlake)
+#define IS_XE2(devid)		(intel_get_device_info(devid)->graphics_ver >= 20)
 
 #define IS_GEN(devid, x)	(intel_get_device_info(devid)->graphics_ver == x)
 #define AT_LEAST_GEN(devid, x)	(intel_get_device_info(devid)->graphics_ver >= x)
diff --git a/lib/intel_pat.c b/lib/intel_pat.c
index 2b892ee52..ccda6b786 100644
--- a/lib/intel_pat.c
+++ b/lib/intel_pat.c
@@ -11,7 +11,7 @@ struct intel_pat_cache {
 	uint8_t uc; /* UC + COH_NONE */
 	uint8_t wt; /* WT + COH_NONE */
 	uint8_t wb; /* WB + COH_AT_LEAST_1WAY */
-
+	uint8_t uc_comp; /* UC + COH_NONE + COMPRESSION*/
 	uint8_t max_index;
 };
 
@@ -23,6 +23,7 @@ static void intel_get_pat_idx(int fd, struct intel_pat_cache *pat)
 		pat->uc = 3;
 		pat->wt = 15; /* Compressed + WB-transient */
 		pat->wb = 2;
+		pat->uc_comp = 12; /* Compressed + UC */
 		pat->max_index = 31;
 	} else if (IS_METEORLAKE(dev_id)) {
 		pat->uc = 2;
@@ -60,6 +61,15 @@ uint8_t intel_get_pat_idx_uc(int fd)
 	return pat.uc;
 }
 
+uint8_t intel_get_pat_idx_uc_comp(int fd)
+{
+	struct intel_pat_cache pat = {};
+	uint16_t dev_id = intel_get_drm_devid(fd);
+	
+	intel_get_pat_idx(fd, &pat);
+	return (intel_get_device_info(dev_id)->graphics_ver == 20) ? pat.uc_comp : pat.uc;
+}
+
 uint8_t intel_get_pat_idx_wt(int fd)
 {
 	struct intel_pat_cache pat = {};
diff --git a/lib/intel_pat.h b/lib/intel_pat.h
index c24dbc275..6d68b9fc2 100644
--- a/lib/intel_pat.h
+++ b/lib/intel_pat.h
@@ -13,6 +13,7 @@
 uint8_t intel_get_max_pat_index(int fd);
 
 uint8_t intel_get_pat_idx_uc(int fd);
+uint8_t intel_get_pat_idx_uc_comp(int fd);
 uint8_t intel_get_pat_idx_wt(int fd);
 uint8_t intel_get_pat_idx_wb(int fd);
 
diff --git a/tests/intel/gem_ccs.c b/tests/intel/gem_ccs.c
index 0a691778d..5a17be4cb 100644
--- a/tests/intel/gem_ccs.c
+++ b/tests/intel/gem_ccs.c
@@ -99,7 +99,7 @@ static void surf_copy(int i915,
 	struct blt_block_copy_data_ext ext = {};
 	struct blt_ctrl_surf_copy_data surf = {};
 	uint32_t bb1, bb2, ccs, ccs2, *ccsmap, *ccsmap2;
-	uint64_t bb_size, ccssize = mid->size / CCS_RATIO;
+	uint64_t bb_size, ccssize = mid->size / (CCS_RATIO(i915));
 	uint32_t *ccscopy;
 	uint8_t uc_mocs = intel_get_uc_mocs_index(i915);
 	int result;
diff --git a/tests/intel/xe_ccs.c b/tests/intel/xe_ccs.c
index 647a6bd2e..f6695f4c0 100644
--- a/tests/intel/xe_ccs.c
+++ b/tests/intel/xe_ccs.c
@@ -63,8 +63,8 @@ static struct param {
 	.write_png = false,
 	.print_bb = false,
 	.print_surface_info = false,
-	.width = 512,
-	.height = 512,
+	.width = 1024,
+	.height = 1024,
 };
 
 struct test_config {
@@ -95,7 +95,8 @@ static void surf_copy(int xe,
 	struct blt_block_copy_data_ext ext = {};
 	struct blt_ctrl_surf_copy_data surf = {};
 	uint32_t bb1, bb2, ccs, ccs2, *ccsmap, *ccsmap2;
-	uint64_t bb_size, ccssize = mid->size / CCS_RATIO;
+	uint64_t ccsratio = CCS_RATIO(xe);
+	uint64_t bb_size, ccssize = mid->size / ccsratio;
 	uint32_t *ccscopy;
 	uint8_t uc_mocs = intel_get_uc_mocs_index(xe);
 	uint32_t sysmem = system_memory(xe);
@@ -103,13 +104,13 @@ static void surf_copy(int xe,
 
 	igt_assert(mid->compression);
 	ccscopy = (uint32_t *) malloc(ccssize);
-	ccs = xe_bo_create_flags(xe, 0, ccssize, sysmem);
-	ccs2 = xe_bo_create_flags(xe, 0, ccssize, sysmem);
+	ccs = xe_bo_create_caching(xe, 0, ccssize, sysmem, DRM_XE_GEM_CPU_CACHING_WC);
+	ccs2 = xe_bo_create_caching(xe, 0, ccssize, sysmem, DRM_XE_GEM_CPU_CACHING_WC);
 
 	blt_ctrl_surf_copy_init(xe, &surf);
 	surf.print_bb = param.print_bb;
 	blt_set_ctrl_surf_object(&surf.src, mid->handle, mid->region, mid->size,
-				 uc_mocs, DEFAULT_PAT_INDEX, BLT_INDIRECT_ACCESS);
+				 uc_mocs, intel_get_pat_idx_uc_comp(xe), BLT_INDIRECT_ACCESS);
 	blt_set_ctrl_surf_object(&surf.dst, ccs, sysmem, ccssize, uc_mocs,
 				 DEFAULT_PAT_INDEX, DIRECT_ACCESS);
 	bb_size = xe_get_default_alignment(xe);
@@ -157,7 +158,7 @@ static void surf_copy(int xe,
 	blt_set_ctrl_surf_object(&surf.src, ccs, sysmem, ccssize,
 				 uc_mocs, DEFAULT_PAT_INDEX, DIRECT_ACCESS);
 	blt_set_ctrl_surf_object(&surf.dst, mid->handle, mid->region, mid->size,
-				 uc_mocs, DEFAULT_PAT_INDEX, INDIRECT_ACCESS);
+				 uc_mocs, intel_get_pat_idx_uc_comp(xe), INDIRECT_ACCESS);
 	blt_ctrl_surf_copy(xe, ctx, NULL, ahnd, &surf);
 	intel_ctx_xe_sync(ctx, true);
 
@@ -234,10 +235,6 @@ static int blt_block_copy3(int xe,
 	igt_assert_f(blt3, "block-copy3 requires data to do blit\n");
 
 	alignment = xe_get_default_alignment(xe);
-	get_offset(ahnd, blt3->src.handle, blt3->src.size, alignment);
-	get_offset(ahnd, blt3->mid.handle, blt3->mid.size, alignment);
-	get_offset(ahnd, blt3->dst.handle, blt3->dst.size, alignment);
-	get_offset(ahnd, blt3->final.handle, blt3->final.size, alignment);
 	bb_offset = get_offset(ahnd, blt3->bb.handle, blt3->bb.size, alignment);
 
 	/* First blit src -> mid */
@@ -284,6 +281,7 @@ static void block_copy(int xe,
 		       enum blt_tiling_type mid_tiling,
 		       const struct test_config *config)
 {
+	bool is_dgfx = is_xe_device(xe) & xe_has_vram(xe) ;
 	struct blt_copy_data blt = {};
 	struct blt_block_copy_data_ext ext = {}, *pext = &ext;
 	struct blt_copy_object *src, *mid, *dst;
@@ -291,8 +289,8 @@ static void block_copy(int xe,
 	uint64_t bb_size = xe_get_default_alignment(xe);
 	uint64_t ahnd = intel_allocator_open(xe, ctx->vm, INTEL_ALLOCATOR_RELOC);
 	uint32_t run_id = mid_tiling;
-	uint32_t mid_region = region2, bb;
-	uint32_t width = param.width, height = param.height;
+	uint32_t mid_region = (IS_XE2(intel_get_drm_devid(xe)) & !is_dgfx) ? region1 : region2;
+	uint32_t width = param.width, height = param.height, bb;
 	enum blt_compression mid_compression = config->compression;
 	int mid_compression_format = param.compression_format;
 	enum blt_compression_type comp_type = COMPRESSION_TYPE_3D;
@@ -405,6 +403,7 @@ static void block_multicopy(int xe,
 			    enum blt_tiling_type mid_tiling,
 			    const struct test_config *config)
 {
+	bool is_dgfx = is_xe_device(xe) & xe_has_vram(xe) ;
 	struct blt_copy3_data blt3 = {};
 	struct blt_copy_data blt = {};
 	struct blt_block_copy3_data_ext ext3 = {}, *pext3 = &ext3;
@@ -413,8 +412,8 @@ static void block_multicopy(int xe,
 	uint64_t bb_size = xe_get_default_alignment(xe);
 	uint64_t ahnd = intel_allocator_open(xe, ctx->vm, INTEL_ALLOCATOR_RELOC);
 	uint32_t run_id = mid_tiling;
-	uint32_t mid_region = region2, bb;
-	uint32_t width = param.width, height = param.height;
+	uint32_t mid_region = (IS_XE2(intel_get_drm_devid(xe)) & !is_dgfx) ? region1 : region2;
+	uint32_t width = param.width, height = param.height, bb;
 	enum blt_compression mid_compression = config->compression;
 	int mid_compression_format = param.compression_format;
 	enum blt_compression_type comp_type = COMPRESSION_TYPE_3D;
@@ -539,8 +538,8 @@ static void block_copy_test(int xe,
 			region1 = igt_collection_get_value(regions, 0);
 			region2 = igt_collection_get_value(regions, 1);
 
-			/* Compressed surface must be in device memory */
-			if (config->compression && !XE_IS_VRAM_MEMORY_REGION(xe, region2))
+			/* if not XE2, then Compressed surface must be in device memory */
+			if (config->compression && !(IS_XE2((intel_get_drm_devid(xe)))) && !XE_IS_VRAM_MEMORY_REGION(xe, region2))
 				continue;
 
 			regtxt = xe_memregion_dynamic_subtest_name(xe, regions);
@@ -621,8 +620,8 @@ const char *help_str =
 	"  -p\tWrite PNG\n"
 	"  -s\tPrint surface info\n"
 	"  -t\tTiling format (0 - linear, 1 - XMAJOR, 2 - YMAJOR, 3 - TILE4, 4 - TILE64)\n"
-	"  -W\tWidth (default 512)\n"
-	"  -H\tHeight (default 512)"
+	"  -W\tWidth (default 1024)\n"
+	"  -H\tHeight (default 1024)"
 	;
 
 igt_main_args("bf:pst:W:H:", NULL, help_str, opt_handler, NULL)
-- 
2.34.1

Re: [igt-dev] [PATCH i-g-t 2/2] XE2: xe_ccs IGT changes to enable xe2 igfx compression

[Kamil Konieczny]

Hi,

same comment as for your 1/2 patch but here goes some more.
First of all this patch do not apply in current igt, please
update your upstream igt and solve all conflicts before
resending.

Btw why only xe2 igfx? Shouldn't it also work on dGPU?

On 2023-11-08 at 11:57:06 -0800, Jahagirdar, Akshata wrote:
> ---
>  lib/intel_blt.c       | 18 ++++++++++--------
>  lib/intel_blt.h       |  3 ++-
>  lib/intel_chipset.h   |  1 +

Looks like you added this in 1/2 patch and forgot to remove?

>  lib/intel_pat.c       | 12 +++++++++++-
>  lib/intel_pat.h       |  1 +

Please split lib/intel_pat to separate patch, add also developer(s)
of this lib to cc.

>  tests/intel/gem_ccs.c |  2 +-

Please split this one into separate patch, like:
tests/intel/gem_ccs: add compression support for LunarLake

>  tests/intel/xe_ccs.c  | 37 ++++++++++++++++++-------------------

This one sould also be separate patch, titled as:

tests/intel/xe_ccs: add compression support for LunarLake

Add also to Cc Karolina and Zbigniew:

Cc: Karolina Stolarek <karolina.stolarek@intel.com>
Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

Regards,
Kamil

>  7 files changed, 44 insertions(+), 30 deletions(-)
> 
> diff --git a/lib/intel_blt.c b/lib/intel_blt.c
> index c0593930c..c60d9fb1e 100644
> --- a/lib/intel_blt.c
> +++ b/lib/intel_blt.c
> @@ -948,15 +948,16 @@ int blt_block_copy(int fd,
>  	return ret;
>  }
>  
> -static uint16_t __ccs_size(const struct blt_ctrl_surf_copy_data *surf)
> +static uint16_t __ccs_size(int fd, const struct blt_ctrl_surf_copy_data *surf)
>  {
>  	uint32_t src_size, dst_size;
> +	uint16_t ccsratio = CCS_RATIO(fd);
>  
>  	src_size = surf->src.access_type == DIRECT_ACCESS ?
> -				surf->src.size : surf->src.size / CCS_RATIO;
> +				surf->src.size : surf->src.size / ccsratio;
>  
>  	dst_size = surf->dst.access_type == DIRECT_ACCESS ?
> -				surf->dst.size : surf->dst.size / CCS_RATIO;
> +				surf->dst.size : surf->dst.size / ccsratio;
>  
>  	igt_assert_f(src_size <= dst_size, "dst size must be >= src size for CCS copy\n");
>  
> @@ -1136,7 +1137,7 @@ uint64_t emit_blt_ctrl_surf_copy(int fd,
>  		data.xe2.dw00.dst_access_type = surf->dst.access_type;
>  
>  		/* Ensure dst has size capable to keep src ccs aux */
> -		data.xe2.dw00.size_of_ctrl_copy = __ccs_size(surf) / CCS_RATIO - 1;
> +		data.xe2.dw00.size_of_ctrl_copy = __ccs_size(fd, surf) / NUM_4KPAGES_OF_CCS - 1;
>  		data.xe2.dw00.length = 0x3;
>  
>  		data.xe2.dw01.src_address_lo = src_offset;
> @@ -1155,7 +1156,7 @@ uint64_t emit_blt_ctrl_surf_copy(int fd,
>  		data.gen12.dw00.dst_access_type = surf->dst.access_type;
>  
>  		/* Ensure dst has size capable to keep src ccs aux */
> -		data.gen12.dw00.size_of_ctrl_copy = __ccs_size(surf) / CCS_RATIO - 1;
> +		data.gen12.dw00.size_of_ctrl_copy = __ccs_size(fd,surf) / (CCS_RATIO(fd)) - 1;
>  		data.gen12.dw00.length = 0x3;
>  
>  		data.gen12.dw01.src_address_lo = src_offset;
> @@ -1794,7 +1795,8 @@ blt_create_object(const struct blt_copy_data *blt, uint32_t region,
>  	uint64_t size = width * height * bpp / 8;
>  	uint32_t stride = tiling == T_LINEAR ? width * 4 : width;
>  	uint32_t handle;
> -
> +	uint8_t pat_index = compression ? intel_get_pat_idx_uc_comp(blt->fd) : intel_get_pat_idx_uc(blt->fd);
> +	uint16_t cpu_caching = DRM_XE_GEM_CPU_CACHING_WC;
>  	igt_assert_f(blt->driver, "Driver isn't set, have you called blt_copy_init()?\n");
>  
>  	obj = calloc(1, sizeof(*obj));
> @@ -1808,13 +1810,13 @@ blt_create_object(const struct blt_copy_data *blt, uint32_t region,
>  			flags |= XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM;
>  
>  		size = ALIGN(size, xe_get_default_alignment(blt->fd));
> -		handle = xe_bo_create_flags(blt->fd, 0, size, flags);
> +		handle = xe_bo_create_caching(blt->fd, 0, size, region, cpu_caching);
>  	} else {
>  		igt_assert(__gem_create_in_memory_regions(blt->fd, &handle,
>  							  &size, region) == 0);
>  	}
>  
> -	blt_set_object(obj, handle, size, region, mocs_index, DEFAULT_PAT_INDEX, tiling,
> +	blt_set_object(obj, handle, size, region, mocs_index, pat_index, tiling,
>  		       compression, compression_type);
>  	blt_set_geom(obj, stride, 0, 0, width, height, 0, 0);
>  
> diff --git a/lib/intel_blt.h b/lib/intel_blt.h
> index 5934ccd67..222c44074 100644
> --- a/lib/intel_blt.h
> +++ b/lib/intel_blt.h
> @@ -52,7 +52,8 @@
>  #include "igt.h"
>  #include "intel_cmds_info.h"
>  
> -#define CCS_RATIO 256
> +#define CCS_RATIO(xe) IS_XE2(intel_get_drm_devid(xe)) ? 512 : 256
> +#define NUM_4KPAGES_OF_CCS 8
>  
>  enum blt_color_depth {
>  	CD_8bit,
> diff --git a/lib/intel_chipset.h b/lib/intel_chipset.h
> index 1edc8e92c..aca429a97 100644
> --- a/lib/intel_chipset.h
> +++ b/lib/intel_chipset.h
> @@ -205,6 +205,7 @@ void intel_check_pch(void);
>  #define IS_METEORLAKE(devid)	(intel_get_device_info(devid)->is_meteorlake)
>  #define IS_PONTEVECCHIO(devid)	(intel_get_device_info(devid)->is_pontevecchio)
>  #define IS_LUNARLAKE(devid)	(intel_get_device_info(devid)->is_lunarlake)
> +#define IS_XE2(devid)		(intel_get_device_info(devid)->graphics_ver >= 20)
>  
>  #define IS_GEN(devid, x)	(intel_get_device_info(devid)->graphics_ver == x)
>  #define AT_LEAST_GEN(devid, x)	(intel_get_device_info(devid)->graphics_ver >= x)
> diff --git a/lib/intel_pat.c b/lib/intel_pat.c
> index 2b892ee52..ccda6b786 100644
> --- a/lib/intel_pat.c
> +++ b/lib/intel_pat.c
> @@ -11,7 +11,7 @@ struct intel_pat_cache {
>  	uint8_t uc; /* UC + COH_NONE */
>  	uint8_t wt; /* WT + COH_NONE */
>  	uint8_t wb; /* WB + COH_AT_LEAST_1WAY */
> -
> +	uint8_t uc_comp; /* UC + COH_NONE + COMPRESSION*/
>  	uint8_t max_index;
>  };
>  
> @@ -23,6 +23,7 @@ static void intel_get_pat_idx(int fd, struct intel_pat_cache *pat)
>  		pat->uc = 3;
>  		pat->wt = 15; /* Compressed + WB-transient */
>  		pat->wb = 2;
> +		pat->uc_comp = 12; /* Compressed + UC */
>  		pat->max_index = 31;
>  	} else if (IS_METEORLAKE(dev_id)) {
>  		pat->uc = 2;
> @@ -60,6 +61,15 @@ uint8_t intel_get_pat_idx_uc(int fd)
>  	return pat.uc;
>  }
>  
> +uint8_t intel_get_pat_idx_uc_comp(int fd)
> +{
> +	struct intel_pat_cache pat = {};
> +	uint16_t dev_id = intel_get_drm_devid(fd);
> +	
> +	intel_get_pat_idx(fd, &pat);
> +	return (intel_get_device_info(dev_id)->graphics_ver == 20) ? pat.uc_comp : pat.uc;
> +}
> +
>  uint8_t intel_get_pat_idx_wt(int fd)
>  {
>  	struct intel_pat_cache pat = {};
> diff --git a/lib/intel_pat.h b/lib/intel_pat.h
> index c24dbc275..6d68b9fc2 100644
> --- a/lib/intel_pat.h
> +++ b/lib/intel_pat.h
> @@ -13,6 +13,7 @@
>  uint8_t intel_get_max_pat_index(int fd);
>  
>  uint8_t intel_get_pat_idx_uc(int fd);
> +uint8_t intel_get_pat_idx_uc_comp(int fd);
>  uint8_t intel_get_pat_idx_wt(int fd);
>  uint8_t intel_get_pat_idx_wb(int fd);
>  
> diff --git a/tests/intel/gem_ccs.c b/tests/intel/gem_ccs.c
> index 0a691778d..5a17be4cb 100644
> --- a/tests/intel/gem_ccs.c
> +++ b/tests/intel/gem_ccs.c
> @@ -99,7 +99,7 @@ static void surf_copy(int i915,
>  	struct blt_block_copy_data_ext ext = {};
>  	struct blt_ctrl_surf_copy_data surf = {};
>  	uint32_t bb1, bb2, ccs, ccs2, *ccsmap, *ccsmap2;
> -	uint64_t bb_size, ccssize = mid->size / CCS_RATIO;
> +	uint64_t bb_size, ccssize = mid->size / (CCS_RATIO(i915));
>  	uint32_t *ccscopy;
>  	uint8_t uc_mocs = intel_get_uc_mocs_index(i915);
>  	int result;
> diff --git a/tests/intel/xe_ccs.c b/tests/intel/xe_ccs.c
> index 647a6bd2e..f6695f4c0 100644
> --- a/tests/intel/xe_ccs.c
> +++ b/tests/intel/xe_ccs.c
> @@ -63,8 +63,8 @@ static struct param {
>  	.write_png = false,
>  	.print_bb = false,
>  	.print_surface_info = false,
> -	.width = 512,
> -	.height = 512,
> +	.width = 1024,
> +	.height = 1024,
>  };
>  
>  struct test_config {
> @@ -95,7 +95,8 @@ static void surf_copy(int xe,
>  	struct blt_block_copy_data_ext ext = {};
>  	struct blt_ctrl_surf_copy_data surf = {};
>  	uint32_t bb1, bb2, ccs, ccs2, *ccsmap, *ccsmap2;
> -	uint64_t bb_size, ccssize = mid->size / CCS_RATIO;
> +	uint64_t ccsratio = CCS_RATIO(xe);
> +	uint64_t bb_size, ccssize = mid->size / ccsratio;
>  	uint32_t *ccscopy;
>  	uint8_t uc_mocs = intel_get_uc_mocs_index(xe);
>  	uint32_t sysmem = system_memory(xe);
> @@ -103,13 +104,13 @@ static void surf_copy(int xe,
>  
>  	igt_assert(mid->compression);
>  	ccscopy = (uint32_t *) malloc(ccssize);
> -	ccs = xe_bo_create_flags(xe, 0, ccssize, sysmem);
> -	ccs2 = xe_bo_create_flags(xe, 0, ccssize, sysmem);
> +	ccs = xe_bo_create_caching(xe, 0, ccssize, sysmem, DRM_XE_GEM_CPU_CACHING_WC);
> +	ccs2 = xe_bo_create_caching(xe, 0, ccssize, sysmem, DRM_XE_GEM_CPU_CACHING_WC);
>  
>  	blt_ctrl_surf_copy_init(xe, &surf);
>  	surf.print_bb = param.print_bb;
>  	blt_set_ctrl_surf_object(&surf.src, mid->handle, mid->region, mid->size,
> -				 uc_mocs, DEFAULT_PAT_INDEX, BLT_INDIRECT_ACCESS);
> +				 uc_mocs, intel_get_pat_idx_uc_comp(xe), BLT_INDIRECT_ACCESS);
>  	blt_set_ctrl_surf_object(&surf.dst, ccs, sysmem, ccssize, uc_mocs,
>  				 DEFAULT_PAT_INDEX, DIRECT_ACCESS);
>  	bb_size = xe_get_default_alignment(xe);
> @@ -157,7 +158,7 @@ static void surf_copy(int xe,
>  	blt_set_ctrl_surf_object(&surf.src, ccs, sysmem, ccssize,
>  				 uc_mocs, DEFAULT_PAT_INDEX, DIRECT_ACCESS);
>  	blt_set_ctrl_surf_object(&surf.dst, mid->handle, mid->region, mid->size,
> -				 uc_mocs, DEFAULT_PAT_INDEX, INDIRECT_ACCESS);
> +				 uc_mocs, intel_get_pat_idx_uc_comp(xe), INDIRECT_ACCESS);
>  	blt_ctrl_surf_copy(xe, ctx, NULL, ahnd, &surf);
>  	intel_ctx_xe_sync(ctx, true);
>  
> @@ -234,10 +235,6 @@ static int blt_block_copy3(int xe,
>  	igt_assert_f(blt3, "block-copy3 requires data to do blit\n");
>  
>  	alignment = xe_get_default_alignment(xe);
> -	get_offset(ahnd, blt3->src.handle, blt3->src.size, alignment);
> -	get_offset(ahnd, blt3->mid.handle, blt3->mid.size, alignment);
> -	get_offset(ahnd, blt3->dst.handle, blt3->dst.size, alignment);
> -	get_offset(ahnd, blt3->final.handle, blt3->final.size, alignment);
>  	bb_offset = get_offset(ahnd, blt3->bb.handle, blt3->bb.size, alignment);
>  
>  	/* First blit src -> mid */
> @@ -284,6 +281,7 @@ static void block_copy(int xe,
>  		       enum blt_tiling_type mid_tiling,
>  		       const struct test_config *config)
>  {
> +	bool is_dgfx = is_xe_device(xe) & xe_has_vram(xe) ;
>  	struct blt_copy_data blt = {};
>  	struct blt_block_copy_data_ext ext = {}, *pext = &ext;
>  	struct blt_copy_object *src, *mid, *dst;
> @@ -291,8 +289,8 @@ static void block_copy(int xe,
>  	uint64_t bb_size = xe_get_default_alignment(xe);
>  	uint64_t ahnd = intel_allocator_open(xe, ctx->vm, INTEL_ALLOCATOR_RELOC);
>  	uint32_t run_id = mid_tiling;
> -	uint32_t mid_region = region2, bb;
> -	uint32_t width = param.width, height = param.height;
> +	uint32_t mid_region = (IS_XE2(intel_get_drm_devid(xe)) & !is_dgfx) ? region1 : region2;
> +	uint32_t width = param.width, height = param.height, bb;
>  	enum blt_compression mid_compression = config->compression;
>  	int mid_compression_format = param.compression_format;
>  	enum blt_compression_type comp_type = COMPRESSION_TYPE_3D;
> @@ -405,6 +403,7 @@ static void block_multicopy(int xe,
>  			    enum blt_tiling_type mid_tiling,
>  			    const struct test_config *config)
>  {
> +	bool is_dgfx = is_xe_device(xe) & xe_has_vram(xe) ;
>  	struct blt_copy3_data blt3 = {};
>  	struct blt_copy_data blt = {};
>  	struct blt_block_copy3_data_ext ext3 = {}, *pext3 = &ext3;
> @@ -413,8 +412,8 @@ static void block_multicopy(int xe,
>  	uint64_t bb_size = xe_get_default_alignment(xe);
>  	uint64_t ahnd = intel_allocator_open(xe, ctx->vm, INTEL_ALLOCATOR_RELOC);
>  	uint32_t run_id = mid_tiling;
> -	uint32_t mid_region = region2, bb;
> -	uint32_t width = param.width, height = param.height;
> +	uint32_t mid_region = (IS_XE2(intel_get_drm_devid(xe)) & !is_dgfx) ? region1 : region2;
> +	uint32_t width = param.width, height = param.height, bb;
>  	enum blt_compression mid_compression = config->compression;
>  	int mid_compression_format = param.compression_format;
>  	enum blt_compression_type comp_type = COMPRESSION_TYPE_3D;
> @@ -539,8 +538,8 @@ static void block_copy_test(int xe,
>  			region1 = igt_collection_get_value(regions, 0);
>  			region2 = igt_collection_get_value(regions, 1);
>  
> -			/* Compressed surface must be in device memory */
> -			if (config->compression && !XE_IS_VRAM_MEMORY_REGION(xe, region2))
> +			/* if not XE2, then Compressed surface must be in device memory */
> +			if (config->compression && !(IS_XE2((intel_get_drm_devid(xe)))) && !XE_IS_VRAM_MEMORY_REGION(xe, region2))
>  				continue;
>  
>  			regtxt = xe_memregion_dynamic_subtest_name(xe, regions);
> @@ -621,8 +620,8 @@ const char *help_str =
>  	"  -p\tWrite PNG\n"
>  	"  -s\tPrint surface info\n"
>  	"  -t\tTiling format (0 - linear, 1 - XMAJOR, 2 - YMAJOR, 3 - TILE4, 4 - TILE64)\n"
> -	"  -W\tWidth (default 512)\n"
> -	"  -H\tHeight (default 512)"
> +	"  -W\tWidth (default 1024)\n"
> +	"  -H\tHeight (default 1024)"
>  	;
>  
>  igt_main_args("bf:pst:W:H:", NULL, help_str, opt_handler, NULL)
> -- 
> 2.34.1
> 

Re: [igt-dev] [PATCH i-g-t 2/2] XE2: xe_ccs IGT changes to enable xe2 igfx compression

[Jahagirdar, Akshata]

-----Original Message-----
From: Kamil Konieczny <kamil.konieczny@linux.intel.com> 
Sent: Wednesday, November 8, 2023 6:48 AM
To: igt-dev@lists.freedesktop.org
Cc: Jahagirdar, Akshata <akshata.jahagirdar@intel.com>; Siddiqui, Ayaz A <ayaz.siddiqui@intel.com>; Stolarek, Karolina <karolina.stolarek@intel.com>; Kempczynski, Zbigniew <zbigniew.kempczynski@intel.com>
Subject: Re: [igt-dev] [PATCH i-g-t 2/2] XE2: xe_ccs IGT changes to enable xe2 igfx compression

Hi,

same comment as for your 1/2 patch but here goes some more.
First of all this patch do not apply in current igt, please update your upstream igt and solve all conflicts before resending.

Btw why only xe2 igfx? Shouldn't it also work on dGPU?

Yes, you are right. Some changes only to igfx, will separate them in next patch.

On 2023-11-08 at 11:57:06 -0800, Jahagirdar, Akshata wrote:
> ---
>  lib/intel_blt.c       | 18 ++++++++++--------
>  lib/intel_blt.h       |  3 ++-
>  lib/intel_chipset.h   |  1 +

Looks like you added this in 1/2 patch and forgot to remove? 

These changes were not there in 1/2 patch, will make this as separate patch in next one

>  lib/intel_pat.c       | 12 +++++++++++-
>  lib/intel_pat.h       |  1 +

Please split lib/intel_pat to separate patch, add also developer(s) of this lib to cc.

Will fix this!

>  tests/intel/gem_ccs.c |  2 +-

Please split this one into separate patch, like:
tests/intel/gem_ccs: add compression support for LunarLake

>  tests/intel/xe_ccs.c  | 37 ++++++++++++++++++-------------------

This one sould also be separate patch, titled as:

tests/intel/xe_ccs: add compression support for LunarLake

Add also to Cc Karolina and Zbigniew:

Cc: Karolina Stolarek <karolina.stolarek@intel.com>
Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

Regards,
Kamil

Will make separate patches for this, Thanks!

>  7 files changed, 44 insertions(+), 30 deletions(-)
> 
> diff --git a/lib/intel_blt.c b/lib/intel_blt.c index 
> c0593930c..c60d9fb1e 100644
> --- a/lib/intel_blt.c
> +++ b/lib/intel_blt.c
> @@ -948,15 +948,16 @@ int blt_block_copy(int fd,
>  	return ret;
>  }
>  
> -static uint16_t __ccs_size(const struct blt_ctrl_surf_copy_data 
> *surf)
> +static uint16_t __ccs_size(int fd, const struct 
> +blt_ctrl_surf_copy_data *surf)
>  {
>  	uint32_t src_size, dst_size;
> +	uint16_t ccsratio = CCS_RATIO(fd);
>  
>  	src_size = surf->src.access_type == DIRECT_ACCESS ?
> -				surf->src.size : surf->src.size / CCS_RATIO;
> +				surf->src.size : surf->src.size / ccsratio;
>  
>  	dst_size = surf->dst.access_type == DIRECT_ACCESS ?
> -				surf->dst.size : surf->dst.size / CCS_RATIO;
> +				surf->dst.size : surf->dst.size / ccsratio;
>  
>  	igt_assert_f(src_size <= dst_size, "dst size must be >= src size for 
> CCS copy\n");
>  
> @@ -1136,7 +1137,7 @@ uint64_t emit_blt_ctrl_surf_copy(int fd,
>  		data.xe2.dw00.dst_access_type = surf->dst.access_type;
>  
>  		/* Ensure dst has size capable to keep src ccs aux */
> -		data.xe2.dw00.size_of_ctrl_copy = __ccs_size(surf) / CCS_RATIO - 1;
> +		data.xe2.dw00.size_of_ctrl_copy = __ccs_size(fd, surf) / 
> +NUM_4KPAGES_OF_CCS - 1;
>  		data.xe2.dw00.length = 0x3;
>  
>  		data.xe2.dw01.src_address_lo = src_offset; @@ -1155,7 +1156,7 @@ 
> uint64_t emit_blt_ctrl_surf_copy(int fd,
>  		data.gen12.dw00.dst_access_type = surf->dst.access_type;
>  
>  		/* Ensure dst has size capable to keep src ccs aux */
> -		data.gen12.dw00.size_of_ctrl_copy = __ccs_size(surf) / CCS_RATIO - 1;
> +		data.gen12.dw00.size_of_ctrl_copy = __ccs_size(fd,surf) / 
> +(CCS_RATIO(fd)) - 1;
>  		data.gen12.dw00.length = 0x3;
>  
>  		data.gen12.dw01.src_address_lo = src_offset; @@ -1794,7 +1795,8 @@ 
> blt_create_object(const struct blt_copy_data *blt, uint32_t region,
>  	uint64_t size = width * height * bpp / 8;
>  	uint32_t stride = tiling == T_LINEAR ? width * 4 : width;
>  	uint32_t handle;
> -
> +	uint8_t pat_index = compression ? intel_get_pat_idx_uc_comp(blt->fd) : intel_get_pat_idx_uc(blt->fd);
> +	uint16_t cpu_caching = DRM_XE_GEM_CPU_CACHING_WC;
>  	igt_assert_f(blt->driver, "Driver isn't set, have you called 
> blt_copy_init()?\n");
>  
>  	obj = calloc(1, sizeof(*obj));
> @@ -1808,13 +1810,13 @@ blt_create_object(const struct blt_copy_data *blt, uint32_t region,
>  			flags |= XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM;
>  
>  		size = ALIGN(size, xe_get_default_alignment(blt->fd));
> -		handle = xe_bo_create_flags(blt->fd, 0, size, flags);
> +		handle = xe_bo_create_caching(blt->fd, 0, size, region, 
> +cpu_caching);
>  	} else {
>  		igt_assert(__gem_create_in_memory_regions(blt->fd, &handle,
>  							  &size, region) == 0);
>  	}
>  
> -	blt_set_object(obj, handle, size, region, mocs_index, DEFAULT_PAT_INDEX, tiling,
> +	blt_set_object(obj, handle, size, region, mocs_index, pat_index, 
> +tiling,
>  		       compression, compression_type);
>  	blt_set_geom(obj, stride, 0, 0, width, height, 0, 0);
>  
> diff --git a/lib/intel_blt.h b/lib/intel_blt.h index 
> 5934ccd67..222c44074 100644
> --- a/lib/intel_blt.h
> +++ b/lib/intel_blt.h
> @@ -52,7 +52,8 @@
>  #include "igt.h"
>  #include "intel_cmds_info.h"
>  
> -#define CCS_RATIO 256
> +#define CCS_RATIO(xe) IS_XE2(intel_get_drm_devid(xe)) ? 512 : 256 
> +#define NUM_4KPAGES_OF_CCS 8
>  
>  enum blt_color_depth {
>  	CD_8bit,
> diff --git a/lib/intel_chipset.h b/lib/intel_chipset.h index 
> 1edc8e92c..aca429a97 100644
> --- a/lib/intel_chipset.h
> +++ b/lib/intel_chipset.h
> @@ -205,6 +205,7 @@ void intel_check_pch(void);
>  #define IS_METEORLAKE(devid)	(intel_get_device_info(devid)->is_meteorlake)
>  #define IS_PONTEVECCHIO(devid)	(intel_get_device_info(devid)->is_pontevecchio)
>  #define IS_LUNARLAKE(devid)	(intel_get_device_info(devid)->is_lunarlake)
> +#define IS_XE2(devid)		(intel_get_device_info(devid)->graphics_ver >= 20)
>  
>  #define IS_GEN(devid, x)	(intel_get_device_info(devid)->graphics_ver == x)
>  #define AT_LEAST_GEN(devid, x)	(intel_get_device_info(devid)->graphics_ver >= x)
> diff --git a/lib/intel_pat.c b/lib/intel_pat.c index 
> 2b892ee52..ccda6b786 100644
> --- a/lib/intel_pat.c
> +++ b/lib/intel_pat.c
> @@ -11,7 +11,7 @@ struct intel_pat_cache {
>  	uint8_t uc; /* UC + COH_NONE */
>  	uint8_t wt; /* WT + COH_NONE */
>  	uint8_t wb; /* WB + COH_AT_LEAST_1WAY */
> -
> +	uint8_t uc_comp; /* UC + COH_NONE + COMPRESSION*/
>  	uint8_t max_index;
>  };
>  
> @@ -23,6 +23,7 @@ static void intel_get_pat_idx(int fd, struct intel_pat_cache *pat)
>  		pat->uc = 3;
>  		pat->wt = 15; /* Compressed + WB-transient */
>  		pat->wb = 2;
> +		pat->uc_comp = 12; /* Compressed + UC */
>  		pat->max_index = 31;
>  	} else if (IS_METEORLAKE(dev_id)) {
>  		pat->uc = 2;
> @@ -60,6 +61,15 @@ uint8_t intel_get_pat_idx_uc(int fd)
>  	return pat.uc;
>  }
>  
> +uint8_t intel_get_pat_idx_uc_comp(int fd) {
> +	struct intel_pat_cache pat = {};
> +	uint16_t dev_id = intel_get_drm_devid(fd);
> +	
> +	intel_get_pat_idx(fd, &pat);
> +	return (intel_get_device_info(dev_id)->graphics_ver == 20) ? 
> +pat.uc_comp : pat.uc; }
> +
>  uint8_t intel_get_pat_idx_wt(int fd)
>  {
>  	struct intel_pat_cache pat = {};
> diff --git a/lib/intel_pat.h b/lib/intel_pat.h index 
> c24dbc275..6d68b9fc2 100644
> --- a/lib/intel_pat.h
> +++ b/lib/intel_pat.h
> @@ -13,6 +13,7 @@
>  uint8_t intel_get_max_pat_index(int fd);
>  
>  uint8_t intel_get_pat_idx_uc(int fd);
> +uint8_t intel_get_pat_idx_uc_comp(int fd);
>  uint8_t intel_get_pat_idx_wt(int fd);  uint8_t 
> intel_get_pat_idx_wb(int fd);
>  
> diff --git a/tests/intel/gem_ccs.c b/tests/intel/gem_ccs.c index 
> 0a691778d..5a17be4cb 100644
> --- a/tests/intel/gem_ccs.c
> +++ b/tests/intel/gem_ccs.c
> @@ -99,7 +99,7 @@ static void surf_copy(int i915,
>  	struct blt_block_copy_data_ext ext = {};
>  	struct blt_ctrl_surf_copy_data surf = {};
>  	uint32_t bb1, bb2, ccs, ccs2, *ccsmap, *ccsmap2;
> -	uint64_t bb_size, ccssize = mid->size / CCS_RATIO;
> +	uint64_t bb_size, ccssize = mid->size / (CCS_RATIO(i915));
>  	uint32_t *ccscopy;
>  	uint8_t uc_mocs = intel_get_uc_mocs_index(i915);
>  	int result;
> diff --git a/tests/intel/xe_ccs.c b/tests/intel/xe_ccs.c index 
> 647a6bd2e..f6695f4c0 100644
> --- a/tests/intel/xe_ccs.c
> +++ b/tests/intel/xe_ccs.c
> @@ -63,8 +63,8 @@ static struct param {
>  	.write_png = false,
>  	.print_bb = false,
>  	.print_surface_info = false,
> -	.width = 512,
> -	.height = 512,
> +	.width = 1024,
> +	.height = 1024,
>  };
>  
>  struct test_config {
> @@ -95,7 +95,8 @@ static void surf_copy(int xe,
>  	struct blt_block_copy_data_ext ext = {};
>  	struct blt_ctrl_surf_copy_data surf = {};
>  	uint32_t bb1, bb2, ccs, ccs2, *ccsmap, *ccsmap2;
> -	uint64_t bb_size, ccssize = mid->size / CCS_RATIO;
> +	uint64_t ccsratio = CCS_RATIO(xe);
> +	uint64_t bb_size, ccssize = mid->size / ccsratio;
>  	uint32_t *ccscopy;
>  	uint8_t uc_mocs = intel_get_uc_mocs_index(xe);
>  	uint32_t sysmem = system_memory(xe); @@ -103,13 +104,13 @@ static 
> void surf_copy(int xe,
>  
>  	igt_assert(mid->compression);
>  	ccscopy = (uint32_t *) malloc(ccssize);
> -	ccs = xe_bo_create_flags(xe, 0, ccssize, sysmem);
> -	ccs2 = xe_bo_create_flags(xe, 0, ccssize, sysmem);
> +	ccs = xe_bo_create_caching(xe, 0, ccssize, sysmem, DRM_XE_GEM_CPU_CACHING_WC);
> +	ccs2 = xe_bo_create_caching(xe, 0, ccssize, sysmem, 
> +DRM_XE_GEM_CPU_CACHING_WC);
>  
>  	blt_ctrl_surf_copy_init(xe, &surf);
>  	surf.print_bb = param.print_bb;
>  	blt_set_ctrl_surf_object(&surf.src, mid->handle, mid->region, mid->size,
> -				 uc_mocs, DEFAULT_PAT_INDEX, BLT_INDIRECT_ACCESS);
> +				 uc_mocs, intel_get_pat_idx_uc_comp(xe), BLT_INDIRECT_ACCESS);
>  	blt_set_ctrl_surf_object(&surf.dst, ccs, sysmem, ccssize, uc_mocs,
>  				 DEFAULT_PAT_INDEX, DIRECT_ACCESS);
>  	bb_size = xe_get_default_alignment(xe); @@ -157,7 +158,7 @@ static 
> void surf_copy(int xe,
>  	blt_set_ctrl_surf_object(&surf.src, ccs, sysmem, ccssize,
>  				 uc_mocs, DEFAULT_PAT_INDEX, DIRECT_ACCESS);
>  	blt_set_ctrl_surf_object(&surf.dst, mid->handle, mid->region, mid->size,
> -				 uc_mocs, DEFAULT_PAT_INDEX, INDIRECT_ACCESS);
> +				 uc_mocs, intel_get_pat_idx_uc_comp(xe), INDIRECT_ACCESS);
>  	blt_ctrl_surf_copy(xe, ctx, NULL, ahnd, &surf);
>  	intel_ctx_xe_sync(ctx, true);
>  
> @@ -234,10 +235,6 @@ static int blt_block_copy3(int xe,
>  	igt_assert_f(blt3, "block-copy3 requires data to do blit\n");
>  
>  	alignment = xe_get_default_alignment(xe);
> -	get_offset(ahnd, blt3->src.handle, blt3->src.size, alignment);
> -	get_offset(ahnd, blt3->mid.handle, blt3->mid.size, alignment);
> -	get_offset(ahnd, blt3->dst.handle, blt3->dst.size, alignment);
> -	get_offset(ahnd, blt3->final.handle, blt3->final.size, alignment);
>  	bb_offset = get_offset(ahnd, blt3->bb.handle, blt3->bb.size, 
> alignment);
>  
>  	/* First blit src -> mid */
> @@ -284,6 +281,7 @@ static void block_copy(int xe,
>  		       enum blt_tiling_type mid_tiling,
>  		       const struct test_config *config)  {
> +	bool is_dgfx = is_xe_device(xe) & xe_has_vram(xe) ;
>  	struct blt_copy_data blt = {};
>  	struct blt_block_copy_data_ext ext = {}, *pext = &ext;
>  	struct blt_copy_object *src, *mid, *dst; @@ -291,8 +289,8 @@ static 
> void block_copy(int xe,
>  	uint64_t bb_size = xe_get_default_alignment(xe);
>  	uint64_t ahnd = intel_allocator_open(xe, ctx->vm, INTEL_ALLOCATOR_RELOC);
>  	uint32_t run_id = mid_tiling;
> -	uint32_t mid_region = region2, bb;
> -	uint32_t width = param.width, height = param.height;
> +	uint32_t mid_region = (IS_XE2(intel_get_drm_devid(xe)) & !is_dgfx) ? region1 : region2;
> +	uint32_t width = param.width, height = param.height, bb;
>  	enum blt_compression mid_compression = config->compression;
>  	int mid_compression_format = param.compression_format;
>  	enum blt_compression_type comp_type = COMPRESSION_TYPE_3D; @@ -405,6 
> +403,7 @@ static void block_multicopy(int xe,
>  			    enum blt_tiling_type mid_tiling,
>  			    const struct test_config *config)  {
> +	bool is_dgfx = is_xe_device(xe) & xe_has_vram(xe) ;
>  	struct blt_copy3_data blt3 = {};
>  	struct blt_copy_data blt = {};
>  	struct blt_block_copy3_data_ext ext3 = {}, *pext3 = &ext3; @@ -413,8 
> +412,8 @@ static void block_multicopy(int xe,
>  	uint64_t bb_size = xe_get_default_alignment(xe);
>  	uint64_t ahnd = intel_allocator_open(xe, ctx->vm, INTEL_ALLOCATOR_RELOC);
>  	uint32_t run_id = mid_tiling;
> -	uint32_t mid_region = region2, bb;
> -	uint32_t width = param.width, height = param.height;
> +	uint32_t mid_region = (IS_XE2(intel_get_drm_devid(xe)) & !is_dgfx) ? region1 : region2;
> +	uint32_t width = param.width, height = param.height, bb;
>  	enum blt_compression mid_compression = config->compression;
>  	int mid_compression_format = param.compression_format;
>  	enum blt_compression_type comp_type = COMPRESSION_TYPE_3D; @@ -539,8 
> +538,8 @@ static void block_copy_test(int xe,
>  			region1 = igt_collection_get_value(regions, 0);
>  			region2 = igt_collection_get_value(regions, 1);
>  
> -			/* Compressed surface must be in device memory */
> -			if (config->compression && !XE_IS_VRAM_MEMORY_REGION(xe, region2))
> +			/* if not XE2, then Compressed surface must be in device memory */
> +			if (config->compression && !(IS_XE2((intel_get_drm_devid(xe)))) && 
> +!XE_IS_VRAM_MEMORY_REGION(xe, region2))
>  				continue;
>  
>  			regtxt = xe_memregion_dynamic_subtest_name(xe, regions); @@ -621,8 
> +620,8 @@ const char *help_str =
>  	"  -p\tWrite PNG\n"
>  	"  -s\tPrint surface info\n"
>  	"  -t\tTiling format (0 - linear, 1 - XMAJOR, 2 - YMAJOR, 3 - TILE4, 4 - TILE64)\n"
> -	"  -W\tWidth (default 512)\n"
> -	"  -H\tHeight (default 512)"
> +	"  -W\tWidth (default 1024)\n"
> +	"  -H\tHeight (default 1024)"
>  	;
>  
>  igt_main_args("bf:pst:W:H:", NULL, help_str, opt_handler, NULL)
> --
> 2.34.1
>