All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/8] drm/exynos: misc fixes and more
@ 2017-04-04 14:02 Tobias Jakobi
  2017-04-04 14:02 ` [RFC 1/8] drm/exynos: mixer: fix chroma comment in vp_video_buffer() Tobias Jakobi
                   ` (8 more replies)
  0 siblings, 9 replies; 13+ messages in thread
From: Tobias Jakobi @ 2017-04-04 14:02 UTC (permalink / raw)
  To: linux-samsung-soc; +Cc: dri-devel, Tobias Jakobi

Hello,

some recent work I did on Exynos. Patches are based on [1] and [2].

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

While testing the NV12MT part, I made these interesting observations.

(1) I used 1920x1080 XRGB8888 on the primary plane, and 1280x768 on the
video plane. With this configuration, it does not matter if you're
using NV12 or NV12MT, the video plane occasionally flickers or shows
heavy artifacting. Reducing the size of the primary plane, e.g. to
32x32 solves this issue, so my guess is that this is memory bandwidth
issue. Does someone know if one can check for buffer underflows of the
mixer with respect to data passed from the VP?

(2) Using 1920x1080i (so an interlaced mode) and NV12, the board
immediately dies with an IOMMU pagefault at address zero. I'm
currently investigating this, and it looks like that the VP setup
is wrong here. In particular it should be the source (!) height
and vertical position that should be halfed in interlaced mode, and
not the destination. Need to look more into this.

Anyway, both the issues are independant of the patches, so please
review! :-)

With best wishes,
Tobias


[1] http://www.spinics.net/lists/linux-samsung-soc/msg58640.html
[2] http://www.spinics.net/lists/linux-samsung-soc/msg58644.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.7.3

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC 1/8] drm/exynos: mixer: fix chroma comment in vp_video_buffer()
  2017-04-04 14:02 [RFC 0/8] drm/exynos: misc fixes and more Tobias Jakobi
@ 2017-04-04 14:02 ` Tobias Jakobi
  2017-04-04 14:02 ` [RFC 2/8] drm/exynos: mixer: enable NV12MT support for the video plane Tobias Jakobi
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Tobias Jakobi @ 2017-04-04 14:02 UTC (permalink / raw)
  To: linux-samsung-soc; +Cc: dri-devel, Tobias Jakobi

