[PATCH -v2 00/14] drm/exynos: clean up + atomic phase 1 and 2

[PATCH -v2 00/14] drm/exynos: clean up + atomic phase 1 and 2

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Hi,

This is the v2 of this patchset. The only difference here is that I added
the zpos refactor to this series after the review of v1 today. No changes
were made to atomic patches besides solving conflicts after the new zpos
changes.

This series clean ups a few more paths from exynos-drm with the most important
being the removal of the global page flip queue, the zpos refactor, and the
removal in driver internal data (struct *_win_data) that was replicating plane
data.

Following these patches comes the first step torwards atomic modesetting
support on exynos.

These patches are applied on top of the dpms clean patches from Joonyoung and
rebased on exynos-drm-next.

Gustavo Padovan (13):
  drm/exynos: remove unused exynos_crtc->win_enable() callback
  drm/exynos: remove struct *_win_data abstraction on planes
  drm/exynos: preset zpos value for overlay planes
  drm/exynos: make zpos property immutable
  drm/exynos: remove exynos_plane_destroy()
  drm/exynos: remove leftover functions declarations
  drm/exynos: atomic phase 1: use drm_plane_helper_update()
  drm/exynos: atomic phase 1: use drm_plane_helper_disable()
  drm/exynos: atomic phase 1: add atomic_begin()/atomic_flush()
  drm/exynos: atomic phase 1: add .mode_set_nofb() callback
  drm/exynos: atomic phase 2: wire up state reset(), duplicate() and
    destroy()
  drm/exynos: atomic phase 2: keep track of framebuffer pointer
  drm/exynos: make exynos_plane_mode_set() static

Mandeep Singh Baines (1):
  drm/exynos: track vblank events on a per crtc basis

 drivers/gpu/drm/bridge/ptn3460.c              |   4 +
 drivers/gpu/drm/exynos/exynos_dp_core.c       |   4 +
 drivers/gpu/drm/exynos/exynos_drm_connector.c |   4 +
 drivers/gpu/drm/exynos/exynos_drm_crtc.c      | 205 ++++++++++-----------
 drivers/gpu/drm/exynos/exynos_drm_crtc.h      |   7 +-
 drivers/gpu/drm/exynos/exynos_drm_dpi.c       |   4 +
 drivers/gpu/drm/exynos/exynos_drm_drv.c       |  29 +--
 drivers/gpu/drm/exynos/exynos_drm_drv.h       |  26 +--
 drivers/gpu/drm/exynos/exynos_drm_dsi.c       |   4 +
 drivers/gpu/drm/exynos/exynos_drm_fb.c        |   2 +-
 drivers/gpu/drm/exynos/exynos_drm_fimd.c      | 245 +++++++++++---------------
 drivers/gpu/drm/exynos/exynos_drm_plane.c     | 126 +++++++------
 drivers/gpu/drm/exynos/exynos_drm_plane.h     |  14 +-
 drivers/gpu/drm/exynos/exynos_drm_vidi.c      | 138 ++++-----------
 drivers/gpu/drm/exynos/exynos_hdmi.c          |   4 +
 drivers/gpu/drm/exynos/exynos_mixer.c         | 217 ++++++++---------------
 16 files changed, 418 insertions(+), 615 deletions(-)

-- 
1.9.3

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

[PATCH -v2 01/14] drm/exynos: remove unused exynos_crtc->win_enable() callback

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

None of the exynos crtc drivers implements win_enable() so remove it for
better clarity of the code.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 1aceafc..66f4a06 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -174,7 +174,6 @@ struct exynos_drm_display {
  *	hardware overlay is updated.
  * @win_mode_set: copy drm overlay info to hw specific overlay info.
  * @win_commit: apply hardware specific overlay data to registers.
- * @win_enable: enable hardware specific overlay.
  * @win_disable: disable hardware specific overlay.
  * @te_handler: trigger to transfer video image at the tearing effect
  *	synchronization signal if there is a page flip request.
@@ -192,7 +191,6 @@ struct exynos_drm_crtc_ops {
 	void (*win_mode_set)(struct exynos_drm_crtc *crtc,
 				struct exynos_drm_plane *plane);
 	void (*win_commit)(struct exynos_drm_crtc *crtc, int zpos);
-	void (*win_enable)(struct exynos_drm_crtc *crtc, int zpos);
 	void (*win_disable)(struct exynos_drm_crtc *crtc, int zpos);
 	void (*te_handler)(struct exynos_drm_crtc *crtc);
 };
-- 
1.9.3

[PATCH -v2 02/14] drm/exynos: remove struct *_win_data abstraction on planes

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

struct {fimd,mixer,vidi}_win_data was just keeping the same data
as struct exynos_drm_plane thus get ride of it and use exynos_drm_plane
directly.

It changes how planes are created and remove .win_mode_set() callback
that was only filling all *_win_data structs.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c  |   9 +-
 drivers/gpu/drm/exynos/exynos_drm_crtc.h  |   1 +
 drivers/gpu/drm/exynos/exynos_drm_drv.c   |  14 --
 drivers/gpu/drm/exynos/exynos_drm_drv.h   |   5 +-
 drivers/gpu/drm/exynos/exynos_drm_fimd.c  | 182 ++++++++++---------------
 drivers/gpu/drm/exynos/exynos_drm_plane.c |  23 +---
 drivers/gpu/drm/exynos/exynos_drm_plane.h |   6 +-
 drivers/gpu/drm/exynos/exynos_drm_vidi.c  | 123 +++++------------
 drivers/gpu/drm/exynos/exynos_mixer.c     | 212 +++++++++++-------------------
 9 files changed, 183 insertions(+), 392 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 48ccab7..47dd2b0 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -239,13 +239,13 @@ static struct drm_crtc_funcs exynos_crtc_funcs = {
 };
 
 struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
+					       struct drm_plane *plane,
 					       int pipe,
 					       enum exynos_drm_output_type type,
 					       struct exynos_drm_crtc_ops *ops,
 					       void *ctx)
 {
 	struct exynos_drm_crtc *exynos_crtc;
-	struct drm_plane *plane;
 	struct exynos_drm_private *private = drm_dev->dev_private;
 	struct drm_crtc *crtc;
 	int ret;
@@ -262,12 +262,6 @@ struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
 	exynos_crtc->type = type;
 	exynos_crtc->ops = ops;
 	exynos_crtc->ctx = ctx;
-	plane = exynos_plane_init(drm_dev, 1 << pipe,
-				  DRM_PLANE_TYPE_PRIMARY);
-	if (IS_ERR(plane)) {
-		ret = PTR_ERR(plane);
-		goto err_plane;
-	}
 
 	crtc = &exynos_crtc->base;
 
@@ -284,7 +278,6 @@ struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
 
 err_crtc:
 	plane->funcs->destroy(plane);
-err_plane:
 	kfree(exynos_crtc);
 	return ERR_PTR(ret);
 }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.h b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
index 6258b80..e1fd2ef 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
@@ -18,6 +18,7 @@
 #include "exynos_drm_drv.h"
 
 struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
+					       struct drm_plane *plane,
 					       int pipe,
 					       enum exynos_drm_output_type type,
 					       struct exynos_drm_crtc_ops *ops,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 1bcbe07..c598197 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -55,7 +55,6 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
 {
 	struct exynos_drm_private *private;
 	int ret;
-	int nr;
 
 	private = kzalloc(sizeof(struct exynos_drm_private), GFP_KERNEL);
 	if (!private)
@@ -81,19 +80,6 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
 
 	exynos_drm_mode_config_init(dev);
 
-	for (nr = 0; nr < MAX_PLANE; nr++) {
-		struct drm_plane *plane;
-		unsigned long possible_crtcs = (1 << MAX_CRTC) - 1;
-
-		plane = exynos_plane_init(dev, possible_crtcs,
-					  DRM_PLANE_TYPE_OVERLAY);
-		if (!IS_ERR(plane))
-			continue;
-
-		ret = PTR_ERR(plane);
-		goto err_mode_config_cleanup;
-	}
-
 	/* setup possible_clones. */
 	exynos_drm_encoder_setup(dev);
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 66f4a06..7ea7648 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -78,6 +78,7 @@ enum exynos_drm_output_type {
  * @transparency: transparency on or off.
  * @activated: activated or not.
  * @enabled: enabled or not.
+ * @resume: to resume or not.
  *
  * this structure is common to exynos SoC and its contents would be copied
  * to hardware specific overlay info.
@@ -112,6 +113,7 @@ struct exynos_drm_plane {
 	bool transparency:1;
 	bool activated:1;
 	bool enabled:1;
+	bool resume:1;
 };
 
 /*
@@ -172,7 +174,6 @@ struct exynos_drm_display {
  * @disable_vblank: specific driver callback for disabling vblank interrupt.
  * @wait_for_vblank: wait for vblank interrupt to make sure that
  *	hardware overlay is updated.
- * @win_mode_set: copy drm overlay info to hw specific overlay info.
  * @win_commit: apply hardware specific overlay data to registers.
  * @win_disable: disable hardware specific overlay.
  * @te_handler: trigger to transfer video image at the tearing effect
@@ -188,8 +189,6 @@ struct exynos_drm_crtc_ops {
 	int (*enable_vblank)(struct exynos_drm_crtc *crtc);
 	void (*disable_vblank)(struct exynos_drm_crtc *crtc);
 	void (*wait_for_vblank)(struct exynos_drm_crtc *crtc);
-	void (*win_mode_set)(struct exynos_drm_crtc *crtc,
-				struct exynos_drm_plane *plane);
 	void (*win_commit)(struct exynos_drm_crtc *crtc, int zpos);
 	void (*win_disable)(struct exynos_drm_crtc *crtc, int zpos);
 	void (*te_handler)(struct exynos_drm_crtc *crtc);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 925fc69..489ce90 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -31,6 +31,7 @@
 #include "exynos_drm_drv.h"
 #include "exynos_drm_fbdev.h"
 #include "exynos_drm_crtc.h"
+#include "exynos_drm_plane.h"
 #include "exynos_drm_iommu.h"
 
 /*
@@ -140,31 +141,15 @@ static struct fimd_driver_data exynos5_fimd_driver_data = {
 	.has_vtsel = 1,
 };
 
-struct fimd_win_data {
-	unsigned int		offset_x;
-	unsigned int		offset_y;
-	unsigned int		ovl_width;
-	unsigned int		ovl_height;
-	unsigned int		fb_width;
-	unsigned int		fb_height;
-	unsigned int		bpp;
-	unsigned int		pixel_format;
-	dma_addr_t		dma_addr;
-	unsigned int		buf_offsize;
-	unsigned int		line_size;	/* bytes */
-	bool			enabled;
-	bool			resume;
-};
-
 struct fimd_context {
 	struct device			*dev;
 	struct drm_device		*drm_dev;
 	struct exynos_drm_crtc		*crtc;
+	struct exynos_drm_plane		planes[WINDOWS_NR];
 	struct clk			*bus_clk;
 	struct clk			*lcd_clk;
 	void __iomem			*regs;
 	struct regmap			*sysreg;
-	struct fimd_win_data		win_data[WINDOWS_NR];
 	unsigned int			default_win;
 	unsigned long			irq_flags;
 	u32				vidcon0;
@@ -506,58 +491,9 @@ static void fimd_disable_vblank(struct exynos_drm_crtc *crtc)
 	}
 }
 
