[PATCH 0/8] drm/exynos: misc fixes and more

[PATCH 0/8] drm/exynos: misc fixes and more

[Tobias Jakobi]

Hello,

this is basically a resend of [1] with some minor changes to the commit
messages, the RFC tag removed, and a rebase on vanilla 4.13-rc4.

Summary:
(a) Enables support for NV12MT in the mixer.
(b) Sanitizes buffer pitch for HW with restrictions.
(c) Misc fixes

With best wishes,
Tobias


[1] https://lists.freedesktop.org/archives/dri-devel/2017-April/138040.html

Tobias Jakobi (8):
  drm/exynos: mixer: fix chroma comment in vp_video_buffer()
  drm/exynos: mixer: enable NV12MT support for the video plane
  drm/exynos: mixer: simplify {vp_video,mixer_graph}_buffer()
  drm/exynos: mixer: remove src offset from mixer_graph_buffer()
  drm/exynos: introduce BYTE_PITCH capability
  drm/exynos: add BYTE_PITCH cap for all supported planes
  drm/exynos: consistent use of cpp
  drm/exynos: simplify set_pixfmt() in DECON and FIMD drivers

 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 17 +++++------
 drivers/gpu/drm/exynos/exynos7_drm_decon.c    | 13 +++-----
 drivers/gpu/drm/exynos/exynos_drm_drv.h       |  2 ++
 drivers/gpu/drm/exynos/exynos_drm_fb.c        |  2 ++
 drivers/gpu/drm/exynos/exynos_drm_fimd.c      | 17 ++++-------
 drivers/gpu/drm/exynos/exynos_drm_plane.c     | 37 ++++++++++++++++++++++
 drivers/gpu/drm/exynos/exynos_mixer.c         | 44 ++++++++-------------------
 7 files changed, 71 insertions(+), 61 deletions(-)

-- 
2.13.0

[PATCH 1/8] drm/exynos: mixer: fix chroma comment in vp_video_buffer()

[Tobias Jakobi]

The current comment sounds like the division op is done to
compensate for some hardware erratum. But the chroma plane
having half the height of the luma plane is just the way
NV12/NV21 is defined, so clarify this behaviour.

Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index a998a8dd783c..c6d6f6d42900 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -532,7 +532,7 @@ static void vp_video_buffer(struct mixer_context *ctx,
 	/* setting size of input image */
 	vp_reg_write(res, VP_IMG_SIZE_Y, VP_IMG_HSIZE(fb->pitches[0]) |
 		VP_IMG_VSIZE(fb->height));
-	/* chroma height has to reduced by 2 to avoid chroma distorions */
+	/* the chroma plane for NV12/NV21 is half the height of the luma plane */
 	vp_reg_write(res, VP_IMG_SIZE_C, VP_IMG_HSIZE(fb->pitches[0]) |
 		VP_IMG_VSIZE(fb->height / 2));
 
-- 
2.13.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/8] drm/exynos: mixer: enable NV12MT support for the video plane

[Tobias Jakobi]

The video processor supports a tiled version of the NV12 format,
known as NV12MT in V4L2 terms. The support was removed in commit
083500baefd5f4c215a5a93aef2492c1aa775828 due to not being a real
pixel format, but rather NV12 with a special memory layout.

With the introduction of FB modifiers, we can now properly support
this format again.

Tested with a hacked up modetest from libdrm's test suite on
an ODROID-X2 (Exynos4412).

Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.h   |  1 +
 drivers/gpu/drm/exynos/exynos_drm_fb.c    |  2 ++
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 27 +++++++++++++++++++++++++++
 drivers/gpu/drm/exynos/exynos_mixer.c     |  6 +++++-
 4 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index a93de321706b..43afab4bebc3 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -91,6 +91,7 @@ struct exynos_drm_plane {
 #define EXYNOS_DRM_PLANE_CAP_DOUBLE	(1 << 0)
 #define EXYNOS_DRM_PLANE_CAP_SCALE	(1 << 1)
 #define EXYNOS_DRM_PLANE_CAP_ZPOS	(1 << 2)
+#define EXYNOS_DRM_PLANE_CAP_TILE	(1 << 3)
 
 /*
  * Exynos DRM plane configuration structure.
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
index d48fd7c918f8..87e32dbd8eda 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
@@ -238,4 +238,6 @@ void exynos_drm_mode_config_init(struct drm_device *dev)
 
 	dev->mode_config.funcs = &exynos_drm_mode_config_funcs;
 	dev->mode_config.helper_private = &exynos_drm_mode_config_helpers;
+
+	dev->mode_config.allow_fb_modifiers = true;
 }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index 611b6fd65433..bd3825617b06 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -180,6 +180,29 @@ static struct drm_plane_funcs exynos_plane_funcs = {
 };
 
 static int
+exynos_drm_plane_check_format(const struct exynos_drm_plane_config *config,
+			      struct exynos_drm_plane_state *state)
+{
+	struct drm_framebuffer *fb = state->base.fb;
+
+	switch (fb->modifier) {
+	case DRM_FORMAT_MOD_SAMSUNG_64_32_TILE:
+		if (!(config->capabilities & EXYNOS_DRM_PLANE_CAP_TILE))
+			return -ENOTSUPP;
+		break;
+
+	case 0:
+		break;
+
+	default:
+		DRM_ERROR("unsupported pixel format modifier");
+		return -ENOTSUPP;
+	}
+
+	return 0;
+}
+
+static int
 exynos_drm_plane_check_size(const struct exynos_drm_plane_config *config,
 			    struct exynos_drm_plane_state *state)
 {
@@ -223,6 +246,10 @@ static int exynos_plane_atomic_check(struct drm_plane *plane,
 	/* translate state into exynos_state */
 	exynos_plane_mode_set(exynos_state);
 
+	ret = exynos_drm_plane_check_format(exynos_plane->config, exynos_state);
+	if (ret)
+		return ret;
+
 	ret = exynos_drm_plane_check_size(exynos_plane->config, exynos_state);
 	return ret;
 }
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index c6d6f6d42900..4c894d97aba3 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -148,7 +148,8 @@ static const struct exynos_drm_plane_config plane_configs[MIXER_WIN_NR] = {
 		.pixel_formats = vp_formats,
 		.num_pixel_formats = ARRAY_SIZE(vp_formats),
 		.capabilities = EXYNOS_DRM_PLANE_CAP_SCALE |
-				EXYNOS_DRM_PLANE_CAP_ZPOS,
+				EXYNOS_DRM_PLANE_CAP_ZPOS |
+				EXYNOS_DRM_PLANE_CAP_TILE,
 	},
 };
 