The current comment sounds like the division 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 9648dd5..77f18f6 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -533,7 +533,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.7.3

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC 2/8] drm/exynos: mixer: enable NV12MT support for the video plane
  2017-04-04 14:02 [RFC 0/8] drm/exynos: misc fixes and more Tobias Jakobi
  2017-04-04 14:02 ` [RFC 1/8] drm/exynos: mixer: fix chroma comment in vp_video_buffer() Tobias Jakobi
@ 2017-04-04 14:02 ` Tobias Jakobi
  2017-04-04 14:02 ` [RFC 3/8] drm/exynos: mixer: simplify {vp_video,mixer_graph}_buffer() Tobias Jakobi
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Tobias Jakobi @ 2017-04-04 14:02 UTC (permalink / raw)
  To: linux-samsung-soc; +Cc: dri-devel, Tobias Jakobi

The video processor supports a tiled variant 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 NV12MT 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 cf6e08c..ee62b0b 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 c77a5ac..35b7ab2 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 c2f17f3..fa6333d 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 77f18f6..35edbf2 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -149,7 +149,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,
 	},
 };
 
@@ -501,6 +502,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.7.3

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC 3/8] drm/exynos: mixer: simplify {vp_video,mixer_graph}_buffer()
  2017-04-04 14:02 [RFC 0/8] drm/exynos: misc fixes and more Tobias Jakobi
  2017-04-04 14:02 ` [RFC 1/8] drm/exynos: mixer: fix chroma comment in vp_video_buffer() Tobias Jakobi
  2017-04-04 14:02 ` [RFC 2/8] drm/exynos: mixer: enable NV12MT support for the video plane Tobias Jakobi
@ 2017-04-04 14:02 ` Tobias Jakobi
  2017-04-04 14:02 ` [RFC 4/8] drm/exynos: mixer: remove src offset from mixer_graph_buffer() Tobias Jakobi
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Tobias Jakobi @ 2017-04-04 14:02 UTC (permalink / raw)
  To: linux-samsung-soc; +Cc: dri-devel, a.hajda, inki.dae, Tobias Jakobi

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

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

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 35edbf2..32970fb 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -485,32 +485,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 {
@@ -530,8 +516,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 */
@@ -621,12 +607,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.7.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC 4/8] drm/exynos: mixer: remove src offset from mixer_graph_buffer()
  2017-04-04 14:02 [RFC 0/8] drm/exynos: misc fixes and more Tobias Jakobi
                   ` (2 preceding siblings ...)
  2017-04-04 14:02 ` [RFC 3/8] drm/exynos: mixer: simplify {vp_video,mixer_graph}_buffer() Tobias Jakobi
@ 2017-04-04 14:02 ` Tobias Jakobi
  2017-04-04 14:02 ` [RFC 5/8] drm/exynos: introduce BYTE_PITCH capability Tobias Jakobi
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Tobias Jakobi @ 2017-04-04 14:02 UTC (permalink / raw)
  To: linux-samsung-soc; +Cc: dri-devel, a.hajda, inki.dae, 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 32970fb..b712f2a 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -585,7 +585,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;
@@ -619,12 +619,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);
@@ -655,11 +653,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.7.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC 5/8] drm/exynos: introduce BYTE_PITCH capability
  2017-04-04 14:02 [RFC 0/8] drm/exynos: misc fixes and more Tobias Jakobi
                   ` (3 preceding siblings ...)
  2017-04-04 14:02 ` [RFC 4/8] drm/exynos: mixer: remove src offset from mixer_graph_buffer() Tobias Jakobi
@ 2017-04-04 14:02 ` Tobias Jakobi
  2017-04-04 14:02 ` [RFC 6/8] drm/exynos: add BYTE_PITCH cap for all supported planes Tobias Jakobi
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Tobias Jakobi @ 2017-04-04 14:02 UTC (permalink / raw)
  To: linux-samsung-soc; +Cc: dri-devel, 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 signals 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 the 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 ee62b0b..8d957a9 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 fa6333d..b31a226 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.7.3

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC 6/8] drm/exynos: add BYTE_PITCH cap for all supported planes
  2017-04-04 14:02 [RFC 0/8] drm/exynos: misc fixes and more Tobias Jakobi
                   ` (4 preceding siblings ...)
  2017-04-04 14:02 ` [RFC 5/8] drm/exynos: introduce BYTE_PITCH capability Tobias Jakobi
@ 2017-04-04 14:02 ` Tobias Jakobi
  2017-04-04 14:03 ` [RFC 7/8] drm/exynos: consistent use of cpp Tobias Jakobi
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Tobias Jakobi @ 2017-04-04 14:02 UTC (permalink / raw)
  To: linux-samsung-soc; +Cc: dri-devel, a.hajda, inki.dae, 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 0fd6f7a..099e22e 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -533,6 +533,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,
 					1 << ctx->pipe, &ctx->configs[win]);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index a9fa444..ae738b8 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -1007,6 +1007,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,
 					1 << ctx->pipe, &ctx->configs[i]);
 		if (ret)
-- 
2.7.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC 7/8] drm/exynos: consistent use of cpp
  2017-04-04 14:02 [RFC 0/8] drm/exynos: misc fixes and more Tobias Jakobi
                   ` (5 preceding siblings ...)
  2017-04-04 14:02 ` [RFC 6/8] drm/exynos: add BYTE_PITCH cap for all supported planes Tobias Jakobi
@ 2017-04-04 14:03 ` Tobias Jakobi
  2017-04-04 14:03 ` [RFC 8/8] drm/exynos: simplify set_pixfmt() in DECON and FIMD drivers Tobias Jakobi
  2017-04-11 10:52 ` [RFC 0/8] drm/exynos: misc fixes and more Tobias Jakobi
  8 siblings, 0 replies; 13+ messages in thread
From: Tobias Jakobi @ 2017-04-04 14:03 UTC (permalink / raw)
  To: linux-samsung-soc; +Cc: dri-devel, 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 099e22e..4343252 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -247,7 +247,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
