All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/10] imx drm atomic mode setting conversion
@ 2016-05-31  9:24 Liu Ying
  2016-05-31  9:24 ` [PATCH v2 01/10] drm/imx: ipuv3 plane: Check different types of plane separately Liu Ying
                   ` (9 more replies)
  0 siblings, 10 replies; 13+ messages in thread
From: Liu Ying @ 2016-05-31  9:24 UTC (permalink / raw)
  To: dri-devel; +Cc: Russell King, Daniel Vetter

Hi,

This patch set converts imx drm into atomic mode setting.
It takes 3 phases to achieve the goal as recommended.

v1->v2:
* Rebase onto Philipp Zabel's open git branch imx-drm/next as Philipp
  required.
* Drop patch 05/14 and 10/14 in v1 which touch drm core to disable
  plane in transitional helper drm_helper_crtc_mode_set and in
  drm_atomic_helper_disable_all, because we won't get ipu plane
  resource in v2 when updating plane and failure won't happen.
* Wait for pending commit on each CRTC for both block and nonblock
  atomic mode settings.  This way, a block commit will not overwrite
  the hardware setting when a nonblock page flip is about to finish,
  so that the page flip may wait for vblank successfully.
* See changelogs in each patch for other trivial updates.

Liu Ying (10):
  drm/imx: ipuv3 plane: Check different types of plane separately
  gpu: ipu-v3: ipu-dmfc: Use static DMFC FIFO allocation mechanism
  drm/imx: atomic phase 1: Use transitional atomic CRTC and plane
    helpers
  drm/imx: atomic phase 2 step 1: Wire up state ->reset, ->duplicate and
    ->destroy
  drm/imx: atomic phase 2 step 2: Track plane_state->fb correctly in
    ->page_flip
  drm/imx: atomic phase 3 step 1: Atomic updates for planes
  drm/imx: atomic phase 3 step 2: Use atomic configuration
  drm/imx: atomic phase 3 step 3: Legacy callback fixups
  drm/imx: atomic phase 3 step 4: Use generic atomic page flip
  drm/imx: atomic phase 3 step 5: Advertise DRIVER_ATOMIC

 drivers/gpu/drm/bridge/dw-hdmi.c       |  19 +-
 drivers/gpu/drm/imx/dw_hdmi-imx.c      |  22 +-
 drivers/gpu/drm/imx/imx-drm-core.c     | 194 +++++++++---
 drivers/gpu/drm/imx/imx-drm.h          |  18 +-
 drivers/gpu/drm/imx/imx-ldb.c          | 129 ++++----
 drivers/gpu/drm/imx/imx-tve.c          |  85 ++----
 drivers/gpu/drm/imx/ipuv3-crtc.c       | 359 ++++++----------------
 drivers/gpu/drm/imx/ipuv3-plane.c      | 539 +++++++++++++++++++--------------
 drivers/gpu/drm/imx/ipuv3-plane.h      |  16 +-
 drivers/gpu/drm/imx/parallel-display.c |  74 +++--
 drivers/gpu/ipu-v3/ipu-dc.c            |   5 +-
 drivers/gpu/ipu-v3/ipu-di.c            |   3 -
 drivers/gpu/ipu-v3/ipu-dmfc.c          | 213 +------------
 include/video/imx-ipu-v3.h             |   3 -
 14 files changed, 716 insertions(+), 963 deletions(-)

-- 
2.7.4

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

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

* [PATCH v2 01/10] drm/imx: ipuv3 plane: Check different types of plane separately
  2016-05-31  9:24 [PATCH v2 00/10] imx drm atomic mode setting conversion Liu Ying
@ 2016-05-31  9:24 ` Liu Ying
  2016-05-31  9:24 ` [PATCH v2 02/10] gpu: ipu-v3: ipu-dmfc: Use static DMFC FIFO allocation mechanism Liu Ying
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Liu Ying @ 2016-05-31  9:24 UTC (permalink / raw)
  To: dri-devel; +Cc: Russell King, Daniel Vetter