@@ -500,6 +501,9 @@ static void vp_video_buffer(struct mixer_context *ctx,
 		return;
 	}
 
+	if (fb->modifier == DRM_FORMAT_MOD_SAMSUNG_64_32_TILE)
+		tiled_mode = true;
+
 	luma_addr[0] = exynos_drm_fb_dma_addr(fb, 0);
 	chroma_addr[0] = exynos_drm_fb_dma_addr(fb, 1);
 
-- 
2.13.0

[PATCH 3/8] drm/exynos: mixer: simplify {vp_video, mixer_graph}_buffer()

[Tobias Jakobi]

DRM core already checks the validity of the pixelformats, so we
can simplify the checks here. The same applies to the FB modifier,
which is now checked in common Exynos plane code.

Also rename the booleans to reflect what true/false actually
means.

Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 31 +++++++------------------------
 1 file changed, 7 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 4c894d97aba3..8d68de85bada 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -484,32 +484,18 @@ static void vp_video_buffer(struct mixer_context *ctx,
 	unsigned int priority = state->base.normalized_zpos + 1;
 	unsigned long flags;
 	dma_addr_t luma_addr[2], chroma_addr[2];
-	bool tiled_mode = false;
-	bool crcb_mode = false;
+	bool is_tiled, is_nv21;
 	u32 val;
 
-	switch (fb->format->format) {
-	case DRM_FORMAT_NV12:
-		crcb_mode = false;
-		break;
-	case DRM_FORMAT_NV21:
-		crcb_mode = true;
-		break;
-	default:
-		DRM_ERROR("pixel format for vp is wrong [%d].\n",
-				fb->format->format);
-		return;
-	}
-
-	if (fb->modifier == DRM_FORMAT_MOD_SAMSUNG_64_32_TILE)
-		tiled_mode = true;
+	is_nv21 = (fb->format->format == DRM_FORMAT_NV21);
+	is_tiled = (fb->modifier == DRM_FORMAT_MOD_SAMSUNG_64_32_TILE);
 
 	luma_addr[0] = exynos_drm_fb_dma_addr(fb, 0);
 	chroma_addr[0] = exynos_drm_fb_dma_addr(fb, 1);
 
 	if (mode->flags & DRM_MODE_FLAG_INTERLACE) {
 		__set_bit(MXR_BIT_INTERLACE, &ctx->flags);
-		if (tiled_mode) {
+		if (is_tiled) {
 			luma_addr[1] = luma_addr[0] + 0x40;
 			chroma_addr[1] = chroma_addr[0] + 0x40;
 		} else {
@@ -529,8 +515,8 @@ static void vp_video_buffer(struct mixer_context *ctx,
 	vp_reg_writemask(res, VP_MODE, val, VP_MODE_LINE_SKIP);
 
 	/* setup format */
-	val = (crcb_mode ? VP_MODE_NV21 : VP_MODE_NV12);
-	val |= (tiled_mode ? VP_MODE_MEM_TILED : VP_MODE_MEM_LINEAR);
+	val = (is_nv21 ? VP_MODE_NV21 : VP_MODE_NV12);
+	val |= (is_tiled ? VP_MODE_MEM_TILED : VP_MODE_MEM_LINEAR);
 	vp_reg_writemask(res, VP_MODE, val, VP_MODE_FMT_MASK);
 
 	/* setting size of input image */
@@ -620,12 +606,9 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
 
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_ARGB8888:
+	default:
 		fmt = MXR_FORMAT_ARGB8888;
 		break;
-
-	default:
-		DRM_DEBUG_KMS("pixelformat unsupported by mixer\n");
-		return;
 	}
 
 	/* ratio is already checked by common plane code */
-- 
2.13.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/8] drm/exynos: mixer: remove src offset from mixer_graph_buffer()

[Tobias Jakobi]

We always translate the dma address such that the offsets of
the source image are zero. Hence we can remove manipulation of
the MXR_GRAPHIC_SXY(win) register.

We leave the register defines (in regs_mixer.h) in place, since
they document the hardware.

Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 8d68de85bada..28fbe79befff 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -584,7 +584,7 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
 	unsigned long flags;
 	unsigned int win = plane->index;
 	unsigned int x_ratio = 0, y_ratio = 0;
-	unsigned int src_x_offset, src_y_offset, dst_x_offset, dst_y_offset;
+	unsigned int dst_x_offset, dst_y_offset;
 	dma_addr_t dma_addr;
 	unsigned int fmt;
 	u32 val;
@@ -618,12 +618,10 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
 	dst_x_offset = state->crtc.x;
 	dst_y_offset = state->crtc.y;
 
-	/* converting dma address base and source offset */
+	/* translate dma address base s.t. the source image offset is zero */
 	dma_addr = exynos_drm_fb_dma_addr(fb, 0)
 		+ (state->src.x * fb->format->cpp[0])
 		+ (state->src.y * fb->pitches[0]);
-	src_x_offset = 0;
-	src_y_offset = 0;
 
 	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
 		__set_bit(MXR_BIT_INTERLACE, &ctx->flags);
@@ -654,11 +652,6 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
 	val |= MXR_GRP_WH_V_SCALE(y_ratio);
 	mixer_reg_write(res, MXR_GRAPHIC_WH(win), val);
 
-	/* setup offsets in source image */
-	val  = MXR_GRP_SXY_SX(src_x_offset);
-	val |= MXR_GRP_SXY_SY(src_y_offset);
-	mixer_reg_write(res, MXR_GRAPHIC_SXY(win), val);
-
 	/* setup offsets in display image */
 	val  = MXR_GRP_DXY_DX(dst_x_offset);
 	val |= MXR_GRP_DXY_DY(dst_y_offset);
-- 
2.13.0

[PATCH 5/8] drm/exynos: introduce BYTE_PITCH capability

[Tobias Jakobi]

In some of drivers we compute something like 'pitch / cpp' at some
point, silently assuming that the pitch (which is in bytes) is
divisible by the buffer's cpp. This is not always true, in particular
DRM core does not check for pitch alignment in the common case.

Introduce a new cap which indicates that the hardware supports a
pitch with 'byte-granularity'. If the cap is not set, assume that
we need pitch aligned to cpp.

We set this cap later for the drivers/planes that support it.

Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.h   |  1 +
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 43afab4bebc3..ec32632485d2 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -92,6 +92,7 @@ struct exynos_drm_plane {
 #define EXYNOS_DRM_PLANE_CAP_SCALE	(1 << 1)
 #define EXYNOS_DRM_PLANE_CAP_ZPOS	(1 << 2)
 #define EXYNOS_DRM_PLANE_CAP_TILE	(1 << 3)
+#define EXYNOS_DRM_PLANE_CAP_BYTE_PITCH	(1 << 4)
 
 /*
  * Exynos DRM plane configuration structure.
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index bd3825617b06..734d5ba4eb99 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -185,6 +185,16 @@ exynos_drm_plane_check_format(const struct exynos_drm_plane_config *config,
 {
 	struct drm_framebuffer *fb = state->base.fb;
 
+	/*
+	 * Some blocks only allow to specify a buffer pitch in terms
+	 * of pixels. In these cases, we need to ensure that the pitch
+	 * provided by userspace is divisible by the cpp.
+	 */
+	if (!(config->capabilities & EXYNOS_DRM_PLANE_CAP_BYTE_PITCH)) {
+		if (fb->pitches[0] % fb->format->cpp[0])
+			return -ENOTSUPP;
+	}
+
 	switch (fb->modifier) {
 	case DRM_FORMAT_MOD_SAMSUNG_64_32_TILE:
 		if (!(config->capabilities & EXYNOS_DRM_PLANE_CAP_TILE))
-- 
2.13.0

[PATCH 6/8] drm/exynos: add BYTE_PITCH cap for all supported planes

[Tobias Jakobi]

Both FIMD and DECON5433 support a pitch in bytes.

Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 1 +
 drivers/gpu/drm/exynos/exynos_drm_fimd.c      | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 5792ca88ab7a..62b50e0685b0 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -546,6 +546,7 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
 		ctx->configs[win].num_pixel_formats = ARRAY_SIZE(decon_formats);
 		ctx->configs[win].zpos = win;
 		ctx->configs[win].type = decon_win_types[tmp];
+		ctx->configs[win].capabilities = EXYNOS_DRM_PLANE_CAP_BYTE_PITCH;
 
 		ret = exynos_plane_init(drm_dev, &ctx->planes[win], win,
 					&ctx->configs[win]);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 60f93cad6643..e88597f6d356 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -986,6 +986,7 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
 		ctx->configs[i].num_pixel_formats = ARRAY_SIZE(fimd_formats);
 		ctx->configs[i].zpos = i;
 		ctx->configs[i].type = fimd_win_types[i];
+		ctx->configs[i].capabilities = EXYNOS_DRM_PLANE_CAP_BYTE_PITCH;
 		ret = exynos_plane_init(drm_dev, &ctx->planes[i], i,
 					&ctx->configs[i]);
 		if (ret)
-- 
2.13.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 7/8] drm/exynos: consistent use of cpp

[Tobias Jakobi]

A recent commit (272725c7db4da1fd3229d944fc76d2e98e3a144e) has removed
the use of 'bits_per_pixel' in DRM. However the corresponding Exynos
driver code still uses the ambiguous 'bpp', even though it is now
initialized from fb->cpp[0].

Consistenly use 'cpp' in FIMD, DECON7 and DECON5433 drivers.

Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 12 ++++++------
 drivers/gpu/drm/exynos/exynos7_drm_decon.c    |  6 +++---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c      |  8 ++++----
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 62b50e0685b0..66ceca0af2a0 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -286,7 +286,7 @@ static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win,
 		return;
 	}
 
-	DRM_DEBUG_KMS("bpp = %u\n", fb->format->cpp[0] * 8);
+	DRM_DEBUG_KMS("cpp = %u\n", fb->format->cpp[0]);
 
 	/*
 	 * In case of exynos, setting dma-burst to 16Word causes permanent
@@ -329,7 +329,7 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
 	struct decon_context *ctx = crtc->ctx;
 	struct drm_framebuffer *fb = state->base.fb;
 	unsigned int win = plane->index;
-	unsigned int bpp = fb->format->cpp[0];
+	unsigned int cpp = fb->format->cpp[0];
 	unsigned int pitch = fb->pitches[0];
 	dma_addr_t dma_addr = exynos_drm_fb_dma_addr(fb, 0);
 	u32 val;
@@ -365,11 +365,11 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
 	writel(val, ctx->addr + DECON_VIDW0xADD1B0(win));
 
 	if (!(ctx->out_type & IFTYPE_HDMI))
-		val = BIT_VAL(pitch - state->crtc.w * bpp, 27, 14)
-			| BIT_VAL(state->crtc.w * bpp, 13, 0);
+		val = BIT_VAL(pitch - state->crtc.w * cpp, 27, 14)
+			| BIT_VAL(state->crtc.w * cpp, 13, 0);
 	else
-		val = BIT_VAL(pitch - state->crtc.w * bpp, 29, 15)
-			| BIT_VAL(state->crtc.w * bpp, 14, 0);
+		val = BIT_VAL(pitch - state->crtc.w * cpp, 29, 15)
+			| BIT_VAL(state->crtc.w * cpp, 14, 0);
 	writel(val, ctx->addr + DECON_VIDW0xADD2(win));
 
 	decon_win_set_pixfmt(ctx, win, fb);
diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index 3e88269fdc2e..4662d55ed988 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -321,7 +321,7 @@ static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win,
 		break;
 	}
 
-	DRM_DEBUG_KMS("bpp = %d\n", fb->format->cpp[0] * 8);
+	DRM_DEBUG_KMS("cpp = %d\n", fb->format->cpp[0]);
 
 	/*
 	 * In case of exynos, setting dma-burst to 16Word causes permanent
@@ -398,7 +398,7 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
 	unsigned int last_x;
 	unsigned int last_y;
 	unsigned int win = plane->index;
-	unsigned int bpp = fb->format->cpp[0];
+	unsigned int cpp = fb->format->cpp[0];
 	unsigned int pitch = fb->pitches[0];
 
 	if (ctx->suspended)
@@ -418,7 +418,7 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
 	val = (unsigned long)exynos_drm_fb_dma_addr(fb, 0);
 	writel(val, ctx->regs + VIDW_BUF_START(win));
 
-	padding = (pitch / bpp) - fb->width;
+	padding = (pitch / cpp) - fb->width;
 
 	/* buffer size */
 	writel(fb->width + padding, ctx->regs + VIDW_WHOLE_X(win));
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index e88597f6d356..9b83d1846589 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -718,13 +718,13 @@ static void fimd_update_plane(struct exynos_drm_crtc *crtc,
 	unsigned long val, size, offset;
 	unsigned int last_x, last_y, buf_offsize, line_size;
 	unsigned int win = plane->index;
-	unsigned int bpp = fb->format->cpp[0];
+	unsigned int cpp = fb->format->cpp[0];
 	unsigned int pitch = fb->pitches[0];
 
 	if (ctx->suspended)
 		return;
 
-	offset = state->src.x * bpp;
+	offset = state->src.x * cpp;
 	offset += state->src.y * pitch;
 
 	/* buffer start address */
@@ -743,8 +743,8 @@ static void fimd_update_plane(struct exynos_drm_crtc *crtc,
 			state->crtc.w, state->crtc.h);
 
 	/* buffer size */
-	buf_offsize = pitch - (state->crtc.w * bpp);
-	line_size = state->crtc.w * bpp;
+	buf_offsize = pitch - (state->crtc.w * cpp);
+	line_size = state->crtc.w * cpp;
 	val = VIDW_BUF_SIZE_OFFSET(buf_offsize) |
 		VIDW_BUF_SIZE_PAGEWIDTH(line_size) |
 		VIDW_BUF_SIZE_OFFSET_E(buf_offsize) |
-- 
2.13.0

[PATCH 8/8] drm/exynos: simplify set_pixfmt() in DECON and FIMD drivers

[Tobias Jakobi]

DRM core already checks the validity of the pixelformat.

Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 4 +---
 drivers/gpu/drm/exynos/exynos7_drm_decon.c    | 7 +------
 drivers/gpu/drm/exynos/exynos_drm_fimd.c      | 8 +-------
 3 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 66ceca0af2a0..3dfba34be24d 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -277,13 +277,11 @@ static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win,
 		val |= WINCONx_BURSTLEN_16WORD;
 		break;
 	case DRM_FORMAT_ARGB8888:
+	default:
 		val |= WINCONx_BPPMODE_32BPP_A8888;
 		val |= WINCONx_WSWP_F | WINCONx_BLD_PIX_F | WINCONx_ALPHA_SEL_F;
 		val |= WINCONx_BURSTLEN_16WORD;
 		break;
-	default:
-		DRM_ERROR("Proper pixel format is not set\n");
-		return;
 	}
 
 	DRM_DEBUG_KMS("cpp = %u\n", fb->format->cpp[0]);
diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index 4662d55ed988..615efcf7782a 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -309,16 +309,11 @@ static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win,
 		val |= WINCONx_BURSTLEN_16WORD;
 		break;
 	case DRM_FORMAT_BGRA8888:
+	default:
 		val |= WINCONx_BPPMODE_32BPP_BGRA | WINCONx_BLD_PIX |
 			WINCONx_ALPHA_SEL;
 		val |= WINCONx_BURSTLEN_16WORD;
 		break;
-	default:
-		DRM_DEBUG_KMS("invalid pixel size so using unpacked 24bpp.\n");
-
-		val |= WINCONx_BPPMODE_24BPP_xRGB;
-		val |= WINCONx_BURSTLEN_16WORD;
-		break;
 	}
 
 	DRM_DEBUG_KMS("cpp = %d\n", fb->format->cpp[0]);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 9b83d1846589..91c62e7afdd5 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -583,18 +583,12 @@ static void fimd_win_set_pixfmt(struct fimd_context *ctx, unsigned int win,
 		val |= WINCONx_BURSTLEN_16WORD;
 		break;
 	case DRM_FORMAT_ARGB8888:
+	default:
 		val |= WINCON1_BPPMODE_25BPP_A1888
 			| WINCON1_BLD_PIX | WINCON1_ALPHA_SEL;
 		val |= WINCONx_WSWP;
 		val |= WINCONx_BURSTLEN_16WORD;
 		break;
-	default:
-		DRM_DEBUG_KMS("invalid pixel size so using unpacked 24bpp.\n");
-
-		val |= WINCON0_BPPMODE_24BPP_888;
-		val |= WINCONx_WSWP;
-		val |= WINCONx_BURSTLEN_16WORD;
-		break;
 	}
 
 	/*
-- 
2.13.0

Re: [PATCH 3/8] drm/exynos: mixer: simplify {vp_video,mixer_graph}_buffer()

[Inki Dae]

2017년 08월 09일 20:48에 Tobias Jakobi 이(가) 쓴 글:
> DRM core already checks the validity of the pixelformats, so we
> can simplify the checks here. The same applies to the FB modifier,
> which is now checked in common Exynos plane code.
> 
> Also rename the booleans to reflect what true/false actually
> means.
> 
> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 31 +++++++------------------------
>  1 file changed, 7 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 4c894d97aba3..8d68de85bada 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -484,32 +484,18 @@ static void vp_video_buffer(struct mixer_context *ctx,
>  	unsigned int priority = state->base.normalized_zpos + 1;
>  	unsigned long flags;
>  	dma_addr_t luma_addr[2], chroma_addr[2];
> -	bool tiled_mode = false;
> -	bool crcb_mode = false;
> +	bool is_tiled, is_nv21;
>  	u32 val;
>  
> -	switch (fb->format->format) {
> -	case DRM_FORMAT_NV12:
> -		crcb_mode = false;
> -		break;
> -	case DRM_FORMAT_NV21:
> -		crcb_mode = true;
> -		break;
> -	default:
> -		DRM_ERROR("pixel format for vp is wrong [%d].\n",
> -				fb->format->format);
> -		return;
> -	}
> -
> -	if (fb->modifier == DRM_FORMAT_MOD_SAMSUNG_64_32_TILE)
> -		tiled_mode = true;
> +	is_nv21 = (fb->format->format == DRM_FORMAT_NV21);
> +	is_tiled = (fb->modifier == DRM_FORMAT_MOD_SAMSUNG_64_32_TILE);
>  
>  	luma_addr[0] = exynos_drm_fb_dma_addr(fb, 0);
>  	chroma_addr[0] = exynos_drm_fb_dma_addr(fb, 1);
>  
>  	if (mode->flags & DRM_MODE_FLAG_INTERLACE) {
>  		__set_bit(MXR_BIT_INTERLACE, &ctx->flags);
> -		if (tiled_mode) {
> +		if (is_tiled) {
>  			luma_addr[1] = luma_addr[0] + 0x40;
>  			chroma_addr[1] = chroma_addr[0] + 0x40;
>  		} else {
> @@ -529,8 +515,8 @@ static void vp_video_buffer(struct mixer_context *ctx,
>  	vp_reg_writemask(res, VP_MODE, val, VP_MODE_LINE_SKIP);
>  
>  	/* setup format */
> -	val = (crcb_mode ? VP_MODE_NV21 : VP_MODE_NV12);
> -	val |= (tiled_mode ? VP_MODE_MEM_TILED : VP_MODE_MEM_LINEAR);
> +	val = (is_nv21 ? VP_MODE_NV21 : VP_MODE_NV12);
> +	val |= (is_tiled ? VP_MODE_MEM_TILED : VP_MODE_MEM_LINEAR);
>  	vp_reg_writemask(res, VP_MODE, val, VP_MODE_FMT_MASK);
>  
>  	/* setting size of input image */
> @@ -620,12 +606,9 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>  
>  	case DRM_FORMAT_XRGB8888:
>  	case DRM_FORMAT_ARGB8888:
> +	default:
>  		fmt = MXR_FORMAT_ARGB8888;
>  		break;
> -
> -	default:
> -		DRM_DEBUG_KMS("pixelformat unsupported by mixer\n");
> -		return;

This change should be made as another patch because this patch changes the behavior whether it allows wrong pixel format or not.

Thanks,
Inki Dae

>  	}
>  
>  	/* ratio is already checked by common plane code */
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/8] drm/exynos: mixer: remove src offset from mixer_graph_buffer()