@@ -296,7 +296,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;
@@ -335,11 +335,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 f9ab19e..c6d06cc 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -330,7 +330,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
@@ -407,7 +407,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)
@@ -427,7 +427,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 ae738b8..098a32d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -736,13 +736,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 */
@@ -761,8 +761,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.7.3

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC 8/8] drm/exynos: simplify set_pixfmt() in DECON and FIMD drivers
  2017-04-04 14:02 [RFC 0/8] drm/exynos: misc fixes and more Tobias Jakobi
                   ` (6 preceding siblings ...)
  2017-04-04 14:03 ` [RFC 7/8] drm/exynos: consistent use of cpp Tobias Jakobi
@ 2017-04-04 14:03 ` Tobias Jakobi
  2017-04-11 10:52 ` [RFC 0/8] drm/exynos: misc fixes and more Tobias Jakobi
  8 siblings, 0 replies; 13+ messages in thread
From: Tobias Jakobi @ 2017-04-04 14:03 UTC (permalink / raw)
  To: linux-samsung-soc; +Cc: dri-devel, 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 4343252..7efa92d 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -238,13 +238,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 c6d06cc..0d60e38 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -318,16 +318,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 098a32d..0529c58 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -603,18 +603,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.7.3

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [RFC 0/8] drm/exynos: misc fixes and more
  2017-04-04 14:02 [RFC 0/8] drm/exynos: misc fixes and more Tobias Jakobi
                   ` (7 preceding siblings ...)
  2017-04-04 14:03 ` [RFC 8/8] drm/exynos: simplify set_pixfmt() in DECON and FIMD drivers Tobias Jakobi
@ 2017-04-11 10:52 ` Tobias Jakobi
  2017-04-12  2:35   ` Inki Dae
  8 siblings, 1 reply; 13+ messages in thread
From: Tobias Jakobi @ 2017-04-11 10:52 UTC (permalink / raw)
  To: inki.dae; +Cc: linux-samsung-soc, dri-devel

Hello Inki,

please don't forget to review this series.

Also some pointers concerning the video plane flickering and the
interlacing issue would be very welcome.

Looking at the current register defines for MXR_INT_EN suggests that
there are at least three more type of interrupt available. Any chance
these could be documented? I hope to get some more insight into this
issue, which apparantly is caused by insufficient bandwidth.

For the interlacing issue I have done partial progress. I now longer see
a IOMMU pagefault, but the video plane is heavily corrupted. Also the
dimensions look wrong.

While I can somehow guess that VP_MODE_LINE_SKIP does, the
VP_MODE_FIELD_ID_AUTO_TOGGLING flag still remains a mystery to me. If
possible, I request some documentation for this as well.


With best wishes,
Tobias



