[PATCH 00/10] drm/exynos: TV path improvements

[PATCH 00/10] drm/exynos: TV path improvements

[Andrzej Hajda]

Hi all,

This patchset does two main things:
- removes mode limitation for Exynos542x chips, multiple modes were filtered
  out due to lack of HW version checking code,
- enables two modes on older chips, thanks to quirk found by Daniel Drake,
  and published by Tobias Jakobi [1][2].
Beside this it consolidates the code and performs multiple cleanups.

[1]: http://www.spinics.net/lists/linux-samsung-soc/msg24617.html
[2]: https://www.spinics.net/lists/dri-devel/msg150906.html

Regards
Andrzej


Andrzej Hajda (10):
  drm/exynos/mixer: abstract out output mode setup code
  drm/exynos/mixer: move mode commit to enable callback
  drm/exynos/mixer: move resolution configuration to single function
  drm/exynos/mixer: fix mode validation code
  drm/exynos/mixer: remove mixer_resources sub-structure
  drm/exynos/hdmi: remove redundant mode field
  drm/exynos: add mode_fixup callback to exynos_drm_crtc_ops
  drm/exynos/mixer: pass actual mode on MIXER to encoder
  drm/exynos/hdmi: quirk for support mode timings conversion
  drm/exynos/mixer: enable support for 1024x768 and 1280x1024 modes

 drivers/gpu/drm/exynos/exynos_drm_crtc.c |  15 +
 drivers/gpu/drm/exynos/exynos_drm_drv.h  |   3 +
 drivers/gpu/drm/exynos/exynos_hdmi.c     |  49 ++--
 drivers/gpu/drm/exynos/exynos_mixer.c    | 452 +++++++++++++++----------------
 4 files changed, 265 insertions(+), 254 deletions(-)

-- 
2.7.4

[PATCH 01/10] drm/exynos/mixer: abstract out output mode setup code

[Andrzej Hajda]

Mode setup code is called from video plane update and mixer plane update.
Lets group it together in mixer_commit function like in case of other
Exynos CRTCs.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 0027554..499ebdc 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -473,6 +473,22 @@ static void mixer_stop(struct mixer_context *ctx)
 		usleep_range(10000, 12000);
 }
 