The IPUv3 primary plane doesn't support partial off screen.
So, this patch separates plane check logics for primary plane and overlay
plane and adds more limitations on the primary plane.

Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v1->v2:
* Remove an unnecessary copy to address Philipp's comment.

 drivers/gpu/drm/imx/ipuv3-plane.c | 67 ++++++++++++++++++++++-----------------
 1 file changed, 38 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index a4bb441..cd7eb26 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -199,37 +199,46 @@ int ipu_plane_mode_set(struct ipu_plane *ipu_plane, struct drm_crtc *crtc,
 	if (src_w != crtc_w || src_h != crtc_h)
 		return -EINVAL;
 
-	/* clip to crtc bounds */
-	if (crtc_x < 0) {
-		if (-crtc_x > crtc_w)
+	if (ipu_plane->base.type == DRM_PLANE_TYPE_PRIMARY) {
+		/* full plane doesn't support partial off screen */
+		if (crtc_x || crtc_y || crtc_w != mode->hdisplay ||
+			crtc_h != mode->vdisplay)
 			return -EINVAL;
-		src_x += -crtc_x;
-		src_w -= -crtc_x;
-		crtc_w -= -crtc_x;
-		crtc_x = 0;
-	}
-	if (crtc_y < 0) {
-		if (-crtc_y > crtc_h)
-			return -EINVAL;
-		src_y += -crtc_y;
-		src_h -= -crtc_y;
-		crtc_h -= -crtc_y;
-		crtc_y = 0;
-	}
-	if (crtc_x + crtc_w > mode->hdisplay) {
-		if (crtc_x > mode->hdisplay)
-			return -EINVAL;
-		crtc_w = mode->hdisplay - crtc_x;
-		src_w = crtc_w;
-	}
-	if (crtc_y + crtc_h > mode->vdisplay) {
-		if (crtc_y > mode->vdisplay)
+
+		/* full plane minimum width is 13 pixels */
+		if (crtc_w < 13)
 			return -EINVAL;
-		crtc_h = mode->vdisplay - crtc_y;
-		src_h = crtc_h;
-	}
-	/* full plane minimum width is 13 pixels */
-	if (crtc_w < 13 && (ipu_plane->dp_flow != IPU_DP_FLOW_SYNC_FG))
+	} else if (ipu_plane->base.type == DRM_PLANE_TYPE_OVERLAY) {
+		/* clip to crtc bounds */
+		if (crtc_x < 0) {
+			if (-crtc_x > crtc_w)
+				return -EINVAL;
+			src_x += -crtc_x;
+			src_w -= -crtc_x;
+			crtc_w -= -crtc_x;
+			crtc_x = 0;
+		}
+		if (crtc_y < 0) {
+			if (-crtc_y > crtc_h)
+				return -EINVAL;
+			src_y += -crtc_y;
+			src_h -= -crtc_y;
+			crtc_h -= -crtc_y;
+			crtc_y = 0;
+		}
+		if (crtc_x + crtc_w > mode->hdisplay) {
+			if (crtc_x > mode->hdisplay)
+				return -EINVAL;
+			crtc_w = mode->hdisplay - crtc_x;
+			src_w = crtc_w;
+		}
+		if (crtc_y + crtc_h > mode->vdisplay) {
+			if (crtc_y > mode->vdisplay)
+				return -EINVAL;
+			crtc_h = mode->vdisplay - crtc_y;
+			src_h = crtc_h;
+		}
+	} else
 		return -EINVAL;
 	if (crtc_h < 2)
 		return -EINVAL;
-- 
2.7.4

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

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

* [PATCH v2 02/10] gpu: ipu-v3: ipu-dmfc: Use static DMFC FIFO allocation mechanism
  2016-05-31  9:24 [PATCH v2 00/10] imx drm atomic mode setting conversion Liu Ying
  2016-05-31  9:24 ` [PATCH v2 01/10] drm/imx: ipuv3 plane: Check different types of plane separately Liu Ying
@ 2016-05-31  9:24 ` Liu Ying
  2016-05-31  9:24 ` [PATCH v2 03/10] drm/imx: atomic phase 1: Use transitional atomic CRTC and plane helpers Liu Ying
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Liu Ying @ 2016-05-31  9:24 UTC (permalink / raw)
  To: dri-devel; +Cc: Russell King, Daniel Vetter

For all video modes we support currently, we always get 2 slots for
a plane by using the current existing dynamic DMFC FIFO allocation
mechanism.  So, let's change to use the static one to simplify the
code.  This also makes it easier to implement the atomic mode setting
as we don't need to handle allocation failure cases then.

Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v1->v2:
* Improve the commit message to better explain why the static allocation
  mechanism can replace the dynamic one.

 drivers/gpu/drm/imx/ipuv3-plane.c |  26 -----
 drivers/gpu/ipu-v3/ipu-dmfc.c     | 213 ++------------------------------------
 include/video/imx-ipu-v3.h        |   3 -
 3 files changed, 7 insertions(+), 235 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index cd7eb26..02701de 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -53,24 +53,6 @@ int ipu_plane_irq(struct ipu_plane *ipu_plane)
 				     IPU_IRQ_EOF);
 }
 
-static int calc_vref(struct drm_display_mode *mode)
-{
-	unsigned long htotal, vtotal;
-
-	htotal = mode->htotal;
-	vtotal = mode->vtotal;
-
-	if (!htotal || !vtotal)
-		return 60;
-
-	return DIV_ROUND_UP(mode->clock * 1000, vtotal * htotal);
-}
-
-static inline int calc_bandwidth(int width, int height, unsigned int vref)
-{
-	return width * height * vref;
-}
-
 int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb,
 		       int x, int y)
 {
@@ -291,14 +273,6 @@ int ipu_plane_mode_set(struct ipu_plane *ipu_plane, struct drm_crtc *crtc,
 		}
 	}
 
-	ret = ipu_dmfc_alloc_bandwidth(ipu_plane->dmfc,
-			calc_bandwidth(crtc_w, crtc_h,
-				       calc_vref(mode)), 64);
-	if (ret) {
-		dev_err(dev, "allocating dmfc bandwidth failed with %d\n", ret);
-		return ret;
-	}
-
 	ipu_dmfc_config_wait4eot(ipu_plane->dmfc, crtc_w);
 
 	ipu_cpmem_zero(ipu_plane->ipu_ch);
diff --git a/drivers/gpu/ipu-v3/ipu-dmfc.c b/drivers/gpu/ipu-v3/ipu-dmfc.c
index 837b1ec2..42705bb 100644
--- a/drivers/gpu/ipu-v3/ipu-dmfc.c
+++ b/drivers/gpu/ipu-v3/ipu-dmfc.c
@@ -45,17 +45,6 @@
 #define DMFC_DP_CHAN_6B_24		16
 #define DMFC_DP_CHAN_6F_29		24
 
-#define DMFC_FIFO_SIZE_64		(3 << 3)
-#define DMFC_FIFO_SIZE_128		(2 << 3)
-#define DMFC_FIFO_SIZE_256		(1 << 3)
-#define DMFC_FIFO_SIZE_512		(0 << 3)
-
-#define DMFC_SEGMENT(x)			((x & 0x7) << 0)
-#define DMFC_BURSTSIZE_128		(0 << 6)
-#define DMFC_BURSTSIZE_64		(1 << 6)
-#define DMFC_BURSTSIZE_32		(2 << 6)
-#define DMFC_BURSTSIZE_16		(3 << 6)
-
 struct dmfc_channel_data {
 	int		ipu_channel;
 	unsigned long	channel_reg;
@@ -104,9 +93,6 @@ struct ipu_dmfc_priv;
 
 struct dmfc_channel {
 	unsigned			slots;
-	unsigned			slotmask;
-	unsigned			segment;
-	int				burstsize;
 	struct ipu_soc			*ipu;
 	struct ipu_dmfc_priv		*priv;
 	const struct dmfc_channel_data	*data;
@@ -117,7 +103,6 @@ struct ipu_dmfc_priv {
 	struct device *dev;
 	struct dmfc_channel channels[DMFC_NUM_CHANNELS];
 	struct mutex mutex;
-	unsigned long bandwidth_per_slot;
 	void __iomem *base;
 	int use_count;
 };
@@ -172,184 +157,6 @@ void ipu_dmfc_disable_channel(struct dmfc_channel *dmfc)
 }
 EXPORT_SYMBOL_GPL(ipu_dmfc_disable_channel);
 
-static int ipu_dmfc_setup_channel(struct dmfc_channel *dmfc, int slots,
-		int segment, int burstsize)
-{
-	struct ipu_dmfc_priv *priv = dmfc->priv;
-	u32 val, field;
-
-	dev_dbg(priv->dev,
-			"dmfc: using %d slots starting from segment %d for IPU channel %d\n",
-			slots, segment, dmfc->data->ipu_channel);
-
-	switch (slots) {
-	case 1:
-		field = DMFC_FIFO_SIZE_64;
-		break;
-	case 2:
-		field = DMFC_FIFO_SIZE_128;
-		break;
-	case 4:
-		field = DMFC_FIFO_SIZE_256;
-		break;
-	case 8:
-		field = DMFC_FIFO_SIZE_512;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	switch (burstsize) {
-	case 16:
-		field |= DMFC_BURSTSIZE_16;
-		break;
-	case 32:
-		field |= DMFC_BURSTSIZE_32;
-		break;
-	case 64:
-		field |= DMFC_BURSTSIZE_64;
-		break;
-	case 128:
-		field |= DMFC_BURSTSIZE_128;
-		break;
-	}
-
-	field |= DMFC_SEGMENT(segment);
-
-	val = readl(priv->base + dmfc->data->channel_reg);
-
-	val &= ~(0xff << dmfc->data->shift);
-	val |= field << dmfc->data->shift;
-
-	writel(val, priv->base + dmfc->data->channel_reg);
-
-	dmfc->slots = slots;
-	dmfc->segment = segment;
-	dmfc->burstsize = burstsize;
-	dmfc->slotmask = ((1 << slots) - 1) << segment;
-
-	return 0;
-}
-
-static int dmfc_bandwidth_to_slots(struct ipu_dmfc_priv *priv,
-		unsigned long bandwidth)
-{
-	int slots = 1;
-
-	while (slots * priv->bandwidth_per_slot < bandwidth)
-		slots *= 2;
-
-	return slots;
-}
-
-static int dmfc_find_slots(struct ipu_dmfc_priv *priv, int slots)
-{
-	unsigned slotmask_need, slotmask_used = 0;
-	int i, segment = 0;
-
-	slotmask_need = (1 << slots) - 1;
-
-	for (i = 0; i < DMFC_NUM_CHANNELS; i++)
-		slotmask_used |= priv->channels[i].slotmask;
-
-	while (slotmask_need <= 0xff) {
-		if (!(slotmask_used & slotmask_need))
-			return segment;
-
-		slotmask_need <<= 1;
-		segment++;
-	}
-
-	return -EBUSY;
-}
-
-void ipu_dmfc_free_bandwidth(struct dmfc_channel *dmfc)
-{
-	struct ipu_dmfc_priv *priv = dmfc->priv;
-	int i;
-
-	dev_dbg(priv->dev, "dmfc: freeing %d slots starting from segment %d\n",
-			dmfc->slots, dmfc->segment);
-
-	mutex_lock(&priv->mutex);
-
-	if (!dmfc->slots)
-		goto out;
-
-	dmfc->slotmask = 0;
-	dmfc->slots = 0;
-	dmfc->segment = 0;
-
-	for (i = 0; i < DMFC_NUM_CHANNELS; i++)
-		priv->channels[i].slotmask = 0;
-
-	for (i = 0; i < DMFC_NUM_CHANNELS; i++) {
-		if (priv->channels[i].slots > 0) {
-			priv->channels[i].segment =
-				dmfc_find_slots(priv, priv->channels[i].slots);
-			priv->channels[i].slotmask =
-				((1 << priv->channels[i].slots) - 1) <<
-				priv->channels[i].segment;
-		}
-	}
-
-	for (i = 0; i < DMFC_NUM_CHANNELS; i++) {
-		if (priv->channels[i].slots > 0)
-			ipu_dmfc_setup_channel(&priv->channels[i],
-					priv->channels[i].slots,
-					priv->channels[i].segment,
-					priv->channels[i].burstsize);
-	}
-out:
-	mutex_unlock(&priv->mutex);
-}
-EXPORT_SYMBOL_GPL(ipu_dmfc_free_bandwidth);
-
-int ipu_dmfc_alloc_bandwidth(struct dmfc_channel *dmfc,
-		unsigned long bandwidth_pixel_per_second, int burstsize)
-{
-	struct ipu_dmfc_priv *priv = dmfc->priv;
-	int slots = dmfc_bandwidth_to_slots(priv, bandwidth_pixel_per_second);
-	int segment = -1, ret = 0;
-
-	dev_dbg(priv->dev, "dmfc: trying to allocate %ldMpixel/s for IPU channel %d\n",
-			bandwidth_pixel_per_second / 1000000,
-			dmfc->data->ipu_channel);
-
-	ipu_dmfc_free_bandwidth(dmfc);
-
-	mutex_lock(&priv->mutex);
-
-	if (slots > 8) {
-		ret = -EBUSY;
-		goto out;
-	}
-
-	/* For the MEM_BG channel, first try to allocate twice the slots */
-	if (dmfc->data->ipu_channel == IPUV3_CHANNEL_MEM_BG_SYNC)
-		segment = dmfc_find_slots(priv, slots * 2);
-	else if (slots < 2)
-		/* Always allocate at least 128*4 bytes (2 slots) */
-		slots = 2;
-
-	if (segment >= 0)
-		slots *= 2;
-	else
-		segment = dmfc_find_slots(priv, slots);
-	if (segment < 0) {
-		ret = -EBUSY;
-		goto out;
-	}
-
-	ipu_dmfc_setup_channel(dmfc, slots, segment, burstsize);
-
-out:
-	mutex_unlock(&priv->mutex);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(ipu_dmfc_alloc_bandwidth);
-
 void ipu_dmfc_config_wait4eot(struct dmfc_channel *dmfc, int width)
 {
 	struct ipu_dmfc_priv *priv = dmfc->priv;
@@ -384,7 +191,6 @@ EXPORT_SYMBOL_GPL(ipu_dmfc_get);
 
 void ipu_dmfc_put(struct dmfc_channel *dmfc)
 {
-	ipu_dmfc_free_bandwidth(dmfc);
 }
 EXPORT_SYMBOL_GPL(ipu_dmfc_put);
 
@@ -412,20 +218,15 @@ int ipu_dmfc_init(struct ipu_soc *ipu, struct device *dev, unsigned long base,
 		priv->channels[i].priv = priv;
 		priv->channels[i].ipu = ipu;
 		priv->channels[i].data = &dmfcdata[i];
-	}
-
-	writel(0x0, priv->base + DMFC_WR_CHAN);
-	writel(0x0, priv->base + DMFC_DP_CHAN);
 
-	/*
-	 * We have a total bandwidth of clkrate * 4pixel divided
-	 * into 8 slots.
-	 */
-	priv->bandwidth_per_slot = clk_get_rate(ipu_clk) * 4 / 8;
-
-	dev_dbg(dev, "dmfc: 8 slots with %ldMpixel/s bandwidth each\n",
-			priv->bandwidth_per_slot / 1000000);
+		if (dmfcdata[i].ipu_channel == IPUV3_CHANNEL_MEM_BG_SYNC ||
+		    dmfcdata[i].ipu_channel == IPUV3_CHANNEL_MEM_FG_SYNC ||
+		    dmfcdata[i].ipu_channel == IPUV3_CHANNEL_MEM_DC_SYNC)
+			priv->channels[i].slots = 2;
+	}
 
+	writel(0x00000050, priv->base + DMFC_WR_CHAN);
+	writel(0x00005654, priv->base + DMFC_DP_CHAN);
 	writel(0x202020f6, priv->base + DMFC_WR_CHAN_DEF);
 	writel(0x2020f6f6, priv->base + DMFC_DP_CHAN_DEF);
 	writel(0x00000003, priv->base + DMFC_GENERAL1);
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index 3a2a794..7adeaae 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -235,9 +235,6 @@ int ipu_di_init_sync_panel(struct ipu_di *, struct ipu_di_signal_cfg *sig);
 struct dmfc_channel;
 int ipu_dmfc_enable_channel(struct dmfc_channel *dmfc);
 void ipu_dmfc_disable_channel(struct dmfc_channel *dmfc);
-int ipu_dmfc_alloc_bandwidth(struct dmfc_channel *dmfc,
-		unsigned long bandwidth_mbs, int burstsize);
-void ipu_dmfc_free_bandwidth(struct dmfc_channel *dmfc);
 void ipu_dmfc_config_wait4eot(struct dmfc_channel *dmfc, int width);
 struct dmfc_channel *ipu_dmfc_get(struct ipu_soc *ipu, int ipuv3_channel);
 void ipu_dmfc_put(struct dmfc_channel *dmfc);
-- 
2.7.4

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

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

* [PATCH v2 03/10] drm/imx: atomic phase 1: Use transitional atomic CRTC and plane helpers
  2016-05-31  9:24 [PATCH v2 00/10] imx drm atomic mode setting conversion Liu Ying
  2016-05-31  9:24 ` [PATCH v2 01/10] drm/imx: ipuv3 plane: Check different types of plane separately Liu Ying
  2016-05-31  9:24 ` [PATCH v2 02/10] gpu: ipu-v3: ipu-dmfc: Use static DMFC FIFO allocation mechanism Liu Ying
@ 2016-05-31  9:24 ` Liu Ying
  2016-05-31  9:24 ` [PATCH v2 04/10] drm/imx: atomic phase 2 step 1: Wire up state ->reset, ->duplicate and ->destroy Liu Ying
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Liu Ying @ 2016-05-31  9:24 UTC (permalink / raw)
  To: dri-devel; +Cc: Russell King, Daniel Vetter

Use the drm_plane_helper_update/disable() and drm_helper_crtc_mode_set()
transitional atomic helpers.  The crtc->mode_set_nofb callback is added
so that the primary plane is no longer tied to the CRTC.  Check/update
logics are separated to make sure crtc->mode_set_nofb and plane->atomic_update
are always successful.  Also, some necessary logics are tweaked for a smooth
transition.

Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v1->v2:
* Get the overlay ipu plane resource when initializing the relevant CRTC
  and do not get ipu plane resource any more when updating plane to avoid
  resource allocation failure.
* Remove obsolete comments from ipu_crtc_enable/disable().

 drivers/gpu/drm/imx/ipuv3-crtc.c  | 197 ++++++++-------
 drivers/gpu/drm/imx/ipuv3-plane.c | 517 +++++++++++++++++++++++---------------
 drivers/gpu/drm/imx/ipuv3-plane.h |  16 +-
 drivers/gpu/ipu-v3/ipu-dc.c       |   5 +-
 drivers/gpu/ipu-v3/ipu-di.c       |   3 -
 5 files changed, 416 insertions(+), 322 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index fc04041..ba880fa 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -73,7 +73,7 @@ struct ipu_crtc {
 
 #define to_ipu_crtc(x) container_of(x, struct ipu_crtc, base)
 
-static void ipu_fb_enable(struct ipu_crtc *ipu_crtc)
+static void ipu_crtc_enable(struct ipu_crtc *ipu_crtc)
 {
 	struct ipu_soc *ipu = dev_get_drvdata(ipu_crtc->dev->parent);
 
@@ -81,30 +81,30 @@ static void ipu_fb_enable(struct ipu_crtc *ipu_crtc)
 		return;
 
 	ipu_dc_enable(ipu);
-	ipu_plane_enable(ipu_crtc->plane[0]);
-	/* Start DC channel and DI after IDMAC */
 	ipu_dc_enable_channel(ipu_crtc->dc);
 	ipu_di_enable(ipu_crtc->di);
-	drm_crtc_vblank_on(&ipu_crtc->base);
-
 	ipu_crtc->enabled = 1;
+
+	/*
+	 * In order not to be warned on enabling vblank failure,
+	 * we should call drm_crtc_vblank_on() after ->enabled is set to 1.
+	 */
+	drm_crtc_vblank_on(&ipu_crtc->base);
 }
 
-static void ipu_fb_disable(struct ipu_crtc *ipu_crtc)
+static void ipu_crtc_disable(struct ipu_crtc *ipu_crtc)
 {
 	struct ipu_soc *ipu = dev_get_drvdata(ipu_crtc->dev->parent);
 
 	if (!ipu_crtc->enabled)
 		return;
 
-	/* Stop DC channel and DI before IDMAC */
 	ipu_dc_disable_channel(ipu_crtc->dc);
 	ipu_di_disable(ipu_crtc->di);
-	ipu_plane_disable(ipu_crtc->plane[0]);
 	ipu_dc_disable(ipu);
-	drm_crtc_vblank_off(&ipu_crtc->base);
-
 	ipu_crtc->enabled = 0;
+
+	drm_crtc_vblank_off(&ipu_crtc->base);
 }
 
 static void ipu_crtc_dpms(struct drm_crtc *crtc, int mode)
@@ -115,12 +115,12 @@ static void ipu_crtc_dpms(struct drm_crtc *crtc, int mode)
 
 	switch (mode) {
 	case DRM_MODE_DPMS_ON:
-		ipu_fb_enable(ipu_crtc);
+		ipu_crtc_enable(ipu_crtc);
 		break;
 	case DRM_MODE_DPMS_STANDBY:
 	case DRM_MODE_DPMS_SUSPEND:
 	case DRM_MODE_DPMS_OFF:
-		ipu_fb_disable(ipu_crtc);
+		ipu_crtc_disable(ipu_crtc);
 		break;
 	}
 }
@@ -234,79 +234,6 @@ static const struct drm_crtc_funcs ipu_crtc_funcs = {
 	.page_flip = ipu_page_flip,
 };
 
-static int ipu_crtc_mode_set(struct drm_crtc *crtc,
-			       struct drm_display_mode *orig_mode,
-			       struct drm_display_mode *mode,
-			       int x, int y,
-			       struct drm_framebuffer *old_fb)
-{
-	struct drm_device *dev = crtc->dev;
-	struct drm_encoder *encoder;
-	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
-	struct ipu_di_signal_cfg sig_cfg = {};
-	unsigned long encoder_types = 0;
-	int ret;
-
-	dev_dbg(ipu_crtc->dev, "%s: mode->hdisplay: %d\n", __func__,
-			mode->hdisplay);
-	dev_dbg(ipu_crtc->dev, "%s: mode->vdisplay: %d\n", __func__,
-			mode->vdisplay);
-
-	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
-		if (encoder->crtc == crtc)
-			encoder_types |= BIT(encoder->encoder_type);
-
-	dev_dbg(ipu_crtc->dev, "%s: attached to encoder types 0x%lx\n",
-		__func__, encoder_types);
-
-	/*
-	 * If we have DAC or LDB, then we need the IPU DI clock to be
-	 * the same as the LDB DI clock. For TVDAC, derive the IPU DI
-	 * clock from 27 MHz TVE_DI clock, but allow to divide it.
-	 */
-	if (encoder_types & (BIT(DRM_MODE_ENCODER_DAC) |
-			     BIT(DRM_MODE_ENCODER_LVDS)))
-		sig_cfg.clkflags = IPU_DI_CLKMODE_SYNC | IPU_DI_CLKMODE_EXT;
-	else if (encoder_types & BIT(DRM_MODE_ENCODER_TVDAC))
-		sig_cfg.clkflags = IPU_DI_CLKMODE_EXT;
-	else
-		sig_cfg.clkflags = 0;
-
-	sig_cfg.enable_pol = !(ipu_crtc->bus_flags & DRM_BUS_FLAG_DE_LOW);
-	/* Default to driving pixel data on negative clock edges */
-	sig_cfg.clk_pol = !!(ipu_crtc->bus_flags &
-			     DRM_BUS_FLAG_PIXDATA_POSEDGE);
-	sig_cfg.bus_format = ipu_crtc->bus_format;
-	sig_cfg.v_to_h_sync = 0;
-	sig_cfg.hsync_pin = ipu_crtc->di_hsync_pin;
-	sig_cfg.vsync_pin = ipu_crtc->di_vsync_pin;
-
-	drm_display_mode_to_videomode(mode, &sig_cfg.mode);
-
-	ret = ipu_dc_init_sync(ipu_crtc->dc, ipu_crtc->di,
-			       mode->flags & DRM_MODE_FLAG_INTERLACE,
-			       ipu_crtc->bus_format, mode->hdisplay);
-	if (ret) {
-		dev_err(ipu_crtc->dev,
-				"initializing display controller failed with %d\n",
-				ret);
-		return ret;
-	}
-
-	ret = ipu_di_init_sync_panel(ipu_crtc->di, &sig_cfg);
-	if (ret) {
-		dev_err(ipu_crtc->dev,
-				"initializing panel failed with %d\n", ret);
-		return ret;
-	}
-
-	return ipu_plane_mode_set(ipu_crtc->plane[0], crtc, mode,
-				  crtc->primary->fb,
-				  0, 0, mode->hdisplay, mode->vdisplay,
-				  x, y, mode->hdisplay, mode->vdisplay,
-				  mode->flags & DRM_MODE_FLAG_INTERLACE);
-}
-
 static void ipu_crtc_handle_pageflip(struct ipu_crtc *ipu_crtc)
 {
 	unsigned long flags;
@@ -330,8 +257,7 @@ static irqreturn_t ipu_irq_handler(int irq, void *dev_id)
 	if (ipu_crtc->flip_state == IPU_FLIP_SUBMITTED) {
 		struct ipu_plane *plane = ipu_crtc->plane[0];
 
-		ipu_plane_set_base(plane, ipu_crtc->base.primary->fb,
-				   plane->x, plane->y);
+		ipu_plane_set_base(plane, ipu_crtc->base.primary->fb);
 		ipu_crtc_handle_pageflip(ipu_crtc);
 		queue_work(ipu_crtc->flip_queue,
 			   &ipu_crtc->flip_work->unref_work);
@@ -355,6 +281,9 @@ static bool ipu_crtc_mode_fixup(struct drm_crtc *crtc,
 	if (ret)
 		return false;
 
+	if ((vm.vsync_len == 0) || (vm.hsync_len == 0))
+		return false;
+
 	drm_display_mode_from_videomode(&vm, adjusted_mode);
 
 	return true;
@@ -364,28 +293,95 @@ static void ipu_crtc_prepare(struct drm_crtc *crtc)
 {
 	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
 
-	ipu_fb_disable(ipu_crtc);
+	ipu_crtc_disable(ipu_crtc);
 }
 
 static void ipu_crtc_commit(struct drm_crtc *crtc)
 {
 	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
 
-	ipu_fb_enable(ipu_crtc);
+	ipu_crtc_enable(ipu_crtc);
+}
+
+static int ipu_crtc_atomic_check(struct drm_crtc *crtc,
+				 struct drm_crtc_state *state)
+{
+	return 0;
+}
+
+static void ipu_crtc_mode_set_nofb(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_encoder *encoder;
+	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
+	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
+	struct ipu_di_signal_cfg sig_cfg = {};
+	unsigned long encoder_types = 0;
+
+	dev_dbg(ipu_crtc->dev, "%s: mode->hdisplay: %d\n", __func__,
+			mode->hdisplay);
+	dev_dbg(ipu_crtc->dev, "%s: mode->vdisplay: %d\n", __func__,
+			mode->vdisplay);
+
+	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
+		if (encoder->crtc == crtc)
+			encoder_types |= BIT(encoder->encoder_type);
+
+	dev_dbg(ipu_crtc->dev, "%s: attached to encoder types 0x%lx\n",
+		__func__, encoder_types);
+
+	/*
+	 * If we have DAC or LDB, then we need the IPU DI clock to be
+	 * the same as the LDB DI clock. For TVDAC, derive the IPU DI
+	 * clock from 27 MHz TVE_DI clock, but allow to divide it.
+	 */
+	if (encoder_types & (BIT(DRM_MODE_ENCODER_DAC) |
+			     BIT(DRM_MODE_ENCODER_LVDS)))
+		sig_cfg.clkflags = IPU_DI_CLKMODE_SYNC | IPU_DI_CLKMODE_EXT;
+	else if (encoder_types & BIT(DRM_MODE_ENCODER_TVDAC))
+		sig_cfg.clkflags = IPU_DI_CLKMODE_EXT;
+	else
+		sig_cfg.clkflags = 0;
+
+	sig_cfg.enable_pol = !(ipu_crtc->bus_flags & DRM_BUS_FLAG_DE_LOW);
+	/* Default to driving pixel data on negative clock edges */
+	sig_cfg.clk_pol = !!(ipu_crtc->bus_flags &
+			     DRM_BUS_FLAG_PIXDATA_POSEDGE);
+	sig_cfg.bus_format = ipu_crtc->bus_format;
+	sig_cfg.v_to_h_sync = 0;
+	sig_cfg.hsync_pin = ipu_crtc->di_hsync_pin;
+	sig_cfg.vsync_pin = ipu_crtc->di_vsync_pin;
+
+	drm_display_mode_to_videomode(mode, &sig_cfg.mode);
+
+	ipu_dc_init_sync(ipu_crtc->dc, ipu_crtc->di,
+			 mode->flags & DRM_MODE_FLAG_INTERLACE,
+			 ipu_crtc->bus_format, mode->hdisplay);
+	ipu_di_init_sync_panel(ipu_crtc->di, &sig_cfg);
 }
 
 static const struct drm_crtc_helper_funcs ipu_helper_funcs = {
 	.dpms = ipu_crtc_dpms,
 	.mode_fixup = ipu_crtc_mode_fixup,
-	.mode_set = ipu_crtc_mode_set,
+	.mode_set = drm_helper_crtc_mode_set,
+	.mode_set_nofb = ipu_crtc_mode_set_nofb,
 	.prepare = ipu_crtc_prepare,
 	.commit = ipu_crtc_commit,
+	.atomic_check = ipu_crtc_atomic_check,
 };
 
 static int ipu_enable_vblank(struct drm_crtc *crtc)
 {
 	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
 
+	/*
+	 * ->commit is done after ->mode_set in drm_crtc_helper_set_mode(),
+	 * so waiting for vblank in drm_plane_helper_commit() will timeout.
+	 * Check the state here to avoid the waiting.
+	 */
+	if (!ipu_crtc->enabled)
+		return -EINVAL;
+
 	enable_irq(ipu_crtc->irq);
 
 	return 0;
@@ -496,8 +492,16 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
 						IPU_DP_FLOW_SYNC_FG,
 						drm_crtc_mask(&ipu_crtc->base),
 						DRM_PLANE_TYPE_OVERLAY);
-		if (IS_ERR(ipu_crtc->plane[1]))
+		if (IS_ERR(ipu_crtc->plane[1])) {
 			ipu_crtc->plane[1] = NULL;
+		} else {
+			ret = ipu_plane_get_resources(ipu_crtc->plane[1]);
+			if (ret) {
+				dev_err(ipu_crtc->dev, "getting plane 1 "
+					"resources failed with %d.\n", ret);
+				goto err_put_plane0_res;
+			}
+		}
 	}
 
 	ipu_crtc->irq = ipu_plane_irq(ipu_crtc->plane[0]);
@@ -505,7 +509,7 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
 			"imx_drm", ipu_crtc);
 	if (ret < 0) {
 		dev_err(ipu_crtc->dev, "irq request failed with %d.\n", ret);
-		goto err_put_plane_res;
+		goto err_put_plane1_res;
 	}
 	/* Only enable IRQ when we actually need it to trigger work. */
 	disable_irq(ipu_crtc->irq);
@@ -514,7 +518,10 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
 
 	return 0;
 
-err_put_plane_res:
+err_put_plane1_res:
+	if (ipu_crtc->plane[1])
+		ipu_plane_put_resources(ipu_crtc->plane[1]);
+err_put_plane0_res:
 	ipu_plane_put_resources(ipu_crtc->plane[0]);
 err_remove_crtc:
 	imx_drm_remove_crtc(ipu_crtc->imx_crtc);
@@ -554,8 +561,10 @@ static void ipu_drm_unbind(struct device *dev, struct device *master,
 	imx_drm_remove_crtc(ipu_crtc->imx_crtc);
 
 	destroy_workqueue(ipu_crtc->flip_queue);
-	ipu_plane_put_resources(ipu_crtc->plane[0]);
 	ipu_put_resources(ipu_crtc);
+	if (ipu_crtc->plane[1])
+		ipu_plane_put_resources(ipu_crtc->plane[1]);
+	ipu_plane_put_resources(ipu_crtc->plane[0]);
 }
 
 static const struct component_ops ipu_crtc_ops = {
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 02701de..9b9260b 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -16,6 +16,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_plane_helper.h>
 
 #include "video/imx-ipu-v3.h"
 #include "ipuv3-plane.h"
@@ -53,12 +54,15 @@ int ipu_plane_irq(struct ipu_plane *ipu_plane)
 				     IPU_IRQ_EOF);
 }
 
-int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb,
-		       int x, int y)
+int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb)
 {
-	struct drm_gem_cma_object *cma_obj[3];
-	unsigned long eba, ubo, vbo;
+	struct drm_gem_cma_object *cma_obj[3], *old_cma_obj[3];
+	struct drm_plane_state *state = ipu_plane->base.state;
+	struct drm_framebuffer *old_fb = state->fb;
+	unsigned long eba, ubo, vbo, old_eba, old_ubo, old_vbo;
 	int active, i;
+	int x = state->src_x >> 16;
+	int y = state->src_y >> 16;
 
 	for (i = 0; i < drm_format_num_planes(fb->pixel_format); i++) {
 		cma_obj[i] = drm_fb_cma_get_gem_obj(fb, i);
@@ -68,6 +72,14 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb,
 		}
 	}
 
+	for (i = 0; i < drm_format_num_planes(old_fb->pixel_format); i++) {
+		old_cma_obj[i] = drm_fb_cma_get_gem_obj(old_fb, i);
+		if (!old_cma_obj[i]) {
+			DRM_DEBUG_KMS("plane %d entry is null.\n", i);
+			return -EFAULT;
+		}
+	}
+
 	eba = cma_obj[0]->paddr + fb->offsets[0] +
 	      fb->pitches[0] * y + (fb->bits_per_pixel >> 3) * x;
 
@@ -81,13 +93,11 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb,
 		return -EINVAL;
 	}
 
-	if (ipu_plane->enabled && fb->pitches[0] != ipu_plane->stride[0]) {
+	if (fb->pitches[0] != old_fb->pitches[0]) {
 		DRM_DEBUG_KMS("pitches must not change while plane is enabled.\n");
 		return -EINVAL;
 	}
 
-	ipu_plane->stride[0] = fb->pitches[0];
-
 	switch (fb->pixel_format) {
 	case DRM_FORMAT_YUV420:
 	case DRM_FORMAT_YVU420:
@@ -104,6 +114,14 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb,
 		vbo = cma_obj[2]->paddr + fb->offsets[2] +
 		      fb->pitches[2] * y / 2 + x / 2 - eba;
 
+		old_eba = old_cma_obj[0]->paddr + old_fb->offsets[0] +
+		      old_fb->pitches[0] * y +
+		      (old_fb->bits_per_pixel >> 3) * x;
+		old_ubo = old_cma_obj[1]->paddr + old_fb->offsets[1] +
+		      old_fb->pitches[1] * y / 2 + x / 2 - old_eba;
+		old_vbo = old_cma_obj[2]->paddr + old_fb->offsets[2] +
+		      old_fb->pitches[2] * y / 2 + x / 2 - old_eba;
+
 		if ((ubo & 0x7) || (vbo & 0x7)) {
 			DRM_DEBUG_KMS("U/V buffer offsets must be a multiple of 8.\n");
 			return -EINVAL;
@@ -114,8 +132,7 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb,
 			return -EINVAL;
 		}
 
-		if (ipu_plane->enabled && ((ipu_plane->u_offset != ubo) ||
-					   (ipu_plane->v_offset != vbo))) {
+		if (old_ubo != ubo || old_vbo != vbo) {
 			DRM_DEBUG_KMS("U/V buffer offsets must not change while plane is enabled.\n");
 			return -EINVAL;
 		}
@@ -130,16 +147,11 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb,
 			return -EINVAL;
 		}
 
-		if (ipu_plane->enabled &&
-		    (ipu_plane->stride[1] != fb->pitches[1])) {
+		if (old_fb->pitches[1] != fb->pitches[1]) {
 			DRM_DEBUG_KMS("U/V pitches must not change while plane is enabled.\n");
 			return -EINVAL;
 		}
 
-		ipu_plane->u_offset = ubo;
-		ipu_plane->v_offset = vbo;
-		ipu_plane->stride[1] = fb->pitches[1];
-
 		dev_dbg(ipu_plane->base.dev->dev,
 			"phys = %pad %pad %pad, x = %d, y = %d",
 			&cma_obj[0]->paddr, &cma_obj[1]->paddr,
@@ -151,164 +163,111 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb,
 		break;
 	}
 
-	if (ipu_plane->enabled) {
-		active = ipu_idmac_get_current_buffer(ipu_plane->ipu_ch);
-		ipu_cpmem_set_buffer(ipu_plane->ipu_ch, !active, eba);
-		ipu_idmac_select_buffer(ipu_plane->ipu_ch, !active);
-	} else {
-		ipu_cpmem_set_buffer(ipu_plane->ipu_ch, 0, eba);
-		ipu_cpmem_set_buffer(ipu_plane->ipu_ch, 1, eba);
-	}
-
-	/* cache offsets for subsequent pageflips */
-	ipu_plane->x = x;
-	ipu_plane->y = y;
+	active = ipu_idmac_get_current_buffer(ipu_plane->ipu_ch);
+	ipu_cpmem_set_buffer(ipu_plane->ipu_ch, !active, eba);
+	ipu_idmac_select_buffer(ipu_plane->ipu_ch, !active);
 
 	return 0;
 }
 
-int ipu_plane_mode_set(struct ipu_plane *ipu_plane, struct drm_crtc *crtc,
-		       struct drm_display_mode *mode,
-		       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, bool interlaced)
+static inline unsigned long
+drm_plane_state_to_eba(struct drm_plane_state *state)
 {
-	struct device *dev = ipu_plane->base.dev->dev;
-	int ret;
+	struct drm_framebuffer *fb = state->fb;
+	struct drm_gem_cma_object *cma_obj;
 
-	/* no scaling */
-	if (src_w != crtc_w || src_h != crtc_h)
-		return -EINVAL;
+	cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
+	BUG_ON(!cma_obj);
 
-	if (ipu_plane->base.type == DRM_PLANE_TYPE_PRIMARY) {
-		/* full plane doesn't support partial off screen */
-		if (crtc_x || crtc_y || crtc_w != mode->hdisplay ||
-			crtc_h != mode->vdisplay)
-			return -EINVAL;
+	return cma_obj->paddr + fb->offsets[0] +
+	       fb->pitches[0] * (state->src_y >> 16) +
+	       (fb->bits_per_pixel >> 3) * (state->src_x >> 16);
+}
 
-		/* full plane minimum width is 13 pixels */
-		if (crtc_w < 13)
-			return -EINVAL;
-	} else if (ipu_plane->base.type == DRM_PLANE_TYPE_OVERLAY) {
-		/* clip to crtc bounds */
-		if (crtc_x < 0) {
-			if (-crtc_x > crtc_w)
-				return -EINVAL;
-			src_x += -crtc_x;
-			src_w -= -crtc_x;
-			crtc_w -= -crtc_x;
-			crtc_x = 0;
-		}
-		if (crtc_y < 0) {
-			if (-crtc_y > crtc_h)
-				return -EINVAL;
-			src_y += -crtc_y;
-			src_h -= -crtc_y;
-			crtc_h -= -crtc_y;
-			crtc_y = 0;
-		}
-		if (crtc_x + crtc_w > mode->hdisplay) {
-			if (crtc_x > mode->hdisplay)
-				return -EINVAL;
-			crtc_w = mode->hdisplay - crtc_x;
-			src_w = crtc_w;
-		}
-		if (crtc_y + crtc_h > mode->vdisplay) {
-			if (crtc_y > mode->vdisplay)
-				return -EINVAL;
-			crtc_h = mode->vdisplay - crtc_y;
-			src_h = crtc_h;
-		}
-	} else
-		return -EINVAL;
-	if (crtc_h < 2)
-		return -EINVAL;
+static inline unsigned long
+drm_plane_state_to_ubo(struct drm_plane_state *state)
+{
+	struct drm_framebuffer *fb = state->fb;
+	struct drm_gem_cma_object *cma_obj;
+	unsigned long eba = drm_plane_state_to_eba(state);
 
-	/*
-	 * since we cannot touch active IDMAC channels, we do not support
-	 * resizing the enabled plane or changing its format
-	 */
-	if (ipu_plane->enabled) {
-		if (src_w != ipu_plane->w || src_h != ipu_plane->h ||
-		    fb->pixel_format != ipu_plane->base.fb->pixel_format)
-			return -EINVAL;
+	cma_obj = drm_fb_cma_get_gem_obj(fb, 1);
+	BUG_ON(!cma_obj);
 
-		return ipu_plane_set_base(ipu_plane, fb, src_x, src_y);
-	}
+	return cma_obj->paddr + fb->offsets[1] +
+	       fb->pitches[1] * (state->src_y >> 16) / 2 +
+	       (state->src_x >> 16) / 2 - eba;
+}
 
-	switch (ipu_plane->dp_flow) {
-	case IPU_DP_FLOW_SYNC_BG:
-		ret = ipu_dp_setup_channel(ipu_plane->dp,
-				IPUV3_COLORSPACE_RGB,
-				IPUV3_COLORSPACE_RGB);
-		if (ret) {
-			dev_err(dev,
-				"initializing display processor failed with %d\n",
-				ret);
-			return ret;
-		}
-		ipu_dp_set_global_alpha(ipu_plane->dp, true, 0, true);
-		break;
-	case IPU_DP_FLOW_SYNC_FG:
-		ipu_dp_setup_channel(ipu_plane->dp,
-				ipu_drm_fourcc_to_colorspace(fb->pixel_format),
-				IPUV3_COLORSPACE_UNKNOWN);
-		ipu_dp_set_window_pos(ipu_plane->dp, crtc_x, crtc_y);
-		/* Enable local alpha on partial plane */
-		switch (fb->pixel_format) {
-		case DRM_FORMAT_ARGB1555:
-		case DRM_FORMAT_ABGR1555:
-		case DRM_FORMAT_RGBA5551:
-		case DRM_FORMAT_BGRA5551:
-		case DRM_FORMAT_ARGB4444:
-		case DRM_FORMAT_ARGB8888:
-		case DRM_FORMAT_ABGR8888:
-		case DRM_FORMAT_RGBA8888:
-		case DRM_FORMAT_BGRA8888:
-			ipu_dp_set_global_alpha(ipu_plane->dp, false, 0, false);
-			break;
-		default:
+static inline unsigned long
+drm_plane_state_to_vbo(struct drm_plane_state *state)
+{
+	struct drm_framebuffer *fb = state->fb;
+	struct drm_gem_cma_object *cma_obj;
+	unsigned long eba = drm_plane_state_to_eba(state);
+
+	cma_obj = drm_fb_cma_get_gem_obj(fb, 2);
+	BUG_ON(!cma_obj);
+
+	return cma_obj->paddr + fb->offsets[2] +
+	       fb->pitches[2] * (state->src_y >> 16) / 2 +
+	       (state->src_x >> 16) / 2 - eba;
+}
+
+void ipu_plane_atomic_set_base(struct ipu_plane *ipu_plane,
+			       struct drm_plane_state *old_state)
+{
+	struct drm_plane *plane = &ipu_plane->base;
+	struct drm_plane_state *state = plane->state;
+	struct drm_framebuffer *fb = state->fb;
+	unsigned long eba, ubo, vbo;
+	int active;
+
+	eba = drm_plane_state_to_eba(state);
+
+	switch (fb->pixel_format) {
+	case DRM_FORMAT_YUV420:
+	case DRM_FORMAT_YVU420:
+		if (old_state->fb)
 			break;
-		}
-	}
 
-	ipu_dmfc_config_wait4eot(ipu_plane->dmfc, crtc_w);
+		/*
+		 * Multiplanar formats have to meet the following restrictions:
+		 * - The (up to) three plane addresses are EBA, EBA+UBO, EBA+VBO
+		 * - EBA, UBO and VBO are a multiple of 8
+		 * - UBO and VBO are unsigned and not larger than 0xfffff8
+		 * - Only EBA may be changed while scanout is active
+		 * - The strides of U and V planes must be identical.
+		 */
+		ubo = drm_plane_state_to_ubo(state);
+		vbo = drm_plane_state_to_vbo(state);
 
-	ipu_cpmem_zero(ipu_plane->ipu_ch);
-	ipu_cpmem_set_resolution(ipu_plane->ipu_ch, src_w, src_h);
-	ret = ipu_cpmem_set_fmt(ipu_plane->ipu_ch, fb->pixel_format);
-	if (ret < 0) {
-		dev_err(dev, "unsupported pixel format 0x%08x\n",
-			fb->pixel_format);
-		return ret;
-	}
-	ipu_cpmem_set_high_priority(ipu_plane->ipu_ch);
-	ipu_idmac_set_double_buffer(ipu_plane->ipu_ch, 1);
-	ipu_cpmem_set_stride(ipu_plane->ipu_ch, fb->pitches[0]);
+		if (fb->pixel_format == DRM_FORMAT_YUV420)
+			ipu_cpmem_set_yuv_planar_full(ipu_plane->ipu_ch,
+						      fb->pitches[1], ubo, vbo);
+		else
+			ipu_cpmem_set_yuv_planar_full(ipu_plane->ipu_ch,
+						      fb->pitches[1], vbo, ubo);
 
-	ret = ipu_plane_set_base(ipu_plane, fb, src_x, src_y);
-	if (ret < 0)
-		return ret;
-	if (interlaced)
-		ipu_cpmem_interlaced_scan(ipu_plane->ipu_ch, fb->pitches[0]);
-
-	if (fb->pixel_format == DRM_FORMAT_YUV420) {
-		ipu_cpmem_set_yuv_planar_full(ipu_plane->ipu_ch,
-					      ipu_plane->stride[1],
-					      ipu_plane->u_offset,
-					      ipu_plane->v_offset);
-	} else if (fb->pixel_format == DRM_FORMAT_YVU420) {
-		ipu_cpmem_set_yuv_planar_full(ipu_plane->ipu_ch,
-					      ipu_plane->stride[1],
-					      ipu_plane->v_offset,
-					      ipu_plane->u_offset);
-	}
+		dev_dbg(ipu_plane->base.dev->dev,
+			"phy = %lu %lu %lu, x = %d, y = %d", eba, ubo, vbo,
+			state->src_x >> 16, state->src_y >> 16);
+		break;
+	default:
+		dev_dbg(ipu_plane->base.dev->dev, "phys = %lu, x = %d, y = %d",
+			eba, state->src_x >> 16, state->src_y >> 16);
 
-	ipu_plane->w = src_w;
-	ipu_plane->h = src_h;
+		break;
+	}
 
-	return 0;
+	if (old_state->fb) {
+		active = ipu_idmac_get_current_buffer(ipu_plane->ipu_ch);
+		ipu_cpmem_set_buffer(ipu_plane->ipu_ch, !active, eba);
+		ipu_idmac_select_buffer(ipu_plane->ipu_ch, !active);
+	} else {
+		ipu_cpmem_set_buffer(ipu_plane->ipu_ch, 0, eba);
+		ipu_cpmem_set_buffer(ipu_plane->ipu_ch, 1, eba);
+	}
 }
 
 void ipu_plane_put_resources(struct ipu_plane *ipu_plane)
@@ -355,7 +314,7 @@ err_out:
 	return ret;
 }
 
-void ipu_plane_enable(struct ipu_plane *ipu_plane)
+static void ipu_plane_enable(struct ipu_plane *ipu_plane)
 {
 	if (ipu_plane->dp)
 		ipu_dp_enable(ipu_plane->ipu);
@@ -363,14 +322,10 @@ void ipu_plane_enable(struct ipu_plane *ipu_plane)
 	ipu_idmac_enable_channel(ipu_plane->ipu_ch);
 	if (ipu_plane->dp)
 		ipu_dp_enable_channel(ipu_plane->dp);
-
-	ipu_plane->enabled = true;
 }
 
-void ipu_plane_disable(struct ipu_plane *ipu_plane)
+static void ipu_plane_disable(struct ipu_plane *ipu_plane)
 {
-	ipu_plane->enabled = false;
-
 	ipu_idmac_wait_busy(ipu_plane->ipu_ch, 50);
 
 	if (ipu_plane->dp)
@@ -381,74 +336,222 @@ void ipu_plane_disable(struct ipu_plane *ipu_plane)
 		ipu_dp_disable(ipu_plane->ipu);
 }
 
-/*
- * drm_plane API
- */
-
-static int ipu_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)
+static int ipu_disable_plane(struct drm_plane *plane)
 {
 	struct ipu_plane *ipu_plane = to_ipu_plane(plane);
-	int ret = 0;
-
-	DRM_DEBUG_KMS("plane - %p\n", plane);
-
-	if (!ipu_plane->enabled)
-		ret = ipu_plane_get_resources(ipu_plane);
-	if (ret < 0)
-		return ret;
-
-	ret = ipu_plane_mode_set(ipu_plane, crtc, &crtc->hwmode, fb,
-			crtc_x, crtc_y, crtc_w, crtc_h,
-			src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
-			false);
-	if (ret < 0) {
-		ipu_plane_put_resources(ipu_plane);
-		return ret;
-	}
 
-	if (crtc != plane->crtc)
-		dev_dbg(plane->dev->dev, "crtc change: %p -> %p\n",
-				plane->crtc, crtc);
+	DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
 
-	if (!ipu_plane->enabled)
-		ipu_plane_enable(ipu_plane);
+	ipu_plane_disable(ipu_plane);
 
 	return 0;
 }
 
-static int ipu_disable_plane(struct drm_plane *plane)
+static void ipu_plane_destroy(struct drm_plane *plane)
 {
 	struct ipu_plane *ipu_plane = to_ipu_plane(plane);
 
 	DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
 
-	if (ipu_plane->enabled)
-		ipu_plane_disable(ipu_plane);
+	ipu_disable_plane(plane);
+	drm_plane_cleanup(plane);
+	kfree(ipu_plane);
+}
 
-	ipu_plane_put_resources(ipu_plane);
+static const struct drm_plane_funcs ipu_plane_funcs = {
+	.update_plane	= drm_plane_helper_update,
+	.disable_plane	= drm_plane_helper_disable,
+	.destroy	= ipu_plane_destroy,
+};
+
+static int ipu_plane_atomic_check(struct drm_plane *plane,
+				  struct drm_plane_state *state)
+{
+	struct drm_plane_state *old_state = plane->state;
+	struct drm_crtc_state *crtc_state;
+	struct device *dev = plane->dev->dev;
+	struct drm_crtc *crtc;
+	struct drm_framebuffer *fb = state->fb;
+	struct drm_framebuffer *old_fb = old_state->fb;
+	unsigned long eba, ubo, vbo, old_ubo, old_vbo;
+
+	/* Ok to disable */
+	if (!fb)
+		return old_fb ? 0 : -EINVAL;
+
+	/* CRTC should be enabled */
+	drm_for_each_crtc(crtc, plane->dev) {
+		if (drm_crtc_mask(crtc) == plane->possible_crtcs) {
+			if (!crtc->enabled)
+				return -EINVAL;
+			break;
+		}
+	}
+
+	/* no scaling */
+	if (state->src_w >> 16 != state->crtc_w ||
+	    state->src_h >> 16 != state->crtc_h)
+		return -EINVAL;
+
+	crtc_state = state->crtc->state;
+
+	switch (plane->type) {
+	case DRM_PLANE_TYPE_PRIMARY:
+		/* full plane doesn't support partial off screen */
+		if (state->crtc_x || state->crtc_y ||
+		    state->crtc_w != crtc_state->adjusted_mode.hdisplay ||
+		    state->crtc_h != crtc_state->adjusted_mode.vdisplay)
+			return -EINVAL;
+
+		/* full plane minimum width is 13 pixels */
+		if (state->crtc_w < 13)
+			return -EINVAL;
+		break;
+	case DRM_PLANE_TYPE_OVERLAY:
+		if (state->crtc_x < 0 || state->crtc_y < 0)
+			return -EINVAL;
+
+		if (state->crtc_x + state->crtc_w >
+		    crtc_state->adjusted_mode.hdisplay)
+			return -EINVAL;
+		if (state->crtc_y + state->crtc_h >
+		    crtc_state->adjusted_mode.vdisplay)
+			return -EINVAL;
+		break;
+	default:
+		dev_warn(dev, "Unsupported plane type\n");
+		return -EINVAL;
+	}
+
+	if (state->crtc_h < 2)
+		return -EINVAL;
+
+	/*
+	 * since we cannot touch active IDMAC channels, we do not support
+	 * resizing the enabled plane or changing its format
+	 */
+	if (old_fb && (state->src_w != old_state->src_w ||
+			      state->src_h != old_state->src_h ||
+			      fb->pixel_format != old_fb->pixel_format))
+		return -EINVAL;
+
+	eba = drm_plane_state_to_eba(state);
+
+	if (eba & 0x7)
+		return -EINVAL;
+
+	if (fb->pitches[0] < 1 || fb->pitches[0] > 16384)
+		return -EINVAL;
+
+	if (old_fb && fb->pitches[0] != old_fb->pitches[0])
+		return -EINVAL;
+
+	switch (fb->pixel_format) {
+	case DRM_FORMAT_YUV420:
+	case DRM_FORMAT_YVU420:
+		/*
+		 * Multiplanar formats have to meet the following restrictions:
+		 * - The (up to) three plane addresses are EBA, EBA+UBO, EBA+VBO
+		 * - EBA, UBO and VBO are a multiple of 8
+		 * - UBO and VBO are unsigned and not larger than 0xfffff8
+		 * - Only EBA may be changed while scanout is active
+		 * - The strides of U and V planes must be identical.
+		 */
+		ubo = drm_plane_state_to_ubo(state);
+		vbo = drm_plane_state_to_vbo(state);
+
+		if ((ubo & 0x7) || (vbo & 0x7))
+			return -EINVAL;
+
+		if ((ubo > 0xfffff8) || (vbo > 0xfffff8))
+			return -EINVAL;
+
+		if (old_fb) {
+			old_ubo = drm_plane_state_to_ubo(old_state);
+			old_vbo = drm_plane_state_to_vbo(old_state);
+			if (ubo != old_ubo || vbo != old_vbo)
+				return -EINVAL;
+		}
+
+		if (fb->pitches[1] != fb->pitches[2])
+			return -EINVAL;
+
+		if (fb->pitches[1] < 1 || fb->pitches[1] > 16384)
+			return -EINVAL;
+
+		if (old_fb && old_fb->pitches[1] != fb->pitches[1])
+			return -EINVAL;
+	}
 
 	return 0;
 }
 
-static void ipu_plane_destroy(struct drm_plane *plane)
+static void ipu_plane_atomic_disable(struct drm_plane *plane,
+				     struct drm_plane_state *old_state)
+{
+	ipu_disable_plane(plane);
+}
+
+static void ipu_plane_atomic_update(struct drm_plane *plane,
+				    struct drm_plane_state *old_state)
 {
 	struct ipu_plane *ipu_plane = to_ipu_plane(plane);
+	struct drm_plane_state *state = plane->state;
+	enum ipu_color_space ics;
 
-	DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
+	if (old_state->fb) {
+		ipu_plane_atomic_set_base(ipu_plane, old_state);
+		return;
+	}
 
-	ipu_disable_plane(plane);
-	drm_plane_cleanup(plane);
-	kfree(ipu_plane);
+	switch (ipu_plane->dp_flow) {
+	case IPU_DP_FLOW_SYNC_BG:
+		ipu_dp_setup_channel(ipu_plane->dp,
+					IPUV3_COLORSPACE_RGB,
+					IPUV3_COLORSPACE_RGB);
+		ipu_dp_set_global_alpha(ipu_plane->dp, true, 0, true);
+		break;
+	case IPU_DP_FLOW_SYNC_FG:
+		ics = ipu_drm_fourcc_to_colorspace(state->fb->pixel_format);
+		ipu_dp_setup_channel(ipu_plane->dp, ics,
+					IPUV3_COLORSPACE_UNKNOWN);
+		ipu_dp_set_window_pos(ipu_plane->dp, state->crtc_x,
+					state->crtc_y);
+		/* Enable local alpha on partial plane */
+		switch (state->fb->pixel_format) {
+		case DRM_FORMAT_ARGB1555:
+		case DRM_FORMAT_ABGR1555:
+		case DRM_FORMAT_RGBA5551:
+		case DRM_FORMAT_BGRA5551:
+		case DRM_FORMAT_ARGB4444:
+		case DRM_FORMAT_ARGB8888:
+		case DRM_FORMAT_ABGR8888:
+		case DRM_FORMAT_RGBA8888:
+		case DRM_FORMAT_BGRA8888:
+			ipu_dp_set_global_alpha(ipu_plane->dp, false, 0, false);
+			break;
+		default:
+			break;
+		}
+	}
+
+	ipu_dmfc_config_wait4eot(ipu_plane->dmfc, state->crtc_w);
+
+	ipu_cpmem_zero(ipu_plane->ipu_ch);
+	ipu_cpmem_set_resolution(ipu_plane->ipu_ch, state->src_w >> 16,
+					state->src_h >> 16);
+	ipu_cpmem_set_fmt(ipu_plane->ipu_ch, state->fb->pixel_format);
+	ipu_cpmem_set_high_priority(ipu_plane->ipu_ch);
+	ipu_idmac_set_double_buffer(ipu_plane->ipu_ch, 1);
+	ipu_cpmem_set_stride(ipu_plane->ipu_ch, state->fb->pitches[0]);
+	ipu_plane_atomic_set_base(ipu_plane, old_state);
+	ipu_plane_enable(ipu_plane);
 }
 
-static const struct drm_plane_funcs ipu_plane_funcs = {
-	.update_plane	= ipu_update_plane,
-	.disable_plane	= ipu_disable_plane,
-	.destroy	= ipu_plane_destroy,
+static const struct drm_plane_helper_funcs ipu_plane_helper_funcs = {
+	.atomic_check = ipu_plane_atomic_check,
+	.atomic_disable = ipu_plane_atomic_disable,
+	.atomic_update = ipu_plane_atomic_update,
 };
 
 struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu,
@@ -481,5 +584,7 @@ struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu,
 		return ERR_PTR(ret);
 	}
 
+	drm_plane_helper_add(&ipu_plane->base, &ipu_plane_helper_funcs);
+
 	return ipu_plane;
 }
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.h b/drivers/gpu/drm/imx/ipuv3-plane.h
index 4448fd4..c51a44b 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.h
+++ b/drivers/gpu/drm/imx/ipuv3-plane.h
@@ -23,17 +23,6 @@ struct ipu_plane {
 
 	int			dma;
 	int			dp_flow;
-
-	int			x;
-	int			y;
-	int			w;
-	int			h;
-
-	unsigned int		u_offset;
-	unsigned int		v_offset;
-	unsigned int		stride[2];
-
-	bool			enabled;
 };
 
 struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu,
@@ -48,10 +37,7 @@ int ipu_plane_mode_set(struct ipu_plane *plane, struct drm_crtc *crtc,
 		       uint32_t src_x, uint32_t src_y, uint32_t src_w,
 		       uint32_t src_h, bool interlaced);
 
-void ipu_plane_enable(struct ipu_plane *plane);
-void ipu_plane_disable(struct ipu_plane *plane);
-int ipu_plane_set_base(struct ipu_plane *plane, struct drm_framebuffer *fb,
-		       int x, int y);
+int ipu_plane_set_base(struct ipu_plane *plane, struct drm_framebuffer *fb);
 
 int ipu_plane_get_resources(struct ipu_plane *plane);
 void ipu_plane_put_resources(struct ipu_plane *plane);
diff --git a/drivers/gpu/ipu-v3/ipu-dc.c b/drivers/gpu/ipu-v3/ipu-dc.c
index 2f29780..cd72dad 100644
--- a/drivers/gpu/ipu-v3/ipu-dc.c
+++ b/drivers/gpu/ipu-v3/ipu-dc.c
@@ -178,10 +178,7 @@ int ipu_dc_init_sync(struct ipu_dc *dc, struct ipu_di *di, bool interlaced,
 	dc->di = ipu_di_get_num(di);
 
 	map = ipu_bus_format_to_map(bus_format);
-	if (map < 0) {
-		dev_dbg(priv->dev, "IPU_DISP: No MAP\n");
-		return map;
-	}
+	BUG_ON(map < 0);
 
 	/*
 	 * In interlaced mode we need more counters to create the asymmetric
diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c
index 359268e..a8d87dd 100644
--- a/drivers/gpu/ipu-v3/ipu-di.c
+++ b/drivers/gpu/ipu-v3/ipu-di.c
@@ -572,9 +572,6 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
 	dev_dbg(di->ipu->dev, "disp %d: panel size = %d x %d\n",
 		di->id, sig->mode.hactive, sig->mode.vactive);
 
-	if ((sig->mode.vsync_len == 0) || (sig->mode.hsync_len == 0))
-		return -EINVAL;
-
 	dev_dbg(di->ipu->dev, "Clocks: IPU %luHz DI %luHz Needed %luHz\n",
 		clk_get_rate(di->clk_ipu),
 		clk_get_rate(di->clk_di),
-- 
2.7.4

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

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

* [PATCH v2 04/10] drm/imx: atomic phase 2 step 1: Wire up state ->reset, ->duplicate and ->destroy
  2016-05-31  9:24 [PATCH v2 00/10] imx drm atomic mode setting conversion Liu Ying
                   ` (2 preceding siblings ...)
  2016-05-31  9:24 ` [PATCH v2 03/10] drm/imx: atomic phase 1: Use transitional atomic CRTC and plane helpers Liu Ying
@ 2016-05-31  9:24 ` Liu Ying
  2016-05-31  9:24 ` [PATCH v2 05/10] drm/imx: atomic phase 2 step 2: Track plane_state->fb correctly in ->page_flip Liu Ying
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Liu Ying @ 2016-05-31  9:24 UTC (permalink / raw)
  To: dri-devel; +Cc: Russell King, Daniel Vetter

Wire up CRTCs', planes' and connectors' ->reset, ->duplicate and ->destroy state
hooks to use the default implementations from the atomic helper library.
The helpers track each DRM object state.

Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v1->v2:
* Remove the 'atomic' from the name of the structure
  dw_hdmi_atomic_connector_funcs to address Philipp's comment.

 drivers/gpu/drm/bridge/dw-hdmi.c       | 19 +++----------------
 drivers/gpu/drm/imx/imx-drm-core.c     |  2 ++
 drivers/gpu/drm/imx/imx-ldb.c          |  4 ++++
 drivers/gpu/drm/imx/imx-tve.c          |  4 ++++
 drivers/gpu/drm/imx/ipuv3-crtc.c       |  4 ++++
 drivers/gpu/drm/imx/ipuv3-plane.c      |  4 ++++
 drivers/gpu/drm/imx/parallel-display.c |  4 ++++
 7 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index c9d9412..3d37e8f 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1504,14 +1504,6 @@ static void dw_hdmi_connector_force(struct drm_connector *connector)
 }
 
 static const struct drm_connector_funcs dw_hdmi_connector_funcs = {
-	.dpms = drm_helper_connector_dpms,
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.detect = dw_hdmi_connector_detect,
-	.destroy = dw_hdmi_connector_destroy,
-	.force = dw_hdmi_connector_force,
-};
-
-static const struct drm_connector_funcs dw_hdmi_atomic_connector_funcs = {
 	.dpms = drm_atomic_helper_connector_dpms,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.detect = dw_hdmi_connector_detect,
@@ -1643,14 +1635,9 @@ static int dw_hdmi_register(struct drm_device *drm, struct dw_hdmi *hdmi)
 	drm_connector_helper_add(&hdmi->connector,
 				 &dw_hdmi_connector_helper_funcs);
 
-	if (drm_core_check_feature(drm, DRIVER_ATOMIC))
-		drm_connector_init(drm, &hdmi->connector,
-				   &dw_hdmi_atomic_connector_funcs,
-				   DRM_MODE_CONNECTOR_HDMIA);
-	else
-		drm_connector_init(drm, &hdmi->connector,
-				   &dw_hdmi_connector_funcs,
-				   DRM_MODE_CONNECTOR_HDMIA);
+	drm_connector_init(drm, &hdmi->connector,
+			   &dw_hdmi_connector_funcs,
+			   DRM_MODE_CONNECTOR_HDMIA);
 
 	drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
 
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 8265665..3c83590 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -279,6 +279,8 @@ static int imx_drm_driver_load(struct drm_device *drm, unsigned long flags)
 		}
 	}
 
+	drm_mode_config_reset(drm);
+
 	/*
 	 * All components are now initialised, so setup the fb helper.
 	 * The fb helper takes copies of key hardware information, so the
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index beff793..12bf368 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -17,6 +17,7 @@
 #include <linux/clk.h>
 #include <linux/component.h>
 #include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_of.h>
@@ -362,6 +363,9 @@ static const struct drm_connector_funcs imx_ldb_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.detect = imx_ldb_connector_detect,
 	.destroy = imx_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,
 };
 
 static const struct drm_connector_helper_funcs imx_ldb_connector_helper_funcs = {
diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c
index baf7881..0b0aeee 100644
--- a/drivers/gpu/drm/imx/imx-tve.c
+++ b/drivers/gpu/drm/imx/imx-tve.c
@@ -23,6 +23,7 @@
 #include <linux/spinlock.h>
 #include <linux/videodev2.h>
 #include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <video/imx-ipu-v3.h>
@@ -360,6 +361,9 @@ static const struct drm_connector_funcs imx_tve_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.detect = imx_tve_connector_detect,
 	.destroy = imx_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,
 };
 
 static const struct drm_connector_helper_funcs imx_tve_connector_helper_funcs = {
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index ba880fa..8a0ef13 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -18,6 +18,7 @@
 #include <linux/device.h>
 #include <linux/platform_device.h>
 #include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <linux/fb.h>
 #include <linux/clk.h>
@@ -232,6 +233,9 @@ static const struct drm_crtc_funcs ipu_crtc_funcs = {
 	.set_config = drm_crtc_helper_set_config,
 	.destroy = drm_crtc_cleanup,
 	.page_flip = ipu_page_flip,
+	.reset = drm_atomic_helper_crtc_reset,
+	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
 };
 
 static void ipu_crtc_handle_pageflip(struct ipu_crtc *ipu_crtc)
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 9b9260b..a9e6da9 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -14,6 +14,7 @@
  */
 
 #include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_plane_helper.h>
@@ -362,6 +363,9 @@ static const struct drm_plane_funcs ipu_plane_funcs = {
 	.update_plane	= drm_plane_helper_update,
 	.disable_plane	= drm_plane_helper_disable,
 	.destroy	= ipu_plane_destroy,
+	.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 ipu_plane_atomic_check(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
index 2d1fd02..9fe88c9 100644
--- a/drivers/gpu/drm/imx/parallel-display.c
+++ b/drivers/gpu/drm/imx/parallel-display.c
@@ -16,6 +16,7 @@
 #include <linux/component.h>
 #include <linux/module.h>
 #include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_panel.h>
@@ -134,6 +135,9 @@ static const struct drm_connector_funcs imx_pd_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.detect = imx_pd_connector_detect,
 	.destroy = imx_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,
 };
 
 static const struct drm_connector_helper_funcs imx_pd_connector_helper_funcs = {
-- 
2.7.4

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

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

* [PATCH v2 05/10] drm/imx: atomic phase 2 step 2: Track plane_state->fb correctly in ->page_flip
  2016-05-31  9:24 [PATCH v2 00/10] imx drm atomic mode setting conversion Liu Ying
                   ` (3 preceding siblings ...)
  2016-05-31  9:24 ` [PATCH v2 04/10] drm/imx: atomic phase 2 step 1: Wire up state ->reset, ->duplicate and ->destroy Liu Ying
@ 2016-05-31  9:24 ` Liu Ying
  2016-05-31  9:24 ` [PATCH v2 06/10] drm/imx: atomic phase 3 step 1: Atomic updates for planes Liu Ying
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Liu Ying @ 2016-05-31  9:24 UTC (permalink / raw)
  To: dri-devel; +Cc: Russell King, Daniel Vetter

Use drm_atomic_set_fb_for_plane() in the legacy ->page_flip path to track
the pointer plane_state->fb correctly.

Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v1->v2:
* None.

 drivers/gpu/drm/imx/ipuv3-crtc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index 8a0ef13..7df51e8 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -18,6 +18,7 @@
 #include <linux/device.h>
 #include <linux/platform_device.h>
 #include <drm/drmP.h>
+#include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <linux/fb.h>
@@ -217,6 +218,9 @@ static int ipu_page_flip(struct drm_crtc *crtc,
 		ipu_crtc->flip_state = IPU_FLIP_SUBMITTED;
 	}
 
+	if (crtc->primary->state)
+		drm_atomic_set_fb_for_plane(crtc->primary->state, fb);
+
 	return 0;
 
 free_flip_work:
-- 
2.7.4

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

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

* [PATCH v2 06/10] drm/imx: atomic phase 3 step 1: Atomic updates for planes
  2016-05-31  9:24 [PATCH v2 00/10] imx drm atomic mode setting conversion Liu Ying
                   ` (4 preceding siblings ...)
  2016-05-31  9:24 ` [PATCH v2 05/10] drm/imx: atomic phase 2 step 2: Track plane_state->fb correctly in ->page_flip Liu Ying
@ 2016-05-31  9:24 ` Liu Ying
  2016-05-31  9:24 ` [PATCH v2 07/10] drm/imx: atomic phase 3 step 2: Use atomic configuration Liu Ying
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Liu Ying @ 2016-05-31  9:24 UTC (permalink / raw)
  To: dri-devel; +Cc: Russell King, Daniel Vetter

This patch switches the update/disable_plane callbacks to their atomic version.
Also, use the default atomic helpers to implement the atomic_check/commit
callbacks for mode configuration.

Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v1->v2:
* None.

 drivers/gpu/drm/imx/imx-drm-core.c | 3 +++
 drivers/gpu/drm/imx/ipuv3-plane.c  | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 3c83590..5893cbc 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -19,6 +19,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_gem_cma_helper.h>
@@ -208,6 +209,8 @@ static void imx_drm_output_poll_changed(struct drm_device *drm)
 static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = {
 	.fb_create = drm_fb_cma_create,
 	.output_poll_changed = imx_drm_output_poll_changed,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
 };
 
 /*
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index a9e6da9..fc1ff7c 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -360,8 +360,8 @@ static void ipu_plane_destroy(struct drm_plane *plane)
 }
 
 static const struct drm_plane_funcs ipu_plane_funcs = {
-	.update_plane	= drm_plane_helper_update,
-	.disable_plane	= drm_plane_helper_disable,
+	.update_plane	= drm_atomic_helper_update_plane,
+	.disable_plane	= drm_atomic_helper_disable_plane,
 	.destroy	= ipu_plane_destroy,
 	.reset		= drm_atomic_helper_plane_reset,
 	.atomic_duplicate_state	= drm_atomic_helper_plane_duplicate_state,
-- 
2.7.4

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

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

* [PATCH v2 07/10] drm/imx: atomic phase 3 step 2: Use atomic configuration
  2016-05-31  9:24 [PATCH v2 00/10] imx drm atomic mode setting conversion Liu Ying
                   ` (5 preceding siblings ...)
  2016-05-31  9:24 ` [PATCH v2 06/10] drm/imx: atomic phase 3 step 1: Atomic updates for planes Liu Ying
@ 2016-05-31  9:24 ` Liu Ying
  2016-05-31  9:24 ` [PATCH v2 08/10] drm/imx: atomic phase 3 step 3: Legacy callback fixups Liu Ying
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Liu Ying @ 2016-05-31  9:24 UTC (permalink / raw)
  To: dri-devel; +Cc: Russell King, Daniel Vetter

Replacing drm_crtc_helper_set_config() by drm_atomic_helper_set_config()
and converting the suspend/resume operations to atomic makes us be able
to use atomic configurations.  All of these allow us to remove the
crtc_funcs->mode_set callback as it is no longer used.  Also, we may remove
all the encoders' ->prepare callbacks as they can be replaced by ->disable.
In consequence, the bus_format, bus_flags, di_vsync_pin and di_hsync_pin
settings are moved from ->prepare to structure imx_encoder.  Furthermore,
switching to the pure atomic version of set_config callback means that we
may implement CRTC/plane atomic checks by using the new CRTC/plane states
instead of the legacy ones and we may remove the private ipu_crtc->enabled
state which was left there for the transitional atomic helpers in phase 1.
Last, the legacy function drm_helper_disable_unused_functions() is removed
from ->load in order not to confuse the atomic driver.

Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v1->v2:
* Handle the newly introduced bus_flags via imx_encoder after the rebasing.
* Remove the legacy function drm_helper_disable_unused_functions() from
  ->load in order not to confuse the atomic driver.

 drivers/gpu/drm/imx/dw_hdmi-imx.c      |  18 +++---
 drivers/gpu/drm/imx/imx-drm-core.c     |  53 ++++------------
 drivers/gpu/drm/imx/imx-drm.h          |  18 +++---
 drivers/gpu/drm/imx/imx-ldb.c          | 109 +++++++++++++++------------------
 drivers/gpu/drm/imx/imx-tve.c          |  58 +++++++-----------
 drivers/gpu/drm/imx/ipuv3-crtc.c       |  69 ++++++---------------
 drivers/gpu/drm/imx/ipuv3-plane.c      |  23 +++----
 drivers/gpu/drm/imx/parallel-display.c |  56 +++++++++--------
 8 files changed, 163 insertions(+), 241 deletions(-)

diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c
index a24631fd..5f64674 100644
--- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
+++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
@@ -22,9 +22,11 @@
 
 #include "imx-drm.h"
 
+#define imx_enc_to_imx_hdmi(x) container_of(x, struct imx_hdmi, imx_encoder)
+
 struct imx_hdmi {
 	struct device *dev;
-	struct drm_encoder encoder;
+	struct imx_drm_encoder imx_encoder;
 	struct regmap *regmap;
 };
 
@@ -117,7 +119,8 @@ static void dw_hdmi_imx_encoder_mode_set(struct drm_encoder *encoder,
 
 static void dw_hdmi_imx_encoder_commit(struct drm_encoder *encoder)
 {
-	struct imx_hdmi *hdmi = container_of(encoder, struct imx_hdmi, encoder);
+	struct imx_drm_encoder *imx_encoder = enc_to_imx_enc(encoder);
+	struct imx_hdmi *hdmi = imx_enc_to_imx_hdmi(imx_encoder);
 	int mux = drm_of_encoder_active_port_id(hdmi->dev->of_node, encoder);
 
 	regmap_update_bits(hdmi->regmap, IOMUXC_GPR3,
@@ -125,14 +128,8 @@ static void dw_hdmi_imx_encoder_commit(struct drm_encoder *encoder)
 			   mux << IMX6Q_GPR3_HDMI_MUX_CTL_SHIFT);
 }
 
-static void dw_hdmi_imx_encoder_prepare(struct drm_encoder *encoder)
-{
-	imx_drm_set_bus_format(encoder, MEDIA_BUS_FMT_RGB888_1X24);
-}
-
 static const struct drm_encoder_helper_funcs dw_hdmi_imx_encoder_helper_funcs = {
 	.mode_set   = dw_hdmi_imx_encoder_mode_set,
-	.prepare    = dw_hdmi_imx_encoder_prepare,
 	.commit     = dw_hdmi_imx_encoder_commit,
 	.disable    = dw_hdmi_imx_encoder_disable,
 };
@@ -215,7 +212,10 @@ static int dw_hdmi_imx_bind(struct device *dev, struct device *master,
 	match = of_match_node(dw_hdmi_imx_dt_ids, pdev->dev.of_node);
 	plat_data = match->data;
 	hdmi->dev = &pdev->dev;
-	encoder = &hdmi->encoder;
+	encoder = &hdmi->imx_encoder.encoder;
+	hdmi->imx_encoder.bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+	hdmi->imx_encoder.di_hsync_pin = 2;
+	hdmi->imx_encoder.di_vsync_pin = 3;
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 5893cbc..2a2ab8c 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -42,6 +42,7 @@ struct imx_drm_device {
 	struct imx_drm_crtc			*crtc[MAX_CRTC];
 	unsigned int				pipes;
 	struct drm_fbdev_cma			*fbhelper;
+	struct drm_atomic_state			*state;
 };
 
 struct imx_drm_crtc {
@@ -86,45 +87,6 @@ static int imx_drm_driver_unload(struct drm_device *drm)
 	return 0;
 }
 
-static struct imx_drm_crtc *imx_drm_find_crtc(struct drm_crtc *crtc)
-{
-	struct imx_drm_device *imxdrm = crtc->dev->dev_private;
-	unsigned i;
-
-	for (i = 0; i < MAX_CRTC; i++)
-		if (imxdrm->crtc[i] && imxdrm->crtc[i]->crtc == crtc)
-			return imxdrm->crtc[i];
-
-	return NULL;
-}
-
-int imx_drm_set_bus_config(struct drm_encoder *encoder, u32 bus_format,
-		int hsync_pin, int vsync_pin, u32 bus_flags)
-{
-	struct imx_drm_crtc_helper_funcs *helper;
-	struct imx_drm_crtc *imx_crtc;
-
-	imx_crtc = imx_drm_find_crtc(encoder->crtc);
-	if (!imx_crtc)
-		return -EINVAL;
-
-	helper = &imx_crtc->imx_drm_helper_funcs;
-	if (helper->set_interface_pix_fmt)
-		return helper->set_interface_pix_fmt(encoder->crtc,
-					bus_format, hsync_pin, vsync_pin,
-					bus_flags);
-	return 0;
-}
-EXPORT_SYMBOL_GPL(imx_drm_set_bus_config);
-
-int imx_drm_set_bus_format(struct drm_encoder *encoder, u32 bus_format)
-{
-	return imx_drm_set_bus_config(encoder, bus_format, 2, 3,
-				      DRM_BUS_FLAG_DE_HIGH |
-				      DRM_BUS_FLAG_PIXDATA_NEGEDGE);
-}
-EXPORT_SYMBOL_GPL(imx_drm_set_bus_format);
-
 int imx_drm_crtc_vblank_get(struct imx_drm_crtc *imx_drm_crtc)
 {
 	return drm_crtc_vblank_get(imx_drm_crtc->crtc);
@@ -294,7 +256,6 @@ static int imx_drm_driver_load(struct drm_device *drm, unsigned long flags)
 		dev_warn(drm->dev, "Invalid legacyfb_depth.  Defaulting to 16bpp\n");
 		legacyfb_depth = 16;
 	}
-	drm_helper_disable_unused_functions(drm);
 	imxdrm->fbhelper = drm_fbdev_cma_init(drm, legacyfb_depth,
 				drm->mode_config.num_crtc, MAX_CRTC);
 	if (IS_ERR(imxdrm->fbhelper)) {
@@ -497,6 +458,7 @@ static int imx_drm_platform_remove(struct platform_device *pdev)
 static int imx_drm_suspend(struct device *dev)
 {
 	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct imx_drm_device *imxdrm;
 
 	/* The drm_dev is NULL before .load hook is called */
 	if (drm_dev == NULL)
@@ -504,17 +466,26 @@ static int imx_drm_suspend(struct device *dev)
 
 	drm_kms_helper_poll_disable(drm_dev);
 
+	imxdrm = drm_dev->dev_private;
+	imxdrm->state = drm_atomic_helper_suspend(drm_dev);
+	if (IS_ERR(imxdrm->state)) {
+		drm_kms_helper_poll_enable(drm_dev);
+		return PTR_ERR(imxdrm->state);
+	}
+
 	return 0;
 }
 
 static int imx_drm_resume(struct device *dev)
 {
 	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct imx_drm_device *imx_drm;
 
 	if (drm_dev == NULL)
 		return 0;
 
-	drm_helper_resume_force_mode(drm_dev);
+	imx_drm = drm_dev->dev_private;
+	drm_atomic_helper_resume(drm_dev, imx_drm->state);
 	drm_kms_helper_poll_enable(drm_dev);
 
 	return 0;
diff --git a/drivers/gpu/drm/imx/imx-drm.h b/drivers/gpu/drm/imx/imx-drm.h
index 74320a1..39cef15 100644
--- a/drivers/gpu/drm/imx/imx-drm.h
+++ b/drivers/gpu/drm/imx/imx-drm.h
@@ -15,12 +15,19 @@ struct platform_device;
 
 unsigned int imx_drm_crtc_id(struct imx_drm_crtc *crtc);
 
+struct imx_drm_encoder {
+	struct drm_encoder			encoder;
+	u32					bus_format;
+	u32					bus_flags;
+	int					di_hsync_pin;
+	int					di_vsync_pin;
+};
+
+#define enc_to_imx_enc(x) container_of(x, struct imx_drm_encoder, encoder)
+
 struct imx_drm_crtc_helper_funcs {
 	int (*enable_vblank)(struct drm_crtc *crtc);
 	void (*disable_vblank)(struct drm_crtc *crtc);
-	int (*set_interface_pix_fmt)(struct drm_crtc *crtc,
-			u32 bus_format, int hsync_pin, int vsync_pin,
-			u32 bus_flags);
 	const struct drm_crtc_helper_funcs *crtc_helper_funcs;
 	const struct drm_crtc_funcs *crtc_funcs;
 };
@@ -42,11 +49,6 @@ void imx_drm_mode_config_init(struct drm_device *drm);
 
 struct drm_gem_cma_object *imx_drm_fb_get_obj(struct drm_framebuffer *fb);
 
-int imx_drm_set_bus_config(struct drm_encoder *encoder, u32 bus_format,
-		int hsync_pin, int vsync_pin, u32 bus_flags);
-int imx_drm_set_bus_format(struct drm_encoder *encoder,
-		u32 bus_format);
-
 int imx_drm_encoder_parse_of(struct drm_device *drm,
 	struct drm_encoder *encoder, struct device_node *np);
 
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index 12bf368..4a98eaa 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -51,14 +51,15 @@
 #define LDB_BGREF_RMODE_INT		(1 << 15)
 
 #define con_to_imx_ldb_ch(x) container_of(x, struct imx_ldb_channel, connector)
-#define enc_to_imx_ldb_ch(x) container_of(x, struct imx_ldb_channel, encoder)
+#define imx_enc_to_imx_ldb_ch(x)	\
+			container_of(x, struct imx_ldb_channel, imx_encoder)
 
 struct imx_ldb;
 
 struct imx_ldb_channel {
 	struct imx_ldb *ldb;
 	struct drm_connector connector;
-	struct drm_encoder encoder;
+	struct imx_drm_encoder imx_encoder;
 	struct drm_panel *panel;
 	struct device_node *child;
 	struct i2c_adapter *ddc;
@@ -67,7 +68,6 @@ struct imx_ldb_channel {
 	int edid_len;
 	struct drm_display_mode mode;
 	int mode_valid;
-	int bus_format;
 };
 
 struct bus_mux {
@@ -104,8 +104,8 @@ static int imx_ldb_connector_get_modes(struct drm_connector *connector)
 		struct drm_display_info *di = &connector->display_info;
 
 		num_modes = imx_ldb_ch->panel->funcs->get_modes(imx_ldb_ch->panel);
-		if (!imx_ldb_ch->bus_format && di->num_bus_formats)
-			imx_ldb_ch->bus_format = di->bus_formats[0];
+		if (!imx_ldb_ch->imx_encoder.bus_format && di->num_bus_formats)
+			imx_ldb_ch->imx_encoder.bus_format = di->bus_formats[0];
 		if (num_modes > 0)
 			return num_modes;
 	}
@@ -139,7 +139,7 @@ static struct drm_encoder *imx_ldb_connector_best_encoder(
 {
 	struct imx_ldb_channel *imx_ldb_ch = con_to_imx_ldb_ch(connector);
 
-	return &imx_ldb_ch->encoder;
+	return &imx_ldb_ch->imx_encoder.encoder;
 }
 
 static void imx_ldb_encoder_dpms(struct drm_encoder *encoder, int mode)
@@ -174,45 +174,10 @@ static void imx_ldb_set_clock(struct imx_ldb *ldb, int mux, int chno,
 			chno);
 }
 
-static void imx_ldb_encoder_prepare(struct drm_encoder *encoder)
-{
-	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
-	struct imx_ldb *ldb = imx_ldb_ch->ldb;
-	int dual = ldb->ldb_ctrl & LDB_SPLIT_MODE_EN;
-	u32 bus_format;
-
-	switch (imx_ldb_ch->bus_format) {
-	default:
-		dev_warn(ldb->dev,
-			 "could not determine data mapping, default to 18-bit \"spwg\"\n");
-		/* fallthrough */
-	case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
-		bus_format = MEDIA_BUS_FMT_RGB666_1X18;
-		break;
-	case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
-		bus_format = MEDIA_BUS_FMT_RGB888_1X24;
-		if (imx_ldb_ch->chno == 0 || dual)
-			ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24;
-		if (imx_ldb_ch->chno == 1 || dual)
-			ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24;
-		break;
-	case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
-		bus_format = MEDIA_BUS_FMT_RGB888_1X24;
-		if (imx_ldb_ch->chno == 0 || dual)
-			ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24 |
-					 LDB_BIT_MAP_CH0_JEIDA;
-		if (imx_ldb_ch->chno == 1 || dual)
-			ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24 |
-					 LDB_BIT_MAP_CH1_JEIDA;
-		break;
-	}
-
-	imx_drm_set_bus_format(encoder, bus_format);
-}
-
 static void imx_ldb_encoder_commit(struct drm_encoder *encoder)
 {
-	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
+	struct imx_drm_encoder *imx_encoder = enc_to_imx_enc(encoder);
+	struct imx_ldb_channel *imx_ldb_ch = imx_enc_to_imx_ldb_ch(imx_encoder);
 	struct imx_ldb *ldb = imx_ldb_ch->ldb;
 	int dual = ldb->ldb_ctrl & LDB_SPLIT_MODE_EN;
 	int mux = drm_of_encoder_active_port_id(imx_ldb_ch->child, encoder);
@@ -260,7 +225,8 @@ static void imx_ldb_encoder_mode_set(struct drm_encoder *encoder,
 			 struct drm_display_mode *orig_mode,
 			 struct drm_display_mode *mode)
 {
-	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
+	struct imx_drm_encoder *imx_encoder = enc_to_imx_enc(encoder);
+	struct imx_ldb_channel *imx_ldb_ch = imx_enc_to_imx_ldb_ch(imx_encoder);
 	struct imx_ldb *ldb = imx_ldb_ch->ldb;
 	int dual = ldb->ldb_ctrl & LDB_SPLIT_MODE_EN;
 	unsigned long serial_clk;
@@ -303,7 +269,8 @@ static void imx_ldb_encoder_mode_set(struct drm_encoder *encoder,
 
 static void imx_ldb_encoder_disable(struct drm_encoder *encoder)
 {
-	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
+	struct imx_drm_encoder *imx_encoder = enc_to_imx_enc(encoder);
+	struct imx_ldb_channel *imx_ldb_ch = imx_enc_to_imx_ldb_ch(imx_encoder);
 	struct imx_ldb *ldb = imx_ldb_ch->ldb;
 	int mux, ret;
 
@@ -379,7 +346,6 @@ static const struct drm_encoder_funcs imx_ldb_encoder_funcs = {
 
 static const struct drm_encoder_helper_funcs imx_ldb_encoder_helper_funcs = {
 	.dpms = imx_ldb_encoder_dpms,
-	.prepare = imx_ldb_encoder_prepare,
 	.commit = imx_ldb_encoder_commit,
 	.mode_set = imx_ldb_encoder_mode_set,
 	.disable = imx_ldb_encoder_disable,
@@ -406,7 +372,7 @@ static int imx_ldb_register(struct drm_device *drm,
 	struct imx_ldb *ldb = imx_ldb_ch->ldb;
 	int ret;
 
-	ret = imx_drm_encoder_parse_of(drm, &imx_ldb_ch->encoder,
+	ret = imx_drm_encoder_parse_of(drm, &imx_ldb_ch->imx_encoder.encoder,
 				       imx_ldb_ch->child);
 	if (ret)
 		return ret;
@@ -421,10 +387,10 @@ static int imx_ldb_register(struct drm_device *drm,
 			return ret;
 	}
 
-	drm_encoder_helper_add(&imx_ldb_ch->encoder,
+	drm_encoder_helper_add(&imx_ldb_ch->imx_encoder.encoder,
 			&imx_ldb_encoder_helper_funcs);
-	drm_encoder_init(drm, &imx_ldb_ch->encoder, &imx_ldb_encoder_funcs,
-			 DRM_MODE_ENCODER_LVDS, NULL);
+	drm_encoder_init(drm, &imx_ldb_ch->imx_encoder.encoder,
+			 &imx_ldb_encoder_funcs, DRM_MODE_ENCODER_LVDS, NULL);
 
 	drm_connector_helper_add(&imx_ldb_ch->connector,
 			&imx_ldb_connector_helper_funcs);
@@ -435,7 +401,7 @@ static int imx_ldb_register(struct drm_device *drm,
 		drm_panel_attach(imx_ldb_ch->panel, &imx_ldb_ch->connector);
 
 	drm_mode_connector_attach_encoder(&imx_ldb_ch->connector,
-			&imx_ldb_ch->encoder);
+			&imx_ldb_ch->imx_encoder.encoder);
 
 	return 0;
 }
@@ -564,6 +530,7 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data)
 		struct imx_ldb_channel *channel;
 		struct device_node *ddc_node;
 		struct device_node *ep;
+		int bus_format;
 
 		ret = of_property_read_u32(child, "reg", &i);
 		if (ret || i < 0 || i > 1)
@@ -636,21 +603,46 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data)
 			}
 		}
 
-		channel->bus_format = of_get_bus_format(dev, child);
-		if (channel->bus_format == -EINVAL) {
+		bus_format = of_get_bus_format(dev, child);
+		if (bus_format == -EINVAL) {
 			/*
 			 * If no bus format was specified in the device tree,
 			 * we can still get it from the connected panel later.
 			 */
 			if (channel->panel && channel->panel->funcs &&
 			    channel->panel->funcs->get_modes)
-				channel->bus_format = 0;
+				bus_format = 0;
 		}
-		if (channel->bus_format < 0) {
+		if (bus_format < 0) {
 			dev_err(dev, "could not determine data mapping: %d\n",
-				channel->bus_format);
-			return channel->bus_format;
+				bus_format);
+			return bus_format;
 		}
+		switch (bus_format) {
+		case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
+			bus_format = MEDIA_BUS_FMT_RGB666_1X18;
+			break;
+		case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
+			bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+			if (i == 0 || dual)
+				imx_ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24;
+			if (i == 1 || dual)
+				imx_ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24;
+			break;
+		case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
+			bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+			if (i == 0 || dual)
+				imx_ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24 |
+						     LDB_BIT_MAP_CH0_JEIDA;
+			if (i == 1 || dual)
+				imx_ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24 |
+						     LDB_BIT_MAP_CH1_JEIDA;
+			break;
+		}
+		channel->imx_encoder.bus_format = bus_format;
+
+		channel->imx_encoder.di_hsync_pin = 2;
+		channel->imx_encoder.di_vsync_pin = 3;
 
 		ret = imx_ldb_register(drm, channel);
 		if (ret)
@@ -675,7 +667,8 @@ static void imx_ldb_unbind(struct device *dev, struct device *master,
 			continue;
 
 		channel->connector.funcs->destroy(&channel->connector);
-		channel->encoder.funcs->destroy(&channel->encoder);
+		channel->imx_encoder.encoder.funcs->destroy(
+					&channel->imx_encoder.encoder);
 
 		kfree(channel->edid);
 		i2c_put_adapter(channel->ddc);
diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c
index 0b0aeee..82a1edd 100644
--- a/drivers/gpu/drm/imx/imx-tve.c
+++ b/drivers/gpu/drm/imx/imx-tve.c
@@ -99,7 +99,7 @@
 #define TVE_TVDAC_TEST_MODE_MASK	(0x7 << 0)
 
 #define con_to_tve(x) container_of(x, struct imx_tve, connector)
-#define enc_to_tve(x) container_of(x, struct imx_tve, encoder)
+#define imx_enc_to_tve(x) container_of(x, struct imx_tve, imx_encoder)
 
 enum {
 	TVE_MODE_TVOUT,
@@ -108,7 +108,7 @@ enum {
 
 struct imx_tve {
 	struct drm_connector connector;
-	struct drm_encoder encoder;
+	struct imx_drm_encoder imx_encoder;
 	struct device *dev;
 	spinlock_t lock;	/* register lock */
 	bool enabled;
@@ -121,8 +121,6 @@ struct imx_tve {
 	struct clk *di_sel_clk;
 	struct clk_hw clk_hw_di;
 	struct clk *di_clk;
-	int vsync_pin;
-	int hsync_pin;
 };
 
 static void tve_lock(void *__tve)
@@ -273,12 +271,13 @@ static struct drm_encoder *imx_tve_connector_best_encoder(
 {
 	struct imx_tve *tve = con_to_tve(connector);
 
-	return &tve->encoder;
+	return &tve->imx_encoder.encoder;
 }
 
 static void imx_tve_encoder_dpms(struct drm_encoder *encoder, int mode)
 {
-	struct imx_tve *tve = enc_to_tve(encoder);
+	struct imx_drm_encoder *imx_encoder = enc_to_imx_enc(encoder);
+	struct imx_tve *tve = imx_enc_to_tve(imx_encoder);
 	int ret;
 
 	ret = regmap_update_bits(tve->regmap, TVE_COM_CONF_REG,
@@ -287,30 +286,12 @@ static void imx_tve_encoder_dpms(struct drm_encoder *encoder, int mode)
 		dev_err(tve->dev, "failed to disable TVOUT: %d\n", ret);
 }
 
-static void imx_tve_encoder_prepare(struct drm_encoder *encoder)
-{
-	struct imx_tve *tve = enc_to_tve(encoder);
-
-	tve_disable(tve);
-
-	switch (tve->mode) {
-	case TVE_MODE_VGA:
-		imx_drm_set_bus_config(encoder, MEDIA_BUS_FMT_GBR888_1X24,
-				       tve->hsync_pin, tve->vsync_pin,
-				       DRM_BUS_FLAG_DE_HIGH |
-				       DRM_BUS_FLAG_PIXDATA_NEGEDGE);
-		break;
-	case TVE_MODE_TVOUT:
-		imx_drm_set_bus_format(encoder, MEDIA_BUS_FMT_YUV8_1X24);
-		break;
-	}
-}
-
 static void imx_tve_encoder_mode_set(struct drm_encoder *encoder,
 				     struct drm_display_mode *orig_mode,
 				     struct drm_display_mode *mode)
 {
-	struct imx_tve *tve = enc_to_tve(encoder);
+	struct imx_drm_encoder *imx_encoder = enc_to_imx_enc(encoder);
+	struct imx_tve *tve = imx_enc_to_tve(imx_encoder);
 	unsigned long rounded_rate;
 	unsigned long rate;
 	int div = 1;
@@ -344,14 +325,16 @@ static void imx_tve_encoder_mode_set(struct drm_encoder *encoder,
 
 static void imx_tve_encoder_commit(struct drm_encoder *encoder)
 {
-	struct imx_tve *tve = enc_to_tve(encoder);
+	struct imx_drm_encoder *imx_encoder = enc_to_imx_enc(encoder);
+	struct imx_tve *tve = imx_enc_to_tve(imx_encoder);
 
 	tve_enable(tve);
 }
 
 static void imx_tve_encoder_disable(struct drm_encoder *encoder)
 {
-	struct imx_tve *tve = enc_to_tve(encoder);
+	struct imx_drm_encoder *imx_encoder = enc_to_imx_enc(encoder);
+	struct imx_tve *tve = imx_enc_to_tve(imx_encoder);
 
 	tve_disable(tve);
 }
@@ -378,7 +361,6 @@ static const struct drm_encoder_funcs imx_tve_encoder_funcs = {
 
 static const struct drm_encoder_helper_funcs imx_tve_encoder_helper_funcs = {
 	.dpms = imx_tve_encoder_dpms,
-	.prepare = imx_tve_encoder_prepare,
 	.mode_set = imx_tve_encoder_mode_set,
 	.commit = imx_tve_encoder_commit,
 	.disable = imx_tve_encoder_disable,
@@ -499,13 +481,14 @@ static int imx_tve_register(struct drm_device *drm, struct imx_tve *tve)
 	encoder_type = tve->mode == TVE_MODE_VGA ?
 				DRM_MODE_ENCODER_DAC : DRM_MODE_ENCODER_TVDAC;
 
-	ret = imx_drm_encoder_parse_of(drm, &tve->encoder,
+	ret = imx_drm_encoder_parse_of(drm, &tve->imx_encoder.encoder,
 				       tve->dev->of_node);
 	if (ret)
 		return ret;
 
-	drm_encoder_helper_add(&tve->encoder, &imx_tve_encoder_helper_funcs);
-	drm_encoder_init(drm, &tve->encoder, &imx_tve_encoder_funcs,
+	drm_encoder_helper_add(&tve->imx_encoder.encoder,
+			       &imx_tve_encoder_helper_funcs);
+	drm_encoder_init(drm, &tve->imx_encoder.encoder, &imx_tve_encoder_funcs,
 			 encoder_type, NULL);
 
 	drm_connector_helper_add(&tve->connector,
@@ -513,7 +496,8 @@ static int imx_tve_register(struct drm_device *drm, struct imx_tve *tve)
 	drm_connector_init(drm, &tve->connector, &imx_tve_connector_funcs,
 			   DRM_MODE_CONNECTOR_VGA);
 
-	drm_mode_connector_attach_encoder(&tve->connector, &tve->encoder);
+	drm_mode_connector_attach_encoder(&tve->connector,
+					  &tve->imx_encoder.encoder);
 
 	return 0;
 }
@@ -591,7 +575,7 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data)
 
 	if (tve->mode == TVE_MODE_VGA) {
 		ret = of_property_read_u32(np, "fsl,hsync-pin",
-					   &tve->hsync_pin);
+					   &tve->imx_encoder.di_hsync_pin);
 
 		if (ret < 0) {
 			dev_err(dev, "failed to get vsync pin\n");
@@ -599,12 +583,14 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data)
 		}
 
 		ret |= of_property_read_u32(np, "fsl,vsync-pin",
-					    &tve->vsync_pin);
+					    &tve->imx_encoder.di_vsync_pin);
 
 		if (ret < 0) {
 			dev_err(dev, "failed to get vsync pin\n");
 			return ret;
 		}
+
+		tve->imx_encoder.bus_format = MEDIA_BUS_FMT_GBR888_1X24;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -693,7 +679,7 @@ static void imx_tve_unbind(struct device *dev, struct device *master,
 	struct imx_tve *tve = dev_get_drvdata(dev);
 
 	tve->connector.funcs->destroy(&tve->connector);
-	tve->encoder.funcs->destroy(&tve->encoder);
+	tve->imx_encoder.encoder.funcs->destroy(&tve->imx_encoder.encoder);
 
 	if (!IS_ERR(tve->dac_reg))
 		regulator_disable(tve->dac_reg);
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index 7df51e8..7f4475a1 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -62,15 +62,10 @@ struct ipu_crtc {
 
 	struct ipu_dc		*dc;
 	struct ipu_di		*di;
-	int			enabled;
 	enum ipu_flip_status	flip_state;
 	struct workqueue_struct *flip_queue;
 	struct ipu_flip_work	*flip_work;
 	int			irq;
-	u32			bus_format;
-	u32			bus_flags;
-	int			di_hsync_pin;
-	int			di_vsync_pin;
 };
 
 #define to_ipu_crtc(x) container_of(x, struct ipu_crtc, base)
@@ -79,18 +74,9 @@ static void ipu_crtc_enable(struct ipu_crtc *ipu_crtc)
 {
 	struct ipu_soc *ipu = dev_get_drvdata(ipu_crtc->dev->parent);
 
-	if (ipu_crtc->enabled)
-		return;
-
 	ipu_dc_enable(ipu);
 	ipu_dc_enable_channel(ipu_crtc->dc);
 	ipu_di_enable(ipu_crtc->di);
-	ipu_crtc->enabled = 1;
-
-	/*
-	 * In order not to be warned on enabling vblank failure,
-	 * we should call drm_crtc_vblank_on() after ->enabled is set to 1.
-	 */
 	drm_crtc_vblank_on(&ipu_crtc->base);
 }
 
@@ -98,14 +84,9 @@ static void ipu_crtc_disable(struct ipu_crtc *ipu_crtc)
 {
 	struct ipu_soc *ipu = dev_get_drvdata(ipu_crtc->dev->parent);
 
-	if (!ipu_crtc->enabled)
-		return;
-
 	ipu_dc_disable_channel(ipu_crtc->dc);
 	ipu_di_disable(ipu_crtc->di);
 	ipu_dc_disable(ipu);
-	ipu_crtc->enabled = 0;
-
 	drm_crtc_vblank_off(&ipu_crtc->base);
 }
 
@@ -234,7 +215,7 @@ put_vblank:
 }
 
 static const struct drm_crtc_funcs ipu_crtc_funcs = {
-	.set_config = drm_crtc_helper_set_config,
+	.set_config = drm_atomic_helper_set_config,
 	.destroy = drm_crtc_cleanup,
 	.page_flip = ipu_page_flip,
 	.reset = drm_atomic_helper_crtc_reset,
@@ -314,6 +295,11 @@ static void ipu_crtc_commit(struct drm_crtc *crtc)
 static int ipu_crtc_atomic_check(struct drm_crtc *crtc,
 				 struct drm_crtc_state *state)
 {
+	u32 primary_plane_mask = 1 << drm_plane_index(crtc->primary);
+
+	if (state->active && (primary_plane_mask & state->plane_mask) == 0)
+		return -EINVAL;
+
 	return 0;
 }
 
@@ -321,6 +307,7 @@ static void ipu_crtc_mode_set_nofb(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_encoder *encoder;
+	struct imx_drm_encoder *imx_encoder = NULL;
 	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
 	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
 	struct ipu_di_signal_cfg sig_cfg = {};
@@ -331,9 +318,12 @@ static void ipu_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	dev_dbg(ipu_crtc->dev, "%s: mode->vdisplay: %d\n", __func__,
 			mode->vdisplay);
 
-	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
-		if (encoder->crtc == crtc)
+	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+		if (encoder->crtc == crtc) {
 			encoder_types |= BIT(encoder->encoder_type);
+			imx_encoder = enc_to_imx_enc(encoder);
+		}
+	}
 
 	dev_dbg(ipu_crtc->dev, "%s: attached to encoder types 0x%lx\n",
 		__func__, encoder_types);
@@ -351,27 +341,26 @@ static void ipu_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	else
 		sig_cfg.clkflags = 0;
 
-	sig_cfg.enable_pol = !(ipu_crtc->bus_flags & DRM_BUS_FLAG_DE_LOW);
+	sig_cfg.enable_pol = !(imx_encoder->bus_flags & DRM_BUS_FLAG_DE_LOW);
 	/* Default to driving pixel data on negative clock edges */
-	sig_cfg.clk_pol = !!(ipu_crtc->bus_flags &
+	sig_cfg.clk_pol = !!(imx_encoder->bus_flags &
 			     DRM_BUS_FLAG_PIXDATA_POSEDGE);
-	sig_cfg.bus_format = ipu_crtc->bus_format;
+	sig_cfg.bus_format = imx_encoder->bus_format;
 	sig_cfg.v_to_h_sync = 0;
-	sig_cfg.hsync_pin = ipu_crtc->di_hsync_pin;
-	sig_cfg.vsync_pin = ipu_crtc->di_vsync_pin;
+	sig_cfg.hsync_pin = imx_encoder->di_hsync_pin;
+	sig_cfg.vsync_pin = imx_encoder->di_vsync_pin;
 
 	drm_display_mode_to_videomode(mode, &sig_cfg.mode);
 
 	ipu_dc_init_sync(ipu_crtc->dc, ipu_crtc->di,
 			 mode->flags & DRM_MODE_FLAG_INTERLACE,
-			 ipu_crtc->bus_format, mode->hdisplay);
+			 imx_encoder->bus_format, mode->hdisplay);
 	ipu_di_init_sync_panel(ipu_crtc->di, &sig_cfg);
 }
 
 static const struct drm_crtc_helper_funcs ipu_helper_funcs = {
 	.dpms = ipu_crtc_dpms,
 	.mode_fixup = ipu_crtc_mode_fixup,
-	.mode_set = drm_helper_crtc_mode_set,
 	.mode_set_nofb = ipu_crtc_mode_set_nofb,
 	.prepare = ipu_crtc_prepare,
 	.commit = ipu_crtc_commit,
@@ -382,14 +371,6 @@ static int ipu_enable_vblank(struct drm_crtc *crtc)
 {
 	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
 
-	/*
-	 * ->commit is done after ->mode_set in drm_crtc_helper_set_mode(),
-	 * so waiting for vblank in drm_plane_helper_commit() will timeout.
-	 * Check the state here to avoid the waiting.
-	 */
-	if (!ipu_crtc->enabled)
-		return -EINVAL;
-
 	enable_irq(ipu_crtc->irq);
 
 	return 0;
@@ -402,23 +383,9 @@ static void ipu_disable_vblank(struct drm_crtc *crtc)
 	disable_irq_nosync(ipu_crtc->irq);
 }
 
-static int ipu_set_interface_pix_fmt(struct drm_crtc *crtc,
-		u32 bus_format, int hsync_pin, int vsync_pin, u32 bus_flags)
-{
-	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
-
-	ipu_crtc->bus_format = bus_format;
-	ipu_crtc->bus_flags = bus_flags;
-	ipu_crtc->di_hsync_pin = hsync_pin;
-	ipu_crtc->di_vsync_pin = vsync_pin;
-
-	return 0;
-}
-
 static const struct imx_drm_crtc_helper_funcs ipu_crtc_helper_funcs = {
 	.enable_vblank = ipu_enable_vblank,
 	.disable_vblank = ipu_disable_vblank,
-	.set_interface_pix_fmt = ipu_set_interface_pix_fmt,
 	.crtc_funcs = &ipu_crtc_funcs,
 	.crtc_helper_funcs = &ipu_helper_funcs,
 };
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index fc1ff7c..79cfff5 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -14,6 +14,7 @@
  */
 
 #include <drm/drmP.h>
+#include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
@@ -374,31 +375,31 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 	struct drm_plane_state *old_state = plane->state;
 	struct drm_crtc_state *crtc_state;
 	struct device *dev = plane->dev->dev;
-	struct drm_crtc *crtc;
 	struct drm_framebuffer *fb = state->fb;
 	struct drm_framebuffer *old_fb = old_state->fb;
 	unsigned long eba, ubo, vbo, old_ubo, old_vbo;
 
 	/* Ok to disable */
 	if (!fb)
-		return old_fb ? 0 : -EINVAL;
+		return 0;
+
+	if (!state->crtc)
+		return -EINVAL;
+
+	crtc_state =
+		drm_atomic_get_existing_crtc_state(state->state, state->crtc);
+	if (WARN_ON(!crtc_state))
+		return -EINVAL;
 
 	/* CRTC should be enabled */
-	drm_for_each_crtc(crtc, plane->dev) {
-		if (drm_crtc_mask(crtc) == plane->possible_crtcs) {
-			if (!crtc->enabled)
-				return -EINVAL;
-			break;
-		}
-	}
+	if (!crtc_state->enable)
+		return -EINVAL;
 
 	/* no scaling */
 	if (state->src_w >> 16 != state->crtc_w ||
 	    state->src_h >> 16 != state->crtc_h)
 		return -EINVAL;
 
-	crtc_state = state->crtc->state;
-
 	switch (plane->type) {
 	case DRM_PLANE_TYPE_PRIMARY:
 		/* full plane doesn't support partial off screen */
diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
index 9fe88c9..7374d82 100644
--- a/drivers/gpu/drm/imx/parallel-display.c
+++ b/drivers/gpu/drm/imx/parallel-display.c
@@ -27,15 +27,15 @@
 #include "imx-drm.h"
 
 #define con_to_imxpd(x) container_of(x, struct imx_parallel_display, connector)
-#define enc_to_imxpd(x) container_of(x, struct imx_parallel_display, encoder)
+#define imx_enc_to_imxpd(x)	\
+		container_of(x, struct imx_parallel_display, imx_encoder)
 
 struct imx_parallel_display {
 	struct drm_connector connector;
-	struct drm_encoder encoder;
+	struct imx_drm_encoder imx_encoder;
 	struct device *dev;
 	void *edid;
 	int edid_len;
-	u32 bus_format;
 	struct drm_display_mode mode;
 	struct drm_panel *panel;
 };
@@ -57,8 +57,9 @@ static int imx_pd_connector_get_modes(struct drm_connector *connector)
 		struct drm_display_info *di = &connector->display_info;
 
 		num_modes = imxpd->panel->funcs->get_modes(imxpd->panel);
-		if (!imxpd->bus_format && di->num_bus_formats)
-			imxpd->bus_format = di->bus_formats[0];
+		if (!imxpd->imx_encoder.bus_format && di->num_bus_formats)
+			imxpd->imx_encoder.bus_format = di->bus_formats[0];
+		imxpd->imx_encoder.bus_flags = di->bus_flags;
 		if (num_modes > 0)
 			return num_modes;
 	}
@@ -88,12 +89,13 @@ static struct drm_encoder *imx_pd_connector_best_encoder(
 {
 	struct imx_parallel_display *imxpd = con_to_imxpd(connector);
 
-	return &imxpd->encoder;
+	return &imxpd->imx_encoder.encoder;
 }
 
 static void imx_pd_encoder_dpms(struct drm_encoder *encoder, int mode)
 {
-	struct imx_parallel_display *imxpd = enc_to_imxpd(encoder);
+	struct imx_drm_encoder *imx_encoder = enc_to_imx_enc(encoder);
+	struct imx_parallel_display *imxpd = imx_enc_to_imxpd(imx_encoder);
 
 	if (mode != DRM_MODE_DPMS_ON)
 		drm_panel_disable(imxpd->panel);
@@ -101,16 +103,10 @@ static void imx_pd_encoder_dpms(struct drm_encoder *encoder, int mode)
 		drm_panel_enable(imxpd->panel);
 }
 
-static void imx_pd_encoder_prepare(struct drm_encoder *encoder)
-{
-	struct imx_parallel_display *imxpd = enc_to_imxpd(encoder);
-	imx_drm_set_bus_config(encoder, imxpd->bus_format, 2, 3,
-			       imxpd->connector.display_info.bus_flags);
-}
-
 static void imx_pd_encoder_commit(struct drm_encoder *encoder)
 {
-	struct imx_parallel_display *imxpd = enc_to_imxpd(encoder);
+	struct imx_drm_encoder *imx_encoder = enc_to_imx_enc(encoder);
+	struct imx_parallel_display *imxpd = imx_enc_to_imxpd(imx_encoder);
 
 	drm_panel_prepare(imxpd->panel);
 	drm_panel_enable(imxpd->panel);
@@ -124,7 +120,8 @@ static void imx_pd_encoder_mode_set(struct drm_encoder *encoder,
 
 static void imx_pd_encoder_disable(struct drm_encoder *encoder)
 {
-	struct imx_parallel_display *imxpd = enc_to_imxpd(encoder);
+	struct imx_drm_encoder *imx_encoder = enc_to_imx_enc(encoder);
+	struct imx_parallel_display *imxpd = imx_enc_to_imxpd(imx_encoder);
 
 	drm_panel_disable(imxpd->panel);
 	drm_panel_unprepare(imxpd->panel);
@@ -151,7 +148,6 @@ static const struct drm_encoder_funcs imx_pd_encoder_funcs = {
 
 static const struct drm_encoder_helper_funcs imx_pd_encoder_helper_funcs = {
 	.dpms = imx_pd_encoder_dpms,
-	.prepare = imx_pd_encoder_prepare,
 	.commit = imx_pd_encoder_commit,
 	.mode_set = imx_pd_encoder_mode_set,
 	.disable = imx_pd_encoder_disable,
@@ -162,7 +158,7 @@ static int imx_pd_register(struct drm_device *drm,
 {
 	int ret;
 
-	ret = imx_drm_encoder_parse_of(drm, &imxpd->encoder,
+	ret = imx_drm_encoder_parse_of(drm, &imxpd->imx_encoder.encoder,
 				       imxpd->dev->of_node);
 	if (ret)
 		return ret;
@@ -174,9 +170,10 @@ static int imx_pd_register(struct drm_device *drm,
 	 */
 	imxpd->connector.dpms = DRM_MODE_DPMS_OFF;
 
-	drm_encoder_helper_add(&imxpd->encoder, &imx_pd_encoder_helper_funcs);
-	drm_encoder_init(drm, &imxpd->encoder, &imx_pd_encoder_funcs,
-			 DRM_MODE_ENCODER_NONE, NULL);
+	drm_encoder_helper_add(&imxpd->imx_encoder.encoder,
+			       &imx_pd_encoder_helper_funcs);
+	drm_encoder_init(drm, &imxpd->imx_encoder.encoder,
+			 &imx_pd_encoder_funcs, DRM_MODE_ENCODER_NONE, NULL);
 
 	drm_connector_helper_add(&imxpd->connector,
 			&imx_pd_connector_helper_funcs);
@@ -186,7 +183,8 @@ static int imx_pd_register(struct drm_device *drm,
 	if (imxpd->panel)
 		drm_panel_attach(imxpd->panel, &imxpd->connector);
 
-	drm_mode_connector_attach_encoder(&imxpd->connector, &imxpd->encoder);
+	drm_mode_connector_attach_encoder(&imxpd->connector,
+					  &imxpd->imx_encoder.encoder);
 
 	return 0;
 }
@@ -199,6 +197,7 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data)
 	const u8 *edidp;
 	struct imx_parallel_display *imxpd;
 	int ret;
+	u32 bus_format = 0;
 	const char *fmt;
 
 	imxpd = devm_kzalloc(dev, sizeof(*imxpd), GFP_KERNEL);
@@ -212,14 +211,17 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data)
 	ret = of_property_read_string(np, "interface-pix-fmt", &fmt);
 	if (!ret) {
 		if (!strcmp(fmt, "rgb24"))
-			imxpd->bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+			bus_format = MEDIA_BUS_FMT_RGB888_1X24;
 		else if (!strcmp(fmt, "rgb565"))
-			imxpd->bus_format = MEDIA_BUS_FMT_RGB565_1X16;
+			bus_format = MEDIA_BUS_FMT_RGB565_1X16;
 		else if (!strcmp(fmt, "bgr666"))
-			imxpd->bus_format = MEDIA_BUS_FMT_RGB666_1X18;
+			bus_format = MEDIA_BUS_FMT_RGB666_1X18;
 		else if (!strcmp(fmt, "lvds666"))
-			imxpd->bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI;
+			bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI;
 	}
+	imxpd->imx_encoder.bus_format = bus_format;
+	imxpd->imx_encoder.di_hsync_pin = 2;
+	imxpd->imx_encoder.di_vsync_pin = 3;
 
 	/* port@1 is the output port */
 	ep = of_graph_get_endpoint_by_regs(np, 1, -1);
@@ -252,7 +254,7 @@ static void imx_pd_unbind(struct device *dev, struct device *master,
 {
 	struct imx_parallel_display *imxpd = dev_get_drvdata(dev);
 
-	imxpd->encoder.funcs->destroy(&imxpd->encoder);
+	imxpd->imx_encoder.encoder.funcs->destroy(&imxpd->imx_encoder.encoder);
 	imxpd->connector.funcs->destroy(&imxpd->connector);
 
 	kfree(imxpd->edid);
-- 
2.7.4

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

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

* [PATCH v2 08/10] drm/imx: atomic phase 3 step 3: Legacy callback fixups
  2016-05-31  9:24 [PATCH v2 00/10] imx drm atomic mode setting conversion Liu Ying
                   ` (6 preceding siblings ...)
  2016-05-31  9:24 ` [PATCH v2 07/10] drm/imx: atomic phase 3 step 2: Use atomic configuration Liu Ying
@ 2016-05-31  9:24 ` Liu Ying
  2016-05-31  9:24 ` [PATCH v2 09/10] drm/imx: atomic phase 3 step 4: Use generic atomic page flip Liu Ying
  2016-05-31  9:24 ` [PATCH v2 10/10] drm/imx: atomic phase 3 step 5: Advertise DRIVER_ATOMIC Liu Ying
  9 siblings, 0 replies; 13+ messages in thread
From: Liu Ying @ 2016-05-31  9:24 UTC (permalink / raw)
  To: dri-devel; +Cc: Russell King, Daniel Vetter

Now that we can use atomic configurations, all the legacy callbacks
of CRTCs, encoders and connectors can be switched to the atomic version.
For the imx-ldb driver, there is a clock parent setting mismatch bewteen
->enable and ->disable after the switch, so a fixup is added.  For the
imx-tve driver, since the encoder's callback ->dpms is replaced by
->disable, we need to move the setting for the IPU_CLK_EN bit(in register
TVE_COM_CONF_REG) from ->enable/->disable to ->mode_set, otherwise, the
relevant CRTC cannot be disabled correctly with a warning on DC stop timeout.

Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v1->v2:
* A fixup on the TVE register bit IPU_CLK_EN to avoid a warning on DC
  stop timeout when doing mode setting.

 drivers/gpu/drm/imx/dw_hdmi-imx.c      |  4 ++--
 drivers/gpu/drm/imx/imx-ldb.c          | 16 ++++++-------
 drivers/gpu/drm/imx/imx-tve.c          | 27 +++++++--------------
 drivers/gpu/drm/imx/ipuv3-crtc.c       | 43 +++++-----------------------------
 drivers/gpu/drm/imx/parallel-display.c | 18 +++-----------
 5 files changed, 27 insertions(+), 81 deletions(-)

diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c
index 5f64674..5f1d437 100644
--- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
+++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
@@ -117,7 +117,7 @@ static void dw_hdmi_imx_encoder_mode_set(struct drm_encoder *encoder,
 {
 }
 
-static void dw_hdmi_imx_encoder_commit(struct drm_encoder *encoder)
+static void dw_hdmi_imx_encoder_enable(struct drm_encoder *encoder)
 {
 	struct imx_drm_encoder *imx_encoder = enc_to_imx_enc(encoder);
 	struct imx_hdmi *hdmi = imx_enc_to_imx_hdmi(imx_encoder);
@@ -130,7 +130,7 @@ static void dw_hdmi_imx_encoder_commit(struct drm_encoder *encoder)
 
 static const struct drm_encoder_helper_funcs dw_hdmi_imx_encoder_helper_funcs = {
 	.mode_set   = dw_hdmi_imx_encoder_mode_set,
-	.commit     = dw_hdmi_imx_encoder_commit,
+	.enable     = dw_hdmi_imx_encoder_enable,
 	.disable    = dw_hdmi_imx_encoder_disable,
 };
 
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index 4a98eaa..9a4190c 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -142,10 +142,6 @@ static struct drm_encoder *imx_ldb_connector_best_encoder(
 	return &imx_ldb_ch->imx_encoder.encoder;
 }
 
-static void imx_ldb_encoder_dpms(struct drm_encoder *encoder, int mode)
-{
-}
-
 static void imx_ldb_set_clock(struct imx_ldb *ldb, int mux, int chno,
 		unsigned long serial_clk, unsigned long di_clk)
 {
@@ -174,7 +170,7 @@ static void imx_ldb_set_clock(struct imx_ldb *ldb, int mux, int chno,
 			chno);
 }
 
-static void imx_ldb_encoder_commit(struct drm_encoder *encoder)
+static void imx_ldb_encoder_enable(struct drm_encoder *encoder)
 {
 	struct imx_drm_encoder *imx_encoder = enc_to_imx_enc(encoder);
 	struct imx_ldb_channel *imx_ldb_ch = imx_enc_to_imx_ldb_ch(imx_encoder);
@@ -185,8 +181,13 @@ static void imx_ldb_encoder_commit(struct drm_encoder *encoder)
 	drm_panel_prepare(imx_ldb_ch->panel);
 
 	if (dual) {
+		clk_set_parent(ldb->clk_sel[mux], ldb->clk[0]);
+		clk_set_parent(ldb->clk_sel[mux], ldb->clk[1]);
+
 		clk_prepare_enable(ldb->clk[0]);
 		clk_prepare_enable(ldb->clk[1]);
+	} else {
+		clk_set_parent(ldb->clk_sel[mux], ldb->clk[imx_ldb_ch->chno]);
 	}
 
 	if (imx_ldb_ch == &ldb->channel[0] || dual) {
@@ -326,7 +327,7 @@ static void imx_ldb_encoder_disable(struct drm_encoder *encoder)
 }
 
 static const struct drm_connector_funcs imx_ldb_connector_funcs = {
-	.dpms = drm_helper_connector_dpms,
+	.dpms = drm_atomic_helper_connector_dpms,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.detect = imx_ldb_connector_detect,
 	.destroy = imx_drm_connector_destroy,
@@ -345,9 +346,8 @@ static const struct drm_encoder_funcs imx_ldb_encoder_funcs = {
 };
 
 static const struct drm_encoder_helper_funcs imx_ldb_encoder_helper_funcs = {
-	.dpms = imx_ldb_encoder_dpms,
-	.commit = imx_ldb_encoder_commit,
 	.mode_set = imx_ldb_encoder_mode_set,
+	.enable = imx_ldb_encoder_enable,
 	.disable = imx_ldb_encoder_disable,
 };
 
diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c
index 82a1edd..cd92aac 100644
--- a/drivers/gpu/drm/imx/imx-tve.c
+++ b/drivers/gpu/drm/imx/imx-tve.c
@@ -147,8 +147,7 @@ static void tve_enable(struct imx_tve *tve)
 		tve->enabled = true;
 		clk_prepare_enable(tve->clk);
 		ret = regmap_update_bits(tve->regmap, TVE_COM_CONF_REG,
-					 TVE_IPU_CLK_EN | TVE_EN,
-					 TVE_IPU_CLK_EN | TVE_EN);
+					 TVE_EN, TVE_EN);
 	}
 
 	/* clear interrupt status register */
@@ -171,7 +170,7 @@ static void tve_disable(struct imx_tve *tve)
 	if (tve->enabled) {
 		tve->enabled = false;
 		ret = regmap_update_bits(tve->regmap, TVE_COM_CONF_REG,
-					 TVE_IPU_CLK_EN | TVE_EN, 0);
+					 TVE_EN, 0);
 		clk_disable_unprepare(tve->clk);
 	}
 }
@@ -274,18 +273,6 @@ static struct drm_encoder *imx_tve_connector_best_encoder(
 	return &tve->imx_encoder.encoder;
 }
 
-static void imx_tve_encoder_dpms(struct drm_encoder *encoder, int mode)
-{
-	struct imx_drm_encoder *imx_encoder = enc_to_imx_enc(encoder);
-	struct imx_tve *tve = imx_enc_to_tve(imx_encoder);
-	int ret;
-
-	ret = regmap_update_bits(tve->regmap, TVE_COM_CONF_REG,
-				 TVE_TV_OUT_MODE_MASK, TVE_TV_OUT_DISABLE);
-	if (ret < 0)
-		dev_err(tve->dev, "failed to disable TVOUT: %d\n", ret);
-}
-
 static void imx_tve_encoder_mode_set(struct drm_encoder *encoder,
 				     struct drm_display_mode *orig_mode,
 				     struct drm_display_mode *mode)
@@ -315,6 +302,9 @@ static void imx_tve_encoder_mode_set(struct drm_encoder *encoder,
 			ret);
 	}
 