[Inki Dae]

2017년 08월 09일 20:48에 Tobias Jakobi 이(가) 쓴 글:
> We always translate the dma address such that the offsets of
> the source image are zero. Hence we can remove manipulation of
> the MXR_GRAPHIC_SXY(win) register.
> 
> We leave the register defines (in regs_mixer.h) in place, since
> they document the hardware.
> 
> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 8d68de85bada..28fbe79befff 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -584,7 +584,7 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>  	unsigned long flags;
>  	unsigned int win = plane->index;
>  	unsigned int x_ratio = 0, y_ratio = 0;
> -	unsigned int src_x_offset, src_y_offset, dst_x_offset, dst_y_offset;
> +	unsigned int dst_x_offset, dst_y_offset;
>  	dma_addr_t dma_addr;
>  	unsigned int fmt;
>  	u32 val;
> @@ -618,12 +618,10 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>  	dst_x_offset = state->crtc.x;
>  	dst_y_offset = state->crtc.y;
>  
> -	/* converting dma address base and source offset */
> +	/* translate dma address base s.t. the source image offset is zero */
>  	dma_addr = exynos_drm_fb_dma_addr(fb, 0)
>  		+ (state->src.x * fb->format->cpp[0])
>  		+ (state->src.y * fb->pitches[0]);
> -	src_x_offset = 0;
> -	src_y_offset = 0;
>  
>  	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
>  		__set_bit(MXR_BIT_INTERLACE, &ctx->flags);
> @@ -654,11 +652,6 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>  	val |= MXR_GRP_WH_V_SCALE(y_ratio);
>  	mixer_reg_write(res, MXR_GRAPHIC_WH(win), val);
>  
> -	/* setup offsets in source image */
> -	val  = MXR_GRP_SXY_SX(src_x_offset);
> -	val |= MXR_GRP_SXY_SY(src_y_offset);
> -	mixer_reg_write(res, MXR_GRAPHIC_SXY(win), val);