+static void mixer_commit(struct mixer_context *ctx)
+{
+	struct drm_display_mode *mode = &ctx->crtc->base.state->adjusted_mode;
+
+	/* setup display size */
+	if (ctx->mxr_ver == MXR_VER_128_0_0_184) {
+		u32 val  = MXR_MXR_RES_HEIGHT(mode->vdisplay)
+			 | MXR_MXR_RES_WIDTH(mode->hdisplay);
+		mixer_reg_write(&ctx->mixer_res, MXR_RESOLUTION, val);
+	}
+
+	mixer_cfg_scan(ctx, mode->vdisplay);
+	mixer_cfg_rgb_fmt(ctx, mode->vdisplay);
+	mixer_run(ctx);
+}
+
 static void vp_video_buffer(struct mixer_context *ctx,
 			    struct exynos_drm_plane *plane)
 {
@@ -553,11 +569,9 @@ static void vp_video_buffer(struct mixer_context *ctx,
 	vp_reg_write(res, VP_TOP_C_PTR, chroma_addr[0]);
 	vp_reg_write(res, VP_BOT_C_PTR, chroma_addr[1]);
 
-	mixer_cfg_scan(ctx, mode->vdisplay);
-	mixer_cfg_rgb_fmt(ctx, mode->vdisplay);
 	mixer_cfg_layer(ctx, plane->index, priority, true);
 	mixer_cfg_vp_blend(ctx);
-	mixer_run(ctx);
+	mixer_commit(ctx);
 
 	spin_unlock_irqrestore(&res->reg_slock, flags);
 
@@ -638,14 +652,6 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
 	mixer_reg_write(res, MXR_GRAPHIC_SPAN(win),
 			fb->pitches[0] / fb->format->cpp[0]);
 
-	/* setup display size */
-	if (ctx->mxr_ver == MXR_VER_128_0_0_184 &&
-		win == DEFAULT_WIN) {
-		val  = MXR_MXR_RES_HEIGHT(mode->vdisplay);
-		val |= MXR_MXR_RES_WIDTH(mode->hdisplay);
-		mixer_reg_write(res, MXR_RESOLUTION, val);
-	}
-
 	val  = MXR_GRP_WH_WIDTH(state->src.w);
 	val |= MXR_GRP_WH_HEIGHT(state->src.h);
 	val |= MXR_GRP_WH_H_SCALE(x_ratio);
@@ -660,18 +666,15 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
 	/* set buffer address to mixer */
 	mixer_reg_write(res, MXR_GRAPHIC_BASE(win), dma_addr);
 
-	mixer_cfg_scan(ctx, mode->vdisplay);
-	mixer_cfg_rgb_fmt(ctx, mode->vdisplay);
 	mixer_cfg_layer(ctx, win, priority, true);
 	mixer_cfg_gfx_blend(ctx, win, is_alpha_format(fb->format->format));
+	mixer_commit(ctx);
 
 	/* layer update mandatory for mixer 16.0.33.0 */
 	if (ctx->mxr_ver == MXR_VER_16_0_33_0 ||
 		ctx->mxr_ver == MXR_VER_128_0_0_184)
 		mixer_layer_update(ctx);
 
-	mixer_run(ctx);
-
 	spin_unlock_irqrestore(&res->reg_slock, flags);
 
 	mixer_regs_dump(ctx);
-- 
2.7.4

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

[PATCH 02/10] drm/exynos/mixer: move mode commit to enable callback

[Andrzej Hajda]

Mode commit should not be called for every plane separately. It is enough
to call it once in enable callback. The change requires also that
interlace check should be moved to mixer_commit. It should be done in
the same patch to avoid regression.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 499ebdc..ae89e53 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -477,6 +477,11 @@ static void mixer_commit(struct mixer_context *ctx)
 {
 	struct drm_display_mode *mode = &ctx->crtc->base.state->adjusted_mode;
 
+	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
+		__set_bit(MXR_BIT_INTERLACE, &ctx->flags);
+	else
+		__clear_bit(MXR_BIT_INTERLACE, &ctx->flags);
+
 	/* setup display size */
 	if (ctx->mxr_ver == MXR_VER_128_0_0_184) {
 		u32 val  = MXR_MXR_RES_HEIGHT(mode->vdisplay)
@@ -494,7 +499,6 @@ static void vp_video_buffer(struct mixer_context *ctx,
 {
 	struct exynos_drm_plane_state *state =
 				to_exynos_plane_state(plane->base.state);
-	struct drm_display_mode *mode = &state->base.crtc->state->adjusted_mode;
 	struct mixer_resources *res = &ctx->mixer_res;
 	struct drm_framebuffer *fb = state->base.fb;
 	unsigned int priority = state->base.normalized_zpos + 1;
@@ -509,8 +513,7 @@ static void vp_video_buffer(struct mixer_context *ctx,
 	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 (test_bit(MXR_BIT_INTERLACE, &ctx->flags)) {
 		if (is_tiled) {
 			luma_addr[1] = luma_addr[0] + 0x40;
 			chroma_addr[1] = chroma_addr[0] + 0x40;
@@ -519,7 +522,6 @@ static void vp_video_buffer(struct mixer_context *ctx,
 			chroma_addr[1] = chroma_addr[0] + fb->pitches[0];
 		}
 	} else {
-		__clear_bit(MXR_BIT_INTERLACE, &ctx->flags);
 		luma_addr[1] = 0;
 		chroma_addr[1] = 0;
 	}
@@ -571,7 +573,6 @@ static void vp_video_buffer(struct mixer_context *ctx,
 
 	mixer_cfg_layer(ctx, plane->index, priority, true);
 	mixer_cfg_vp_blend(ctx);
-	mixer_commit(ctx);
 
 	spin_unlock_irqrestore(&res->reg_slock, flags);
 
@@ -591,7 +592,6 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
 {
 	struct exynos_drm_plane_state *state =
 				to_exynos_plane_state(plane->base.state);
-	struct drm_display_mode *mode = &state->base.crtc->state->adjusted_mode;
 	struct mixer_resources *res = &ctx->mixer_res;
 	struct drm_framebuffer *fb = state->base.fb;
 	unsigned int priority = state->base.normalized_zpos + 1;
@@ -637,11 +637,6 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
 		+ (state->src.x * fb->format->cpp[0])
 		+ (state->src.y * fb->pitches[0]);
 
-	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
-		__set_bit(MXR_BIT_INTERLACE, &ctx->flags);
-	else
-		__clear_bit(MXR_BIT_INTERLACE, &ctx->flags);
-
 	spin_lock_irqsave(&res->reg_slock, flags);
 
 	/* setup format */
@@ -668,7 +663,6 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
 
 	mixer_cfg_layer(ctx, win, priority, true);
 	mixer_cfg_gfx_blend(ctx, win, is_alpha_format(fb->format->format));
-	mixer_commit(ctx);
 
 	/* layer update mandatory for mixer 16.0.33.0 */
 	if (ctx->mxr_ver == MXR_VER_16_0_33_0 ||
@@ -1021,6 +1015,8 @@ static void mixer_enable(struct exynos_drm_crtc *crtc)
 	}
 	mixer_win_reset(ctx);
 
+	mixer_commit(ctx);
+
 	mixer_vsync_set_update(ctx, true);
 
 	set_bit(MXR_BIT_POWERED, &ctx->flags);
-- 
2.7.4

[PATCH 03/10] drm/exynos/mixer: move resolution configuration to single function

[Andrzej Hajda]

Screen resolution configuration depends on HW version, let put it into
single function to make it consistent and simplify the code.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index ae89e53..a87f60b 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -367,7 +367,7 @@ static void mixer_vsync_set_update(struct mixer_context *ctx, bool enable)
 			VP_SHADOW_UPDATE_ENABLE : 0);
 }
 
-static void mixer_cfg_scan(struct mixer_context *ctx, unsigned int height)
+static void mixer_cfg_scan(struct mixer_context *ctx, int width, int height)
 {
 	struct mixer_resources *res = &ctx->mixer_res;
 	u32 val;
@@ -376,7 +376,11 @@ static void mixer_cfg_scan(struct mixer_context *ctx, unsigned int height)
 	val = test_bit(MXR_BIT_INTERLACE, &ctx->flags) ?
 		MXR_CFG_SCAN_INTERLACE : MXR_CFG_SCAN_PROGRESSIVE;
 
-	if (ctx->mxr_ver != MXR_VER_128_0_0_184) {
+	/* setup display size */
+	if (ctx->mxr_ver == MXR_VER_128_0_0_184) {
+		mixer_reg_write(&ctx->mixer_res, MXR_RESOLUTION,
+			MXR_MXR_RES_HEIGHT(height) | MXR_MXR_RES_WIDTH(width));
+	} else {
 		/* choosing between proper HD and SD mode */
 		if (height <= 480)
 			val |= MXR_CFG_SCAN_NTSC | MXR_CFG_SCAN_SD;
@@ -482,14 +486,7 @@ static void mixer_commit(struct mixer_context *ctx)
 	else
 		__clear_bit(MXR_BIT_INTERLACE, &ctx->flags);
 
-	/* setup display size */
-	if (ctx->mxr_ver == MXR_VER_128_0_0_184) {
-		u32 val  = MXR_MXR_RES_HEIGHT(mode->vdisplay)
-			 | MXR_MXR_RES_WIDTH(mode->hdisplay);
-		mixer_reg_write(&ctx->mixer_res, MXR_RESOLUTION, val);
-	}
-
-	mixer_cfg_scan(ctx, mode->vdisplay);
+	mixer_cfg_scan(ctx, mode->hdisplay, mode->vdisplay);
 	mixer_cfg_rgb_fmt(ctx, mode->vdisplay);
 	mixer_run(ctx);
 }
-- 
2.7.4

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

[PATCH 04/10] drm/exynos/mixer: fix mode validation code

[Andrzej Hajda]

Mode limitation checked in mixer driver affects only older HW.
Mixer in Exynos542x has no such limitations. While at it patch changes
validation callback to recently introduced mode_valid which is more
suitable for the check. Additionally little cleanup is performed.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index a87f60b..d530c18 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -1040,26 +1040,24 @@ static void mixer_disable(struct exynos_drm_crtc *crtc)
 	clear_bit(MXR_BIT_POWERED, &ctx->flags);
 }
 
-/* Only valid for Mixer version 16.0.33.0 */
-static int mixer_atomic_check(struct exynos_drm_crtc *crtc,
-		       struct drm_crtc_state *state)
+static int mixer_mode_valid(struct exynos_drm_crtc *crtc,
+		const struct drm_display_mode *mode)
 {
-	struct drm_display_mode *mode = &state->adjusted_mode;
-	u32 w, h;
+	struct mixer_context *ctx = crtc->ctx;
+	u32 w = mode->hdisplay, h = mode->vdisplay;
 
-	w = mode->hdisplay;
-	h = mode->vdisplay;
+	DRM_DEBUG_KMS("xres=%d, yres=%d, refresh=%d, intl=%d\n", w, h,
+		mode->vrefresh, !!(mode->flags & DRM_MODE_FLAG_INTERLACE));
 
-	DRM_DEBUG_KMS("xres=%d, yres=%d, refresh=%d, intl=%d\n",
-		mode->hdisplay, mode->vdisplay, mode->vrefresh,
-		(mode->flags & DRM_MODE_FLAG_INTERLACE) ? 1 : 0);
+	if (ctx->mxr_ver == MXR_VER_128_0_0_184)
+		return MODE_OK;
 
 	if ((w >= 464 && w <= 720 && h >= 261 && h <= 576) ||
 		(w >= 1024 && w <= 1280 && h >= 576 && h <= 720) ||
 		(w >= 1664 && w <= 1920 && h >= 936 && h <= 1080))
-		return 0;
+		return MODE_OK;
 
-	return -EINVAL;
+	return MODE_BAD;
 }
 
 static const struct exynos_drm_crtc_ops mixer_crtc_ops = {
@@ -1071,7 +1069,7 @@ static const struct exynos_drm_crtc_ops mixer_crtc_ops = {
 	.update_plane		= mixer_update_plane,
 	.disable_plane		= mixer_disable_plane,
 	.atomic_flush		= mixer_atomic_flush,
-	.atomic_check		= mixer_atomic_check,
+	.mode_valid		= mixer_mode_valid,
 };
 
 static const struct mixer_drv_data exynos5420_mxr_drv_data = {
-- 
2.7.4

[PATCH 05/10] drm/exynos/mixer: remove mixer_resources sub-structure

[Andrzej Hajda]

mixer_resources adds only unnecessary redirection, removing it makes the
code shorter and cleaner.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 323 ++++++++++++++++------------------
 1 file changed, 147 insertions(+), 176 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index d530c18..f6ea9d9 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -67,19 +67,6 @@
 #define MXR_FORMAT_ARGB4444	6
 #define MXR_FORMAT_ARGB8888	7
 
-struct mixer_resources {
-	int			irq;
-	void __iomem		*mixer_regs;
-	void __iomem		*vp_regs;
-	spinlock_t		reg_slock;
-	struct clk		*mixer;
-	struct clk		*vp;
-	struct clk		*hdmi;
-	struct clk		*sclk_mixer;
-	struct clk		*sclk_hdmi;
-	struct clk		*mout_mixer;
-};
-
 enum mixer_version_id {
 	MXR_VER_0_0_0_16,
 	MXR_VER_16_0_33_0,
@@ -117,7 +104,16 @@ struct mixer_context {
 	struct exynos_drm_plane	planes[MIXER_WIN_NR];
 	unsigned long		flags;
 
-	struct mixer_resources	mixer_res;
+	int			irq;
+	void __iomem		*mixer_regs;
+	void __iomem		*vp_regs;
+	spinlock_t		reg_slock;
+	struct clk		*mixer;
+	struct clk		*vp;
+	struct clk		*hdmi;
+	struct clk		*sclk_mixer;
+	struct clk		*sclk_hdmi;
+	struct clk		*mout_mixer;
 	enum mixer_version_id	mxr_ver;
 };
 
@@ -194,44 +190,44 @@ static inline bool is_alpha_format(unsigned int pixel_format)
 	}
 }
 
-static inline u32 vp_reg_read(struct mixer_resources *res, u32 reg_id)
+static inline u32 vp_reg_read(struct mixer_context *ctx, u32 reg_id)
 {
-	return readl(res->vp_regs + reg_id);
+	return readl(ctx->vp_regs + reg_id);
 }
 
-static inline void vp_reg_write(struct mixer_resources *res, u32 reg_id,
+static inline void vp_reg_write(struct mixer_context *ctx, u32 reg_id,
 				 u32 val)
 {
-	writel(val, res->vp_regs + reg_id);
+	writel(val, ctx->vp_regs + reg_id);
 }
 
-static inline void vp_reg_writemask(struct mixer_resources *res, u32 reg_id,
+static inline void vp_reg_writemask(struct mixer_context *ctx, u32 reg_id,
 				 u32 val, u32 mask)
 {
-	u32 old = vp_reg_read(res, reg_id);
+	u32 old = vp_reg_read(ctx, reg_id);
 
 	val = (val & mask) | (old & ~mask);
-	writel(val, res->vp_regs + reg_id);
+	writel(val, ctx->vp_regs + reg_id);
 }
 
-static inline u32 mixer_reg_read(struct mixer_resources *res, u32 reg_id)
+static inline u32 mixer_reg_read(struct mixer_context *ctx, u32 reg_id)
 {
-	return readl(res->mixer_regs + reg_id);
+	return readl(ctx->mixer_regs + reg_id);
 }
 
-static inline void mixer_reg_write(struct mixer_resources *res, u32 reg_id,
+static inline void mixer_reg_write(struct mixer_context *ctx, u32 reg_id,
 				 u32 val)
 {
-	writel(val, res->mixer_regs + reg_id);
+	writel(val, ctx->mixer_regs + reg_id);
 }
 
-static inline void mixer_reg_writemask(struct mixer_resources *res,
+static inline void mixer_reg_writemask(struct mixer_context *ctx,
 				 u32 reg_id, u32 val, u32 mask)
 {
-	u32 old = mixer_reg_read(res, reg_id);
+	u32 old = mixer_reg_read(ctx, reg_id);
 
 	val = (val & mask) | (old & ~mask);
-	writel(val, res->mixer_regs + reg_id);
+	writel(val, ctx->mixer_regs + reg_id);
 }
 
 static void mixer_regs_dump(struct mixer_context *ctx)
@@ -239,7 +235,7 @@ static void mixer_regs_dump(struct mixer_context *ctx)
 #define DUMPREG(reg_id) \
 do { \
 	DRM_DEBUG_KMS(#reg_id " = %08x\n", \
-		(u32)readl(ctx->mixer_res.mixer_regs + reg_id)); \
+		(u32)readl(ctx->mixer_regs + reg_id)); \
 } while (0)
 
 	DUMPREG(MXR_STATUS);
@@ -271,7 +267,7 @@ static void vp_regs_dump(struct mixer_context *ctx)
 #define DUMPREG(reg_id) \
 do { \
 	DRM_DEBUG_KMS(#reg_id " = %08x\n", \
-		(u32) readl(ctx->mixer_res.vp_regs + reg_id)); \
+		(u32) readl(ctx->vp_regs + reg_id)); \
 } while (0)
 
 	DUMPREG(VP_ENABLE);
@@ -301,7 +297,7 @@ do { \
 #undef DUMPREG
 }
 
-static inline void vp_filter_set(struct mixer_resources *res,
+static inline void vp_filter_set(struct mixer_context *ctx,
 		int reg_id, const u8 *data, unsigned int size)
 {
 	/* assure 4-byte align */
@@ -309,24 +305,23 @@ static inline void vp_filter_set(struct mixer_resources *res,
 	for (; size; size -= 4, reg_id += 4, data += 4) {
 		u32 val = (data[0] << 24) |  (data[1] << 16) |
 			(data[2] << 8) | data[3];
-		vp_reg_write(res, reg_id, val);
+		vp_reg_write(ctx, reg_id, val);
 	}
 }
 
-static void vp_default_filter(struct mixer_resources *res)
+static void vp_default_filter(struct mixer_context *ctx)
 {
-	vp_filter_set(res, VP_POLY8_Y0_LL,
+	vp_filter_set(ctx, VP_POLY8_Y0_LL,
 		filter_y_horiz_tap8, sizeof(filter_y_horiz_tap8));
-	vp_filter_set(res, VP_POLY4_Y0_LL,
+	vp_filter_set(ctx, VP_POLY4_Y0_LL,
 		filter_y_vert_tap4, sizeof(filter_y_vert_tap4));
-	vp_filter_set(res, VP_POLY4_C0_LL,
+	vp_filter_set(ctx, VP_POLY4_C0_LL,
 		filter_cr_horiz_tap4, sizeof(filter_cr_horiz_tap4));
 }
 
 static void mixer_cfg_gfx_blend(struct mixer_context *ctx, unsigned int win,
 				bool alpha)
 {
-	struct mixer_resources *res = &ctx->mixer_res;
 	u32 val;
 
 	val  = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
@@ -335,13 +330,12 @@ static void mixer_cfg_gfx_blend(struct mixer_context *ctx, unsigned int win,
 		val |= MXR_GRP_CFG_BLEND_PRE_MUL;
 		val |= MXR_GRP_CFG_PIXEL_BLEND_EN;
 	}
-	mixer_reg_writemask(res, MXR_GRAPHIC_CFG(win),
+	mixer_reg_writemask(ctx, MXR_GRAPHIC_CFG(win),
 			    val, MXR_GRP_CFG_MISC_MASK);
 }
 
 static void mixer_cfg_vp_blend(struct mixer_context *ctx)
 {
-	struct mixer_resources *res = &ctx->mixer_res;
 	u32 val;
 
 	/*
@@ -351,25 +345,22 @@ static void mixer_cfg_vp_blend(struct mixer_context *ctx)
 	 * support blending of the video layer through this.
 	 */
 	val = 0;
-	mixer_reg_write(res, MXR_VIDEO_CFG, val);
+	mixer_reg_write(ctx, MXR_VIDEO_CFG, val);
 }
 
 static void mixer_vsync_set_update(struct mixer_context *ctx, bool enable)
 {
-	struct mixer_resources *res = &ctx->mixer_res;
-
 	/* block update on vsync */
-	mixer_reg_writemask(res, MXR_STATUS, enable ?
+	mixer_reg_writemask(ctx, MXR_STATUS, enable ?
 			MXR_STATUS_SYNC_ENABLE : 0, MXR_STATUS_SYNC_ENABLE);
 
 	if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags))
-		vp_reg_write(res, VP_SHADOW_UPDATE, enable ?
+		vp_reg_write(ctx, VP_SHADOW_UPDATE, enable ?
 			VP_SHADOW_UPDATE_ENABLE : 0);
 }
 
 static void mixer_cfg_scan(struct mixer_context *ctx, int width, int height)
 {
-	struct mixer_resources *res = &ctx->mixer_res;
 	u32 val;
 
 	/* choosing between interlace and progressive mode */
@@ -378,7 +369,7 @@ static void mixer_cfg_scan(struct mixer_context *ctx, int width, int height)
 
 	/* setup display size */
 	if (ctx->mxr_ver == MXR_VER_128_0_0_184) {
-		mixer_reg_write(&ctx->mixer_res, MXR_RESOLUTION,
+		mixer_reg_write(ctx, MXR_RESOLUTION,
 			MXR_MXR_RES_HEIGHT(height) | MXR_MXR_RES_WIDTH(width));
 	} else {
 		/* choosing between proper HD and SD mode */
@@ -394,12 +385,11 @@ static void mixer_cfg_scan(struct mixer_context *ctx, int width, int height)
 			val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD;
 	}
 
-	mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_SCAN_MASK);
+	mixer_reg_writemask(ctx, MXR_CFG, val, MXR_CFG_SCAN_MASK);
 }
 
 static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height)
 {
-	struct mixer_resources *res = &ctx->mixer_res;
 	u32 val;
 
 	switch (height) {
@@ -412,45 +402,44 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height)
 	default:
 		val = MXR_CFG_RGB709_16_235;
 		/* Configure the BT.709 CSC matrix for full range RGB. */
-		mixer_reg_write(res, MXR_CM_COEFF_Y,
+		mixer_reg_write(ctx, MXR_CM_COEFF_Y,
 			MXR_CSC_CT( 0.184,  0.614,  0.063) |
 			MXR_CM_COEFF_RGB_FULL);
-		mixer_reg_write(res, MXR_CM_COEFF_CB,
+		mixer_reg_write(ctx, MXR_CM_COEFF_CB,
 			MXR_CSC_CT(-0.102, -0.338,  0.440));
-		mixer_reg_write(res, MXR_CM_COEFF_CR,
+		mixer_reg_write(ctx, MXR_CM_COEFF_CR,
 			MXR_CSC_CT( 0.440, -0.399, -0.040));
 		break;
 	}
 
-	mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_RGB_FMT_MASK);
+	mixer_reg_writemask(ctx, MXR_CFG, val, MXR_CFG_RGB_FMT_MASK);
 }
 
 static void mixer_cfg_layer(struct mixer_context *ctx, unsigned int win,
 			    unsigned int priority, bool enable)
 {
-	struct mixer_resources *res = &ctx->mixer_res;
 	u32 val = enable ? ~0 : 0;
 
 	switch (win) {
 	case 0:
-		mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_GRP0_ENABLE);
-		mixer_reg_writemask(res, MXR_LAYER_CFG,
+		mixer_reg_writemask(ctx, MXR_CFG, val, MXR_CFG_GRP0_ENABLE);
+		mixer_reg_writemask(ctx, MXR_LAYER_CFG,
 				    MXR_LAYER_CFG_GRP0_VAL(priority),
 				    MXR_LAYER_CFG_GRP0_MASK);
 		break;
 	case 1:
-		mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_GRP1_ENABLE);
-		mixer_reg_writemask(res, MXR_LAYER_CFG,
+		mixer_reg_writemask(ctx, MXR_CFG, val, MXR_CFG_GRP1_ENABLE);
+		mixer_reg_writemask(ctx, MXR_LAYER_CFG,
 				    MXR_LAYER_CFG_GRP1_VAL(priority),
 				    MXR_LAYER_CFG_GRP1_MASK);
 
 		break;
 	case VP_DEFAULT_WIN:
 		if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) {
-			vp_reg_writemask(res, VP_ENABLE, val, VP_ENABLE_ON);
-			mixer_reg_writemask(res, MXR_CFG, val,
+			vp_reg_writemask(ctx, VP_ENABLE, val, VP_ENABLE_ON);
+			mixer_reg_writemask(ctx, MXR_CFG, val,
 				MXR_CFG_VP_ENABLE);
-			mixer_reg_writemask(res, MXR_LAYER_CFG,
+			mixer_reg_writemask(ctx, MXR_LAYER_CFG,
 					    MXR_LAYER_CFG_VP_VAL(priority),
 					    MXR_LAYER_CFG_VP_MASK);
 		}
@@ -460,19 +449,16 @@ static void mixer_cfg_layer(struct mixer_context *ctx, unsigned int win,
 
 static void mixer_run(struct mixer_context *ctx)
 {
-	struct mixer_resources *res = &ctx->mixer_res;
-
-	mixer_reg_writemask(res, MXR_STATUS, ~0, MXR_STATUS_REG_RUN);
+	mixer_reg_writemask(ctx, MXR_STATUS, ~0, MXR_STATUS_REG_RUN);
 }
 
 static void mixer_stop(struct mixer_context *ctx)
 {
-	struct mixer_resources *res = &ctx->mixer_res;
 	int timeout = 20;
 
-	mixer_reg_writemask(res, MXR_STATUS, 0, MXR_STATUS_REG_RUN);
+	mixer_reg_writemask(ctx, MXR_STATUS, 0, MXR_STATUS_REG_RUN);
 
-	while (!(mixer_reg_read(res, MXR_STATUS) & MXR_STATUS_REG_IDLE) &&
+	while (!(mixer_reg_read(ctx, MXR_STATUS) & MXR_STATUS_REG_IDLE) &&
 			--timeout)
 		usleep_range(10000, 12000);
 }
@@ -496,7 +482,6 @@ static void vp_video_buffer(struct mixer_context *ctx,
 {
 	struct exynos_drm_plane_state *state =
 				to_exynos_plane_state(plane->base.state);
-	struct mixer_resources *res = &ctx->mixer_res;
 	struct drm_framebuffer *fb = state->base.fb;
 	unsigned int priority = state->base.normalized_zpos + 1;
 	unsigned long flags;
@@ -523,55 +508,55 @@ static void vp_video_buffer(struct mixer_context *ctx,
 		chroma_addr[1] = 0;
 	}
 
-	spin_lock_irqsave(&res->reg_slock, flags);
+	spin_lock_irqsave(&ctx->reg_slock, flags);
 
 	/* interlace or progressive scan mode */
 	val = (test_bit(MXR_BIT_INTERLACE, &ctx->flags) ? ~0 : 0);
-	vp_reg_writemask(res, VP_MODE, val, VP_MODE_LINE_SKIP);
+	vp_reg_writemask(ctx, VP_MODE, val, VP_MODE_LINE_SKIP);
 
 	/* setup format */
 	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);
+	vp_reg_writemask(ctx, VP_MODE, val, VP_MODE_FMT_MASK);
 
 	/* setting size of input image */
-	vp_reg_write(res, VP_IMG_SIZE_Y, VP_IMG_HSIZE(fb->pitches[0]) |
+	vp_reg_write(ctx, VP_IMG_SIZE_Y, VP_IMG_HSIZE(fb->pitches[0]) |
 		VP_IMG_VSIZE(fb->height));
 	/* 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_reg_write(ctx, VP_IMG_SIZE_C, VP_IMG_HSIZE(fb->pitches[0]) |
 		VP_IMG_VSIZE(fb->height / 2));
 
-	vp_reg_write(res, VP_SRC_WIDTH, state->src.w);
-	vp_reg_write(res, VP_SRC_HEIGHT, state->src.h);
-	vp_reg_write(res, VP_SRC_H_POSITION,
+	vp_reg_write(ctx, VP_SRC_WIDTH, state->src.w);
+	vp_reg_write(ctx, VP_SRC_HEIGHT, state->src.h);
+	vp_reg_write(ctx, VP_SRC_H_POSITION,
 			VP_SRC_H_POSITION_VAL(state->src.x));
-	vp_reg_write(res, VP_SRC_V_POSITION, state->src.y);
+	vp_reg_write(ctx, VP_SRC_V_POSITION, state->src.y);
 
-	vp_reg_write(res, VP_DST_WIDTH, state->crtc.w);
-	vp_reg_write(res, VP_DST_H_POSITION, state->crtc.x);
+	vp_reg_write(ctx, VP_DST_WIDTH, state->crtc.w);
+	vp_reg_write(ctx, VP_DST_H_POSITION, state->crtc.x);
 	if (test_bit(MXR_BIT_INTERLACE, &ctx->flags)) {
-		vp_reg_write(res, VP_DST_HEIGHT, state->crtc.h / 2);
-		vp_reg_write(res, VP_DST_V_POSITION, state->crtc.y / 2);
+		vp_reg_write(ctx, VP_DST_HEIGHT, state->crtc.h / 2);
+		vp_reg_write(ctx, VP_DST_V_POSITION, state->crtc.y / 2);
 	} else {
-		vp_reg_write(res, VP_DST_HEIGHT, state->crtc.h);
-		vp_reg_write(res, VP_DST_V_POSITION, state->crtc.y);
+		vp_reg_write(ctx, VP_DST_HEIGHT, state->crtc.h);
+		vp_reg_write(ctx, VP_DST_V_POSITION, state->crtc.y);
 	}
 
-	vp_reg_write(res, VP_H_RATIO, state->h_ratio);
-	vp_reg_write(res, VP_V_RATIO, state->v_ratio);
+	vp_reg_write(ctx, VP_H_RATIO, state->h_ratio);
+	vp_reg_write(ctx, VP_V_RATIO, state->v_ratio);
 
-	vp_reg_write(res, VP_ENDIAN_MODE, VP_ENDIAN_MODE_LITTLE);
+	vp_reg_write(ctx, VP_ENDIAN_MODE, VP_ENDIAN_MODE_LITTLE);
 
 	/* set buffer address to vp */
-	vp_reg_write(res, VP_TOP_Y_PTR, luma_addr[0]);
-	vp_reg_write(res, VP_BOT_Y_PTR, luma_addr[1]);
-	vp_reg_write(res, VP_TOP_C_PTR, chroma_addr[0]);
-	vp_reg_write(res, VP_BOT_C_PTR, chroma_addr[1]);
+	vp_reg_write(ctx, VP_TOP_Y_PTR, luma_addr[0]);
+	vp_reg_write(ctx, VP_BOT_Y_PTR, luma_addr[1]);
+	vp_reg_write(ctx, VP_TOP_C_PTR, chroma_addr[0]);
+	vp_reg_write(ctx, VP_BOT_C_PTR, chroma_addr[1]);
 
 	mixer_cfg_layer(ctx, plane->index, priority, true);
 	mixer_cfg_vp_blend(ctx);
 
-	spin_unlock_irqrestore(&res->reg_slock, flags);
+	spin_unlock_irqrestore(&ctx->reg_slock, flags);
 
 	mixer_regs_dump(ctx);
 	vp_regs_dump(ctx);
@@ -579,9 +564,7 @@ static void vp_video_buffer(struct mixer_context *ctx,
 
 static void mixer_layer_update(struct mixer_context *ctx)
 {
-	struct mixer_resources *res = &ctx->mixer_res;
-
-	mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
+	mixer_reg_writemask(ctx, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
 }
 
 static void mixer_graph_buffer(struct mixer_context *ctx,
@@ -589,7 +572,6 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
 {
 	struct exynos_drm_plane_state *state =
 				to_exynos_plane_state(plane->base.state);
-	struct mixer_resources *res = &ctx->mixer_res;
 	struct drm_framebuffer *fb = state->base.fb;
 	unsigned int priority = state->base.normalized_zpos + 1;
 	unsigned long flags;
@@ -634,29 +616,29 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
 		+ (state->src.x * fb->format->cpp[0])
 		+ (state->src.y * fb->pitches[0]);
 
-	spin_lock_irqsave(&res->reg_slock, flags);
+	spin_lock_irqsave(&ctx->reg_slock, flags);
 
 	/* setup format */
-	mixer_reg_writemask(res, MXR_GRAPHIC_CFG(win),
+	mixer_reg_writemask(ctx, MXR_GRAPHIC_CFG(win),
 		MXR_GRP_CFG_FORMAT_VAL(fmt), MXR_GRP_CFG_FORMAT_MASK);
 
 	/* setup geometry */
-	mixer_reg_write(res, MXR_GRAPHIC_SPAN(win),
+	mixer_reg_write(ctx, MXR_GRAPHIC_SPAN(win),
 			fb->pitches[0] / fb->format->cpp[0]);
 
 	val  = MXR_GRP_WH_WIDTH(state->src.w);
 	val |= MXR_GRP_WH_HEIGHT(state->src.h);
 	val |= MXR_GRP_WH_H_SCALE(x_ratio);
 	val |= MXR_GRP_WH_V_SCALE(y_ratio);
-	mixer_reg_write(res, MXR_GRAPHIC_WH(win), val);
+	mixer_reg_write(ctx, MXR_GRAPHIC_WH(win), val);
 
 	/* setup offsets in display image */
 	val  = MXR_GRP_DXY_DX(dst_x_offset);
 	val |= MXR_GRP_DXY_DY(dst_y_offset);
-	mixer_reg_write(res, MXR_GRAPHIC_DXY(win), val);
+	mixer_reg_write(ctx, MXR_GRAPHIC_DXY(win), val);
 
 	/* set buffer address to mixer */
-	mixer_reg_write(res, MXR_GRAPHIC_BASE(win), dma_addr);
+	mixer_reg_write(ctx, MXR_GRAPHIC_BASE(win), dma_addr);
 
 	mixer_cfg_layer(ctx, win, priority, true);
 	mixer_cfg_gfx_blend(ctx, win, is_alpha_format(fb->format->format));
@@ -666,20 +648,19 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
 		ctx->mxr_ver == MXR_VER_128_0_0_184)
 		mixer_layer_update(ctx);
 
-	spin_unlock_irqrestore(&res->reg_slock, flags);
+	spin_unlock_irqrestore(&ctx->reg_slock, flags);
 
 	mixer_regs_dump(ctx);
 }
 
 static void vp_win_reset(struct mixer_context *ctx)
 {
-	struct mixer_resources *res = &ctx->mixer_res;
 	unsigned int tries = 100;
 
-	vp_reg_write(res, VP_SRESET, VP_SRESET_PROCESSING);
+	vp_reg_write(ctx, VP_SRESET, VP_SRESET_PROCESSING);
 	while (--tries) {
 		/* waiting until VP_SRESET_PROCESSING is 0 */
-		if (~vp_reg_read(res, VP_SRESET) & VP_SRESET_PROCESSING)
+		if (~vp_reg_read(ctx, VP_SRESET) & VP_SRESET_PROCESSING)
 			break;
 		mdelay(10);
 	}
@@ -688,57 +669,55 @@ static void vp_win_reset(struct mixer_context *ctx)
 
 static void mixer_win_reset(struct mixer_context *ctx)
 {
-	struct mixer_resources *res = &ctx->mixer_res;
 	unsigned long flags;
 
-	spin_lock_irqsave(&res->reg_slock, flags);
+	spin_lock_irqsave(&ctx->reg_slock, flags);
 
-	mixer_reg_writemask(res, MXR_CFG, MXR_CFG_DST_HDMI, MXR_CFG_DST_MASK);
+	mixer_reg_writemask(ctx, MXR_CFG, MXR_CFG_DST_HDMI, MXR_CFG_DST_MASK);
 
 	/* set output in RGB888 mode */
-	mixer_reg_writemask(res, MXR_CFG, MXR_CFG_OUT_RGB888, MXR_CFG_OUT_MASK);
+	mixer_reg_writemask(ctx, MXR_CFG, MXR_CFG_OUT_RGB888, MXR_CFG_OUT_MASK);
 
 	/* 16 beat burst in DMA */
-	mixer_reg_writemask(res, MXR_STATUS, MXR_STATUS_16_BURST,
+	mixer_reg_writemask(ctx, MXR_STATUS, MXR_STATUS_16_BURST,
 		MXR_STATUS_BURST_MASK);
 
 	/* reset default layer priority */
-	mixer_reg_write(res, MXR_LAYER_CFG, 0);
+	mixer_reg_write(ctx, MXR_LAYER_CFG, 0);
 
 	/* set all background colors to RGB (0,0,0) */
-	mixer_reg_write(res, MXR_BG_COLOR0, MXR_YCBCR_VAL(0, 128, 128));
-	mixer_reg_write(res, MXR_BG_COLOR1, MXR_YCBCR_VAL(0, 128, 128));
-	mixer_reg_write(res, MXR_BG_COLOR2, MXR_YCBCR_VAL(0, 128, 128));
+	mixer_reg_write(ctx, MXR_BG_COLOR0, MXR_YCBCR_VAL(0, 128, 128));
+	mixer_reg_write(ctx, MXR_BG_COLOR1, MXR_YCBCR_VAL(0, 128, 128));
+	mixer_reg_write(ctx, MXR_BG_COLOR2, MXR_YCBCR_VAL(0, 128, 128));
 
 	if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) {
 		/* configuration of Video Processor Registers */
 		vp_win_reset(ctx);
-		vp_default_filter(res);
+		vp_default_filter(ctx);
 	}
 
 	/* disable all layers */
-	mixer_reg_writemask(res, MXR_CFG, 0, MXR_CFG_GRP0_ENABLE);
-	mixer_reg_writemask(res, MXR_CFG, 0, MXR_CFG_GRP1_ENABLE);
+	mixer_reg_writemask(ctx, MXR_CFG, 0, MXR_CFG_GRP0_ENABLE);
+	mixer_reg_writemask(ctx, MXR_CFG, 0, MXR_CFG_GRP1_ENABLE);
 	if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags))
-		mixer_reg_writemask(res, MXR_CFG, 0, MXR_CFG_VP_ENABLE);
+		mixer_reg_writemask(ctx, MXR_CFG, 0, MXR_CFG_VP_ENABLE);
 
 	/* set all source image offsets to zero */
-	mixer_reg_write(res, MXR_GRAPHIC_SXY(0), 0);
-	mixer_reg_write(res, MXR_GRAPHIC_SXY(1), 0);
+	mixer_reg_write(ctx, MXR_GRAPHIC_SXY(0), 0);
+	mixer_reg_write(ctx, MXR_GRAPHIC_SXY(1), 0);
 
-	spin_unlock_irqrestore(&res->reg_slock, flags);
+	spin_unlock_irqrestore(&ctx->reg_slock, flags);
 }
 
 static irqreturn_t mixer_irq_handler(int irq, void *arg)
 {
 	struct mixer_context *ctx = arg;
-	struct mixer_resources *res = &ctx->mixer_res;
 	u32 val, base, shadow;
 
-	spin_lock(&res->reg_slock);
+	spin_lock(&ctx->reg_slock);
 
 	/* read interrupt status for handling and clearing flags for VSYNC */
-	val = mixer_reg_read(res, MXR_INT_STATUS);
+	val = mixer_reg_read(ctx, MXR_INT_STATUS);
 
 	/* handling VSYNC */
 	if (val & MXR_INT_STATUS_VSYNC) {
@@ -748,13 +727,13 @@ static irqreturn_t mixer_irq_handler(int irq, void *arg)
 
 		/* interlace scan need to check shadow register */
 		if (test_bit(MXR_BIT_INTERLACE, &ctx->flags)) {
-			base = mixer_reg_read(res, MXR_GRAPHIC_BASE(0));
-			shadow = mixer_reg_read(res, MXR_GRAPHIC_BASE_S(0));
+			base = mixer_reg_read(ctx, MXR_GRAPHIC_BASE(0));
+			shadow = mixer_reg_read(ctx, MXR_GRAPHIC_BASE_S(0));
 			if (base != shadow)
 				goto out;
 
-			base = mixer_reg_read(res, MXR_GRAPHIC_BASE(1));
-			shadow = mixer_reg_read(res, MXR_GRAPHIC_BASE_S(1));
+			base = mixer_reg_read(ctx, MXR_GRAPHIC_BASE(1));
+			shadow = mixer_reg_read(ctx, MXR_GRAPHIC_BASE_S(1));
 			if (base != shadow)
 				goto out;
 		}
@@ -764,9 +743,9 @@ static irqreturn_t mixer_irq_handler(int irq, void *arg)
 
 out:
 	/* clear interrupts */
-	mixer_reg_write(res, MXR_INT_STATUS, val);
+	mixer_reg_write(ctx, MXR_INT_STATUS, val);
 
-	spin_unlock(&res->reg_slock);
+	spin_unlock(&ctx->reg_slock);
 
 	return IRQ_HANDLED;
 }
@@ -774,26 +753,25 @@ static irqreturn_t mixer_irq_handler(int irq, void *arg)
 static int mixer_resources_init(struct mixer_context *mixer_ctx)
 {
 	struct device *dev = &mixer_ctx->pdev->dev;
-	struct mixer_resources *mixer_res = &mixer_ctx->mixer_res;
 	struct resource *res;
 	int ret;
 
-	spin_lock_init(&mixer_res->reg_slock);
+	spin_lock_init(&mixer_ctx->reg_slock);
 
-	mixer_res->mixer = devm_clk_get(dev, "mixer");
-	if (IS_ERR(mixer_res->mixer)) {
+	mixer_ctx->mixer = devm_clk_get(dev, "mixer");
+	if (IS_ERR(mixer_ctx->mixer)) {
 		dev_err(dev, "failed to get clock 'mixer'\n");
 		return -ENODEV;
 	}
 
-	mixer_res->hdmi = devm_clk_get(dev, "hdmi");
-	if (IS_ERR(mixer_res->hdmi)) {
+	mixer_ctx->hdmi = devm_clk_get(dev, "hdmi");
+	if (IS_ERR(mixer_ctx->hdmi)) {
 		dev_err(dev, "failed to get clock 'hdmi'\n");
-		return PTR_ERR(mixer_res->hdmi);
+		return PTR_ERR(mixer_ctx->hdmi);
 	}
 
-	mixer_res->sclk_hdmi = devm_clk_get(dev, "sclk_hdmi");
-	if (IS_ERR(mixer_res->sclk_hdmi)) {
+	mixer_ctx->sclk_hdmi = devm_clk_get(dev, "sclk_hdmi");
+	if (IS_ERR(mixer_ctx->sclk_hdmi)) {
 		dev_err(dev, "failed to get clock 'sclk_hdmi'\n");
 		return -ENODEV;
 	}
@@ -803,9 +781,9 @@ static int mixer_resources_init(struct mixer_context *mixer_ctx)
 		return -ENXIO;
 	}
 
-	mixer_res->mixer_regs = devm_ioremap(dev, res->start,
+	mixer_ctx->mixer_regs = devm_ioremap(dev, res->start,
 							resource_size(res));
-	if (mixer_res->mixer_regs == NULL) {
+	if (mixer_ctx->mixer_regs == NULL) {
 		dev_err(dev, "register mapping failed.\n");
 		return -ENXIO;
 	}
@@ -822,7 +800,7 @@ static int mixer_resources_init(struct mixer_context *mixer_ctx)
 		dev_err(dev, "request interrupt failed.\n");
 		return ret;
 	}
-	mixer_res->irq = res->start;
+	mixer_ctx->irq = res->start;
 
 	return 0;
 }
@@ -830,30 +808,29 @@ static int mixer_resources_init(struct mixer_context *mixer_ctx)
 static int vp_resources_init(struct mixer_context *mixer_ctx)
 {
 	struct device *dev = &mixer_ctx->pdev->dev;
-	struct mixer_resources *mixer_res = &mixer_ctx->mixer_res;
 	struct resource *res;
 
-	mixer_res->vp = devm_clk_get(dev, "vp");
-	if (IS_ERR(mixer_res->vp)) {
+	mixer_ctx->vp = devm_clk_get(dev, "vp");
+	if (IS_ERR(mixer_ctx->vp)) {
 		dev_err(dev, "failed to get clock 'vp'\n");
 		return -ENODEV;
 	}
 
 	if (test_bit(MXR_BIT_HAS_SCLK, &mixer_ctx->flags)) {
-		mixer_res->sclk_mixer = devm_clk_get(dev, "sclk_mixer");
-		if (IS_ERR(mixer_res->sclk_mixer)) {
+		mixer_ctx->sclk_mixer = devm_clk_get(dev, "sclk_mixer");
+		if (IS_ERR(mixer_ctx->sclk_mixer)) {
 			dev_err(dev, "failed to get clock 'sclk_mixer'\n");
 			return -ENODEV;
 		}
-		mixer_res->mout_mixer = devm_clk_get(dev, "mout_mixer");
-		if (IS_ERR(mixer_res->mout_mixer)) {
+		mixer_ctx->mout_mixer = devm_clk_get(dev, "mout_mixer");
+		if (IS_ERR(mixer_ctx->mout_mixer)) {
 			dev_err(dev, "failed to get clock 'mout_mixer'\n");
 			return -ENODEV;
 		}
 
-		if (mixer_res->sclk_hdmi && mixer_res->mout_mixer)
-			clk_set_parent(mixer_res->mout_mixer,
-				       mixer_res->sclk_hdmi);
+		if (mixer_ctx->sclk_hdmi && mixer_ctx->mout_mixer)
+			clk_set_parent(mixer_ctx->mout_mixer,
+				       mixer_ctx->sclk_hdmi);
 	}
 
 	res = platform_get_resource(mixer_ctx->pdev, IORESOURCE_MEM, 1);
@@ -862,9 +839,9 @@ static int vp_resources_init(struct mixer_context *mixer_ctx)
 		return -ENXIO;
 	}
 
-	mixer_res->vp_regs = devm_ioremap(dev, res->start,
+	mixer_ctx->vp_regs = devm_ioremap(dev, res->start,
 							resource_size(res));
-	if (mixer_res->vp_regs == NULL) {
+	if (mixer_ctx->vp_regs == NULL) {
 		dev_err(dev, "register mapping failed.\n");
 		return -ENXIO;
 	}
@@ -908,15 +885,14 @@ static void mixer_ctx_remove(struct mixer_context *mixer_ctx)
 static int mixer_enable_vblank(struct exynos_drm_crtc *crtc)
 {
 	struct mixer_context *mixer_ctx = crtc->ctx;
-	struct mixer_resources *res = &mixer_ctx->mixer_res;
 
 	__set_bit(MXR_BIT_VSYNC, &mixer_ctx->flags);
 	if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags))
 		return 0;
 
 	/* enable vsync interrupt */
-	mixer_reg_writemask(res, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC);
-	mixer_reg_writemask(res, MXR_INT_EN, ~0, MXR_INT_EN_VSYNC);
+	mixer_reg_writemask(mixer_ctx, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC);
+	mixer_reg_writemask(mixer_ctx, MXR_INT_EN, ~0, MXR_INT_EN_VSYNC);
 
 	return 0;
 }
@@ -924,7 +900,6 @@ static int mixer_enable_vblank(struct exynos_drm_crtc *crtc)
 static void mixer_disable_vblank(struct exynos_drm_crtc *crtc)
 {
 	struct mixer_context *mixer_ctx = crtc->ctx;
-	struct mixer_resources *res = &mixer_ctx->mixer_res;
 
 	__clear_bit(MXR_BIT_VSYNC, &mixer_ctx->flags);
 
@@ -932,8 +907,8 @@ static void mixer_disable_vblank(struct exynos_drm_crtc *crtc)
 		return;
 
 	/* disable vsync interrupt */
-	mixer_reg_writemask(res, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC);
-	mixer_reg_writemask(res, MXR_INT_EN, 0, MXR_INT_EN_VSYNC);
+	mixer_reg_writemask(mixer_ctx, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC);
+	mixer_reg_writemask(mixer_ctx, MXR_INT_EN, 0, MXR_INT_EN_VSYNC);
 }
 
 static void mixer_atomic_begin(struct exynos_drm_crtc *crtc)
@@ -966,7 +941,6 @@ static void mixer_disable_plane(struct exynos_drm_crtc *crtc,
 				struct exynos_drm_plane *plane)
 {
 	struct mixer_context *mixer_ctx = crtc->ctx;
-	struct mixer_resources *res = &mixer_ctx->mixer_res;
 	unsigned long flags;
 
 	DRM_DEBUG_KMS("win: %d\n", plane->index);
@@ -974,9 +948,9 @@ static void mixer_disable_plane(struct exynos_drm_crtc *crtc,
 	if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags))
 		return;
 
-	spin_lock_irqsave(&res->reg_slock, flags);
+	spin_lock_irqsave(&mixer_ctx->reg_slock, flags);
 	mixer_cfg_layer(mixer_ctx, plane->index, 0, false);
-	spin_unlock_irqrestore(&res->reg_slock, flags);
+	spin_unlock_irqrestore(&mixer_ctx->reg_slock, flags);
 }
 
 static void mixer_atomic_flush(struct exynos_drm_crtc *crtc)
@@ -993,7 +967,6 @@ static void mixer_atomic_flush(struct exynos_drm_crtc *crtc)
 static void mixer_enable(struct exynos_drm_crtc *crtc)
 {
 	struct mixer_context *ctx = crtc->ctx;
-	struct mixer_resources *res = &ctx->mixer_res;
 
 	if (test_bit(MXR_BIT_POWERED, &ctx->flags))
 		return;
@@ -1004,11 +977,11 @@ static void mixer_enable(struct exynos_drm_crtc *crtc)
 
 	mixer_vsync_set_update(ctx, false);
 
-	mixer_reg_writemask(res, MXR_STATUS, ~0, MXR_STATUS_SOFT_RESET);
+	mixer_reg_writemask(ctx, MXR_STATUS, ~0, MXR_STATUS_SOFT_RESET);
 
 	if (test_bit(MXR_BIT_VSYNC, &ctx->flags)) {
-		mixer_reg_writemask(res, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC);
-		mixer_reg_writemask(res, MXR_INT_EN, ~0, MXR_INT_EN_VSYNC);
+		mixer_reg_writemask(ctx, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC);
+		mixer_reg_writemask(ctx, MXR_INT_EN, ~0, MXR_INT_EN_VSYNC);
 	}
 	mixer_win_reset(ctx);
 
@@ -1211,14 +1184,13 @@ static int mixer_remove(struct platform_device *pdev)
 static int __maybe_unused exynos_mixer_suspend(struct device *dev)
 {
 	struct mixer_context *ctx = dev_get_drvdata(dev);
-	struct mixer_resources *res = &ctx->mixer_res;
 
-	clk_disable_unprepare(res->hdmi);
-	clk_disable_unprepare(res->mixer);
+	clk_disable_unprepare(ctx->hdmi);
+	clk_disable_unprepare(ctx->mixer);
 	if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) {
-		clk_disable_unprepare(res->vp);
+		clk_disable_unprepare(ctx->vp);
 		if (test_bit(MXR_BIT_HAS_SCLK, &ctx->flags))
-			clk_disable_unprepare(res->sclk_mixer);
+			clk_disable_unprepare(ctx->sclk_mixer);
 	}
 
 	return 0;
@@ -1227,28 +1199,27 @@ static int __maybe_unused exynos_mixer_suspend(struct device *dev)
 static int __maybe_unused exynos_mixer_resume(struct device *dev)
 {
 	struct mixer_context *ctx = dev_get_drvdata(dev);
-	struct mixer_resources *res = &ctx->mixer_res;
 	int ret;
 
-	ret = clk_prepare_enable(res->mixer);
+	ret = clk_prepare_enable(ctx->mixer);
 	if (ret < 0) {
 		DRM_ERROR("Failed to prepare_enable the mixer clk [%d]\n", ret);
 		return ret;
 	}
-	ret = clk_prepare_enable(res->hdmi);
+	ret = clk_prepare_enable(ctx->hdmi);
 	if (ret < 0) {
 		DRM_ERROR("Failed to prepare_enable the hdmi clk [%d]\n", ret);
 		return ret;
 	}
 	if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) {
-		ret = clk_prepare_enable(res->vp);
+		ret = clk_prepare_enable(ctx->vp);
 		if (ret < 0) {
 			DRM_ERROR("Failed to prepare_enable the vp clk [%d]\n",
 				  ret);
 			return ret;
 		}
 		if (test_bit(MXR_BIT_HAS_SCLK, &ctx->flags)) {
-			ret = clk_prepare_enable(res->sclk_mixer);
+			ret = clk_prepare_enable(ctx->sclk_mixer);
 			if (ret < 0) {
 				DRM_ERROR("Failed to prepare_enable the " \
 					   "sclk_mixer clk [%d]\n",
-- 
2.7.4

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

[PATCH 06/10] drm/exynos/hdmi: remove redundant mode field

[Andrzej Hajda]

Display mode is preserved in CRTC state, there is no need to keep local
copy of it. Moreover since HDMI should configure registers according to
requested mode, use it instead of adjusted_mode, which should contain
mode produced by CRTC - functionally it does not change anything, but
subsequent patches will make the difference.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 34 +++++++++-------------------------
 1 file changed, 9 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 214fa5e..7225b65 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -119,7 +119,6 @@ struct hdmi_context {
 	bool				powered;
 	bool				dvi_mode;
 	struct delayed_work		hotplug_work;
-	struct drm_display_mode		current_mode;
 	struct cec_notifier		*notifier;
 	const struct hdmi_driver_data	*drv_data;
 
@@ -770,6 +769,7 @@ static int hdmi_clk_set_parents(struct hdmi_context *hdata, bool to_phy)
 
 static void hdmi_reg_infoframes(struct hdmi_context *hdata)
 {
+	struct drm_display_mode *m = &hdata->encoder.crtc->state->mode;
 	union hdmi_infoframe frm;
 	u8 buf[25];
 	int ret;
@@ -783,8 +783,7 @@ static void hdmi_reg_infoframes(struct hdmi_context *hdata)
 		return;
 	}
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
-			&hdata->current_mode, false);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi, m, false);
 	if (!ret)
 		ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
 	if (ret > 0) {
@@ -794,8 +793,7 @@ static void hdmi_reg_infoframes(struct hdmi_context *hdata)
 		DRM_INFO("%s: invalid AVI infoframe (%d)\n", __func__, ret);
 	}
 
-	ret = drm_hdmi_vendor_infoframe_from_display_mode(&frm.vendor.hdmi,
-			&hdata->current_mode);
+	ret = drm_hdmi_vendor_infoframe_from_display_mode(&frm.vendor.hdmi, m);
 	if (!ret)
 		ret = hdmi_vendor_infoframe_pack(&frm.vendor.hdmi, buf,
 				sizeof(buf));
@@ -1088,9 +1086,10 @@ static void hdmi_audio_control(struct hdmi_context *hdata, bool onoff)
 
 static void hdmi_start(struct hdmi_context *hdata, bool start)
 {
+	struct drm_display_mode *m = &hdata->encoder.crtc->state->mode;
 	u32 val = start ? HDMI_TG_EN : 0;
 
-	if (hdata->current_mode.flags & DRM_MODE_FLAG_INTERLACE)
+	if (m->flags & DRM_MODE_FLAG_INTERLACE)
 		val |= HDMI_FIELD_EN;
 
 	hdmi_reg_writemask(hdata, HDMI_CON_0, val, HDMI_EN);
@@ -1160,7 +1159,7 @@ static void hdmiphy_wait_for_pll(struct hdmi_context *hdata)
 
 static void hdmi_v13_mode_apply(struct hdmi_context *hdata)
 {
-	struct drm_display_mode *m = &hdata->current_mode;
+	struct drm_display_mode *m = &hdata->encoder.crtc->state->mode;
 	unsigned int val;
 
 	hdmi_reg_writev(hdata, HDMI_H_BLANK_0, 2, m->htotal - m->hdisplay);
@@ -1239,7 +1238,7 @@ static void hdmi_v13_mode_apply(struct hdmi_context *hdata)
 
 static void hdmi_v14_mode_apply(struct hdmi_context *hdata)
 {
-	struct drm_display_mode *m = &hdata->current_mode;
+	struct drm_display_mode *m = &hdata->encoder.crtc->state->mode;
 
 	hdmi_reg_writev(hdata, HDMI_H_BLANK_0, 2, m->htotal - m->hdisplay);
 	hdmi_reg_writev(hdata, HDMI_V_LINE_0, 2, m->vtotal);
@@ -1372,10 +1371,11 @@ static void hdmiphy_enable_mode_set(struct hdmi_context *hdata, bool enable)
 
 static void hdmiphy_conf_apply(struct hdmi_context *hdata)
 {
+	struct drm_display_mode *m = &hdata->encoder.crtc->state->mode;
 	int ret;
 	const u8 *phy_conf;
 
-	ret = hdmi_find_phy_conf(hdata, hdata->current_mode.clock * 1000);
+	ret = hdmi_find_phy_conf(hdata, m->clock * 1000);
 	if (ret < 0) {
 		DRM_ERROR("failed to find hdmiphy conf\n");
 		return;
@@ -1407,21 +1407,6 @@ static void hdmi_conf_apply(struct hdmi_context *hdata)
 	hdmi_audio_control(hdata, true);
 }
 
-static void hdmi_mode_set(struct drm_encoder *encoder,
-			  struct drm_display_mode *mode,
-			  struct drm_display_mode *adjusted_mode)
-{
-	struct hdmi_context *hdata = encoder_to_hdmi(encoder);
-	struct drm_display_mode *m = adjusted_mode;
-
-	DRM_DEBUG_KMS("xres=%d, yres=%d, refresh=%d, intl=%s\n",
-		m->hdisplay, m->vdisplay,
-		m->vrefresh, (m->flags & DRM_MODE_FLAG_INTERLACE) ?
-		"INTERLACED" : "PROGRESSIVE");
-
-	drm_mode_copy(&hdata->current_mode, m);
-}
-
 static void hdmi_set_refclk(struct hdmi_context *hdata, bool on)
 {
 	if (!hdata->sysreg)
@@ -1504,7 +1489,6 @@ static void hdmi_disable(struct drm_encoder *encoder)
 
 static const struct drm_encoder_helper_funcs exynos_hdmi_encoder_helper_funcs = {
 	.mode_fixup	= hdmi_mode_fixup,
-	.mode_set	= hdmi_mode_set,
 	.enable		= hdmi_enable,
 	.disable	= hdmi_disable,
 };
-- 
2.7.4

[PATCH 07/10] drm/exynos: add mode_fixup callback to exynos_drm_crtc_ops

[Andrzej Hajda]

crtc::mode_fixup callback is required by crtcs which use internally
different mode than requested by user - case of Exynos Mixer.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c | 15 +++++++++++++++
 drivers/gpu/drm/exynos/exynos_drm_drv.h  |  3 +++
 2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 6ce0821..dc01342 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -95,8 +95,23 @@ static enum drm_mode_status exynos_crtc_mode_valid(struct drm_crtc *crtc,
 	return MODE_OK;
 }
 
+static bool exynos_crtc_mode_fixup(struct drm_crtc *crtc,
+		const struct drm_display_mode *mode,
+		struct drm_display_mode *adjusted_mode)
+{
+	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
+
+	if (exynos_crtc->ops->mode_fixup)
+		return exynos_crtc->ops->mode_fixup(exynos_crtc, mode,
+				adjusted_mode);
+
+	return true;
+}
+
+
 static const struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
 	.mode_valid	= exynos_crtc_mode_valid,
+	.mode_fixup	= exynos_crtc_mode_fixup,
 	.atomic_check	= exynos_crtc_atomic_check,
 	.atomic_begin	= exynos_crtc_atomic_begin,
 	.atomic_flush	= exynos_crtc_atomic_flush,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index cf131c2..e8bcc72 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -136,6 +136,9 @@ struct exynos_drm_crtc_ops {
 	u32 (*get_vblank_counter)(struct exynos_drm_crtc *crtc);
 	enum drm_mode_status (*mode_valid)(struct exynos_drm_crtc *crtc,
 		const struct drm_display_mode *mode);
+	bool (*mode_fixup)(struct exynos_drm_crtc *crtc,
+			   const struct drm_display_mode *mode,
+			   struct drm_display_mode *adjusted_mode);
 	int (*atomic_check)(struct exynos_drm_crtc *crtc,
 			    struct drm_crtc_state *state);
 	void (*atomic_begin)(struct exynos_drm_crtc *crtc);
-- 
2.7.4

[PATCH 08/10] drm/exynos/mixer: pass actual mode on MIXER to encoder

[Andrzej Hajda]

MIXER in SoCs prior to Exynos5420 supports only 4 video modes:
720x480, 720x576, 1280x720, 1920x1080. Support for other modes can be
enabled by manipulating timings of HDMI. To allow it MIXER must pass
actual video mode to HDMI, the proper way to do it is to modify
adjusted_mode property in crtc::mode_fixup callback. Adding such callback
allows also to simplify mixer_cfg_scan code - choosing mode is performed
already in crtc::mode_fixup. mode_fixup is also better place to check
interlace flag.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 70 +++++++++++++++++++++++++----------
 1 file changed, 50 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index f6ea9d9..5aae82b 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -115,6 +115,7 @@ struct mixer_context {
 	struct clk		*sclk_hdmi;
 	struct clk		*mout_mixer;
 	enum mixer_version_id	mxr_ver;
+	int			scan_value;
 };
 
 struct mixer_drv_data {
@@ -367,23 +368,11 @@ static void mixer_cfg_scan(struct mixer_context *ctx, int width, int height)
 	val = test_bit(MXR_BIT_INTERLACE, &ctx->flags) ?
 		MXR_CFG_SCAN_INTERLACE : MXR_CFG_SCAN_PROGRESSIVE;
 
-	/* setup display size */
-	if (ctx->mxr_ver == MXR_VER_128_0_0_184) {
+	if (ctx->mxr_ver == MXR_VER_128_0_0_184)
 		mixer_reg_write(ctx, MXR_RESOLUTION,
 			MXR_MXR_RES_HEIGHT(height) | MXR_MXR_RES_WIDTH(width));
-	} else {
-		/* choosing between proper HD and SD mode */
-		if (height <= 480)
-			val |= MXR_CFG_SCAN_NTSC | MXR_CFG_SCAN_SD;
-		else if (height <= 576)
-			val |= MXR_CFG_SCAN_PAL | MXR_CFG_SCAN_SD;
-		else if (height <= 720)
-			val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD;
-		else if (height <= 1080)
-			val |= MXR_CFG_SCAN_HD_1080 | MXR_CFG_SCAN_HD;
-		else
-			val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD;
-	}
+	else
+		val |= ctx->scan_value;
 
 	mixer_reg_writemask(ctx, MXR_CFG, val, MXR_CFG_SCAN_MASK);
 }
@@ -467,11 +456,6 @@ static void mixer_commit(struct mixer_context *ctx)
 {
 	struct drm_display_mode *mode = &ctx->crtc->base.state->adjusted_mode;
 
-	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
-		__set_bit(MXR_BIT_INTERLACE, &ctx->flags);
-	else
-		__clear_bit(MXR_BIT_INTERLACE, &ctx->flags);
-
 	mixer_cfg_scan(ctx, mode->hdisplay, mode->vdisplay);
 	mixer_cfg_rgb_fmt(ctx, mode->vdisplay);
 	mixer_run(ctx);
@@ -1033,6 +1017,51 @@ static int mixer_mode_valid(struct exynos_drm_crtc *crtc,
 	return MODE_BAD;
 }
 
+static bool mixer_mode_fixup(struct exynos_drm_crtc *crtc,
+		   const struct drm_display_mode *mode,
+		   struct drm_display_mode *adjusted_mode)
+{
+	struct mixer_context *ctx = crtc->ctx;
+	int width = mode->hdisplay, height = mode->vdisplay, i;
+
+	struct {
+		int hdisplay, vdisplay, htotal, vtotal, scan_val;
+	} static const modes[] = {
+		{ 720, 480, 858, 525, MXR_CFG_SCAN_NTSC | MXR_CFG_SCAN_SD },
+		{ 720, 576, 864, 625, MXR_CFG_SCAN_PAL | MXR_CFG_SCAN_SD },
+		{ 1280, 720, 1650, 750, MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD },
+		{ 1920, 1080, 2200, 1125, MXR_CFG_SCAN_HD_1080 | MXR_CFG_SCAN_HD }
+	};
+
+	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
+		__set_bit(MXR_BIT_INTERLACE, &ctx->flags);
+	else
+		__clear_bit(MXR_BIT_INTERLACE, &ctx->flags);
+
+	if (ctx->mxr_ver == MXR_VER_128_0_0_184)
+		return true;
+
+	for (i = 0; i < ARRAY_SIZE(modes); ++i)
+		if (width <= modes[i].hdisplay && height <= modes[i].vdisplay) {
+			ctx->scan_value = modes[i].scan_val;
+			if (width < modes[i].hdisplay ||
+			    height < modes[i].vdisplay) {
+				adjusted_mode->hdisplay = modes[i].hdisplay;
+				adjusted_mode->hsync_start = modes[i].hdisplay;
+				adjusted_mode->hsync_end = modes[i].htotal;
+				adjusted_mode->htotal = modes[i].htotal;
+				adjusted_mode->vdisplay = modes[i].vdisplay;
+				adjusted_mode->vsync_start = modes[i].vdisplay;
+				adjusted_mode->vsync_end = modes[i].vtotal;
+				adjusted_mode->vtotal = modes[i].vtotal;
+			}
+
+			return true;
+		}
+
+	return false;
+}
+
 static const struct exynos_drm_crtc_ops mixer_crtc_ops = {
 	.enable			= mixer_enable,
 	.disable		= mixer_disable,
@@ -1043,6 +1072,7 @@ static const struct exynos_drm_crtc_ops mixer_crtc_ops = {
 	.disable_plane		= mixer_disable_plane,
 	.atomic_flush		= mixer_atomic_flush,
 	.mode_valid		= mixer_mode_valid,
+	.mode_fixup		= mixer_mode_fixup,
 };
 
 static const struct mixer_drv_data exynos5420_mxr_drv_data = {
-- 
2.7.4

[PATCH 09/10] drm/exynos/hdmi: quirk for support mode timings conversion

[Andrzej Hajda]

MIXER in SoCs prior to Exynos5420 supports only 4 video modes:
720x480, 720x576, 1280x720, 1920x1080. Support for other modes
can be enabled by manipulating timings of HDMI. To do it
adjusted_mode should contain actual mode set on crtc.
With this patch it is possible to enable 1024x768 and 1280x1024
modes in MIXER.

Suggested-by: Daniel Drake <drake@endlessm.com>
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 7225b65..4b081f6 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -1239,6 +1239,17 @@ static void hdmi_v13_mode_apply(struct hdmi_context *hdata)
 static void hdmi_v14_mode_apply(struct hdmi_context *hdata)
 {
 	struct drm_display_mode *m = &hdata->encoder.crtc->state->mode;
+	struct drm_display_mode *am = &hdata->encoder.crtc->state->adjusted_mode;
+	int hquirk = 0;
+
+	/*
+	 * In case video mode coming from CRTC differs from requested one HDMI
+	 * sometimes is able to almost properly perform conversion - only
+	 * first line is distorted.
+	 */
+	if ((m->vdisplay != am->vdisplay) &&
+	    (m->hdisplay == 1280 || m->hdisplay == 1024))
+		hquirk = 258;
 
 	hdmi_reg_writev(hdata, HDMI_H_BLANK_0, 2, m->htotal - m->hdisplay);
 	hdmi_reg_writev(hdata, HDMI_V_LINE_0, 2, m->vtotal);
@@ -1332,8 +1343,8 @@ static void hdmi_v14_mode_apply(struct hdmi_context *hdata)
 	hdmi_reg_writev(hdata, HDMI_V_SYNC_LINE_AFT_PXL_6_0, 2, 0xffff);
 
 	hdmi_reg_writev(hdata, HDMI_TG_H_FSZ_L, 2, m->htotal);
-	hdmi_reg_writev(hdata, HDMI_TG_HACT_ST_L, 2, m->htotal - m->hdisplay);
-	hdmi_reg_writev(hdata, HDMI_TG_HACT_SZ_L, 2, m->hdisplay);
+	hdmi_reg_writev(hdata, HDMI_TG_HACT_ST_L, 2, m->htotal - m->hdisplay - hquirk);
+	hdmi_reg_writev(hdata, HDMI_TG_HACT_SZ_L, 2, m->hdisplay + hquirk);
 	hdmi_reg_writev(hdata, HDMI_TG_V_FSZ_L, 2, m->vtotal);
 	if (hdata->drv_data == &exynos5433_hdmi_driver_data)
 		hdmi_reg_writeb(hdata, HDMI_TG_DECON_EN, 1);
-- 
2.7.4

[PATCH 10/10] drm/exynos/mixer: enable support for 1024x768 and 1280x1024 modes

[Andrzej Hajda]

Since HDMI can handle these modes despite of MIXER limitations lets
enable them.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 5aae82b..108dccb 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -1014,6 +1014,9 @@ static int mixer_mode_valid(struct exynos_drm_crtc *crtc,
 		(w >= 1664 && w <= 1920 && h >= 936 && h <= 1080))
 		return MODE_OK;
 
+	if ((w == 1024 && h == 768) || (w == 1280 && h == 1024))
+		return MODE_OK;
+
 	return MODE_BAD;
 }
 
-- 
2.7.4

Re: [PATCH 00/10] drm/exynos: TV path improvements

[Tobias Jakobi]

Hello Andrzej,

just did a quick test and looks good so far. One thing I noticed is that
compared to Daniel's approach you just manipulate HDMI_TG_HACT_{ST,SZ}_L,
instead of HDMI_TG_VACT_SZ_L + HDMI_TG_HACT_{ST,SZ}_L. I just wanted to make
sure that on the HDMI level, this does the same thing?

I'll try to do a proper review tomorrow.

I also noticed that I have another patch by Daniel in my tree, which, together
with the quirk, enables 1366x768@60Hz. Going to send it shortly, since it could
probably go along with this series.

With best wishes,
Tobias



Andrzej Hajda wrote:
> Hi all,
> 
> This patchset does two main things:
> - removes mode limitation for Exynos542x chips, multiple modes were filtered
>   out due to lack of HW version checking code,
> - enables two modes on older chips, thanks to quirk found by Daniel Drake,
>   and published by Tobias Jakobi [1][2].
> Beside this it consolidates the code and performs multiple cleanups.
> 
> [1]: http://www.spinics.net/lists/linux-samsung-soc/msg24617.html
> [2]: https://www.spinics.net/lists/dri-devel/msg150906.html
> 
> Regards
> Andrzej
> 
> 
> Andrzej Hajda (10):
>   drm/exynos/mixer: abstract out output mode setup code
>   drm/exynos/mixer: move mode commit to enable callback
>   drm/exynos/mixer: move resolution configuration to single function
>   drm/exynos/mixer: fix mode validation code
>   drm/exynos/mixer: remove mixer_resources sub-structure
>   drm/exynos/hdmi: remove redundant mode field
>   drm/exynos: add mode_fixup callback to exynos_drm_crtc_ops
>   drm/exynos/mixer: pass actual mode on MIXER to encoder
>   drm/exynos/hdmi: quirk for support mode timings conversion
>   drm/exynos/mixer: enable support for 1024x768 and 1280x1024 modes
> 
>  drivers/gpu/drm/exynos/exynos_drm_crtc.c |  15 +
>  drivers/gpu/drm/exynos/exynos_drm_drv.h  |   3 +
>  drivers/gpu/drm/exynos/exynos_hdmi.c     |  49 ++--
>  drivers/gpu/drm/exynos/exynos_mixer.c    | 452 +++++++++++++++----------------
>  4 files changed, 265 insertions(+), 254 deletions(-)
> 

Re: [PATCH 00/10] drm/exynos: TV path improvements

[Andrzej Hajda]

On 11.09.2017 15:54, Tobias Jakobi wrote:
> Hello Andrzej,
>
> just did a quick test and looks good so far. One thing I noticed is that
> compared to Daniel's approach you just manipulate HDMI_TG_HACT_{ST,SZ}_L,
> instead of HDMI_TG_VACT_SZ_L + HDMI_TG_HACT_{ST,SZ}_L. I just wanted to make
> sure that on the HDMI level, this does the same thing?

With Daniel's patch 1st line and the last column are missing.
With this patch 1st line is distorted, last column is OK, IMO better result.
Another alternative I see is to hide 1st line and make all columns OK,
but I am not really convinced to it, but it is just my feelings :)

Btw, I have also played with 800x600, but I was not able to find good
timings - HBLANK is too small.
There are also other resolutions which one can try to fix. Especially
after adding more clocks to HDMI-PHY.

Regards
Andrzej



>
> I'll try to do a proper review tomorrow.
>
> I also noticed that I have another patch by Daniel in my tree, which, together
> with the quirk, enables 1366x768@60Hz. Going to send it shortly, since it could
> probably go along with this series.
>
> With best wishes,
> Tobias
>
>
>
> Andrzej Hajda wrote:
>> Hi all,
>>
>> This patchset does two main things:
>> - removes mode limitation for Exynos542x chips, multiple modes were filtered
>>   out due to lack of HW version checking code,
>> - enables two modes on older chips, thanks to quirk found by Daniel Drake,
>>   and published by Tobias Jakobi [1][2].
>> Beside this it consolidates the code and performs multiple cleanups.
>>
>> [1]: http://www.spinics.net/lists/linux-samsung-soc/msg24617.html
>> [2]: https://www.spinics.net/lists/dri-devel/msg150906.html
>>
>> Regards
>> Andrzej
>>
>>
>> Andrzej Hajda (10):
>>   drm/exynos/mixer: abstract out output mode setup code
>>   drm/exynos/mixer: move mode commit to enable callback
>>   drm/exynos/mixer: move resolution configuration to single function
>>   drm/exynos/mixer: fix mode validation code
>>   drm/exynos/mixer: remove mixer_resources sub-structure
>>   drm/exynos/hdmi: remove redundant mode field
>>   drm/exynos: add mode_fixup callback to exynos_drm_crtc_ops
>>   drm/exynos/mixer: pass actual mode on MIXER to encoder
>>   drm/exynos/hdmi: quirk for support mode timings conversion
>>   drm/exynos/mixer: enable support for 1024x768 and 1280x1024 modes
>>
>>  drivers/gpu/drm/exynos/exynos_drm_crtc.c |  15 +
>>  drivers/gpu/drm/exynos/exynos_drm_drv.h  |   3 +
>>  drivers/gpu/drm/exynos/exynos_hdmi.c     |  49 ++--
>>  drivers/gpu/drm/exynos/exynos_mixer.c    | 452 +++++++++++++++----------------
>>  4 files changed, 265 insertions(+), 254 deletions(-)
>>
>
>
>

Re: [PATCH 01/10] drm/exynos/mixer: abstract out output mode setup code

[Tobias Jakobi]

Hello Andrzej,


Andrzej Hajda wrote:
> Mode setup code is called from video plane update and mixer plane update.
> Lets group it together in mixer_commit function like in case of other
> Exynos CRTCs.
Minor typo: Lets --> Let's

Reviewed-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>

With some small question below.


> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 33 ++++++++++++++++++---------------
>  1 file changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 0027554..499ebdc 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -473,6 +473,22 @@ static void mixer_stop(struct mixer_context *ctx)
>  		usleep_range(10000, 12000);
>  }
>  
> +static void mixer_commit(struct mixer_context *ctx)
> +{
> +	struct drm_display_mode *mode = &ctx->crtc->base.state->adjusted_mode;
> +
> +	/* setup display size */
> +	if (ctx->mxr_ver == MXR_VER_128_0_0_184) {
> +		u32 val  = MXR_MXR_RES_HEIGHT(mode->vdisplay)
> +			 | MXR_MXR_RES_WIDTH(mode->hdisplay);
> +		mixer_reg_write(&ctx->mixer_res, MXR_RESOLUTION, val);
> +	}
> +
> +	mixer_cfg_scan(ctx, mode->vdisplay);
> +	mixer_cfg_rgb_fmt(ctx, mode->vdisplay);
> +	mixer_run(ctx);
> +}
> +
>  static void vp_video_buffer(struct mixer_context *ctx,
>  			    struct exynos_drm_plane *plane)
>  {
> @@ -553,11 +569,9 @@ static void vp_video_buffer(struct mixer_context *ctx,
>  	vp_reg_write(res, VP_TOP_C_PTR, chroma_addr[0]);
>  	vp_reg_write(res, VP_BOT_C_PTR, chroma_addr[1]);
>  
> -	mixer_cfg_scan(ctx, mode->vdisplay);
> -	mixer_cfg_rgb_fmt(ctx, mode->vdisplay);
>  	mixer_cfg_layer(ctx, plane->index, priority, true);
>  	mixer_cfg_vp_blend(ctx);
> -	mixer_run(ctx);
> +	mixer_commit(ctx);
>  
>  	spin_unlock_irqrestore(&res->reg_slock, flags);
>  
> @@ -638,14 +652,6 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>  	mixer_reg_write(res, MXR_GRAPHIC_SPAN(win),
>  			fb->pitches[0] / fb->format->cpp[0]);
>  
> -	/* setup display size */
> -	if (ctx->mxr_ver == MXR_VER_128_0_0_184 &&
> -		win == DEFAULT_WIN) {
> -		val  = MXR_MXR_RES_HEIGHT(mode->vdisplay);
> -		val |= MXR_MXR_RES_WIDTH(mode->hdisplay);
> -		mixer_reg_write(res, MXR_RESOLUTION, val);
> -	}
> -
>  	val  = MXR_GRP_WH_WIDTH(state->src.w);
>  	val |= MXR_GRP_WH_HEIGHT(state->src.h);
>  	val |= MXR_GRP_WH_H_SCALE(x_ratio);
> @@ -660,18 +666,15 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>  	/* set buffer address to mixer */
>  	mixer_reg_write(res, MXR_GRAPHIC_BASE(win), dma_addr);
>  
> -	mixer_cfg_scan(ctx, mode->vdisplay);
> -	mixer_cfg_rgb_fmt(ctx, mode->vdisplay);
>  	mixer_cfg_layer(ctx, win, priority, true);
>  	mixer_cfg_gfx_blend(ctx, win, is_alpha_format(fb->format->format));
> +	mixer_commit(ctx);
With this change, mixer_run() is now done before mixer_layer_update(). I just
wanted to make sure that the order of operations isn't critical here.



>  	/* layer update mandatory for mixer 16.0.33.0 */
>  	if (ctx->mxr_ver == MXR_VER_16_0_33_0 ||
>  		ctx->mxr_ver == MXR_VER_128_0_0_184)
>  		mixer_layer_update(ctx);
>  
> -	mixer_run(ctx);
> -
>  	spin_unlock_irqrestore(&res->reg_slock, flags);
>  
>  	mixer_regs_dump(ctx);
> 

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

Re: [PATCH 02/10] drm/exynos/mixer: move mode commit to enable callback

[Tobias Jakobi]

Hello Andrzej,


Andrzej Hajda wrote:
> Mode commit should not be called for every plane separately. It is enough
> to call it once in enable callback. The change requires also that
> interlace check should be moved to mixer_commit. It should be done in
> the same patch to avoid regression.
"...also requires that the interlace check is moved to..."


Reviewed-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>


> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 499ebdc..ae89e53 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -477,6 +477,11 @@ static void mixer_commit(struct mixer_context *ctx)
>  {
>  	struct drm_display_mode *mode = &ctx->crtc->base.state->adjusted_mode;
>  
> +	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> +		__set_bit(MXR_BIT_INTERLACE, &ctx->flags);
> +	else
> +		__clear_bit(MXR_BIT_INTERLACE, &ctx->flags);
> +
>  	/* setup display size */
>  	if (ctx->mxr_ver == MXR_VER_128_0_0_184) {
>  		u32 val  = MXR_MXR_RES_HEIGHT(mode->vdisplay)
> @@ -494,7 +499,6 @@ static void vp_video_buffer(struct mixer_context *ctx,
>  {
>  	struct exynos_drm_plane_state *state =
>  				to_exynos_plane_state(plane->base.state);
> -	struct drm_display_mode *mode = &state->base.crtc->state->adjusted_mode;
>  	struct mixer_resources *res = &ctx->mixer_res;
>  	struct drm_framebuffer *fb = state->base.fb;
>  	unsigned int priority = state->base.normalized_zpos + 1;
> @@ -509,8 +513,7 @@ static void vp_video_buffer(struct mixer_context *ctx,
>  	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 (test_bit(MXR_BIT_INTERLACE, &ctx->flags)) {
>  		if (is_tiled) {
>  			luma_addr[1] = luma_addr[0] + 0x40;
>  			chroma_addr[1] = chroma_addr[0] + 0x40;
> @@ -519,7 +522,6 @@ static void vp_video_buffer(struct mixer_context *ctx,
>  			chroma_addr[1] = chroma_addr[0] + fb->pitches[0];
>  		}
>  	} else {
> -		__clear_bit(MXR_BIT_INTERLACE, &ctx->flags);
>  		luma_addr[1] = 0;
>  		chroma_addr[1] = 0;
>  	}
> @@ -571,7 +573,6 @@ static void vp_video_buffer(struct mixer_context *ctx,
>  
>  	mixer_cfg_layer(ctx, plane->index, priority, true);
>  	mixer_cfg_vp_blend(ctx);
> -	mixer_commit(ctx);
>  
>  	spin_unlock_irqrestore(&res->reg_slock, flags);
>  
> @@ -591,7 +592,6 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>  {
>  	struct exynos_drm_plane_state *state =
>  				to_exynos_plane_state(plane->base.state);
> -	struct drm_display_mode *mode = &state->base.crtc->state->adjusted_mode;
>  	struct mixer_resources *res = &ctx->mixer_res;
>  	struct drm_framebuffer *fb = state->base.fb;
>  	unsigned int priority = state->base.normalized_zpos + 1;
> @@ -637,11 +637,6 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>  		+ (state->src.x * fb->format->cpp[0])
>  		+ (state->src.y * fb->pitches[0]);
>  
> -	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> -		__set_bit(MXR_BIT_INTERLACE, &ctx->flags);
> -	else
> -		__clear_bit(MXR_BIT_INTERLACE, &ctx->flags);
> -
>  	spin_lock_irqsave(&res->reg_slock, flags);
>  
>  	/* setup format */
> @@ -668,7 +663,6 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>  
>  	mixer_cfg_layer(ctx, win, priority, true);
>  	mixer_cfg_gfx_blend(ctx, win, is_alpha_format(fb->format->format));
> -	mixer_commit(ctx);
>  
>  	/* layer update mandatory for mixer 16.0.33.0 */
>  	if (ctx->mxr_ver == MXR_VER_16_0_33_0 ||
> @@ -1021,6 +1015,8 @@ static void mixer_enable(struct exynos_drm_crtc *crtc)
>  	}
>  	mixer_win_reset(ctx);
>  
> +	mixer_commit(ctx);
> +
>  	mixer_vsync_set_update(ctx, true);
>  
>  	set_bit(MXR_BIT_POWERED, &ctx->flags);
> 

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

Re: [PATCH 03/10] drm/exynos/mixer: move resolution configuration to single function

[Tobias Jakobi]

Hello Andrzej,


Andrzej Hajda wrote:
> Screen resolution configuration depends on HW version, let put it into
> single function to make it consistent and simplify the code.
"...let's put it into..."

Reviewed-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>


> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index ae89e53..a87f60b 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -367,7 +367,7 @@ static void mixer_vsync_set_update(struct mixer_context *ctx, bool enable)
>  			VP_SHADOW_UPDATE_ENABLE : 0);
>  }
>  
> -static void mixer_cfg_scan(struct mixer_context *ctx, unsigned int height)
> +static void mixer_cfg_scan(struct mixer_context *ctx, int width, int height)
>  {
>  	struct mixer_resources *res = &ctx->mixer_res;
>  	u32 val;
> @@ -376,7 +376,11 @@ static void mixer_cfg_scan(struct mixer_context *ctx, unsigned int height)
>  	val = test_bit(MXR_BIT_INTERLACE, &ctx->flags) ?
>  		MXR_CFG_SCAN_INTERLACE : MXR_CFG_SCAN_PROGRESSIVE;
>  
> -	if (ctx->mxr_ver != MXR_VER_128_0_0_184) {
> +	/* setup display size */
> +	if (ctx->mxr_ver == MXR_VER_128_0_0_184) {
> +		mixer_reg_write(&ctx->mixer_res, MXR_RESOLUTION,
> +			MXR_MXR_RES_HEIGHT(height) | MXR_MXR_RES_WIDTH(width));
> +	} else {
>  		/* choosing between proper HD and SD mode */
>  		if (height <= 480)
>  			val |= MXR_CFG_SCAN_NTSC | MXR_CFG_SCAN_SD;
> @@ -482,14 +486,7 @@ static void mixer_commit(struct mixer_context *ctx)
>  	else
>  		__clear_bit(MXR_BIT_INTERLACE, &ctx->flags);
>  
> -	/* setup display size */
> -	if (ctx->mxr_ver == MXR_VER_128_0_0_184) {
> -		u32 val  = MXR_MXR_RES_HEIGHT(mode->vdisplay)
> -			 | MXR_MXR_RES_WIDTH(mode->hdisplay);
> -		mixer_reg_write(&ctx->mixer_res, MXR_RESOLUTION, val);
> -	}
> -
> -	mixer_cfg_scan(ctx, mode->vdisplay);
> +	mixer_cfg_scan(ctx, mode->hdisplay, mode->vdisplay);
>  	mixer_cfg_rgb_fmt(ctx, mode->vdisplay);
>  	mixer_run(ctx);
>  }
> 

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

Re: [PATCH 04/10] drm/exynos/mixer: fix mode validation code

[Tobias Jakobi]

Hello Andrzej,


Andrzej Hajda wrote:
> Mode limitation checked in mixer driver affects only older HW.
> Mixer in Exynos542x has no such limitations. While at it patch changes
> validation callback to recently introduced mode_valid which is more
> suitable for the check. Additionally little cleanup is performed.

Reviewed-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>

And some small suggestion below.


> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index a87f60b..d530c18 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -1040,26 +1040,24 @@ static void mixer_disable(struct exynos_drm_crtc *crtc)
>  	clear_bit(MXR_BIT_POWERED, &ctx->flags);
>  }
>  
> -/* Only valid for Mixer version 16.0.33.0 */
> -static int mixer_atomic_check(struct exynos_drm_crtc *crtc,
> -		       struct drm_crtc_state *state)
> +static int mixer_mode_valid(struct exynos_drm_crtc *crtc,
> +		const struct drm_display_mode *mode)
>  {
> -	struct drm_display_mode *mode = &state->adjusted_mode;
> -	u32 w, h;
> +	struct mixer_context *ctx = crtc->ctx;
> +	u32 w = mode->hdisplay, h = mode->vdisplay;
>  
> -	w = mode->hdisplay;
> -	h = mode->vdisplay;
> +	DRM_DEBUG_KMS("xres=%d, yres=%d, refresh=%d, intl=%d\n", w, h,
> +		mode->vrefresh, !!(mode->flags & DRM_MODE_FLAG_INTERLACE));
>  
> -	DRM_DEBUG_KMS("xres=%d, yres=%d, refresh=%d, intl=%d\n",
> -		mode->hdisplay, mode->vdisplay, mode->vrefresh,
> -		(mode->flags & DRM_MODE_FLAG_INTERLACE) ? 1 : 0);
> +	if (ctx->mxr_ver == MXR_VER_128_0_0_184)
> +		return MODE_OK;
>  
>  	if ((w >= 464 && w <= 720 && h >= 261 && h <= 576) ||
>  		(w >= 1024 && w <= 1280 && h >= 576 && h <= 720) ||
>  		(w >= 1664 && w <= 1920 && h >= 936 && h <= 1080))
I think it would be nice to have these aligned.


> -		return 0;
> +		return MODE_OK;
>  
> -	return -EINVAL;
> +	return MODE_BAD;
>  }
>  
>  static const struct exynos_drm_crtc_ops mixer_crtc_ops = {
> @@ -1071,7 +1069,7 @@ static const struct exynos_drm_crtc_ops mixer_crtc_ops = {
>  	.update_plane		= mixer_update_plane,
>  	.disable_plane		= mixer_disable_plane,
>  	.atomic_flush		= mixer_atomic_flush,
> -	.atomic_check		= mixer_atomic_check,
> +	.mode_valid		= mixer_mode_valid,
>  };
>  
>  static const struct mixer_drv_data exynos5420_mxr_drv_data = {
> 

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

Re: [PATCH 05/10] drm/exynos/mixer: remove mixer_resources sub-structure

[Tobias Jakobi]

Hello Andrzej,

Andrzej Hajda wrote:
> mixer_resources adds only unnecessary redirection, removing it makes the
> code shorter and cleaner.

Reviewed-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>


> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 323 ++++++++++++++++------------------
>  1 file changed, 147 insertions(+), 176 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index d530c18..f6ea9d9 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -67,19 +67,6 @@
>  #define MXR_FORMAT_ARGB4444	6
>  #define MXR_FORMAT_ARGB8888	7
>  
> -struct mixer_resources {
> -	int			irq;
> -	void __iomem		*mixer_regs;
> -	void __iomem		*vp_regs;
> -	spinlock_t		reg_slock;
> -	struct clk		*mixer;
> -	struct clk		*vp;
> -	struct clk		*hdmi;
> -	struct clk		*sclk_mixer;
> -	struct clk		*sclk_hdmi;
> -	struct clk		*mout_mixer;
> -};
> -
>  enum mixer_version_id {
>  	MXR_VER_0_0_0_16,
>  	MXR_VER_16_0_33_0,
> @@ -117,7 +104,16 @@ struct mixer_context {
>  	struct exynos_drm_plane	planes[MIXER_WIN_NR];
>  	unsigned long		flags;
>  
> -	struct mixer_resources	mixer_res;
> +	int			irq;
> +	void __iomem		*mixer_regs;
> +	void __iomem		*vp_regs;
> +	spinlock_t		reg_slock;
> +	struct clk		*mixer;
> +	struct clk		*vp;
> +	struct clk		*hdmi;
> +	struct clk		*sclk_mixer;
> +	struct clk		*sclk_hdmi;
> +	struct clk		*mout_mixer;
>  	enum mixer_version_id	mxr_ver;
>  };
>  
> @@ -194,44 +190,44 @@ static inline bool is_alpha_format(unsigned int pixel_format)
>  	}
>  }
>  
> -static inline u32 vp_reg_read(struct mixer_resources *res, u32 reg_id)
> +static inline u32 vp_reg_read(struct mixer_context *ctx, u32 reg_id)
>  {
> -	return readl(res->vp_regs + reg_id);
> +	return readl(ctx->vp_regs + reg_id);
>  }
>  
> -static inline void vp_reg_write(struct mixer_resources *res, u32 reg_id,
> +static inline void vp_reg_write(struct mixer_context *ctx, u32 reg_id,
>  				 u32 val)
>  {
> -	writel(val, res->vp_regs + reg_id);
> +	writel(val, ctx->vp_regs + reg_id);
>  }
>  
> -static inline void vp_reg_writemask(struct mixer_resources *res, u32 reg_id,
> +static inline void vp_reg_writemask(struct mixer_context *ctx, u32 reg_id,
>  				 u32 val, u32 mask)
>  {
> -	u32 old = vp_reg_read(res, reg_id);
> +	u32 old = vp_reg_read(ctx, reg_id);
>  
>  	val = (val & mask) | (old & ~mask);
> -	writel(val, res->vp_regs + reg_id);
> +	writel(val, ctx->vp_regs + reg_id);
>  }
>  
> -static inline u32 mixer_reg_read(struct mixer_resources *res, u32 reg_id)
> +static inline u32 mixer_reg_read(struct mixer_context *ctx, u32 reg_id)
>  {
> -	return readl(res->mixer_regs + reg_id);
> +	return readl(ctx->mixer_regs + reg_id);
>  }
>  
> -static inline void mixer_reg_write(struct mixer_resources *res, u32 reg_id,
> +static inline void mixer_reg_write(struct mixer_context *ctx, u32 reg_id,
>  				 u32 val)
>  {
> -	writel(val, res->mixer_regs + reg_id);
> +	writel(val, ctx->mixer_regs + reg_id);
>  }
>  
> -static inline void mixer_reg_writemask(struct mixer_resources *res,
> +static inline void mixer_reg_writemask(struct mixer_context *ctx,
>  				 u32 reg_id, u32 val, u32 mask)
>  {
> -	u32 old = mixer_reg_read(res, reg_id);
> +	u32 old = mixer_reg_read(ctx, reg_id);
>  
>  	val = (val & mask) | (old & ~mask);
> -	writel(val, res->mixer_regs + reg_id);
> +	writel(val, ctx->mixer_regs + reg_id);
>  }
>  
>  static void mixer_regs_dump(struct mixer_context *ctx)
> @@ -239,7 +235,7 @@ static void mixer_regs_dump(struct mixer_context *ctx)
>  #define DUMPREG(reg_id) \
>  do { \
>  	DRM_DEBUG_KMS(#reg_id " = %08x\n", \
> -		(u32)readl(ctx->mixer_res.mixer_regs + reg_id)); \
> +		(u32)readl(ctx->mixer_regs + reg_id)); \
>  } while (0)
>  
>  	DUMPREG(MXR_STATUS);
> @@ -271,7 +267,7 @@ static void vp_regs_dump(struct mixer_context *ctx)
>  #define DUMPREG(reg_id) \
>  do { \
>  	DRM_DEBUG_KMS(#reg_id " = %08x\n", \
> -		(u32) readl(ctx->mixer_res.vp_regs + reg_id)); \
> +		(u32) readl(ctx->vp_regs + reg_id)); \
>  } while (0)
>  
>  	DUMPREG(VP_ENABLE);
> @@ -301,7 +297,7 @@ do { \
>  #undef DUMPREG
>  }
>  
> -static inline void vp_filter_set(struct mixer_resources *res,
> +static inline void vp_filter_set(struct mixer_context *ctx,
>  		int reg_id, const u8 *data, unsigned int size)
>  {
>  	/* assure 4-byte align */
> @@ -309,24 +305,23 @@ static inline void vp_filter_set(struct mixer_resources *res,
>  	for (; size; size -= 4, reg_id += 4, data += 4) {
>  		u32 val = (data[0] << 24) |  (data[1] << 16) |
>  			(data[2] << 8) | data[3];
> -		vp_reg_write(res, reg_id, val);
> +		vp_reg_write(ctx, reg_id, val);
>  	}
>  }
>  
> -static void vp_default_filter(struct mixer_resources *res)
> +static void vp_default_filter(struct mixer_context *ctx)
>  {
> -	vp_filter_set(res, VP_POLY8_Y0_LL,
> +	vp_filter_set(ctx, VP_POLY8_Y0_LL,
>  		filter_y_horiz_tap8, sizeof(filter_y_horiz_tap8));
> -	vp_filter_set(res, VP_POLY4_Y0_LL,
> +	vp_filter_set(ctx, VP_POLY4_Y0_LL,
>  		filter_y_vert_tap4, sizeof(filter_y_vert_tap4));
> -	vp_filter_set(res, VP_POLY4_C0_LL,
> +	vp_filter_set(ctx, VP_POLY4_C0_LL,
>  		filter_cr_horiz_tap4, sizeof(filter_cr_horiz_tap4));
>  }
>  
>  static void mixer_cfg_gfx_blend(struct mixer_context *ctx, unsigned int win,
>  				bool alpha)
>  {
> -	struct mixer_resources *res = &ctx->mixer_res;
>  	u32 val;
>  
>  	val  = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
> @@ -335,13 +330,12 @@ static void mixer_cfg_gfx_blend(struct mixer_context *ctx, unsigned int win,
>  		val |= MXR_GRP_CFG_BLEND_PRE_MUL;
>  		val |= MXR_GRP_CFG_PIXEL_BLEND_EN;
>  	}
> -	mixer_reg_writemask(res, MXR_GRAPHIC_CFG(win),
> +	mixer_reg_writemask(ctx, MXR_GRAPHIC_CFG(win),
>  			    val, MXR_GRP_CFG_MISC_MASK);
>  }
>  
>  static void mixer_cfg_vp_blend(struct mixer_context *ctx)
>  {
> -	struct mixer_resources *res = &ctx->mixer_res;
>  	u32 val;
>  
>  	/*
> @@ -351,25 +345,22 @@ static void mixer_cfg_vp_blend(struct mixer_context *ctx)
>  	 * support blending of the video layer through this.
>  	 */
>  	val = 0;
> -	mixer_reg_write(res, MXR_VIDEO_CFG, val);
> +	mixer_reg_write(ctx, MXR_VIDEO_CFG, val);
>  }
>  
>  static void mixer_vsync_set_update(struct mixer_context *ctx, bool enable)
>  {
> -	struct mixer_resources *res = &ctx->mixer_res;
> -
>  	/* block update on vsync */
> -	mixer_reg_writemask(res, MXR_STATUS, enable ?
> +	mixer_reg_writemask(ctx, MXR_STATUS, enable ?
>  			MXR_STATUS_SYNC_ENABLE : 0, MXR_STATUS_SYNC_ENABLE);
>  
>  	if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags))
> -		vp_reg_write(res, VP_SHADOW_UPDATE, enable ?
> +		vp_reg_write(ctx, VP_SHADOW_UPDATE, enable ?
>  			VP_SHADOW_UPDATE_ENABLE : 0);
>  }
>  
>  static void mixer_cfg_scan(struct mixer_context *ctx, int width, int height)
>  {
> -	struct mixer_resources *res = &ctx->mixer_res;
>  	u32 val;
>  
>  	/* choosing between interlace and progressive mode */
> @@ -378,7 +369,7 @@ static void mixer_cfg_scan(struct mixer_context *ctx, int width, int height)
>  
>  	/* setup display size */
>  	if (ctx->mxr_ver == MXR_VER_128_0_0_184) {
> -		mixer_reg_write(&ctx->mixer_res, MXR_RESOLUTION,
> +		mixer_reg_write(ctx, MXR_RESOLUTION,
>  			MXR_MXR_RES_HEIGHT(height) | MXR_MXR_RES_WIDTH(width));
>  	} else {
>  		/* choosing between proper HD and SD mode */
> @@ -394,12 +385,11 @@ static void mixer_cfg_scan(struct mixer_context *ctx, int width, int height)
>  			val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD;
>  	}
>  
> -	mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_SCAN_MASK);
> +	mixer_reg_writemask(ctx, MXR_CFG, val, MXR_CFG_SCAN_MASK);
>  }
>  
>  static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height)
>  {
> -	struct mixer_resources *res = &ctx->mixer_res;
>  	u32 val;
>  
>  	switch (height) {
> @@ -412,45 +402,44 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height)
>  	default:
>  		val = MXR_CFG_RGB709_16_235;
>  		/* Configure the BT.709 CSC matrix for full range RGB. */
> -		mixer_reg_write(res, MXR_CM_COEFF_Y,
> +		mixer_reg_write(ctx, MXR_CM_COEFF_Y,
>  			MXR_CSC_CT( 0.184,  0.614,  0.063) |
>  			MXR_CM_COEFF_RGB_FULL);
> -		mixer_reg_write(res, MXR_CM_COEFF_CB,
> +		mixer_reg_write(ctx, MXR_CM_COEFF_CB,
>  			MXR_CSC_CT(-0.102, -0.338,  0.440));
> -		mixer_reg_write(res, MXR_CM_COEFF_CR,
> +		mixer_reg_write(ctx, MXR_CM_COEFF_CR,
>  			MXR_CSC_CT( 0.440, -0.399, -0.040));
>  		break;
>  	}
>  
> -	mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_RGB_FMT_MASK);
> +	mixer_reg_writemask(ctx, MXR_CFG, val, MXR_CFG_RGB_FMT_MASK);
>  }
>  
>  static void mixer_cfg_layer(struct mixer_context *ctx, unsigned int win,
>  			    unsigned int priority, bool enable)
>  {
> -	struct mixer_resources *res = &ctx->mixer_res;
>  	u32 val = enable ? ~0 : 0;
>  
>  	switch (win) {
>  	case 0:
> -		mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_GRP0_ENABLE);
> -		mixer_reg_writemask(res, MXR_LAYER_CFG,
> +		mixer_reg_writemask(ctx, MXR_CFG, val, MXR_CFG_GRP0_ENABLE);
> +		mixer_reg_writemask(ctx, MXR_LAYER_CFG,
>  				    MXR_LAYER_CFG_GRP0_VAL(priority),
>  				    MXR_LAYER_CFG_GRP0_MASK);
>  		break;
>  	case 1:
> -		mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_GRP1_ENABLE);
> -		mixer_reg_writemask(res, MXR_LAYER_CFG,
> +		mixer_reg_writemask(ctx, MXR_CFG, val, MXR_CFG_GRP1_ENABLE);
> +		mixer_reg_writemask(ctx, MXR_LAYER_CFG,
>  				    MXR_LAYER_CFG_GRP1_VAL(priority),
>  				    MXR_LAYER_CFG_GRP1_MASK);
>  
>  		break;
>  	case VP_DEFAULT_WIN:
>  		if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) {
> -			vp_reg_writemask(res, VP_ENABLE, val, VP_ENABLE_ON);
> -			mixer_reg_writemask(res, MXR_CFG, val,
> +			vp_reg_writemask(ctx, VP_ENABLE, val, VP_ENABLE_ON);
> +			mixer_reg_writemask(ctx, MXR_CFG, val,
>  				MXR_CFG_VP_ENABLE);
> -			mixer_reg_writemask(res, MXR_LAYER_CFG,
> +			mixer_reg_writemask(ctx, MXR_LAYER_CFG,
>  					    MXR_LAYER_CFG_VP_VAL(priority),
>  					    MXR_LAYER_CFG_VP_MASK);
>  		}
> @@ -460,19 +449,16 @@ static void mixer_cfg_layer(struct mixer_context *ctx, unsigned int win,
>  
>  static void mixer_run(struct mixer_context *ctx)
>  {
> -	struct mixer_resources *res = &ctx->mixer_res;
> -
> -	mixer_reg_writemask(res, MXR_STATUS, ~0, MXR_STATUS_REG_RUN);
> +	mixer_reg_writemask(ctx, MXR_STATUS, ~0, MXR_STATUS_REG_RUN);
>  }
>  
>  static void mixer_stop(struct mixer_context *ctx)
>  {
> -	struct mixer_resources *res = &ctx->mixer_res;
>  	int timeout = 20;
>  
> -	mixer_reg_writemask(res, MXR_STATUS, 0, MXR_STATUS_REG_RUN);
> +	mixer_reg_writemask(ctx, MXR_STATUS, 0, MXR_STATUS_REG_RUN);
>  
> -	while (!(mixer_reg_read(res, MXR_STATUS) & MXR_STATUS_REG_IDLE) &&
> +	while (!(mixer_reg_read(ctx, MXR_STATUS) & MXR_STATUS_REG_IDLE) &&
>  			--timeout)
>  		usleep_range(10000, 12000);
>  }
> @@ -496,7 +482,6 @@ static void vp_video_buffer(struct mixer_context *ctx,
>  {
>  	struct exynos_drm_plane_state *state =
>  				to_exynos_plane_state(plane->base.state);
> -	struct mixer_resources *res = &ctx->mixer_res;
>  	struct drm_framebuffer *fb = state->base.fb;
>  	unsigned int priority = state->base.normalized_zpos + 1;
>  	unsigned long flags;
> @@ -523,55 +508,55 @@ static void vp_video_buffer(struct mixer_context *ctx,
>  		chroma_addr[1] = 0;
>  	}
>  
> -	spin_lock_irqsave(&res->reg_slock, flags);
> +	spin_lock_irqsave(&ctx->reg_slock, flags);
>  
>  	/* interlace or progressive scan mode */
>  	val = (test_bit(MXR_BIT_INTERLACE, &ctx->flags) ? ~0 : 0);
> -	vp_reg_writemask(res, VP_MODE, val, VP_MODE_LINE_SKIP);
> +	vp_reg_writemask(ctx, VP_MODE, val, VP_MODE_LINE_SKIP);
>  
>  	/* setup format */
>  	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);
> +	vp_reg_writemask(ctx, VP_MODE, val, VP_MODE_FMT_MASK);
>  
>  	/* setting size of input image */
> -	vp_reg_write(res, VP_IMG_SIZE_Y, VP_IMG_HSIZE(fb->pitches[0]) |
> +	vp_reg_write(ctx, VP_IMG_SIZE_Y, VP_IMG_HSIZE(fb->pitches[0]) |
>  		VP_IMG_VSIZE(fb->height));
>  	/* 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_reg_write(ctx, VP_IMG_SIZE_C, VP_IMG_HSIZE(fb->pitches[0]) |
>  		VP_IMG_VSIZE(fb->height / 2));
>  
> -	vp_reg_write(res, VP_SRC_WIDTH, state->src.w);
> -	vp_reg_write(res, VP_SRC_HEIGHT, state->src.h);
> -	vp_reg_write(res, VP_SRC_H_POSITION,
> +	vp_reg_write(ctx, VP_SRC_WIDTH, state->src.w);
> +	vp_reg_write(ctx, VP_SRC_HEIGHT, state->src.h);
> +	vp_reg_write(ctx, VP_SRC_H_POSITION,
>  			VP_SRC_H_POSITION_VAL(state->src.x));
> -	vp_reg_write(res, VP_SRC_V_POSITION, state->src.y);
> +	vp_reg_write(ctx, VP_SRC_V_POSITION, state->src.y);
>  
> -	vp_reg_write(res, VP_DST_WIDTH, state->crtc.w);
> -	vp_reg_write(res, VP_DST_H_POSITION, state->crtc.x);
> +	vp_reg_write(ctx, VP_DST_WIDTH, state->crtc.w);
> +	vp_reg_write(ctx, VP_DST_H_POSITION, state->crtc.x);
>  	if (test_bit(MXR_BIT_INTERLACE, &ctx->flags)) {
> -		vp_reg_write(res, VP_DST_HEIGHT, state->crtc.h / 2);
> -		vp_reg_write(res, VP_DST_V_POSITION, state->crtc.y / 2);
> +		vp_reg_write(ctx, VP_DST_HEIGHT, state->crtc.h / 2);
> +		vp_reg_write(ctx, VP_DST_V_POSITION, state->crtc.y / 2);
>  	} else {
> -		vp_reg_write(res, VP_DST_HEIGHT, state->crtc.h);
> -		vp_reg_write(res, VP_DST_V_POSITION, state->crtc.y);
> +		vp_reg_write(ctx, VP_DST_HEIGHT, state->crtc.h);
> +		vp_reg_write(ctx, VP_DST_V_POSITION, state->crtc.y);
>  	}
>  
> -	vp_reg_write(res, VP_H_RATIO, state->h_ratio);
> -	vp_reg_write(res, VP_V_RATIO, state->v_ratio);
> +	vp_reg_write(ctx, VP_H_RATIO, state->h_ratio);
> +	vp_reg_write(ctx, VP_V_RATIO, state->v_ratio);
>  
> -	vp_reg_write(res, VP_ENDIAN_MODE, VP_ENDIAN_MODE_LITTLE);
> +	vp_reg_write(ctx, VP_ENDIAN_MODE, VP_ENDIAN_MODE_LITTLE);
>  
>  	/* set buffer address to vp */
> -	vp_reg_write(res, VP_TOP_Y_PTR, luma_addr[0]);
> -	vp_reg_write(res, VP_BOT_Y_PTR, luma_addr[1]);
> -	vp_reg_write(res, VP_TOP_C_PTR, chroma_addr[0]);
> -	vp_reg_write(res, VP_BOT_C_PTR, chroma_addr[1]);
> +	vp_reg_write(ctx, VP_TOP_Y_PTR, luma_addr[0]);
> +	vp_reg_write(ctx, VP_BOT_Y_PTR, luma_addr[1]);
> +	vp_reg_write(ctx, VP_TOP_C_PTR, chroma_addr[0]);
> +	vp_reg_write(ctx, VP_BOT_C_PTR, chroma_addr[1]);
>  
>  	mixer_cfg_layer(ctx, plane->index, priority, true);
>  	mixer_cfg_vp_blend(ctx);
>  
> -	spin_unlock_irqrestore(&res->reg_slock, flags);
> +	spin_unlock_irqrestore(&ctx->reg_slock, flags);
>  
>  	mixer_regs_dump(ctx);
>  	vp_regs_dump(ctx);
> @@ -579,9 +564,7 @@ static void vp_video_buffer(struct mixer_context *ctx,
>  
>  static void mixer_layer_update(struct mixer_context *ctx)
>  {
> -	struct mixer_resources *res = &ctx->mixer_res;
> -
> -	mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
> +	mixer_reg_writemask(ctx, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
>  }
>  
>  static void mixer_graph_buffer(struct mixer_context *ctx,
> @@ -589,7 +572,6 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>  {
>  	struct exynos_drm_plane_state *state =
>  				to_exynos_plane_state(plane->base.state);
> -	struct mixer_resources *res = &ctx->mixer_res;
>  	struct drm_framebuffer *fb = state->base.fb;
>  	unsigned int priority = state->base.normalized_zpos + 1;
>  	unsigned long flags;
> @@ -634,29 +616,29 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>  		+ (state->src.x * fb->format->cpp[0])
>  		+ (state->src.y * fb->pitches[0]);
>  
> -	spin_lock_irqsave(&res->reg_slock, flags);
> +	spin_lock_irqsave(&ctx->reg_slock, flags);
>  
>  	/* setup format */
> -	mixer_reg_writemask(res, MXR_GRAPHIC_CFG(win),
> +	mixer_reg_writemask(ctx, MXR_GRAPHIC_CFG(win),
>  		MXR_GRP_CFG_FORMAT_VAL(fmt), MXR_GRP_CFG_FORMAT_MASK);
>  
>  	/* setup geometry */
> -	mixer_reg_write(res, MXR_GRAPHIC_SPAN(win),
> +	mixer_reg_write(ctx, MXR_GRAPHIC_SPAN(win),
>  			fb->pitches[0] / fb->format->cpp[0]);
>  
>  	val  = MXR_GRP_WH_WIDTH(state->src.w);
>  	val |= MXR_GRP_WH_HEIGHT(state->src.h);
>  	val |= MXR_GRP_WH_H_SCALE(x_ratio);
>  	val |= MXR_GRP_WH_V_SCALE(y_ratio);
> -	mixer_reg_write(res, MXR_GRAPHIC_WH(win), val);
> +	mixer_reg_write(ctx, MXR_GRAPHIC_WH(win), val);
>  
>  	/* setup offsets in display image */
>  	val  = MXR_GRP_DXY_DX(dst_x_offset);
>  	val |= MXR_GRP_DXY_DY(dst_y_offset);
> -	mixer_reg_write(res, MXR_GRAPHIC_DXY(win), val);
> +	mixer_reg_write(ctx, MXR_GRAPHIC_DXY(win), val);
>  
>  	/* set buffer address to mixer */
> -	mixer_reg_write(res, MXR_GRAPHIC_BASE(win), dma_addr);
> +	mixer_reg_write(ctx, MXR_GRAPHIC_BASE(win), dma_addr);
>  
>  	mixer_cfg_layer(ctx, win, priority, true);
>  	mixer_cfg_gfx_blend(ctx, win, is_alpha_format(fb->format->format));
> @@ -666,20 +648,19 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>  		ctx->mxr_ver == MXR_VER_128_0_0_184)
>  		mixer_layer_update(ctx);
>  
> -	spin_unlock_irqrestore(&res->reg_slock, flags);
> +	spin_unlock_irqrestore(&ctx->reg_slock, flags);
>  
>  	mixer_regs_dump(ctx);
>  }
>  
>  static void vp_win_reset(struct mixer_context *ctx)
>  {
> -	struct mixer_resources *res = &ctx->mixer_res;
>  	unsigned int tries = 100;
>  
> -	vp_reg_write(res, VP_SRESET, VP_SRESET_PROCESSING);
> +	vp_reg_write(ctx, VP_SRESET, VP_SRESET_PROCESSING);
>  	while (--tries) {
>  		/* waiting until VP_SRESET_PROCESSING is 0 */
> -		if (~vp_reg_read(res, VP_SRESET) & VP_SRESET_PROCESSING)
> +		if (~vp_reg_read(ctx, VP_SRESET) & VP_SRESET_PROCESSING)
>  			break;
>  		mdelay(10);
>  	}
> @@ -688,57 +669,55 @@ static void vp_win_reset(struct mixer_context *ctx)
>  
>  static void mixer_win_reset(struct mixer_context *ctx)
>  {
> -	struct mixer_resources *res = &ctx->mixer_res;
>  	unsigned long flags;
>  
> -	spin_lock_irqsave(&res->reg_slock, flags);
> +	spin_lock_irqsave(&ctx->reg_slock, flags);
>  
> -	mixer_reg_writemask(res, MXR_CFG, MXR_CFG_DST_HDMI, MXR_CFG_DST_MASK);
> +	mixer_reg_writemask(ctx, MXR_CFG, MXR_CFG_DST_HDMI, MXR_CFG_DST_MASK);
>  
>  	/* set output in RGB888 mode */
> -	mixer_reg_writemask(res, MXR_CFG, MXR_CFG_OUT_RGB888, MXR_CFG_OUT_MASK);
> +	mixer_reg_writemask(ctx, MXR_CFG, MXR_CFG_OUT_RGB888, MXR_CFG_OUT_MASK);
>  
>  	/* 16 beat burst in DMA */
> -	mixer_reg_writemask(res, MXR_STATUS, MXR_STATUS_16_BURST,
> +	mixer_reg_writemask(ctx, MXR_STATUS, MXR_STATUS_16_BURST,
>  		MXR_STATUS_BURST_MASK);
>  
>  	/* reset default layer priority */
> -	mixer_reg_write(res, MXR_LAYER_CFG, 0);
> +	mixer_reg_write(ctx, MXR_LAYER_CFG, 0);
>  
>  	/* set all background colors to RGB (0,0,0) */
> -	mixer_reg_write(res, MXR_BG_COLOR0, MXR_YCBCR_VAL(0, 128, 128));
> -	mixer_reg_write(res, MXR_BG_COLOR1, MXR_YCBCR_VAL(0, 128, 128));
> -	mixer_reg_write(res, MXR_BG_COLOR2, MXR_YCBCR_VAL(0, 128, 128));
> +	mixer_reg_write(ctx, MXR_BG_COLOR0, MXR_YCBCR_VAL(0, 128, 128));
> +	mixer_reg_write(ctx, MXR_BG_COLOR1, MXR_YCBCR_VAL(0, 128, 128));
> +	mixer_reg_write(ctx, MXR_BG_COLOR2, MXR_YCBCR_VAL(0, 128, 128));
>  
>  	if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) {
>  		/* configuration of Video Processor Registers */
>  		vp_win_reset(ctx);
> -		vp_default_filter(res);
> +		vp_default_filter(ctx);
>  	}
>  
>  	/* disable all layers */
> -	mixer_reg_writemask(res, MXR_CFG, 0, MXR_CFG_GRP0_ENABLE);
> -	mixer_reg_writemask(res, MXR_CFG, 0, MXR_CFG_GRP1_ENABLE);
> +	mixer_reg_writemask(ctx, MXR_CFG, 0, MXR_CFG_GRP0_ENABLE);
> +	mixer_reg_writemask(ctx, MXR_CFG, 0, MXR_CFG_GRP1_ENABLE);
>  	if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags))
> -		mixer_reg_writemask(res, MXR_CFG, 0, MXR_CFG_VP_ENABLE);
> +		mixer_reg_writemask(ctx, MXR_CFG, 0, MXR_CFG_VP_ENABLE);
>  
>  	/* set all source image offsets to zero */
> -	mixer_reg_write(res, MXR_GRAPHIC_SXY(0), 0);
> -	mixer_reg_write(res, MXR_GRAPHIC_SXY(1), 0);
> +	mixer_reg_write(ctx, MXR_GRAPHIC_SXY(0), 0);
> +	mixer_reg_write(ctx, MXR_GRAPHIC_SXY(1), 0);
>  
> -	spin_unlock_irqrestore(&res->reg_slock, flags);
> +	spin_unlock_irqrestore(&ctx->reg_slock, flags);
>  }
>  
>  static irqreturn_t mixer_irq_handler(int irq, void *arg)
>  {
>  	struct mixer_context *ctx = arg;
> -	struct mixer_resources *res = &ctx->mixer_res;
>  	u32 val, base, shadow;
>  
> -	spin_lock(&res->reg_slock);
> +	spin_lock(&ctx->reg_slock);
>  
>  	/* read interrupt status for handling and clearing flags for VSYNC */
> -	val = mixer_reg_read(res, MXR_INT_STATUS);
> +	val = mixer_reg_read(ctx, MXR_INT_STATUS);
>  
>  	/* handling VSYNC */
>  	if (val & MXR_INT_STATUS_VSYNC) {
> @@ -748,13 +727,13 @@ static irqreturn_t mixer_irq_handler(int irq, void *arg)
>  
>  		/* interlace scan need to check shadow register */
>  		if (test_bit(MXR_BIT_INTERLACE, &ctx->flags)) {
> -			base = mixer_reg_read(res, MXR_GRAPHIC_BASE(0));
> -			shadow = mixer_reg_read(res, MXR_GRAPHIC_BASE_S(0));
> +			base = mixer_reg_read(ctx, MXR_GRAPHIC_BASE(0));
> +			shadow = mixer_reg_read(ctx, MXR_GRAPHIC_BASE_S(0));
>  			if (base != shadow)
>  				goto out;
>  
> -			base = mixer_reg_read(res, MXR_GRAPHIC_BASE(1));
> -			shadow = mixer_reg_read(res, MXR_GRAPHIC_BASE_S(1));
> +			base = mixer_reg_read(ctx, MXR_GRAPHIC_BASE(1));
> +			shadow = mixer_reg_read(ctx, MXR_GRAPHIC_BASE_S(1));
>  			if (base != shadow)
>  				goto out;
>  		}
> @@ -764,9 +743,9 @@ static irqreturn_t mixer_irq_handler(int irq, void *arg)
>  
>  out:
>  	/* clear interrupts */
> -	mixer_reg_write(res, MXR_INT_STATUS, val);
> +	mixer_reg_write(ctx, MXR_INT_STATUS, val);
>  
> -	spin_unlock(&res->reg_slock);
> +	spin_unlock(&ctx->reg_slock);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -774,26 +753,25 @@ static irqreturn_t mixer_irq_handler(int irq, void *arg)
>  static int mixer_resources_init(struct mixer_context *mixer_ctx)
>  {
>  	struct device *dev = &mixer_ctx->pdev->dev;
> -	struct mixer_resources *mixer_res = &mixer_ctx->mixer_res;
>  	struct resource *res;
>  	int ret;
>  
> -	spin_lock_init(&mixer_res->reg_slock);
> +	spin_lock_init(&mixer_ctx->reg_slock);
>  
> -	mixer_res->mixer = devm_clk_get(dev, "mixer");
> -	if (IS_ERR(mixer_res->mixer)) {
> +	mixer_ctx->mixer = devm_clk_get(dev, "mixer");
> +	if (IS_ERR(mixer_ctx->mixer)) {
>  		dev_err(dev, "failed to get clock 'mixer'\n");
>  		return -ENODEV;
>  	}
>  
> -	mixer_res->hdmi = devm_clk_get(dev, "hdmi");
> -	if (IS_ERR(mixer_res->hdmi)) {
> +	mixer_ctx->hdmi = devm_clk_get(dev, "hdmi");
> +	if (IS_ERR(mixer_ctx->hdmi)) {
>  		dev_err(dev, "failed to get clock 'hdmi'\n");
> -		return PTR_ERR(mixer_res->hdmi);
> +		return PTR_ERR(mixer_ctx->hdmi);
>  	}
>  
> -	mixer_res->sclk_hdmi = devm_clk_get(dev, "sclk_hdmi");
> -	if (IS_ERR(mixer_res->sclk_hdmi)) {
> +	mixer_ctx->sclk_hdmi = devm_clk_get(dev, "sclk_hdmi");
> +	if (IS_ERR(mixer_ctx->sclk_hdmi)) {
>  		dev_err(dev, "failed to get clock 'sclk_hdmi'\n");
>  		return -ENODEV;
>  	}
> @@ -803,9 +781,9 @@ static int mixer_resources_init(struct mixer_context *mixer_ctx)
>  		return -ENXIO;
>  	}
>  
> -	mixer_res->mixer_regs = devm_ioremap(dev, res->start,
> +	mixer_ctx->mixer_regs = devm_ioremap(dev, res->start,
>  							resource_size(res));
> -	if (mixer_res->mixer_regs == NULL) {
> +	if (mixer_ctx->mixer_regs == NULL) {
>  		dev_err(dev, "register mapping failed.\n");
>  		return -ENXIO;
>  	}
> @@ -822,7 +800,7 @@ static int mixer_resources_init(struct mixer_context *mixer_ctx)
>  		dev_err(dev, "request interrupt failed.\n");
>  		return ret;
>  	}
> -	mixer_res->irq = res->start;
> +	mixer_ctx->irq = res->start;
>  
>  	return 0;
>  }
> @@ -830,30 +808,29 @@ static int mixer_resources_init(struct mixer_context *mixer_ctx)
>  static int vp_resources_init(struct mixer_context *mixer_ctx)
>  {
>  	struct device *dev = &mixer_ctx->pdev->dev;
> -	struct mixer_resources *mixer_res = &mixer_ctx->mixer_res;
>  	struct resource *res;
>  
> -	mixer_res->vp = devm_clk_get(dev, "vp");
> -	if (IS_ERR(mixer_res->vp)) {
> +	mixer_ctx->vp = devm_clk_get(dev, "vp");
> +	if (IS_ERR(mixer_ctx->vp)) {
>  		dev_err(dev, "failed to get clock 'vp'\n");
>  		return -ENODEV;
>  	}
>  
>  	if (test_bit(MXR_BIT_HAS_SCLK, &mixer_ctx->flags)) {
> -		mixer_res->sclk_mixer = devm_clk_get(dev, "sclk_mixer");
> -		if (IS_ERR(mixer_res->sclk_mixer)) {
> +		mixer_ctx->sclk_mixer = devm_clk_get(dev, "sclk_mixer");
> +		if (IS_ERR(mixer_ctx->sclk_mixer)) {
>  			dev_err(dev, "failed to get clock 'sclk_mixer'\n");
>  			return -ENODEV;
>  		}
> -		mixer_res->mout_mixer = devm_clk_get(dev, "mout_mixer");
> -		if (IS_ERR(mixer_res->mout_mixer)) {
> +		mixer_ctx->mout_mixer = devm_clk_get(dev, "mout_mixer");
> +		if (IS_ERR(mixer_ctx->mout_mixer)) {
>  			dev_err(dev, "failed to get clock 'mout_mixer'\n");
>  			return -ENODEV;
>  		}
>  
> -		if (mixer_res->sclk_hdmi && mixer_res->mout_mixer)
> -			clk_set_parent(mixer_res->mout_mixer,
> -				       mixer_res->sclk_hdmi);
> +		if (mixer_ctx->sclk_hdmi && mixer_ctx->mout_mixer)
> +			clk_set_parent(mixer_ctx->mout_mixer,
> +				       mixer_ctx->sclk_hdmi);
>  	}
>  
>  	res = platform_get_resource(mixer_ctx->pdev, IORESOURCE_MEM, 1);
> @@ -862,9 +839,9 @@ static int vp_resources_init(struct mixer_context *mixer_ctx)
>  		return -ENXIO;
>  	}
>  
> -	mixer_res->vp_regs = devm_ioremap(dev, res->start,
> +	mixer_ctx->vp_regs = devm_ioremap(dev, res->start,
>  							resource_size(res));
> -	if (mixer_res->vp_regs == NULL) {
> +	if (mixer_ctx->vp_regs == NULL) {
>  		dev_err(dev, "register mapping failed.\n");
>  		return -ENXIO;
>  	}
> @@ -908,15 +885,14 @@ static void mixer_ctx_remove(struct mixer_context *mixer_ctx)
>  static int mixer_enable_vblank(struct exynos_drm_crtc *crtc)
>  {
>  	struct mixer_context *mixer_ctx = crtc->ctx;
> -	struct mixer_resources *res = &mixer_ctx->mixer_res;
>  
>  	__set_bit(MXR_BIT_VSYNC, &mixer_ctx->flags);
>  	if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags))
>  		return 0;
>  
>  	/* enable vsync interrupt */
> -	mixer_reg_writemask(res, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC);
> -	mixer_reg_writemask(res, MXR_INT_EN, ~0, MXR_INT_EN_VSYNC);
> +	mixer_reg_writemask(mixer_ctx, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC);
> +	mixer_reg_writemask(mixer_ctx, MXR_INT_EN, ~0, MXR_INT_EN_VSYNC);
>  
>  	return 0;
>  }
> @@ -924,7 +900,6 @@ static int mixer_enable_vblank(struct exynos_drm_crtc *crtc)
>  static void mixer_disable_vblank(struct exynos_drm_crtc *crtc)
>  {
>  	struct mixer_context *mixer_ctx = crtc->ctx;
> -	struct mixer_resources *res = &mixer_ctx->mixer_res;
>  
>  	__clear_bit(MXR_BIT_VSYNC, &mixer_ctx->flags);
>  
> @@ -932,8 +907,8 @@ static void mixer_disable_vblank(struct exynos_drm_crtc *crtc)
>  		return;
>  
>  	/* disable vsync interrupt */
> -	mixer_reg_writemask(res, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC);
> -	mixer_reg_writemask(res, MXR_INT_EN, 0, MXR_INT_EN_VSYNC);
> +	mixer_reg_writemask(mixer_ctx, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC);
> +	mixer_reg_writemask(mixer_ctx, MXR_INT_EN, 0, MXR_INT_EN_VSYNC);
>  }
>  
>  static void mixer_atomic_begin(struct exynos_drm_crtc *crtc)
> @@ -966,7 +941,6 @@ static void mixer_disable_plane(struct exynos_drm_crtc *crtc,
>  				struct exynos_drm_plane *plane)
>  {
>  	struct mixer_context *mixer_ctx = crtc->ctx;
> -	struct mixer_resources *res = &mixer_ctx->mixer_res;
>  	unsigned long flags;
>  
>  	DRM_DEBUG_KMS("win: %d\n", plane->index);
> @@ -974,9 +948,9 @@ static void mixer_disable_plane(struct exynos_drm_crtc *crtc,
>  	if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags))
>  		return;
>  
> -	spin_lock_irqsave(&res->reg_slock, flags);
> +	spin_lock_irqsave(&mixer_ctx->reg_slock, flags);
>  	mixer_cfg_layer(mixer_ctx, plane->index, 0, false);
> -	spin_unlock_irqrestore(&res->reg_slock, flags);
> +	spin_unlock_irqrestore(&mixer_ctx->reg_slock, flags);
>  }
>  
>  static void mixer_atomic_flush(struct exynos_drm_crtc *crtc)
> @@ -993,7 +967,6 @@ static void mixer_atomic_flush(struct exynos_drm_crtc *crtc)
>  static void mixer_enable(struct exynos_drm_crtc *crtc)
>  {
>  	struct mixer_context *ctx = crtc->ctx;
> -	struct mixer_resources *res = &ctx->mixer_res;
>  
>  	if (test_bit(MXR_BIT_POWERED, &ctx->flags))
>  		return;
> @@ -1004,11 +977,11 @@ static void mixer_enable(struct exynos_drm_crtc *crtc)
>  
>  	mixer_vsync_set_update(ctx, false);
>  
> -	mixer_reg_writemask(res, MXR_STATUS, ~0, MXR_STATUS_SOFT_RESET);
> +	mixer_reg_writemask(ctx, MXR_STATUS, ~0, MXR_STATUS_SOFT_RESET);
>  
>  	if (test_bit(MXR_BIT_VSYNC, &ctx->flags)) {
> -		mixer_reg_writemask(res, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC);
> -		mixer_reg_writemask(res, MXR_INT_EN, ~0, MXR_INT_EN_VSYNC);
> +		mixer_reg_writemask(ctx, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC);
> +		mixer_reg_writemask(ctx, MXR_INT_EN, ~0, MXR_INT_EN_VSYNC);
>  	}
>  	mixer_win_reset(ctx);
>  
> @@ -1211,14 +1184,13 @@ static int mixer_remove(struct platform_device *pdev)
>  static int __maybe_unused exynos_mixer_suspend(struct device *dev)
>  {
>  	struct mixer_context *ctx = dev_get_drvdata(dev);
> -	struct mixer_resources *res = &ctx->mixer_res;
>  
> -	clk_disable_unprepare(res->hdmi);
> -	clk_disable_unprepare(res->mixer);
> +	clk_disable_unprepare(ctx->hdmi);
> +	clk_disable_unprepare(ctx->mixer);
>  	if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) {
> -		clk_disable_unprepare(res->vp);
> +		clk_disable_unprepare(ctx->vp);
>  		if (test_bit(MXR_BIT_HAS_SCLK, &ctx->flags))
> -			clk_disable_unprepare(res->sclk_mixer);
> +			clk_disable_unprepare(ctx->sclk_mixer);
>  	}
>  
>  	return 0;
> @@ -1227,28 +1199,27 @@ static int __maybe_unused exynos_mixer_suspend(struct device *dev)
>  static int __maybe_unused exynos_mixer_resume(struct device *dev)
>  {
>  	struct mixer_context *ctx = dev_get_drvdata(dev);
> -	struct mixer_resources *res = &ctx->mixer_res;
>  	int ret;
>  
> -	ret = clk_prepare_enable(res->mixer);
> +	ret = clk_prepare_enable(ctx->mixer);
>  	if (ret < 0) {
>  		DRM_ERROR("Failed to prepare_enable the mixer clk [%d]\n", ret);
>  		return ret;
>  	}
> -	ret = clk_prepare_enable(res->hdmi);
> +	ret = clk_prepare_enable(ctx->hdmi);
>  	if (ret < 0) {
>  		DRM_ERROR("Failed to prepare_enable the hdmi clk [%d]\n", ret);
>  		return ret;
>  	}
>  	if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) {
> -		ret = clk_prepare_enable(res->vp);
> +		ret = clk_prepare_enable(ctx->vp);
>  		if (ret < 0) {
>  			DRM_ERROR("Failed to prepare_enable the vp clk [%d]\n",
>  				  ret);
>  			return ret;
>  		}
>  		if (test_bit(MXR_BIT_HAS_SCLK, &ctx->flags)) {
> -			ret = clk_prepare_enable(res->sclk_mixer);
> +			ret = clk_prepare_enable(ctx->sclk_mixer);
>  			if (ret < 0) {
>  				DRM_ERROR("Failed to prepare_enable the " \
>  					   "sclk_mixer clk [%d]\n",
> 

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

Re: [PATCH 06/10] drm/exynos/hdmi: remove redundant mode field

[Tobias Jakobi]

Hello Andrzej,


Andrzej Hajda wrote:
> Display mode is preserved in CRTC state, there is no need to keep local
> copy of it. Moreover since HDMI should configure registers according to
> requested mode, use it instead of adjusted_mode, which should contain
> mode produced by CRTC - functionally it does not change anything, but
> subsequent patches will make the difference.

Reviewed-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>


> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_hdmi.c | 34 +++++++++-------------------------
>  1 file changed, 9 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 214fa5e..7225b65 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -119,7 +119,6 @@ struct hdmi_context {
>  	bool				powered;
>  	bool				dvi_mode;
>  	struct delayed_work		hotplug_work;
> -	struct drm_display_mode		current_mode;
>  	struct cec_notifier		*notifier;
>  	const struct hdmi_driver_data	*drv_data;
>  
> @@ -770,6 +769,7 @@ static int hdmi_clk_set_parents(struct hdmi_context *hdata, bool to_phy)
>  
>  static void hdmi_reg_infoframes(struct hdmi_context *hdata)
>  {
> +	struct drm_display_mode *m = &hdata->encoder.crtc->state->mode;
>  	union hdmi_infoframe frm;
>  	u8 buf[25];
>  	int ret;
> @@ -783,8 +783,7 @@ static void hdmi_reg_infoframes(struct hdmi_context *hdata)
>  		return;
>  	}
>  
> -	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
> -			&hdata->current_mode, false);
> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi, m, false);
>  	if (!ret)
>  		ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
>  	if (ret > 0) {
> @@ -794,8 +793,7 @@ static void hdmi_reg_infoframes(struct hdmi_context *hdata)
>  		DRM_INFO("%s: invalid AVI infoframe (%d)\n", __func__, ret);
>  	}
>  
> -	ret = drm_hdmi_vendor_infoframe_from_display_mode(&frm.vendor.hdmi,
> -			&hdata->current_mode);
> +	ret = drm_hdmi_vendor_infoframe_from_display_mode(&frm.vendor.hdmi, m);
>  	if (!ret)
>  		ret = hdmi_vendor_infoframe_pack(&frm.vendor.hdmi, buf,
>  				sizeof(buf));
> @@ -1088,9 +1086,10 @@ static void hdmi_audio_control(struct hdmi_context *hdata, bool onoff)
>  
>  static void hdmi_start(struct hdmi_context *hdata, bool start)
>  {
> +	struct drm_display_mode *m = &hdata->encoder.crtc->state->mode;
>  	u32 val = start ? HDMI_TG_EN : 0;
>  
> -	if (hdata->current_mode.flags & DRM_MODE_FLAG_INTERLACE)
> +	if (m->flags & DRM_MODE_FLAG_INTERLACE)
>  		val |= HDMI_FIELD_EN;
>  
>  	hdmi_reg_writemask(hdata, HDMI_CON_0, val, HDMI_EN);
> @@ -1160,7 +1159,7 @@ static void hdmiphy_wait_for_pll(struct hdmi_context *hdata)
>  
>  static void hdmi_v13_mode_apply(struct hdmi_context *hdata)
>  {
> -	struct drm_display_mode *m = &hdata->current_mode;
> +	struct drm_display_mode *m = &hdata->encoder.crtc->state->mode;
>  	unsigned int val;
>  
>  	hdmi_reg_writev(hdata, HDMI_H_BLANK_0, 2, m->htotal - m->hdisplay);
> @@ -1239,7 +1238,7 @@ static void hdmi_v13_mode_apply(struct hdmi_context *hdata)
>  
>  static void hdmi_v14_mode_apply(struct hdmi_context *hdata)
>  {
> -	struct drm_display_mode *m = &hdata->current_mode;
> +	struct drm_display_mode *m = &hdata->encoder.crtc->state->mode;
>  
>  	hdmi_reg_writev(hdata, HDMI_H_BLANK_0, 2, m->htotal - m->hdisplay);
>  	hdmi_reg_writev(hdata, HDMI_V_LINE_0, 2, m->vtotal);
> @@ -1372,10 +1371,11 @@ static void hdmiphy_enable_mode_set(struct hdmi_context *hdata, bool enable)
>  
>  static void hdmiphy_conf_apply(struct hdmi_context *hdata)
>  {
> +	struct drm_display_mode *m = &hdata->encoder.crtc->state->mode;
>  	int ret;
>  	const u8 *phy_conf;
>  
> -	ret = hdmi_find_phy_conf(hdata, hdata->current_mode.clock * 1000);
> +	ret = hdmi_find_phy_conf(hdata, m->clock * 1000);
>  	if (ret < 0) {
>  		DRM_ERROR("failed to find hdmiphy conf\n");
>  		return;
> @@ -1407,21 +1407,6 @@ static void hdmi_conf_apply(struct hdmi_context *hdata)
>  	hdmi_audio_control(hdata, true);
>  }
>  
> -static void hdmi_mode_set(struct drm_encoder *encoder,
> -			  struct drm_display_mode *mode,
> -			  struct drm_display_mode *adjusted_mode)
> -{
> -	struct hdmi_context *hdata = encoder_to_hdmi(encoder);
> -	struct drm_display_mode *m = adjusted_mode;
> -
> -	DRM_DEBUG_KMS("xres=%d, yres=%d, refresh=%d, intl=%s\n",
> -		m->hdisplay, m->vdisplay,
> -		m->vrefresh, (m->flags & DRM_MODE_FLAG_INTERLACE) ?
> -		"INTERLACED" : "PROGRESSIVE");
> -
> -	drm_mode_copy(&hdata->current_mode, m);
> -}
> -
>  static void hdmi_set_refclk(struct hdmi_context *hdata, bool on)
>  {
>  	if (!hdata->sysreg)
> @@ -1504,7 +1489,6 @@ static void hdmi_disable(struct drm_encoder *encoder)
>  
>  static const struct drm_encoder_helper_funcs exynos_hdmi_encoder_helper_funcs = {
>  	.mode_fixup	= hdmi_mode_fixup,
> -	.mode_set	= hdmi_mode_set,
>  	.enable		= hdmi_enable,
>  	.disable	= hdmi_disable,
>  };
> 

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

Re: [PATCH 07/10] drm/exynos: add mode_fixup callback to exynos_drm_crtc_ops

[Tobias Jakobi]

Hello Andrzej,


Andrzej Hajda wrote:
> crtc::mode_fixup callback is required by crtcs which use internally
> different mode than requested by user - case of Exynos Mixer.
"...which internally use a different..."


Reviewed-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>

With one suggestion below.


> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_crtc.c | 15 +++++++++++++++
>  drivers/gpu/drm/exynos/exynos_drm_drv.h  |  3 +++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> index 6ce0821..dc01342 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> @@ -95,8 +95,23 @@ static enum drm_mode_status exynos_crtc_mode_valid(struct drm_crtc *crtc,
>  	return MODE_OK;
>  }
>  
> +static bool exynos_crtc_mode_fixup(struct drm_crtc *crtc,
> +		const struct drm_display_mode *mode,
> +		struct drm_display_mode *adjusted_mode)
> +{
> +	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
> +
> +	if (exynos_crtc->ops->mode_fixup)
> +		return exynos_crtc->ops->mode_fixup(exynos_crtc, mode,
> +				adjusted_mode);
> +
> +	return true;
> +}
> +
> +
>  static const struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
>  	.mode_valid	= exynos_crtc_mode_valid,
> +	.mode_fixup	= exynos_crtc_mode_fixup,
>  	.atomic_check	= exynos_crtc_atomic_check,
>  	.atomic_begin	= exynos_crtc_atomic_begin,
>  	.atomic_flush	= exynos_crtc_atomic_flush,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index cf131c2..e8bcc72 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -136,6 +136,9 @@ struct exynos_drm_crtc_ops {
>  	u32 (*get_vblank_counter)(struct exynos_drm_crtc *crtc);
>  	enum drm_mode_status (*mode_valid)(struct exynos_drm_crtc *crtc,
>  		const struct drm_display_mode *mode);
> +	bool (*mode_fixup)(struct exynos_drm_crtc *crtc,
> +			   const struct drm_display_mode *mode,
> +			   struct drm_display_mode *adjusted_mode);
I'm always wary when I encounter a bool as return value, since to check what
true/false actually encodes, I need to have some reference which I can check.
Just go for an integer here and use standard convention that negative values are
errors?



>  	int (*atomic_check)(struct exynos_drm_crtc *crtc,
>  			    struct drm_crtc_state *state);
>  	void (*atomic_begin)(struct exynos_drm_crtc *crtc);
> 

Re: [PATCH 08/10] drm/exynos/mixer: pass actual mode on MIXER to encoder

[Tobias Jakobi]

Hello Andrzej,


Andrzej Hajda wrote:
> MIXER in SoCs prior to Exynos5420 supports only 4 video modes:
> 720x480, 720x576, 1280x720, 1920x1080. Support for other modes can be
> enabled by manipulating timings of HDMI. To allow it MIXER must pass
> actual video mode to HDMI, the proper way to do it is to modify
> adjusted_mode property in crtc::mode_fixup callback. Adding such callback
> allows also to simplify mixer_cfg_scan code - choosing mode is performed
> already in crtc::mode_fixup. mode_fixup is also better place to check
> interlace flag.

Reviewed-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>


> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 70 +++++++++++++++++++++++++----------
>  1 file changed, 50 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index f6ea9d9..5aae82b 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -115,6 +115,7 @@ struct mixer_context {
>  	struct clk		*sclk_hdmi;
>  	struct clk		*mout_mixer;
>  	enum mixer_version_id	mxr_ver;
> +	int			scan_value;
>  };
>  
>  struct mixer_drv_data {
> @@ -367,23 +368,11 @@ static void mixer_cfg_scan(struct mixer_context *ctx, int width, int height)
>  	val = test_bit(MXR_BIT_INTERLACE, &ctx->flags) ?
>  		MXR_CFG_SCAN_INTERLACE : MXR_CFG_SCAN_PROGRESSIVE;
>  
> -	/* setup display size */
> -	if (ctx->mxr_ver == MXR_VER_128_0_0_184) {
> +	if (ctx->mxr_ver == MXR_VER_128_0_0_184)
>  		mixer_reg_write(ctx, MXR_RESOLUTION,
>  			MXR_MXR_RES_HEIGHT(height) | MXR_MXR_RES_WIDTH(width));
> -	} else {
> -		/* choosing between proper HD and SD mode */
> -		if (height <= 480)
> -			val |= MXR_CFG_SCAN_NTSC | MXR_CFG_SCAN_SD;
> -		else if (height <= 576)
> -			val |= MXR_CFG_SCAN_PAL | MXR_CFG_SCAN_SD;
> -		else if (height <= 720)
> -			val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD;
> -		else if (height <= 1080)
> -			val |= MXR_CFG_SCAN_HD_1080 | MXR_CFG_SCAN_HD;
> -		else
> -			val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD;
> -	}
> +	else
> +		val |= ctx->scan_value;
>  
>  	mixer_reg_writemask(ctx, MXR_CFG, val, MXR_CFG_SCAN_MASK);
>  }
> @@ -467,11 +456,6 @@ static void mixer_commit(struct mixer_context *ctx)
>  {
>  	struct drm_display_mode *mode = &ctx->crtc->base.state->adjusted_mode;
>  
> -	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> -		__set_bit(MXR_BIT_INTERLACE, &ctx->flags);
> -	else
> -		__clear_bit(MXR_BIT_INTERLACE, &ctx->flags);
> -
>  	mixer_cfg_scan(ctx, mode->hdisplay, mode->vdisplay);
>  	mixer_cfg_rgb_fmt(ctx, mode->vdisplay);
>  	mixer_run(ctx);
> @@ -1033,6 +1017,51 @@ static int mixer_mode_valid(struct exynos_drm_crtc *crtc,
>  	return MODE_BAD;
>  }
>  
> +static bool mixer_mode_fixup(struct exynos_drm_crtc *crtc,
> +		   const struct drm_display_mode *mode,
> +		   struct drm_display_mode *adjusted_mode)
> +{
> +	struct mixer_context *ctx = crtc->ctx;
> +	int width = mode->hdisplay, height = mode->vdisplay, i;
> +
> +	struct {
> +		int hdisplay, vdisplay, htotal, vtotal, scan_val;
> +	} static const modes[] = {
> +		{ 720, 480, 858, 525, MXR_CFG_SCAN_NTSC | MXR_CFG_SCAN_SD },
> +		{ 720, 576, 864, 625, MXR_CFG_SCAN_PAL | MXR_CFG_SCAN_SD },
> +		{ 1280, 720, 1650, 750, MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD },
> +		{ 1920, 1080, 2200, 1125, MXR_CFG_SCAN_HD_1080 | MXR_CFG_SCAN_HD }
> +	};
> +
> +	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> +		__set_bit(MXR_BIT_INTERLACE, &ctx->flags);
> +	else
> +		__clear_bit(MXR_BIT_INTERLACE, &ctx->flags);
> +
> +	if (ctx->mxr_ver == MXR_VER_128_0_0_184)
> +		return true;
> +
> +	for (i = 0; i < ARRAY_SIZE(modes); ++i)
> +		if (width <= modes[i].hdisplay && height <= modes[i].vdisplay) {
> +			ctx->scan_value = modes[i].scan_val;
> +			if (width < modes[i].hdisplay ||
> +			    height < modes[i].vdisplay) {
> +				adjusted_mode->hdisplay = modes[i].hdisplay;
> +				adjusted_mode->hsync_start = modes[i].hdisplay;
> +				adjusted_mode->hsync_end = modes[i].htotal;
> +				adjusted_mode->htotal = modes[i].htotal;
> +				adjusted_mode->vdisplay = modes[i].vdisplay;
> +				adjusted_mode->vsync_start = modes[i].vdisplay;
> +				adjusted_mode->vsync_end = modes[i].vtotal;
> +				adjusted_mode->vtotal = modes[i].vtotal;
> +			}
> +
> +			return true;
> +		}
> +
> +	return false;
> +}
> +
>  static const struct exynos_drm_crtc_ops mixer_crtc_ops = {
>  	.enable			= mixer_enable,
>  	.disable		= mixer_disable,
> @@ -1043,6 +1072,7 @@ static const struct exynos_drm_crtc_ops mixer_crtc_ops = {
>  	.disable_plane		= mixer_disable_plane,
>  	.atomic_flush		= mixer_atomic_flush,
>  	.mode_valid		= mixer_mode_valid,
> +	.mode_fixup		= mixer_mode_fixup,
>  };
>  
>  static const struct mixer_drv_data exynos5420_mxr_drv_data = {
> 

Re: [PATCH 09/10] drm/exynos/hdmi: quirk for support mode timings conversion

[Tobias Jakobi]

Hello Andrzej,


Andrzej Hajda wrote:
> MIXER in SoCs prior to Exynos5420 supports only 4 video modes:
> 720x480, 720x576, 1280x720, 1920x1080. Support for other modes
> can be enabled by manipulating timings of HDMI. To do it
> adjusted_mode should contain actual mode set on crtc.
> With this patch it is possible to enable 1024x768 and 1280x1024
> modes in MIXER.

Reviewed-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>

And some question below.


> Suggested-by: Daniel Drake <drake@endlessm.com>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_hdmi.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 7225b65..4b081f6 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -1239,6 +1239,17 @@ static void hdmi_v13_mode_apply(struct hdmi_context *hdata)
>  static void hdmi_v14_mode_apply(struct hdmi_context *hdata)
>  {
>  	struct drm_display_mode *m = &hdata->encoder.crtc->state->mode;
> +	struct drm_display_mode *am = &hdata->encoder.crtc->state->adjusted_mode;
> +	int hquirk = 0;
> +
> +	/*
> +	 * In case video mode coming from CRTC differs from requested one HDMI
> +	 * sometimes is able to almost properly perform conversion - only
> +	 * first line is distorted.
> +	 */
> +	if ((m->vdisplay != am->vdisplay) &&
> +	    (m->hdisplay == 1280 || m->hdisplay == 1024))
> +		hquirk = 258;
Can it even happen, that m->hdisplay is neither 1280 nor 1024 but "m->vdisplay
!= am->vdisplay" still holds?



>  	hdmi_reg_writev(hdata, HDMI_H_BLANK_0, 2, m->htotal - m->hdisplay);
>  	hdmi_reg_writev(hdata, HDMI_V_LINE_0, 2, m->vtotal);
> @@ -1332,8 +1343,8 @@ static void hdmi_v14_mode_apply(struct hdmi_context *hdata)
>  	hdmi_reg_writev(hdata, HDMI_V_SYNC_LINE_AFT_PXL_6_0, 2, 0xffff);
>  
>  	hdmi_reg_writev(hdata, HDMI_TG_H_FSZ_L, 2, m->htotal);
> -	hdmi_reg_writev(hdata, HDMI_TG_HACT_ST_L, 2, m->htotal - m->hdisplay);
> -	hdmi_reg_writev(hdata, HDMI_TG_HACT_SZ_L, 2, m->hdisplay);
> +	hdmi_reg_writev(hdata, HDMI_TG_HACT_ST_L, 2, m->htotal - m->hdisplay - hquirk);
> +	hdmi_reg_writev(hdata, HDMI_TG_HACT_SZ_L, 2, m->hdisplay + hquirk);
>  	hdmi_reg_writev(hdata, HDMI_TG_V_FSZ_L, 2, m->vtotal);
>  	if (hdata->drv_data == &exynos5433_hdmi_driver_data)
>  		hdmi_reg_writeb(hdata, HDMI_TG_DECON_EN, 1);
> 

Re: [PATCH 10/10] drm/exynos/mixer: enable support for 1024x768 and 1280x1024 modes

[Tobias Jakobi]

Hello Andrzej,


Andrzej Hajda wrote:
> Since HDMI can handle these modes despite of MIXER limitations lets
> enable them.
lets --> let's

Reviewed-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>


> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 5aae82b..108dccb 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -1014,6 +1014,9 @@ static int mixer_mode_valid(struct exynos_drm_crtc *crtc,
>  		(w >= 1664 && w <= 1920 && h >= 936 && h <= 1080))
>  		return MODE_OK;
>  
> +	if ((w == 1024 && h == 768) || (w == 1280 && h == 1024))
> +		return MODE_OK;
> +
>  	return MODE_BAD;
>  }
>  
> 

Re: [PATCH 01/10] drm/exynos/mixer: abstract out output mode setup code

[Andrzej Hajda]

Hi Tobias,

Thanks for review.


On 12.09.2017 14:29, Tobias Jakobi wrote:
> Hello Andrzej,
>
>
> Andrzej Hajda wrote:
>> Mode setup code is called from video plane update and mixer plane update.
>> Lets group it together in mixer_commit function like in case of other
>> Exynos CRTCs.
> Minor typo: Lets --> Let's
>
> Reviewed-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
>
> With some small question below.
>
>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_mixer.c | 33 ++++++++++++++++++---------------
>>  1 file changed, 18 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index 0027554..499ebdc 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -473,6 +473,22 @@ static void mixer_stop(struct mixer_context *ctx)
>>  		usleep_range(10000, 12000);
>>  }
>>  
>> +static void mixer_commit(struct mixer_context *ctx)
>> +{
>> +	struct drm_display_mode *mode = &ctx->crtc->base.state->adjusted_mode;
>> +
>> +	/* setup display size */
>> +	if (ctx->mxr_ver == MXR_VER_128_0_0_184) {
>> +		u32 val  = MXR_MXR_RES_HEIGHT(mode->vdisplay)
>> +			 | MXR_MXR_RES_WIDTH(mode->hdisplay);
>> +		mixer_reg_write(&ctx->mixer_res, MXR_RESOLUTION, val);
>> +	}
>> +
>> +	mixer_cfg_scan(ctx, mode->vdisplay);
>> +	mixer_cfg_rgb_fmt(ctx, mode->vdisplay);
>> +	mixer_run(ctx);
>> +}
>> +
>>  static void vp_video_buffer(struct mixer_context *ctx,
>>  			    struct exynos_drm_plane *plane)
>>  {
>> @@ -553,11 +569,9 @@ static void vp_video_buffer(struct mixer_context *ctx,
>>  	vp_reg_write(res, VP_TOP_C_PTR, chroma_addr[0]);
>>  	vp_reg_write(res, VP_BOT_C_PTR, chroma_addr[1]);
>>  
>> -	mixer_cfg_scan(ctx, mode->vdisplay);
>> -	mixer_cfg_rgb_fmt(ctx, mode->vdisplay);
>>  	mixer_cfg_layer(ctx, plane->index, priority, true);
>>  	mixer_cfg_vp_blend(ctx);
>> -	mixer_run(ctx);
>> +	mixer_commit(ctx);
>>  
>>  	spin_unlock_irqrestore(&res->reg_slock, flags);
>>  
>> @@ -638,14 +652,6 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>>  	mixer_reg_write(res, MXR_GRAPHIC_SPAN(win),
>>  			fb->pitches[0] / fb->format->cpp[0]);
>>  
>> -	/* setup display size */
>> -	if (ctx->mxr_ver == MXR_VER_128_0_0_184 &&
>> -		win == DEFAULT_WIN) {
>> -		val  = MXR_MXR_RES_HEIGHT(mode->vdisplay);
>> -		val |= MXR_MXR_RES_WIDTH(mode->hdisplay);
>> -		mixer_reg_write(res, MXR_RESOLUTION, val);
>> -	}
>> -
>>  	val  = MXR_GRP_WH_WIDTH(state->src.w);
>>  	val |= MXR_GRP_WH_HEIGHT(state->src.h);
>>  	val |= MXR_GRP_WH_H_SCALE(x_ratio);
>> @@ -660,18 +666,15 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
>>  	/* set buffer address to mixer */
>>  	mixer_reg_write(res, MXR_GRAPHIC_BASE(win), dma_addr);
>>  
>> -	mixer_cfg_scan(ctx, mode->vdisplay);
>> -	mixer_cfg_rgb_fmt(ctx, mode->vdisplay);
>>  	mixer_cfg_layer(ctx, win, priority, true);
>>  	mixer_cfg_gfx_blend(ctx, win, is_alpha_format(fb->format->format));
>> +	mixer_commit(ctx);
> With this change, mixer_run() is now done before mixer_layer_update(). I just
> wanted to make sure that the order of operations isn't critical here.

It shoudn't matter, as it is between atomic_begin, which disables shadow
updates,
and atomic_flush, which re-enables them.

Regards
Andrzej

>
>
>
>>  	/* layer update mandatory for mixer 16.0.33.0 */
>>  	if (ctx->mxr_ver == MXR_VER_16_0_33_0 ||
>>  		ctx->mxr_ver == MXR_VER_128_0_0_184)
>>  		mixer_layer_update(ctx);
>>  
>> -	mixer_run(ctx);
>> -
>>  	spin_unlock_irqrestore(&res->reg_slock, flags);
>>  
>>  	mixer_regs_dump(ctx);
>>
>
>
>

Re: [PATCH 04/10] drm/exynos/mixer: fix mode validation code

[Andrzej Hajda]

On 12.09.2017 14:31, Tobias Jakobi wrote:
> Hello Andrzej,
>
>
> Andrzej Hajda wrote:
>> Mode limitation checked in mixer driver affects only older HW.
>> Mixer in Exynos542x has no such limitations. While at it patch changes
>> validation callback to recently introduced mode_valid which is more
>> suitable for the check. Additionally little cleanup is performed.
> Reviewed-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
>
> And some small suggestion below.
>
>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_mixer.c | 24 +++++++++++-------------
>>  1 file changed, 11 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index a87f60b..d530c18 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -1040,26 +1040,24 @@ static void mixer_disable(struct exynos_drm_crtc *crtc)
>>  	clear_bit(MXR_BIT_POWERED, &ctx->flags);
>>  }
>>  
>> -/* Only valid for Mixer version 16.0.33.0 */
>> -static int mixer_atomic_check(struct exynos_drm_crtc *crtc,
>> -		       struct drm_crtc_state *state)
>> +static int mixer_mode_valid(struct exynos_drm_crtc *crtc,
>> +		const struct drm_display_mode *mode)
>>  {
>> -	struct drm_display_mode *mode = &state->adjusted_mode;
>> -	u32 w, h;
>> +	struct mixer_context *ctx = crtc->ctx;
>> +	u32 w = mode->hdisplay, h = mode->vdisplay;
>>  
>> -	w = mode->hdisplay;
>> -	h = mode->vdisplay;
>> +	DRM_DEBUG_KMS("xres=%d, yres=%d, refresh=%d, intl=%d\n", w, h,
>> +		mode->vrefresh, !!(mode->flags & DRM_MODE_FLAG_INTERLACE));
>>  
>> -	DRM_DEBUG_KMS("xres=%d, yres=%d, refresh=%d, intl=%d\n",
>> -		mode->hdisplay, mode->vdisplay, mode->vrefresh,
>> -		(mode->flags & DRM_MODE_FLAG_INTERLACE) ? 1 : 0);
>> +	if (ctx->mxr_ver == MXR_VER_128_0_0_184)
>> +		return MODE_OK;
>>  
>>  	if ((w >= 464 && w <= 720 && h >= 261 && h <= 576) ||
>>  		(w >= 1024 && w <= 1280 && h >= 576 && h <= 720) ||
>>  		(w >= 1664 && w <= 1920 && h >= 936 && h <= 1080))
> I think it would be nice to have these aligned.

Yes, it should look better.

Regards
Andrzej


>
>
>> -		return 0;
>> +		return MODE_OK;
>>  
>> -	return -EINVAL;
>> +	return MODE_BAD;
>>  }
>>  
>>  static const struct exynos_drm_crtc_ops mixer_crtc_ops = {
>> @@ -1071,7 +1069,7 @@ static const struct exynos_drm_crtc_ops mixer_crtc_ops = {
>>  	.update_plane		= mixer_update_plane,
>>  	.disable_plane		= mixer_disable_plane,
>>  	.atomic_flush		= mixer_atomic_flush,
>> -	.atomic_check		= mixer_atomic_check,
>> +	.mode_valid		= mixer_mode_valid,
>>  };
>>  
>>  static const struct mixer_drv_data exynos5420_mxr_drv_data = {
>>
>
>
>

Re: [PATCH 07/10] drm/exynos: add mode_fixup callback to exynos_drm_crtc_ops

[Andrzej Hajda]

On 12.09.2017 14:41, Tobias Jakobi wrote:
> Hello Andrzej,
>
>
> Andrzej Hajda wrote:
>> crtc::mode_fixup callback is required by crtcs which use internally
>> different mode than requested by user - case of Exynos Mixer.
> "...which internally use a different..."
>
>
> Reviewed-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
>
> With one suggestion below.
>
>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_crtc.c | 15 +++++++++++++++
>>  drivers/gpu/drm/exynos/exynos_drm_drv.h  |  3 +++
>>  2 files changed, 18 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> index 6ce0821..dc01342 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> @@ -95,8 +95,23 @@ static enum drm_mode_status exynos_crtc_mode_valid(struct drm_crtc *crtc,
>>  	return MODE_OK;
>>  }
>>  
>> +static bool exynos_crtc_mode_fixup(struct drm_crtc *crtc,
>> +		const struct drm_display_mode *mode,
>> +		struct drm_display_mode *adjusted_mode)
>> +{
>> +	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
>> +
>> +	if (exynos_crtc->ops->mode_fixup)
>> +		return exynos_crtc->ops->mode_fixup(exynos_crtc, mode,
>> +				adjusted_mode);
>> +
>> +	return true;
>> +}
>> +
>> +
>>  static const struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
>>  	.mode_valid	= exynos_crtc_mode_valid,
>> +	.mode_fixup	= exynos_crtc_mode_fixup,
>>  	.atomic_check	= exynos_crtc_atomic_check,
>>  	.atomic_begin	= exynos_crtc_atomic_begin,
>>  	.atomic_flush	= exynos_crtc_atomic_flush,
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> index cf131c2..e8bcc72 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> @@ -136,6 +136,9 @@ struct exynos_drm_crtc_ops {
>>  	u32 (*get_vblank_counter)(struct exynos_drm_crtc *crtc);
>>  	enum drm_mode_status (*mode_valid)(struct exynos_drm_crtc *crtc,
>>  		const struct drm_display_mode *mode);
>> +	bool (*mode_fixup)(struct exynos_drm_crtc *crtc,
>> +			   const struct drm_display_mode *mode,
>> +			   struct drm_display_mode *adjusted_mode);
> I'm always wary when I encounter a bool as return value, since to check what
> true/false actually encodes, I need to have some reference which I can check.
> Just go for an integer here and use standard convention that negative values are
> errors?

It is forwarder for drm core op so I prefer to keep return values
consistent.

Regards
Andrzej

>
>
>
>>  	int (*atomic_check)(struct exynos_drm_crtc *crtc,
>>  			    struct drm_crtc_state *state);
>>  	void (*atomic_begin)(struct exynos_drm_crtc *crtc);
>>
>
>
>

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

Re: [PATCH 09/10] drm/exynos/hdmi: quirk for support mode timings conversion

[Andrzej Hajda]

On 12.09.2017 14:50, Tobias Jakobi wrote:
> Hello Andrzej,
>
>
> Andrzej Hajda wrote:
>> MIXER in SoCs prior to Exynos5420 supports only 4 video modes:
>> 720x480, 720x576, 1280x720, 1920x1080. Support for other modes
>> can be enabled by manipulating timings of HDMI. To do it
>> adjusted_mode should contain actual mode set on crtc.
>> With this patch it is possible to enable 1024x768 and 1280x1024
>> modes in MIXER.
> Reviewed-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
>
> And some question below.
>
>
>> Suggested-by: Daniel Drake <drake@endlessm.com>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_hdmi.c | 15 +++++++++++++--
>>  1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index 7225b65..4b081f6 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -1239,6 +1239,17 @@ static void hdmi_v13_mode_apply(struct hdmi_context *hdata)
>>  static void hdmi_v14_mode_apply(struct hdmi_context *hdata)
>>  {
>>  	struct drm_display_mode *m = &hdata->encoder.crtc->state->mode;
>> +	struct drm_display_mode *am = &hdata->encoder.crtc->state->adjusted_mode;
>> +	int hquirk = 0;
>> +
>> +	/*
>> +	 * In case video mode coming from CRTC differs from requested one HDMI
>> +	 * sometimes is able to almost properly perform conversion - only
>> +	 * first line is distorted.
>> +	 */
>> +	if ((m->vdisplay != am->vdisplay) &&
>> +	    (m->hdisplay == 1280 || m->hdisplay == 1024))
>> +		hquirk = 258;
> Can it even happen, that m->hdisplay is neither 1280 nor 1024 but "m->vdisplay
> != am->vdisplay" still holds?

1680x1050-60 for example, it works without quirks.

Regards
Andrzej

>
>
>
>>  	hdmi_reg_writev(hdata, HDMI_H_BLANK_0, 2, m->htotal - m->hdisplay);
>>  	hdmi_reg_writev(hdata, HDMI_V_LINE_0, 2, m->vtotal);
>> @@ -1332,8 +1343,8 @@ static void hdmi_v14_mode_apply(struct hdmi_context *hdata)
>>  	hdmi_reg_writev(hdata, HDMI_V_SYNC_LINE_AFT_PXL_6_0, 2, 0xffff);
>>  
>>  	hdmi_reg_writev(hdata, HDMI_TG_H_FSZ_L, 2, m->htotal);
>> -	hdmi_reg_writev(hdata, HDMI_TG_HACT_ST_L, 2, m->htotal - m->hdisplay);
>> -	hdmi_reg_writev(hdata, HDMI_TG_HACT_SZ_L, 2, m->hdisplay);
>> +	hdmi_reg_writev(hdata, HDMI_TG_HACT_ST_L, 2, m->htotal - m->hdisplay - hquirk);
>> +	hdmi_reg_writev(hdata, HDMI_TG_HACT_SZ_L, 2, m->hdisplay + hquirk);
>>  	hdmi_reg_writev(hdata, HDMI_TG_V_FSZ_L, 2, m->vtotal);
>>  	if (hdata->drv_data == &exynos5433_hdmi_driver_data)
>>  		hdmi_reg_writeb(hdata, HDMI_TG_DECON_EN, 1);
>>
>
>
>

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

Re: [PATCH 10/10] drm/exynos/mixer: enable support for 1024x768 and 1280x1024 modes

[Andrzej Hajda]

On 12.09.2017 14:50, Tobias Jakobi wrote:
> Hello Andrzej,
>
>
> Andrzej Hajda wrote:
>> Since HDMI can handle these modes despite of MIXER limitations lets
>> enable them.
> lets --> let's
>
> Reviewed-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>


Thanks for review, I will apply all your grammar/style suggestions in
next iteration.

Regards
Andrzej

>
>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_mixer.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index 5aae82b..108dccb 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -1014,6 +1014,9 @@ static int mixer_mode_valid(struct exynos_drm_crtc *crtc,
>>  		(w >= 1664 && w <= 1920 && h >= 936 && h <= 1080))
>>  		return MODE_OK;
>>  
>> +	if ((w == 1024 && h == 768) || (w == 1280 && h == 1024))
>> +		return MODE_OK;
>> +
>>  	return MODE_BAD;
>>  }
>>  
>>
>
>
>

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