drm/tegra: Sanitize format modifiers

PATCH In review Thierry Reding 3 participants 6 messages Nov 27, 2017 4 messages not in the archive

Subsystems (4)

DRM drivers DRM misc Tegra architecture support DRM drivers for NVIDIA Tegra

lore

Share & tools

lore

Thierry Reding

author

2017-11-27 09:39 UTC #0 lore raw

cover drm/tegra: Sanitize format modifiers

From: Thierry Reding <[email protected]>

Hi,

This series is preparatory work in order to enable format modifiers on both Tegra and Nouveau drivers to allow drivers to exchange tiling mode information. I also have the corresponding libdrm changes that I can send out as soon as these patches have been merged.

Technically this breaks ABI, but these modifiers are not used anywhere yet, except in libdrm's modetest where they are used to decode the IN_FORMATS property. However, the Tegra DRM driver doesn't implement the ->format_mod_supported() callback yet, so the modifiers are never actually exposed.

I had discussed this with Daniel Vetter a little while back and he thought this was okay. As suggested by him, I renamed the modifier definitions for good measure to avoid clashing with the old definitions in case they are used in test code somewhere.

Thierry

Thierry Reding (2):
drm/fourcc: Fix fourcc_mod_code() definition
drm/tegra: Sanitize format modifiers

 drivers/gpu/drm/tegra/fb.c    | 35 +++++++++++++++++++++++++++++------
 include/uapi/drm/drm_fourcc.h | 38 ++++++++++++++++++++------------------
 2 files changed, 49 insertions(+), 24 deletions(-)

Signature · 1 line

2.15.0

A message that is not in the archive (sent off-list or not yet received): 20171127093948.20986-1-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

Thierry Reding

author

2017-11-27 09:39 UTC #2 lore raw

PATCH 1/2 drm/fourcc: Fix fourcc_mod_code() definition

From: Thierry Reding <[email protected]>

Avoid compiler warnings when the val parameter is an expression.

Signed-off-by Thierry Reding <[email protected]>

---
 include/uapi/drm/drm_fourcc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/uapi/drm/drm_fourcc.h +1 -1