-static void fimd_win_mode_set(struct exynos_drm_crtc *crtc,
-			struct exynos_drm_plane *plane)
-{
-	struct fimd_context *ctx = crtc->ctx;
-	struct fimd_win_data *win_data;
-	int win;
-	unsigned long offset;
-
-	if (!plane) {
-		DRM_ERROR("plane is NULL\n");
-		return;
-	}
-
-	win = plane->zpos;
-	if (win == DEFAULT_ZPOS)
-		win = ctx->default_win;
-
-	if (win < 0 || win >= WINDOWS_NR)
-		return;
-
-	offset = plane->fb_x * (plane->bpp >> 3);
-	offset += plane->fb_y * plane->pitch;
-
-	DRM_DEBUG_KMS("offset = 0x%lx, pitch = %x\n", offset, plane->pitch);
-
-	win_data = &ctx->win_data[win];
-
-	win_data->offset_x = plane->crtc_x;
-	win_data->offset_y = plane->crtc_y;
-	win_data->ovl_width = plane->crtc_width;
-	win_data->ovl_height = plane->crtc_height;
-	win_data->fb_width = plane->fb_width;
-	win_data->fb_height = plane->fb_height;
-	win_data->dma_addr = plane->dma_addr[0] + offset;
-	win_data->bpp = plane->bpp;
-	win_data->pixel_format = plane->pixel_format;
-	win_data->buf_offsize = (plane->fb_width - plane->crtc_width) *
-				(plane->bpp >> 3);
-	win_data->line_size = plane->crtc_width * (plane->bpp >> 3);
-
-	DRM_DEBUG_KMS("offset_x = %d, offset_y = %d\n",
-			win_data->offset_x, win_data->offset_y);
-	DRM_DEBUG_KMS("ovl_width = %d, ovl_height = %d\n",
-			win_data->ovl_width, win_data->ovl_height);
-	DRM_DEBUG_KMS("paddr = 0x%lx\n", (unsigned long)win_data->dma_addr);
-	DRM_DEBUG_KMS("fb_width = %d, crtc_width = %d\n",
-			plane->fb_width, plane->crtc_width);
-}
-
 static void fimd_win_set_pixfmt(struct fimd_context *ctx, unsigned int win)
 {
-	struct fimd_win_data *win_data = &ctx->win_data[win];
+	struct exynos_drm_plane *plane = &ctx->planes[win];
 	unsigned long val;
 
 	val = WINCONx_ENWIN;
@@ -567,11 +503,11 @@ static void fimd_win_set_pixfmt(struct fimd_context *ctx, unsigned int win)
 	 * So the request format is ARGB8888 then change it to XRGB8888.
 	 */
 	if (ctx->driver_data->has_limited_fmt && !win) {
-		if (win_data->pixel_format == DRM_FORMAT_ARGB8888)
-			win_data->pixel_format = DRM_FORMAT_XRGB8888;
+		if (plane->pixel_format == DRM_FORMAT_ARGB8888)
+			plane->pixel_format = DRM_FORMAT_XRGB8888;
 	}
 
-	switch (win_data->pixel_format) {
+	switch (plane->pixel_format) {
 	case DRM_FORMAT_C8:
 		val |= WINCON0_BPPMODE_8BPP_PALETTE;
 		val |= WINCONx_BURSTLEN_8WORD;
@@ -607,7 +543,7 @@ static void fimd_win_set_pixfmt(struct fimd_context *ctx, unsigned int win)
 		break;
 	}
 
-	DRM_DEBUG_KMS("bpp = %d\n", win_data->bpp);
+	DRM_DEBUG_KMS("bpp = %d\n", plane->bpp);
 
 	/*
 	 * In case of exynos, setting dma-burst to 16Word causes permanent
@@ -617,7 +553,7 @@ static void fimd_win_set_pixfmt(struct fimd_context *ctx, unsigned int win)
 	 * movement causes unstable DMA which results into iommu crash/tear.
 	 */
 
-	if (win_data->fb_width < MIN_FB_WIDTH_FOR_16WORD_BURST) {
+	if (plane->fb_width < MIN_FB_WIDTH_FOR_16WORD_BURST) {
 		val &= ~WINCONx_BURSTLEN_MASK;
 		val |= WINCONx_BURSTLEN_4WORD;
 	}
@@ -668,11 +604,11 @@ static void fimd_shadow_protect_win(struct fimd_context *ctx,
 static void fimd_win_commit(struct exynos_drm_crtc *crtc, int zpos)
 {
 	struct fimd_context *ctx = crtc->ctx;
-	struct fimd_win_data *win_data;
+	struct exynos_drm_plane *plane;
 	int win = zpos;
-	unsigned long val, alpha, size;
-	unsigned int last_x;
-	unsigned int last_y;
+	dma_addr_t dma_addr;
+	unsigned long val, alpha, size, offset;
+	unsigned int last_x, last_y, buf_offsize, line_size;
 
 	if (ctx->suspended)
 		return;
@@ -683,11 +619,11 @@ static void fimd_win_commit(struct exynos_drm_crtc *crtc, int zpos)
 	if (win < 0 || win >= WINDOWS_NR)
 		return;
 
-	win_data = &ctx->win_data[win];
+	plane = &ctx->planes[win];
 
 	/* If suspended, enable this on resume */
 	if (ctx->suspended) {
-		win_data->resume = true;
+		plane->resume = true;
 		return;
 	}
 
@@ -704,38 +640,45 @@ static void fimd_win_commit(struct exynos_drm_crtc *crtc, int zpos)
 	/* protect windows */
 	fimd_shadow_protect_win(ctx, win, true);
 
+
+	offset = plane->fb_x * (plane->bpp >> 3);
+	offset += plane->fb_y * plane->pitch;
+
 	/* buffer start address */
-	val = (unsigned long)win_data->dma_addr;
+	dma_addr = plane->dma_addr[0] + offset;
+	val = (unsigned long)dma_addr;
 	writel(val, ctx->regs + VIDWx_BUF_START(win, 0));
 
 	/* buffer end address */
-	size = win_data->fb_width * win_data->ovl_height * (win_data->bpp >> 3);
-	val = (unsigned long)(win_data->dma_addr + size);
+	size = plane->fb_width * plane->crtc_height * (plane->bpp >> 3);
+	val = (unsigned long)(dma_addr + size);
 	writel(val, ctx->regs + VIDWx_BUF_END(win, 0));
 
 	DRM_DEBUG_KMS("start addr = 0x%lx, end addr = 0x%lx, size = 0x%lx\n",
-			(unsigned long)win_data->dma_addr, val, size);
+			(unsigned long)dma_addr, val, size);
 	DRM_DEBUG_KMS("ovl_width = %d, ovl_height = %d\n",
-			win_data->ovl_width, win_data->ovl_height);
+			plane->crtc_width, plane->crtc_height);
 
 	/* buffer size */
-	val = VIDW_BUF_SIZE_OFFSET(win_data->buf_offsize) |
-		VIDW_BUF_SIZE_PAGEWIDTH(win_data->line_size) |
-		VIDW_BUF_SIZE_OFFSET_E(win_data->buf_offsize) |
-		VIDW_BUF_SIZE_PAGEWIDTH_E(win_data->line_size);
+	buf_offsize = (plane->fb_width - plane->crtc_width) * (plane->bpp >> 3);
+	line_size = plane->crtc_width * (plane->bpp >> 3);
+	val = VIDW_BUF_SIZE_OFFSET(buf_offsize) |
+		VIDW_BUF_SIZE_PAGEWIDTH(line_size) |
+		VIDW_BUF_SIZE_OFFSET_E(buf_offsize) |
+		VIDW_BUF_SIZE_PAGEWIDTH_E(line_size);
 	writel(val, ctx->regs + VIDWx_BUF_SIZE(win, 0));
 
 	/* OSD position */
-	val = VIDOSDxA_TOPLEFT_X(win_data->offset_x) |
-		VIDOSDxA_TOPLEFT_Y(win_data->offset_y) |
-		VIDOSDxA_TOPLEFT_X_E(win_data->offset_x) |
-		VIDOSDxA_TOPLEFT_Y_E(win_data->offset_y);
+	val = VIDOSDxA_TOPLEFT_X(plane->crtc_x) |
+		VIDOSDxA_TOPLEFT_Y(plane->crtc_y) |
+		VIDOSDxA_TOPLEFT_X_E(plane->crtc_x) |
+		VIDOSDxA_TOPLEFT_Y_E(plane->crtc_y);
 	writel(val, ctx->regs + VIDOSD_A(win));
 
-	last_x = win_data->offset_x + win_data->ovl_width;
+	last_x = plane->crtc_x + plane->crtc_width;
 	if (last_x)
 		last_x--;
-	last_y = win_data->offset_y + win_data->ovl_height;
+	last_y = plane->crtc_y + plane->crtc_height;
 	if (last_y)
 		last_y--;
 
@@ -745,7 +688,7 @@ static void fimd_win_commit(struct exynos_drm_crtc *crtc, int zpos)
 	writel(val, ctx->regs + VIDOSD_B(win));
 
 	DRM_DEBUG_KMS("osd pos: tx = %d, ty = %d, bx = %d, by = %d\n",
-			win_data->offset_x, win_data->offset_y, last_x, last_y);
+			plane->crtc_x, plane->crtc_y, last_x, last_y);
 
 	/* hardware window 0 doesn't support alpha channel. */
 	if (win != 0) {
@@ -762,7 +705,7 @@ static void fimd_win_commit(struct exynos_drm_crtc *crtc, int zpos)
 		u32 offset = VIDOSD_D(win);
 		if (win == 0)
 			offset = VIDOSD_C(win);
-		val = win_data->ovl_width * win_data->ovl_height;
+		val = plane->crtc_width * plane->crtc_height;
 		writel(val, ctx->regs + offset);
 
 		DRM_DEBUG_KMS("osd size = 0x%x\n", (unsigned int)val);
@@ -782,7 +725,7 @@ static void fimd_win_commit(struct exynos_drm_crtc *crtc, int zpos)
 	/* Enable DMA channel and unprotect windows */
 	fimd_shadow_protect_win(ctx, win, false);
 
-	win_data->enabled = true;
+	plane->enabled = true;
 
 	if (ctx->i80_if)
 		atomic_set(&ctx->win_updated, 1);
@@ -791,7 +734,7 @@ static void fimd_win_commit(struct exynos_drm_crtc *crtc, int zpos)
 static void fimd_win_disable(struct exynos_drm_crtc *crtc, int zpos)
 {
 	struct fimd_context *ctx = crtc->ctx;
-	struct fimd_win_data *win_data;
+	struct exynos_drm_plane *plane;
 	int win = zpos;
 
 	if (win == DEFAULT_ZPOS)
@@ -800,11 +743,11 @@ static void fimd_win_disable(struct exynos_drm_crtc *crtc, int zpos)
 	if (win < 0 || win >= WINDOWS_NR)
 		return;
 
-	win_data = &ctx->win_data[win];
+	plane = &ctx->planes[win];
 
 	if (ctx->suspended) {
 		/* do not resume this window*/
-		win_data->resume = false;
+		plane->resume = false;
 		return;
 	}
 
@@ -819,42 +762,42 @@ static void fimd_win_disable(struct exynos_drm_crtc *crtc, int zpos)
 	/* unprotect windows */
 	fimd_shadow_protect_win(ctx, win, false);
 
-	win_data->enabled = false;
+	plane->enabled = false;
 }
 
 static void fimd_window_suspend(struct fimd_context *ctx)
 {
-	struct fimd_win_data *win_data;
+	struct exynos_drm_plane *plane;
 	int i;
 
 	for (i = 0; i < WINDOWS_NR; i++) {
-		win_data = &ctx->win_data[i];
-		win_data->resume = win_data->enabled;
-		if (win_data->enabled)
+		plane = &ctx->planes[i];
+		plane->resume = plane->enabled;
+		if (plane->enabled)
 			fimd_win_disable(ctx->crtc, i);
 	}
 }
 
 static void fimd_window_resume(struct fimd_context *ctx)
 {
-	struct fimd_win_data *win_data;
+	struct exynos_drm_plane *plane;
 	int i;
 
 	for (i = 0; i < WINDOWS_NR; i++) {
-		win_data = &ctx->win_data[i];
-		win_data->enabled = win_data->resume;
-		win_data->resume = false;
+		plane = &ctx->planes[i];
+		plane->enabled = plane->resume;
+		plane->resume = false;
 	}
 }
 
 static void fimd_apply(struct fimd_context *ctx)
 {
-	struct fimd_win_data *win_data;
+	struct exynos_drm_plane *plane;
 	int i;
 
 	for (i = 0; i < WINDOWS_NR; i++) {
-		win_data = &ctx->win_data[i];
-		if (win_data->enabled)
+		plane = &ctx->planes[i];
+		if (plane->enabled)
 			fimd_win_commit(ctx->crtc, i);
 		else
 			fimd_win_disable(ctx->crtc, i);
@@ -1011,7 +954,6 @@ static struct exynos_drm_crtc_ops fimd_crtc_ops = {
 	.enable_vblank = fimd_enable_vblank,
 	.disable_vblank = fimd_disable_vblank,
 	.wait_for_vblank = fimd_wait_for_vblank,
-	.win_mode_set = fimd_win_mode_set,
 	.win_commit = fimd_win_commit,
 	.win_disable = fimd_win_disable,
 	.te_handler = fimd_te_handler,
@@ -1056,7 +998,16 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
 {
 	struct fimd_context *ctx = dev_get_drvdata(dev);
 	struct drm_device *drm_dev = data;
-	int ret;
+	struct exynos_drm_plane *exynos_plane;
+	enum drm_plane_type type;
+	int zpos, ret;
+
+	for (zpos = 0; zpos < WINDOWS_NR; zpos++) {
+		type = (zpos == ctx->default_win) ? DRM_PLANE_TYPE_PRIMARY :
+						DRM_PLANE_TYPE_OVERLAY;
+		exynos_plane_init(drm_dev, &ctx->planes[zpos], 1 << ctx->pipe,
+				  type);
+	}
 
 	ret = fimd_ctx_initialize(ctx, drm_dev);
 	if (ret) {
@@ -1064,8 +1015,9 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
 		return ret;
 	}
 
-	ctx->crtc = exynos_drm_crtc_create(drm_dev, ctx->pipe,
-					   EXYNOS_DISPLAY_TYPE_LCD,
+	exynos_plane = &ctx->planes[ctx->default_win];
+	ctx->crtc = exynos_drm_crtc_create(drm_dev, &exynos_plane->base,
+					   ctx->pipe, EXYNOS_DISPLAY_TYPE_LCD,
 					   &fimd_crtc_ops, ctx);
 	if (IS_ERR(ctx->crtc)) {
 		fimd_ctx_remove(ctx);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index 2dfb847..61c3954 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -93,7 +93,6 @@ void exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc,
 			  uint32_t src_w, uint32_t src_h)
 {
 	struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
-	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
 	unsigned int actual_w;
 	unsigned int actual_h;
 
@@ -140,9 +139,6 @@ void exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc,
 			exynos_plane->crtc_width, exynos_plane->crtc_height);
 
 	plane->crtc = crtc;
-
-	if (exynos_crtc->ops->win_mode_set)
-		exynos_crtc->ops->win_mode_set(exynos_crtc, exynos_plane);
 }
 
 int
@@ -185,11 +181,8 @@ static int exynos_disable_plane(struct drm_plane *plane)
 
 static void exynos_plane_destroy(struct drm_plane *plane)
 {
-	struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
-
 	exynos_disable_plane(plane);
 	drm_plane_cleanup(plane);
-	kfree(exynos_plane);
 }
 
 static int exynos_plane_set_property(struct drm_plane *plane,
@@ -234,24 +227,18 @@ static void exynos_plane_attach_zpos_property(struct drm_plane *plane)
 	drm_object_attach_property(&plane->base, prop, 0);
 }
 
-struct drm_plane *exynos_plane_init(struct drm_device *dev,
-				    unsigned long possible_crtcs,
-				    enum drm_plane_type type)
+int exynos_plane_init(struct drm_device *dev,
+		      struct exynos_drm_plane *exynos_plane,
+		      unsigned long possible_crtcs, enum drm_plane_type type)
 {
-	struct exynos_drm_plane *exynos_plane;
 	int err;
 
-	exynos_plane = kzalloc(sizeof(struct exynos_drm_plane), GFP_KERNEL);
-	if (!exynos_plane)
-		return ERR_PTR(-ENOMEM);
-
 	err = drm_universal_plane_init(dev, &exynos_plane->base, possible_crtcs,
 				       &exynos_plane_funcs, formats,
 				       ARRAY_SIZE(formats), type);
 	if (err) {
 		DRM_ERROR("failed to initialize plane\n");
-		kfree(exynos_plane);
-		return ERR_PTR(err);
+		return err;
 	}
 
 	if (type == DRM_PLANE_TYPE_PRIMARY)
@@ -259,5 +246,5 @@ struct drm_plane *exynos_plane_init(struct drm_device *dev,
 	else
 		exynos_plane_attach_zpos_property(&exynos_plane->base);
 
-	return &exynos_plane->base;
+	return 0;
 }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.h b/drivers/gpu/drm/exynos/exynos_drm_plane.h
index 9d3c374..d8a3494 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.h
@@ -20,6 +20,6 @@ int exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 			unsigned int crtc_w, unsigned int crtc_h,
 			uint32_t src_x, uint32_t src_y,
 			uint32_t src_w, uint32_t src_h);
-struct drm_plane *exynos_plane_init(struct drm_device *dev,
-				    unsigned long possible_crtcs,
-				    enum drm_plane_type type);
+int exynos_plane_init(struct drm_device *dev,
+		      struct exynos_drm_plane *exynos_plane,
+		      unsigned long possible_crtcs, enum drm_plane_type type);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
index b886972..f33974e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -23,6 +23,7 @@
 
 #include "exynos_drm_drv.h"
 #include "exynos_drm_crtc.h"
+#include "exynos_drm_plane.h"
 #include "exynos_drm_encoder.h"
 #include "exynos_drm_vidi.h"
 
@@ -32,20 +33,6 @@
 #define ctx_from_connector(c)	container_of(c, struct vidi_context, \
 					connector)
 
-struct vidi_win_data {
-	unsigned int		offset_x;
-	unsigned int		offset_y;
-	unsigned int		ovl_width;
-	unsigned int		ovl_height;
-	unsigned int		fb_width;
-	unsigned int		fb_height;
-	unsigned int		bpp;
-	dma_addr_t		dma_addr;
-	unsigned int		buf_offsize;
-	unsigned int		line_size;	/* bytes */
-	bool			enabled;
-};
-
 struct vidi_context {
 	struct exynos_drm_display	display;
 	struct platform_device		*pdev;
@@ -53,7 +40,7 @@ struct vidi_context {
 	struct exynos_drm_crtc		*crtc;
 	struct drm_encoder		*encoder;
 	struct drm_connector		connector;
-	struct vidi_win_data		win_data[WINDOWS_NR];
+	struct exynos_drm_plane		planes[WINDOWS_NR];
 	struct edid			*raw_edid;
 	unsigned int			clkdiv;
 	unsigned int			default_win;
@@ -97,19 +84,6 @@ static const char fake_edid_info[] = {
 	0x00, 0x00, 0x00, 0x06
 };
 
-static void vidi_apply(struct vidi_context *ctx)
-{
-	struct exynos_drm_crtc_ops *crtc_ops = ctx->crtc->ops;
-	struct vidi_win_data *win_data;
-	int i;
-
-	for (i = 0; i < WINDOWS_NR; i++) {
-		win_data = &ctx->win_data[i];
-		if (win_data->enabled && (crtc_ops && crtc_ops->win_commit))
-			crtc_ops->win_commit(ctx->crtc, i);
-	}
-}
-
 static int vidi_enable_vblank(struct exynos_drm_crtc *crtc)
 {
 	struct vidi_context *ctx = crtc->ctx;
@@ -143,63 +117,10 @@ static void vidi_disable_vblank(struct exynos_drm_crtc *crtc)
 		ctx->vblank_on = false;
 }
 
-static void vidi_win_mode_set(struct exynos_drm_crtc *crtc,
-			struct exynos_drm_plane *plane)
-{
-	struct vidi_context *ctx = crtc->ctx;
-	struct vidi_win_data *win_data;
-	int win;
-	unsigned long offset;
-
-	if (!plane) {
-		DRM_ERROR("plane is NULL\n");
-		return;
-	}
-
-	win = plane->zpos;
-	if (win == DEFAULT_ZPOS)
-		win = ctx->default_win;
-
-	if (win < 0 || win >= WINDOWS_NR)
-		return;
-
-	offset = plane->fb_x * (plane->bpp >> 3);
-	offset += plane->fb_y * plane->pitch;
-
-	DRM_DEBUG_KMS("offset = 0x%lx, pitch = %x\n", offset, plane->pitch);
-
-	win_data = &ctx->win_data[win];
-
-	win_data->offset_x = plane->crtc_x;
-	win_data->offset_y = plane->crtc_y;
-	win_data->ovl_width = plane->crtc_width;
-	win_data->ovl_height = plane->crtc_height;
-	win_data->fb_width = plane->fb_width;
-	win_data->fb_height = plane->fb_height;
-	win_data->dma_addr = plane->dma_addr[0] + offset;
-	win_data->bpp = plane->bpp;
-	win_data->buf_offsize = (plane->fb_width - plane->crtc_width) *
-				(plane->bpp >> 3);
-	win_data->line_size = plane->crtc_width * (plane->bpp >> 3);
-
-	/*
-	 * some parts of win_data should be transferred to user side
-	 * through specific ioctl.
-	 */
-
-	DRM_DEBUG_KMS("offset_x = %d, offset_y = %d\n",
-			win_data->offset_x, win_data->offset_y);
-	DRM_DEBUG_KMS("ovl_width = %d, ovl_height = %d\n",
-			win_data->ovl_width, win_data->ovl_height);
-	DRM_DEBUG_KMS("paddr = 0x%lx\n", (unsigned long)win_data->dma_addr);
-	DRM_DEBUG_KMS("fb_width = %d, crtc_width = %d\n",
-			plane->fb_width, plane->crtc_width);
-}
-
 static void vidi_win_commit(struct exynos_drm_crtc *crtc, int zpos)
 {
 	struct vidi_context *ctx = crtc->ctx;
-	struct vidi_win_data *win_data;
+	struct exynos_drm_plane *plane;
 	int win = zpos;
 
 	if (ctx->suspended)
@@ -211,11 +132,11 @@ static void vidi_win_commit(struct exynos_drm_crtc *crtc, int zpos)
 	if (win < 0 || win >= WINDOWS_NR)
 		return;
 
-	win_data = &ctx->win_data[win];
+	plane = &ctx->planes[win];
 
-	win_data->enabled = true;
+	plane->enabled = true;
 
-	DRM_DEBUG_KMS("dma_addr = %pad\n", &win_data->dma_addr);
+	DRM_DEBUG_KMS("dma_addr = %pad\n", plane->dma_addr);
 
 	if (ctx->vblank_on)
 		schedule_work(&ctx->work);
@@ -224,7 +145,7 @@ static void vidi_win_commit(struct exynos_drm_crtc *crtc, int zpos)
 static void vidi_win_disable(struct exynos_drm_crtc *crtc, int zpos)
 {
 	struct vidi_context *ctx = crtc->ctx;
-	struct vidi_win_data *win_data;
+	struct exynos_drm_plane *plane;
 	int win = zpos;
 
 	if (win == DEFAULT_ZPOS)
@@ -233,14 +154,17 @@ static void vidi_win_disable(struct exynos_drm_crtc *crtc, int zpos)
 	if (win < 0 || win >= WINDOWS_NR)
 		return;
 
-	win_data = &ctx->win_data[win];
-	win_data->enabled = false;
+	plane = &ctx->planes[win];
+	plane->enabled = false;
 
 	/* TODO. */
 }
 
 static int vidi_power_on(struct vidi_context *ctx, bool enable)
 {
+	struct exynos_drm_plane *plane;
+	int i;
+
 	DRM_DEBUG_KMS("%s\n", __FILE__);
 
 	if (enable != false && enable != true)
@@ -253,7 +177,11 @@ static int vidi_power_on(struct vidi_context *ctx, bool enable)
 		if (test_and_clear_bit(0, &ctx->irq_flags))
 			vidi_enable_vblank(ctx->crtc);
 
-		vidi_apply(ctx);
+		for (i = 0; i < WINDOWS_NR; i++) {
+			plane = &ctx->planes[i];
+			if (plane->enabled)
+				vidi_win_commit(ctx->crtc, i);
+		}
 	} else {
 		ctx->suspended = true;
 	}
@@ -301,7 +229,6 @@ static struct exynos_drm_crtc_ops vidi_crtc_ops = {
 	.dpms = vidi_dpms,
 	.enable_vblank = vidi_enable_vblank,
 	.disable_vblank = vidi_disable_vblank,
-	.win_mode_set = vidi_win_mode_set,
 	.win_commit = vidi_win_commit,
 	.win_disable = vidi_win_disable,
 };
@@ -543,12 +470,22 @@ static int vidi_bind(struct device *dev, struct device *master, void *data)
 {
 	struct vidi_context *ctx = dev_get_drvdata(dev);
 	struct drm_device *drm_dev = data;
-	int ret;
+	struct exynos_drm_plane *exynos_plane;
+	enum drm_plane_type type;
+	int zpos, ret;
+
+	for (zpos = 0; zpos < WINDOWS_NR; zpos++) {
+		type = (zpos == ctx->default_win) ? DRM_PLANE_TYPE_PRIMARY :
+						DRM_PLANE_TYPE_OVERLAY;
+		exynos_plane_init(drm_dev, &ctx->planes[zpos], 1 << ctx->pipe,
+				  type);
+	}
 
 	vidi_ctx_initialize(ctx, drm_dev);
 
-	ctx->crtc = exynos_drm_crtc_create(drm_dev, ctx->pipe,
-					   EXYNOS_DISPLAY_TYPE_VIDI,
+	exynos_plane = &ctx->planes[ctx->default_win];
+	ctx->crtc = exynos_drm_crtc_create(drm_dev, &exynos_plane->base,
+					   ctx->pipe, EXYNOS_DISPLAY_TYPE_VIDI,
 					   &vidi_crtc_ops, ctx);
 	if (IS_ERR(ctx->crtc)) {
 		DRM_ERROR("failed to create crtc.\n");
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 496c861..6fa0831 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -37,34 +37,13 @@
 
 #include "exynos_drm_drv.h"
 #include "exynos_drm_crtc.h"
+#include "exynos_drm_plane.h"
 #include "exynos_drm_iommu.h"
 #include "exynos_mixer.h"
 
 #define MIXER_WIN_NR		3
 #define MIXER_DEFAULT_WIN	0
 
-struct hdmi_win_data {
-	dma_addr_t		dma_addr;
-	dma_addr_t		chroma_dma_addr;
-	uint32_t		pixel_format;
-	unsigned int		bpp;
-	unsigned int		crtc_x;
-	unsigned int		crtc_y;
-	unsigned int		crtc_width;
-	unsigned int		crtc_height;
-	unsigned int		fb_x;
-	unsigned int		fb_y;
-	unsigned int		fb_width;
-	unsigned int		fb_height;
-	unsigned int		src_width;
-	unsigned int		src_height;
-	unsigned int		mode_width;
-	unsigned int		mode_height;
-	unsigned int		scan_flags;
-	bool			enabled;
-	bool			resume;
-};
-
 struct mixer_resources {
 	int			irq;
 	void __iomem		*mixer_regs;
@@ -89,6 +68,7 @@ struct mixer_context {
 	struct device		*dev;
 	struct drm_device	*drm_dev;
 	struct exynos_drm_crtc	*crtc;
+	struct exynos_drm_plane	planes[MIXER_WIN_NR];
 	int			pipe;
 	bool			interlace;
 	bool			powered;
@@ -98,7 +78,6 @@ struct mixer_context {
 
 	struct mutex		mixer_mutex;
 	struct mixer_resources	mixer_res;
-	struct hdmi_win_data	win_data[MIXER_WIN_NR];
 	enum mixer_version_id	mxr_ver;
 	wait_queue_head_t	wait_vsync_queue;
 	atomic_t		wait_vsync_event;
@@ -402,7 +381,7 @@ static void vp_video_buffer(struct mixer_context *ctx, int win)
 {
 	struct mixer_resources *res = &ctx->mixer_res;
 	unsigned long flags;
-	struct hdmi_win_data *win_data;
+	struct exynos_drm_plane *plane;
 	unsigned int x_ratio, y_ratio;
 	unsigned int buf_num = 1;
 	dma_addr_t luma_addr[2], chroma_addr[2];
@@ -410,9 +389,9 @@ static void vp_video_buffer(struct mixer_context *ctx, int win)
 	bool crcb_mode = false;
 	u32 val;
 
-	win_data = &ctx->win_data[win];
+	plane = &ctx->planes[win];
 
-	switch (win_data->pixel_format) {
+	switch (plane->pixel_format) {
 	case DRM_FORMAT_NV12MT:
 		tiled_mode = true;
 	case DRM_FORMAT_NV12:
@@ -422,35 +401,35 @@ static void vp_video_buffer(struct mixer_context *ctx, int win)
 	/* TODO: single buffer format NV12, NV21 */
 	default:
 		/* ignore pixel format at disable time */
-		if (!win_data->dma_addr)
+		if (!plane->dma_addr[0])
 			break;
 
 		DRM_ERROR("pixel format for vp is wrong [%d].\n",
-				win_data->pixel_format);
+				plane->pixel_format);
 		return;
 	}
 
 	/* scaling feature: (src << 16) / dst */
-	x_ratio = (win_data->src_width << 16) / win_data->crtc_width;
-	y_ratio = (win_data->src_height << 16) / win_data->crtc_height;
+	x_ratio = (plane->src_width << 16) / plane->crtc_width;
+	y_ratio = (plane->src_height << 16) / plane->crtc_height;
 
 	if (buf_num == 2) {
-		luma_addr[0] = win_data->dma_addr;
-		chroma_addr[0] = win_data->chroma_dma_addr;
+		luma_addr[0] = plane->dma_addr[0];
+		chroma_addr[0] = plane->dma_addr[1];
 	} else {
-		luma_addr[0] = win_data->dma_addr;
-		chroma_addr[0] = win_data->dma_addr
-			+ (win_data->fb_width * win_data->fb_height);
+		luma_addr[0] = plane->dma_addr[0];
+		chroma_addr[0] = plane->dma_addr[0]
+			+ (plane->fb_width * plane->fb_height);
 	}
 
-	if (win_data->scan_flags & DRM_MODE_FLAG_INTERLACE) {
+	if (plane->scan_flag & DRM_MODE_FLAG_INTERLACE) {
 		ctx->interlace = true;
 		if (tiled_mode) {
 			luma_addr[1] = luma_addr[0] + 0x40;
 			chroma_addr[1] = chroma_addr[0] + 0x40;
 		} else {
-			luma_addr[1] = luma_addr[0] + win_data->fb_width;
-			chroma_addr[1] = chroma_addr[0] + win_data->fb_width;
+			luma_addr[1] = luma_addr[0] + plane->fb_width;
+			chroma_addr[1] = chroma_addr[0] + plane->fb_width;
 		}
 	} else {
 		ctx->interlace = false;
@@ -471,26 +450,26 @@ static void vp_video_buffer(struct mixer_context *ctx, int win)
 	vp_reg_writemask(res, VP_MODE, val, VP_MODE_FMT_MASK);
 
 	/* setting size of input image */
-	vp_reg_write(res, VP_IMG_SIZE_Y, VP_IMG_HSIZE(win_data->fb_width) |
-		VP_IMG_VSIZE(win_data->fb_height));
+	vp_reg_write(res, VP_IMG_SIZE_Y, VP_IMG_HSIZE(plane->fb_width) |
+		VP_IMG_VSIZE(plane->fb_height));
 	/* chroma height has to reduced by 2 to avoid chroma distorions */
-	vp_reg_write(res, VP_IMG_SIZE_C, VP_IMG_HSIZE(win_data->fb_width) |
-		VP_IMG_VSIZE(win_data->fb_height / 2));
+	vp_reg_write(res, VP_IMG_SIZE_C, VP_IMG_HSIZE(plane->fb_width) |
+		VP_IMG_VSIZE(plane->fb_height / 2));
 
-	vp_reg_write(res, VP_SRC_WIDTH, win_data->src_width);
-	vp_reg_write(res, VP_SRC_HEIGHT, win_data->src_height);
+	vp_reg_write(res, VP_SRC_WIDTH, plane->src_width);
+	vp_reg_write(res, VP_SRC_HEIGHT, plane->src_height);
 	vp_reg_write(res, VP_SRC_H_POSITION,
-			VP_SRC_H_POSITION_VAL(win_data->fb_x));
-	vp_reg_write(res, VP_SRC_V_POSITION, win_data->fb_y);
+			VP_SRC_H_POSITION_VAL(plane->fb_x));
+	vp_reg_write(res, VP_SRC_V_POSITION, plane->fb_y);
 
-	vp_reg_write(res, VP_DST_WIDTH, win_data->crtc_width);
-	vp_reg_write(res, VP_DST_H_POSITION, win_data->crtc_x);
+	vp_reg_write(res, VP_DST_WIDTH, plane->crtc_width);
+	vp_reg_write(res, VP_DST_H_POSITION, plane->crtc_x);
 	if (ctx->interlace) {
-		vp_reg_write(res, VP_DST_HEIGHT, win_data->crtc_height / 2);
-		vp_reg_write(res, VP_DST_V_POSITION, win_data->crtc_y / 2);
+		vp_reg_write(res, VP_DST_HEIGHT, plane->crtc_height / 2);
+		vp_reg_write(res, VP_DST_V_POSITION, plane->crtc_y / 2);
 	} else {
-		vp_reg_write(res, VP_DST_HEIGHT, win_data->crtc_height);
-		vp_reg_write(res, VP_DST_V_POSITION, win_data->crtc_y);
+		vp_reg_write(res, VP_DST_HEIGHT, plane->crtc_height);
+		vp_reg_write(res, VP_DST_V_POSITION, plane->crtc_y);
 	}
 
 	vp_reg_write(res, VP_H_RATIO, x_ratio);
@@ -504,8 +483,8 @@ static void vp_video_buffer(struct mixer_context *ctx, int win)
 	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, win_data->mode_height);
-	mixer_cfg_rgb_fmt(ctx, win_data->mode_height);
+	mixer_cfg_scan(ctx, plane->mode_height);
+	mixer_cfg_rgb_fmt(ctx, plane->mode_height);
 	mixer_cfg_layer(ctx, win, true);
 	mixer_run(ctx);
 
@@ -526,21 +505,21 @@ static void mixer_graph_buffer(struct mixer_context *ctx, int win)
 {
 	struct mixer_resources *res = &ctx->mixer_res;
 	unsigned long flags;
-	struct hdmi_win_data *win_data;
+	struct exynos_drm_plane *plane;
 	unsigned int x_ratio, y_ratio;
 	unsigned int src_x_offset, src_y_offset, dst_x_offset, dst_y_offset;
 	dma_addr_t dma_addr;
 	unsigned int fmt;
 	u32 val;
 
-	win_data = &ctx->win_data[win];
+	plane = &ctx->planes[win];
 
 	#define RGB565 4
 	#define ARGB1555 5
 	#define ARGB4444 6
 	#define ARGB8888 7
 
-	switch (win_data->bpp) {
+	switch (plane->bpp) {
 	case 16:
 		fmt = ARGB4444;
 		break;
@@ -555,17 +534,17 @@ static void mixer_graph_buffer(struct mixer_context *ctx, int win)
 	x_ratio = 0;
 	y_ratio = 0;
 
-	dst_x_offset = win_data->crtc_x;
-	dst_y_offset = win_data->crtc_y;
+	dst_x_offset = plane->crtc_x;
+	dst_y_offset = plane->crtc_y;
 
 	/* converting dma address base and source offset */
-	dma_addr = win_data->dma_addr
-		+ (win_data->fb_x * win_data->bpp >> 3)
-		+ (win_data->fb_y * win_data->fb_width * win_data->bpp >> 3);
+	dma_addr = plane->dma_addr[0]
+		+ (plane->fb_x * plane->bpp >> 3)
+		+ (plane->fb_y * plane->fb_width * plane->bpp >> 3);
 	src_x_offset = 0;
 	src_y_offset = 0;
 
-	if (win_data->scan_flags & DRM_MODE_FLAG_INTERLACE)
+	if (plane->scan_flag & DRM_MODE_FLAG_INTERLACE)
 		ctx->interlace = true;
 	else
 		ctx->interlace = false;
@@ -578,18 +557,18 @@ static void mixer_graph_buffer(struct mixer_context *ctx, int win)
 		MXR_GRP_CFG_FORMAT_VAL(fmt), MXR_GRP_CFG_FORMAT_MASK);
 
 	/* setup geometry */
-	mixer_reg_write(res, MXR_GRAPHIC_SPAN(win), win_data->fb_width);
+	mixer_reg_write(res, MXR_GRAPHIC_SPAN(win), plane->fb_width);
 
 	/* setup display size */
 	if (ctx->mxr_ver == MXR_VER_128_0_0_184 &&
 		win == MIXER_DEFAULT_WIN) {
-		val  = MXR_MXR_RES_HEIGHT(win_data->mode_height);
-		val |= MXR_MXR_RES_WIDTH(win_data->mode_width);
+		val  = MXR_MXR_RES_HEIGHT(plane->mode_height);
+		val |= MXR_MXR_RES_WIDTH(plane->mode_width);
 		mixer_reg_write(res, MXR_RESOLUTION, val);
 	}
 
-	val  = MXR_GRP_WH_WIDTH(win_data->crtc_width);
-	val |= MXR_GRP_WH_HEIGHT(win_data->crtc_height);
+	val  = MXR_GRP_WH_WIDTH(plane->crtc_width);
+	val |= MXR_GRP_WH_HEIGHT(plane->crtc_height);
 	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);
@@ -607,8 +586,8 @@ static void mixer_graph_buffer(struct mixer_context *ctx, int win)
 	/* set buffer address to mixer */
 	mixer_reg_write(res, MXR_GRAPHIC_BASE(win), dma_addr);
 
-	mixer_cfg_scan(ctx, win_data->mode_height);
-	mixer_cfg_rgb_fmt(ctx, win_data->mode_height);
+	mixer_cfg_scan(ctx, plane->mode_height);
+	mixer_cfg_rgb_fmt(ctx, plane->mode_height);
 	mixer_cfg_layer(ctx, win, true);
 
 	/* layer update mandatory for mixer 16.0.33.0 */
@@ -920,58 +899,6 @@ static void mixer_disable_vblank(struct exynos_drm_crtc *crtc)
 	mixer_reg_writemask(res, MXR_INT_EN, 0, MXR_INT_EN_VSYNC);
 }
 
-static void mixer_win_mode_set(struct exynos_drm_crtc *crtc,
-			struct exynos_drm_plane *plane)
-{
-	struct mixer_context *mixer_ctx = crtc->ctx;
-	struct hdmi_win_data *win_data;
-	int win;
-
-	if (!plane) {
-		DRM_ERROR("plane is NULL\n");
-		return;
-	}
-
-	DRM_DEBUG_KMS("set [%d]x[%d] at (%d,%d) to [%d]x[%d] at (%d,%d)\n",
-				 plane->fb_width, plane->fb_height,
-				 plane->fb_x, plane->fb_y,
-				 plane->crtc_width, plane->crtc_height,
-				 plane->crtc_x, plane->crtc_y);
-
-	win = plane->zpos;
-	if (win == DEFAULT_ZPOS)
-		win = MIXER_DEFAULT_WIN;
-
-	if (win < 0 || win >= MIXER_WIN_NR) {
-		DRM_ERROR("mixer window[%d] is wrong\n", win);
-		return;
-	}
-
-	win_data = &mixer_ctx->win_data[win];
-
-	win_data->dma_addr = plane->dma_addr[0];
-	win_data->chroma_dma_addr = plane->dma_addr[1];
-	win_data->pixel_format = plane->pixel_format;
-	win_data->bpp = plane->bpp;
-
-	win_data->crtc_x = plane->crtc_x;
-	win_data->crtc_y = plane->crtc_y;
-	win_data->crtc_width = plane->crtc_width;
-	win_data->crtc_height = plane->crtc_height;
-
-	win_data->fb_x = plane->fb_x;
-	win_data->fb_y = plane->fb_y;
-	win_data->fb_width = plane->fb_width;
-	win_data->fb_height = plane->fb_height;
-	win_data->src_width = plane->src_width;
-	win_data->src_height = plane->src_height;
-
-	win_data->mode_width = plane->mode_width;
-	win_data->mode_height = plane->mode_height;
-
-	win_data->scan_flags = plane->scan_flag;
-}
-
 static void mixer_win_commit(struct exynos_drm_crtc *crtc, int zpos)
 {
 	struct mixer_context *mixer_ctx = crtc->ctx;
@@ -991,7 +918,7 @@ static void mixer_win_commit(struct exynos_drm_crtc *crtc, int zpos)
 	else
 		mixer_graph_buffer(mixer_ctx, win);
 
-	mixer_ctx->win_data[win].enabled = true;
+	mixer_ctx->planes[win].enabled = true;
 }
 
 static void mixer_win_disable(struct exynos_drm_crtc *crtc, int zpos)
@@ -1006,7 +933,7 @@ static void mixer_win_disable(struct exynos_drm_crtc *crtc, int zpos)
 	mutex_lock(&mixer_ctx->mixer_mutex);
 	if (!mixer_ctx->powered) {
 		mutex_unlock(&mixer_ctx->mixer_mutex);
-		mixer_ctx->win_data[win].resume = false;
+		mixer_ctx->planes[win].resume = false;
 		return;
 	}
 	mutex_unlock(&mixer_ctx->mixer_mutex);
@@ -1019,7 +946,7 @@ static void mixer_win_disable(struct exynos_drm_crtc *crtc, int zpos)
 	mixer_vsync_set_update(mixer_ctx, true);
 	spin_unlock_irqrestore(&res->reg_slock, flags);
 
-	mixer_ctx->win_data[win].enabled = false;
+	mixer_ctx->planes[win].enabled = false;
 }
 
 static void mixer_wait_for_vblank(struct exynos_drm_crtc *crtc)
@@ -1056,12 +983,12 @@ static void mixer_wait_for_vblank(struct exynos_drm_crtc *crtc)
 
 static void mixer_window_suspend(struct mixer_context *ctx)
 {
-	struct hdmi_win_data *win_data;
+	struct exynos_drm_plane *plane;
 	int i;
 
 	for (i = 0; i < MIXER_WIN_NR; i++) {
-		win_data = &ctx->win_data[i];
-		win_data->resume = win_data->enabled;
+		plane = &ctx->planes[i];
+		plane->resume = plane->enabled;
 		mixer_win_disable(ctx->crtc, i);
 	}
 	mixer_wait_for_vblank(ctx->crtc);
@@ -1069,14 +996,14 @@ static void mixer_window_suspend(struct mixer_context *ctx)
 
 static void mixer_window_resume(struct mixer_context *ctx)
 {
-	struct hdmi_win_data *win_data;
+	struct exynos_drm_plane *plane;
 	int i;
 
 	for (i = 0; i < MIXER_WIN_NR; i++) {
-		win_data = &ctx->win_data[i];
-		win_data->enabled = win_data->resume;
-		win_data->resume = false;
-		if (win_data->enabled)
+		plane = &ctx->planes[i];
+		plane->enabled = plane->resume;
+		plane->resume = false;
+		if (plane->enabled)
 			mixer_win_commit(ctx->crtc, i);
 	}
 }
@@ -1188,7 +1115,6 @@ static struct exynos_drm_crtc_ops mixer_crtc_ops = {
 	.enable_vblank		= mixer_enable_vblank,
 	.disable_vblank		= mixer_disable_vblank,
 	.wait_for_vblank	= mixer_wait_for_vblank,
-	.win_mode_set		= mixer_win_mode_set,
 	.win_commit		= mixer_win_commit,
 	.win_disable		= mixer_win_disable,
 };
@@ -1252,15 +1178,25 @@ static int mixer_bind(struct device *dev, struct device *manager, void *data)
 {
 	struct mixer_context *ctx = dev_get_drvdata(dev);
 	struct drm_device *drm_dev = data;
-	int ret;
+	struct exynos_drm_plane *exynos_plane;
+	enum drm_plane_type type;
+	int zpos, ret;
+
+	for (zpos = 0; zpos < MIXER_WIN_NR; zpos++) {
+		type = (zpos == MIXER_DEFAULT_WIN) ? DRM_PLANE_TYPE_PRIMARY :
+						DRM_PLANE_TYPE_OVERLAY;
+		exynos_plane_init(drm_dev, &ctx->planes[zpos], 1 << ctx->pipe,
+				  type);
+	}
 
 	ret = mixer_initialize(ctx, drm_dev);
 	if (ret)
 		return ret;
 
-	ctx->crtc = exynos_drm_crtc_create(drm_dev, ctx->pipe,
-				     EXYNOS_DISPLAY_TYPE_HDMI,
-				     &mixer_crtc_ops, ctx);
+	exynos_plane = &ctx->planes[MIXER_DEFAULT_WIN];
+	ctx->crtc = exynos_drm_crtc_create(drm_dev, &exynos_plane->base,
+					   ctx->pipe, EXYNOS_DISPLAY_TYPE_HDMI,
+					   &mixer_crtc_ops, ctx);
 	if (IS_ERR(ctx->crtc)) {
 		mixer_ctx_remove(ctx);
 		ret = PTR_ERR(ctx->crtc);
-- 
1.9.3

[PATCH -v2 03/14] drm/exynos: preset zpos value for overlay planes

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Usually userspace don't want to have two overlay planes on the same zpos
so this change assign a different zpos for each plane. Before this change
a zpos of value zero was created for all planes so the userspace had to
set up the zpos of every plane it wanted to use.

Also all places that were storing zpos positions are now unsigned int.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.h   |  7 +++----
 drivers/gpu/drm/exynos/exynos_drm_fimd.c  | 24 +++++++++++-------------
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 16 +++++++++-------
 drivers/gpu/drm/exynos/exynos_drm_plane.h |  3 ++-
 drivers/gpu/drm/exynos/exynos_drm_vidi.c  | 17 +++++------------
 drivers/gpu/drm/exynos/exynos_mixer.c     | 11 +++++------
 6 files changed, 35 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 7ea7648..a3a2d63 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -21,7 +21,6 @@
 #define MAX_CRTC	3
 #define MAX_PLANE	5
 #define MAX_FB_BUFFER	4
-#define DEFAULT_ZPOS	-1
 
 #define to_exynos_crtc(x)	container_of(x, struct exynos_drm_crtc, base)
 #define to_exynos_plane(x)	container_of(x, struct exynos_drm_plane, base)
@@ -104,7 +103,7 @@ struct exynos_drm_plane {
 	unsigned int pitch;
 	uint32_t pixel_format;
 	dma_addr_t dma_addr[MAX_FB_BUFFER];
-	int zpos;
+	unsigned int zpos;
 	unsigned int index_color;
 
 	bool default_win:1;
@@ -189,8 +188,8 @@ struct exynos_drm_crtc_ops {
 	int (*enable_vblank)(struct exynos_drm_crtc *crtc);
 	void (*disable_vblank)(struct exynos_drm_crtc *crtc);
 	void (*wait_for_vblank)(struct exynos_drm_crtc *crtc);
-	void (*win_commit)(struct exynos_drm_crtc *crtc, int zpos);
-	void (*win_disable)(struct exynos_drm_crtc *crtc, int zpos);
+	void (*win_commit)(struct exynos_drm_crtc *crtc, unsigned int zpos);
+	void (*win_disable)(struct exynos_drm_crtc *crtc, unsigned int zpos);
 	void (*te_handler)(struct exynos_drm_crtc *crtc);
 };
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 489ce90..990ead434 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -195,7 +195,12 @@ static inline struct fimd_driver_data *drm_fimd_get_driver_data(
 
 static void fimd_wait_for_vblank(struct exynos_drm_crtc *crtc)
 {
-	struct fimd_context *ctx = crtc->ctx;
+	struct fimd_context *ctx;
+
+	if (!crtc)
+		return;
+
+	ctx = crtc->ctx;
 
 	if (ctx->suspended)
 		return;
@@ -601,11 +606,10 @@ static void fimd_shadow_protect_win(struct fimd_context *ctx,
 	writel(val, ctx->regs + reg);
 }
 
-static void fimd_win_commit(struct exynos_drm_crtc *crtc, int zpos)
+static void fimd_win_commit(struct exynos_drm_crtc *crtc, unsigned int win)
 {
 	struct fimd_context *ctx = crtc->ctx;
 	struct exynos_drm_plane *plane;
-	int win = zpos;
 	dma_addr_t dma_addr;
 	unsigned long val, alpha, size, offset;
 	unsigned int last_x, last_y, buf_offsize, line_size;
@@ -613,9 +617,6 @@ static void fimd_win_commit(struct exynos_drm_crtc *crtc, int zpos)
 	if (ctx->suspended)
 		return;
 
-	if (win == DEFAULT_ZPOS)
-		win = ctx->default_win;
-
 	if (win < 0 || win >= WINDOWS_NR)
 		return;
 
@@ -731,14 +732,10 @@ static void fimd_win_commit(struct exynos_drm_crtc *crtc, int zpos)
 		atomic_set(&ctx->win_updated, 1);
 }
 
-static void fimd_win_disable(struct exynos_drm_crtc *crtc, int zpos)
+static void fimd_win_disable(struct exynos_drm_crtc *crtc, unsigned int win)
 {
 	struct fimd_context *ctx = crtc->ctx;
 	struct exynos_drm_plane *plane;
-	int win = zpos;
-
-	if (win == DEFAULT_ZPOS)
-		win = ctx->default_win;
 
 	if (win < 0 || win >= WINDOWS_NR)
 		return;
@@ -1000,13 +997,14 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
 	struct drm_device *drm_dev = data;
 	struct exynos_drm_plane *exynos_plane;
 	enum drm_plane_type type;
-	int zpos, ret;
+	unsigned int zpos;
+	int ret;
 
 	for (zpos = 0; zpos < WINDOWS_NR; zpos++) {
 		type = (zpos == ctx->default_win) ? DRM_PLANE_TYPE_PRIMARY :
 						DRM_PLANE_TYPE_OVERLAY;
 		exynos_plane_init(drm_dev, &ctx->planes[zpos], 1 << ctx->pipe,
-				  type);
+				  type, zpos);
 	}
 
 	ret = fimd_ctx_initialize(ctx, drm_dev);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index 61c3954..8292819 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -208,7 +208,8 @@ static struct drm_plane_funcs exynos_plane_funcs = {
 	.set_property	= exynos_plane_set_property,
 };
 
-static void exynos_plane_attach_zpos_property(struct drm_plane *plane)
+static void exynos_plane_attach_zpos_property(struct drm_plane *plane,
+					      unsigned int zpos)
 {
 	struct drm_device *dev = plane->dev;
 	struct exynos_drm_private *dev_priv = dev->dev_private;
@@ -224,12 +225,13 @@ static void exynos_plane_attach_zpos_property(struct drm_plane *plane)
 		dev_priv->plane_zpos_property = prop;
 	}
 
-	drm_object_attach_property(&plane->base, prop, 0);
+	drm_object_attach_property(&plane->base, prop, zpos);
 }
 
 int exynos_plane_init(struct drm_device *dev,
 		      struct exynos_drm_plane *exynos_plane,
-		      unsigned long possible_crtcs, enum drm_plane_type type)
+		      unsigned long possible_crtcs, enum drm_plane_type type,
+		      unsigned int zpos)
 {
 	int err;
 
@@ -241,10 +243,10 @@ int exynos_plane_init(struct drm_device *dev,
 		return err;
 	}
 
-	if (type == DRM_PLANE_TYPE_PRIMARY)
-		exynos_plane->zpos = DEFAULT_ZPOS;
-	else
-		exynos_plane_attach_zpos_property(&exynos_plane->base);
+	exynos_plane->zpos = zpos;
+
+	if (type == DRM_PLANE_TYPE_OVERLAY)
+		exynos_plane_attach_zpos_property(&exynos_plane->base, zpos);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.h b/drivers/gpu/drm/exynos/exynos_drm_plane.h
index d8a3494..f360590 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.h
@@ -22,4 +22,5 @@ int exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 			uint32_t src_w, uint32_t src_h);
 int exynos_plane_init(struct drm_device *dev,
 		      struct exynos_drm_plane *exynos_plane,
-		      unsigned long possible_crtcs, enum drm_plane_type type);
+		      unsigned long possible_crtcs, enum drm_plane_type type,
+		      unsigned int zpos);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
index f33974e..0c1bdd9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -117,18 +117,14 @@ static void vidi_disable_vblank(struct exynos_drm_crtc *crtc)
 		ctx->vblank_on = false;
 }
 
-static void vidi_win_commit(struct exynos_drm_crtc *crtc, int zpos)
+static void vidi_win_commit(struct exynos_drm_crtc *crtc, unsigned int win)
 {
 	struct vidi_context *ctx = crtc->ctx;
 	struct exynos_drm_plane *plane;
-	int win = zpos;
 
 	if (ctx->suspended)
 		return;
 
-	if (win == DEFAULT_ZPOS)
-		win = ctx->default_win;
-
 	if (win < 0 || win >= WINDOWS_NR)
 		return;
 
@@ -142,14 +138,10 @@ static void vidi_win_commit(struct exynos_drm_crtc *crtc, int zpos)
 		schedule_work(&ctx->work);
 }
 
-static void vidi_win_disable(struct exynos_drm_crtc *crtc, int zpos)
+static void vidi_win_disable(struct exynos_drm_crtc *crtc, unsigned int win)
 {
 	struct vidi_context *ctx = crtc->ctx;
 	struct exynos_drm_plane *plane;
-	int win = zpos;
-
-	if (win == DEFAULT_ZPOS)
-		win = ctx->default_win;
 
 	if (win < 0 || win >= WINDOWS_NR)
 		return;
@@ -472,13 +464,14 @@ static int vidi_bind(struct device *dev, struct device *master, void *data)
 	struct drm_device *drm_dev = data;
 	struct exynos_drm_plane *exynos_plane;
 	enum drm_plane_type type;
-	int zpos, ret;
+	unsigned int zpos;
+	int ret;
 
 	for (zpos = 0; zpos < WINDOWS_NR; zpos++) {
 		type = (zpos == ctx->default_win) ? DRM_PLANE_TYPE_PRIMARY :
 						DRM_PLANE_TYPE_OVERLAY;
 		exynos_plane_init(drm_dev, &ctx->planes[zpos], 1 << ctx->pipe,
-				  type);
+				  type, zpos);
 	}
 
 	vidi_ctx_initialize(ctx, drm_dev);
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 6fa0831..ddaaaa0 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -899,10 +899,9 @@ static void mixer_disable_vblank(struct exynos_drm_crtc *crtc)
 	mixer_reg_writemask(res, MXR_INT_EN, 0, MXR_INT_EN_VSYNC);
 }
 
-static void mixer_win_commit(struct exynos_drm_crtc *crtc, int zpos)
+static void mixer_win_commit(struct exynos_drm_crtc *crtc, unsigned int win)
 {
 	struct mixer_context *mixer_ctx = crtc->ctx;
-	int win = zpos == DEFAULT_ZPOS ? MIXER_DEFAULT_WIN : zpos;
 
 	DRM_DEBUG_KMS("win: %d\n", win);
 
@@ -921,11 +920,10 @@ static void mixer_win_commit(struct exynos_drm_crtc *crtc, int zpos)
 	mixer_ctx->planes[win].enabled = true;
 }
 
-static void mixer_win_disable(struct exynos_drm_crtc *crtc, int zpos)
+static void mixer_win_disable(struct exynos_drm_crtc *crtc, unsigned int win)
 {
 	struct mixer_context *mixer_ctx = crtc->ctx;
 	struct mixer_resources *res = &mixer_ctx->mixer_res;
-	int win = zpos == DEFAULT_ZPOS ? MIXER_DEFAULT_WIN : zpos;
 	unsigned long flags;
 
 	DRM_DEBUG_KMS("win: %d\n", win);
@@ -1180,13 +1178,14 @@ static int mixer_bind(struct device *dev, struct device *manager, void *data)
 	struct drm_device *drm_dev = data;
 	struct exynos_drm_plane *exynos_plane;
 	enum drm_plane_type type;
-	int zpos, ret;
+	unsigned int zpos;
+	int ret;
 
 	for (zpos = 0; zpos < MIXER_WIN_NR; zpos++) {
 		type = (zpos == MIXER_DEFAULT_WIN) ? DRM_PLANE_TYPE_PRIMARY :
 						DRM_PLANE_TYPE_OVERLAY;
 		exynos_plane_init(drm_dev, &ctx->planes[zpos], 1 << ctx->pipe,
-				  type);
+				  type, zpos);
 	}
 
 	ret = mixer_initialize(ctx, drm_dev);
-- 
1.9.3

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

[PATCH -v2 04/14] drm/exynos: make zpos property immutable

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

We already set each plane zpos at init, after that changes to zpos are
not expected. This patch turns zpos into a read-only property so now it is
impossible to set zpos.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index 8292819..3847545 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -185,27 +185,10 @@ static void exynos_plane_destroy(struct drm_plane *plane)
 	drm_plane_cleanup(plane);
 }
 
-static int exynos_plane_set_property(struct drm_plane *plane,
-				     struct drm_property *property,
-				     uint64_t val)
-{
-	struct drm_device *dev = plane->dev;
-	struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
-	struct exynos_drm_private *dev_priv = dev->dev_private;
-
-	if (property == dev_priv->plane_zpos_property) {
-		exynos_plane->zpos = val;
-		return 0;
-	}
-
-	return -EINVAL;
-}
-
 static struct drm_plane_funcs exynos_plane_funcs = {
 	.update_plane	= exynos_update_plane,
 	.disable_plane	= exynos_disable_plane,
 	.destroy	= exynos_plane_destroy,
-	.set_property	= exynos_plane_set_property,
 };
 
 static void exynos_plane_attach_zpos_property(struct drm_plane *plane,
@@ -217,8 +200,8 @@ static void exynos_plane_attach_zpos_property(struct drm_plane *plane,
 
 	prop = dev_priv->plane_zpos_property;
 	if (!prop) {
-		prop = drm_property_create_range(dev, 0, "zpos", 0,
-						 MAX_PLANE - 1);
+		prop = drm_property_create_range(dev, DRM_MODE_PROP_IMMUTABLE,
+						 "zpos", 0, MAX_PLANE - 1);
 		if (!prop)
 			return;
 
-- 
1.9.3

[PATCH -v2 05/14] drm/exynos: remove exynos_plane_destroy()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

The .destroy() callback for exynos can be replaced by drm_plane_cleanup().
The only extra operation on exynos_plane_destroy() was a call to
exynos_plane_disable() but the plane is already disabled by a earlier call
to drm_framebuffer_remove().

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index 3847545..4367379 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -179,16 +179,10 @@ static int exynos_disable_plane(struct drm_plane *plane)
 	return 0;
 }
 
-static void exynos_plane_destroy(struct drm_plane *plane)
-{
-	exynos_disable_plane(plane);
-	drm_plane_cleanup(plane);
-}
-
 static struct drm_plane_funcs exynos_plane_funcs = {
 	.update_plane	= exynos_update_plane,
 	.disable_plane	= exynos_disable_plane,
-	.destroy	= exynos_plane_destroy,
+	.destroy	= drm_plane_cleanup,
 };
 
 static void exynos_plane_attach_zpos_property(struct drm_plane *plane,
-- 
1.9.3

[PATCH -v2 06/14] drm/exynos: remove leftover functions declarations

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

These functions were already removed by previous cleanup work, but these
ones were left behind.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Acked-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.h b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
index e1fd2ef..0ecd8fc 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
@@ -28,12 +28,6 @@ void exynos_drm_crtc_disable_vblank(struct drm_device *dev, int pipe);
 void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int pipe);
 void exynos_drm_crtc_complete_scanout(struct drm_framebuffer *fb);
 
-void exynos_drm_crtc_plane_mode_set(struct drm_crtc *crtc,
-			struct exynos_drm_plane *plane);
-void exynos_drm_crtc_plane_commit(struct drm_crtc *crtc, int zpos);
-void exynos_drm_crtc_plane_enable(struct drm_crtc *crtc, int zpos);
-void exynos_drm_crtc_plane_disable(struct drm_crtc *crtc, int zpos);
-
 /* This function gets pipe value to crtc device matched with out_type. */
 int exynos_drm_crtc_get_pipe_from_type(struct drm_device *drm_dev,
 					unsigned int out_type);
-- 
1.9.3

[PATCH -v2 07/14] drm/exynos: track vblank events on a per crtc basis

[Gustavo Padovan]

From: Mandeep Singh Baines <msb@chromium.org>

The goal of the change is to make sure we send the vblank event on the
current vblank. My hope is to fix any races that might be causing flicker.
After this change I only see a flicker in the transition plymouth and
X11.

Simplified the code by tracking vblank events on a per-crtc basis. This
allowed me to remove all error paths from the callback. It also allowed
me to remove the vblank wait from the callback.

Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c | 92 +++++++++++++++-----------------
 drivers/gpu/drm/exynos/exynos_drm_drv.c  | 13 -----
 drivers/gpu/drm/exynos/exynos_drm_drv.h  |  6 +--
 3 files changed, 44 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 47dd2b0..eb49195 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -34,9 +34,8 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode)
 	if (mode > DRM_MODE_DPMS_ON) {
 		/* wait for the completion of page flip. */
 		if (!wait_event_timeout(exynos_crtc->pending_flip_queue,
-				!atomic_read(&exynos_crtc->pending_flip),
-				HZ/20))
-			atomic_set(&exynos_crtc->pending_flip, 0);
+				(exynos_crtc->event == NULL), HZ/20))
+			exynos_crtc->event = NULL;
 		drm_crtc_vblank_off(crtc);
 	}
 
@@ -164,11 +163,10 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
 				     uint32_t page_flip_flags)
 {
 	struct drm_device *dev = crtc->dev;
-	struct exynos_drm_private *dev_priv = dev->dev_private;
 	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
 	struct drm_framebuffer *old_fb = crtc->primary->fb;
 	unsigned int crtc_w, crtc_h;
-	int ret = -EINVAL;
+	int ret;
 
 	/* when the page flip is requested, crtc's dpms should be on */
 	if (exynos_crtc->dpms > DRM_MODE_DPMS_ON) {
@@ -176,48 +174,49 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
 		return -EINVAL;
 	}
 
-	mutex_lock(&dev->struct_mutex);
+	if (!event)
+		return -EINVAL;
 
-	if (event) {
-		/*
-		 * the pipe from user always is 0 so we can set pipe number
-		 * of current owner to event.
-		 */
-		event->pipe = exynos_crtc->pipe;
+	spin_lock_irq(&dev->event_lock);
+	if (exynos_crtc->event) {
+		ret = -EBUSY;
+		goto out;
+	}
 
-		ret = drm_vblank_get(dev, exynos_crtc->pipe);
-		if (ret) {
-			DRM_DEBUG("failed to acquire vblank counter\n");
+	ret = drm_vblank_get(dev, exynos_crtc->pipe);
+	if (ret) {
+		DRM_DEBUG("failed to acquire vblank counter\n");
+		goto out;
+	}
 
-			goto out;
-		}
+	exynos_crtc->event = event;
+	spin_unlock_irq(&dev->event_lock);
 
+	/*
+	 * the pipe from user always is 0 so we can set pipe number
+	 * of current owner to event.
+	 */
+	event->pipe = exynos_crtc->pipe;
+
+	crtc->primary->fb = fb;
+	crtc_w = fb->width - crtc->x;
+	crtc_h = fb->height - crtc->y;
+	ret = exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
+				  crtc_w, crtc_h, crtc->x, crtc->y,
+				  crtc_w, crtc_h);
+	if (ret) {
+		crtc->primary->fb = old_fb;
 		spin_lock_irq(&dev->event_lock);
-		list_add_tail(&event->base.link,
-				&dev_priv->pageflip_event_list);
-		atomic_set(&exynos_crtc->pending_flip, 1);
+		exynos_crtc->event = NULL;
+		drm_vblank_put(dev, exynos_crtc->pipe);
 		spin_unlock_irq(&dev->event_lock);
-
-		crtc->primary->fb = fb;
-		crtc_w = fb->width - crtc->x;
-		crtc_h = fb->height - crtc->y;
-		ret = exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
-					  crtc_w, crtc_h, crtc->x, crtc->y,
-					  crtc_w, crtc_h);
-		if (ret) {
-			crtc->primary->fb = old_fb;
-
-			spin_lock_irq(&dev->event_lock);
-			drm_vblank_put(dev, exynos_crtc->pipe);
-			list_del(&event->base.link);
-			atomic_set(&exynos_crtc->pending_flip, 0);
-			spin_unlock_irq(&dev->event_lock);
-
-			goto out;
-		}
+		return ret;
 	}
+
+	return 0;
+
 out:
-	mutex_unlock(&dev->struct_mutex);
+	spin_unlock_irq(&dev->event_lock);
 	return ret;
 }
 
@@ -255,7 +254,6 @@ struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
 		return ERR_PTR(-ENOMEM);
 
 	init_waitqueue_head(&exynos_crtc->pending_flip_queue);
-	atomic_set(&exynos_crtc->pending_flip, 0);
 
 	exynos_crtc->dpms = DRM_MODE_DPMS_OFF;
 	exynos_crtc->pipe = pipe;
@@ -313,26 +311,20 @@ void exynos_drm_crtc_disable_vblank(struct drm_device *dev, int pipe)
 void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int pipe)
 {
 	struct exynos_drm_private *dev_priv = dev->dev_private;
-	struct drm_pending_vblank_event *e, *t;
 	struct drm_crtc *drm_crtc = dev_priv->crtc[pipe];
 	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(drm_crtc);
 	unsigned long flags;
 
 	spin_lock_irqsave(&dev->event_lock, flags);
+	if (exynos_crtc->event) {
 
-	list_for_each_entry_safe(e, t, &dev_priv->pageflip_event_list,
-			base.link) {
-		/* if event's pipe isn't same as crtc then ignore it. */
-		if (pipe != e->pipe)
-			continue;
-
-		list_del(&e->base.link);
-		drm_send_vblank_event(dev, -1, e);
+		drm_send_vblank_event(dev, -1, exynos_crtc->event);
 		drm_vblank_put(dev, pipe);
-		atomic_set(&exynos_crtc->pending_flip, 0);
 		wake_up(&exynos_crtc->pending_flip_queue);
+
 	}
 
+	exynos_crtc->event = NULL;
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 }
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index c598197..778c91e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -60,7 +60,6 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
 	if (!private)
 		return -ENOMEM;
 
-	INIT_LIST_HEAD(&private->pageflip_event_list);
 	dev_set_drvdata(dev->dev, dev);
 	dev->dev_private = (void *)private;
 
@@ -223,25 +222,13 @@ static void exynos_drm_preclose(struct drm_device *dev,
 
 static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file)
 {
-	struct exynos_drm_private *private = dev->dev_private;
-	struct drm_pending_vblank_event *v, *vt;
 	struct drm_pending_event *e, *et;
 	unsigned long flags;
 
 	if (!file->driver_priv)
 		return;
 
-	/* Release all events not unhandled by page flip handler. */
 	spin_lock_irqsave(&dev->event_lock, flags);
-	list_for_each_entry_safe(v, vt, &private->pageflip_event_list,
-			base.link) {
-		if (v->base.file_priv == file) {
-			list_del(&v->base.link);
-			drm_vblank_put(dev, v->pipe);
-			v->base.destroy(&v->base);
-		}
-	}
-
 	/* Release all events handled by page flip handler but not freed. */
 	list_for_each_entry_safe(e, et, &file->event_list, link) {
 		list_del(&e->link);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index a3a2d63..ab82772 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -206,6 +206,7 @@ struct exynos_drm_crtc_ops {
  *	we can refer to the crtc to current hardware interrupt occurred through
  *	this pipe value.
  * @dpms: store the crtc dpms value
+ * @event: vblank event that is currently queued for flip
  * @ops: pointer to callbacks for exynos drm specific functionality
  * @ctx: A pointer to the crtc's implementation specific context
  */
@@ -215,7 +216,7 @@ struct exynos_drm_crtc {
 	unsigned int			pipe;
 	unsigned int			dpms;
 	wait_queue_head_t		pending_flip_queue;
-	atomic_t			pending_flip;
+	struct drm_pending_vblank_event	*event;
 	struct exynos_drm_crtc_ops	*ops;
 	void				*ctx;
 };
@@ -245,9 +246,6 @@ struct drm_exynos_file_private {
 struct exynos_drm_private {
 	struct drm_fb_helper *fb_helper;
 
-	/* list head for new event to be added. */
-	struct list_head pageflip_event_list;
-
 	/*
 	 * created crtc object would be contained at this array and
 	 * this array is used to be aware of which crtc did it request vblank.
-- 
1.9.3

[PATCH -v2 08/14] drm/exynos: atomic phase 1: use drm_plane_helper_update()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Rip out the check from exynos_update_plane() and create
exynos_check_plane() for the check phase enabling use to use
the atomic helpers to call our check and update phases when updating
planes.

Update all users of exynos_update_plane() accordingly to call
exynos_check_plane() before.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c  | 29 ++++++++++++------------
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 37 ++++++++++++++++++++++---------
 drivers/gpu/drm/exynos/exynos_drm_plane.h |  2 +-
 3 files changed, 43 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index eb49195..1c0d936 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -116,6 +116,7 @@ static int exynos_drm_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 	struct drm_framebuffer *fb = crtc->primary->fb;
 	unsigned int crtc_w;
 	unsigned int crtc_h;
+	int ret;
 
 	/* when framebuffer changing is requested, crtc's dpms should be on */
 	if (exynos_crtc->dpms > DRM_MODE_DPMS_ON) {
@@ -123,11 +124,16 @@ static int exynos_drm_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 		return -EPERM;
 	}
 
+	ret = exynos_check_plane(crtc->primary, fb);
+	if (ret)
+		return ret;
+
 	crtc_w = fb->width - x;
 	crtc_h = fb->height - y;
+	exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
+			    crtc_w, crtc_h, x, y, crtc_w, crtc_h);
 
-	return exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
-				   crtc_w, crtc_h, x, y, crtc_w, crtc_h);
+	return 0;
 }
 
 static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
@@ -164,7 +170,6 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
 {
 	struct drm_device *dev = crtc->dev;
 	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
-	struct drm_framebuffer *old_fb = crtc->primary->fb;
 	unsigned int crtc_w, crtc_h;
 	int ret;
 
@@ -183,6 +188,10 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
 		goto out;
 	}
 
+	ret = exynos_check_plane(crtc->primary, fb);
+	if (ret)
+		goto out;
+
 	ret = drm_vblank_get(dev, exynos_crtc->pipe);
 	if (ret) {
 		DRM_DEBUG("failed to acquire vblank counter\n");
@@ -201,17 +210,9 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
 	crtc->primary->fb = fb;
 	crtc_w = fb->width - crtc->x;
 	crtc_h = fb->height - crtc->y;
-	ret = exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
-				  crtc_w, crtc_h, crtc->x, crtc->y,
-				  crtc_w, crtc_h);
-	if (ret) {
-		crtc->primary->fb = old_fb;
-		spin_lock_irq(&dev->event_lock);
-		exynos_crtc->event = NULL;
-		drm_vblank_put(dev, exynos_crtc->pipe);
-		spin_unlock_irq(&dev->event_lock);
-		return ret;
-	}
+	exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
+			    crtc_w, crtc_h, crtc->x, crtc->y,
+			    crtc_w, crtc_h);
 
 	return 0;
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index 4367379..3d7c749 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -141,21 +141,15 @@ void exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc,
 	plane->crtc = crtc;
 }
 
-int
+void
 exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 		     struct drm_framebuffer *fb, int crtc_x, int crtc_y,
 		     unsigned int crtc_w, unsigned int crtc_h,
 		     uint32_t src_x, uint32_t src_y,
 		     uint32_t src_w, uint32_t src_h)
 {
-
 	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
 	struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
-	int ret;
-
-	ret = exynos_check_plane(plane, fb);
-	if (ret < 0)
-		return ret;
 
 	exynos_plane_mode_set(plane, crtc, fb, crtc_x, crtc_y,
 			      crtc_w, crtc_h, src_x >> 16, src_y >> 16,
@@ -163,8 +157,6 @@ exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 
 	if (exynos_crtc->ops->win_commit)
 		exynos_crtc->ops->win_commit(exynos_crtc, exynos_plane->zpos);
-
-	return 0;
 }
 
 static int exynos_disable_plane(struct drm_plane *plane)
@@ -180,11 +172,34 @@ static int exynos_disable_plane(struct drm_plane *plane)
 }
 
 static struct drm_plane_funcs exynos_plane_funcs = {
-	.update_plane	= exynos_update_plane,
+	.update_plane	= drm_plane_helper_update,
 	.disable_plane	= exynos_disable_plane,
 	.destroy	= drm_plane_cleanup,
 };
 
+static int exynos_plane_atomic_check(struct drm_plane *plane,
+				     struct drm_plane_state *state)
+{
+	return exynos_check_plane(plane, state->fb);
+}
+
+static void exynos_plane_atomic_update(struct drm_plane *plane,
+				       struct drm_plane_state *old_state)
+{
+	struct drm_plane_state *state = plane->state;
+
+	exynos_update_plane(plane, state->crtc, state->fb,
+			    state->crtc_x, state->crtc_y,
+			    state->crtc_w, state->crtc_h,
+			    state->src_x >> 16, state->src_y >> 16,
+			    state->src_w >> 16, state->src_h >> 16);
+}
+
+static const struct drm_plane_helper_funcs plane_helper_funcs = {
+	.atomic_check = exynos_plane_atomic_check,
+	.atomic_update = exynos_plane_atomic_update,
+};
+
 static void exynos_plane_attach_zpos_property(struct drm_plane *plane,
 					      unsigned int zpos)
 {
@@ -220,6 +235,8 @@ int exynos_plane_init(struct drm_device *dev,
 		return err;
 	}
 
+	drm_plane_helper_add(&exynos_plane->base, &plane_helper_funcs);
+
 	exynos_plane->zpos = zpos;
 
 	if (type == DRM_PLANE_TYPE_OVERLAY)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.h b/drivers/gpu/drm/exynos/exynos_drm_plane.h
index f360590..560ca71 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.h
@@ -15,7 +15,7 @@ void exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc,
 			   unsigned int crtc_w, unsigned int crtc_h,
 			   uint32_t src_x, uint32_t src_y,
 			   uint32_t src_w, uint32_t src_h);
-int exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
+void exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 			struct drm_framebuffer *fb, int crtc_x, int crtc_y,
 			unsigned int crtc_w, unsigned int crtc_h,
 			uint32_t src_x, uint32_t src_y,
-- 
1.9.3

[PATCH -v2 09/14] drm/exynos: atomic phase 1: use drm_plane_helper_disable()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

The atomic helper to disable planes also uses the optional
.atomic_disable() helper. The unique operation it does is calling
.win_disable()

exynos_drm_fb_get_buf_cnt() needs a fb check too to avoid a null pointer.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_drm_fb.c    |  2 +-
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 26 +++++++++++++-------------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
index d346d1e..470456d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
@@ -136,7 +136,7 @@ unsigned int exynos_drm_fb_get_buf_cnt(struct drm_framebuffer *fb)
 
 	exynos_fb = to_exynos_fb(fb);
 
-	return exynos_fb->buf_cnt;
+	return exynos_fb ? exynos_fb->buf_cnt : 0;
 }
 
 struct drm_framebuffer *
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index 3d7c749..5d3243e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -159,21 +159,9 @@ exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 		exynos_crtc->ops->win_commit(exynos_crtc, exynos_plane->zpos);
 }
 
-static int exynos_disable_plane(struct drm_plane *plane)
-{
-	struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
-	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(plane->crtc);
-
-	if (exynos_crtc->ops->win_disable)
-		exynos_crtc->ops->win_disable(exynos_crtc,
-					      exynos_plane->zpos);
-
-	return 0;
-}
-
 static struct drm_plane_funcs exynos_plane_funcs = {
 	.update_plane	= drm_plane_helper_update,
-	.disable_plane	= exynos_disable_plane,
+	.disable_plane	= drm_plane_helper_disable,
 	.destroy	= drm_plane_cleanup,
 };
 
@@ -195,9 +183,21 @@ static void exynos_plane_atomic_update(struct drm_plane *plane,
 			    state->src_w >> 16, state->src_h >> 16);
 }
 
+static void exynos_plane_atomic_disable(struct drm_plane *plane,
+				        struct drm_plane_state *old_state)
+{
+	struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
+	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(old_state->crtc);
+
+	if (exynos_crtc->ops->win_disable)
+		exynos_crtc->ops->win_disable(exynos_crtc,
+					      exynos_plane->zpos);
+}
+
 static const struct drm_plane_helper_funcs plane_helper_funcs = {
 	.atomic_check = exynos_plane_atomic_check,
 	.atomic_update = exynos_plane_atomic_update,
+	.atomic_disable = exynos_plane_atomic_disable,
 };
 
 static void exynos_plane_attach_zpos_property(struct drm_plane *plane,
-- 
1.9.3

[PATCH -v2 10/14] drm/exynos: atomic phase 1: add atomic_begin()/atomic_flush()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Add CRTC callbacks .atomic_begin() .atomic_flush(). On exynos they
unprotect the windows before the commit and protects it after based on
a plane mask tha store which plane will be updated.

For that we create two new exynos_crtc callbacks: .win_protect() and
.win_unprotect(). The only driver that implement those now is FIMD.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c  | 34 +++++++++++++++++++++
 drivers/gpu/drm/exynos/exynos_drm_drv.h   |  6 ++++
 drivers/gpu/drm/exynos/exynos_drm_fimd.c  | 49 ++++++++++++++++++++-----------
 drivers/gpu/drm/exynos/exynos_drm_plane.c |  4 +++
 4 files changed, 76 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 1c0d936..5e7c13e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -153,6 +153,38 @@ static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
 	}
 }
 
+static void exynos_crtc_atomic_begin(struct drm_crtc *crtc)
+{
+	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
+	struct drm_plane *plane;
+	int index = 0;
+
+	list_for_each_entry(plane, &crtc->dev->mode_config.plane_list, head) {
+		if (exynos_crtc->ops->win_protect &&
+		    exynos_crtc->plane_mask & (0x01 << index))
+			exynos_crtc->ops->win_protect(exynos_crtc, index);
+
+		index++;
+	}
+}
+
+static void exynos_crtc_atomic_flush(struct drm_crtc *crtc)
+{
+	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
+	struct drm_plane *plane;
+	int index = 0;
+
+	list_for_each_entry(plane, &crtc->dev->mode_config.plane_list, head) {
+		if (exynos_crtc->ops->win_unprotect &&
+		    exynos_crtc->plane_mask & (0x01 << index))
+			exynos_crtc->ops->win_unprotect(exynos_crtc, index);
+
+		index++;
+	}
+
+	exynos_crtc->plane_mask = 0;
+}
+
 static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
 	.dpms		= exynos_drm_crtc_dpms,
 	.prepare	= exynos_drm_crtc_prepare,
@@ -161,6 +193,8 @@ static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
 	.mode_set	= exynos_drm_crtc_mode_set,
 	.mode_set_base	= exynos_drm_crtc_mode_set_base,
 	.disable	= exynos_drm_crtc_disable,
+	.atomic_begin	= exynos_crtc_atomic_begin,
+	.atomic_flush	= exynos_crtc_atomic_flush,
 };
 
 static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index ab82772..f025a54 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -175,6 +175,8 @@ struct exynos_drm_display {
  *	hardware overlay is updated.
  * @win_commit: apply hardware specific overlay data to registers.
  * @win_disable: disable hardware specific overlay.
+ * @win_protect: protect hardware specific window.
+ * @win_unprotect: unprotect hardware specific window.
  * @te_handler: trigger to transfer video image at the tearing effect
  *	synchronization signal if there is a page flip request.
  */
@@ -190,6 +192,8 @@ struct exynos_drm_crtc_ops {
 	void (*wait_for_vblank)(struct exynos_drm_crtc *crtc);
 	void (*win_commit)(struct exynos_drm_crtc *crtc, unsigned int zpos);
 	void (*win_disable)(struct exynos_drm_crtc *crtc, unsigned int zpos);
+	void (*win_protect)(struct exynos_drm_crtc *crtc, unsigned int zpos);
+	void (*win_unprotect)(struct exynos_drm_crtc *crtc, unsigned int zpos);
 	void (*te_handler)(struct exynos_drm_crtc *crtc);
 };
 
@@ -206,6 +210,7 @@ struct exynos_drm_crtc_ops {
  *	we can refer to the crtc to current hardware interrupt occurred through
  *	this pipe value.
  * @dpms: store the crtc dpms value
+ * @plane_mask: store planes to be updated on atomic modesetting
  * @event: vblank event that is currently queued for flip
  * @ops: pointer to callbacks for exynos drm specific functionality
  * @ctx: A pointer to the crtc's implementation specific context
@@ -215,6 +220,7 @@ struct exynos_drm_crtc {
 	enum exynos_drm_output_type	type;
 	unsigned int			pipe;
 	unsigned int			dpms;
+	unsigned int			plane_mask;
 	wait_queue_head_t		pending_flip_queue;
 	struct drm_pending_vblank_event	*event;
 	struct exynos_drm_crtc_ops	*ops;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 990ead434..bea70f6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -590,6 +590,16 @@ static void fimd_shadow_protect_win(struct fimd_context *ctx,
 {
 	u32 reg, bits, val;
 
+	/*
+	 * SHADOWCON/PRTCON register is used for enabling timing.
+	 *
+	 * for example, once only width value of a register is set,
+	 * if the dma is started then fimd hardware could malfunction so
+	 * with protect window setting, the register fields with prefix '_F'
+	 * wouldn't be updated at vsync also but updated once unprotect window
+	 * is set.
+	 */
+
 	if (ctx->driver_data->has_shadowcon) {
 		reg = SHADOWCON;
 		bits = SHADOWCON_WINx_PROTECT(win);
@@ -628,20 +638,6 @@ static void fimd_win_commit(struct exynos_drm_crtc *crtc, unsigned int win)
 		return;
 	}
 
-	/*
-	 * SHADOWCON/PRTCON register is used for enabling timing.
-	 *
-	 * for example, once only width value of a register is set,
-	 * if the dma is started then fimd hardware could malfunction so
-	 * with protect window setting, the register fields with prefix '_F'
-	 * wouldn't be updated at vsync also but updated once unprotect window
-	 * is set.
-	 */
-
-	/* protect windows */
-	fimd_shadow_protect_win(ctx, win, true);
-
-
 	offset = plane->fb_x * (plane->bpp >> 3);
 	offset += plane->fb_y * plane->pitch;
 
@@ -723,9 +719,6 @@ static void fimd_win_commit(struct exynos_drm_crtc *crtc, unsigned int win)
 	if (ctx->driver_data->has_shadowcon)
 		fimd_enable_shadow_channel_path(ctx, win, true);
 
-	/* Enable DMA channel and unprotect windows */
-	fimd_shadow_protect_win(ctx, win, false);
-
 	plane->enabled = true;
 
 	if (ctx->i80_if)
@@ -944,6 +937,26 @@ static void fimd_te_handler(struct exynos_drm_crtc *crtc)
 		drm_handle_vblank(ctx->drm_dev, ctx->pipe);
 }
 
+static void fimd_win_protect(struct exynos_drm_crtc *crtc, unsigned int win)
+{
+	struct fimd_context *ctx = crtc->ctx;
+
+	if (win < 0 || win >= WINDOWS_NR)
+		return;
+
+	fimd_shadow_protect_win(ctx, win, true);
+}
+
+static void fimd_win_unprotect(struct exynos_drm_crtc *crtc, unsigned int win)
+{
+	struct fimd_context *ctx = crtc->ctx;
+
+	if (win < 0 || win >= WINDOWS_NR)
+		return;
+
+	fimd_shadow_protect_win(ctx, win, false);
+}
+
 static struct exynos_drm_crtc_ops fimd_crtc_ops = {
 	.dpms = fimd_dpms,
 	.mode_fixup = fimd_mode_fixup,
@@ -954,6 +967,8 @@ static struct exynos_drm_crtc_ops fimd_crtc_ops = {
 	.win_commit = fimd_win_commit,
 	.win_disable = fimd_win_disable,
 	.te_handler = fimd_te_handler,
+	.win_protect = fimd_win_protect,
+	.win_unprotect = fimd_win_unprotect,
 };
 
 static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index 5d3243e..ee81dc2 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -65,6 +65,7 @@ static int exynos_plane_get_size(int start, unsigned length, unsigned last)
 int exynos_check_plane(struct drm_plane *plane, struct drm_framebuffer *fb)
 {
 	struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
+	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(plane->crtc);
 	int nr;
 	int i;
 
@@ -83,6 +84,9 @@ int exynos_check_plane(struct drm_plane *plane, struct drm_framebuffer *fb)
 				i, (unsigned long)exynos_plane->dma_addr[i]);
 	}
 
+	if (exynos_crtc)
+		exynos_crtc->plane_mask += 1 << exynos_plane->zpos;
+
 	return 0;
 }
 
-- 
1.9.3

[PATCH -v2 11/14] drm/exynos: atomic phase 1: add .mode_set_nofb() callback

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

The new atomic infrastructure needs the .mode_set_nofb() callback to
update CRTC timings before setting any plane.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c | 60 +++++---------------------------
 1 file changed, 9 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 5e7c13e..7fbea8e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -81,59 +81,16 @@ exynos_drm_crtc_mode_fixup(struct drm_crtc *crtc,
 	return true;
 }
 
-static int
-exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode,
-			  struct drm_display_mode *adjusted_mode, int x, int y,
-			  struct drm_framebuffer *old_fb)
-{
-	struct drm_framebuffer *fb = crtc->primary->fb;
-	unsigned int crtc_w;
-	unsigned int crtc_h;
-	int ret;
-
-	/*
-	 * copy the mode data adjusted by mode_fixup() into crtc->mode
-	 * so that hardware can be seet to proper mode.
-	 */
-	memcpy(&crtc->mode, adjusted_mode, sizeof(*adjusted_mode));
-
-	ret = exynos_check_plane(crtc->primary, fb);
-	if (ret < 0)
-		return ret;
-
-	crtc_w = fb->width - x;
-	crtc_h = fb->height - y;
-	exynos_plane_mode_set(crtc->primary, crtc, fb, 0, 0,
-			      crtc_w, crtc_h, x, y, crtc_w, crtc_h);
-
-	return 0;
-}
-
-static int exynos_drm_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
-					  struct drm_framebuffer *old_fb)
+static void
+exynos_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
 {
 	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
-	struct drm_framebuffer *fb = crtc->primary->fb;
-	unsigned int crtc_w;
-	unsigned int crtc_h;
-	int ret;
 
-	/* when framebuffer changing is requested, crtc's dpms should be on */
-	if (exynos_crtc->dpms > DRM_MODE_DPMS_ON) {
-		DRM_ERROR("failed framebuffer changing request.\n");
-		return -EPERM;
-	}
-
-	ret = exynos_check_plane(crtc->primary, fb);
-	if (ret)
-		return ret;
-
-	crtc_w = fb->width - x;
-	crtc_h = fb->height - y;
-	exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
-			    crtc_w, crtc_h, x, y, crtc_w, crtc_h);
+	if (WARN_ON(!crtc->state))
+		return;
 
-	return 0;
+	if (exynos_crtc->ops->commit)
+		exynos_crtc->ops->commit(exynos_crtc);
 }
 
 static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
@@ -190,8 +147,9 @@ static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
 	.prepare	= exynos_drm_crtc_prepare,
 	.commit		= exynos_drm_crtc_commit,
 	.mode_fixup	= exynos_drm_crtc_mode_fixup,
-	.mode_set	= exynos_drm_crtc_mode_set,
-	.mode_set_base	= exynos_drm_crtc_mode_set_base,
+	.mode_set	= drm_helper_crtc_mode_set,
+	.mode_set_nofb	= exynos_drm_crtc_mode_set_nofb,
+	.mode_set_base	= drm_helper_crtc_mode_set_base,
 	.disable	= exynos_drm_crtc_disable,
 	.atomic_begin	= exynos_crtc_atomic_begin,
 	.atomic_flush	= exynos_crtc_atomic_flush,
-- 
1.9.3

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

[PATCH -v2 12/14] drm/exynos: atomic phase 2: wire up state reset(), duplicate() and destroy()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Set CRTC, planes and connectors to use the default implementations from
the atomic helper library. The helpers will work to keep track of state
for each DRM object.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/bridge/ptn3460.c              | 4 ++++
 drivers/gpu/drm/exynos/exynos_dp_core.c       | 4 ++++
 drivers/gpu/drm/exynos/exynos_drm_connector.c | 4 ++++
 drivers/gpu/drm/exynos/exynos_drm_crtc.c      | 6 ++++++
 drivers/gpu/drm/exynos/exynos_drm_dpi.c       | 4 ++++
 drivers/gpu/drm/exynos/exynos_drm_drv.c       | 2 ++
 drivers/gpu/drm/exynos/exynos_drm_dsi.c       | 4 ++++
 drivers/gpu/drm/exynos/exynos_drm_plane.c     | 4 ++++
 drivers/gpu/drm/exynos/exynos_drm_vidi.c      | 4 ++++
 drivers/gpu/drm/exynos/exynos_hdmi.c          | 4 ++++
 10 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ptn3460.c b/drivers/gpu/drm/bridge/ptn3460.c
index 826833e..30da10c 100644
--- a/drivers/gpu/drm/bridge/ptn3460.c
+++ b/drivers/gpu/drm/bridge/ptn3460.c
@@ -27,6 +27,7 @@
 
 #include "drm_crtc.h"
 #include "drm_crtc_helper.h"
+#include "drm_atomic_helper.h"
 #include "drm_edid.h"
 #include "drmP.h"
 
@@ -263,6 +264,9 @@ static struct drm_connector_funcs ptn3460_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.detect = ptn3460_detect,
 	.destroy = ptn3460_connector_destroy,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
 int ptn3460_bridge_attach(struct drm_bridge *bridge)
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
index bf17a60..6704d5c 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -28,6 +28,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_panel.h>
 #include <drm/bridge/ptn3460.h>
 
@@ -952,6 +953,9 @@ static struct drm_connector_funcs exynos_dp_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.detect = exynos_dp_detect,
 	.destroy = exynos_dp_connector_destroy,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
 static int exynos_dp_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_connector.c b/drivers/gpu/drm/exynos/exynos_drm_connector.c
index ba9b3d5..980b085 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_connector.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_connector.c
@@ -13,6 +13,7 @@
 
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_atomic_helper.h>
 
 #include <drm/exynos_drm.h>
 #include "exynos_drm_drv.h"
@@ -182,6 +183,9 @@ static struct drm_connector_funcs exynos_connector_funcs = {
 	.fill_modes	= exynos_drm_connector_fill_modes,
 	.detect		= exynos_drm_connector_detect,
 	.destroy	= exynos_drm_connector_destroy,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
 struct drm_connector *exynos_drm_connector_create(struct drm_device *dev,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 7fbea8e..adb56656 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -14,6 +14,8 @@
 
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
 
 #include "exynos_drm_crtc.h"
 #include "exynos_drm_drv.h"
@@ -228,8 +230,12 @@ static struct drm_crtc_funcs exynos_crtc_funcs = {
 	.set_config	= drm_crtc_helper_set_config,
 	.page_flip	= exynos_drm_crtc_page_flip,
 	.destroy	= exynos_drm_crtc_destroy,
+	.reset = drm_atomic_helper_crtc_reset,
+	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
 };
 
+
 struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
 					       struct drm_plane *plane,
 					       int pipe,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
index 37678cf..ced5c23 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
@@ -13,6 +13,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_panel.h>
+#include <drm/drm_atomic_helper.h>
 
 #include <linux/regulator/consumer.h>
 
@@ -63,6 +64,9 @@ static struct drm_connector_funcs exynos_dpi_connector_funcs = {
 	.detect = exynos_dpi_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.destroy = exynos_dpi_connector_destroy,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
 static int exynos_dpi_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 778c91e..0b76a42 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -98,6 +98,8 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
 	if (ret)
 		goto err_cleanup_vblank;
 
+	drm_mode_config_reset(dev);
+
 	/*
 	 * enable drm irq mode.
 	 * - with irq_enabled = true, we can use the vblank feature.
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 05fe93d..6e9c2c3 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -14,6 +14,7 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_mipi_dsi.h>
 #include <drm/drm_panel.h>
+#include <drm/drm_atomic_helper.h>
 
 #include <linux/clk.h>
 #include <linux/gpio/consumer.h>
@@ -1461,6 +1462,9 @@ static struct drm_connector_funcs exynos_dsi_connector_funcs = {
 	.detect = exynos_dsi_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.destroy = exynos_dsi_connector_destroy,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
 static int exynos_dsi_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index ee81dc2..840e618 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -13,6 +13,7 @@
 
 #include <drm/exynos_drm.h>
 #include <drm/drm_plane_helper.h>
+#include <drm/drm_atomic_helper.h>
 #include "exynos_drm_drv.h"
 #include "exynos_drm_crtc.h"
 #include "exynos_drm_fb.h"
@@ -167,6 +168,9 @@ static struct drm_plane_funcs exynos_plane_funcs = {
 	.update_plane	= drm_plane_helper_update,
 	.disable_plane	= drm_plane_helper_disable,
 	.destroy	= drm_plane_cleanup,
+	.reset = drm_atomic_helper_plane_reset,
+	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
 };
 
 static int exynos_plane_atomic_check(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
index 0c1bdd9..7ba993a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -20,6 +20,7 @@
 
 #include <drm/drm_edid.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_atomic_helper.h>
 
 #include "exynos_drm_drv.h"
 #include "exynos_drm_crtc.h"
@@ -388,6 +389,9 @@ static struct drm_connector_funcs vidi_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.detect = vidi_detect,
 	.destroy = vidi_connector_destroy,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
 static int vidi_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 229b361..5b597bc 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -17,6 +17,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_atomic_helper.h>
 
 #include "regs-hdmi.h"
 
@@ -1054,6 +1055,9 @@ static struct drm_connector_funcs hdmi_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.detect = hdmi_detect,
 	.destroy = hdmi_connector_destroy,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
 static int hdmi_get_modes(struct drm_connector *connector)
-- 
1.9.3

[PATCH -v2 13/14] drm/exynos: atomic phase 2: keep track of framebuffer pointer

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Use drm_atomic_set_fb_for_plane() in the legacy page_flip path to keep
track of the framebuffer pointer and reference.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index adb56656..1739212 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -208,6 +208,9 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
 			    crtc_w, crtc_h, crtc->x, crtc->y,
 			    crtc_w, crtc_h);
 
+	if (crtc->primary->state)
+		drm_atomic_set_fb_for_plane(crtc->primary->state, fb);
+
 	return 0;
 
 out:
-- 
1.9.3

[PATCH -v2 14/14] drm/exynos: make exynos_plane_mode_set() static

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

It is not used outside of the plane scope anymore.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 11 ++++++-----
 drivers/gpu/drm/exynos/exynos_drm_plane.h |  5 -----
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index 840e618..31c8b13 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -91,11 +91,12 @@ int exynos_check_plane(struct drm_plane *plane, struct drm_framebuffer *fb)
 	return 0;
 }
 
-void exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc,
-			  struct drm_framebuffer *fb, int crtc_x, int crtc_y,
-			  unsigned int crtc_w, unsigned int crtc_h,
-			  uint32_t src_x, uint32_t src_y,
-			  uint32_t src_w, uint32_t src_h)
+static void exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc,
+				  struct drm_framebuffer *fb,
+				  int crtc_x, int crtc_y,
+				  unsigned int crtc_w, unsigned int crtc_h,
+				  uint32_t src_x, uint32_t src_y,
+				  uint32_t src_w, uint32_t src_h)
 {
 	struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
 	unsigned int actual_w;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.h b/drivers/gpu/drm/exynos/exynos_drm_plane.h
index 560ca71..a7b1a21 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.h
@@ -10,11 +10,6 @@
  */
 
 int exynos_check_plane(struct drm_plane *plane, struct drm_framebuffer *fb);
-void exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc,
-			   struct drm_framebuffer *fb, int crtc_x, int crtc_y,
-			   unsigned int crtc_w, unsigned int crtc_h,
-			   uint32_t src_x, uint32_t src_y,
-			   uint32_t src_w, uint32_t src_h);
 void exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 			struct drm_framebuffer *fb, int crtc_x, int crtc_y,
 			unsigned int crtc_w, unsigned int crtc_h,
-- 
1.9.3

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

Re: [PATCH -v2 10/14] drm/exynos: atomic phase 1: add atomic_begin()/atomic_flush()

[Inki Dae]

Hi,

I'm merging this patch series but I found two issues. One is already
pointed out.

On 2015년 02월 07일 04:37, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> Add CRTC callbacks .atomic_begin() .atomic_flush(). On exynos they
> unprotect the windows before the commit and protects it after based on
> a plane mask tha store which plane will be updated.
> 
> For that we create two new exynos_crtc callbacks: .win_protect() and
> .win_unprotect(). The only driver that implement those now is FIMD.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_crtc.c  | 34 +++++++++++++++++++++
>  drivers/gpu/drm/exynos/exynos_drm_drv.h   |  6 ++++
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c  | 49 ++++++++++++++++++++-----------
>  drivers/gpu/drm/exynos/exynos_drm_plane.c |  4 +++
>  4 files changed, 76 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> index 1c0d936..5e7c13e 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> @@ -153,6 +153,38 @@ static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
>  	}
>  }
>  
> +static void exynos_crtc_atomic_begin(struct drm_crtc *crtc)
> +{
> +	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
> +	struct drm_plane *plane;
> +	int index = 0;
> +
> +	list_for_each_entry(plane, &crtc->dev->mode_config.plane_list, head) {
> +		if (exynos_crtc->ops->win_protect &&
> +		    exynos_crtc->plane_mask & (0x01 << index))
> +			exynos_crtc->ops->win_protect(exynos_crtc, index);
> +
> +		index++;
> +	}
> +}
> +
> +static void exynos_crtc_atomic_flush(struct drm_crtc *crtc)
> +{
> +	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
> +	struct drm_plane *plane;
> +	int index = 0;
> +
> +	list_for_each_entry(plane, &crtc->dev->mode_config.plane_list, head) {
> +		if (exynos_crtc->ops->win_unprotect &&
> +		    exynos_crtc->plane_mask & (0x01 << index))
> +			exynos_crtc->ops->win_unprotect(exynos_crtc, index);
> +
> +		index++;
> +	}
> +
> +	exynos_crtc->plane_mask = 0;
> +}
> +
>  static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
>  	.dpms		= exynos_drm_crtc_dpms,
>  	.prepare	= exynos_drm_crtc_prepare,
> @@ -161,6 +193,8 @@ static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
>  	.mode_set	= exynos_drm_crtc_mode_set,
>  	.mode_set_base	= exynos_drm_crtc_mode_set_base,
>  	.disable	= exynos_drm_crtc_disable,
> +	.atomic_begin	= exynos_crtc_atomic_begin,
> +	.atomic_flush	= exynos_crtc_atomic_flush,
>  };
>  
>  static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index ab82772..f025a54 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -175,6 +175,8 @@ struct exynos_drm_display {
>   *	hardware overlay is updated.
>   * @win_commit: apply hardware specific overlay data to registers.
>   * @win_disable: disable hardware specific overlay.
> + * @win_protect: protect hardware specific window.
> + * @win_unprotect: unprotect hardware specific window.
>   * @te_handler: trigger to transfer video image at the tearing effect
>   *	synchronization signal if there is a page flip request.
>   */
> @@ -190,6 +192,8 @@ struct exynos_drm_crtc_ops {
>  	void (*wait_for_vblank)(struct exynos_drm_crtc *crtc);
>  	void (*win_commit)(struct exynos_drm_crtc *crtc, unsigned int zpos);
>  	void (*win_disable)(struct exynos_drm_crtc *crtc, unsigned int zpos);
> +	void (*win_protect)(struct exynos_drm_crtc *crtc, unsigned int zpos);
> +	void (*win_unprotect)(struct exynos_drm_crtc *crtc, unsigned int zpos);
>  	void (*te_handler)(struct exynos_drm_crtc *crtc);
>  };
>  
> @@ -206,6 +210,7 @@ struct exynos_drm_crtc_ops {
>   *	we can refer to the crtc to current hardware interrupt occurred through
>   *	this pipe value.
>   * @dpms: store the crtc dpms value
> + * @plane_mask: store planes to be updated on atomic modesetting
>   * @event: vblank event that is currently queued for flip
>   * @ops: pointer to callbacks for exynos drm specific functionality
>   * @ctx: A pointer to the crtc's implementation specific context
> @@ -215,6 +220,7 @@ struct exynos_drm_crtc {
>  	enum exynos_drm_output_type	type;
>  	unsigned int			pipe;
>  	unsigned int			dpms;
> +	unsigned int			plane_mask;
>  	wait_queue_head_t		pending_flip_queue;
>  	struct drm_pending_vblank_event	*event;
>  	struct exynos_drm_crtc_ops	*ops;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 990ead434..bea70f6 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -590,6 +590,16 @@ static void fimd_shadow_protect_win(struct fimd_context *ctx,
>  {
>  	u32 reg, bits, val;
>  
> +	/*
> +	 * SHADOWCON/PRTCON register is used for enabling timing.
> +	 *
> +	 * for example, once only width value of a register is set,
> +	 * if the dma is started then fimd hardware could malfunction so
> +	 * with protect window setting, the register fields with prefix '_F'
> +	 * wouldn't be updated at vsync also but updated once unprotect window
> +	 * is set.
> +	 */
> +
>  	if (ctx->driver_data->has_shadowcon) {
>  		reg = SHADOWCON;
>  		bits = SHADOWCON_WINx_PROTECT(win);
> @@ -628,20 +638,6 @@ static void fimd_win_commit(struct exynos_drm_crtc *crtc, unsigned int win)
>  		return;
>  	}
>  
> -	/*
> -	 * SHADOWCON/PRTCON register is used for enabling timing.
> -	 *
> -	 * for example, once only width value of a register is set,
> -	 * if the dma is started then fimd hardware could malfunction so
> -	 * with protect window setting, the register fields with prefix '_F'
> -	 * wouldn't be updated at vsync also but updated once unprotect window
> -	 * is set.
> -	 */
> -
> -	/* protect windows */
> -	fimd_shadow_protect_win(ctx, win, true);

You removed to protect shadow register updating at vsync so fimd
hardware could malfunction if setcrtc/page flip/setplane are requested
by userspace. Actually, when I run modetest repeatedly, I could see
overlay isn't rarely turned on.

So how about calling win_protect/unprotect callbacks like below? If you
are ok, I can modify it and merge it.

static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
{
        ...
        if (exynos_crtc->ops->win_protect)
                exynos_crtc->ops->win_protect(exynos_crtc,
exynos_plane->zpos);

        if (exynos_crtc->ops->win_commit)
                exynos_crtc->ops->win_commit(exynos_crtc,
exynos_plane->zpos);

        if (exynos_crtc->ops->win_unprotect)
                exynos_crtc->ops->win_unprotect(exynos_crtc,
exynos_plane->zpos);
        ...
}


And other, I could see below warning messages when I tried to unload
Exynos dsi module with a command, "echo 11c80000.dsi
>/sys/bus/platform/drivers/exynos-dsi/unbind"

# echo 11c80000.dsi >/sys/bus/platform/drivers/exynos-dsi/unbind
[  230.800467] Console: switching to colour dummy device 80x30
[  230.805005] drm_kms_helper: drm: unregistered panic notifier
[  230.849266] ------------[ cut here ]------------
[  230.852516] WARNING: CPU: 3 PID: 1428 at
drivers/gpu/drm/drm_crtc.c:5455 drm_mode_config_cleanup+0x1f4/0x1fc()
[  230.862532] Modules linked in:
[  230.865472] CPU: 3 PID: 1428 Comm: sh Not tainted
3.19.0-rc6-161746-g9d96aee-dirty #1243
[  230.873564] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[  230.879677] [<c0014430>] (unwind_backtrace) from [<c001158c>]
(show_stack+0x10/0x14)
[  230.887370] [<c001158c>] (show_stack) from [<c047cfbc>]
(dump_stack+0x84/0xc4)
[  230.894580] [<c047cfbc>] (dump_stack) from [<c0020b00>]
(warn_slowpath_common+0x80/0xb0)
[  230.902639] [<c0020b00>] (warn_slowpath_common) from [<c0020bcc>]
(warn_slowpath_null+0x1c/0x24)
[  230.911405] [<c0020bcc>] (warn_slowpath_null) from [<c0274370>]
(drm_mode_config_cleanup+0x1f4/0x1fc)
[  230.920608] [<c0274370>] (drm_mode_config_cleanup) from [<c0282970>]
(exynos_drm_unload+0x38/0x4c)
[  230.929548] [<c0282970>] (exynos_drm_unload) from [<c026c250>]
(drm_dev_unregister+0x24/0x98)
[  230.938050] [<c026c250>] (drm_dev_unregister) from [<c026c4dc>]
(drm_put_dev+0x2c/0x60)
[  230.946046] [<c026c4dc>] (drm_put_dev) from [<c029b1d0>]
(take_down_master+0x24/0x44)
[  230.953861] [<c029b1d0>] (take_down_master) from [<c029b328>]
(component_del+0x90/0xd0)
[  230.961850] [<c029b328>] (component_del) from [<c0288184>]
(exynos_dsi_remove+0x18/0x2c)
[  230.969919] [<c0288184>] (exynos_dsi_remove) from [<c02a10cc>]
(platform_drv_remove+0x18/0x30)
[  230.978505] [<c02a10cc>] (platform_drv_remove) from [<c029f58c>]
(__device_release_driver+0x70/0xc4)
[  230.987615] [<c029f58c>] (__device_release_driver) from [<c029f5fc>]
(device_release_driver+0x1c/0x28)
[  230.996904] [<c029f5fc>] (device_release_driver) from [<c029e730>]
(unbind_store+0x78/0xf8)
[  231.005240] [<c029e730>] (unbind_store) from [<c0125f1c>]
(kernfs_fop_write+0xb8/0x19c)
[  231.013223] [<c0125f1c>] (kernfs_fop_write) from [<c00cd830>]
(vfs_write+0xa0/0x1ac)
[  231.020946] [<c00cd830>] (vfs_write) from [<c00cde88>]
(SyS_write+0x44/0x9c)
[  231.027983] [<c00cde88>] (SyS_write) from [<c000e6e0>]
(ret_fast_syscall+0x0/0x34)
[  231.035523] ---[ end trace 954377a3aab4ab59 ]---
[  231.042414] lcd0-power-domain: Power-off latency exceeded, new value
201333 ns

This warning would mean a fb object leak and while I have a review, I
couldn't find why it causes the leak.

However, I'd like to merge this patch series this time and fix this
issue at RC for phase 3.

Thanks,
Inki Dae

Re: [PATCH -v2 10/14] drm/exynos: atomic phase 1: add atomic_begin()/atomic_flush()

[Daniel Stone]

Hi Inki,

On 9 February 2015 at 14:53, Inki Dae <inki.dae@samsung.com> wrote:
> I'm merging this patch series but I found two issues. One is already
> pointed out.

Fantastic - thanks a lot for doing this!

> On 2015년 02월 07일 04:37, Gustavo Padovan wrote:
>> @@ -628,20 +638,6 @@ static void fimd_win_commit(struct exynos_drm_crtc *crtc, unsigned int win)
>>               return;
>>       }
>>
>> -     /*
>> -      * SHADOWCON/PRTCON register is used for enabling timing.
>> -      *
>> -      * for example, once only width value of a register is set,
>> -      * if the dma is started then fimd hardware could malfunction so
>> -      * with protect window setting, the register fields with prefix '_F'
>> -      * wouldn't be updated at vsync also but updated once unprotect window
>> -      * is set.
>> -      */
>> -
>> -     /* protect windows */
>> -     fimd_shadow_protect_win(ctx, win, true);
>
> You removed to protect shadow register updating at vsync so fimd
> hardware could malfunction if setcrtc/page flip/setplane are requested
> by userspace. Actually, when I run modetest repeatedly, I could see
> overlay isn't rarely turned on.
>
> So how about calling win_protect/unprotect callbacks like below? If you
> are ok, I can modify it and merge it.

I think you are missing some details about atomic and the helpers.

The helpers wrap _all_ legacy codepaths, e.g. SetCrtc, SetPlane, etc.
All of these operations are intercepted by the code in
drm_atomic_helper.c / drm_plane_helper.c code, and the legacy
operations are turned into atomic operations. For the driver - there
are no legacy operations.

The atomic helpers guarantee that atomic_begin() will be called before
the state is committed, and atomic_flush() will be called after the
state is committed. Thus this change is completely safe.

> This warning would mean a fb object leak and while I have a review, I
> couldn't find why it causes the leak.

I'm not really surprised; the fb refcounting in Exynos has long had
quite a few issues. The refcounting is incredibly difficult to get
right, and whilst atomic changes the order of operations a little, it
also makes the semantics of fb/vblank refcounting much more
consistent, so it should be much easier to fix than usual.

> However, I'd like to merge this patch series this time and fix this
> issue at RC for phase 3.

Great, thankyou! Really very happy about this. :)

Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH -v2 10/14] drm/exynos: atomic phase 1: add atomic_begin()/atomic_flush()

[Gustavo Padovan]

2015-02-09 Daniel Stone <daniel@fooishbar.org>:

> Hi Inki,
> 
> On 9 February 2015 at 14:53, Inki Dae <inki.dae@samsung.com> wrote:
> > I'm merging this patch series but I found two issues. One is already
> > pointed out.
> 
> Fantastic - thanks a lot for doing this!
> 
> > On 2015년 02월 07일 04:37, Gustavo Padovan wrote:
> >> @@ -628,20 +638,6 @@ static void fimd_win_commit(struct exynos_drm_crtc *crtc, unsigned int win)
> >>               return;
> >>       }
> >>
> >> -     /*
> >> -      * SHADOWCON/PRTCON register is used for enabling timing.
> >> -      *
> >> -      * for example, once only width value of a register is set,
> >> -      * if the dma is started then fimd hardware could malfunction so
> >> -      * with protect window setting, the register fields with prefix '_F'
> >> -      * wouldn't be updated at vsync also but updated once unprotect window
> >> -      * is set.
> >> -      */
> >> -
> >> -     /* protect windows */
> >> -     fimd_shadow_protect_win(ctx, win, true);
> >
> > You removed to protect shadow register updating at vsync so fimd
> > hardware could malfunction if setcrtc/page flip/setplane are requested
> > by userspace. Actually, when I run modetest repeatedly, I could see
> > overlay isn't rarely turned on.
> >
> > So how about calling win_protect/unprotect callbacks like below? If you
> > are ok, I can modify it and merge it.
> 
> I think you are missing some details about atomic and the helpers.
> 
> The helpers wrap _all_ legacy codepaths, e.g. SetCrtc, SetPlane, etc.
> All of these operations are intercepted by the code in
> drm_atomic_helper.c / drm_plane_helper.c code, and the legacy
> operations are turned into atomic operations. For the driver - there
> are no legacy operations.
> 
> The atomic helpers guarantee that atomic_begin() will be called before
> the state is committed, and atomic_flush() will be called after the
> state is committed. Thus this change is completely safe.

Exactly, when phase 3 is merged you won't see any of these issues. I have
patches ready for most part of phase 3. Expect them soon in the mailing list.

	Gustavo

Re: [PATCH -v2 10/14] drm/exynos: atomic phase 1: add atomic_begin()/atomic_flush()

[Inki Dae]

On 2015년 02월 10일 02:07, Gustavo Padovan wrote:
> 2015-02-09 Daniel Stone <daniel@fooishbar.org>:
> 
>> Hi Inki,
>>
>> On 9 February 2015 at 14:53, Inki Dae <inki.dae@samsung.com> wrote:
>>> I'm merging this patch series but I found two issues. One is already
>>> pointed out.
>>
>> Fantastic - thanks a lot for doing this!
>>
>>> On 2015년 02월 07일 04:37, Gustavo Padovan wrote:
>>>> @@ -628,20 +638,6 @@ static void fimd_win_commit(struct exynos_drm_crtc *crtc, unsigned int win)
>>>>               return;
>>>>       }
>>>>
>>>> -     /*
>>>> -      * SHADOWCON/PRTCON register is used for enabling timing.
>>>> -      *
>>>> -      * for example, once only width value of a register is set,
>>>> -      * if the dma is started then fimd hardware could malfunction so
>>>> -      * with protect window setting, the register fields with prefix '_F'
>>>> -      * wouldn't be updated at vsync also but updated once unprotect window
>>>> -      * is set.
>>>> -      */
>>>> -
>>>> -     /* protect windows */
>>>> -     fimd_shadow_protect_win(ctx, win, true);
>>>
>>> You removed to protect shadow register updating at vsync so fimd
>>> hardware could malfunction if setcrtc/page flip/setplane are requested
>>> by userspace. Actually, when I run modetest repeatedly, I could see
>>> overlay isn't rarely turned on.
>>>
>>> So how about calling win_protect/unprotect callbacks like below? If you
>>> are ok, I can modify it and merge it.
>>
>> I think you are missing some details about atomic and the helpers.
>>
>> The helpers wrap _all_ legacy codepaths, e.g. SetCrtc, SetPlane, etc.
>> All of these operations are intercepted by the code in
>> drm_atomic_helper.c / drm_plane_helper.c code, and the legacy
>> operations are turned into atomic operations. For the driver - there
>> are no legacy operations.

Yes, SetCrtc does this but not page flip and SetPlane. When I had page
flip and SetPlane test using modetest, win_protect/unprotect callbacks
are never called. In my opinion, they, page flip and SetPlane, also are
required for win_protect/unprotect when updating overlay registers.

In addition, I could find some issues.

With regard to SetPlane relevent codes, it seems considered for
win_protect/unprotect but actually, they are never called because when
atomic_check callback is called, plane->crtc is NULL which would be set
 at exynos_plane_mode_set function. However, exynos_plane_mode_set
function is called later than atomic_check callback.

One more thing, it seems your below patch makes possible_crtc of each
crtc driver have wrong value because this patch calls exynos_plane_init
function before fimd_ctx_initialize and vidi_ctx_initialize functions
are called.

 commit fba6fe129efda336f84cf05d61c152ee45d4d9b5
Author: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Date:   Fri Feb 6 17:37:46 2015 -0200

    drm/exynos: remove struct *_win_data abstraction on planes

I wanted to merge this patch series this time but it seems that we have
to review more them. Unfortunately, now merge window and we have no much
time to review them. So how about merging this patch series next time
after reviews enough? Your patch series is great and thanks for your
contribution. But as of now, I'm not sure that this patch series is safe
and would work well.

Thanks,
Inki Dae

>>
>> The atomic helpers guarantee that atomic_begin() will be called before
>> the state is committed, and atomic_flush() will be called after the
>> state is committed. Thus this change is completely safe.
> 
> Exactly, when phase 3 is merged you won't see any of these issues. I have
> patches ready for most part of phase 3. Expect them soon in the mailing list.
> 
> 	Gustavo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

Re: [PATCH -v2 10/14] drm/exynos: atomic phase 1: add atomic_begin()/atomic_flush()

[Gustavo Padovan]

2015-02-10 Inki Dae <inki.dae@samsung.com>:

> On 2015년 02월 10일 02:07, Gustavo Padovan wrote:
> > 2015-02-09 Daniel Stone <daniel@fooishbar.org>:
> > 
> >> Hi Inki,
> >>
> >> On 9 February 2015 at 14:53, Inki Dae <inki.dae@samsung.com> wrote:
> >>> I'm merging this patch series but I found two issues. One is already
> >>> pointed out.
> >>
> >> Fantastic - thanks a lot for doing this!
> >>
> >>> On 2015년 02월 07일 04:37, Gustavo Padovan wrote:
> >>>> @@ -628,20 +638,6 @@ static void fimd_win_commit(struct exynos_drm_crtc *crtc, unsigned int win)
> >>>>               return;
> >>>>       }
> >>>>
> >>>> -     /*
> >>>> -      * SHADOWCON/PRTCON register is used for enabling timing.
> >>>> -      *
> >>>> -      * for example, once only width value of a register is set,
> >>>> -      * if the dma is started then fimd hardware could malfunction so
> >>>> -      * with protect window setting, the register fields with prefix '_F'
> >>>> -      * wouldn't be updated at vsync also but updated once unprotect window
> >>>> -      * is set.
> >>>> -      */
> >>>> -
> >>>> -     /* protect windows */
> >>>> -     fimd_shadow_protect_win(ctx, win, true);
> >>>
> >>> You removed to protect shadow register updating at vsync so fimd
> >>> hardware could malfunction if setcrtc/page flip/setplane are requested
> >>> by userspace. Actually, when I run modetest repeatedly, I could see
> >>> overlay isn't rarely turned on.
> >>>
> >>> So how about calling win_protect/unprotect callbacks like below? If you
> >>> are ok, I can modify it and merge it.
> >>
> >> I think you are missing some details about atomic and the helpers.
> >>
> >> The helpers wrap _all_ legacy codepaths, e.g. SetCrtc, SetPlane, etc.
> >> All of these operations are intercepted by the code in
> >> drm_atomic_helper.c / drm_plane_helper.c code, and the legacy
> >> operations are turned into atomic operations. For the driver - there
> >> are no legacy operations.
> 
> Yes, SetCrtc does this but not page flip and SetPlane. When I had page
> flip and SetPlane test using modetest, win_protect/unprotect callbacks
> are never called. In my opinion, they, page flip and SetPlane, also are
> required for win_protect/unprotect when updating overlay registers.

Once atomic phase 3 all operations will call win_protect/win_unprotect. I'll
send the next version of this patchset with the atomic phase 3 patches
included.

> 
> In addition, I could find some issues.
> 
> With regard to SetPlane relevent codes, it seems considered for
> win_protect/unprotect but actually, they are never called because when
> atomic_check callback is called, plane->crtc is NULL which would be set
>  at exynos_plane_mode_set function. However, exynos_plane_mode_set
> function is called later than atomic_check callback.
> 
> One more thing, it seems your below patch makes possible_crtc of each
> crtc driver have wrong value because this patch calls exynos_plane_init
> function before fimd_ctx_initialize and vidi_ctx_initialize functions
> are called.

Sure, I can fix this. Thanks for your review.

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