Nouveau - Aug 2017 - [PATCH 0/4] Overlay / format improvements

This was all kicked off by me figuring out how the PPC byteswap thing worked.
In the end, we're keeping the implicit byteswap based on architecture, but
also
expose the correctly supported formats, and fix some overlay details.

Overlay framebuffers have various funky requirements that should be enforced at
framebuffer creation time (these are set apart by their YUV formats). Further,
I simplify some error-checking details for both nv4 and nv10+ framebuffers as
well as using the pitch properly instead of making wild assumptions (how did
that
every work... not sure).

This has seen some modest modetest testing on NV5, NV17, NV34, and NV4A. I
noticed
some odd issues on NV5 with certain scaling factors, but I'm somewhat sure
that
those issues were there before - either the hw can't keep up or we're
not twiddling
some clocks.

Ilia Mirkin (4):
  drm/nouveau/display: prevent undisplayable framebuffers from creation
  drm/nouveau/overlay: improve error detection, fix pitch setting
  drm/nouveau/overlay: add NV21 support
  drm/nouveau/dispnv04: fix exposed format list

 drivers/gpu/drm/nouveau/dispnv04/crtc.c    | 36 ++++++++++++++-
 drivers/gpu/drm/nouveau/dispnv04/overlay.c | 71 +++++++++++++++++-------------
 drivers/gpu/drm/nouveau/nouveau_display.c  | 21 +++++++++
 3 files changed, 96 insertions(+), 32 deletions(-)

-- 
2.13.0

Pre-nv50 YUV overlays have stringent requirements for working with the
internal machinery. Instead of rejecting these at update_plane time, we
should instead prevent the framebuffers from being created in the first
place.

Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
---
 drivers/gpu/drm/nouveau/nouveau_display.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c
b/drivers/gpu/drm/nouveau/nouveau_display.c
index 549763f5e17d..99ae77f66177 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -256,9 +256,30 @@ nouveau_framebuffer_new(struct drm_device *dev,
 			struct nouveau_bo *nvbo,
 			struct nouveau_framebuffer **pfb)
 {
+	struct nouveau_drm *drm = nouveau_drm(dev);
 	struct nouveau_framebuffer *fb;
 	int ret;
 
+        /* YUV overlays have special requirements pre-NV50 */
+	if (drm->client.device.info.family < NV_DEVICE_INFO_V0_TESLA &&
+
+	    (mode_cmd->pixel_format == DRM_FORMAT_YUYV ||
+	     mode_cmd->pixel_format == DRM_FORMAT_UYVY ||
+	     mode_cmd->pixel_format == DRM_FORMAT_NV12 ||
+	     mode_cmd->pixel_format == DRM_FORMAT_NV21) &&
+	    (mode_cmd->pitches[0] & 0x3f || /* align 64 */
+	     mode_cmd->pitches[0] >= 0x10000 || /* at most 64k pitch */
+	     (mode_cmd->pitches[1] && /* pitches for planes must match */
+	      mode_cmd->pitches[0] != mode_cmd->pitches[1]))) {
+		struct drm_format_name_buf format_name;
+		DRM_DEBUG_KMS("Unsuitable framebuffer: format: %s; pitches: 0x%x\n
0x%x\n",
+			      drm_get_format_name(mode_cmd->pixel_format,
+						  &format_name),
+			      mode_cmd->pitches[0],
+			      mode_cmd->pitches[1]);
+		return -EINVAL;
+	}
+
 	if (!(fb = *pfb = kzalloc(sizeof(*fb), GFP_KERNEL)))
 		return -ENOMEM;
 
-- 
2.13.0

We were previously setting the pitch based on a perfectly packed buffer.
This does not necessarily happen. Either modetest started generating
such buffers recently, or earlier testing only happened with well-picked
overlay sizes.

While we're at it, beef up and refactor the error state detection.

Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
---
 drivers/gpu/drm/nouveau/dispnv04/overlay.c | 62 ++++++++++++++++--------------
 1 file changed, 34 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
index e54944d23268..5bd63c2f14a6 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
@@ -90,6 +90,26 @@ cos_mul(int degrees, int factor)
 }
 
 static int