The offset of source buffer can be set at bootloader so previous value can be keeped. I think MXR_GRAPHIC_SXY register should be cleared at mixer_win_reset function if you want to remove above lines.

> -
>  	/* setup offsets in display image */
>  	val  = MXR_GRP_DXY_DX(dst_x_offset);
>  	val |= MXR_GRP_DXY_DY(dst_y_offset);
> 

Re: [PATCH 5/8] drm/exynos: introduce BYTE_PITCH capability

[Inki Dae]

2017년 08월 09일 20:48에 Tobias Jakobi 이(가) 쓴 글:
> In some of drivers we compute something like 'pitch / cpp' at some
> point, silently assuming that the pitch (which is in bytes) is
> divisible by the buffer's cpp. This is not always true, in particular
> DRM core does not check for pitch alignment in the common case.
> 
> Introduce a new cap which indicates that the hardware supports a
> pitch with 'byte-granularity'. If the cap is not set, assume that
> we need pitch aligned to cpp.
> 
> We set this cap later for the drivers/planes that support it.
> 
> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_drv.h   |  1 +
>  drivers/gpu/drm/exynos/exynos_drm_plane.c | 10 ++++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index 43afab4bebc3..ec32632485d2 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -92,6 +92,7 @@ struct exynos_drm_plane {
>  #define EXYNOS_DRM_PLANE_CAP_SCALE	(1 << 1)
>  #define EXYNOS_DRM_PLANE_CAP_ZPOS	(1 << 2)
>  #define EXYNOS_DRM_PLANE_CAP_TILE	(1 << 3)
> +#define EXYNOS_DRM_PLANE_CAP_BYTE_PITCH	(1 << 4)

I don't see why this flag is required because cpp value is given always. So I think we can check the pitch alignment regardless of a given flag.
BTW, it'd better for pitch alignment to be checked by drm core?

>  
>  /*
>   * Exynos DRM plane configuration structure.
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> index bd3825617b06..734d5ba4eb99 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> @@ -185,6 +185,16 @@ exynos_drm_plane_check_format(const struct exynos_drm_plane_config *config,
>  {
>  	struct drm_framebuffer *fb = state->base.fb;
>  
> +	/*
> +	 * Some blocks only allow to specify a buffer pitch in terms
> +	 * of pixels. In these cases, we need to ensure that the pitch
> +	 * provided by userspace is divisible by the cpp.
> +	 */
> +	if (!(config->capabilities & EXYNOS_DRM_PLANE_CAP_BYTE_PITCH)) {
> +		if (fb->pitches[0] % fb->format->cpp[0])
> +			return -ENOTSUPP;
> +	}
> +
>  	switch (fb->modifier) {
>  	case DRM_FORMAT_MOD_SAMSUNG_64_32_TILE:
>  		if (!(config->capabilities & EXYNOS_DRM_PLANE_CAP_TILE))
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/8] drm/exynos: mixer: simplify {vp_video,mixer_graph}_buffer()

[Tobias Jakobi]

Inki Dae wrote:
> 
> 
> 2017년 08월 09일 20:48에 Tobias Jakobi 이(가) 쓴 글:
>> DRM core already checks the validity of the pixelformats, so we
>> can simplify the checks here. The same applies to the FB modifier,
>> which is now checked in common Exynos plane code.
>>
>> Also rename the booleans to reflect what true/false actually
>> means.
>>
>> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
>> ---
>>  drivers/gpu/drm/exynos/exynos_mixer.c | 31 +++++++------------------------
>>  1 file changed, 7 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index 4c894d97aba3..8d68de85bada 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -484,32 +484,18 @@ static void vp_video_buffer(struct mixer_context *ctx,
>>  	unsigned int priority =tate->base.normalized_zpos + 1;
>>  	unsigned long flags;
>>  	dma_addr_t luma_addr[2], chroma_addr[2];
>> -	bool tiled_mode =alse;
>> -	bool crcb_mode =alse;
>> +	bool is_tiled, is_nv21;
>>  	u32 val;
>>  
>> -	switch (fb->format->format) {
>> -	case DRM_FORMAT_NV12:
>> -		crcb_mode =alse;
>> -		break;
>> -	case DRM_FORMAT_NV21:
>> -		crcb_mode =rue;
>> -		break;
>> -	default:
>> -		DRM_ERROR("pixel format for vp is wrong [%d].\n",
>> -				fb->format->format);
>> -		return;
>> -	}
>> -
>> -	if (fb->modifier =DRM_FORMAT_MOD_SAMSUNG_64_32_TILE)
>> -		tiled_mode =rue;
>> +	is_nv21 =fb->format->format == DRM_FORMAT_NV21);
>> +	is_tiled =fb->modifier == DRM_FORMAT_MOD_SAMSUNG_64_32_TILE);
>>  
>>  	luma_addr[0] =xynos_drm_fb_dma_addr(fb, 0);
>>  	chroma_addr[0] =xynos_drm_fb_dma_addr(fb, 1);
>>  
>>  	if (mode->flags & DRM_MODE_FLAG_INTERLACE) {
>>  		__set_bit(MXR_BIT_INTERLACE, &ctx->flags);
>> -		if (tiled_mode) {
>> +		if (is_tiled) {
>>  			luma_addr[1] =uma_addr[0] + 0x40;
>>  			chroma_addr[1] =hroma_addr[0] + 0x40;
>>  		} else {
>> @@ -529,8 +515,8 @@ static void vp_video_buffer(struct mixer_context *ctx,
>>  	vp_reg_writemask(res, VP_MODE, val, VP_MODE_LINE_SKIP);
>>  
>>  	/* setup format */
>> -	val =crcb_mode ? VP_MODE_NV21 : VP_MODE_NV12);
>> -	val |=tiled_mode ? VP_MODE_MEM_TILED : VP_MODE_MEM_LINEAR);
>> +	val =is_nv21 ? VP_MODE_NV21 : VP_MODE_NV12);
>> +	val |=is_tiled ? VP_MODE_MEM_TILED : VP_MODE_MEM_LINEAR);
>>  	vp_reg_writemask(res, VP_MODE, val, VP_MODE_FMT_MASK);
>>  
>>  	/* setting size of input image */
>> @@ -620,12 +606,9 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>>  
>>  	case DRM_FORMAT_XRGB8888:
>>  	case DRM_FORMAT_ARGB8888:
>> +	default:
>>  		fmt =XR_FORMAT_ARGB8888;
>>  		break;
>> -
>> -	default:
>> -		DRM_DEBUG_KMS("pixelformat unsupported by mixer\n");
>> -		return;
> 
> This change should be made as another patch because this patch changes the behavior whether it allows wrong pixel format or not.
> 
OK, I will split the change to mixer_graph_buffer() into another patch here.

