drm/vc4: Add support for SAND modifier.
diff mbox

Message ID 20180303013431.1088-1-eric@anholt.net
State New
Headers show

Commit Message

Eric Anholt March 3, 2018, 1:34 a.m. UTC
From: Dave Stevenson <dave.stevenson@raspberrypi.org>

This is the format generated by VC4's H.264 engine, and preferred by
the ISP as well.  By displaying SAND buffers directly, we can avoid
needing to use the ISP to rewrite the SAND H.264 output to linear
before display.

This is a joint effort by Dave Stevenson (who wrote the initial patch
and DRM demo) and Eric Anholt (drm_fourcc.h generalization, safety
checks, RGBA support).

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
Signed-off-by: Eric Anholt <eric@anholt.net>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Daniel Stone <daniels@collabora.com>
Cc: Boris Brezillon <boris.brezillon@bootlin.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
---

Ccing a couple of folks who are likely to have opinions about
drm_fourcc.h additions (Do we have enough docs?  Are the macros OK?),
and Bootlin who are likely reviewers.

The plan is to use these modifiers in VC4 GL imports as well, and for
buffers coming from the v4l2 mmal camera driver.  You can find a demo
using this in KMS planes at
https://github.com/anholt/drm_mmal/commit/sand for now.

 drivers/gpu/drm/vc4/vc4_plane.c | 84 +++++++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/vc4/vc4_regs.h  |  6 +++
 include/uapi/drm/drm_fourcc.h   | 59 +++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+), 7 deletions(-)

Comments

Daniel Stone March 14, 2018, 11:08 a.m. UTC | #1
Hey Eric,

On 3 March 2018 at 01:34, Eric Anholt <eric@anholt.net> wrote:
> Ccing a couple of folks who are likely to have opinions about
> drm_fourcc.h additions (Do we have enough docs?  Are the macros OK?),
> and Bootlin who are likely reviewers.
>
> The plan is to use these modifiers in VC4 GL imports as well, and for
> buffers coming from the v4l2 mmal camera driver.  You can find a demo
> using this in KMS planes at
> https://github.com/anholt/drm_mmal/commit/sand for now.

I had a dig through and this seems like the most sensible thing to do
if you have a reasonable variety of tile heights. If you only see a
couple of combinations in the wild, then hardcoding them as separate
modifiers might make things easier than hiving off 24 bits. If you do
keep the split though, and especially if you're envisioning future
flexible tile formats, maybe something like an 8 code / 48 params
split would make more sense.

Either way, those are just my opinions (you did ask), and I don't see
anything really objectionable, so if you think that's a good split
then this is:
Acked-by: Daniel Stone <daniels@collabora.com>

Cheers,
Daniel
Eric Anholt March 16, 2018, 10:05 p.m. UTC | #2
Daniel Stone <daniel@fooishbar.org> writes:

> Hey Eric,
>
> On 3 March 2018 at 01:34, Eric Anholt <eric@anholt.net> wrote:
>> Ccing a couple of folks who are likely to have opinions about
>> drm_fourcc.h additions (Do we have enough docs?  Are the macros OK?),
>> and Bootlin who are likely reviewers.
>>
>> The plan is to use these modifiers in VC4 GL imports as well, and for
>> buffers coming from the v4l2 mmal camera driver.  You can find a demo
>> using this in KMS planes at
>> https://github.com/anholt/drm_mmal/commit/sand for now.
>
> I had a dig through and this seems like the most sensible thing to do
> if you have a reasonable variety of tile heights. If you only see a
> couple of combinations in the wild, then hardcoding them as separate
> modifiers might make things easier than hiving off 24 bits. If you do
> keep the split though, and especially if you're envisioning future
> flexible tile formats, maybe something like an 8 code / 48 params
> split would make more sense.
>
> Either way, those are just my opinions (you did ask), and I don't see
> anything really objectionable, so if you think that's a good split
> then this is:
> Acked-by: Daniel Stone <daniels@collabora.com>