+	regmap_update_bits(tve->regmap, TVE_COM_CONF_REG,
+			   TVE_IPU_CLK_EN, TVE_IPU_CLK_EN);
+
 	if (tve->mode == TVE_MODE_VGA)
 		ret = tve_setup_vga(tve);
 	else
@@ -323,7 +313,7 @@ static void imx_tve_encoder_mode_set(struct drm_encoder *encoder,
 		dev_err(tve->dev, "failed to set configuration: %d\n", ret);
 }
 
-static void imx_tve_encoder_commit(struct drm_encoder *encoder)
+static void imx_tve_encoder_enable(struct drm_encoder *encoder)
 {
 	struct imx_drm_encoder *imx_encoder = enc_to_imx_enc(encoder);
 	struct imx_tve *tve = imx_enc_to_tve(imx_encoder);
@@ -340,7 +330,7 @@ static void imx_tve_encoder_disable(struct drm_encoder *encoder)
 }
 
 static const struct drm_connector_funcs imx_tve_connector_funcs = {
-	.dpms = drm_helper_connector_dpms,
+	.dpms = drm_atomic_helper_connector_dpms,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.detect = imx_tve_connector_detect,
 	.destroy = imx_drm_connector_destroy,
@@ -360,9 +350,8 @@ static const struct drm_encoder_funcs imx_tve_encoder_funcs = {
 };
 
 static const struct drm_encoder_helper_funcs imx_tve_encoder_helper_funcs = {
-	.dpms = imx_tve_encoder_dpms,
 	.mode_set = imx_tve_encoder_mode_set,
-	.commit = imx_tve_encoder_commit,
+	.enable = imx_tve_encoder_enable,
 	.disable = imx_tve_encoder_disable,
 };
 
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index 7f4475a1..f20340f 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -70,8 +70,9 @@ struct ipu_crtc {
 
 #define to_ipu_crtc(x) container_of(x, struct ipu_crtc, base)
 
-static void ipu_crtc_enable(struct ipu_crtc *ipu_crtc)
+static void ipu_crtc_enable(struct drm_crtc *crtc)
 {
+	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
 	struct ipu_soc *ipu = dev_get_drvdata(ipu_crtc->dev->parent);
 
 	ipu_dc_enable(ipu);
@@ -80,8 +81,9 @@ static void ipu_crtc_enable(struct ipu_crtc *ipu_crtc)
 	drm_crtc_vblank_on(&ipu_crtc->base);
 }
 
-static void ipu_crtc_disable(struct ipu_crtc *ipu_crtc)
+static void ipu_crtc_disable(struct drm_crtc *crtc)
 {
+	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
 	struct ipu_soc *ipu = dev_get_drvdata(ipu_crtc->dev->parent);
 
 	ipu_dc_disable_channel(ipu_crtc->dc);
@@ -90,24 +92,6 @@ static void ipu_crtc_disable(struct ipu_crtc *ipu_crtc)
 	drm_crtc_vblank_off(&ipu_crtc->base);
 }
 
-static void ipu_crtc_dpms(struct drm_crtc *crtc, int mode)
-{
-	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
-
-	dev_dbg(ipu_crtc->dev, "%s mode: %d\n", __func__, mode);
-
-	switch (mode) {
-	case DRM_MODE_DPMS_ON:
-		ipu_crtc_enable(ipu_crtc);
-		break;
-	case DRM_MODE_DPMS_STANDBY:
-	case DRM_MODE_DPMS_SUSPEND:
-	case DRM_MODE_DPMS_OFF:
-		ipu_crtc_disable(ipu_crtc);
-		break;
-	}
-}
-
 static void ipu_flip_unref_work_func(struct work_struct *__work)
 {
 	struct ipu_flip_work *work =
@@ -278,20 +262,6 @@ static bool ipu_crtc_mode_fixup(struct drm_crtc *crtc,
 	return true;
 }
 
-static void ipu_crtc_prepare(struct drm_crtc *crtc)
-{
-	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
-
-	ipu_crtc_disable(ipu_crtc);
-}
-
-static void ipu_crtc_commit(struct drm_crtc *crtc)
-{
-	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
-
-	ipu_crtc_enable(ipu_crtc);
-}
-
 static int ipu_crtc_atomic_check(struct drm_crtc *crtc,
 				 struct drm_crtc_state *state)
 {
@@ -359,12 +329,11 @@ static void ipu_crtc_mode_set_nofb(struct drm_crtc *crtc)
 }
 
 static const struct drm_crtc_helper_funcs ipu_helper_funcs = {
-	.dpms = ipu_crtc_dpms,
 	.mode_fixup = ipu_crtc_mode_fixup,
 	.mode_set_nofb = ipu_crtc_mode_set_nofb,
-	.prepare = ipu_crtc_prepare,
-	.commit = ipu_crtc_commit,
 	.atomic_check = ipu_crtc_atomic_check,
+	.disable = ipu_crtc_disable,
+	.enable = ipu_crtc_enable,
 };
 
 static int ipu_enable_vblank(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
index 7374d82..bb5dbd6 100644
--- a/drivers/gpu/drm/imx/parallel-display.c
+++ b/drivers/gpu/drm/imx/parallel-display.c
@@ -92,18 +92,7 @@ static struct drm_encoder *imx_pd_connector_best_encoder(
 	return &imxpd->imx_encoder.encoder;
 }
 
-static void imx_pd_encoder_dpms(struct drm_encoder *encoder, int mode)
-{
-	struct imx_drm_encoder *imx_encoder = enc_to_imx_enc(encoder);
-	struct imx_parallel_display *imxpd = imx_enc_to_imxpd(imx_encoder);
-
-	if (mode != DRM_MODE_DPMS_ON)
-		drm_panel_disable(imxpd->panel);
-	else
-		drm_panel_enable(imxpd->panel);
-}
-
-static void imx_pd_encoder_commit(struct drm_encoder *encoder)
+static void imx_pd_encoder_enable(struct drm_encoder *encoder)
 {
 	struct imx_drm_encoder *imx_encoder = enc_to_imx_enc(encoder);
 	struct imx_parallel_display *imxpd = imx_enc_to_imxpd(imx_encoder);
@@ -128,7 +117,7 @@ static void imx_pd_encoder_disable(struct drm_encoder *encoder)
 }
 
 static const struct drm_connector_funcs imx_pd_connector_funcs = {
-	.dpms = drm_helper_connector_dpms,
+	.dpms = drm_atomic_helper_connector_dpms,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.detect = imx_pd_connector_detect,
 	.destroy = imx_drm_connector_destroy,
@@ -147,9 +136,8 @@ static const struct drm_encoder_funcs imx_pd_encoder_funcs = {
 };
 
 static const struct drm_encoder_helper_funcs imx_pd_encoder_helper_funcs = {
-	.dpms = imx_pd_encoder_dpms,
-	.commit = imx_pd_encoder_commit,
 	.mode_set = imx_pd_encoder_mode_set,
+	.enable = imx_pd_encoder_enable,
 	.disable = imx_pd_encoder_disable,
 };
 
-- 
2.7.4

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

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

* [PATCH v2 09/10] drm/imx: atomic phase 3 step 4: Use generic atomic page flip
  2016-05-31  9:24 [PATCH v2 00/10] imx drm atomic mode setting conversion Liu Ying
                   ` (7 preceding siblings ...)
  2016-05-31  9:24 ` [PATCH v2 08/10] drm/imx: atomic phase 3 step 3: Legacy callback fixups Liu Ying
@ 2016-05-31  9:24 ` Liu Ying
  2016-05-31 11:03   ` Daniel Vetter
  2016-06-11 20:29   ` Daniel Vetter
  2016-05-31  9:24 ` [PATCH v2 10/10] drm/imx: atomic phase 3 step 5: Advertise DRIVER_ATOMIC Liu Ying
  9 siblings, 2 replies; 13+ messages in thread
From: Liu Ying @ 2016-05-31  9:24 UTC (permalink / raw)
  To: dri-devel; +Cc: Russell King, Daniel Vetter

To support generic atomic page flip, this patch customizes ->atomic_commit
for nonblock commits.

Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v1->v2:
* s/async/nonblock/ on this patch to address Daniel Vetter's comment.
* Wait for pending commit on each CRTC for both block and nonblock
  atomic mode settings.  This way, a block commit will not overwrite
  the hardware setting when a nonblock page flip is about to finish,
  so that the page flip may wait for vblank successfully.

 drivers/gpu/drm/imx/imx-drm-core.c | 135 +++++++++++++++++++++++++++++++-
 drivers/gpu/drm/imx/ipuv3-crtc.c   | 156 ++-----------------------------------
 2 files changed, 142 insertions(+), 149 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 2a2ab8c..d2bb743 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -15,10 +15,14 @@
  */
 #include <linux/component.h>
 #include <linux/device.h>
+#include <linux/dma-buf.h>
 #include <linux/fb.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/reservation.h>
+#include <linux/wait.h>
 #include <drm/drmP.h>
+#include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_crtc_helper.h>
@@ -48,6 +52,14 @@ struct imx_drm_device {
 struct imx_drm_crtc {
 	struct drm_crtc				*crtc;
 	struct imx_drm_crtc_helper_funcs	imx_drm_helper_funcs;
+	wait_queue_head_t			commit_wait;
+	bool					commit_pending;
+};
+
+struct imx_drm_commit {
+	struct work_struct work;
+	struct drm_device *dev;
+	struct drm_atomic_state *state;
 };
 
 #if IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION)
@@ -168,11 +180,130 @@ static void imx_drm_output_poll_changed(struct drm_device *drm)
 	drm_fbdev_cma_hotplug_event(imxdrm->fbhelper);
 }
 
+static void imx_drm_atomic_complete(struct imx_drm_commit *commit)
+{
+	struct drm_device *dev = commit->dev;
+	struct imx_drm_device *imxdrm = dev->dev_private;
+	struct imx_drm_crtc *imx_crtc;
+	struct drm_atomic_state *old_state = commit->state;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *old_crtc_state;
+	struct drm_plane_state *plane_state;
+	struct drm_gem_cma_object *cma_obj;
+	struct fence *excl;
+	unsigned shared_count;
+	struct fence **shared;
+	unsigned int i, j;
+	int ret;
+
+	/* Wait for fences. */
+	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+		if (crtc->state->event) {
+			plane_state = crtc->primary->state;
+			cma_obj = drm_fb_cma_get_gem_obj(plane_state->fb, 0);
+			if (cma_obj->base.dma_buf) {
+				ret = reservation_object_get_fences_rcu(
+					cma_obj->base.dma_buf->resv, &excl,
+					&shared_count, &shared);
+				if (unlikely(ret))
+					DRM_ERROR("failed to get fences "
+						  "for buffer\n");
+
+				if (excl) {
+					fence_wait(excl, false);
+					fence_put(excl);
+				}
+				for (j = 0; j < shared_count; i++) {
+					fence_wait(shared[j], false);
+					fence_put(shared[j]);
+				}
+			}
+		}
+	}
+
+	/* Apply the atomic update. */
+	drm_atomic_helper_commit_modeset_disables(dev, old_state);
+	drm_atomic_helper_commit_modeset_enables(dev, old_state);
+	drm_atomic_helper_commit_planes(dev, old_state, false);
+	drm_atomic_helper_wait_for_vblanks(dev, old_state);
+	drm_atomic_helper_cleanup_planes(dev, old_state);
+
+	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+		imx_crtc = imxdrm->crtc[i];
+
+		/* Complete the commit, wake up any waiter. */
+		spin_lock(&imx_crtc->commit_wait.lock);
+		imx_crtc->commit_pending = false;
+		wake_up_all_locked(&imx_crtc->commit_wait);
+		spin_unlock(&imx_crtc->commit_wait.lock);
+	}
+
+	drm_atomic_state_free(old_state);
+
+	kfree(commit);
+}
+
+static void imx_drm_atomic_work(struct work_struct *work)
+{
+	struct imx_drm_commit *commit =
+		container_of(work, struct imx_drm_commit, work);
+
+	imx_drm_atomic_complete(commit);
+}
+
+static int imx_drm_atomic_commit(struct drm_device *dev,
+				 struct drm_atomic_state *state, bool nonblock)
+{
+	struct imx_drm_device *imxdrm = dev->dev_private;
+	struct imx_drm_crtc *imx_crtc;
+	struct imx_drm_commit *commit;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+	unsigned int i;
+	int ret;
+
+	commit = kzalloc(sizeof(*commit), GFP_KERNEL);
+	if (commit == NULL)
+		return -ENOMEM;
+
+	commit->dev = dev;
+	commit->state = state;
+
+	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		imx_crtc = imxdrm->crtc[i];
+
+		spin_lock(&imx_crtc->commit_wait.lock);
+		ret = wait_event_interruptible_locked(
+				imx_crtc->commit_wait,
+				!imx_crtc->commit_pending);
+		if (ret == 0)
+			imx_crtc->commit_pending = true;
+		spin_unlock(&imx_crtc->commit_wait.lock);
+
+		if (ret) {
+			kfree(commit);
+			return ret;
+		}
+	}
+
+	if (nonblock)
+		INIT_WORK(&commit->work, imx_drm_atomic_work);
+
+	drm_atomic_helper_swap_state(dev, state);
+
+	if (nonblock)
+		schedule_work(&commit->work);
+	else
+		imx_drm_atomic_complete(commit);
+
+	return 0;
+}
+
 static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = {
 	.fb_create = drm_fb_cma_create,
 	.output_poll_changed = imx_drm_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
-	.atomic_commit = drm_atomic_helper_commit,
+	.atomic_commit = imx_drm_atomic_commit,
 };
 
 /*
@@ -307,6 +438,8 @@ int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc,
 	imx_drm_crtc->imx_drm_helper_funcs = *imx_drm_helper_funcs;
 	imx_drm_crtc->crtc = crtc;
 
+	init_waitqueue_head(&imx_drm_crtc->commit_wait);
+
 	crtc->port = port;
 
 	imxdrm->crtc[imxdrm->pipes++] = imx_drm_crtc;
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index f20340f..7ee51db 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -24,8 +24,6 @@
 #include <linux/fb.h>
 #include <linux/clk.h>
 #include <linux/errno.h>
-#include <linux/reservation.h>
-#include <linux/dma-buf.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 
@@ -35,23 +33,6 @@
 
 #define DRIVER_DESC		"i.MX IPUv3 Graphics"
 
-enum ipu_flip_status {
-	IPU_FLIP_NONE,
-	IPU_FLIP_PENDING,
-	IPU_FLIP_SUBMITTED,
-};
-
-struct ipu_flip_work {
-	struct work_struct		unref_work;
-	struct drm_gem_object		*bo;
-	struct drm_pending_vblank_event *page_flip_event;
-	struct work_struct		fence_work;
-	struct ipu_crtc			*crtc;
-	struct fence			*excl;
-	unsigned			shared_count;
-	struct fence			**shared;
-};
-
 struct ipu_crtc {
 	struct device		*dev;
 	struct drm_crtc		base;
@@ -62,9 +43,6 @@ struct ipu_crtc {
 
 	struct ipu_dc		*dc;
 	struct ipu_di		*di;
-	enum ipu_flip_status	flip_state;
-	struct workqueue_struct *flip_queue;
-	struct ipu_flip_work	*flip_work;
 	int			irq;
 };
 
@@ -92,150 +70,35 @@ static void ipu_crtc_disable(struct drm_crtc *crtc)
 	drm_crtc_vblank_off(&ipu_crtc->base);
 }
 
-static void ipu_flip_unref_work_func(struct work_struct *__work)
-{
-	struct ipu_flip_work *work =
-			container_of(__work, struct ipu_flip_work, unref_work);
-
-	drm_gem_object_unreference_unlocked(work->bo);
-	kfree(work);
-}
-
-static void ipu_flip_fence_work_func(struct work_struct *__work)
-{
-	struct ipu_flip_work *work =
-			container_of(__work, struct ipu_flip_work, fence_work);
-	int i;
-
-	/* wait for all fences attached to the FB obj to signal */
-	if (work->excl) {
-		fence_wait(work->excl, false);
-		fence_put(work->excl);
-	}
-	for (i = 0; i < work->shared_count; i++) {
-		fence_wait(work->shared[i], false);
-		fence_put(work->shared[i]);
-	}
-
-	work->crtc->flip_state = IPU_FLIP_SUBMITTED;
-}
-
-static int ipu_page_flip(struct drm_crtc *crtc,
-		struct drm_framebuffer *fb,
-		struct drm_pending_vblank_event *event,
-		uint32_t page_flip_flags)
-{
-	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
-	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
-	struct ipu_flip_work *flip_work;
-	int ret;
-
-	if (ipu_crtc->flip_state != IPU_FLIP_NONE)
-		return -EBUSY;
-
-	ret = imx_drm_crtc_vblank_get(ipu_crtc->imx_crtc);
-	if (ret) {
-		dev_dbg(ipu_crtc->dev, "failed to acquire vblank counter\n");
-		list_del(&event->base.link);
-
-		return ret;
-	}
-
-	flip_work = kzalloc(sizeof *flip_work, GFP_KERNEL);
-	if (!flip_work) {
-		ret = -ENOMEM;
-		goto put_vblank;
-	}
-	INIT_WORK(&flip_work->unref_work, ipu_flip_unref_work_func);
-	flip_work->page_flip_event = event;
-
-	/* get BO backing the old framebuffer and take a reference */
-	flip_work->bo = &drm_fb_cma_get_gem_obj(crtc->primary->fb, 0)->base;
-	drm_gem_object_reference(flip_work->bo);
-
-	ipu_crtc->flip_work = flip_work;
-	/*
-	 * If the object has a DMABUF attached, we need to wait on its fences
-	 * if there are any.
-	 */
-	if (cma_obj->base.dma_buf) {
-		INIT_WORK(&flip_work->fence_work, ipu_flip_fence_work_func);
-		flip_work->crtc = ipu_crtc;
-
-		ret = reservation_object_get_fences_rcu(
-				cma_obj->base.dma_buf->resv, &flip_work->excl,
-				&flip_work->shared_count, &flip_work->shared);
-
-		if (unlikely(ret)) {
-			DRM_ERROR("failed to get fences for buffer\n");
-			goto free_flip_work;
-		}
-
-		/* No need to queue the worker if the are no fences */
-		if (!flip_work->excl && !flip_work->shared_count) {
-			ipu_crtc->flip_state = IPU_FLIP_SUBMITTED;
-		} else {
-			ipu_crtc->flip_state = IPU_FLIP_PENDING;
-			queue_work(ipu_crtc->flip_queue,
-				   &flip_work->fence_work);
-		}
-	} else {
-		ipu_crtc->flip_state = IPU_FLIP_SUBMITTED;
-	}
-
-	if (crtc->primary->state)
-		drm_atomic_set_fb_for_plane(crtc->primary->state, fb);
-
-	return 0;
-
-free_flip_work:
-	drm_gem_object_unreference_unlocked(flip_work->bo);
-	kfree(flip_work);
-	ipu_crtc->flip_work = NULL;
-put_vblank:
-	imx_drm_crtc_vblank_put(ipu_crtc->imx_crtc);
-
-	return ret;
-}
-
 static const struct drm_crtc_funcs ipu_crtc_funcs = {
 	.set_config = drm_atomic_helper_set_config,
 	.destroy = drm_crtc_cleanup,
-	.page_flip = ipu_page_flip,
+	.page_flip = drm_atomic_helper_page_flip,
 	.reset = drm_atomic_helper_crtc_reset,
 	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
 };
 
-static void ipu_crtc_handle_pageflip(struct ipu_crtc *ipu_crtc)
+static void ipu_crtc_handle_pageflip(struct drm_crtc *crtc)
 {
+	struct drm_device *drm = crtc->dev;
 	unsigned long flags;
-	struct drm_device *drm = ipu_crtc->base.dev;
-	struct ipu_flip_work *work = ipu_crtc->flip_work;
 
 	spin_lock_irqsave(&drm->event_lock, flags);
-	if (work->page_flip_event)
-		drm_crtc_send_vblank_event(&ipu_crtc->base,
-					   work->page_flip_event);
-	imx_drm_crtc_vblank_put(ipu_crtc->imx_crtc);
+	drm_crtc_send_vblank_event(crtc, crtc->state->event);
+	crtc->state->event = NULL;
 	spin_unlock_irqrestore(&drm->event_lock, flags);
 }
 
 static irqreturn_t ipu_irq_handler(int irq, void *dev_id)
 {
 	struct ipu_crtc *ipu_crtc = dev_id;
+	struct drm_crtc *crtc = &ipu_crtc->base;
 
 	imx_drm_handle_vblank(ipu_crtc->imx_crtc);
 
-	if (ipu_crtc->flip_state == IPU_FLIP_SUBMITTED) {
-		struct ipu_plane *plane = ipu_crtc->plane[0];
-
-		ipu_plane_set_base(plane, ipu_crtc->base.primary->fb);
-		ipu_crtc_handle_pageflip(ipu_crtc);
-		queue_work(ipu_crtc->flip_queue,
-			   &ipu_crtc->flip_work->unref_work);
-		ipu_crtc->flip_state = IPU_FLIP_NONE;
-	}
+	if (crtc->state->event)
+		ipu_crtc_handle_pageflip(crtc);
 
 	return IRQ_HANDLED;
 }
@@ -458,8 +321,6 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
 	/* Only enable IRQ when we actually need it to trigger work. */
 	disable_irq(ipu_crtc->irq);
 
-	ipu_crtc->flip_queue = create_singlethread_workqueue("ipu-crtc-flip");
-
 	return 0;
 
 err_put_plane1_res:
@@ -504,7 +365,6 @@ static void ipu_drm_unbind(struct device *dev, struct device *master,
 
 	imx_drm_remove_crtc(ipu_crtc->imx_crtc);
 
-	destroy_workqueue(ipu_crtc->flip_queue);
 	ipu_put_resources(ipu_crtc);
 	if (ipu_crtc->plane[1])
 		ipu_plane_put_resources(ipu_crtc->plane[1]);
-- 
2.7.4

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

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

* [PATCH v2 10/10] drm/imx: atomic phase 3 step 5: Advertise DRIVER_ATOMIC
  2016-05-31  9:24 [PATCH v2 00/10] imx drm atomic mode setting conversion Liu Ying
                   ` (8 preceding siblings ...)
  2016-05-31  9:24 ` [PATCH v2 09/10] drm/imx: atomic phase 3 step 4: Use generic atomic page flip Liu Ying
@ 2016-05-31  9:24 ` Liu Ying
  9 siblings, 0 replies; 13+ messages in thread
From: Liu Ying @ 2016-05-31  9:24 UTC (permalink / raw)
  To: dri-devel; +Cc: Russell King, Daniel Vetter

With all the beforehand phases and steps done, we can adverstise DRIVER_ATOMIC.

Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v1->v2:
* None.

 drivers/gpu/drm/imx/imx-drm-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index d2bb743..a741980 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -502,7 +502,8 @@ static const struct drm_ioctl_desc imx_drm_ioctls[] = {
 };
 
 static struct drm_driver imx_drm_driver = {
-	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME,
+	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
+				  DRIVER_ATOMIC,
 	.load			= imx_drm_driver_load,
 	.unload			= imx_drm_driver_unload,
 	.lastclose		= imx_drm_driver_lastclose,
-- 
2.7.4

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

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

* Re: [PATCH v2 09/10] drm/imx: atomic phase 3 step 4: Use generic atomic page flip
  2016-05-31  9:24 ` [PATCH v2 09/10] drm/imx: atomic phase 3 step 4: Use generic atomic page flip Liu Ying
@ 2016-05-31 11:03   ` Daniel Vetter
  2016-06-11 20:29   ` Daniel Vetter
  1 sibling, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2016-05-31 11:03 UTC (permalink / raw)
  To: Liu Ying; +Cc: Russell King, dri-devel, Daniel Vetter

On Tue, May 31, 2016 at 05:24:30PM +0800, Liu Ying wrote:
> To support generic atomic page flip, this patch customizes ->atomic_commit
> for nonblock commits.
> 
> Signed-off-by: Liu Ying <gnuiyl@gmail.com>
> ---
> v1->v2:
> * s/async/nonblock/ on this patch to address Daniel Vetter's comment.
> * Wait for pending commit on each CRTC for both block and nonblock
>   atomic mode settings.  This way, a block commit will not overwrite
>   the hardware setting when a nonblock page flip is about to finish,
>   so that the page flip may wait for vblank successfully.

Ok, my generic nonblocking code seems pretty stable nowadays, please
rebase on top of that. Mostly you just have to replace your
commit_complete with an atomic_commit_tail that you plug into
dev->mode_config.helper_private->atomic_commit_tail.

Note that the nonblocking helpers are _very_ picky about your handling of
crtc_state->event. If anything times out it's probably because your driver
didn't send out that event correctly. But that's just atomic event
handling, so really just a driver bug if you hit it.

For an example that closely resembles your driver look at rockchip. We're
still hunting down some of the event handling issues.
-Daniel

> 
>  drivers/gpu/drm/imx/imx-drm-core.c | 135 +++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/imx/ipuv3-crtc.c   | 156 ++-----------------------------------
>  2 files changed, 142 insertions(+), 149 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
> index 2a2ab8c..d2bb743 100644
> --- a/drivers/gpu/drm/imx/imx-drm-core.c
> +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> @@ -15,10 +15,14 @@
>   */
>  #include <linux/component.h>
>  #include <linux/device.h>
> +#include <linux/dma-buf.h>
>  #include <linux/fb.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/reservation.h>
> +#include <linux/wait.h>
>  #include <drm/drmP.h>
> +#include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_crtc_helper.h>
> @@ -48,6 +52,14 @@ struct imx_drm_device {
>  struct imx_drm_crtc {
>  	struct drm_crtc				*crtc;
>  	struct imx_drm_crtc_helper_funcs	imx_drm_helper_funcs;
> +	wait_queue_head_t			commit_wait;
> +	bool					commit_pending;
> +};
> +
> +struct imx_drm_commit {
> +	struct work_struct work;
> +	struct drm_device *dev;
> +	struct drm_atomic_state *state;
>  };
>  
>  #if IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION)
> @@ -168,11 +180,130 @@ static void imx_drm_output_poll_changed(struct drm_device *drm)
>  	drm_fbdev_cma_hotplug_event(imxdrm->fbhelper);
>  }
>  
> +static void imx_drm_atomic_complete(struct imx_drm_commit *commit)
> +{
> +	struct drm_device *dev = commit->dev;
> +	struct imx_drm_device *imxdrm = dev->dev_private;
> +	struct imx_drm_crtc *imx_crtc;
> +	struct drm_atomic_state *old_state = commit->state;
> +	struct drm_crtc *crtc;
> +	struct drm_crtc_state *old_crtc_state;
> +	struct drm_plane_state *plane_state;
> +	struct drm_gem_cma_object *cma_obj;
> +	struct fence *excl;
> +	unsigned shared_count;
> +	struct fence **shared;
> +	unsigned int i, j;
> +	int ret;
> +
> +	/* Wait for fences. */
> +	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
> +		if (crtc->state->event) {
> +			plane_state = crtc->primary->state;
> +			cma_obj = drm_fb_cma_get_gem_obj(plane_state->fb, 0);
> +			if (cma_obj->base.dma_buf) {
> +				ret = reservation_object_get_fences_rcu(
> +					cma_obj->base.dma_buf->resv, &excl,
> +					&shared_count, &shared);
> +				if (unlikely(ret))
> +					DRM_ERROR("failed to get fences "
> +						  "for buffer\n");
> +
> +				if (excl) {
> +					fence_wait(excl, false);
> +					fence_put(excl);
> +				}
> +				for (j = 0; j < shared_count; i++) {
> +					fence_wait(shared[j], false);
> +					fence_put(shared[j]);
> +				}
> +			}
> +		}
> +	}
> +
> +	/* Apply the atomic update. */
> +	drm_atomic_helper_commit_modeset_disables(dev, old_state);
> +	drm_atomic_helper_commit_modeset_enables(dev, old_state);
> +	drm_atomic_helper_commit_planes(dev, old_state, false);
> +	drm_atomic_helper_wait_for_vblanks(dev, old_state);
> +	drm_atomic_helper_cleanup_planes(dev, old_state);
> +
> +	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
> +		imx_crtc = imxdrm->crtc[i];
> +
> +		/* Complete the commit, wake up any waiter. */
> +		spin_lock(&imx_crtc->commit_wait.lock);
> +		imx_crtc->commit_pending = false;
> +		wake_up_all_locked(&imx_crtc->commit_wait);
> +		spin_unlock(&imx_crtc->commit_wait.lock);
> +	}
> +
> +	drm_atomic_state_free(old_state);
> +
> +	kfree(commit);
> +}
> +
> +static void imx_drm_atomic_work(struct work_struct *work)
> +{
> +	struct imx_drm_commit *commit =
> +		container_of(work, struct imx_drm_commit, work);
> +
> +	imx_drm_atomic_complete(commit);
> +}
> +
> +static int imx_drm_atomic_commit(struct drm_device *dev,
> +				 struct drm_atomic_state *state, bool nonblock)
> +{
> +	struct imx_drm_device *imxdrm = dev->dev_private;
> +	struct imx_drm_crtc *imx_crtc;
> +	struct imx_drm_commit *commit;
> +	struct drm_crtc *crtc;
> +	struct drm_crtc_state *crtc_state;
> +	unsigned int i;
> +	int ret;
> +
> +	commit = kzalloc(sizeof(*commit), GFP_KERNEL);
> +	if (commit == NULL)
> +		return -ENOMEM;
> +
> +	commit->dev = dev;
> +	commit->state = state;
> +
> +	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +		imx_crtc = imxdrm->crtc[i];
> +
> +		spin_lock(&imx_crtc->commit_wait.lock);
> +		ret = wait_event_interruptible_locked(
> +				imx_crtc->commit_wait,
> +				!imx_crtc->commit_pending);
> +		if (ret == 0)
> +			imx_crtc->commit_pending = true;
> +		spin_unlock(&imx_crtc->commit_wait.lock);
> +
> +		if (ret) {
> +			kfree(commit);
> +			return ret;
> +		}
> +	}
> +
> +	if (nonblock)
> +		INIT_WORK(&commit->work, imx_drm_atomic_work);
> +
> +	drm_atomic_helper_swap_state(dev, state);
> +
> +	if (nonblock)
> +		schedule_work(&commit->work);
> +	else
> +		imx_drm_atomic_complete(commit);
> +
> +	return 0;
> +}
> +
>  static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = {
>  	.fb_create = drm_fb_cma_create,
>  	.output_poll_changed = imx_drm_output_poll_changed,
>  	.atomic_check = drm_atomic_helper_check,
> -	.atomic_commit = drm_atomic_helper_commit,
> +	.atomic_commit = imx_drm_atomic_commit,
>  };
>  
>  /*
> @@ -307,6 +438,8 @@ int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc,
>  	imx_drm_crtc->imx_drm_helper_funcs = *imx_drm_helper_funcs;
>  	imx_drm_crtc->crtc = crtc;
>  
> +	init_waitqueue_head(&imx_drm_crtc->commit_wait);
> +
>  	crtc->port = port;
>  
>  	imxdrm->crtc[imxdrm->pipes++] = imx_drm_crtc;
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index f20340f..7ee51db 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -24,8 +24,6 @@
>  #include <linux/fb.h>
>  #include <linux/clk.h>
>  #include <linux/errno.h>
> -#include <linux/reservation.h>
> -#include <linux/dma-buf.h>
>  #include <drm/drm_gem_cma_helper.h>
>  #include <drm/drm_fb_cma_helper.h>
>  
> @@ -35,23 +33,6 @@
>  
>  #define DRIVER_DESC		"i.MX IPUv3 Graphics"
>  
> -enum ipu_flip_status {
> -	IPU_FLIP_NONE,
> -	IPU_FLIP_PENDING,
> -	IPU_FLIP_SUBMITTED,
> -};
> -
> -struct ipu_flip_work {
> -	struct work_struct		unref_work;
> -	struct drm_gem_object		*bo;
> -	struct drm_pending_vblank_event *page_flip_event;
> -	struct work_struct		fence_work;
> -	struct ipu_crtc			*crtc;
> -	struct fence			*excl;
> -	unsigned			shared_count;
> -	struct fence			**shared;
> -};
> -
>  struct ipu_crtc {
>  	struct device		*dev;
>  	struct drm_crtc		base;
> @@ -62,9 +43,6 @@ struct ipu_crtc {
>  
>  	struct ipu_dc		*dc;
>  	struct ipu_di		*di;
> -	enum ipu_flip_status	flip_state;
> -	struct workqueue_struct *flip_queue;
> -	struct ipu_flip_work	*flip_work;
>  	int			irq;
>  };
>  
> @@ -92,150 +70,35 @@ static void ipu_crtc_disable(struct drm_crtc *crtc)
>  	drm_crtc_vblank_off(&ipu_crtc->base);
>  }
>  
> -static void ipu_flip_unref_work_func(struct work_struct *__work)
> -{
> -	struct ipu_flip_work *work =
> -			container_of(__work, struct ipu_flip_work, unref_work);
> -
> -	drm_gem_object_unreference_unlocked(work->bo);
> -	kfree(work);
> -}
> -
> -static void ipu_flip_fence_work_func(struct work_struct *__work)
> -{
> -	struct ipu_flip_work *work =
> -			container_of(__work, struct ipu_flip_work, fence_work);
> -	int i;
> -
> -	/* wait for all fences attached to the FB obj to signal */
> -	if (work->excl) {
> -		fence_wait(work->excl, false);
> -		fence_put(work->excl);
> -	}
> -	for (i = 0; i < work->shared_count; i++) {
> -		fence_wait(work->shared[i], false);
> -		fence_put(work->shared[i]);
> -	}
> -
> -	work->crtc->flip_state = IPU_FLIP_SUBMITTED;
> -}
> -
> -static int ipu_page_flip(struct drm_crtc *crtc,
> -		struct drm_framebuffer *fb,
> -		struct drm_pending_vblank_event *event,
> -		uint32_t page_flip_flags)
> -{
> -	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
> -	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
> -	struct ipu_flip_work *flip_work;
> -	int ret;
> -
> -	if (ipu_crtc->flip_state != IPU_FLIP_NONE)
> -		return -EBUSY;
> -
> -	ret = imx_drm_crtc_vblank_get(ipu_crtc->imx_crtc);
> -	if (ret) {
> -		dev_dbg(ipu_crtc->dev, "failed to acquire vblank counter\n");
> -		list_del(&event->base.link);
> -
> -		return ret;
> -	}
> -
> -	flip_work = kzalloc(sizeof *flip_work, GFP_KERNEL);
> -	if (!flip_work) {
> -		ret = -ENOMEM;
> -		goto put_vblank;
> -	}
> -	INIT_WORK(&flip_work->unref_work, ipu_flip_unref_work_func);
> -	flip_work->page_flip_event = event;
> -
> -	/* get BO backing the old framebuffer and take a reference */
> -	flip_work->bo = &drm_fb_cma_get_gem_obj(crtc->primary->fb, 0)->base;
> -	drm_gem_object_reference(flip_work->bo);
> -
> -	ipu_crtc->flip_work = flip_work;
> -	/*
> -	 * If the object has a DMABUF attached, we need to wait on its fences
> -	 * if there are any.
> -	 */
> -	if (cma_obj->base.dma_buf) {
> -		INIT_WORK(&flip_work->fence_work, ipu_flip_fence_work_func);
> -		flip_work->crtc = ipu_crtc;
> -
> -		ret = reservation_object_get_fences_rcu(
> -				cma_obj->base.dma_buf->resv, &flip_work->excl,
> -				&flip_work->shared_count, &flip_work->shared);
> -
> -		if (unlikely(ret)) {
> -			DRM_ERROR("failed to get fences for buffer\n");
> -			goto free_flip_work;
> -		}
> -
> -		/* No need to queue the worker if the are no fences */
> -		if (!flip_work->excl && !flip_work->shared_count) {
> -			ipu_crtc->flip_state = IPU_FLIP_SUBMITTED;
> -		} else {
> -			ipu_crtc->flip_state = IPU_FLIP_PENDING;
> -			queue_work(ipu_crtc->flip_queue,
> -				   &flip_work->fence_work);
> -		}
> -	} else {
> -		ipu_crtc->flip_state = IPU_FLIP_SUBMITTED;
> -	}
> -
> -	if (crtc->primary->state)
> -		drm_atomic_set_fb_for_plane(crtc->primary->state, fb);
> -
> -	return 0;
> -
> -free_flip_work:
> -	drm_gem_object_unreference_unlocked(flip_work->bo);
> -	kfree(flip_work);
> -	ipu_crtc->flip_work = NULL;
> -put_vblank:
> -	imx_drm_crtc_vblank_put(ipu_crtc->imx_crtc);
> -
> -	return ret;
> -}
> -
>  static const struct drm_crtc_funcs ipu_crtc_funcs = {
>  	.set_config = drm_atomic_helper_set_config,
>  	.destroy = drm_crtc_cleanup,
> -	.page_flip = ipu_page_flip,
> +	.page_flip = drm_atomic_helper_page_flip,
>  	.reset = drm_atomic_helper_crtc_reset,
>  	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>  };
>  
> -static void ipu_crtc_handle_pageflip(struct ipu_crtc *ipu_crtc)
> +static void ipu_crtc_handle_pageflip(struct drm_crtc *crtc)
>  {
> +	struct drm_device *drm = crtc->dev;
>  	unsigned long flags;
> -	struct drm_device *drm = ipu_crtc->base.dev;
> -	struct ipu_flip_work *work = ipu_crtc->flip_work;
>  
>  	spin_lock_irqsave(&drm->event_lock, flags);
> -	if (work->page_flip_event)
> -		drm_crtc_send_vblank_event(&ipu_crtc->base,
> -					   work->page_flip_event);
> -	imx_drm_crtc_vblank_put(ipu_crtc->imx_crtc);
> +	drm_crtc_send_vblank_event(crtc, crtc->state->event);
> +	crtc->state->event = NULL;
>  	spin_unlock_irqrestore(&drm->event_lock, flags);
>  }
>  
>  static irqreturn_t ipu_irq_handler(int irq, void *dev_id)
>  {
>  	struct ipu_crtc *ipu_crtc = dev_id;
> +	struct drm_crtc *crtc = &ipu_crtc->base;
>  
>  	imx_drm_handle_vblank(ipu_crtc->imx_crtc);
>  
> -	if (ipu_crtc->flip_state == IPU_FLIP_SUBMITTED) {
> -		struct ipu_plane *plane = ipu_crtc->plane[0];
> -
> -		ipu_plane_set_base(plane, ipu_crtc->base.primary->fb);
> -		ipu_crtc_handle_pageflip(ipu_crtc);
> -		queue_work(ipu_crtc->flip_queue,
> -			   &ipu_crtc->flip_work->unref_work);
> -		ipu_crtc->flip_state = IPU_FLIP_NONE;
> -	}
> +	if (crtc->state->event)
> +		ipu_crtc_handle_pageflip(crtc);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -458,8 +321,6 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
>  	/* Only enable IRQ when we actually need it to trigger work. */
>  	disable_irq(ipu_crtc->irq);
>  
> -	ipu_crtc->flip_queue = create_singlethread_workqueue("ipu-crtc-flip");
> -
>  	return 0;
>  
>  err_put_plane1_res:
> @@ -504,7 +365,6 @@ static void ipu_drm_unbind(struct device *dev, struct device *master,
>  
>  	imx_drm_remove_crtc(ipu_crtc->imx_crtc);
>  
> -	destroy_workqueue(ipu_crtc->flip_queue);
>  	ipu_put_resources(ipu_crtc);
>  	if (ipu_crtc->plane[1])
>  		ipu_plane_put_resources(ipu_crtc->plane[1]);
> -- 
> 2.7.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v2 09/10] drm/imx: atomic phase 3 step 4: Use generic atomic page flip
  2016-05-31  9:24 ` [PATCH v2 09/10] drm/imx: atomic phase 3 step 4: Use generic atomic page flip Liu Ying
  2016-05-31 11:03   ` Daniel Vetter
@ 2016-06-11 20:29   ` Daniel Vetter
  1 sibling, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2016-06-11 20:29 UTC (permalink / raw)
  To: Liu Ying; +Cc: Russell King, dri-devel

On Tue, May 31, 2016 at 11:24 AM, Liu Ying <gnuiyl@gmail.com> wrote:
> To support generic atomic page flip, this patch customizes ->atomic_commit
> for nonblock commits.
>
> Signed-off-by: Liu Ying <gnuiyl@gmail.com>
> ---
> v1->v2:
> * s/async/nonblock/ on this patch to address Daniel Vetter's comment.
> * Wait for pending commit on each CRTC for both block and nonblock
>   atomic mode settings.  This way, a block commit will not overwrite
>   the hardware setting when a nonblock page flip is about to finish,
>   so that the page flip may wait for vblank successfully.

Generic nonblocking commit just landed in drm-misc, which means you
can remove all your special imx commit functions and work items here.
Please do, less boilerplate in driver code is good ;-) Note that if
you don't handle crtc_state->event correctly and in all cases this
will results in some issues and errors in dmesg. But should be easy to
fix up your driver to be compliant.

Also another thing to double check after the conversion is done is
that you don't have any uncessary boilerplate code left:
- ipu_crtc->enabled checks in enable/disable can be remove, atomic
helpers already ensure that you don't get enabled/disabled twice
- all the legacy hooks like dpms, off, prepare and commit should
removed, instead use enable/disable (both crtc and encoder)
- best_encoder can be removed (except when you dynamically select
encoders, but then you need to switch to atomic_best_encoder and can
still remove best_encoder)
- mode_fixup can be removed if empty
- all other hooks which are empty can be removed too (but I didn't spot any)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

end of thread, other threads:[~2016-06-11 20:29 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-31  9:24 [PATCH v2 00/10] imx drm atomic mode setting conversion Liu Ying
2016-05-31  9:24 ` [PATCH v2 01/10] drm/imx: ipuv3 plane: Check different types of plane separately Liu Ying
2016-05-31  9:24 ` [PATCH v2 02/10] gpu: ipu-v3: ipu-dmfc: Use static DMFC FIFO allocation mechanism Liu Ying
2016-05-31  9:24 ` [PATCH v2 03/10] drm/imx: atomic phase 1: Use transitional atomic CRTC and plane helpers Liu Ying
2016-05-31  9:24 ` [PATCH v2 04/10] drm/imx: atomic phase 2 step 1: Wire up state ->reset, ->duplicate and ->destroy Liu Ying
2016-05-31  9:24 ` [PATCH v2 05/10] drm/imx: atomic phase 2 step 2: Track plane_state->fb correctly in ->page_flip Liu Ying
2016-05-31  9:24 ` [PATCH v2 06/10] drm/imx: atomic phase 3 step 1: Atomic updates for planes Liu Ying
2016-05-31  9:24 ` [PATCH v2 07/10] drm/imx: atomic phase 3 step 2: Use atomic configuration Liu Ying
2016-05-31  9:24 ` [PATCH v2 08/10] drm/imx: atomic phase 3 step 3: Legacy callback fixups Liu Ying
2016-05-31  9:24 ` [PATCH v2 09/10] drm/imx: atomic phase 3 step 4: Use generic atomic page flip Liu Ying
2016-05-31 11:03   ` Daniel Vetter
2016-06-11 20:29   ` Daniel Vetter
2016-05-31  9:24 ` [PATCH v2 10/10] drm/imx: atomic phase 3 step 5: Advertise DRIVER_ATOMIC Liu Ying

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