- Tobias


> Thanks,
> Inki Dae
> 
>>  	}
>>  
>>  	/* ratio is already checked by common plane code */
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/8] drm/exynos: mixer: remove src offset from mixer_graph_buffer()

[Tobias Jakobi]

Inki Dae wrote:
> 
> 
> 2017년 08월 09일 20:48에 Tobias Jakobi 이(가) 쓴 글:
>> We always translate the dma address such that the offsets of
>> the source image are zero. Hence we can remove manipulation of
>> the MXR_GRAPHIC_SXY(win) register.
>>
>> We leave the register defines (in regs_mixer.h) in place, since
>> they document the hardware.
>>
>> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
>> ---
>>  drivers/gpu/drm/exynos/exynos_mixer.c | 11 ++---------
>>  1 file changed, 2 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index 8d68de85bada..28fbe79befff 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -584,7 +584,7 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>>  	unsigned long flags;
>>  	unsigned int win =lane->index;
>>  	unsigned int x_ratio =, y_ratio = 0;
>> -	unsigned int src_x_offset, src_y_offset, dst_x_offset, dst_y_offset;
>> +	unsigned int dst_x_offset, dst_y_offset;
>>  	dma_addr_t dma_addr;
>>  	unsigned int fmt;
>>  	u32 val;
>> @@ -618,12 +618,10 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>>  	dst_x_offset =tate->crtc.x;
>>  	dst_y_offset =tate->crtc.y;
>>  
>> -	/* converting dma address base and source offset */
>> +	/* translate dma address base s.t. the source image offset is zero */
>>  	dma_addr =xynos_drm_fb_dma_addr(fb, 0)
>>  		+ (state->src.x * fb->format->cpp[0])
>>  		+ (state->src.y * fb->pitches[0]);
>> -	src_x_offset =;
>> -	src_y_offset =;
>>  
>>  	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
>>  		__set_bit(MXR_BIT_INTERLACE, &ctx->flags);
>> @@ -654,11 +652,6 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>>  	val |=XR_GRP_WH_V_SCALE(y_ratio);
>>  	mixer_reg_write(res, MXR_GRAPHIC_WH(win), val);
>>  
>> -	/* setup offsets in source image */
>> -	val  =XR_GRP_SXY_SX(src_x_offset);
>> -	val |=XR_GRP_SXY_SY(src_y_offset);
>> -	mixer_reg_write(res, MXR_GRAPHIC_SXY(win), val);
> 
> The offset of source buffer can be set at bootloader so previous value can be keeped. I think MXR_GRAPHIC_SXY register should be cleared at mixer_win_reset function if you want to remove above lines.
> 
Good idea, will do so.