index 3ad838d3f93f..a76ed8f9e383 100644
@@ -188,7 +188,7 @@ extern "C" {
 #define DRM_FORMAT_RESERVED	      ((1ULL << 56) - 1)
 
 #define fourcc_mod_code(vendor, val) \
-	((((__u64)DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | (val & 0x00ffffffffffffffULL))
+	((((__u64)DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | ((val) & 0x00ffffffffffffffULL))
 
 /*
  * Format Modifier tokens:

Signature · 1 line

2.15.0

A message that is not in the archive (sent off-list or not yet received): 20171127093948.20986-2-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

Daniel Vetter

Reviewed-by

2017-11-27 10:35 UTC #4 lore raw

Re: [PATCH 1/2] drm/fourcc: Fix fourcc_mod_code() definition

Quoted 5 lines · Thierry Reding

From: Thierry Reding <[email protected]>
Avoid compiler warnings when the val parameter is an expression.
Signed-off-by Thierry Reding <[email protected]>

Reviewed-by Daniel Vetter <daniel.vetter-/[email protected]>

Quoted 24 lines

---
 include/uapi/drm/drm_fourcc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/uapi/drm/drm_fourcc.h +1 -1

index 3ad838d3f93f..a76ed8f9e383 100644
@@ -188,7 +188,7 @@ extern "C" {
 #define DRM_FORMAT_RESERVED	      ((1ULL << 56) - 1)
 
 #define fourcc_mod_code(vendor, val) \
-	((((__u64)DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | (val & 0x00ffffffffffffffULL))
+	((((__u64)DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | ((val) & 0x00ffffffffffffffULL))
 
 /*
  * Format Modifier tokens:

Signature · 6 lines

2.15.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Signature · 3 lines

Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Thierry Reding

2017-11-27 09:39 UTC #5 lore raw

PATCH 2/2 drm/tegra: Sanitize format modifiers

In reply to Thierry Reding

From: Thierry Reding <[email protected]>

The existing format modifier definitions were merged prematurely, and recent work has unveiled that the definitions are suboptimal in several ways:

- The format specifiers, except for one, are not Tegra specific, but the names don't reflect that.
- The number space is split into two, reserving 32 bits for some "parameter" which most of the modifiers are not going to have.
- Symbolic names for the modifiers are not using the standard DRM_FORMAT_MOD_* prefix, which makes them awkward to use.
- The vendor prefix NV is somewhat ambiguous.

Fortunately, nobody's started using these modifiers, so we can still fix the above issues. Do so by using the standard prefix. Also, remove TEGRA from the name of those modifiers that exist on NVIDIA GPUs as well. In case of the block linear modifiers, make the "parameter" smaller (4 bits, though only 6 values are valid) and don't let that leak into any of the other modifiers.

Finally, also use the more canonical NVIDIA instead of the ambiguous NV prefix.

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/fb.c    | 35 +++++++++++++++++++++++++++++------
 include/uapi/drm/drm_fourcc.h | 36 +++++++++++++++++++-----------------
 2 files changed, 48 insertions(+), 23 deletions(-)

drivers/gpu/drm/tegra/fb.c +29 -6

index 80540c1c66dc..406e895d82cc 100644
@@ -54,17 +54,40 @@ int tegra_fb_get_tiling(struct drm_framebuffer *framebuffer,
 	struct tegra_fb *fb = to_tegra_fb(framebuffer);
 	uint64_t modifier = fb->base.modifier;
 
-	switch (fourcc_mod_tegra_mod(modifier)) {
-	case NV_FORMAT_MOD_TEGRA_TILED:
+	switch (modifier) {
+	case DRM_FORMAT_MOD_NVIDIA_TEGRA_TILED:
 		tiling->mode = TEGRA_BO_TILING_MODE_TILED;
 		tiling->value = 0;
 		break;
 
-	case NV_FORMAT_MOD_TEGRA_16BX2_BLOCK(0):
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(0):
 		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
-		tiling->value = fourcc_mod_tegra_param(modifier);
-		if (tiling->value > 5)
-			return -EINVAL;
+		tiling->value = 0;
+		break;
+
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(1):
+		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
+		tiling->value = 1;
+		break;
+
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(2):
+		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
+		tiling->value = 2;
+		break;
+
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(3):
+		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
+		tiling->value = 3;
+		break;
+
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(4):
+		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
+		tiling->value = 4;
+		break;
+
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(5):
+		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
+		tiling->value = 5;
 		break;
 
 	default:

include/uapi/drm/drm_fourcc.h +19 ~~-17~~

index a76ed8f9e383..e04613d30a13 100644
@@ -178,7 +178,7 @@ extern "C" {
 #define DRM_FORMAT_MOD_VENDOR_NONE    0
 #define DRM_FORMAT_MOD_VENDOR_INTEL   0x01
 #define DRM_FORMAT_MOD_VENDOR_AMD     0x02
-#define DRM_FORMAT_MOD_VENDOR_NV      0x03
+#define DRM_FORMAT_MOD_VENDOR_NVIDIA  0x03
 #define DRM_FORMAT_MOD_VENDOR_SAMSUNG 0x04
 #define DRM_FORMAT_MOD_VENDOR_QCOM    0x05
 #define DRM_FORMAT_MOD_VENDOR_VIVANTE 0x06

@@ -338,29 +338,17 @@ extern "C" {
  */
 #define DRM_FORMAT_MOD_VIVANTE_SPLIT_SUPER_TILED fourcc_mod_code(VIVANTE, 4)
 
-/* NVIDIA Tegra frame buffer modifiers */
-
-/*
- * Some modifiers take parameters, for example the number of vertical GOBs in
- * a block. Reserve the lower 32 bits for parameters
- */
-#define __fourcc_mod_tegra_mode_shift 32
-#define fourcc_mod_tegra_code(val, params) \
-	fourcc_mod_code(NV, ((((__u64)val) << __fourcc_mod_tegra_mode_shift) | params))
-#define fourcc_mod_tegra_mod(m) \
-	(m & ~((1ULL << __fourcc_mod_tegra_mode_shift) - 1))
-#define fourcc_mod_tegra_param(m) \
-	(m & ((1ULL << __fourcc_mod_tegra_mode_shift) - 1))
+/* NVIDIA frame buffer modifiers */
 
 /*
  * Tegra Tiled Layout, used by Tegra 2, 3 and 4.
  *
  * Pixels are arranged in simple tiles of 16 x 16 bytes.
  */
-#define NV_FORMAT_MOD_TEGRA_TILED fourcc_mod_tegra_code(1, 0)
+#define DRM_FORMAT_MOD_NVIDIA_TEGRA_TILED fourcc_mod_code(NVIDIA, 1)
 
 /*
- * Tegra 16Bx2 Block Linear layout, used by TK1/TX1
+ * 16Bx2 Block Linear layout, used by desktop GPUs, and Tegra K1 and later
  *
  * Pixels are arranged in 64x8 Groups Of Bytes (GOBs). GOBs are then stacked
  * vertically by a power of 2 (1 to 32 GOBs) to form a block.

@@ -380,7 +368,21 @@ extern "C" {
  * Chapter 20 "Pixel Memory Formats" of the Tegra X1 TRM describes this format
  * in full detail.
  */
-#define NV_FORMAT_MOD_TEGRA_16BX2_BLOCK(v) fourcc_mod_tegra_code(2, v)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(v) \
+	fourcc_mod_code(NVIDIA, 0x10 | ((v) & 0xf))
+
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_ONE_GOB \
+	fourcc_mod_code(NVIDIA, 0x10)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_TWO_GOB \
+	fourcc_mod_code(NVIDIA, 0x11)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_FOUR_GOB \
+	fourcc_mod_code(NVIDIA, 0x12)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_EIGHT_GOB \
+	fourcc_mod_code(NVIDIA, 0x13)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_SIXTEEN_GOB \
+	fourcc_mod_code(NVIDIA, 0x14)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_THIRTYTWO_GOB \
+	fourcc_mod_code(NVIDIA, 0x15)
 
 /*
  * Broadcom VC4 "T" format

Signature · 6 lines

2.15.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

A message that is not in the archive (sent off-list or not yet received): 20171127093948.20986-3-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

Daniel Vetter

Acked-by

2017-11-27 10:38 UTC #7 lore raw

Quoted 128 lines · Thierry Reding

From: Thierry Reding <[email protected]>

The existing format modifier definitions were merged prematurely, and recent work has unveiled that the definitions are suboptimal in several ways:

Finally, also use the more canonical NVIDIA instead of the ambiguous NV prefix.

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/fb.c    | 35 +++++++++++++++++++++++++++++------
 include/uapi/drm/drm_fourcc.h | 36 +++++++++++++++++++-----------------
 2 files changed, 48 insertions(+), 23 deletions(-)

drivers/gpu/drm/tegra/fb.c +29 -6

index 80540c1c66dc..406e895d82cc 100644
@@ -54,17 +54,40 @@ int tegra_fb_get_tiling(struct drm_framebuffer *framebuffer,
 	struct tegra_fb *fb = to_tegra_fb(framebuffer);
 	uint64_t modifier = fb->base.modifier;
 
-	switch (fourcc_mod_tegra_mod(modifier)) {
-	case NV_FORMAT_MOD_TEGRA_TILED:
+	switch (modifier) {
+	case DRM_FORMAT_MOD_NVIDIA_TEGRA_TILED:
 		tiling->mode = TEGRA_BO_TILING_MODE_TILED;
 		tiling->value = 0;
 		break;
 
-	case NV_FORMAT_MOD_TEGRA_16BX2_BLOCK(0):
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(0):
 		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
-		tiling->value = fourcc_mod_tegra_param(modifier);
-		if (tiling->value > 5)
-			return -EINVAL;
+		tiling->value = 0;
+		break;
+
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(1):
+		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
+		tiling->value = 1;
+		break;
+
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(2):
+		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
+		tiling->value = 2;
+		break;
+
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(3):
+		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
+		tiling->value = 3;
+		break;
+
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(4):
+		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
+		tiling->value = 4;
+		break;
+
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(5):
+		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
+		tiling->value = 5;
 		break;
 
 	default:

include/uapi/drm/drm_fourcc.h +4 ~~-16~~

index a76ed8f9e383..e04613d30a13 100644
@@ -178,7 +178,7 @@ extern "C" {
 #define DRM_FORMAT_MOD_VENDOR_NONE    0
 #define DRM_FORMAT_MOD_VENDOR_INTEL   0x01
 #define DRM_FORMAT_MOD_VENDOR_AMD     0x02
-#define DRM_FORMAT_MOD_VENDOR_NV      0x03
+#define DRM_FORMAT_MOD_VENDOR_NVIDIA  0x03
 #define DRM_FORMAT_MOD_VENDOR_SAMSUNG 0x04
 #define DRM_FORMAT_MOD_VENDOR_QCOM    0x05
 #define DRM_FORMAT_MOD_VENDOR_VIVANTE 0x06

@@ -338,29 +338,17 @@ extern "C" {
  */
 #define DRM_FORMAT_MOD_VIVANTE_SPLIT_SUPER_TILED fourcc_mod_code(VIVANTE, 4)
 
-/* NVIDIA Tegra frame buffer modifiers */
-
-/*
- * Some modifiers take parameters, for example the number of vertical GOBs in
- * a block. Reserve the lower 32 bits for parameters
- */
-#define __fourcc_mod_tegra_mode_shift 32
-#define fourcc_mod_tegra_code(val, params) \
-	fourcc_mod_code(NV, ((((__u64)val) << __fourcc_mod_tegra_mode_shift) | params))
-#define fourcc_mod_tegra_mod(m) \
-	(m & ~((1ULL << __fourcc_mod_tegra_mode_shift) - 1))
-#define fourcc_mod_tegra_param(m) \
-	(m & ((1ULL << __fourcc_mod_tegra_mode_shift) - 1))
+/* NVIDIA frame buffer modifiers */
 
 /*
  * Tegra Tiled Layout, used by Tegra 2, 3 and 4.
  *
  * Pixels are arranged in simple tiles of 16 x 16 bytes.
  */
-#define NV_FORMAT_MOD_TEGRA_TILED fourcc_mod_tegra_code(1, 0)
+#define DRM_FORMAT_MOD_NVIDIA_TEGRA_TILED fourcc_mod_code(NVIDIA, 1)
 
 /*
- * Tegra 16Bx2 Block Linear layout, used by TK1/TX1
+ * 16Bx2 Block Linear layout, used by desktop GPUs, and Tegra K1 and later
  *
  * Pixels are arranged in 64x8 Groups Of Bytes (GOBs). GOBs are then stacked
  * vertically by a power of 2 (1 to 32 GOBs) to form a block.

include/uapi/drm/drm_fourcc.h extern "C" { · line 375

Quoted 9 lines

include/uapi/drm/drm_fourcc.h +5 -1

@@ -380,7 +368,21 @@ extern "C" {
  * Chapter 20 "Pixel Memory Formats" of the Tegra X1 TRM describes this format
  * in full detail.
  */
-#define NV_FORMAT_MOD_TEGRA_16BX2_BLOCK(v) fourcc_mod_tegra_code(2, v)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(v) \
+	fourcc_mod_code(NVIDIA, 0x10 | ((v) & 0xf))
+
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_ONE_GOB \
+	fourcc_mod_code(NVIDIA, 0x10)

Any reason to start at 0x10? Either way I think this looks a lot more in line with what we generally do with modifiers:

Acked-by Daniel Vetter <daniel.vetter-/[email protected]>

Cheers, Daniel

Quoted 20 lines

include/uapi/drm/drm_fourcc.h +10 -0

+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_TWO_GOB \
+	fourcc_mod_code(NVIDIA, 0x11)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_FOUR_GOB \
+	fourcc_mod_code(NVIDIA, 0x12)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_EIGHT_GOB \
+	fourcc_mod_code(NVIDIA, 0x13)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_SIXTEEN_GOB \
+	fourcc_mod_code(NVIDIA, 0x14)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_THIRTYTWO_GOB \
+	fourcc_mod_code(NVIDIA, 0x15)
 
 /*
  * Broadcom VC4 "T" format

Signature · 6 lines

2.15.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Signature · 3 lines

Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

A message that is not in the archive (sent off-list or not yet received): 20171127103814.qnywbtkjqzaj74g4-dv86pmgwkMBes7Z6vYuT8azUEOm+Xw19@public.gmane.org

Thierry Reding

author

2017-11-27 11:18 UTC #9 lore raw

Quoted 132 lines · Daniel Vetter

Quoted 131 lines · Thierry Reding

From: Thierry Reding <[email protected]>

The existing format modifier definitions were merged prematurely, and recent work has unveiled that the definitions are suboptimal in several ways:

Finally, also use the more canonical NVIDIA instead of the ambiguous NV prefix.

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/fb.c    | 35 +++++++++++++++++++++++++++++------
 include/uapi/drm/drm_fourcc.h | 36 +++++++++++++++++++-----------------
 2 files changed, 48 insertions(+), 23 deletions(-)

drivers/gpu/drm/tegra/fb.c +29 -6

index 80540c1c66dc..406e895d82cc 100644
@@ -54,17 +54,40 @@ int tegra_fb_get_tiling(struct drm_framebuffer *framebuffer,
 	struct tegra_fb *fb = to_tegra_fb(framebuffer);
 	uint64_t modifier = fb->base.modifier;
 
-	switch (fourcc_mod_tegra_mod(modifier)) {
-	case NV_FORMAT_MOD_TEGRA_TILED:
+	switch (modifier) {
+	case DRM_FORMAT_MOD_NVIDIA_TEGRA_TILED:
 		tiling->mode = TEGRA_BO_TILING_MODE_TILED;
 		tiling->value = 0;
 		break;
 
-	case NV_FORMAT_MOD_TEGRA_16BX2_BLOCK(0):
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(0):
 		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
-		tiling->value = fourcc_mod_tegra_param(modifier);
-		if (tiling->value > 5)
-			return -EINVAL;
+		tiling->value = 0;
+		break;
+
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(1):
+		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
+		tiling->value = 1;
+		break;
+
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(2):
+		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
+		tiling->value = 2;
+		break;
+
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(3):
+		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
+		tiling->value = 3;
+		break;
+
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(4):
+		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
+		tiling->value = 4;
+		break;
+
+	case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(5):
+		tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
+		tiling->value = 5;
 		break;
 
 	default:

include/uapi/drm/drm_fourcc.h +4 ~~-16~~

index a76ed8f9e383..e04613d30a13 100644
@@ -178,7 +178,7 @@ extern "C" {
 #define DRM_FORMAT_MOD_VENDOR_NONE    0
 #define DRM_FORMAT_MOD_VENDOR_INTEL   0x01
 #define DRM_FORMAT_MOD_VENDOR_AMD     0x02
-#define DRM_FORMAT_MOD_VENDOR_NV      0x03
+#define DRM_FORMAT_MOD_VENDOR_NVIDIA  0x03
 #define DRM_FORMAT_MOD_VENDOR_SAMSUNG 0x04
 #define DRM_FORMAT_MOD_VENDOR_QCOM    0x05
 #define DRM_FORMAT_MOD_VENDOR_VIVANTE 0x06

@@ -338,29 +338,17 @@ extern "C" {
  */
 #define DRM_FORMAT_MOD_VIVANTE_SPLIT_SUPER_TILED fourcc_mod_code(VIVANTE, 4)
 
-/* NVIDIA Tegra frame buffer modifiers */
-
-/*
- * Some modifiers take parameters, for example the number of vertical GOBs in
- * a block. Reserve the lower 32 bits for parameters
- */
-#define __fourcc_mod_tegra_mode_shift 32
-#define fourcc_mod_tegra_code(val, params) \
-	fourcc_mod_code(NV, ((((__u64)val) << __fourcc_mod_tegra_mode_shift) | params))
-#define fourcc_mod_tegra_mod(m) \
-	(m & ~((1ULL << __fourcc_mod_tegra_mode_shift) - 1))
-#define fourcc_mod_tegra_param(m) \
-	(m & ((1ULL << __fourcc_mod_tegra_mode_shift) - 1))
+/* NVIDIA frame buffer modifiers */
 
 /*
  * Tegra Tiled Layout, used by Tegra 2, 3 and 4.
  *
  * Pixels are arranged in simple tiles of 16 x 16 bytes.
  */
-#define NV_FORMAT_MOD_TEGRA_TILED fourcc_mod_tegra_code(1, 0)
+#define DRM_FORMAT_MOD_NVIDIA_TEGRA_TILED fourcc_mod_code(NVIDIA, 1)
 
 /*
- * Tegra 16Bx2 Block Linear layout, used by TK1/TX1
+ * 16Bx2 Block Linear layout, used by desktop GPUs, and Tegra K1 and later
  *
  * Pixels are arranged in 64x8 Groups Of Bytes (GOBs). GOBs are then stacked
  * vertically by a power of 2 (1 to 32 GOBs) to form a block.

@@ -380,7 +368,21 @@ extern "C" {
  * Chapter 20 "Pixel Memory Formats" of the Tegra X1 TRM describes this format
  * in full detail.
  */

Quoted 8 lines

include/uapi/drm/drm_fourcc.h extern "C" { · line 375
Quoted 6 lines
include/uapi/drm/drm_fourcc.h +5 -1
-#define NV_FORMAT_MOD_TEGRA_16BX2_BLOCK(v) fourcc_mod_tegra_code(2, v)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(v) \
+	fourcc_mod_code(NVIDIA, 0x10 | ((v) & 0xf))
+
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_ONE_GOB \
+	fourcc_mod_code(NVIDIA, 0x10)
Any reason to start at 0x10?

As best I can tell the block size parameter is 4 bits wide, although only the six values defined here are actually valid. So I thought I'd align this to four bits and give it four bits in case this ever needs to expand beyond the existing 6 values. I doubt that somewhat, but it would be awkward to have discontinuities in this. We could still get discontinuities if this ever extends beyond four bits, but we can cross that bridge when we get to it. reserving four bits for this parameter seems like a good compromise to me.

We can always pack other modifiers in the gap between 1 and 0x10 to fill it up.

Quoted 4 lines

Either way I think this looks a lot more in line with what we generally do with modifiers:
Acked-by Daniel Vetter <daniel.vetter-/[email protected]>

Thanks!
Thierry