Tobias Jakobi wrote:
> Hello,
> 
> some recent work I did on Exynos. Patches are based on [1] and [2].
> 
> Summary:
> (a) Enables support for NV12MT in the mixer.
> (b) Sanitizes buffer pitch for HW with restrictions.
> (c) Misc fixes
> 
> While testing the NV12MT part, I made these interesting observations.
> 
> (1) I used 1920x1080 XRGB8888 on the primary plane, and 1280x768 on the
> video plane. With this configuration, it does not matter if you're
> using NV12 or NV12MT, the video plane occasionally flickers or shows
> heavy artifacting. Reducing the size of the primary plane, e.g. to
> 32x32 solves this issue, so my guess is that this is memory bandwidth
> issue. Does someone know if one can check for buffer underflows of the
> mixer with respect to data passed from the VP?
> 
> (2) Using 1920x1080i (so an interlaced mode) and NV12, the board
> immediately dies with an IOMMU pagefault at address zero. I'm
> currently investigating this, and it looks like that the VP setup
> is wrong here. In particular it should be the source (!) height
> and vertical position that should be halfed in interlaced mode, and
> not the destination. Need to look more into this.
> 
> Anyway, both the issues are independant of the patches, so please
> review! :-)
> 
> With best wishes,
> Tobias
> 
> 
> [1] http://www.spinics.net/lists/linux-samsung-soc/msg58640.html
> [2] http://www.spinics.net/lists/linux-samsung-soc/msg58644.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(-)
> 

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC 0/8] drm/exynos: misc fixes and more
  2017-04-11 10:52 ` [RFC 0/8] drm/exynos: misc fixes and more Tobias Jakobi
@ 2017-04-12  2:35   ` Inki Dae
  2017-04-16 11:51     ` Tobias Jakobi
  0 siblings, 1 reply; 13+ messages in thread
From: Inki Dae @ 2017-04-12  2:35 UTC (permalink / raw)
  To: Tobias Jakobi; +Cc: linux-samsung-soc, dri-devel

Hello Tobias,

2017년 04월 11일 19:52에 Tobias Jakobi 이(가) 쓴 글:
> Hello Inki,
> 
> please don't forget to review this series.

Thanks for your contribution, and don't worry about that. Will review this series.

Just sharing a plan for -next,

I plan to have pull-request after reviewing a patch set[1] posted by Andrzej.
After that, I will start to review your patch set - we would need to enough review this RFC patch set. 

It would be very helpful to me if other people could review this.


Thanks,
Inki Dae

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


> 
> Also some pointers concerning the video plane flickering and the
> interlacing issue would be very welcome.
> 
> Looking at the current register defines for MXR_INT_EN suggests that
> there are at least three more type of interrupt available. Any chance
> these could be documented? I hope to get some more insight into this
> issue, which apparantly is caused by insufficient bandwidth.
> 
> For the interlacing issue I have done partial progress. I now longer see
> a IOMMU pagefault, but the video plane is heavily corrupted. Also the
> dimensions look wrong.
> 
> While I can somehow guess that VP_MODE_LINE_SKIP does, the
> VP_MODE_FIELD_ID_AUTO_TOGGLING flag still remains a mystery to me. If
> possible, I request some documentation for this as well.
> 
> 
> With best wishes,
> Tobias
> 
> 
> 
> Tobias Jakobi wrote:
>> Hello,
>>
>> some recent work I did on Exynos. Patches are based on [1] and [2].
>>
>> Summary:
>> (a) Enables support for NV12MT in the mixer.
>> (b) Sanitizes buffer pitch for HW with restrictions.
>> (c) Misc fixes
>>
>> While testing the NV12MT part, I made these interesting observations.
>>
>> (1) I used 1920x1080 XRGB8888 on the primary plane, and 1280x768 on the
>> video plane. With this configuration, it does not matter if you're
>> using NV12 or NV12MT, the video plane occasionally flickers or shows
>> heavy artifacting. Reducing the size of the primary plane, e.g. to
>> 32x32 solves this issue, so my guess is that this is memory bandwidth
>> issue. Does someone know if one can check for buffer underflows of the
>> mixer with respect to data passed from the VP?
>>
>> (2) Using 1920x1080i (so an interlaced mode) and NV12, the board
>> immediately dies with an IOMMU pagefault at address zero. I'm
>> currently investigating this, and it looks like that the VP setup
>> is wrong here. In particular it should be the source (!) height
>> and vertical position that should be halfed in interlaced mode, and
>> not the destination. Need to look more into this.
>>
>> Anyway, both the issues are independant of the patches, so please
>> review! :-)
>>
>> With best wishes,
>> Tobias
>>
>>
>> [1] http://www.spinics.net/lists/linux-samsung-soc/msg58640.html
>> [2] http://www.spinics.net/lists/linux-samsung-soc/msg58644.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(-)
>>
> 
> --
> 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC 0/8] drm/exynos: misc fixes and more
  2017-04-12  2:35   ` Inki Dae
@ 2017-04-16 11:51     ` Tobias Jakobi
  2017-04-19  1:46       ` Inki Dae
  0 siblings, 1 reply; 13+ messages in thread
From: Tobias Jakobi @ 2017-04-16 11:51 UTC (permalink / raw)
  To: Inki Dae; +Cc: linux-samsung-soc, dri-devel, a.hajda

Hello Inki,


Inki Dae wrote:
> Hello Tobias,
> 
> 2017년 04월 11일 19:52에 Tobias Jakobi 이(가) 쓴 글:
>> Hello Inki,
>>
>> please don't forget to review this series.
> 
> Thanks for your contribution, and don't worry about that. Will review this series.


Thank you for the confirmation!

In the meantime, i.e. until we have figured out the IOMMU pagefault, it
would make sense to reject the combination "interlaced mode + video
plane". Should I prepare a patch for that?


With best wishes,
Tobias