- Tobias


>> -
>>  	/* setup offsets in display image */
>>  	val  =XR_GRP_DXY_DX(dst_x_offset);
>>  	val |=XR_GRP_DXY_DY(dst_y_offset);
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 5/8] drm/exynos: introduce BYTE_PITCH capability

[Tobias Jakobi]

Inki Dae wrote:
> 
> 
> 2017년 08월 09일 20:48에 Tobias Jakobi 이(가) 쓴 글:
>> In some of drivers we compute something like 'pitch / cpp' at some
>> point, silently assuming that the pitch (which is in bytes) is
>> divisible by the buffer's cpp. This is not always true, in particular
>> DRM core does not check for pitch alignment in the common case.
>>
>> Introduce a new cap which indicates that the hardware supports a
>> pitch with 'byte-granularity'. If the cap is not set, assume that
>> we need pitch aligned to cpp.
>>
>> We set this cap later for the drivers/planes that support it.
>>
>> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_drv.h   |  1 +
>>  drivers/gpu/drm/exynos/exynos_drm_plane.c | 10 ++++++++++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> index 43afab4bebc3..ec32632485d2 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> @@ -92,6 +92,7 @@ struct exynos_drm_plane {
>>  #define EXYNOS_DRM_PLANE_CAP_SCALE	(1 << 1)
>>  #define EXYNOS_DRM_PLANE_CAP_ZPOS	(1 << 2)
>>  #define EXYNOS_DRM_PLANE_CAP_TILE	(1 << 3)
>> +#define EXYNOS_DRM_PLANE_CAP_BYTE_PITCH	(1 << 4)
> 
> I don't see why this flag is required because cpp value is given always. So I think we can check the pitch alignment regardless of a given flag.
> BTW, it'd better for pitch alignment to be checked by drm core?
How do you want to check the pitch value, if you don't know what the HW can handle?

- Tobias

> 
>>  
>>  /*
>>   * Exynos DRM plane configuration structure.
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
>> index bd3825617b06..734d5ba4eb99 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
>> @@ -185,6 +185,16 @@ exynos_drm_plane_check_format(const struct exynos_drm_plane_config *config,
>>  {
>>  	struct drm_framebuffer *fb = state->base.fb;
>>  
>> +	/*
>> +	 * Some blocks only allow to specify a buffer pitch in terms
>> +	 * of pixels. In these cases, we need to ensure that the pitch
>> +	 * provided by userspace is divisible by the cpp.
>> +	 */
>> +	if (!(config->capabilities & EXYNOS_DRM_PLANE_CAP_BYTE_PITCH)) {
>> +		if (fb->pitches[0] % fb->format->cpp[0])
>> +			return -ENOTSUPP;
>> +	}
>> +
>>  	switch (fb->modifier) {
>>  	case DRM_FORMAT_MOD_SAMSUNG_64_32_TILE:
>>  		if (!(config->capabilities & EXYNOS_DRM_PLANE_CAP_TILE))
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>