I had been thinking of potentially specifying a meaning for the other 24
bits (separate U/V plane v stride if set, if we ever get a component
that needs to do that), so I went ahead and did this, along with a
couple of fixes due to Ville's patch.

Patch
diff mbox

diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index c4c7af11fec5..97de9c7821c7 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -520,10 +520,12 @@  static int vc4_plane_mode_set(struct drm_plane *plane,
 	struct drm_framebuffer *fb = state->fb;
 	u32 ctl0_offset = vc4_state->dlist_count;
 	const struct hvs_format *format = vc4_get_hvs_format(fb->format->format);
+	u64 base_format_mod = fourcc_mod_broadcom_mod(fb->modifier);
 	int num_planes = drm_format_num_planes(format->drm);
 	u32 scl0, scl1, pitch0;
 	u32 lbm_size, tiling;
 	unsigned long irqflags;
+	u32 hvs_format = format->hvs;
 	int ret, i;
 
 	ret = vc4_plane_setup_clipping_and_scaling(state);
@@ -563,7 +565,7 @@  static int vc4_plane_mode_set(struct drm_plane *plane,
 		scl1 = vc4_get_scl_field(state, 0);
 	}
 
-	switch (fb->modifier) {
+	switch (base_format_mod) {
 	case DRM_FORMAT_MOD_LINEAR:
 		tiling = SCALER_CTL0_TILING_LINEAR;
 		pitch0 = VC4_SET_FIELD(fb->pitches[0], SCALER_SRC_PITCH);
@@ -586,6 +588,49 @@  static int vc4_plane_mode_set(struct drm_plane *plane,
 		break;
 	}
 
+	case DRM_FORMAT_MOD_BROADCOM_SAND64:
+	case DRM_FORMAT_MOD_BROADCOM_SAND128:
+	case DRM_FORMAT_MOD_BROADCOM_SAND256: {
+		uint32_t param = fourcc_mod_broadcom_param(fb->modifier);
+
+		/* Column-based NV12 or RGBA.
+		 */
+		if (fb->format->num_planes > 1) {
+			if (hvs_format != HVS_PIXEL_FORMAT_YCBCR_YUV420_2PLANE) {
+				DRM_DEBUG_KMS("SAND format only valid for NV12/21");
+				return -EINVAL;
+			}
+			hvs_format = HVS_PIXEL_FORMAT_H264;
+		} else {
+			if (base_format_mod == DRM_FORMAT_MOD_BROADCOM_SAND256) {
+				DRM_DEBUG_KMS("SAND256 format only valid for H.264");
+				return -EINVAL;
+			}
+		}
+
+		switch (base_format_mod) {
+		case DRM_FORMAT_MOD_BROADCOM_SAND64:
+			tiling = SCALER_CTL0_TILING_64B;
+			break;
+		case DRM_FORMAT_MOD_BROADCOM_SAND128:
+			tiling = SCALER_CTL0_TILING_128B;
+			break;
+		case DRM_FORMAT_MOD_BROADCOM_SAND256:
+			tiling = SCALER_CTL0_TILING_256B_OR_T;
+			break;
+		default:
+			break;
+		}
+
+		if (param > SCALER_TILE_HEIGHT_MASK) {
+			DRM_DEBUG_KMS("SAND height too large (%d)\n", param);
+			return -EINVAL;
+		}
+
+		pitch0 = VC4_SET_FIELD(param, SCALER_TILE_HEIGHT);
+		break;
+	}
+
 	default:
 		DRM_DEBUG_KMS("Unsupported FB tiling flag 0x%16llx",
 			      (long long)fb->modifier);
@@ -596,7 +641,7 @@  static int vc4_plane_mode_set(struct drm_plane *plane,
 	vc4_dlist_write(vc4_state,
 			SCALER_CTL0_VALID |
 			(format->pixel_order << SCALER_CTL0_ORDER_SHIFT) |
-			(format->hvs << SCALER_CTL0_PIXEL_FORMAT_SHIFT) |
+			(hvs_format << SCALER_CTL0_PIXEL_FORMAT_SHIFT) |
 			VC4_SET_FIELD(tiling, SCALER_CTL0_TILING) |
 			(vc4_state->is_unity ? SCALER_CTL0_UNITY : 0) |
 			VC4_SET_FIELD(scl0, SCALER_CTL0_SCL0) |
@@ -649,8 +694,13 @@  static int vc4_plane_mode_set(struct drm_plane *plane,
 
 	/* Pitch word 1/2 */
 	for (i = 1; i < num_planes; i++) {
-		vc4_dlist_write(vc4_state,
-				VC4_SET_FIELD(fb->pitches[i], SCALER_SRC_PITCH));
+		if (hvs_format != HVS_PIXEL_FORMAT_H264) {
+			vc4_dlist_write(vc4_state,
+					VC4_SET_FIELD(fb->pitches[i],
+						      SCALER_SRC_PITCH));
+		} else {
+			vc4_dlist_write(vc4_state, pitch0);
+		}
 	}
 
 	/* Colorspace conversion words */
@@ -920,11 +970,28 @@  static bool vc4_format_mod_supported(struct drm_plane *plane,
 	case DRM_FORMAT_BGR565:
 	case DRM_FORMAT_ARGB1555:
 	case DRM_FORMAT_XRGB1555:
-		return true;
-	case DRM_FORMAT_YUV422:
-	case DRM_FORMAT_YVU422:
+		switch (fourcc_mod_broadcom_mod(modifier)) {
+		case DRM_FORMAT_MOD_LINEAR:
+		case DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED:
+		case DRM_FORMAT_MOD_BROADCOM_SAND64:
+		case DRM_FORMAT_MOD_BROADCOM_SAND128:
+			return true;
+		default:
+			return false;
+		}
 	case DRM_FORMAT_YUV420:
 	case DRM_FORMAT_YVU420:
+		switch (fourcc_mod_broadcom_mod(modifier)) {
+		case DRM_FORMAT_MOD_LINEAR:
+		case DRM_FORMAT_MOD_BROADCOM_SAND64:
+		case DRM_FORMAT_MOD_BROADCOM_SAND128:
+		case DRM_FORMAT_MOD_BROADCOM_SAND256:
+			return true;
+		default:
+			return false;
+		}
+	case DRM_FORMAT_YUV422:
+	case DRM_FORMAT_YVU422:
 	case DRM_FORMAT_NV12:
 	case DRM_FORMAT_NV16:
 	default:
@@ -954,6 +1021,9 @@  struct drm_plane *vc4_plane_init(struct drm_device *dev,
 	unsigned i;
 	static const uint64_t modifiers[] = {
 		DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED,
+		DRM_FORMAT_MOD_BROADCOM_SAND128,
+		DRM_FORMAT_MOD_BROADCOM_SAND64,
+		DRM_FORMAT_MOD_BROADCOM_SAND256,
 		DRM_FORMAT_MOD_LINEAR,
 		DRM_FORMAT_MOD_INVALID
 	};
diff --git a/drivers/gpu/drm/vc4/vc4_regs.h b/drivers/gpu/drm/vc4/vc4_regs.h
index ce8bb7486456..00d2d90069b0 100644
--- a/drivers/gpu/drm/vc4/vc4_regs.h
+++ b/drivers/gpu/drm/vc4/vc4_regs.h
@@ -1029,6 +1029,12 @@  enum hvs_pixel_format {
 #define SCALER_SRC_PITCH_MASK			VC4_MASK(15, 0)
 #define SCALER_SRC_PITCH_SHIFT			0
 
+/* PITCH0/1/2 fields for tiled (SAND). */
+#define SCALER_TILE_SKIP_0_MASK			VC4_MASK(18, 16)
+#define SCALER_TILE_SKIP_0_SHIFT		16
+#define SCALER_TILE_HEIGHT_MASK			VC4_MASK(15, 0)
+#define SCALER_TILE_HEIGHT_SHIFT		0
+
 /* PITCH0 fields for T-tiled. */
 #define SCALER_PITCH0_TILE_WIDTH_L_MASK		VC4_MASK(22, 16)
 #define SCALER_PITCH0_TILE_WIDTH_L_SHIFT	16
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index e04613d30a13..f6b7aab3dab1 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -384,6 +384,23 @@  extern "C" {
 #define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_THIRTYTWO_GOB \
 	fourcc_mod_code(NVIDIA, 0x15)
 
+/*
+ * Some Broadcom modifiers take parameters, for example the number of
+ * vertical lines in the image. Reserve the lower 32 bits for modifier
+ * type, and the next 24 bits for parameters. Top 8 bits are the
+ * vendor code.
+ */
+#define __fourcc_mod_broadcom_param_shift 32
+#define __fourcc_mod_broadcom_param_bits 24
+#define fourcc_mod_broadcom_code(val, params) \
+	fourcc_mod_code(BROADCOM, ((((__u64)params) << __fourcc_mod_broadcom_param_shift) | val))
+#define fourcc_mod_broadcom_param(m) \
+	((int)(((m) >> __fourcc_mod_broadcom_param_shift) &	\
+	       ((1ULL << __fourcc_mod_broadcom_param_bits) - 1)))
+#define fourcc_mod_broadcom_mod(m) \
+	((m) & ~(((1ULL << __fourcc_mod_broadcom_param_shift) - 1) <<	\
+		 __fourcc_mod_broadcom_param_bits))
+
 /*
  * Broadcom VC4 "T" format
  *
@@ -405,6 +422,48 @@  extern "C" {
  */
 #define DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED fourcc_mod_code(BROADCOM, 1)
 
+/*
+ * Broadcom SAND format
+ *
+ * This is the native format that the H.264 codec block uses.  For VC4
+ * HVS, it is only valid for H.264 (NV12/21) and RGBA modes.
+ *
+ * The image can be considered to be split into columns, and the
+ * columns are placed consecutively into memory.  The width of those
+ * columns can be either 32, 64, 128, or 256 pixels, but in practice
+ * only 128 pixel columns are used.
+ *
+ * The pitch between the start of each column is set to optimally
+ * switch between SDRAM banks. This is passed as the number of lines
+ * of column width in the modifier (we can't use the stride value due
+ * to various core checks that look at it , so you should set the
+ * stride to width*cpp).
+ *
+ * Note that the column height for this format modifier is the same
+ * for all of the planes, assuming that each column contains both Y
+ * and UV.  Some SAND-using hardware stores UV in a separate tiled
+ * image from Y to reduce the column height, which is not supported
+ * with these modifiers.
+ */
+
+#define DRM_FORMAT_MOD_BROADCOM_SAND32_COL_HEIGHT(v) \
+	fourcc_mod_broadcom_code(2, v)
+#define DRM_FORMAT_MOD_BROADCOM_SAND64_COL_HEIGHT(v) \
+	fourcc_mod_broadcom_code(3, v)
+#define DRM_FORMAT_MOD_BROADCOM_SAND128_COL_HEIGHT(v) \
+	fourcc_mod_broadcom_code(4, v)
+#define DRM_FORMAT_MOD_BROADCOM_SAND256_COL_HEIGHT(v) \
+	fourcc_mod_broadcom_code(5, v)
+
+#define DRM_FORMAT_MOD_BROADCOM_SAND32 \
+	DRM_FORMAT_MOD_BROADCOM_SAND32_COL_HEIGHT(0)
+#define DRM_FORMAT_MOD_BROADCOM_SAND64 \
+	DRM_FORMAT_MOD_BROADCOM_SAND64_COL_HEIGHT(0)
+#define DRM_FORMAT_MOD_BROADCOM_SAND128 \
+	DRM_FORMAT_MOD_BROADCOM_SAND128_COL_HEIGHT(0)
+#define DRM_FORMAT_MOD_BROADCOM_SAND256 \
+	DRM_FORMAT_MOD_BROADCOM_SAND256_COL_HEIGHT(0)
+
 #if defined(__cplusplus)
 }
 #endif