+verify_scaling(const struct drm_framebuffer *fb, uint8_t shift,
+               uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h,
+               uint32_t crtc_w, uint32_t crtc_h)
+{
+	if (crtc_w < (src_w >> shift) || crtc_h < (src_h >> shift))
{
+		DRM_DEBUG_KMS("Unsuitable framebuffer scaling: %dx%d ->
%dx%d\n",
+			      src_w, src_h, crtc_w, crtc_h);
+		return -ERANGE;
+	}
+
+	if (src_x != 0 || src_y != 0) {
+		DRM_DEBUG_KMS("Unsuitable framebuffer offset: %d,%d\n",
+                              src_x, src_y);
+		return -ERANGE;
+	}
+
+	return 0;
+}
+
+static int
 nv10_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 		  struct drm_framebuffer *fb, int crtc_x, int crtc_y,
 		  unsigned int crtc_w, unsigned int crtc_h,
@@ -107,7 +127,9 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
 	bool flip = nv_plane->flip;
 	int soff = NV_PCRTC0_SIZE * nv_crtc->index;
 	int soff2 = NV_PCRTC0_SIZE * !nv_crtc->index;
-	int format, ret;
+	unsigned shift = drm->client.device.info.chipset >= 0x30 ? 1 : 3;
+	unsigned format = 0;
+	int ret;
 
 	/* Source parameters given in 16.16 fixed point, ignore fractional. */
 	src_x >>= 16;
@@ -115,18 +137,9 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
 	src_w >>= 16;
 	src_h >>= 16;
 
-	format = ALIGN(src_w * 4, 0x100);
-
-	if (format > 0xffff)
-		return -ERANGE;
-
-	if (drm->client.device.info.chipset >= 0x30) {
-		if (crtc_w < (src_w >> 1) || crtc_h < (src_h >> 1))
-			return -ERANGE;
-	} else {
-		if (crtc_w < (src_w >> 3) || crtc_h < (src_h >> 3))
-			return -ERANGE;
-	}
+	ret = verify_scaling(fb, shift, 0, 0, src_w, src_h, crtc_w, crtc_h);
+	if (ret)
+		return ret;
 
 	ret = nouveau_bo_pin(nv_fb->nvbo, TTM_PL_FLAG_VRAM, false);
 	if (ret)
@@ -160,7 +173,7 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
 		nvif_wr32(dev, NV_PVIDEO_UVPLANE_OFFSET_BUFF(flip),
 			nv_fb->nvbo->bo.offset + fb->offsets[1]);
 	}
-	nvif_wr32(dev, NV_PVIDEO_FORMAT(flip), format);
+	nvif_wr32(dev, NV_PVIDEO_FORMAT(flip), format | fb->pitches[0]);
 	nvif_wr32(dev, NV_PVIDEO_STOP, 0);
 	/* TODO: wait for vblank? */
 	nvif_wr32(dev, NV_PVIDEO_BUFFER, flip ? 0x10 : 0x1);
@@ -357,7 +370,7 @@ nv04_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
 	struct nouveau_bo *cur = nv_plane->cur;
 	uint32_t overlay = 1;
 	int brightness = (nv_plane->brightness - 512) * 62 / 512;
-	int pitch, ret, i;
+	int ret, i;
 
 	/* Source parameters given in 16.16 fixed point, ignore fractional. */
 	src_x >>= 16;
@@ -365,17 +378,9 @@ nv04_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
 	src_w >>= 16;
 	src_h >>= 16;
 
-	pitch = ALIGN(src_w * 4, 0x100);
-
-	if (pitch > 0xffff)
-		return -ERANGE;
-
-	/* TODO: Compute an offset? Not sure how to do this for YUYV. */
-	if (src_x != 0 || src_y != 0)
-		return -ERANGE;
-
-	if (crtc_w < src_w || crtc_h < src_h)
-		return -ERANGE;
+	ret = verify_scaling(fb, 0, src_x, src_y, src_w, src_h, crtc_w, crtc_h);
+	if (ret)
+		return ret;
 
 	ret = nouveau_bo_pin(nv_fb->nvbo, TTM_PL_FLAG_VRAM, false);
 	if (ret)
@@ -389,8 +394,9 @@ nv04_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
 
 	for (i = 0; i < 2; i++) {
 		nvif_wr32(dev, NV_PVIDEO_BUFF0_START_ADDRESS + 4 * i,
-			nv_fb->nvbo->bo.offset);
-		nvif_wr32(dev, NV_PVIDEO_BUFF0_PITCH_LENGTH + 4 * i, pitch);
+			  nv_fb->nvbo->bo.offset);
+		nvif_wr32(dev, NV_PVIDEO_BUFF0_PITCH_LENGTH + 4 * i,
+			  fb->pitches[0]);
 		nvif_wr32(dev, NV_PVIDEO_BUFF0_OFFSET + 4 * i, 0);
 	}
 	nvif_wr32(dev, NV_PVIDEO_WINDOW_START, crtc_y << 16 | crtc_x);
