* [PATCH 1/4] drm/fourcc: Add modifiers for AMD GFX6-8
2026-07-15 10:57 [PATCH 0/4] drm/amdgpu: Support DRM format modifiers on GFX6-8 (v2) Timur Kristóf
@ 2026-07-15 10:57 ` Timur Kristóf
2026-07-15 12:11 ` Christian König
2026-07-15 10:57 ` [PATCH 2/4] drm/amdgpu: Convert tiling flags to modifiers on GFX6-8 Timur Kristóf
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Timur Kristóf @ 2026-07-15 10:57 UTC (permalink / raw)
To: amd-gfx, dri-devel, Alex Deucher, christian.koenig,
Tvrtko Ursulin, pierre-eric.pelloux-prayer, Natalie Vock, Dieter
Cc: Timur Kristóf, Bas Nieuwenhuizen, Marek Olšák,
Daniel Stone
GFX6-8 are the oldest GPUs supported by the amdgpu
kernel driver, and the last ones that didn't support
DRM format modifiers until now. These are the Southern
Islands, Sea Islands and Volcanic Islands families
of GPUs.
On GFX6-8, the GFX block can only use pre-determined tiling
modes which are programmed by the kernel according to the
tiling mode table. GFX6 uses the GB_TILE_MODE0...31 registers,
and GFX7-8 also has GB_MACROTILE_MODE0...15 registers.
DCC is also supported on GFX8, albeit not displayable.
Note that the tiling table is uAPI and userspace relies on
specific modes being present at specific indices.
How the tiling works is primarily determined by the
so-called array mode.
Use the TILE field to specify the array mode.
Pixel data is organized into micro tiles.
Each micro tile may be 8x8 / 8x8x4 / 8x8x8 pixels,
depending on the array mode.
Add the MICROTILE field to specify microtile mode.
Microtiles may be further organized into macro tiles,
which have many configurable parameters. Macro tile mode
selection depends on how many bits per pixel an image has.
Add the PIPE_CONFIG, TILE_SPLIT, BANK_WIDTH, BANK_HEIGHT,
MACRO_TILE_ASPECT, NUM_BANKS fields to specify parameters
of macro tiled modes.
Furthermore, tiling is also influenced by memory
configuration. Old RFC patches received feedback
concerning that, so I looked into it specifically:
GB_ADDR_CONFIG.ROW_SIZE needs to be considered when
calculating TILE_SPLIT, but does not need to be included
in the modifiers, and also PIPE_INTERLEAVE matters,
but it's hardcoded to the same value on all GFX6-8 GPUs
and changing it would break userspace, so let's assume
it isn't going to change. Therefore we don't need to
include that in modifiers. Mesa also reads NUM_RANKS
but actually doesn't use its value on GFX6-8.
As a side note, tiling works similarly on GFX4-5
(that is Evergreen and Northern Islands). But that
will need some additional PIPE_CONFIG enum values
as well as some extra fields not relevant to GFX6-8.
Initially, let's only expose the tiling modes that are
most relevant to sharing buffers between different
processes:
Exposed array modes (TILE field):
- 1D_TILED_THIN1: micro tiled only
- 2D_TILED_THIN1: macro tiled
Exposed micro tile modes (MICROTILE field):
- DISPLAY: supported by DCE (the display engine)
- THIN: more efficient but not displayable
Exposed macro tile modes:
All possible parameters (25088 permutations).
More modes may be exposed in the future as needed.
Technically, the amount of possible combinations
of all possible tiling parameters is in the range
of hundreds of thousands, but in practice, there are
just a handful of possible modifiers for a surface.
For example on GFX8, a surface would have these
modifiers, from best to worst performance:
- 2D_TILED_THIN1 + THIN + DCC + macrotile params [1]
- 2D_TILED_THIN1 + THIN + macrotile params [1]
- 2D_TILED_THIN1 + DISPLAY + macrotile params [1]
- 1D_TILED_THIN1 + THIN
- 1D_TILED_THIN1 + DISPLAY
- LINEAR
[1] The macro tiling parameters depend on how many
bits per pixel of the specific surface has and
how the chip is configured. There is only one set
of valid macrotile params for a given surface.
DCC is only supported by GFX8 and newer, and only
with non-displayable macrotiling modes.
When sharing buffers between different GFX6-8 GPUs,
it is very unlikely that they support the exact same
macrotiling configuration, so they will likely need
to use micro tiled modes, which are still much better
than using linear buffers. (Note that currently Mesa
always uses LINEAR when copying between two GPUs.)
Suggested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
Reviewed-by: Marek Olšák <maraeo@gmail.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
include/uapi/drm/drm_fourcc.h | 209 +++++++++++++++++++++++++++++++---
1 file changed, 191 insertions(+), 18 deletions(-)
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index e527b24bd824..7a0a1e5dac75 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -1620,36 +1620,69 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
* For multi-plane formats the above surfaces get merged into one plane for
* each format plane, based on the required alignment only.
*
- * Bits Parameter Notes
- * ----- ------------------------ ---------------------------------------------
+ * Bits Parameter Notes
+ * ------- ------------------------ ---------------------------------------------
+ *
+ * DRM format modifier fields on AMD GPUs:
+ * 7:0 TILE_VERSION Values are AMD_FMT_MOD_TILE_VER_*
+ * 12:8 TILE Values are AMD_FMT_MOD_TILE_<version>_*
+ * 13 DCC Delta Color Compression, supported on GFX8 and newer
+ 55:14 (chip specific) See below for details, depends on GFX block version
+ * 63:56 Vendor Value is DRM_FORMAT_MOD_VENDOR_AMD
+ *
+ * Chip specific fields on Gfx9 and newer:
+ * 14 DCC_RETILE
+ * 15 DCC_PIPE_ALIGN
+ * 16 DCC_INDEPENDENT_64B
+ * 17 DCC_INDEPENDENT_128B
+ * 19:18 DCC_MAX_COMPRESSED_BLOCK Values are AMD_FMT_MOD_DCC_BLOCK_*
+ * 20 DCC_CONSTANT_ENCODE
+ * 23:21 PIPE_XOR_BITS Only for some chips
+ * 26:24 BANK_XOR_BITS Only for some chips
+ * 29:27 PACKERS Only for some chips
+ * 32:30 RB Only for some chips
+ * 35:33 PIPE Only for some chips
+ * 55:36 - Reserved for future use, must be zero
+ *
+ * Chip specific fields on Gfx6-8:
+ * 16:14 MICROTILE Micro tile format
+ * 21:17 PIPE_CONFIG Number of pipes and how pipes are interleaved
+ * 24:22 TILE_SPLIT Tile split size
+ * 26:25 BANK_WIDTH Number of tiles in the X direction in the same bank
+ * 28:27 BANK_HEIGHT Number of tiles in the Y direction in the same bank
+ * 30:29 MACRO_TILE_ASPECT Macro tile aspect ratio
+ * 32:31 NUM_BANKS Number of banks
+ * 55:33 - Reserved for future use, must be zero
*
- * 7:0 TILE_VERSION Values are AMD_FMT_MOD_TILE_VER_*
- * 12:8 TILE Values are AMD_FMT_MOD_TILE_<version>_*
- * 13 DCC
- * 14 DCC_RETILE
- * 15 DCC_PIPE_ALIGN
- * 16 DCC_INDEPENDENT_64B
- * 17 DCC_INDEPENDENT_128B
- * 19:18 DCC_MAX_COMPRESSED_BLOCK Values are AMD_FMT_MOD_DCC_BLOCK_*
- * 20 DCC_CONSTANT_ENCODE
- * 23:21 PIPE_XOR_BITS Only for some chips
- * 26:24 BANK_XOR_BITS Only for some chips
- * 29:27 PACKERS Only for some chips
- * 32:30 RB Only for some chips
- * 35:33 PIPE Only for some chips
- * 55:36 - Reserved for future use, must be zero
*/
#define AMD_FMT_MOD fourcc_mod_code(AMD, 0)
#define IS_AMD_FMT_MOD(val) (((val) >> 56) == DRM_FORMAT_MOD_VENDOR_AMD)
-/* Reserve 0 for GFX8 and older */
+#define AMD_FMT_MOD_TILE_VER_GFX6 0
#define AMD_FMT_MOD_TILE_VER_GFX9 1
#define AMD_FMT_MOD_TILE_VER_GFX10 2
#define AMD_FMT_MOD_TILE_VER_GFX10_RBPLUS 3
#define AMD_FMT_MOD_TILE_VER_GFX11 4
#define AMD_FMT_MOD_TILE_VER_GFX12 5
+/*
+ * Gfx6-8 tiling modes.
+ * A complete reference implementation is found in addrlib in the Mesa code base.
+ *
+ * - Microtiled modes (1D):
+ * Pixel data is organized into micro tiles of 8x8 pixels.
+ *
+ * - Macrotiled modes (2D):
+ * Micro tiles are further organized into macro tiles.
+ * These are optimized for even load distribution among memory channels.
+ *
+ * Note that only THIN1 modes are exposed here.
+ * THICK and XTHICK are for 3D images and not relevant to DRM format modifiers.
+ */
+#define AMD_FMT_MOD_TILE_GFX6_1D_TILED_THIN1 0x2
+#define AMD_FMT_MOD_TILE_GFX6_2D_TILED_THIN1 0x4
+
/*
* 64K_S is the same for GFX9/GFX10/GFX10_RBPLUS and hence has GFX9 as canonical
* version.
@@ -1748,6 +1781,146 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
#define AMD_FMT_MOD_PIPE_SHIFT 33
#define AMD_FMT_MOD_PIPE_MASK 0x7
+/*
+ * MICRO_TILE_MODE, 3 bits. Determines the micro tile format.
+ * Only relevant to Gfx6-8.
+ *
+ * DISPLAY - Displayable tiling
+ * THIN - Non-displayable tiling, a.k.a thin micro tiling
+ * DEPTH, THICK - not exposed, not relevant to DRM format modifier use cases
+ * ROTATED - not exposed, not implemented in Linux or Mesa
+ */
+#define AMD_FMT_MOD_MICROTILE_SHIFT 14ULL
+#define AMD_FMT_MOD_MICROTILE_MASK 0x7
+
+#define AMD_FMT_MOD_MICROTILE_DISPLAY 0x0
+#define AMD_FMT_MOD_MICROTILE_THIN 0x1
+
+/*
+ * PIPE_CONFIG, 5 bits. Number of pipes and how pipes are interleaved on the surface,
+ * which means the shader engine tile size and packer tile size.
+ * Typically matches the number of memory channels, or number of RBs.
+ * Only relevant to Gfx6-8 macro tiled modes.
+ *
+ * P<n>_<a>x<b>_<c>x<d>
+ * where:
+ * <n> - number of pipes
+ * <a>x<b> - shader engine tile size
+ * <c>x<d> - packer tile size
+ */
+#define AMD_FMT_MOD_PIPE_CONFIG_SHIFT 17ULL
+#define AMD_FMT_MOD_PIPE_CONFIG_MASK 0x1f
+
+#define AMD_FMT_MOD_PIPE_CONFIG_P2 0x0
+#define AMD_FMT_MOD_PIPE_CONFIG_P4_8x16 0x4
+#define AMD_FMT_MOD_PIPE_CONFIG_P4_16x16 0x5
+#define AMD_FMT_MOD_PIPE_CONFIG_P4_16x32 0x6
+#define AMD_FMT_MOD_PIPE_CONFIG_P4_32x32 0x7
+#define AMD_FMT_MOD_PIPE_CONFIG_P8_16x16_8x16 0x8
+#define AMD_FMT_MOD_PIPE_CONFIG_P8_16x32_8x16 0x9
+#define AMD_FMT_MOD_PIPE_CONFIG_P8_32x32_8x16 0xa
+#define AMD_FMT_MOD_PIPE_CONFIG_P8_16x32_16x16 0xb
+#define AMD_FMT_MOD_PIPE_CONFIG_P8_32x32_16x16 0xc
+#define AMD_FMT_MOD_PIPE_CONFIG_P8_32x32_16x32 0xd
+#define AMD_FMT_MOD_PIPE_CONFIG_P8_32x64_32x32 0xe
+#define AMD_FMT_MOD_PIPE_CONFIG_P16_32x32_8x16 0x10
+#define AMD_FMT_MOD_PIPE_CONFIG_P16_32x32_16x16 0x11
+
+/*
+ * TILE_SPLIT, 3 bits.
+ * Only relevant to Gfx6-8 macro tiled modes.
+ *
+ * On GFX6 (or with depth tiling modes on GFX7 and newer),
+ * the GFX block uses the GB_TILE_MODE.TILE_SPLIT field directly.
+ *
+ * On GFX7 and newer with non-depth tiling modes, the GFX block uses a
+ * split factor which is stored in the GB_TILE_MODE.SAMPLE_SPLIT field.
+ * SAMPLE_SPLIT may be: 0 - 1 byte; 1 - 2 bytes; 2 - 4 bytes; 3 - 8 bytes.
+ * The actual tile size and tile split bytes are calculated as follows:
+ *
+ * bpp = ... <- bits per pixel in the current image
+ * thickness = ... <- depends on array mode; may be: 1, 4, 8
+ * num_samples = ... <- number of samples in the current image
+ * tile_size_pixels = 8 * 8
+ * tile_bytes_1x = thickness * tile_size_pixels * bpp / 8
+ * sample_split_factor = 1 << SAMPLE_SPLIT
+ * tile_split_bytes = clamp(tile_bytes_1x * sample_split_factor, 256, dram_row_size_bytes)
+ * tile_bytes = clamp(tile_bytes_1x * num_samples, 64, tile_split_bytes)
+ *
+ * In both cases, the display block (DCE) has no SAMPLE_SPLIT
+ * and just needs the tile split bytes in the GRPH_CONTROL.GRPH_TILE_SPLIT field.
+ * To maximize compatibility between GFX6-7, we don't include the SAMPLE_SPLIT
+ * in the format modifiers.
+ *
+ * The actual tile split in bytes is: 64 << field value
+ * Possible values of this field:
+ *
+ * 0 - Tile split is 64 bytes
+ * 1 - Tile split is 128 bytes
+ * 2 - Tile split is 256 bytes
+ * 3 - Tile split is 512 bytes
+ * 4 - Tile split is 1 KiB
+ * 5 - Tile split is 2 KiB
+ * 6 - Tile split is 4 KiB
+ */
+#define AMD_FMT_MOD_TILE_SPLIT_SHIFT 22ULL
+#define AMD_FMT_MOD_TILE_SPLIT_MASK 0x7
+
+/*
+ * BANK_WIDTH, 2 bits. Number of tiles in the X direction in the same bank.
+ * Only relevant to Gfx6-8 macro tiled modes.
+ * The actual bank width is: 1 << field value
+ * Possible values:
+ *
+ * 0 - bank width is 1
+ * 1 - bank width is 2
+ * 2 - bank width is 4
+ * 3 - bank width is 8
+ */
+#define AMD_FMT_MOD_BANK_WIDTH_SHIFT 25ULL
+#define AMD_FMT_MOD_BANK_WIDTH_MASK 0x3
+
+/*
+ * BANK_HEIGHT, 2 bits. Number of tiles in the Y direction in the same bank.
+ * Only relevant to Gfx6-8 macro tiled modes.
+ * The actual bank height is: 1 << field value
+ * Possible values:
+ *
+ * 0 - bank height is 1
+ * 1 - bank height is 2
+ * 2 - bank height is 4
+ * 3 - bank height is 8
+ */
+#define AMD_FMT_MOD_BANK_HEIGHT_SHIFT 27ULL
+#define AMD_FMT_MOD_BANK_HEIGHT_MASK 0x3
+
+/*
+ * MACRO_TILE_ASPECT, 2 bits. Macro tile aspect ratio.
+ * Only relevant to Gfx6-8 macro tiled modes.
+ * Possible values:
+ *
+ * 0 - aspect ratio is 1:1
+ * 1 - aspect ratio is 4:1
+ * 2 - aspect ratio is 16:1
+ * 3 - aspect ratio is 64:1
+ */
+#define AMD_FMT_MOD_MACRO_TILE_ASPECT_SHIFT 29ULL
+#define AMD_FMT_MOD_MACRO_TILE_ASPECT_MASK 0x3
+
+/*
+ * NUM_BANKS, 2 bits. Number of banks.
+ * Only relevant to Gfx6-8 macro tiled modes.
+ * The actual number of banks is: 2 << field value
+ * Possible values:
+ *
+ * 0 - number of banks is 2
+ * 1 - number of banks is 4
+ * 2 - number of banks is 8
+ * 3 - number of banks is 16
+ */
+#define AMD_FMT_MOD_NUM_BANKS_SHIFT 31ULL
+#define AMD_FMT_MOD_NUM_BANKS_MASK 0x3
+
#define AMD_FMT_MOD_SET(field, value) \
((__u64)(value) << AMD_FMT_MOD_##field##_SHIFT)
#define AMD_FMT_MOD_GET(field, value) \
--
2.55.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] drm/fourcc: Add modifiers for AMD GFX6-8
2026-07-15 10:57 ` [PATCH 1/4] drm/fourcc: Add modifiers for AMD GFX6-8 Timur Kristóf
@ 2026-07-15 12:11 ` Christian König
0 siblings, 0 replies; 13+ messages in thread
From: Christian König @ 2026-07-15 12:11 UTC (permalink / raw)
To: Timur Kristóf, amd-gfx, dri-devel, Alex Deucher,
Tvrtko Ursulin, pierre-eric.pelloux-prayer, Natalie Vock, Dieter
Cc: Bas Nieuwenhuizen, Marek Olšák, Daniel Stone
On 7/15/26 12:57, Timur Kristóf wrote:
> GFX6-8 are the oldest GPUs supported by the amdgpu
> kernel driver, and the last ones that didn't support
> DRM format modifiers until now. These are the Southern
> Islands, Sea Islands and Volcanic Islands families
> of GPUs.
>
> On GFX6-8, the GFX block can only use pre-determined tiling
> modes which are programmed by the kernel according to the
> tiling mode table. GFX6 uses the GB_TILE_MODE0...31 registers,
> and GFX7-8 also has GB_MACROTILE_MODE0...15 registers.
> DCC is also supported on GFX8, albeit not displayable.
>
> Note that the tiling table is uAPI and userspace relies on
> specific modes being present at specific indices.
>
> How the tiling works is primarily determined by the
> so-called array mode.
> Use the TILE field to specify the array mode.
>
> Pixel data is organized into micro tiles.
> Each micro tile may be 8x8 / 8x8x4 / 8x8x8 pixels,
> depending on the array mode.
> Add the MICROTILE field to specify microtile mode.
>
> Microtiles may be further organized into macro tiles,
> which have many configurable parameters. Macro tile mode
> selection depends on how many bits per pixel an image has.
> Add the PIPE_CONFIG, TILE_SPLIT, BANK_WIDTH, BANK_HEIGHT,
> MACRO_TILE_ASPECT, NUM_BANKS fields to specify parameters
> of macro tiled modes.
>
> Furthermore, tiling is also influenced by memory
> configuration. Old RFC patches received feedback
> concerning that, so I looked into it specifically:
> GB_ADDR_CONFIG.ROW_SIZE needs to be considered when
> calculating TILE_SPLIT, but does not need to be included
> in the modifiers, and also PIPE_INTERLEAVE matters,
> but it's hardcoded to the same value on all GFX6-8 GPUs
> and changing it would break userspace, so let's assume
> it isn't going to change. Therefore we don't need to
> include that in modifiers. Mesa also reads NUM_RANKS
> but actually doesn't use its value on GFX6-8.
>
> As a side note, tiling works similarly on GFX4-5
> (that is Evergreen and Northern Islands). But that
> will need some additional PIPE_CONFIG enum values
> as well as some extra fields not relevant to GFX6-8.
>
> Initially, let's only expose the tiling modes that are
> most relevant to sharing buffers between different
> processes:
>
> Exposed array modes (TILE field):
> - 1D_TILED_THIN1: micro tiled only
> - 2D_TILED_THIN1: macro tiled
>
> Exposed micro tile modes (MICROTILE field):
> - DISPLAY: supported by DCE (the display engine)
> - THIN: more efficient but not displayable
>
> Exposed macro tile modes:
> All possible parameters (25088 permutations).
>
> More modes may be exposed in the future as needed.
>
> Technically, the amount of possible combinations
> of all possible tiling parameters is in the range
> of hundreds of thousands, but in practice, there are
> just a handful of possible modifiers for a surface.
>
> For example on GFX8, a surface would have these
> modifiers, from best to worst performance:
>
> - 2D_TILED_THIN1 + THIN + DCC + macrotile params [1]
> - 2D_TILED_THIN1 + THIN + macrotile params [1]
> - 2D_TILED_THIN1 + DISPLAY + macrotile params [1]
> - 1D_TILED_THIN1 + THIN
> - 1D_TILED_THIN1 + DISPLAY
> - LINEAR
>
> [1] The macro tiling parameters depend on how many
> bits per pixel of the specific surface has and
> how the chip is configured. There is only one set
> of valid macrotile params for a given surface.
>
> DCC is only supported by GFX8 and newer, and only
> with non-displayable macrotiling modes.
>
> When sharing buffers between different GFX6-8 GPUs,
> it is very unlikely that they support the exact same
> macrotiling configuration, so they will likely need
> to use micro tiled modes, which are still much better
> than using linear buffers. (Note that currently Mesa
> always uses LINEAR when copying between two GPUs.)
>
> Suggested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> Reviewed-by: Marek Olšák <maraeo@gmail.com>
> Reviewed-by: Daniel Stone <daniels@collabora.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Feel free to add Acked-by: Christian König <christian.koenig@amd.com> to the entire series.
> ---
> include/uapi/drm/drm_fourcc.h | 209 +++++++++++++++++++++++++++++++---
> 1 file changed, 191 insertions(+), 18 deletions(-)
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index e527b24bd824..7a0a1e5dac75 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -1620,36 +1620,69 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
> * For multi-plane formats the above surfaces get merged into one plane for
> * each format plane, based on the required alignment only.
> *
> - * Bits Parameter Notes
> - * ----- ------------------------ ---------------------------------------------
> + * Bits Parameter Notes
> + * ------- ------------------------ ---------------------------------------------
> + *
> + * DRM format modifier fields on AMD GPUs:
> + * 7:0 TILE_VERSION Values are AMD_FMT_MOD_TILE_VER_*
> + * 12:8 TILE Values are AMD_FMT_MOD_TILE_<version>_*
> + * 13 DCC Delta Color Compression, supported on GFX8 and newer
> + 55:14 (chip specific) See below for details, depends on GFX block version
> + * 63:56 Vendor Value is DRM_FORMAT_MOD_VENDOR_AMD
> + *
> + * Chip specific fields on Gfx9 and newer:
> + * 14 DCC_RETILE
> + * 15 DCC_PIPE_ALIGN
> + * 16 DCC_INDEPENDENT_64B
> + * 17 DCC_INDEPENDENT_128B
> + * 19:18 DCC_MAX_COMPRESSED_BLOCK Values are AMD_FMT_MOD_DCC_BLOCK_*
> + * 20 DCC_CONSTANT_ENCODE
> + * 23:21 PIPE_XOR_BITS Only for some chips
> + * 26:24 BANK_XOR_BITS Only for some chips
> + * 29:27 PACKERS Only for some chips
> + * 32:30 RB Only for some chips
> + * 35:33 PIPE Only for some chips
> + * 55:36 - Reserved for future use, must be zero
> + *
> + * Chip specific fields on Gfx6-8:
> + * 16:14 MICROTILE Micro tile format
> + * 21:17 PIPE_CONFIG Number of pipes and how pipes are interleaved
> + * 24:22 TILE_SPLIT Tile split size
> + * 26:25 BANK_WIDTH Number of tiles in the X direction in the same bank
> + * 28:27 BANK_HEIGHT Number of tiles in the Y direction in the same bank
> + * 30:29 MACRO_TILE_ASPECT Macro tile aspect ratio
> + * 32:31 NUM_BANKS Number of banks
> + * 55:33 - Reserved for future use, must be zero
> *
> - * 7:0 TILE_VERSION Values are AMD_FMT_MOD_TILE_VER_*
> - * 12:8 TILE Values are AMD_FMT_MOD_TILE_<version>_*
> - * 13 DCC
> - * 14 DCC_RETILE
> - * 15 DCC_PIPE_ALIGN
> - * 16 DCC_INDEPENDENT_64B
> - * 17 DCC_INDEPENDENT_128B
> - * 19:18 DCC_MAX_COMPRESSED_BLOCK Values are AMD_FMT_MOD_DCC_BLOCK_*
> - * 20 DCC_CONSTANT_ENCODE
> - * 23:21 PIPE_XOR_BITS Only for some chips
> - * 26:24 BANK_XOR_BITS Only for some chips
> - * 29:27 PACKERS Only for some chips
> - * 32:30 RB Only for some chips
> - * 35:33 PIPE Only for some chips
> - * 55:36 - Reserved for future use, must be zero
> */
> #define AMD_FMT_MOD fourcc_mod_code(AMD, 0)
>
> #define IS_AMD_FMT_MOD(val) (((val) >> 56) == DRM_FORMAT_MOD_VENDOR_AMD)
>
> -/* Reserve 0 for GFX8 and older */
> +#define AMD_FMT_MOD_TILE_VER_GFX6 0
> #define AMD_FMT_MOD_TILE_VER_GFX9 1
> #define AMD_FMT_MOD_TILE_VER_GFX10 2
> #define AMD_FMT_MOD_TILE_VER_GFX10_RBPLUS 3
> #define AMD_FMT_MOD_TILE_VER_GFX11 4
> #define AMD_FMT_MOD_TILE_VER_GFX12 5
>
> +/*
> + * Gfx6-8 tiling modes.
> + * A complete reference implementation is found in addrlib in the Mesa code base.
> + *
> + * - Microtiled modes (1D):
> + * Pixel data is organized into micro tiles of 8x8 pixels.
> + *
> + * - Macrotiled modes (2D):
> + * Micro tiles are further organized into macro tiles.
> + * These are optimized for even load distribution among memory channels.
> + *
> + * Note that only THIN1 modes are exposed here.
> + * THICK and XTHICK are for 3D images and not relevant to DRM format modifiers.
> + */
> +#define AMD_FMT_MOD_TILE_GFX6_1D_TILED_THIN1 0x2
> +#define AMD_FMT_MOD_TILE_GFX6_2D_TILED_THIN1 0x4
> +
> /*
> * 64K_S is the same for GFX9/GFX10/GFX10_RBPLUS and hence has GFX9 as canonical
> * version.
> @@ -1748,6 +1781,146 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
> #define AMD_FMT_MOD_PIPE_SHIFT 33
> #define AMD_FMT_MOD_PIPE_MASK 0x7
>
> +/*
> + * MICRO_TILE_MODE, 3 bits. Determines the micro tile format.
> + * Only relevant to Gfx6-8.
> + *
> + * DISPLAY - Displayable tiling
> + * THIN - Non-displayable tiling, a.k.a thin micro tiling
> + * DEPTH, THICK - not exposed, not relevant to DRM format modifier use cases
> + * ROTATED - not exposed, not implemented in Linux or Mesa
> + */
> +#define AMD_FMT_MOD_MICROTILE_SHIFT 14ULL
> +#define AMD_FMT_MOD_MICROTILE_MASK 0x7
> +
> +#define AMD_FMT_MOD_MICROTILE_DISPLAY 0x0
> +#define AMD_FMT_MOD_MICROTILE_THIN 0x1
> +
> +/*
> + * PIPE_CONFIG, 5 bits. Number of pipes and how pipes are interleaved on the surface,
> + * which means the shader engine tile size and packer tile size.
> + * Typically matches the number of memory channels, or number of RBs.
> + * Only relevant to Gfx6-8 macro tiled modes.
> + *
> + * P<n>_<a>x<b>_<c>x<d>
> + * where:
> + * <n> - number of pipes
> + * <a>x<b> - shader engine tile size
> + * <c>x<d> - packer tile size
> + */
> +#define AMD_FMT_MOD_PIPE_CONFIG_SHIFT 17ULL
> +#define AMD_FMT_MOD_PIPE_CONFIG_MASK 0x1f
> +
> +#define AMD_FMT_MOD_PIPE_CONFIG_P2 0x0
> +#define AMD_FMT_MOD_PIPE_CONFIG_P4_8x16 0x4
> +#define AMD_FMT_MOD_PIPE_CONFIG_P4_16x16 0x5
> +#define AMD_FMT_MOD_PIPE_CONFIG_P4_16x32 0x6
> +#define AMD_FMT_MOD_PIPE_CONFIG_P4_32x32 0x7
> +#define AMD_FMT_MOD_PIPE_CONFIG_P8_16x16_8x16 0x8
> +#define AMD_FMT_MOD_PIPE_CONFIG_P8_16x32_8x16 0x9
> +#define AMD_FMT_MOD_PIPE_CONFIG_P8_32x32_8x16 0xa
> +#define AMD_FMT_MOD_PIPE_CONFIG_P8_16x32_16x16 0xb
> +#define AMD_FMT_MOD_PIPE_CONFIG_P8_32x32_16x16 0xc
> +#define AMD_FMT_MOD_PIPE_CONFIG_P8_32x32_16x32 0xd
> +#define AMD_FMT_MOD_PIPE_CONFIG_P8_32x64_32x32 0xe
> +#define AMD_FMT_MOD_PIPE_CONFIG_P16_32x32_8x16 0x10
> +#define AMD_FMT_MOD_PIPE_CONFIG_P16_32x32_16x16 0x11
> +
> +/*
> + * TILE_SPLIT, 3 bits.
> + * Only relevant to Gfx6-8 macro tiled modes.
> + *
> + * On GFX6 (or with depth tiling modes on GFX7 and newer),
> + * the GFX block uses the GB_TILE_MODE.TILE_SPLIT field directly.
> + *
> + * On GFX7 and newer with non-depth tiling modes, the GFX block uses a
> + * split factor which is stored in the GB_TILE_MODE.SAMPLE_SPLIT field.
> + * SAMPLE_SPLIT may be: 0 - 1 byte; 1 - 2 bytes; 2 - 4 bytes; 3 - 8 bytes.
> + * The actual tile size and tile split bytes are calculated as follows:
> + *
> + * bpp = ... <- bits per pixel in the current image
> + * thickness = ... <- depends on array mode; may be: 1, 4, 8
> + * num_samples = ... <- number of samples in the current image
> + * tile_size_pixels = 8 * 8
> + * tile_bytes_1x = thickness * tile_size_pixels * bpp / 8
> + * sample_split_factor = 1 << SAMPLE_SPLIT
> + * tile_split_bytes = clamp(tile_bytes_1x * sample_split_factor, 256, dram_row_size_bytes)
> + * tile_bytes = clamp(tile_bytes_1x * num_samples, 64, tile_split_bytes)
> + *
> + * In both cases, the display block (DCE) has no SAMPLE_SPLIT
> + * and just needs the tile split bytes in the GRPH_CONTROL.GRPH_TILE_SPLIT field.
> + * To maximize compatibility between GFX6-7, we don't include the SAMPLE_SPLIT
> + * in the format modifiers.
> + *
> + * The actual tile split in bytes is: 64 << field value
> + * Possible values of this field:
> + *
> + * 0 - Tile split is 64 bytes
> + * 1 - Tile split is 128 bytes
> + * 2 - Tile split is 256 bytes
> + * 3 - Tile split is 512 bytes
> + * 4 - Tile split is 1 KiB
> + * 5 - Tile split is 2 KiB
> + * 6 - Tile split is 4 KiB
> + */
> +#define AMD_FMT_MOD_TILE_SPLIT_SHIFT 22ULL
> +#define AMD_FMT_MOD_TILE_SPLIT_MASK 0x7
> +
> +/*
> + * BANK_WIDTH, 2 bits. Number of tiles in the X direction in the same bank.
> + * Only relevant to Gfx6-8 macro tiled modes.
> + * The actual bank width is: 1 << field value
> + * Possible values:
> + *
> + * 0 - bank width is 1
> + * 1 - bank width is 2
> + * 2 - bank width is 4
> + * 3 - bank width is 8
> + */
> +#define AMD_FMT_MOD_BANK_WIDTH_SHIFT 25ULL
> +#define AMD_FMT_MOD_BANK_WIDTH_MASK 0x3
> +
> +/*
> + * BANK_HEIGHT, 2 bits. Number of tiles in the Y direction in the same bank.
> + * Only relevant to Gfx6-8 macro tiled modes.
> + * The actual bank height is: 1 << field value
> + * Possible values:
> + *
> + * 0 - bank height is 1
> + * 1 - bank height is 2
> + * 2 - bank height is 4
> + * 3 - bank height is 8
> + */
> +#define AMD_FMT_MOD_BANK_HEIGHT_SHIFT 27ULL
> +#define AMD_FMT_MOD_BANK_HEIGHT_MASK 0x3
> +
> +/*
> + * MACRO_TILE_ASPECT, 2 bits. Macro tile aspect ratio.
> + * Only relevant to Gfx6-8 macro tiled modes.
> + * Possible values:
> + *
> + * 0 - aspect ratio is 1:1
> + * 1 - aspect ratio is 4:1
> + * 2 - aspect ratio is 16:1
> + * 3 - aspect ratio is 64:1
> + */
> +#define AMD_FMT_MOD_MACRO_TILE_ASPECT_SHIFT 29ULL
> +#define AMD_FMT_MOD_MACRO_TILE_ASPECT_MASK 0x3
> +
> +/*
> + * NUM_BANKS, 2 bits. Number of banks.
> + * Only relevant to Gfx6-8 macro tiled modes.
> + * The actual number of banks is: 2 << field value
> + * Possible values:
> + *
> + * 0 - number of banks is 2
> + * 1 - number of banks is 4
> + * 2 - number of banks is 8
> + * 3 - number of banks is 16
> + */
> +#define AMD_FMT_MOD_NUM_BANKS_SHIFT 31ULL
> +#define AMD_FMT_MOD_NUM_BANKS_MASK 0x3
> +
> #define AMD_FMT_MOD_SET(field, value) \
> ((__u64)(value) << AMD_FMT_MOD_##field##_SHIFT)
> #define AMD_FMT_MOD_GET(field, value) \
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] drm/amdgpu: Convert tiling flags to modifiers on GFX6-8
2026-07-15 10:57 [PATCH 0/4] drm/amdgpu: Support DRM format modifiers on GFX6-8 (v2) Timur Kristóf
2026-07-15 10:57 ` [PATCH 1/4] drm/fourcc: Add modifiers for AMD GFX6-8 Timur Kristóf
@ 2026-07-15 10:57 ` Timur Kristóf
2026-07-15 11:10 ` sashiko-bot
2026-07-15 10:57 ` [PATCH 3/4] drm/amd/display: Support DRM format " Timur Kristóf
2026-07-15 10:57 ` [PATCH 4/4] drm/amd/display: Don't use tiling flags anymore Timur Kristóf
3 siblings, 1 reply; 13+ messages in thread
From: Timur Kristóf @ 2026-07-15 10:57 UTC (permalink / raw)
To: amd-gfx, dri-devel, Alex Deucher, christian.koenig,
Tvrtko Ursulin, pierre-eric.pelloux-prayer, Natalie Vock, Dieter
Cc: Timur Kristóf, Link Mauve, Nikola Medić,
Marek Olšák, Daniel Stone
This is done for consistency between different GPU generations.
Tiling flags are "implicit modifiers" which are used on AMD GPUs
to let the kernel know the tiling information without modifiers.
Convert the tiling flags to modifers on GFX8 and older, so that
the DC display driver can rely on them like on newer generations.
Note that this code path will only be taken when DC actually
exposes any modifiers on GFX6-8, which is handled in a subsequent
commit after this one.
Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
Tested-by: Link Mauve <linkmauve@linkmauve.fr>
Tested-by: Nikola Medić <nmedic89@gmail.com>
Reviewed-by: Marek Olšák <maraeo@gmail.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 111 +++++++++++++++++++-
1 file changed, 108 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 950937b363fb..dcca930ed3c0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -746,7 +746,7 @@ static int convert_tiling_flags_to_modifier_gfx12(struct amdgpu_framebuffer *afb
return 0;
}
-static int convert_tiling_flags_to_modifier(struct amdgpu_framebuffer *afb)
+static int convert_tiling_flags_to_modifier_gfx9(struct amdgpu_framebuffer *afb)
{
struct amdgpu_device *adev = drm_to_adev(afb->base.dev);
uint64_t modifier = 0;
@@ -940,6 +940,55 @@ static int convert_tiling_flags_to_modifier(struct amdgpu_framebuffer *afb)
return 0;
}
+static int convert_tiling_flags_to_modifier_gfx6(struct amdgpu_framebuffer *afb)
+{
+ const uint32_t array_mode = AMDGPU_TILING_GET(afb->tiling_flags, ARRAY_MODE);
+ const uint32_t pipe_config = AMDGPU_TILING_GET(afb->tiling_flags, PIPE_CONFIG);
+ const uint32_t tile_split = AMDGPU_TILING_GET(afb->tiling_flags, TILE_SPLIT);
+ const uint32_t micro_tile_mode = AMDGPU_TILING_GET(afb->tiling_flags, MICRO_TILE_MODE);
+ const uint32_t bank_width = AMDGPU_TILING_GET(afb->tiling_flags, BANK_WIDTH);
+ const uint32_t bank_height = AMDGPU_TILING_GET(afb->tiling_flags, BANK_HEIGHT);
+ const uint32_t macro_tile_aspect = AMDGPU_TILING_GET(afb->tiling_flags, MACRO_TILE_ASPECT);
+ const uint32_t num_banks = AMDGPU_TILING_GET(afb->tiling_flags, NUM_BANKS);
+ struct amdgpu_device *adev = drm_to_adev(afb->base.dev);
+ uint64_t modifier = 0;
+
+ switch (array_mode) {
+ case DC_ARRAY_LINEAR_GENERAL:
+ case DC_ARRAY_LINEAR_ALLIGNED:
+ modifier = DRM_FORMAT_MOD_LINEAR;
+ break;
+
+ case DC_ARRAY_2D_TILED_THIN1:
+ /* Macro tiled modes only */
+ modifier |=
+ AMD_FMT_MOD_SET(PIPE_CONFIG, pipe_config) |
+ AMD_FMT_MOD_SET(TILE_SPLIT, tile_split) |
+ AMD_FMT_MOD_SET(BANK_WIDTH, bank_width) |
+ AMD_FMT_MOD_SET(BANK_HEIGHT, bank_height) |
+ AMD_FMT_MOD_SET(MACRO_TILE_ASPECT, macro_tile_aspect) |
+ AMD_FMT_MOD_SET(NUM_BANKS, num_banks);
+ fallthrough;
+
+ case DC_ARRAY_1D_TILED_THIN1:
+ /* Micro and macro tiled modes */
+ modifier |=
+ AMD_FMT_MOD |
+ AMD_FMT_MOD_SET(TILE_VERSION, AMD_FMT_MOD_TILE_VER_GFX6) |
+ AMD_FMT_MOD_SET(TILE, array_mode) |
+ AMD_FMT_MOD_SET(MICROTILE, micro_tile_mode);
+ break;
+
+ default:
+ drm_err(&adev->ddev, "array mode 0x%x not supported by DCE\n", array_mode);
+ return -EINVAL;
+ }
+
+ afb->base.modifier = modifier;
+ afb->base.flags |= DRM_MODE_FB_MODIFIERS;
+ return 0;
+}
+
/* Mirrors the is_displayable check in radeonsi's gfx6_compute_surface */
static int check_tiling_flags_gfx6(struct amdgpu_framebuffer *afb)
{
@@ -1093,7 +1142,7 @@ static int amdgpu_display_verify_sizes(struct amdgpu_framebuffer *rfb)
get_block_dimensions(block_size_log2, format_info->cpp[i],
&block_width, &block_height);
- } else {
+ } else if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) >= AMD_FMT_MOD_TILE_VER_GFX9) {
int swizzle = AMD_FMT_MOD_GET(TILE, modifier);
switch ((swizzle & ~3) + 1) {
@@ -1120,6 +1169,60 @@ static int amdgpu_display_verify_sizes(struct amdgpu_framebuffer *rfb)
get_block_dimensions(block_size_log2, format_info->cpp[i],
&block_width, &block_height);
+ } else if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) == AMD_FMT_MOD_TILE_VER_GFX6) {
+ const u32 display_micro_tile_pitch = 32; /* required by DCE */
+ const u32 micro_tile_width = 8;
+ const u32 micro_tile_height = 8;
+ const u32 micro_tile_mode = AMD_FMT_MOD_GET(MICROTILE, modifier);
+ const u32 array_mode = AMD_FMT_MOD_GET(TILE, modifier);
+ u32 num_banks, bank_width, bank_height, pipe_config, macro_tile_aspect;
+ u32 num_pipes;
+
+ if (AMD_FMT_MOD_GET(DCC, modifier)) {
+ drm_dbg_kms(rfb->base.dev, "DCC is not displayable on GFX6-8\n");
+ return -EINVAL;
+ }
+ if (array_mode != AMD_FMT_MOD_TILE_GFX6_1D_TILED_THIN1 &&
+ array_mode != AMD_FMT_MOD_TILE_GFX6_2D_TILED_THIN1) {
+ drm_dbg_kms(rfb->base.dev,
+ "Array mode %u is not supported by the display driver\n",
+ array_mode);
+ return -EINVAL;
+ }
+ if (micro_tile_mode != AMD_FMT_MOD_MICROTILE_DISPLAY) {
+ drm_dbg_kms(rfb->base.dev,
+ "Micro tile mode %u is not displayable on GFX6-8\n",
+ micro_tile_mode);
+ return -EINVAL;
+ }
+
+ num_banks = 2 << AMD_FMT_MOD_GET(NUM_BANKS, modifier);
+ bank_width = 1 << AMD_FMT_MOD_GET(BANK_WIDTH, modifier);
+ bank_height = 1 << AMD_FMT_MOD_GET(BANK_HEIGHT, modifier);
+ pipe_config = AMD_FMT_MOD_GET(PIPE_CONFIG, modifier);
+ macro_tile_aspect = 1 << AMD_FMT_MOD_GET(MACRO_TILE_ASPECT, modifier);
+
+ if (pipe_config >= AMD_FMT_MOD_PIPE_CONFIG_P16_32x32_8x16)
+ num_pipes = 16;
+ else if (pipe_config >= AMD_FMT_MOD_PIPE_CONFIG_P8_16x16_8x16)
+ num_pipes = 8;
+ else if (pipe_config >= AMD_FMT_MOD_PIPE_CONFIG_P4_8x16)
+ num_pipes = 4;
+ else if (pipe_config == AMD_FMT_MOD_PIPE_CONFIG_P2)
+ num_pipes = 2;
+ else
+ unreachable();
+
+ if (array_mode < AMD_FMT_MOD_TILE_GFX6_2D_TILED_THIN1) {
+ block_width = display_micro_tile_pitch;
+ block_height = micro_tile_height;
+ } else {
+ /* Assume non-PRT macro tiling modes */
+ block_width = num_pipes * micro_tile_width *
+ bank_width * macro_tile_aspect;
+ block_height = micro_tile_height * bank_height *
+ num_banks / macro_tile_aspect;
+ }
}
ret = amdgpu_display_verify_plane(rfb, i, format_info,
@@ -1271,8 +1374,10 @@ static int amdgpu_display_framebuffer_init(struct drm_device *dev,
!(rfb->base.flags & DRM_MODE_FB_MODIFIERS)) {
if (amdgpu_ip_version(adev, GC_HWIP, 0) >= IP_VERSION(12, 0, 0))
ret = convert_tiling_flags_to_modifier_gfx12(rfb);
+ else if (amdgpu_ip_version(adev, GC_HWIP, 0) >= IP_VERSION(9, 0, 0))
+ ret = convert_tiling_flags_to_modifier_gfx9(rfb);
else
- ret = convert_tiling_flags_to_modifier(rfb);
+ ret = convert_tiling_flags_to_modifier_gfx6(rfb);
if (ret) {
drm_dbg_kms(dev, "Failed to convert tiling flags 0x%llX to a modifier",
--
2.55.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] drm/amdgpu: Convert tiling flags to modifiers on GFX6-8
2026-07-15 10:57 ` [PATCH 2/4] drm/amdgpu: Convert tiling flags to modifiers on GFX6-8 Timur Kristóf
@ 2026-07-15 11:10 ` sashiko-bot
2026-07-15 20:33 ` Timur Kristóf
0 siblings, 1 reply; 13+ messages in thread
From: sashiko-bot @ 2026-07-15 11:10 UTC (permalink / raw)
To: Timur Kristóf; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Uninitialized variable `block_size_log2` in GFX6 modifier path causes undefined behavior and potential kernel panic.
- [High] User-controlled `PIPE_CONFIG` in GFX6 modifier can trigger `unreachable()`, leading to kernel panic or undefined behavior.
- [Medium] Unbounded `drm_err()` on invalid GFX6 array mode allows userspace to flood the kernel log.
--
--- Patch [2]: [PATCH 2/4] drm/amdgpu: Convert tiling flags to modifiers on GFX6-8 ---
commit 642125971654c581668fe700356dfbee6905eccc
Author: Timur Kristóf <timur.kristof@gmail.com>
Subject: drm/amdgpu: Convert tiling flags to modifiers on GFX6-8
This commit converts the tiling flags to modifiers on GFX8 and older,
so that the DC display driver can rely on them like on newer generations.
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index 950937b363fbe..dcca930ed3c04 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
[ ... ]
> @@ -940,6 +940,55 @@ static int convert_tiling_flags_to_modifier(struct amdgpu_framebuffer *afb)
> return 0;
> }
>
> +static int convert_tiling_flags_to_modifier_gfx6(struct amdgpu_framebuffer *afb)
> +{
> + const uint32_t array_mode = AMDGPU_TILING_GET(afb->tiling_flags, ARRAY_MODE);
> + const uint32_t pipe_config = AMDGPU_TILING_GET(afb->tiling_flags, PIPE_CONFIG);
[ ... ]
> + switch (array_mode) {
> + case DC_ARRAY_LINEAR_GENERAL:
> + case DC_ARRAY_LINEAR_ALLIGNED:
> + modifier = DRM_FORMAT_MOD_LINEAR;
> + break;
> +
> + case DC_ARRAY_2D_TILED_THIN1:
> + /* Macro tiled modes only */
> + modifier |=
> + AMD_FMT_MOD_SET(PIPE_CONFIG, pipe_config) |
> + AMD_FMT_MOD_SET(TILE_SPLIT, tile_split) |
> + AMD_FMT_MOD_SET(BANK_WIDTH, bank_width) |
> + AMD_FMT_MOD_SET(BANK_HEIGHT, bank_height) |
> + AMD_FMT_MOD_SET(MACRO_TILE_ASPECT, macro_tile_aspect) |
> + AMD_FMT_MOD_SET(NUM_BANKS, num_banks);
> + fallthrough;
> +
> + case DC_ARRAY_1D_TILED_THIN1:
> + /* Micro and macro tiled modes */
> + modifier |=
> + AMD_FMT_MOD |
> + AMD_FMT_MOD_SET(TILE_VERSION, AMD_FMT_MOD_TILE_VER_GFX6) |
> + AMD_FMT_MOD_SET(TILE, array_mode) |
> + AMD_FMT_MOD_SET(MICROTILE, micro_tile_mode);
> + break;
> +
> + default:
> + drm_err(&adev->ddev, "array mode 0x%x not supported by DCE\n", array_mode);
[Severity: Medium]
Could this drm_err() be triggered repeatedly by a user space process providing
an invalid array_mode to DRM_IOCTL_MODE_ADDFB2, flooding the kernel log?
Should this use a rate-limited print or drm_dbg_kms() instead to prevent log
spam from unprivileged users?
> + return -EINVAL;
> + }
> +
> + afb->base.modifier = modifier;
> + afb->base.flags |= DRM_MODE_FB_MODIFIERS;
> + return 0;
> +}
[ ... ]
> @@ -1093,7 +1142,7 @@ static int amdgpu_display_verify_sizes(struct amdgpu_framebuffer *rfb)
>
> get_block_dimensions(block_size_log2, format_info->cpp[i],
> &block_width, &block_height);
> - } else {
> + } else if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) >= AMD_FMT_MOD_TILE_VER_GFX9) {
> int swizzle = AMD_FMT_MOD_GET(TILE, modifier);
>
> switch ((swizzle & ~3) + 1) {
> @@ -1120,6 +1169,60 @@ static int amdgpu_display_verify_sizes(struct amdgpu_framebuffer *rfb)
>
> get_block_dimensions(block_size_log2, format_info->cpp[i],
> &block_width, &block_height);
> + } else if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) == AMD_FMT_MOD_TILE_VER_GFX6) {
> + const u32 display_micro_tile_pitch = 32; /* required by DCE */
> + const u32 micro_tile_width = 8;
> + const u32 micro_tile_height = 8;
> + const u32 micro_tile_mode = AMD_FMT_MOD_GET(MICROTILE, modifier);
> + const u32 array_mode = AMD_FMT_MOD_GET(TILE, modifier);
> + u32 num_banks, bank_width, bank_height, pipe_config, macro_tile_aspect;
> + u32 num_pipes;
> +
> + if (AMD_FMT_MOD_GET(DCC, modifier)) {
> + drm_dbg_kms(rfb->base.dev, "DCC is not displayable on GFX6-8\n");
> + return -EINVAL;
> + }
> + if (array_mode != AMD_FMT_MOD_TILE_GFX6_1D_TILED_THIN1 &&
> + array_mode != AMD_FMT_MOD_TILE_GFX6_2D_TILED_THIN1) {
> + drm_dbg_kms(rfb->base.dev,
> + "Array mode %u is not supported by the display driver\n",
> + array_mode);
> + return -EINVAL;
> + }
> + if (micro_tile_mode != AMD_FMT_MOD_MICROTILE_DISPLAY) {
> + drm_dbg_kms(rfb->base.dev,
> + "Micro tile mode %u is not displayable on GFX6-8\n",
> + micro_tile_mode);
> + return -EINVAL;
> + }
> +
> + num_banks = 2 << AMD_FMT_MOD_GET(NUM_BANKS, modifier);
> + bank_width = 1 << AMD_FMT_MOD_GET(BANK_WIDTH, modifier);
> + bank_height = 1 << AMD_FMT_MOD_GET(BANK_HEIGHT, modifier);
> + pipe_config = AMD_FMT_MOD_GET(PIPE_CONFIG, modifier);
> + macro_tile_aspect = 1 << AMD_FMT_MOD_GET(MACRO_TILE_ASPECT, modifier);
> +
> + if (pipe_config >= AMD_FMT_MOD_PIPE_CONFIG_P16_32x32_8x16)
> + num_pipes = 16;
> + else if (pipe_config >= AMD_FMT_MOD_PIPE_CONFIG_P8_16x16_8x16)
> + num_pipes = 8;
> + else if (pipe_config >= AMD_FMT_MOD_PIPE_CONFIG_P4_8x16)
> + num_pipes = 4;
> + else if (pipe_config == AMD_FMT_MOD_PIPE_CONFIG_P2)
> + num_pipes = 2;
> + else
> + unreachable();
[Severity: High]
Since pipe_config comes from AMD_FMT_MOD_GET(PIPE_CONFIG, modifier), which is
user-controlled via the framebuffer modifier, could a user pass a value smaller
than AMD_FMT_MOD_PIPE_CONFIG_P2?
If so, hitting unreachable() here would result in undefined compiler behavior.
With CONFIG_OBJTOOL enabled, it can emit an invalid instruction that immediately
panics the kernel. If execution falls through, num_pipes remains uninitialized
and is subsequently used in a multiplication.
> +
> + if (array_mode < AMD_FMT_MOD_TILE_GFX6_2D_TILED_THIN1) {
> + block_width = display_micro_tile_pitch;
> + block_height = micro_tile_height;
> + } else {
> + /* Assume non-PRT macro tiling modes */
> + block_width = num_pipes * micro_tile_width *
> + bank_width * macro_tile_aspect;
> + block_height = micro_tile_height * bank_height *
> + num_banks / macro_tile_aspect;
> + }
> }
>
> ret = amdgpu_display_verify_plane(rfb, i, format_info,
> block_width, block_height, block_size_log2);
[Severity: High]
Does this pass an uninitialized block_size_log2 into
amdgpu_display_verify_plane()?
It appears block_size_log2 is never set in the new AMD_FMT_MOD_TILE_VER_GFX6
branch just above this call.
Inside amdgpu_display_verify_plane(), this uninitialized value is used in a
shift:
unsigned int block_size = 1 << block_size_log2;
If the uninitialized stack garbage causes block_size to evaluate to 0, this
will trigger a divide-by-zero panic on the subsequent modulo check:
if (rfb->base.offsets[plane] % block_size)
Or if it underestimates the BO size, could this lead to out-of-bounds VRAM
access during scanout?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260715105757.13125-1-timur.kristof@gmail.com?part=2
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] drm/amdgpu: Convert tiling flags to modifiers on GFX6-8
2026-07-15 11:10 ` sashiko-bot
@ 2026-07-15 20:33 ` Timur Kristóf
0 siblings, 0 replies; 13+ messages in thread
From: Timur Kristóf @ 2026-07-15 20:33 UTC (permalink / raw)
To: sashiko-reviews; +Cc: dri-devel
On 2026. július 15., szerda 13:10:51 közép-európai nyári idő sashiko-
bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider: - [High] Uninitialized variable `block_size_log2` in
> GFX6 modifier path causes undefined behavior and potential kernel panic. -
> [High] User-controlled `PIPE_CONFIG` in GFX6 modifier can trigger
> `unreachable()`, leading to kernel panic or undefined behavior. - [Medium]
> Unbounded `drm_err()` on invalid GFX6 array mode allows userspace to flood
> the kernel log. --
>
> --- Patch [2]: [PATCH 2/4] drm/amdgpu: Convert tiling flags to modifiers on
> GFX6-8 --- commit 642125971654c581668fe700356dfbee6905eccc
> Author: Timur Kristóf <timur.kristof@gmail.com>
> Subject: drm/amdgpu: Convert tiling flags to modifiers on GFX6-8
>
> This commit converts the tiling flags to modifiers on GFX8 and older,
> so that the DC display driver can rely on them like on newer generations.
>
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index
> > 950937b363fbe..dcca930ed3c04 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>
> [ ... ]
>
> > @@ -940,6 +940,55 @@ static int convert_tiling_flags_to_modifier(struct
> > amdgpu_framebuffer *afb)>
> > return 0;
> >
> > }
> >
> > +static int convert_tiling_flags_to_modifier_gfx6(struct
> > amdgpu_framebuffer *afb) +{
> > + const uint32_t array_mode = AMDGPU_TILING_GET(afb->tiling_flags,
> > ARRAY_MODE); + const uint32_t pipe_config =
> > AMDGPU_TILING_GET(afb->tiling_flags, PIPE_CONFIG);
> [ ... ]
>
> > + switch (array_mode) {
> > + case DC_ARRAY_LINEAR_GENERAL:
> > + case DC_ARRAY_LINEAR_ALLIGNED:
> > + modifier = DRM_FORMAT_MOD_LINEAR;
> > + break;
> > +
> > + case DC_ARRAY_2D_TILED_THIN1:
> > + /* Macro tiled modes only */
> > + modifier |=
> > + AMD_FMT_MOD_SET(PIPE_CONFIG, pipe_config) |
> > + AMD_FMT_MOD_SET(TILE_SPLIT, tile_split) |
> > + AMD_FMT_MOD_SET(BANK_WIDTH, bank_width) |
> > + AMD_FMT_MOD_SET(BANK_HEIGHT, bank_height) |
> > + AMD_FMT_MOD_SET(MACRO_TILE_ASPECT,
macro_tile_aspect) |
> > + AMD_FMT_MOD_SET(NUM_BANKS, num_banks);
> > + fallthrough;
> > +
> > + case DC_ARRAY_1D_TILED_THIN1:
> > + /* Micro and macro tiled modes */
> > + modifier |=
> > + AMD_FMT_MOD |
> > + AMD_FMT_MOD_SET(TILE_VERSION,
AMD_FMT_MOD_TILE_VER_GFX6) |
> > + AMD_FMT_MOD_SET(TILE, array_mode) |
> > + AMD_FMT_MOD_SET(MICROTILE, micro_tile_mode);
> > + break;
> > +
> > + default:
> > + drm_err(&adev->ddev, "array mode 0x%x not supported by
DCE\n",
> > array_mode);
> [Severity: Medium]
> Could this drm_err() be triggered repeatedly by a user space process
> providing an invalid array_mode to DRM_IOCTL_MODE_ADDFB2, flooding the
> kernel log?
>
> Should this use a rate-limited print or drm_dbg_kms() instead to prevent log
> spam from unprivileged users?
I will just change this to drm_dbg_kms()
>
> > + return -EINVAL;
> > + }
> > +
> > + afb->base.modifier = modifier;
> > + afb->base.flags |= DRM_MODE_FB_MODIFIERS;
> > + return 0;
> > +}
>
> [ ... ]
>
> > @@ -1093,7 +1142,7 @@ static int amdgpu_display_verify_sizes(struct
> > amdgpu_framebuffer *rfb)>
> > get_block_dimensions(block_size_log2,
format_info->cpp[i],
> >
> > &block_width,
&block_height);
> >
> > - } else {
> > + } else if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) >=
> > AMD_FMT_MOD_TILE_VER_GFX9) {>
> > int swizzle = AMD_FMT_MOD_GET(TILE,
modifier);
> >
> > switch ((swizzle & ~3) + 1) {
> >
> > @@ -1120,6 +1169,60 @@ static int amdgpu_display_verify_sizes(struct
> > amdgpu_framebuffer *rfb)>
> > get_block_dimensions(block_size_log2,
format_info->cpp[i],
> >
> > &block_width,
&block_height);
> >
> > + } else if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) ==
> > AMD_FMT_MOD_TILE_VER_GFX6) { + const u32
display_micro_tile_pitch = 32;
> > /* required by DCE */ + const u32
micro_tile_width = 8;
> > + const u32 micro_tile_height = 8;
> > + const u32 micro_tile_mode =
AMD_FMT_MOD_GET(MICROTILE, modifier);
> > + const u32 array_mode = AMD_FMT_MOD_GET(TILE,
modifier);
> > + u32 num_banks, bank_width, bank_height,
pipe_config,
> > macro_tile_aspect;
> > + u32 num_pipes;
> > +
> > + if (AMD_FMT_MOD_GET(DCC, modifier)) {
> > + drm_dbg_kms(rfb->base.dev, "DCC
is not displayable on GFX6-8\n");
> > + return -EINVAL;
> > + }
> > + if (array_mode !=
AMD_FMT_MOD_TILE_GFX6_1D_TILED_THIN1 &&
> > + array_mode !=
AMD_FMT_MOD_TILE_GFX6_2D_TILED_THIN1) {
> > + drm_dbg_kms(rfb->base.dev,
> > + "Array mode %u is not
supported by the display driver\n",
> > + array_mode);
> > + return -EINVAL;
> > + }
> > + if (micro_tile_mode !=
AMD_FMT_MOD_MICROTILE_DISPLAY) {
> > + drm_dbg_kms(rfb->base.dev,
> > + "Micro tile mode %u is
not displayable on GFX6-8\n",
> > + micro_tile_mode);
> > + return -EINVAL;
> > + }
> > +
> > + num_banks = 2 << AMD_FMT_MOD_GET(NUM_BANKS,
modifier);
> > + bank_width = 1 <<
AMD_FMT_MOD_GET(BANK_WIDTH, modifier);
> > + bank_height = 1 <<
AMD_FMT_MOD_GET(BANK_HEIGHT, modifier);
> > + pipe_config = AMD_FMT_MOD_GET(PIPE_CONFIG,
modifier);
> > + macro_tile_aspect = 1 <<
AMD_FMT_MOD_GET(MACRO_TILE_ASPECT, modifier);
> > +
> > + if (pipe_config >=
AMD_FMT_MOD_PIPE_CONFIG_P16_32x32_8x16)
> > + num_pipes = 16;
> > + else if (pipe_config >=
AMD_FMT_MOD_PIPE_CONFIG_P8_16x16_8x16)
> > + num_pipes = 8;
> > + else if (pipe_config >=
AMD_FMT_MOD_PIPE_CONFIG_P4_8x16)
> > + num_pipes = 4;
> > + else if (pipe_config ==
AMD_FMT_MOD_PIPE_CONFIG_P2)
> > + num_pipes = 2;
> > + else
> > + unreachable();
>
> [Severity: High]
> Since pipe_config comes from AMD_FMT_MOD_GET(PIPE_CONFIG, modifier), which
> is user-controlled via the framebuffer modifier, could a user pass a value
> smaller than AMD_FMT_MOD_PIPE_CONFIG_P2?
>
> If so, hitting unreachable() here would result in undefined compiler
> behavior. With CONFIG_OBJTOOL enabled, it can emit an invalid instruction
> that immediately panics the kernel. If execution falls through, num_pipes
> remains uninitialized and is subsequently used in a multiplication.
I will replace unreachable() with "return -EINVAL"
>
> > +
> > + if (array_mode <
AMD_FMT_MOD_TILE_GFX6_2D_TILED_THIN1) {
> > + block_width =
display_micro_tile_pitch;
> > + block_height = micro_tile_height;
> > + } else {
> > + /* Assume non-PRT macro tiling
modes */
> > + block_width = num_pipes *
micro_tile_width *
> > + bank_width *
macro_tile_aspect;
> > + block_height = micro_tile_height
* bank_height *
> > + num_banks /
macro_tile_aspect;
> > + }
> >
> > }
> >
> > ret = amdgpu_display_verify_plane(rfb, i, format_info,
> >
> >
block_width, block_height, block_size_log2);
>
> [Severity: High]
> Does this pass an uninitialized block_size_log2 into
> amdgpu_display_verify_plane()?
I will fix that, thanks for noticing.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] drm/amd/display: Support DRM format modifiers on GFX6-8
2026-07-15 10:57 [PATCH 0/4] drm/amdgpu: Support DRM format modifiers on GFX6-8 (v2) Timur Kristóf
2026-07-15 10:57 ` [PATCH 1/4] drm/fourcc: Add modifiers for AMD GFX6-8 Timur Kristóf
2026-07-15 10:57 ` [PATCH 2/4] drm/amdgpu: Convert tiling flags to modifiers on GFX6-8 Timur Kristóf
@ 2026-07-15 10:57 ` Timur Kristóf
2026-07-15 11:08 ` sashiko-bot
2026-07-15 10:57 ` [PATCH 4/4] drm/amd/display: Don't use tiling flags anymore Timur Kristóf
3 siblings, 1 reply; 13+ messages in thread
From: Timur Kristóf @ 2026-07-15 10:57 UTC (permalink / raw)
To: amd-gfx, dri-devel, Alex Deucher, christian.koenig,
Tvrtko Ursulin, pierre-eric.pelloux-prayer, Natalie Vock, Dieter
Cc: Timur Kristóf, Link Mauve, Nikola Medić,
Marek Olšák, Daniel Stone
Expose displayable DRM format modifiers based on which
tiling modes are supported by the GFX block.
Technically, DCE (Display Controller Engine) could
support all possible permutations of all parameters
independently of what GFX supports, with the limitation
that it can only display LINEAR images and the
displayable micro tiling mode (MICROTILE == DISPLAY).
It doesn't make sense to expose all possible permutations
of macro tiling modes, so let's just expose what the GFX
block of the current chip supports.
The following modes will be advertised:
- 2D_TILED_THIN1 + DISPLAY + macrotile params [1]
- 1D_TILED_THIN1 + DISPLAY
- LINEAR
[1] The macro tiling parameters depend on how many
bits per pixel of the specific surface has and
how the chip is configured. There is only one set
of valid macrotile params for a given surface.
Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
Tested-by: Link Mauve <linkmauve@linkmauve.fr>
Tested-by: Nikola Medić <nmedic89@gmail.com>
Reviewed-by: Marek Olšák <maraeo@gmail.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
.../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 316 +++++++++++++++++-
1 file changed, 309 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index b2507c5de7c8..9d56ce635cbd 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -180,6 +180,19 @@ STATIC_IFN_KUNIT void amdgpu_dm_plane_add_modifier(uint64_t **mods, uint64_t *si
}
EXPORT_IF_KUNIT(amdgpu_dm_plane_add_modifier);
+STATIC_IFN_KUNIT void amdgpu_dm_plane_add_modifier_dedup(uint64_t **mods, uint64_t *size,
+ uint64_t *cap, uint64_t mod)
+{
+ uint64_t i;
+
+ for (i = 0; i < *size; ++i)
+ if ((*mods)[i] == mod)
+ return;
+
+ amdgpu_dm_plane_add_modifier(mods, size, cap, mod);
+}
+EXPORT_IF_KUNIT(amdgpu_dm_plane_add_modifier_dedup);
+
STATIC_IFN_KUNIT bool amdgpu_dm_plane_modifier_has_dcc(uint64_t modifier)
{
return IS_AMD_FMT_MOD(modifier) && AMD_FMT_MOD_GET(DCC, modifier);
@@ -228,6 +241,41 @@ STATIC_IFN_KUNIT void amdgpu_dm_plane_fill_gfx8_tiling_info_from_flags(struct dc
}
EXPORT_IF_KUNIT(amdgpu_dm_plane_fill_gfx8_tiling_info_from_flags);
+STATIC_IFN_KUNIT int amdgpu_dm_plane_fill_gfx6_tiling_info_from_modifier(
+ struct dc_tiling_info *tiling_info,
+ uint64_t modifier)
+{
+ if (modifier == DRM_FORMAT_MOD_LINEAR) {
+ tiling_info->gfx8.array_mode = DC_ARRAY_LINEAR_GENERAL;
+ tiling_info->gfxversion = DcGfxVersion8;
+
+ return 0;
+ }
+
+ if (!IS_AMD_FMT_MOD(modifier))
+ return -EINVAL;
+
+ if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) != AMD_FMT_MOD_TILE_VER_GFX6)
+ return -EINVAL;
+
+ tiling_info->gfx8.array_mode = AMD_FMT_MOD_GET(TILE, modifier);
+ tiling_info->gfxversion = DcGfxVersion8;
+ tiling_info->gfx8.tile_mode = AMD_FMT_MOD_GET(MICROTILE, modifier);
+
+ if (AMD_FMT_MOD_GET(TILE, modifier) < AMD_FMT_MOD_TILE_GFX6_2D_TILED_THIN1)
+ return 0;
+
+ tiling_info->gfx8.pipe_config = AMD_FMT_MOD_GET(PIPE_CONFIG, modifier);
+ tiling_info->gfx8.tile_split = AMD_FMT_MOD_GET(TILE_SPLIT, modifier);
+ tiling_info->gfx8.bank_width = AMD_FMT_MOD_GET(BANK_WIDTH, modifier);
+ tiling_info->gfx8.bank_height = AMD_FMT_MOD_GET(BANK_HEIGHT, modifier);
+ tiling_info->gfx8.tile_aspect = AMD_FMT_MOD_GET(MACRO_TILE_ASPECT, modifier);
+ tiling_info->gfx8.num_banks = AMD_FMT_MOD_GET(NUM_BANKS, modifier);
+
+ return 0;
+}
+EXPORT_IF_KUNIT(amdgpu_dm_plane_fill_gfx6_tiling_info_from_modifier);
+
STATIC_IFN_KUNIT void amdgpu_dm_plane_fill_gfx9_tiling_info_from_device(const struct amdgpu_device *adev,
struct dc_tiling_info *tiling_info)
{
@@ -468,6 +516,246 @@ static void amdgpu_dm_plane_add_gfx10_1_modifiers(const struct amdgpu_device *ad
AMD_FMT_MOD_SET(TILE_VERSION, AMD_FMT_MOD_TILE_VER_GFX9));
}
+/**
+ * amdgpu_dm_plane_get_gfx6_tile_idx() - Get tile mode index on GFX6-8
+ *
+ * @adev: amdgpu_device pointer
+ * @bpp: bits per pixel in the image
+ * @arr: array mode (aka. tile mode) of the image layout
+ *
+ * Select which tiling mode from the table is suitable for the given bits per pixel
+ * using the given array mode, assuming the displayable micro tile mode is used.
+ *
+ * On GFX6-8, the GFX block can only use pre-programmed tiling modes from GB_TILE_MODEn
+ * registers which are programmed by the kernel according to the tiling mode table.
+ * Note that the tiling table is uAPI and userspace relies on specific modes being
+ * present at specific indices.
+ * See SiLib::HwlSetupTileInfo() and CiLib::HwlSetupTileInfo() in addrlib.
+ */
+static u32 amdgpu_dm_plane_get_gfx6_tile_idx(const struct amdgpu_device *adev,
+ const u32 bpp,
+ const enum array_mode_values arr)
+{
+ /* Assume that the microtile mode is DISPLAY. */
+
+ if (arr == DC_ARRAY_1D_TILED_THIN1)
+ return 9;
+
+ ASSERT(arr == DC_ARRAY_2D_TILED_THIN1);
+
+ if (adev->family > AMDGPU_FAMILY_SI)
+ return 10;
+
+ switch (bpp) {
+ case 8:
+ return 10;
+ case 16:
+ return 11;
+ default:
+ return 12;
+ }
+}
+
+/**
+ * amdgpu_dm_plane_calc_gfx7_tile_split() - Calculate tile split on GFX7-8
+ *
+ * @adev: amdgpu_device pointer
+ * @bpp: bits per pixel in the image
+ * @gb_tile_mode: GB_TILE_MODEn register value for the current tiling mode
+ *
+ * Calculate the actual tile split value on GFX7-8, assuming 2D_TILED_THIN1 array mode
+ * with a non-depth micro tile mode.
+ *
+ * On GFX7-8, SAMPLE_SPLIT holds a factor from which the actual tile split bytes
+ * can be calculated. The TILE_SPLIT field is only used for the depth micro tile mode.
+ * See CiLib::HwlComputeMacroModeIndex() in addrlib.
+ */
+static u32 amdgpu_dm_plane_calc_gfx7_tile_split(const struct amdgpu_device *adev,
+ const u32 bpp,
+ const u32 gb_tile_mode)
+{
+ /* Assume 2D_TILED_THIN1 mode with non-DEPTH microtiles */
+ const u32 sample_split = (gb_tile_mode >> 25) & 0x3;
+ const u32 thickness = 1;
+ const u32 tile_size_pixels = 8 * 8;
+ const u32 tile_bytes_1x = tile_size_pixels * bpp * thickness / 8;
+ const u32 sample_split_factor = 1 << sample_split;
+
+ return clamp(tile_bytes_1x * sample_split_factor,
+ 256,
+ adev->gfx.config.mem_row_size_in_kb * 1024);
+}
+
+/**
+ * amdgpu_dm_plane_get_gfx7_macro_tile_idx() - Get macro tile mode index on GFX7-8
+ *
+ * @bpp: bits per pixel in the image
+ * @tile_split_bytes: actual tile split bytes, see amdgpu_dm_plane_calc_gfx7_tile_split()
+ *
+ * Select which macro tiling mode from the table is suitable for the given bits per pixel,
+ * assuming 2D_TILED_THIN1 array mode and DISPLAY micro tile mode (and no multisampling).
+ * Note that the tiling table is uAPI and userspace relies on specific modes being
+ * present at specific indices.
+ * See CiLib::HwlComputeMacroModeIndex() in addrlib.
+ */
+static u32 amdgpu_dm_plane_get_gfx7_macro_tile_idx(const u32 bpp, const u32 tile_split_bytes)
+{
+ const u32 tile_bytes = clamp(8 * 8 * bpp, 64, tile_split_bytes);
+ const u32 macro_tile_idx = ilog2(tile_bytes / 64);
+
+ WARN_ON(macro_tile_idx >= 16);
+
+ return macro_tile_idx;
+}
+
+/**
+ * amdgpu_dm_plane_calc_gfx6_mod() - Calculate a DRM format modifier for GFX6-8
+ *
+ * @adev: amdgpu_device pointer
+ * @bpp: bits per pixel in the image
+ * @arr: array mode (aka. tile mode) of the image layout
+ *
+ * Select suitable micro and macro tile modes for the given bits per pixel,
+ * and calculate the corresponding DRM format modifier.
+ */
+static u64 amdgpu_dm_plane_calc_gfx6_mod(const struct amdgpu_device *adev,
+ const u32 bpp,
+ const enum array_mode_values arr)
+{
+ u32 array_mode, micro_tile_mode, tile_split_bytes;
+ u32 gb_macrotile_mode, macrotile_idx;
+ u32 gb_tile_mode, tile_idx;
+
+ u64 modifier_base =
+ AMD_FMT_MOD |
+ AMD_FMT_MOD_SET(TILE_VERSION, AMD_FMT_MOD_TILE_VER_GFX6) |
+ AMD_FMT_MOD_SET(TILE, arr) |
+ AMD_FMT_MOD_SET(MICROTILE, AMD_FMT_MOD_MICROTILE_DISPLAY);
+
+ if (arr < DC_ARRAY_2D_TILED_THIN1)
+ return modifier_base;
+
+ tile_idx = amdgpu_dm_plane_get_gfx6_tile_idx(adev, bpp, arr);
+ gb_tile_mode = adev->gfx.config.tile_mode_array[tile_idx];
+ array_mode = (gb_tile_mode >> 2) & 0xf;
+
+ if (adev->family == AMDGPU_FAMILY_SI) {
+ micro_tile_mode = (gb_tile_mode >> 0) & 0x3;
+ tile_split_bytes = 64 << ((gb_tile_mode >> 11) & 0x7);
+ } else {
+ micro_tile_mode = (gb_tile_mode >> 22) & 0x7;
+ tile_split_bytes = amdgpu_dm_plane_calc_gfx7_tile_split(adev, bpp, gb_tile_mode);
+ }
+
+ ASSERT(array_mode == arr);
+ ASSERT(micro_tile_mode == AMD_FMT_MOD_MICROTILE_DISPLAY);
+
+ modifier_base |=
+ AMD_FMT_MOD_SET(PIPE_CONFIG, (gb_tile_mode >> 6) & 0x1f) |
+ AMD_FMT_MOD_SET(TILE_SPLIT, ilog2(tile_split_bytes / 64));
+
+ if (adev->family == AMDGPU_FAMILY_SI)
+ return modifier_base |
+ AMD_FMT_MOD_SET(BANK_WIDTH, (gb_tile_mode >> 14) & 0x3) |
+ AMD_FMT_MOD_SET(BANK_HEIGHT, (gb_tile_mode >> 16) & 0x3) |
+ AMD_FMT_MOD_SET(MACRO_TILE_ASPECT, (gb_tile_mode >> 18) & 0x3) |
+ AMD_FMT_MOD_SET(NUM_BANKS, (gb_tile_mode >> 20) & 0x3);
+
+ macrotile_idx = amdgpu_dm_plane_get_gfx7_macro_tile_idx(bpp, tile_split_bytes);
+ gb_macrotile_mode = adev->gfx.config.macrotile_mode_array[macrotile_idx];
+
+ return modifier_base |
+ AMD_FMT_MOD_SET(BANK_WIDTH, (gb_macrotile_mode >> 0) & 0x3) |
+ AMD_FMT_MOD_SET(BANK_HEIGHT, (gb_macrotile_mode >> 2) & 0x3) |
+ AMD_FMT_MOD_SET(MACRO_TILE_ASPECT, (gb_macrotile_mode >> 4) & 0x3) |
+ AMD_FMT_MOD_SET(NUM_BANKS, (gb_macrotile_mode >> 6) & 0x3);
+}
+
+/**
+ * amdgpu_dm_plane_gfx6_format_mod_supported() - Check if a modifier is supported on GFX6-8
+ *
+ * @adev: amdgpu_device pointer
+ * @bpp: bits per pixel in the image
+ * @modifier: the modifier whose support we check
+ *
+ * On GFX6-8, not all DRM format modifier can be used with all image formats.
+ * Check whether the specified modifier is supported with the given bits per pixel value.
+ */
+static bool amdgpu_dm_plane_gfx6_format_mod_supported(const struct amdgpu_device *adev,
+ const u32 bpp,
+ const u64 modifier)
+{
+ const u32 array_mode = AMD_FMT_MOD_GET(TILE, modifier);
+ const u32 micro_tile_mode = AMD_FMT_MOD_GET(MICROTILE, modifier);
+
+ if (!IS_AMD_FMT_MOD(modifier))
+ return false;
+
+ /* GFX9 and newer format modifiers are not supported on GFX6-8 yet. */
+ if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) != AMD_FMT_MOD_TILE_VER_GFX6)
+ return false;
+
+ /* GFX8 and older format modifiers are not supported on GFX9+ yet. */
+ if (adev->family >= AMDGPU_FAMILY_AI)
+ return false;
+
+ /* GFX6-7 doesn't have any DCC, GFX8 doesn't have displayable DCC. */
+ if (AMD_FMT_MOD_GET(DCC, modifier))
+ return false;
+
+ /*
+ * For now, only expose 1D and 2D tiles THIN array modes.
+ * Linear is already exposed through DRM_FORMAT_MOD_LINEAR.
+ */
+ if (array_mode != AMD_FMT_MOD_TILE_GFX6_1D_TILED_THIN1 &&
+ array_mode != AMD_FMT_MOD_TILE_GFX6_2D_TILED_THIN1)
+ return false;
+
+ /*
+ * For now, only expose DISPLAY micro tile mode.
+ * THIN, DEPTH and THICK modes are not displayable.
+ * ROTATED has never been supported on Linux.
+ */
+ if (micro_tile_mode != AMD_FMT_MOD_MICROTILE_DISPLAY)
+ return false;
+
+ /* Verify that the modifier is the same that we'd expose for this bpp */
+ return amdgpu_dm_plane_calc_gfx6_mod(adev, bpp, array_mode) == modifier;
+}
+
+/**
+ * amdgpu_dm_plane_add_gfx6_modifiers() - Expose modifiers for GFX6-8
+ *
+ * @adev: amdgpu_device pointer
+ * @mods: Pointer to array of format modifiers
+ * @size: Pointer to size of the array
+ * @capacity: Pointer to capacity of the array
+ *
+ * Calculate a DRM format modifier for macro tiled modes for each supported
+ * bits per pixel value. Use de-duplication because on some GPUs it may happen
+ * that different bpp results in the exact same macro tiling mode, depending
+ * on the tiling table.
+ *
+ * Also expose a micro tiled only mode. This is less optimal, but supported.
+ */
+static void amdgpu_dm_plane_add_gfx6_modifiers(const struct amdgpu_device *adev,
+ u64 **mods,
+ u64 *size,
+ u64 *capacity)
+{
+ /* 2D tiled displayable */
+ amdgpu_dm_plane_add_modifier(mods, size, capacity,
+ amdgpu_dm_plane_calc_gfx6_mod(adev, 16, DC_ARRAY_2D_TILED_THIN1));
+ amdgpu_dm_plane_add_modifier_dedup(mods, size, capacity,
+ amdgpu_dm_plane_calc_gfx6_mod(adev, 32, DC_ARRAY_2D_TILED_THIN1));
+ amdgpu_dm_plane_add_modifier_dedup(mods, size, capacity,
+ amdgpu_dm_plane_calc_gfx6_mod(adev, 64, DC_ARRAY_2D_TILED_THIN1));
+
+ /* 1D tiled displayable */
+ amdgpu_dm_plane_add_modifier(mods, size, capacity,
+ amdgpu_dm_plane_calc_gfx6_mod(adev, 0, DC_ARRAY_1D_TILED_THIN1));
+}
+
static void amdgpu_dm_plane_add_gfx9_modifiers(const struct amdgpu_device *adev,
uint64_t **mods,
uint64_t *size,
@@ -749,12 +1037,6 @@ static void amdgpu_dm_plane_add_gfx12_modifiers(struct amdgpu_device *adev,
STATIC_IFN_KUNIT int amdgpu_dm_plane_get_plane_modifiers(struct amdgpu_device *adev, unsigned int plane_type, uint64_t **mods)
{
uint64_t size = 0, capacity = 128;
- *mods = NULL;
-
- /* We have not hooked up any pre-GFX9 modifiers. */
- if (adev->family < AMDGPU_FAMILY_AI)
- return 0;
-
*mods = kmalloc_array(capacity, sizeof(uint64_t), GFP_KERNEL);
if (plane_type == DRM_PLANE_TYPE_CURSOR) {
@@ -764,6 +1046,13 @@ STATIC_IFN_KUNIT int amdgpu_dm_plane_get_plane_modifiers(struct amdgpu_device *a
}
switch (adev->family) {
+ case AMDGPU_FAMILY_SI:
+ case AMDGPU_FAMILY_CI:
+ case AMDGPU_FAMILY_KV:
+ case AMDGPU_FAMILY_VI:
+ case AMDGPU_FAMILY_CZ:
+ amdgpu_dm_plane_add_gfx6_modifiers(adev, mods, &size, &capacity);
+ break;
case AMDGPU_FAMILY_AI:
case AMDGPU_FAMILY_RV:
amdgpu_dm_plane_add_gfx9_modifiers(adev, mods, &size, &capacity);
@@ -940,8 +1229,13 @@ int amdgpu_dm_plane_fill_plane_buffer_attributes(struct amdgpu_device *adev,
address);
if (ret)
return ret;
- } else {
+ } else if (afb->base.modifier == DRM_FORMAT_MOD_INVALID) {
amdgpu_dm_plane_fill_gfx8_tiling_info_from_flags(tiling_info, tiling_flags);
+ } else {
+ ret = amdgpu_dm_plane_fill_gfx6_tiling_info_from_modifier(tiling_info,
+ afb->base.modifier);
+ if (ret)
+ return ret;
}
return 0;
@@ -1587,6 +1881,7 @@ STATIC_IFN_KUNIT bool amdgpu_dm_plane_format_mod_supported(struct drm_plane *pla
{
struct amdgpu_device *adev = drm_to_adev(plane->dev);
const struct drm_format_info *info = drm_format_info(format);
+ const u32 bpp = drm_format_info_bpp(info, 0);
int i;
if (!info)
@@ -1610,6 +1905,13 @@ STATIC_IFN_KUNIT bool amdgpu_dm_plane_format_mod_supported(struct drm_plane *pla
if (i == plane->modifier_count)
return false;
+ if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) == AMD_FMT_MOD_TILE_VER_GFX6)
+ return amdgpu_dm_plane_gfx6_format_mod_supported(adev, bpp, modifier);
+
+ /* GFX9+ modifers are not supported on GFX8 and older yet. */
+ if (adev->family < AMDGPU_FAMILY_AI)
+ return false;
+
/* GFX12 doesn't have these limitations. */
if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) <= AMD_FMT_MOD_TILE_VER_GFX11) {
enum dm_micro_swizzle microtile = amdgpu_dm_plane_modifier_gfx9_swizzle_mode(modifier) & 3;
--
2.55.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] drm/amd/display: Support DRM format modifiers on GFX6-8
2026-07-15 10:57 ` [PATCH 3/4] drm/amd/display: Support DRM format " Timur Kristóf
@ 2026-07-15 11:08 ` sashiko-bot
2026-07-15 20:18 ` Timur Kristóf
0 siblings, 1 reply; 13+ messages in thread
From: sashiko-bot @ 2026-07-15 11:08 UTC (permalink / raw)
To: Timur Kristóf; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing bit-to-byte conversion in `tile_bytes` calculation.
- [High] NULL pointer dereference in `amdgpu_dm_plane_add_modifier_dedup`.
- [Medium] Missing 8-bpp modifier exposure in `amdgpu_dm_plane_add_gfx6_modifiers`.
--
--- Patch [3]: [PATCH 3/4] drm/amd/display: Support DRM format modifiers on GFX6-8 ---
commit 3fb28c857b70a3a3413b8818708816df4bf9fc7e
Author: Timur Kristóf <timur.kristof@gmail.com>
drm/amd/display: Support DRM format modifiers on GFX6-8
Expose displayable DRM format modifiers based on which
tiling modes are supported by the GFX block.
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index b2507c5de7c86..9d56ce635cbda 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
[ ... ]
> @@ -180,6 +180,19 @@ STATIC_IFN_KUNIT void amdgpu_dm_plane_add_modifier(uint64_t **mods, uint64_t *si
> }
> EXPORT_IF_KUNIT(amdgpu_dm_plane_add_modifier);
>
> +STATIC_IFN_KUNIT void amdgpu_dm_plane_add_modifier_dedup(uint64_t **mods, uint64_t *size,
> + uint64_t *cap, uint64_t mod)
> +{
> + uint64_t i;
> +
> + for (i = 0; i < *size; ++i)
> + if ((*mods)[i] == mod)
> + return;
[Severity: High]
If a previous call to amdgpu_dm_plane_add_modifier() fails to reallocate the
array, it sets *mods to NULL but leaves *size at its previous non-zero value:
amdgpu_dm_plane_add_modifier() {
...
if (!new_mods) {
kfree(*mods);
*mods = NULL;
return;
}
...
}
Will this loop cause a NULL pointer dereference on the next call to
amdgpu_dm_plane_add_modifier_dedup() since it unconditionally evaluates
(*mods)[i] without checking if *mods is NULL?
> +
> + amdgpu_dm_plane_add_modifier(mods, size, cap, mod);
> +}
> +EXPORT_IF_KUNIT(amdgpu_dm_plane_add_modifier_dedup);
[ ... ]
> @@ -468,6 +516,246 @@ static void amdgpu_dm_plane_add_gfx10_1_modifiers(const struct amdgpu_device *ad
[ ... ]
> +/**
> + * amdgpu_dm_plane_get_gfx7_macro_tile_idx() - Get macro tile mode index on GFX7-8
> + *
> + * @bpp: bits per pixel in the image
> + * @tile_split_bytes: actual tile split bytes, see amdgpu_dm_plane_calc_gfx7_tile_split()
> + *
> + * Select which macro tiling mode from the table is suitable for the given bits per pixel,
> + * assuming 2D_TILED_THIN1 array mode and DISPLAY micro tile mode (and no multisampling).
> + * Note that the tiling table is uAPI and userspace relies on specific modes being
> + * present at specific indices.
> + * See CiLib::HwlComputeMacroModeIndex() in addrlib.
> + */
> +static u32 amdgpu_dm_plane_get_gfx7_macro_tile_idx(const u32 bpp, const u32 tile_split_bytes)
> +{
> + const u32 tile_bytes = clamp(8 * 8 * bpp, 64, tile_split_bytes);
[Severity: High]
Does this calculation compute the microtile size in bits instead of bytes?
Because it lacks a division by 8 (e.g., (8 * 8 * bpp) / 8), the computed
tile_bytes is artificially inflated. For a 16bpp format, this would
calculate 1024 instead of 128, leading to an incorrect macro tile index
being selected which may cause tiled buffers to be rejected or misrendered
on GFX7-8.
> + const u32 macro_tile_idx = ilog2(tile_bytes / 64);
> +
> + WARN_ON(macro_tile_idx >= 16);
> +
> + return macro_tile_idx;
> +}
[ ... ]
> +/**
> + * amdgpu_dm_plane_add_gfx6_modifiers() - Expose modifiers for GFX6-8
> + *
> + * @adev: amdgpu_device pointer
> + * @mods: Pointer to array of format modifiers
> + * @size: Pointer to size of the array
> + * @capacity: Pointer to capacity of the array
> + *
> + * Calculate a DRM format modifier for macro tiled modes for each supported
> + * bits per pixel value. Use de-duplication because on some GPUs it may happen
> + * that different bpp results in the exact same macro tiling mode, depending
> + * on the tiling table.
> + *
> + * Also expose a micro tiled only mode. This is less optimal, but supported.
> + */
> +static void amdgpu_dm_plane_add_gfx6_modifiers(const struct amdgpu_device *adev,
> + u64 **mods,
> + u64 *size,
> + u64 *capacity)
> +{
> + /* 2D tiled displayable */
> + amdgpu_dm_plane_add_modifier(mods, size, capacity,
> + amdgpu_dm_plane_calc_gfx6_mod(adev, 16, DC_ARRAY_2D_TILED_THIN1));
[Severity: Medium]
Are we missing the 8-bpp modifier exposure here?
The helper function amdgpu_dm_plane_get_gfx6_tile_idx() explicitly supports
bpp == 8, but we only seem to expose 16, 32, and 64 bpp formats here.
Without this, userspace might not be able to negotiate 2D tiling for 8-bpp
formats.
> + amdgpu_dm_plane_add_modifier_dedup(mods, size, capacity,
> + amdgpu_dm_plane_calc_gfx6_mod(adev, 32, DC_ARRAY_2D_TILED_THIN1));
> + amdgpu_dm_plane_add_modifier_dedup(mods, size, capacity,
> + amdgpu_dm_plane_calc_gfx6_mod(adev, 64, DC_ARRAY_2D_TILED_THIN1));
> +
> + /* 1D tiled displayable */
> + amdgpu_dm_plane_add_modifier(mods, size, capacity,
> + amdgpu_dm_plane_calc_gfx6_mod(adev, 0, DC_ARRAY_1D_TILED_THIN1));
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260715105757.13125-1-timur.kristof@gmail.com?part=3
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] drm/amd/display: Support DRM format modifiers on GFX6-8
2026-07-15 11:08 ` sashiko-bot
@ 2026-07-15 20:18 ` Timur Kristóf
0 siblings, 0 replies; 13+ messages in thread
From: Timur Kristóf @ 2026-07-15 20:18 UTC (permalink / raw)
To: sashiko-reviews; +Cc: dri-devel
On 2026. július 15., szerda 13:08:20 közép-európai nyári idő sashiko-
bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider: - [High] Missing bit-to-byte conversion in
> `tile_bytes` calculation. - [High] NULL pointer dereference in
> `amdgpu_dm_plane_add_modifier_dedup`. - [Medium] Missing 8-bpp modifier
> exposure in `amdgpu_dm_plane_add_gfx6_modifiers`. --
>
> --- Patch [3]: [PATCH 3/4] drm/amd/display: Support DRM format modifiers on
> GFX6-8 --- commit 3fb28c857b70a3a3413b8818708816df4bf9fc7e
> Author: Timur Kristóf <timur.kristof@gmail.com>
>
> drm/amd/display: Support DRM format modifiers on GFX6-8
>
> Expose displayable DRM format modifiers based on which
> tiling modes are supported by the GFX block.
>
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c index
> > b2507c5de7c86..9d56ce635cbda 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
>
> [ ... ]
>
> > @@ -180,6 +180,19 @@ STATIC_IFN_KUNIT void
> > amdgpu_dm_plane_add_modifier(uint64_t **mods, uint64_t *si>
> > }
> > EXPORT_IF_KUNIT(amdgpu_dm_plane_add_modifier);
> >
> > +STATIC_IFN_KUNIT void amdgpu_dm_plane_add_modifier_dedup(uint64_t **mods,
> > uint64_t *size, +
uint64_t *cap, uint64_t mod)
> > +{
> > + uint64_t i;
> > +
> > + for (i = 0; i < *size; ++i)
> > + if ((*mods)[i] == mod)
> > + return;
>
> [Severity: High]
> If a previous call to amdgpu_dm_plane_add_modifier() fails to reallocate the
> array, it sets *mods to NULL but leaves *size at its previous non-zero
> value:
>
> amdgpu_dm_plane_add_modifier() {
> ...
> if (!new_mods) {
> kfree(*mods);
> *mods = NULL;
> return;
> }
> ...
> }
>
> Will this loop cause a NULL pointer dereference on the next call to
> amdgpu_dm_plane_add_modifier_dedup() since it unconditionally evaluates
> (*mods)[i] without checking if *mods is NULL?
This can't happen in practice. amdgpu_dm_plane_get_plane_modifiers() allocates
enough space for 128 modifiers but amdgpu_dm_plane_add_gfx6_modifiers() only
adds up to 4 modifiers to the array.
That being said, let me know what you recommend to do with this.
>
> > +
> > + amdgpu_dm_plane_add_modifier(mods, size, cap, mod);
> > +}
> > +EXPORT_IF_KUNIT(amdgpu_dm_plane_add_modifier_dedup);
>
> [ ... ]
>
> > @@ -468,6 +516,246 @@ static void
> > amdgpu_dm_plane_add_gfx10_1_modifiers(const struct amdgpu_device *ad
> [ ... ]
>
> > +/**
> > + * amdgpu_dm_plane_get_gfx7_macro_tile_idx() - Get macro tile mode index
> > on GFX7-8 + *
> > + * @bpp: bits per pixel in the image
> > + * @tile_split_bytes: actual tile split bytes, see
> > amdgpu_dm_plane_calc_gfx7_tile_split() + *
> > + * Select which macro tiling mode from the table is suitable for the
> > given bits per pixel, + * assuming 2D_TILED_THIN1 array mode and DISPLAY
> > micro tile mode (and no multisampling). + * Note that the tiling table is
> > uAPI and userspace relies on specific modes being + * present at specific
> > indices.
> > + * See CiLib::HwlComputeMacroModeIndex() in addrlib.
> > + */
> > +static u32 amdgpu_dm_plane_get_gfx7_macro_tile_idx(const u32 bpp, const
> > u32 tile_split_bytes) +{
> > + const u32 tile_bytes = clamp(8 * 8 * bpp, 64, tile_split_bytes);
>
> [Severity: High]
> Does this calculation compute the microtile size in bits instead of bytes?
> Because it lacks a division by 8 (e.g., (8 * 8 * bpp) / 8), the computed
> tile_bytes is artificially inflated. For a 16bpp format, this would
> calculate 1024 instead of 128, leading to an incorrect macro tile index
> being selected which may cause tiled buffers to be rejected or misrendered
> on GFX7-8.
This may be a real issue. I will look into this.
>
> > + const u32 macro_tile_idx = ilog2(tile_bytes / 64);
> > +
> > + WARN_ON(macro_tile_idx >= 16);
> > +
> > + return macro_tile_idx;
> > +}
>
> [ ... ]
>
> > +/**
> > + * amdgpu_dm_plane_add_gfx6_modifiers() - Expose modifiers for GFX6-8
> > + *
> > + * @adev: amdgpu_device pointer
> > + * @mods: Pointer to array of format modifiers
> > + * @size: Pointer to size of the array
> > + * @capacity: Pointer to capacity of the array
> > + *
> > + * Calculate a DRM format modifier for macro tiled modes for each
> > supported + * bits per pixel value. Use de-duplication because on some
> > GPUs it may happen + * that different bpp results in the exact same macro
> > tiling mode, depending + * on the tiling table.
> > + *
> > + * Also expose a micro tiled only mode. This is less optimal, but
> > supported. + */
> > +static void amdgpu_dm_plane_add_gfx6_modifiers(const struct amdgpu_device
> > *adev, + u64 **mods,
> > + u64 *size,
> > + u64 *capacity)
> > +{
> > + /* 2D tiled displayable */
> > + amdgpu_dm_plane_add_modifier(mods, size, capacity,
> > + amdgpu_dm_plane_calc_gfx6_mod(adev, 16,
DC_ARRAY_2D_TILED_THIN1));
>
> [Severity: Medium]
> Are we missing the 8-bpp modifier exposure here?
>
> The helper function amdgpu_dm_plane_get_gfx6_tile_idx() explicitly supports
> bpp == 8, but we only seem to expose 16, 32, and 64 bpp formats here.
> Without this, userspace might not be able to negotiate 2D tiling for 8-bpp
> formats.
The macro tile mode array has macro tile modes for 8 bpp, but DC doesn't
expose any formats with 8 bpp. So we don't need to expose modifiers for 8 bpp
either.
>
> > + amdgpu_dm_plane_add_modifier_dedup(mods, size, capacity,
> > + amdgpu_dm_plane_calc_gfx6_mod(adev, 32,
DC_ARRAY_2D_TILED_THIN1));
> > + amdgpu_dm_plane_add_modifier_dedup(mods, size, capacity,
> > + amdgpu_dm_plane_calc_gfx6_mod(adev, 64,
DC_ARRAY_2D_TILED_THIN1));
> > +
> > + /* 1D tiled displayable */
> > + amdgpu_dm_plane_add_modifier(mods, size, capacity,
> > + amdgpu_dm_plane_calc_gfx6_mod(adev, 0,
DC_ARRAY_1D_TILED_THIN1));
> > +}
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] drm/amd/display: Don't use tiling flags anymore
2026-07-15 10:57 [PATCH 0/4] drm/amdgpu: Support DRM format modifiers on GFX6-8 (v2) Timur Kristóf
` (2 preceding siblings ...)
2026-07-15 10:57 ` [PATCH 3/4] drm/amd/display: Support DRM format " Timur Kristóf
@ 2026-07-15 10:57 ` Timur Kristóf
2026-07-15 11:42 ` sashiko-bot
3 siblings, 1 reply; 13+ messages in thread
From: Timur Kristóf @ 2026-07-15 10:57 UTC (permalink / raw)
To: amd-gfx, dri-devel, Alex Deucher, christian.koenig,
Tvrtko Ursulin, pierre-eric.pelloux-prayer, Natalie Vock, Dieter
Cc: Timur Kristóf, Link Mauve, Nikola Medić,
Marek Olšák, Daniel Stone
All supported GPU generations now support DRM format modifiers.
Remove all code from amdgpu_dm that dealt with tiling flags.
Note that the legacy non-DC display code still relies on
tiling flags, so we can't remove them outside of DC
until we also remove the legacy display code.
Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
Tested-by: Link Mauve <linkmauve@linkmauve.fr>
Tested-by: Nikola Medić <nmedic89@gmail.com>
Reviewed-by: Marek Olšák <maraeo@gmail.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 29 +------
.../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 37 --------
.../amd/display/amdgpu_dm/amdgpu_dm_plane.h | 3 -
.../amdgpu_dm/tests/amdgpu_dm_plane_test.c | 84 -------------------
4 files changed, 2 insertions(+), 151 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c643cc542857..2f7388c47d02 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3057,7 +3057,6 @@ EXPORT_IF_KUNIT(fill_plane_color_attributes);
static int
fill_dc_plane_info_and_addr(struct amdgpu_device *adev,
const struct drm_plane_state *plane_state,
- const u64 tiling_flags,
struct dc_plane_info *plane_info,
struct dc_plane_address *address,
bool tmz_surface)
@@ -3155,7 +3154,7 @@ fill_dc_plane_info_and_addr(struct amdgpu_device *adev,
return ret;
ret = amdgpu_dm_plane_fill_plane_buffer_attributes(adev, afb, plane_info->format,
- plane_info->rotation, tiling_flags,
+ plane_info->rotation,
&plane_info->tiling_info,
&plane_info->plane_size,
&plane_info->dcc, address,
@@ -3191,7 +3190,6 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev,
dc_plane_state->scaling_quality = scaling_info.scaling_quality;
ret = fill_dc_plane_info_and_addr(adev, plane_state,
- afb->tiling_flags,
&plane_info,
&dc_plane_state->address,
afb->tmz_surface);
@@ -4279,7 +4277,6 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
fill_dc_plane_info_and_addr(
dm->adev, new_plane_state,
- afb->tiling_flags,
&bundle->plane_infos[planes_count],
&bundle->flip_addrs[planes_count].address,
afb->tmz_surface);
@@ -6248,8 +6245,7 @@ static bool should_reset_plane(struct drm_atomic_state *state,
new_afb = (struct amdgpu_framebuffer *)new_other_state->fb;
/* Tiling and DCC changes also require bandwidth updates. */
- if (old_afb->tiling_flags != new_afb->tiling_flags ||
- old_afb->base.modifier != new_afb->base.modifier)
+ if (old_afb->base.modifier != new_afb->base.modifier)
return true;
}
@@ -6261,9 +6257,7 @@ static int dm_check_cursor_fb(struct amdgpu_crtc *new_acrtc,
struct drm_framebuffer *fb)
{
struct amdgpu_device *adev = drm_to_adev(new_acrtc->base.dev);
- struct amdgpu_framebuffer *afb = to_amdgpu_framebuffer(fb);
unsigned int pitch;
- bool linear;
if (fb->width > new_acrtc->max_cursor_width ||
fb->height > new_acrtc->max_cursor_height) {
@@ -6298,25 +6292,6 @@ static int dm_check_cursor_fb(struct amdgpu_crtc *new_acrtc,
return -EINVAL;
}
- /* Core DRM takes care of checking FB modifiers, so we only need to
- * check tiling flags when the FB doesn't have a modifier.
- */
- if (!(fb->flags & DRM_MODE_FB_MODIFIERS)) {
- if (adev->family == AMDGPU_FAMILY_GC_12_0_0) {
- linear = AMDGPU_TILING_GET(afb->tiling_flags, GFX12_SWIZZLE_MODE) == 0;
- } else if (adev->family >= AMDGPU_FAMILY_AI) {
- linear = AMDGPU_TILING_GET(afb->tiling_flags, SWIZZLE_MODE) == 0;
- } else {
- linear = AMDGPU_TILING_GET(afb->tiling_flags, ARRAY_MODE) != DC_ARRAY_2D_TILED_THIN1 &&
- AMDGPU_TILING_GET(afb->tiling_flags, ARRAY_MODE) != DC_ARRAY_1D_TILED_THIN1 &&
- AMDGPU_TILING_GET(afb->tiling_flags, MICRO_TILE_MODE) == 0;
- }
- if (!linear) {
- drm_dbg_atomic(adev_to_drm(adev), "Cursor FB not linear");
- return -EINVAL;
- }
- }
-
return 0;
}
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 9d56ce635cbd..d066947f59e1 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -208,39 +208,6 @@ STATIC_IFN_KUNIT unsigned int amdgpu_dm_plane_modifier_gfx9_swizzle_mode(uint64_
}
EXPORT_IF_KUNIT(amdgpu_dm_plane_modifier_gfx9_swizzle_mode);
-STATIC_IFN_KUNIT void amdgpu_dm_plane_fill_gfx8_tiling_info_from_flags(struct dc_tiling_info *tiling_info,
- uint64_t tiling_flags)
-{
- /* Fill GFX8 params */
- if (AMDGPU_TILING_GET(tiling_flags, ARRAY_MODE) == DC_ARRAY_2D_TILED_THIN1) {
- unsigned int bankw, bankh, mtaspect, tile_split, num_banks;
- bankw = AMDGPU_TILING_GET(tiling_flags, BANK_WIDTH);
- bankh = AMDGPU_TILING_GET(tiling_flags, BANK_HEIGHT);
- mtaspect = AMDGPU_TILING_GET(tiling_flags, MACRO_TILE_ASPECT);
- tile_split = AMDGPU_TILING_GET(tiling_flags, TILE_SPLIT);
- num_banks = AMDGPU_TILING_GET(tiling_flags, NUM_BANKS);
-
- tiling_info->gfxversion = DcGfxVersion8;
- /* XXX fix me for VI */
- tiling_info->gfx8.num_banks = num_banks;
- tiling_info->gfx8.array_mode =
- DC_ARRAY_2D_TILED_THIN1;
- tiling_info->gfx8.tile_split = tile_split;
- tiling_info->gfx8.bank_width = bankw;
- tiling_info->gfx8.bank_height = bankh;
- tiling_info->gfx8.tile_aspect = mtaspect;
- tiling_info->gfx8.tile_mode =
- DC_ADDR_SURF_MICRO_TILING_DISPLAY;
- } else if (AMDGPU_TILING_GET(tiling_flags, ARRAY_MODE)
- == DC_ARRAY_1D_TILED_THIN1) {
- tiling_info->gfx8.array_mode = DC_ARRAY_1D_TILED_THIN1;
- }
-
- tiling_info->gfx8.pipe_config =
- AMDGPU_TILING_GET(tiling_flags, PIPE_CONFIG);
-}
-EXPORT_IF_KUNIT(amdgpu_dm_plane_fill_gfx8_tiling_info_from_flags);
-
STATIC_IFN_KUNIT int amdgpu_dm_plane_fill_gfx6_tiling_info_from_modifier(
struct dc_tiling_info *tiling_info,
uint64_t modifier)
@@ -1154,7 +1121,6 @@ int amdgpu_dm_plane_fill_plane_buffer_attributes(struct amdgpu_device *adev,
const struct amdgpu_framebuffer *afb,
const enum surface_pixel_format format,
const enum dc_rotation_angle rotation,
- const uint64_t tiling_flags,
struct dc_tiling_info *tiling_info,
struct plane_size *plane_size,
struct dc_plane_dcc_param *dcc,
@@ -1229,8 +1195,6 @@ int amdgpu_dm_plane_fill_plane_buffer_attributes(struct amdgpu_device *adev,
address);
if (ret)
return ret;
- } else if (afb->base.modifier == DRM_FORMAT_MOD_INVALID) {
- amdgpu_dm_plane_fill_gfx8_tiling_info_from_flags(tiling_info, tiling_flags);
} else {
ret = amdgpu_dm_plane_fill_gfx6_tiling_info_from_modifier(tiling_info,
afb->base.modifier);
@@ -1326,7 +1290,6 @@ static int amdgpu_dm_plane_helper_prepare_fb(struct drm_plane *plane,
amdgpu_dm_plane_fill_plane_buffer_attributes(
adev, afb, plane_state->format, plane_state->rotation,
- afb->tiling_flags,
&plane_state->tiling_info, &plane_state->plane_size,
&plane_state->dcc, &plane_state->address,
afb->tmz_surface);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h
index 45230df1947e..c20ed57dbe63 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h
@@ -48,7 +48,6 @@ int amdgpu_dm_plane_fill_plane_buffer_attributes(struct amdgpu_device *adev,
const struct amdgpu_framebuffer *afb,
const enum surface_pixel_format format,
const enum dc_rotation_angle rotation,
- const uint64_t tiling_flags,
struct dc_tiling_info *tiling_info,
struct plane_size *plane_size,
struct dc_plane_dcc_param *dcc,
@@ -71,8 +70,6 @@ bool amdgpu_dm_plane_is_video_format(uint32_t format);
#if IS_ENABLED(CONFIG_DRM_AMD_DC_KUNIT_TEST)
void amdgpu_dm_plane_add_modifier(uint64_t **mods, uint64_t *size,
uint64_t *cap, uint64_t mod);
-void amdgpu_dm_plane_fill_gfx8_tiling_info_from_flags(struct dc_tiling_info *tiling_info,
- uint64_t tiling_flags);
void amdgpu_dm_plane_fill_gfx9_tiling_info_from_device(const struct amdgpu_device *adev,
struct dc_tiling_info *tiling_info);
void amdgpu_dm_plane_fill_gfx9_tiling_info_from_modifier(const struct amdgpu_device *adev,
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_plane_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_plane_test.c
index b89e1224b266..068bf94a6b25 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_plane_test.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_plane_test.c
@@ -998,86 +998,6 @@ static void dm_test_add_modifier_noop_when_mods_null(struct kunit *test)
KUNIT_EXPECT_EQ(test, cap, 7ULL);
}
-/**
- * dm_test_fill_gfx8_tiling_info_2d_tiled() - Verify GFX8 2D tiled flag parsing.
- * @test: KUnit test context.
- *
- * Verify if 2D tiled GFX8 flags populate expected tiling fields.
- */
-static void dm_test_fill_gfx8_tiling_info_2d_tiled(struct kunit *test)
-{
- struct dc_tiling_info tiling_info = {0};
- uint64_t tiling_flags = 0;
-
- tiling_flags |= AMDGPU_TILING_SET(ARRAY_MODE, DC_ARRAY_2D_TILED_THIN1);
- tiling_flags |= AMDGPU_TILING_SET(BANK_WIDTH, 2);
- tiling_flags |= AMDGPU_TILING_SET(BANK_HEIGHT, 1);
- tiling_flags |= AMDGPU_TILING_SET(MACRO_TILE_ASPECT, 3);
- tiling_flags |= AMDGPU_TILING_SET(TILE_SPLIT, 4);
- tiling_flags |= AMDGPU_TILING_SET(NUM_BANKS, 2);
- tiling_flags |= AMDGPU_TILING_SET(PIPE_CONFIG, 7);
-
- amdgpu_dm_plane_fill_gfx8_tiling_info_from_flags(&tiling_info, tiling_flags);
-
- KUNIT_EXPECT_EQ(test, (int)tiling_info.gfxversion, (int)DcGfxVersion8);
- KUNIT_EXPECT_EQ(test, (int)tiling_info.gfx8.array_mode, (int)DC_ARRAY_2D_TILED_THIN1);
- KUNIT_EXPECT_EQ(test, tiling_info.gfx8.bank_width, 2U);
- KUNIT_EXPECT_EQ(test, tiling_info.gfx8.bank_height, 1U);
- KUNIT_EXPECT_EQ(test, tiling_info.gfx8.tile_aspect, 3U);
- KUNIT_EXPECT_EQ(test, tiling_info.gfx8.tile_split, 4U);
- KUNIT_EXPECT_EQ(test, tiling_info.gfx8.num_banks, 2U);
- KUNIT_EXPECT_EQ(test, (int)tiling_info.gfx8.tile_mode,
- (int)DC_ADDR_SURF_MICRO_TILING_DISPLAY);
- KUNIT_EXPECT_EQ(test, tiling_info.gfx8.pipe_config, 7U);
-}
-
-/**
- * dm_test_fill_gfx8_tiling_info_1d_tiled() - Verify GFX8 1D tiled flag parsing.
- * @test: KUnit test context.
- *
- * Verify if 1D tiled GFX8 flags populate array mode and pipe config.
- */
-static void dm_test_fill_gfx8_tiling_info_1d_tiled(struct kunit *test)
-{
- struct dc_tiling_info tiling_info = {0};
- uint64_t tiling_flags = 0;
-
- tiling_flags |= AMDGPU_TILING_SET(ARRAY_MODE, DC_ARRAY_1D_TILED_THIN1);
- tiling_flags |= AMDGPU_TILING_SET(PIPE_CONFIG, 5);
-
- amdgpu_dm_plane_fill_gfx8_tiling_info_from_flags(&tiling_info, tiling_flags);
-
- KUNIT_EXPECT_EQ(test, (int)tiling_info.gfx8.array_mode, (int)DC_ARRAY_1D_TILED_THIN1);
- KUNIT_EXPECT_EQ(test, tiling_info.gfx8.pipe_config, 5U);
-}
-
-/**
- * dm_test_fill_gfx8_tiling_info_other_mode() - Verify non-1D/non-2D mode handling.
- * @test: KUnit test context.
- *
- * Verify if unsupported array mode keeps preset fields and updates pipe config.
- */
-static void dm_test_fill_gfx8_tiling_info_other_mode(struct kunit *test)
-{
- struct dc_tiling_info tiling_info = {0};
- uint64_t tiling_flags = 0;
-
- tiling_info.gfxversion = 0x7f;
- tiling_info.gfx8.array_mode = 0x7f;
- tiling_info.gfx8.tile_mode = 0x7f;
- tiling_info.gfx8.num_banks = 0x7f;
-
- tiling_flags |= AMDGPU_TILING_SET(PIPE_CONFIG, 6);
-
- amdgpu_dm_plane_fill_gfx8_tiling_info_from_flags(&tiling_info, tiling_flags);
-
- KUNIT_EXPECT_EQ(test, tiling_info.gfxversion, 0x7f);
- KUNIT_EXPECT_EQ(test, tiling_info.gfx8.array_mode, 0x7f);
- KUNIT_EXPECT_EQ(test, tiling_info.gfx8.tile_mode, 0x7f);
- KUNIT_EXPECT_EQ(test, tiling_info.gfx8.num_banks, 0x7f);
- KUNIT_EXPECT_EQ(test, tiling_info.gfx8.pipe_config, 6U);
-}
-
/**
* dm_test_fill_gfx9_tiling_info_from_device_pre_10_3() - Verify GFX9 field copy before 10.3.
* @test: KUnit test context.
@@ -3251,10 +3171,6 @@ static struct kunit_case amdgpu_dm_plane_test_cases[] = {
KUNIT_CASE(dm_test_add_modifier_appends_value),
KUNIT_CASE(dm_test_add_modifier_grows_capacity),
KUNIT_CASE(dm_test_add_modifier_noop_when_mods_null),
- /* amdgpu_dm_plane_fill_gfx8_tiling_info_from_flags() */
- KUNIT_CASE(dm_test_fill_gfx8_tiling_info_2d_tiled),
- KUNIT_CASE(dm_test_fill_gfx8_tiling_info_1d_tiled),
- KUNIT_CASE(dm_test_fill_gfx8_tiling_info_other_mode),
/* amdgpu_dm_plane_fill_gfx9_tiling_info_from_device() */
KUNIT_CASE(dm_test_fill_gfx9_tiling_info_from_device_pre_10_3),
KUNIT_CASE(dm_test_fill_gfx9_tiling_info_from_device_10_3_plus),
--
2.55.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] drm/amd/display: Don't use tiling flags anymore
2026-07-15 10:57 ` [PATCH 4/4] drm/amd/display: Don't use tiling flags anymore Timur Kristóf
@ 2026-07-15 11:42 ` sashiko-bot
2026-07-15 20:12 ` Timur Kristóf
0 siblings, 1 reply; 13+ messages in thread
From: sashiko-bot @ 2026-07-15 11:42 UTC (permalink / raw)
To: Timur Kristóf; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the `tiling_flags` check in `should_reset_plane` allows framebuffers with different DCC pitches or dimensions (but identical modifiers) to bypass DRM atomic check validation, leading to hardware corruption and stale DML calculations during commit.
--
--- Patch [4]: [PATCH 4/4] drm/amd/display: Don't use tiling flags anymore ---
commit b71fc193d0b1254508d4ef4e5b1e90decbcba27a
Author: Timur Kristóf <timur.kristof@gmail.com>
drm/amd/display: Don't use tiling flags anymore
This commit removes all code from amdgpu_dm that dealt with tiling flags,
as all supported GPU generations now support DRM format modifiers. The legacy
non-DC display code still relies on tiling flags.
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index c643cc5428577..2f7388c47d02a 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
[ ... ]
> @@ -4279,7 +4277,6 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>
> fill_dc_plane_info_and_addr(
> dm->adev, new_plane_state,
> - afb->tiling_flags,
> &bundle->plane_infos[planes_count],
> &bundle->flip_addrs[planes_count].address,
> afb->tmz_surface);
[Severity: High]
Since the return value of fill_dc_plane_info_and_addr() is ignored here,
what happens if DCC validation fails due to missed dimension or pitch
changes?
Could this result in a zero-filled plane_info structure being sent to the
hardware (for example, setting global_alpha to 0) and causing a black
screen?
[ ... ]
> @@ -6248,8 +6245,7 @@ static bool should_reset_plane(struct drm_atomic_state *state,
> new_afb = (struct amdgpu_framebuffer *)new_other_state->fb;
>
> /* Tiling and DCC changes also require bandwidth updates. */
> - if (old_afb->tiling_flags != new_afb->tiling_flags ||
> - old_afb->base.modifier != new_afb->base.modifier)
> + if (old_afb->base.modifier != new_afb->base.modifier)
> return true;
> }
[Severity: High]
By removing the tiling_flags check here in should_reset_plane(), how do we
ensure that framebuffers with identical modifiers but different DCC pitches
or dimensions still trigger a validation reset?
If a user-space client flips between two framebuffers with the same format
and source rectangle, but different backing buffer dimensions, it seems
this might bypass DRM atomic check validation.
Could this lead to DC hardware being programmed using stale Display Mode
Logger (DML) calculations and watermarks, resulting in display corruption?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260715105757.13125-1-timur.kristof@gmail.com?part=4
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] drm/amd/display: Don't use tiling flags anymore
2026-07-15 11:42 ` sashiko-bot
@ 2026-07-15 20:12 ` Timur Kristóf
0 siblings, 0 replies; 13+ messages in thread
From: Timur Kristóf @ 2026-07-15 20:12 UTC (permalink / raw)
To: sashiko-reviews; +Cc: dri-devel
On 2026. július 15., szerda 13:42:38 közép-európai nyári idő sashiko-
bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider: - [High] Removing the `tiling_flags` check in
> `should_reset_plane` allows framebuffers with different DCC pitches or
> dimensions (but identical modifiers) to bypass DRM atomic check validation,
> leading to hardware corruption and stale DML calculations during commit. --
This is not an issue. DCC is not supported at all on GFX6-7 and is not
supported by the display engine on GFX8. The code does not advertise any
modifiers with DCC on GFX6-8 either.
>
> --- Patch [4]: [PATCH 4/4] drm/amd/display: Don't use tiling flags anymore
> --- commit b71fc193d0b1254508d4ef4e5b1e90decbcba27a
> Author: Timur Kristóf <timur.kristof@gmail.com>
>
> drm/amd/display: Don't use tiling flags anymore
>
> This commit removes all code from amdgpu_dm that dealt with tiling flags,
> as all supported GPU generations now support DRM format modifiers. The
> legacy non-DC display code still relies on tiling flags.
>
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index
> > c643cc5428577..2f7388c47d02a 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>
> [ ... ]
>
> > @@ -4279,7 +4277,6 @@ static void amdgpu_dm_commit_planes(struct
> > drm_atomic_state *state,>
> > fill_dc_plane_info_and_addr(
> >
> > dm->adev, new_plane_state,
> >
> > - afb->tiling_flags,
> >
> > &bundle->plane_infos[planes_count],
> > &bundle->flip_addrs[planes_count].address,
> > afb->tmz_surface);
>
> [Severity: High]
> Since the return value of fill_dc_plane_info_and_addr() is ignored here,
> what happens if DCC validation fails due to missed dimension or pitch
> changes?
DCC is not supported by the display engine on GFX6-8 so there is no DCC
validation.
>
> Could this result in a zero-filled plane_info structure being sent to the
> hardware (for example, setting global_alpha to 0) and causing a black
> screen?
No, it couldn't.
>
> [ ... ]
>
> > @@ -6248,8 +6245,7 @@ static bool should_reset_plane(struct
> > drm_atomic_state *state,>
> > new_afb = (struct amdgpu_framebuffer *)new_other_state-
>fb;
> >
> > /* Tiling and DCC changes also require bandwidth
updates. */
> >
> > - if (old_afb->tiling_flags != new_afb->tiling_flags ||
> > - old_afb->base.modifier != new_afb->base.modifier)
> > + if (old_afb->base.modifier != new_afb->base.modifier)
> >
> > return true;
> >
> > }
>
> [Severity: High]
> By removing the tiling_flags check here in should_reset_plane(), how do we
> ensure that framebuffers with identical modifiers but different DCC pitches
> or dimensions still trigger a validation reset?
The tiling flags are no longer used because they are converted to modifiers.
Therefore, removing the tiling flag check is not changing the behavior of the
code.
As I explained above, DCC is not supported by this HW.
Additionally, "identical modifiers but with different DCC pitches" would be
illegal for DRM format modifiers.
>
> If a user-space client flips between two framebuffers with the same format
> and source rectangle, but different backing buffer dimensions, it seems
> this might bypass DRM atomic check validation.
How?
>
> Could this lead to DC hardware being programmed using stale Display Mode
> Logger (DML) calculations and watermarks, resulting in display corruption?
How?
^ permalink raw reply [flat|nested] 13+ messages in thread