> Just sharing a plan for -next,
> 
> I plan to have pull-request after reviewing a patch set[1] posted by Andrzej.
> After that, I will start to review your patch set - we would need to enough review this RFC patch set. 
> 
> It would be very helpful to me if other people could review this.
> 
> 
> Thanks,
> Inki Dae
> 
> [1] https://lists.freedesktop.org/archives/dri-devel/2017-April/138112.html
> 
> 
>>
>> Also some pointers concerning the video plane flickering and the
>> interlacing issue would be very welcome.
>>
>> Looking at the current register defines for MXR_INT_EN suggests that
>> there are at least three more type of interrupt available. Any chance
>> these could be documented? I hope to get some more insight into this
>> issue, which apparantly is caused by insufficient bandwidth.
>>
>> For the interlacing issue I have done partial progress. I now longer see
>> a IOMMU pagefault, but the video plane is heavily corrupted. Also the
>> dimensions look wrong.
>>
>> While I can somehow guess that VP_MODE_LINE_SKIP does, the
>> VP_MODE_FIELD_ID_AUTO_TOGGLING flag still remains a mystery to me. If
>> possible, I request some documentation for this as well.
>>
>>
>> With best wishes,
>> Tobias
>>
>>
>>
>> Tobias Jakobi wrote:
>>> Hello,
>>>
>>> some recent work I did on Exynos. Patches are based on [1] and [2].
>>>
>>> Summary:
>>> (a) Enables support for NV12MT in the mixer.
>>> (b) Sanitizes buffer pitch for HW with restrictions.
>>> (c) Misc fixes
>>>
>>> While testing the NV12MT part, I made these interesting observations.
>>>
>>> (1) I used 1920x1080 XRGB8888 on the primary plane, and 1280x768 on the
>>> video plane. With this configuration, it does not matter if you're
>>> using NV12 or NV12MT, the video plane occasionally flickers or shows
>>> heavy artifacting. Reducing the size of the primary plane, e.g. to
>>> 32x32 solves this issue, so my guess is that this is memory bandwidth
>>> issue. Does someone know if one can check for buffer underflows of the
>>> mixer with respect to data passed from the VP?
>>>
>>> (2) Using 1920x1080i (so an interlaced mode) and NV12, the board
>>> immediately dies with an IOMMU pagefault at address zero. I'm
>>> currently investigating this, and it looks like that the VP setup
>>> is wrong here. In particular it should be the source (!) height
>>> and vertical position that should be halfed in interlaced mode, and
>>> not the destination. Need to look more into this.
>>>
>>> Anyway, both the issues are independant of the patches, so please
>>> review! :-)
>>>
>>> With best wishes,
>>> Tobias
>>>
>>>
>>> [1] http://www.spinics.net/lists/linux-samsung-soc/msg58640.html
>>> [2] http://www.spinics.net/lists/linux-samsung-soc/msg58644.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(-)
>>>
>>
>> --
>> 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
>>
>>
>>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC 0/8] drm/exynos: misc fixes and more
  2017-04-16 11:51     ` Tobias Jakobi
@ 2017-04-19  1:46       ` Inki Dae
  0 siblings, 0 replies; 13+ messages in thread
From: Inki Dae @ 2017-04-19  1:46 UTC (permalink / raw)
  To: Tobias Jakobi; +Cc: linux-samsung-soc, dri-devel, a.hajda

Hello Tobias,


2017년 04월 16일 20:51에 Tobias Jakobi 이(가) 쓴 글:
> Hello Inki,
> 
> 
> Inki Dae wrote:
>> Hello Tobias,
>>
>> 2017년 04월 11일 19:52에 Tobias Jakobi 이(가) 쓴 글:
>>> Hello Inki,
>>>
>>> please don't forget to review this series.
>>
>> Thanks for your contribution, and don't worry about that. Will review this series.
> 
> 
> Thank you for the confirmation!
> 
> In the meantime, i.e. until we have figured out the IOMMU pagefault, it
> would make sense to reject the combination "interlaced mode + video
> plane". Should I prepare a patch for that?
> 

I think you may be only person who could look into this as of now. Other person including me would have other tasks more important than this.

Thanks,
Inki Dae