-- 
2.13.0

Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
---
 drivers/gpu/drm/nouveau/dispnv04/overlay.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
index 5bd63c2f14a6..c8c2333f24ee 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
@@ -63,6 +63,7 @@ static uint32_t formats[] = {
 	DRM_FORMAT_YUYV,
 	DRM_FORMAT_UYVY,
 	DRM_FORMAT_NV12,
+	DRM_FORMAT_NV21,
 };
 
 /* Sine can be approximated with
@@ -159,16 +160,18 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
 	nvif_wr32(dev, NV_PVIDEO_POINT_OUT(flip), crtc_y << 16 | crtc_x);
 	nvif_wr32(dev, NV_PVIDEO_SIZE_OUT(flip), crtc_h << 16 | crtc_w);
 
-	if (fb->format->format != DRM_FORMAT_UYVY)
+	if (fb->format->format == DRM_FORMAT_YUYV ||
+	    fb->format->format == DRM_FORMAT_NV12)
 		format |= NV_PVIDEO_FORMAT_COLOR_LE_CR8YB8CB8YA8;
-	if (fb->format->format == DRM_FORMAT_NV12)
+	if (fb->format->format == DRM_FORMAT_NV12 ||
+	    fb->format->format == DRM_FORMAT_NV21)
 		format |= NV_PVIDEO_FORMAT_PLANAR;
 	if (nv_plane->iturbt_709)
 		format |= NV_PVIDEO_FORMAT_MATRIX_ITURBT709;
 	if (nv_plane->colorkey & (1 << 24))
 		format |= NV_PVIDEO_FORMAT_DISPLAY_COLOR_KEY;
 
-	if (fb->format->format == DRM_FORMAT_NV12) {
+	if (format & NV_PVIDEO_FORMAT_PLANAR) {
 		nvif_wr32(dev, NV_PVIDEO_UVPLANE_BASE(flip), 0);
 		nvif_wr32(dev, NV_PVIDEO_UVPLANE_OFFSET_BUFF(flip),
 			nv_fb->nvbo->bo.offset + fb->offsets[1]);
-- 
2.13.0

drm_crtc_init exposes the XRGB8888 and ARGB8888 formats. In actuality,
ARGB8888's 32-bit depth messes up some formulas that weren't meant for
it, and the alpha is fairly meaningless for the primary plane.

The modesetting logic appears to be fully prepared for RGB565 as well as
XRGB1555 however, as tested with modetest.

Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
---
 drivers/gpu/drm/nouveau/dispnv04/crtc.c | 36 ++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 4b4b0b496262..c078811b4e11 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -1099,6 +1099,38 @@ static const struct drm_crtc_helper_funcs
nv04_crtc_helper_funcs = {
 	.disable = nv_crtc_disable,
 };
 
+static const uint32_t modeset_formats[] = {
+        DRM_FORMAT_XRGB8888,
+        DRM_FORMAT_RGB565,
+        DRM_FORMAT_XRGB1555,
+};
+
+static struct drm_plane *
+create_primary_plane(struct drm_device *dev)
+{
+        struct drm_plane *primary;
+        int ret;
+
+        primary = kzalloc(sizeof(*primary), GFP_KERNEL);
+        if (primary == NULL) {
+                DRM_DEBUG_KMS("Failed to allocate primary plane\n");
+                return NULL;
+        }
+
+        /* possible_crtc's will be filled in later by crtc_init */
+        ret = drm_universal_plane_init(dev, primary, 0,
+                                       &drm_primary_helper_funcs,
+                                       modeset_formats,
+                                       ARRAY_SIZE(modeset_formats),
+                                       DRM_PLANE_TYPE_PRIMARY, NULL);
+        if (ret) {
+                kfree(primary);
+                primary = NULL;
+        }
+
+        return primary;
+}
+
 int
 nv04_crtc_create(struct drm_device *dev, int crtc_num)
 {
@@ -1122,7 +1154,9 @@ nv04_crtc_create(struct drm_device *dev, int crtc_num)
 	nv_crtc->save = nv_crtc_save;
 	nv_crtc->restore = nv_crtc_restore;
 
-	drm_crtc_init(dev, &nv_crtc->base, &nv04_crtc_funcs);
+	drm_crtc_init_with_planes(dev, &nv_crtc->base,
+                                  create_primary_plane(dev), NULL,
+                                  &nv04_crtc_funcs, NULL);
 	drm_crtc_helper_add(&nv_crtc->base, &nv04_crtc_helper_funcs);
 	drm_mode_crtc_set_gamma_size(&nv_crtc->base, 256);
 
-- 
2.13.0