> 
> With best wishes,
> Tobias
> 
> 
> 
>> Just sharing a plan for -next,
>>
>> I plan to have pull-request after reviewing a patch set[1] posted by Andrzej.
>> After that, I will start to review your patch set - we would need to enough review this RFC patch set. 
>>
>> It would be very helpful to me if other people could review this.
>>
>>
>> Thanks,
>> Inki Dae
>>
>> [1] https://lists.freedesktop.org/archives/dri-devel/2017-April/138112.html
>>
>>
>>>
>>> Also some pointers concerning the video plane flickering and the
>>> interlacing issue would be very welcome.
>>>
>>> Looking at the current register defines for MXR_INT_EN suggests that
>>> there are at least three more type of interrupt available. Any chance
>>> these could be documented? I hope to get some more insight into this
>>> issue, which apparantly is caused by insufficient bandwidth.
>>>
>>> For the interlacing issue I have done partial progress. I now longer see
>>> a IOMMU pagefault, but the video plane is heavily corrupted. Also the
>>> dimensions look wrong.
>>>
>>> While I can somehow guess that VP_MODE_LINE_SKIP does, the
>>> VP_MODE_FIELD_ID_AUTO_TOGGLING flag still remains a mystery to me. If
>>> possible, I request some documentation for this as well.
>>>
>>>
>>> With best wishes,
>>> Tobias
>>>
>>>
>>>
>>> Tobias Jakobi wrote:
>>>> Hello,
>>>>
>>>> some recent work I did on Exynos. Patches are based on [1] and [2].
>>>>
>>>> Summary:
>>>> (a) Enables support for NV12MT in the mixer.
>>>> (b) Sanitizes buffer pitch for HW with restrictions.
>>>> (c) Misc fixes
>>>>
>>>> While testing the NV12MT part, I made these interesting observations.
>>>>
>>>> (1) I used 1920x1080 XRGB8888 on the primary plane, and 1280x768 on the
>>>> video plane. With this configuration, it does not matter if you're
>>>> using NV12 or NV12MT, the video plane occasionally flickers or shows
>>>> heavy artifacting. Reducing the size of the primary plane, e.g. to
>>>> 32x32 solves this issue, so my guess is that this is memory bandwidth
>>>> issue. Does someone know if one can check for buffer underflows of the
>>>> mixer with respect to data passed from the VP?
>>>>
>>>> (2) Using 1920x1080i (so an interlaced mode) and NV12, the board
>>>> immediately dies with an IOMMU pagefault at address zero. I'm
>>>> currently investigating this, and it looks like that the VP setup
>>>> is wrong here. In particular it should be the source (!) height
>>>> and vertical position that should be halfed in interlaced mode, and
>>>> not the destination. Need to look more into this.
>>>>
>>>> Anyway, both the issues are independant of the patches, so please
>>>> review! :-)
>>>>
>>>> With best wishes,
>>>> Tobias
>>>>
>>>>
>>>> [1] http://www.spinics.net/lists/linux-samsung-soc/msg58640.html
>>>> [2] http://www.spinics.net/lists/linux-samsung-soc/msg58644.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(-)
>>>>
>>>
>>> --
>>> 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
>>>
>>>
>>>
> 
> --
> 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
> 
> 
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-04-19  1:46 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-04 14:02 [RFC 0/8] drm/exynos: misc fixes and more Tobias Jakobi
2017-04-04 14:02 ` [RFC 1/8] drm/exynos: mixer: fix chroma comment in vp_video_buffer() Tobias Jakobi
2017-04-04 14:02 ` [RFC 2/8] drm/exynos: mixer: enable NV12MT support for the video plane Tobias Jakobi
2017-04-04 14:02 ` [RFC 3/8] drm/exynos: mixer: simplify {vp_video,mixer_graph}_buffer() Tobias Jakobi
2017-04-04 14:02 ` [RFC 4/8] drm/exynos: mixer: remove src offset from mixer_graph_buffer() Tobias Jakobi
2017-04-04 14:02 ` [RFC 5/8] drm/exynos: introduce BYTE_PITCH capability Tobias Jakobi
2017-04-04 14:02 ` [RFC 6/8] drm/exynos: add BYTE_PITCH cap for all supported planes Tobias Jakobi
2017-04-04 14:03 ` [RFC 7/8] drm/exynos: consistent use of cpp Tobias Jakobi
2017-04-04 14:03 ` [RFC 8/8] drm/exynos: simplify set_pixfmt() in DECON and FIMD drivers Tobias Jakobi
2017-04-11 10:52 ` [RFC 0/8] drm/exynos: misc fixes and more Tobias Jakobi
2017-04-12  2:35   ` Inki Dae
2017-04-16 11:51     ` Tobias Jakobi
2017-04-19  1:46       ` Inki Dae

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.