[PATCH v3 00/10] imx drm atomic mode setting conversion

[PATCH v3 00/10] imx drm atomic mode setting conversion

[Liu Ying]

Hi,

This is the v3 patch set to convert imx drm into atomic mode setting.
It takes 3 phases to achieve the goal.

v2->v3:
* Rebase onto Daniel Vetter's open git branch topic/drm-misc so that
  we may better support nonblock atomic commit with the aid from
  drm atomic helper.
* Remove dw-hdmi bridge driver's legacy drm_connector_funcs struture
  step-by-step instead of doing that in patch 04/10 directly.
  So, patch 08/10 in this set is newly introduced.

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: Remove encoders' ->prepare callbacks
  drm/imx: atomic phase 3 step 1: Use atomic configuration
  drm/bridge: dw-hdmi: Remove the legacy drm_connector_funcs structure
  drm/imx: atomic phase 3 step 2: Legacy callback fixups
  drm/imx: atomic phase 3 step 3: 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     | 120 +++++---
 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       | 370 ++++++----------------
 drivers/gpu/drm/imx/ipuv3-plane.c      | 543 ++++++++++++++++-----------------
 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, 592 insertions(+), 1028 deletions(-)

-- 
2.7.4

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

[PATCH v3 01/10] drm/imx: ipuv3 plane: Check different types of plane separately

[Liu Ying]

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>
---
v2->v3:
* None.

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

[PATCH v3 02/10] gpu: ipu-v3: ipu-dmfc: Use static DMFC FIFO allocation mechanism

[Liu Ying]

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>
---
v2->v3:
* None.

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

[PATCH v3 03/10] drm/imx: atomic phase 1: Use transitional atomic CRTC and plane helpers

[Liu Ying]

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>
---
v2->v3:
* A minor change to simplify the way we find crtc->enabled in
  drm_plane_helper_funcs->atomic_check.

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 | 511 +++++++++++++++++++++++---------------
 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, 410 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..59f2353 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,216 @@ 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_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 */
+	if (!state->crtc->enabled)
+		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 */
+		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 +578,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

Re: [PATCH v3 03/10] drm/imx: atomic phase 1: Use transitional atomic CRTC and plane helpers

[Russell King - ARM Linux]

On Mon, Jul 04, 2016 at 03:40:32PM +0800, Liu Ying wrote:
> 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>

This patch causes a regression with my xorg ddx driver:

[    29.850] (EE) armada(0): [drm] failed to set mode on crtc 24: Invalid argument

I'm not sure why (yet).

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 03/10] drm/imx: atomic phase 1: Use transitional atomic CRTC and plane helpers

[Russell King - ARM Linux]

On Sat, Aug 13, 2016 at 11:11:54AM +0100, Russell King - ARM Linux wrote:
> On Mon, Jul 04, 2016 at 03:40:32PM +0800, Liu Ying wrote:
> > 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>
> 
> This patch causes a regression with my xorg ddx driver:
> 
> [    29.850] (EE) armada(0): [drm] failed to set mode on crtc 24: Invalid argument
> 
> I'm not sure why (yet).

Hi,

Enabling DRM debugging doesn't seem to provide much in the way of clues:

[drm:drm_ioctl] pid=1015, dev=0xe200, auth=1, DRM_IOCTL_MODE_SETCRTC
[drm:drm_mode_setcrtc] [CRTC:24:crtc-0]
[drm:drm_mode_setcrtc] [CONNECTOR:34:HDMI-A-1]
[drm:drm_crtc_helper_set_config]
[drm:drm_crtc_helper_set_config] [CRTC:24:crtc-0] [FB:48] #connectors=1 (x y) (0 0)
[drm:drm_crtc_helper_set_config] [CONNECTOR:34:HDMI-A-1] to [CRTC:24:crtc-0]
[drm:drm_crtc_helper_set_config] attempting to set mode from userspace
[drm:drm_mode_debug_printmodeline] Modeline 49:"1920x1080" 0 148500 1920 2448 2492 2640 1080 1084 1089 1125 0x0 0x5
[drm:drm_crtc_helper_set_mode] [CRTC:24:crtc-0]
[drm:drm_vblank_off] crtc 0, vblank enabled 0, inmodeset 0
[drm:drm_update_vblank_count] updating vblank count on crtc 0: current=2, diff=0, hw=0 hw_last=0
[drm:drm_mode_object_reference] OBJ ID: 51 (1)
[drm:drm_mode_object_unreference] OBJ ID: 51 (2)
[drm:drm_atomic_set_mode_for_crtc] Set [MODE:1920x1080] for CRTC state ed81ee00
[drm:drm_mode_object_reference] OBJ ID: 47 (3)
[drm:drm_mode_object_unreference] OBJ ID: 47 (4)
[drm:drm_mode_object_reference] OBJ ID: 48 (2)
[drm:drm_atomic_set_fb_for_plane] Set [FB:48] for plane state ed632d00
[drm:drm_mode_object_unreference] OBJ ID: 48 (3)
[drm:drm_mode_object_unreference] OBJ ID: 51 (1)
[drm:drm_crtc_helper_set_config] *ERROR* failed to set mode on [CRTC:24:crtc-0]
[drm:drm_crtc_helper_set_mode] [CRTC:24:crtc-0]
[drm:drm_mode_object_reference] OBJ ID: 52 (1)
[drm:drm_mode_object_unreference] OBJ ID: 52 (2)
[drm:drm_atomic_set_mode_for_crtc] Set [MODE:1920x1080] for CRTC state ed81f600
[drm:drm_mode_object_reference] OBJ ID: 47 (3)
[drm:drm_mode_object_unreference] OBJ ID: 47 (4)
[drm:drm_mode_object_reference] OBJ ID: 47 (3)
[drm:drm_atomic_set_fb_for_plane] Set [FB:47] for plane state ed632d00
[drm:drm_mode_object_unreference] OBJ ID: 47 (4)
[drm:drm_mode_object_unreference] OBJ ID: 52 (1)
[drm:drm_crtc_helper_set_mode] [ENCODER:33:TMDS-33] set [MODE:46:1920x1080]
[drm:drm_vblank_on] crtc 0, vblank enabled 0, inmodeset 1
[drm:drm_calc_timestamping_constants] crtc 24: hwmode: htotal 2640, vtotal 1125, vdisplay 1080
[drm:drm_calc_timestamping_constants] crtc 24: clock 148500 kHz framedur 20000000 linedur 17777
[drm:drm_mode_object_reference] OBJ ID: 47 (3)
[drm:drm_mode_object_unreference] OBJ ID: 47 (4)
[drm:drm_mode_object_unreference] OBJ ID: 48 (2)
[drm:drm_mode_object_unreference] OBJ ID: 34 (4)
[drm:drm_ioctl] ret = -22

With a bit more debugging, this is what's failing:

ipu_plane_atomic_check:442: fail
[drm:drm_crtc_helper_set_config] *ERROR* failed to set mode on [CRTC:24:crtc-0]

        if (old_fb && (state->src_w != old_state->src_w ||
                              state->src_h != old_state->src_h ||
                              fb->pixel_format != old_fb->pixel_format)) {
                printk("%s:%d: fail\n", __func__, __LINE__);
                return -EINVAL;
        }

This test is stupid as it stands - it means we can _never_ change the
format or size of _any_ plane, something that the old code allowed:

-       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;

This used to work because we'd call ipu_crtc_prepare()->ipu_fb_disable()
before the mode set, which would clear ipu_plane->enabled, causing this
test to be skipped.  However, in the new code, we merely test for the
presence of the previous framebuffer, which is really not the same thing
at all.

The old functionality needs to be restored because significantly
changing the displayed mode is something which must be supported with
HDMI.

Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 03/10] drm/imx: atomic phase 1: Use transitional atomic CRTC and plane helpers

[Russell King - ARM Linux]

On Sat, Aug 13, 2016 at 11:45:31AM +0100, Russell King - ARM Linux wrote:
> On Sat, Aug 13, 2016 at 11:11:54AM +0100, Russell King - ARM Linux wrote:
> > On Mon, Jul 04, 2016 at 03:40:32PM +0800, Liu Ying wrote:
> > > 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>
> > 
> > This patch causes a regression with my xorg ddx driver:
> > 
> > [    29.850] (EE) armada(0): [drm] failed to set mode on crtc 24: Invalid argument
> > 
> > I'm not sure why (yet).
> 
> Hi,
> 
> Enabling DRM debugging doesn't seem to provide much in the way of clues:
> 
> [drm:drm_ioctl] pid=1015, dev=0xe200, auth=1, DRM_IOCTL_MODE_SETCRTC
> [drm:drm_mode_setcrtc] [CRTC:24:crtc-0]
> [drm:drm_mode_setcrtc] [CONNECTOR:34:HDMI-A-1]
> [drm:drm_crtc_helper_set_config]
> [drm:drm_crtc_helper_set_config] [CRTC:24:crtc-0] [FB:48] #connectors=1 (x y) (0 0)
> [drm:drm_crtc_helper_set_config] [CONNECTOR:34:HDMI-A-1] to [CRTC:24:crtc-0]
> [drm:drm_crtc_helper_set_config] attempting to set mode from userspace
> [drm:drm_mode_debug_printmodeline] Modeline 49:"1920x1080" 0 148500 1920 2448 2492 2640 1080 1084 1089 1125 0x0 0x5
> [drm:drm_crtc_helper_set_mode] [CRTC:24:crtc-0]
> [drm:drm_vblank_off] crtc 0, vblank enabled 0, inmodeset 0
> [drm:drm_update_vblank_count] updating vblank count on crtc 0: current=2, diff=0, hw=0 hw_last=0
> [drm:drm_mode_object_reference] OBJ ID: 51 (1)
> [drm:drm_mode_object_unreference] OBJ ID: 51 (2)
> [drm:drm_atomic_set_mode_for_crtc] Set [MODE:1920x1080] for CRTC state ed81ee00
> [drm:drm_mode_object_reference] OBJ ID: 47 (3)
> [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
> [drm:drm_mode_object_reference] OBJ ID: 48 (2)
> [drm:drm_atomic_set_fb_for_plane] Set [FB:48] for plane state ed632d00
> [drm:drm_mode_object_unreference] OBJ ID: 48 (3)
> [drm:drm_mode_object_unreference] OBJ ID: 51 (1)
> [drm:drm_crtc_helper_set_config] *ERROR* failed to set mode on [CRTC:24:crtc-0]
> [drm:drm_crtc_helper_set_mode] [CRTC:24:crtc-0]
> [drm:drm_mode_object_reference] OBJ ID: 52 (1)
> [drm:drm_mode_object_unreference] OBJ ID: 52 (2)
> [drm:drm_atomic_set_mode_for_crtc] Set [MODE:1920x1080] for CRTC state ed81f600
> [drm:drm_mode_object_reference] OBJ ID: 47 (3)
> [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
> [drm:drm_mode_object_reference] OBJ ID: 47 (3)
> [drm:drm_atomic_set_fb_for_plane] Set [FB:47] for plane state ed632d00
> [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
> [drm:drm_mode_object_unreference] OBJ ID: 52 (1)
> [drm:drm_crtc_helper_set_mode] [ENCODER:33:TMDS-33] set [MODE:46:1920x1080]
> [drm:drm_vblank_on] crtc 0, vblank enabled 0, inmodeset 1
> [drm:drm_calc_timestamping_constants] crtc 24: hwmode: htotal 2640, vtotal 1125, vdisplay 1080
> [drm:drm_calc_timestamping_constants] crtc 24: clock 148500 kHz framedur 20000000 linedur 17777
> [drm:drm_mode_object_reference] OBJ ID: 47 (3)
> [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
> [drm:drm_mode_object_unreference] OBJ ID: 48 (2)
> [drm:drm_mode_object_unreference] OBJ ID: 34 (4)
> [drm:drm_ioctl] ret = -22
> 
> With a bit more debugging, this is what's failing:
> 
> ipu_plane_atomic_check:442: fail
> [drm:drm_crtc_helper_set_config] *ERROR* failed to set mode on [CRTC:24:crtc-0]
> 
>         if (old_fb && (state->src_w != old_state->src_w ||
>                               state->src_h != old_state->src_h ||
>                               fb->pixel_format != old_fb->pixel_format)) {
>                 printk("%s:%d: fail\n", __func__, __LINE__);
>                 return -EINVAL;
>         }
> 
> This test is stupid as it stands - it means we can _never_ change the
> format or size of _any_ plane, something that the old code allowed:
> 
> -       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;
> 
> This used to work because we'd call ipu_crtc_prepare()->ipu_fb_disable()
> before the mode set, which would clear ipu_plane->enabled, causing this
> test to be skipped.  However, in the new code, we merely test for the
> presence of the previous framebuffer, which is really not the same thing
> at all.
> 
> The old functionality needs to be restored because significantly
> changing the displayed mode is something which must be supported with
> HDMI.

Bypassing the above check also shows that:

        if (old_fb && fb->pitches[0] != old_fb->pitches[0]) {
                printk("%s:%d: fail\n", __func__, __LINE__);
                return -EINVAL;
        }

also fails.  Disabling this as well results in loss of sync on the
HDMI display, although the mode set now succeeds.

The more I think about this, the more I come to one of two possible
conclusions:

1. These checks were not appropriate with the old code as we were
   always disabling the display channel prior to making changes.

   We need atomic mode set to work in exactly the same way as the
   previous code: as a series of disable-modify-enable set of steps,
   so that we don't need these restrictive and regression causing
   checks.

or

2. imx-drm has these particular restrictions which make it inappropriate
   to be converted to atomic mode set, as we always need to go through
   a disable-modify-enable series of steps - which means the atomic
   mode set changes for imx-drm need to be reverted back to this patch.

I'm pretty sure (2) doesn't really apply, because I see no reason why
we can't disable the display channel while we reconfigure it.  That,
to me, seems to be an entirely reasonable thing to do.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 03/10] drm/imx: atomic phase 1: Use transitional atomic CRTC and plane helpers

[Russell King - ARM Linux]

Okay, this is what I've ended up with - I'm not sure whether it's
correct or not, but this dirty patch allows the full series to be
applied and still have working userspace.

I still need to undo all the reverts I have touching imx-drm between
patch 10 of this set and 4.8-rc1...

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 3f5f9566b152..6f9a43880381 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -275,7 +275,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 	if (WARN_ON(!crtc_state))
 		return -EINVAL;
 
-	/* CRTC should be enabled */
+	/* CRTC should be enabled -- why? */
 	if (!crtc_state->enable)
 		return -EINVAL;
 
@@ -319,10 +319,14 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 	 * since we cannot touch active IDMAC channels, we do not support
 	 * resizing the enabled plane or changing its format
 	 */
+#if 0
 	if (old_fb && (state->src_w != old_state->src_w ||
 			      state->src_h != old_state->src_h ||
-			      fb->pixel_format != old_fb->pixel_format))
+			      fb->pixel_format != old_fb->pixel_format)) {
+		printk("%s:%d: fail\n", __func__, __LINE__);
 		return -EINVAL;
+	}
+#endif
 
 	eba = drm_plane_state_to_eba(state);
 
@@ -332,8 +336,12 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 	if (fb->pitches[0] < 1 || fb->pitches[0] > 16384)
 		return -EINVAL;
 
-	if (old_fb && fb->pitches[0] != old_fb->pitches[0])
+#if 0
+	if (old_fb && fb->pitches[0] != old_fb->pitches[0]) {
+		printk("%s:%d: fail\n", __func__, __LINE__);
 		return -EINVAL;
+	}
+#endif
 
 	switch (fb->pixel_format) {
 	case DRM_FORMAT_YUV420:
@@ -388,10 +396,14 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
 	struct drm_plane_state *state = plane->state;
 	enum ipu_color_space ics;
 
+#if 0
 	if (old_state->fb) {
 		ipu_plane_atomic_set_base(ipu_plane, old_state);
 		return;
 	}
+#endif
+
+	ipu_plane_disable(ipu_plane);
 
 	switch (ipu_plane->dp_flow) {
 	case IPU_DP_FLOW_SYNC_BG:

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 03/10] drm/imx: atomic phase 1: Use transitional atomic CRTC and plane helpers

[Russell King - ARM Linux]

On Sat, Aug 13, 2016 at 03:09:10PM +0100, Russell King - ARM Linux wrote:
> Okay, this is what I've ended up with - I'm not sure whether it's
> correct or not, but this dirty patch allows the full series to be
> applied and still have working userspace.
> 
> I still need to undo all the reverts I have touching imx-drm between
> patch 10 of this set and 4.8-rc1...

I tried a slightly different approach, so we at least get the simple
plane updates without having to go through the disable path:

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 4ad67d015ec7..1cd3422e6e62 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -278,7 +278,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 	if (WARN_ON(!crtc_state))
 		return -EINVAL;
 
-	/* CRTC should be enabled */
+	/* CRTC should be enabled -- why? */
 	if (!crtc_state->enable)
 		return -EINVAL;
 
@@ -322,10 +322,14 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 	 * since we cannot touch active IDMAC channels, we do not support
 	 * resizing the enabled plane or changing its format
 	 */
+#if 0
 	if (old_fb && (state->src_w != old_state->src_w ||
 			      state->src_h != old_state->src_h ||
-			      fb->pixel_format != old_fb->pixel_format))
+			      fb->pixel_format != old_fb->pixel_format)) {
+		printk("%s:%d: fail\n", __func__, __LINE__);
 		return -EINVAL;
+	}
+#endif
 
 	eba = drm_plane_state_to_eba(state);
 
@@ -335,9 +339,6 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 	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:
@@ -391,11 +392,17 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
 	struct drm_plane_state *state = plane->state;
 	enum ipu_color_space ics;
 
-	if (old_state->fb) {
+	if (old_state->fb &&
+	    old_state->src_w == state->src_w &&
+	    old_state->src_h == state->src_h &&
+	    old_state->fb->pixel_format == state->fb->pixel_format &&
+	    old_state->fb->pitches[0] == state->fb->pitches[0]) {
 		ipu_plane_atomic_set_base(ipu_plane, old_state);
 		return;
 	}
 
+	ipu_plane_disable(ipu_plane);
+
 	switch (ipu_plane->dp_flow) {
 	case IPU_DP_FLOW_SYNC_BG:
 		ipu_dp_setup_channel(ipu_plane->dp,

but unfortunately this leads to a kernel oops.  From what I can tell,
lock->class_cache[subclass] was 0xffffffff.  subclass was 0, lock
was 0xec58631c.

[drm:drm_atomic_helper_commit_cleanup_done] *ERROR* [CRTC:24:crtc-0] flip_done timed out
Alignment trap: not handling instruction e1921f9f at [<c007fe4c>][  651.472496] Unhandled fault: alignment exception (0x001) at 0x00000103
pgd = ecea4000
[00000103] *pgd=00000000
Internal error: : 1 [#1] SMP ARM
Modules linked in: bnep rfcomm bluetooth nfsd rc_cec snd_soc_fsl_spdif caam_jr coda imx_pcm_dma imx_sdma v4l2_mem2mem videobuf2_dma_contig imx2_wdt videobuf2_vmalloc dw_hdmi_cec imx_thermal dw_hdmi_ahb_audio videobuf2_memops caam etnaviv snd_soc_imx_spdif
CPU: 0 PID: 1091 Comm: dbus-daemon Not tainted 4.8.0-rc1+ #2043
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
task: ee3755c0 task.stack: ece74000
PC is at __lock_acquire+0xd4/0x18fc
LR is at lock_acquire+0xd8/0x250
pc : [<c007fe50>]    lr : [<c0081bd0>]    psr: a0070193
sp : ece75730  ip : ece74000  fp : ece757b4
r10: 00000000  r9 : c0acbef0  r8 : ec58631c
r7 : 00000001  r6 : ee3755c0  r5 : c0a925f8  r4 : c125ad8c
r3 : 00000000  r2 : 00000103  r1 : 00000000  r0 : ffffffff
Flags: NzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 3cea404a  DAC: 00000051
Process dbus-daemon (pid: 1091, stack limit = 0xece74210)
Stack: (0xece75730 to 0xece76000)
(omitted)
Backtrace:
[<c007fd7c>] (__lock_acquire) from [<c0081bd0>] (lock_acquire+0xd8/0x250)
 r10:00000000 r9:c0acbef0 r8:00000000 r7:00000000 r6:ec58631c r5:60070193
 r4:00000000
[<c0081af8>] (lock_acquire) from [<c074d9cc>] (_raw_spin_lock_irqsave+0x4c/0x60)
 r10:ee1ca000 r9:0000040a r8:00000000 r7:00000000 r6:c007a48c r5:a0070193
 r4:ec58630c
[<c074d980>] (_raw_spin_lock_irqsave) from [<c007a48c>] (complete_all+0x1c/0x4c)
 r6:c0acdd18 r5:ec586308 r4:ec58630c
[<c007a470>] (complete_all) from [<c03fb480>] (drm_send_event_locked+0x30/0x104)
 r6:c0acdd18 r5:c0acb61d r4:ebc57200 r3:04240423
[<c03fb450>] (drm_send_event_locked) from [<c03fe4ac>] (send_vblank_event+0x94/0x1bc)
 r5:c0acb61d r4:ebc57200
[<c03fe418>] (send_vblank_event) from [<c03feda0>] (drm_handle_vblank+0x158/0x364)
 r10:ee1ca23c r9:ee1ca234 r8:0000040a r7:ee1ca000 r6:00000000 r5:ee1ca228
 r4:ebc57200
[<c03fec48>] (drm_handle_vblank) from [<c03fefc8>] (drm_crtc_handle_vblank+0x1c/0x20)
 r10:ece75964 r9:c0a3d554 r8:00000000 r7:0000012f r6:ee114a10 r5:ee114a00
 r4:ee055940
[<c03fefac>] (drm_crtc_handle_vblank) from [<c041ace4>] (imx_drm_handle_vblank+0x14/0x18)
[<c041acd0>] (imx_drm_handle_vblank) from [<c041b4c8>] (ipu_irq_handler+0x14/0x1c)
[<c041b4b4>] (ipu_irq_handler) from [<c0096630>] (__handle_irq_event_percpu+0xa4/0x428)
[<c009658c>] (__handle_irq_event_percpu) from [<c00969d8>] (handle_irq_event_percpu+0x24/0x60)
 r10:ece75ab0 r9:f4001100 r8:00000009 r7:00000000 r6:ee114a10 r5:ee114a00
 r4:ee114a00
[<c00969b4>] (handle_irq_event_percpu) from [<c0096a54>] (handle_irq_event+0x40/0x64)
 r5:ee114a60 r4:ee114a00
[<c0096a14>] (handle_irq_event) from [<c009a1c4>] (handle_level_irq+0xb0/0x138)
 r6:ee114a10 r5:ee114a60 r4:ee114a00 r3:c0a323d8
[<c009a114>] (handle_level_irq) from [<c0095df0>] (generic_handle_irq+0x20/0x30)
 r6:ee146810 r5:ece75a08 r4:00000017 r3:c009a114
[<c0095dd0>] (generic_handle_irq) from [<c041f344>] (ipu_irq_handle+0xa8/0xd8)
[<c041f29c>] (ipu_irq_handle) from [<c041f474>] (ipu_irq_handler+0x5c/0xb4)
 r8:ee820000 r7:00000026 r6:ee146810 r5:c0a44288 r4:eea46f10
[<c041f418>] (ipu_irq_handler) from [<c0095df0>] (generic_handle_irq+0x20/0x30)
 r6:00000000 r5:ece75c68 r4:c0a37240
[<c0095dd0>] (generic_handle_irq) from [<c0095f24>] (__handle_domain_irq+0x5c/0xb8)
[<c0095ec8>] (__handle_domain_irq) from [<c00094c8>] (gic_handle_irq+0x4c/0x9c)
 r8:c0a92950 r7:000003eb r6:c0a3dba4 r5:f400010c r4:f4000100 r3:ece75ab0
[<c000947c>] (gic_handle_irq) from [<c0014930>] (__irq_svc+0x70/0x98)
Exception stack(0xece75ab0 to 0xece75af8)
5aa0:                                     00000001 ee375a70 00000000 60070193
5ac0: 20070113 c0a4b910 00000001 00000002 00000002 c0a4b680 c0acbc34 ece75b14
5ae0: ece75ad0 ece75b00 c00821a0 c074e028 60070113 ffffffff
 r10:c0acbc34 r9:ece74000 r8:00000002 r7:ece75ae4 r6:ffffffff r5:60070113
 r4:c074e028 r3:ee3755c0
[<c074dfec>] (_raw_spin_unlock_irqrestore) from [<c007a388>] (swake_up+0x3c/0x40)
 r5:20070113 r4:c0a4b910
[<c007a34c>] (swake_up) from [<c00a0820>] (rcu_gp_kthread_wake+0x48/0x4c)
 r5:60070113 r4:eef85340
[<c00a07d8>] (rcu_gp_kthread_wake) from [<c00a2d98>] (rcu_process_callbacks+0x3a8/0x93c)
[<c00a29f0>] (rcu_process_callbacks) from [<c0034cb8>] (__do_softirq+0xe8/0x5e4)
 r10:c0acbc34 r9:00000024 r8:00000002 r7:00000002 r6:c0a3d554 r5:00000009
 r4:c0a3d0a4
[<c0034bd0>] (__do_softirq) from [<c0035534>] (irq_exit+0xe4/0x150)
 r10:ece75c68 r9:f4001100 r8:ee820000 r7:0000001d r6:00000000 r5:00000000
 r4:c0a37240
[<c0035450>] (irq_exit) from [<c0095f28>] (__handle_domain_irq+0x60/0xb8)
[<c0095ec8>] (__handle_domain_irq) from [<c00094c8>] (gic_handle_irq+0x4c/0x9c)
 r8:c0a92950 r7:000003eb r6:c0a3dba4 r5:f400010c r4:f4000100 r3:ece75c68
[<c000947c>] (gic_handle_irq) from [<c0014930>] (__irq_svc+0x70/0x98)
Exception stack(0xece75c68 to 0xece75cb0)
5c60:                   00000001 ece75eb0 ece75ca8 ee765748 ee765748 ee4357e8
5c80: ed7e9011 eeefa000 c0acb1f6 00002081 ece75eb0 ece75cf4 ece75cb8 ece75cb8
5ca0: c0181764 c0181838 60070013 ffffffff
 r10:ece75eb0 r9:ece74000 r8:c0acb1f6 r7:ece75c9c r6:ffffffff r5:60070013
 r4:c0181838 r3:ee3755c0
[<c0181724>] (__d_lookup_rcu) from [<c0171da0>] (lookup_fast+0x48/0x384)
 r10:ece75d5c r9:ee955910 r8:ece75d58 r7:ee4357e8 r6:00000000 r5:ece75d60
 r4:ece75ea8
[<c0171d58>] (lookup_fast) from [<c0174644>] (walk_component+0x34/0x284)
 r10:90954373 r9:ff010301 r8:ece75ea8 r7:00000001 r6:00000000 r5:2f727375
 r4:ece75ea8
[<c0174610>] (walk_component) from [<c0174a30>] (link_path_walk+0x19c/0x4b8)
 r7:ed7e9015 r6:ee4357e8 r5:2f727375 r4:7fffffff
[<c0174894>] (link_path_walk) from [<c017514c>] (path_openat+0x70/0xee0)
 r10:ece75ea8 r9:ece74000 r8:ece75f5c r7:00000003 r6:ece75ea8 r5:ece75f5c
 r4:00000046
[<c01750dc>] (path_openat) from [<c0176dd0>] (do_filp_open+0x68/0xbc)
 r10:00000000 r9:ece74000 r8:c000ff44 r7:00000003 r6:ece75ea8 r5:ece75f5c
 r4:00000046
[<c0176d68>] (do_filp_open) from [<c0165b4c>] (do_sys_open+0x120/0x1d4)
 r7:00000142 r6:ffffff9c r5:ed7e9000 r4:00000046
[<c0165a2c>] (do_sys_open) from [<c0165c3c>] (SyS_openat+0x14/0x18)
 r10:00000000 r8:c000ff44 r7:00000142 r6:801462c8 r5:bed94468 r4:80146450
[<c0165c28>] (SyS_openat) from [<c000fda0>] (ret_fast_syscall+0x0/0x1c)
Code: 0affffe9 e2802f41 f592f000 e1921f9f (e2811001)
---[ end trace 8ffd323ac322f062 ]---

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 03/10] drm/imx: atomic phase 1: Use transitional atomic CRTC and plane helpers

[Daniel Vetter]

On Sat, Aug 13, 2016 at 12:29:47PM +0100, Russell King - ARM Linux wrote:
> On Sat, Aug 13, 2016 at 11:45:31AM +0100, Russell King - ARM Linux wrote:
> > On Sat, Aug 13, 2016 at 11:11:54AM +0100, Russell King - ARM Linux wrote:
> > > On Mon, Jul 04, 2016 at 03:40:32PM +0800, Liu Ying wrote:
> > > > 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>
> > > 
> > > This patch causes a regression with my xorg ddx driver:
> > > 
> > > [    29.850] (EE) armada(0): [drm] failed to set mode on crtc 24: Invalid argument
> > > 
> > > I'm not sure why (yet).
> > 
> > Hi,
> > 
> > Enabling DRM debugging doesn't seem to provide much in the way of clues:
> > 
> > [drm:drm_ioctl] pid=1015, dev=0xe200, auth=1, DRM_IOCTL_MODE_SETCRTC
> > [drm:drm_mode_setcrtc] [CRTC:24:crtc-0]
> > [drm:drm_mode_setcrtc] [CONNECTOR:34:HDMI-A-1]
> > [drm:drm_crtc_helper_set_config]
> > [drm:drm_crtc_helper_set_config] [CRTC:24:crtc-0] [FB:48] #connectors=1 (x y) (0 0)
> > [drm:drm_crtc_helper_set_config] [CONNECTOR:34:HDMI-A-1] to [CRTC:24:crtc-0]
> > [drm:drm_crtc_helper_set_config] attempting to set mode from userspace
> > [drm:drm_mode_debug_printmodeline] Modeline 49:"1920x1080" 0 148500 1920 2448 2492 2640 1080 1084 1089 1125 0x0 0x5
> > [drm:drm_crtc_helper_set_mode] [CRTC:24:crtc-0]
> > [drm:drm_vblank_off] crtc 0, vblank enabled 0, inmodeset 0
> > [drm:drm_update_vblank_count] updating vblank count on crtc 0: current=2, diff=0, hw=0 hw_last=0
> > [drm:drm_mode_object_reference] OBJ ID: 51 (1)
> > [drm:drm_mode_object_unreference] OBJ ID: 51 (2)
> > [drm:drm_atomic_set_mode_for_crtc] Set [MODE:1920x1080] for CRTC state ed81ee00
> > [drm:drm_mode_object_reference] OBJ ID: 47 (3)
> > [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
> > [drm:drm_mode_object_reference] OBJ ID: 48 (2)
> > [drm:drm_atomic_set_fb_for_plane] Set [FB:48] for plane state ed632d00
> > [drm:drm_mode_object_unreference] OBJ ID: 48 (3)
> > [drm:drm_mode_object_unreference] OBJ ID: 51 (1)
> > [drm:drm_crtc_helper_set_config] *ERROR* failed to set mode on [CRTC:24:crtc-0]
> > [drm:drm_crtc_helper_set_mode] [CRTC:24:crtc-0]
> > [drm:drm_mode_object_reference] OBJ ID: 52 (1)
> > [drm:drm_mode_object_unreference] OBJ ID: 52 (2)
> > [drm:drm_atomic_set_mode_for_crtc] Set [MODE:1920x1080] for CRTC state ed81f600
> > [drm:drm_mode_object_reference] OBJ ID: 47 (3)
> > [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
> > [drm:drm_mode_object_reference] OBJ ID: 47 (3)
> > [drm:drm_atomic_set_fb_for_plane] Set [FB:47] for plane state ed632d00
> > [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
> > [drm:drm_mode_object_unreference] OBJ ID: 52 (1)
> > [drm:drm_crtc_helper_set_mode] [ENCODER:33:TMDS-33] set [MODE:46:1920x1080]
> > [drm:drm_vblank_on] crtc 0, vblank enabled 0, inmodeset 1
> > [drm:drm_calc_timestamping_constants] crtc 24: hwmode: htotal 2640, vtotal 1125, vdisplay 1080
> > [drm:drm_calc_timestamping_constants] crtc 24: clock 148500 kHz framedur 20000000 linedur 17777
> > [drm:drm_mode_object_reference] OBJ ID: 47 (3)
> > [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
> > [drm:drm_mode_object_unreference] OBJ ID: 48 (2)
> > [drm:drm_mode_object_unreference] OBJ ID: 34 (4)
> > [drm:drm_ioctl] ret = -22
> > 
> > With a bit more debugging, this is what's failing:
> > 
> > ipu_plane_atomic_check:442: fail
> > [drm:drm_crtc_helper_set_config] *ERROR* failed to set mode on [CRTC:24:crtc-0]
> > 
> >         if (old_fb && (state->src_w != old_state->src_w ||
> >                               state->src_h != old_state->src_h ||
> >                               fb->pixel_format != old_fb->pixel_format)) {
> >                 printk("%s:%d: fail\n", __func__, __LINE__);
> >                 return -EINVAL;
> >         }
> > 
> > This test is stupid as it stands - it means we can _never_ change the
> > format or size of _any_ plane, something that the old code allowed:
> > 
> > -       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;
> > 
> > This used to work because we'd call ipu_crtc_prepare()->ipu_fb_disable()
> > before the mode set, which would clear ipu_plane->enabled, causing this
> > test to be skipped.  However, in the new code, we merely test for the
> > presence of the previous framebuffer, which is really not the same thing
> > at all.
> > 
> > The old functionality needs to be restored because significantly
> > changing the displayed mode is something which must be supported with
> > HDMI.
> 
> Bypassing the above check also shows that:
> 
>         if (old_fb && fb->pitches[0] != old_fb->pitches[0]) {
>                 printk("%s:%d: fail\n", __func__, __LINE__);
>                 return -EINVAL;
>         }
> 
> also fails.  Disabling this as well results in loss of sync on the
> HDMI display, although the mode set now succeeds.
> 
> The more I think about this, the more I come to one of two possible
> conclusions:
> 
> 1. These checks were not appropriate with the old code as we were
>    always disabling the display channel prior to making changes.
> 
>    We need atomic mode set to work in exactly the same way as the
>    previous code: as a series of disable-modify-enable set of steps,
>    so that we don't need these restrictive and regression causing
>    checks.
> 
> or
> 
> 2. imx-drm has these particular restrictions which make it inappropriate
>    to be converted to atomic mode set, as we always need to go through
>    a disable-modify-enable series of steps - which means the atomic
>    mode set changes for imx-drm need to be reverted back to this patch.
> 
> I'm pretty sure (2) doesn't really apply, because I see no reason why
> we can't disable the display channel while we reconfigure it.  That,
> to me, seems to be an entirely reasonable thing to do.

Already ddiscussed this at lenght on irc with Peter Senna. If you have
plane update restrictions where you need to cycle the crtc completely,
the right thing to do is to set crtc_state->mode_changed instead of return
-EINVAL. The helper/core will then convert that into an EINVAL if
userspace doesn't set ALLOW_MODESET. And since the helper function to map
set_config to atomic does this, it should all just work.

Note: There's about 3 such conditions in imx' atomic_check function which
needs this change.
-Daniel
-- 
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

Re: [PATCH v3 03/10] drm/imx: atomic phase 1: Use transitional atomic CRTC and plane helpers

[Daniel Vetter]

On Sun, Aug 14, 2016 at 11:44:14AM +0200, Daniel Vetter wrote:
> On Sat, Aug 13, 2016 at 12:29:47PM +0100, Russell King - ARM Linux wrote:
> > On Sat, Aug 13, 2016 at 11:45:31AM +0100, Russell King - ARM Linux wrote:
> > > On Sat, Aug 13, 2016 at 11:11:54AM +0100, Russell King - ARM Linux wrote:
> > > > On Mon, Jul 04, 2016 at 03:40:32PM +0800, Liu Ying wrote:
> > > > > 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>
> > > >
> > > > This patch causes a regression with my xorg ddx driver:
> > > >
> > > > [    29.850] (EE) armada(0): [drm] failed to set mode on crtc 24: Invalid argument
> > > >
> > > > I'm not sure why (yet).
> > >
> > > Hi,
> > >
> > > Enabling DRM debugging doesn't seem to provide much in the way of clues:
> > >
> > > [drm:drm_ioctl] pid=1015, dev=0xe200, auth=1, DRM_IOCTL_MODE_SETCRTC
> > > [drm:drm_mode_setcrtc] [CRTC:24:crtc-0]
> > > [drm:drm_mode_setcrtc] [CONNECTOR:34:HDMI-A-1]
> > > [drm:drm_crtc_helper_set_config]
> > > [drm:drm_crtc_helper_set_config] [CRTC:24:crtc-0] [FB:48] #connectors=1 (x y) (0 0)
> > > [drm:drm_crtc_helper_set_config] [CONNECTOR:34:HDMI-A-1] to [CRTC:24:crtc-0]
> > > [drm:drm_crtc_helper_set_config] attempting to set mode from userspace
> > > [drm:drm_mode_debug_printmodeline] Modeline 49:"1920x1080" 0 148500 1920 2448 2492 2640 1080 1084 1089 1125 0x0 0x5
> > > [drm:drm_crtc_helper_set_mode] [CRTC:24:crtc-0]
> > > [drm:drm_vblank_off] crtc 0, vblank enabled 0, inmodeset 0
> > > [drm:drm_update_vblank_count] updating vblank count on crtc 0: current=2, diff=0, hw=0 hw_last=0
> > > [drm:drm_mode_object_reference] OBJ ID: 51 (1)
> > > [drm:drm_mode_object_unreference] OBJ ID: 51 (2)
> > > [drm:drm_atomic_set_mode_for_crtc] Set [MODE:1920x1080] for CRTC state ed81ee00
> > > [drm:drm_mode_object_reference] OBJ ID: 47 (3)
> > > [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
> > > [drm:drm_mode_object_reference] OBJ ID: 48 (2)
> > > [drm:drm_atomic_set_fb_for_plane] Set [FB:48] for plane state ed632d00
> > > [drm:drm_mode_object_unreference] OBJ ID: 48 (3)
> > > [drm:drm_mode_object_unreference] OBJ ID: 51 (1)
> > > [drm:drm_crtc_helper_set_config] *ERROR* failed to set mode on [CRTC:24:crtc-0]
> > > [drm:drm_crtc_helper_set_mode] [CRTC:24:crtc-0]
> > > [drm:drm_mode_object_reference] OBJ ID: 52 (1)
> > > [drm:drm_mode_object_unreference] OBJ ID: 52 (2)
> > > [drm:drm_atomic_set_mode_for_crtc] Set [MODE:1920x1080] for CRTC state ed81f600
> > > [drm:drm_mode_object_reference] OBJ ID: 47 (3)
> > > [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
> > > [drm:drm_mode_object_reference] OBJ ID: 47 (3)
> > > [drm:drm_atomic_set_fb_for_plane] Set [FB:47] for plane state ed632d00
> > > [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
> > > [drm:drm_mode_object_unreference] OBJ ID: 52 (1)
> > > [drm:drm_crtc_helper_set_mode] [ENCODER:33:TMDS-33] set [MODE:46:1920x1080]
> > > [drm:drm_vblank_on] crtc 0, vblank enabled 0, inmodeset 1
> > > [drm:drm_calc_timestamping_constants] crtc 24: hwmode: htotal 2640, vtotal 1125, vdisplay 1080
> > > [drm:drm_calc_timestamping_constants] crtc 24: clock 148500 kHz framedur 20000000 linedur 17777
> > > [drm:drm_mode_object_reference] OBJ ID: 47 (3)
> > > [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
> > > [drm:drm_mode_object_unreference] OBJ ID: 48 (2)
> > > [drm:drm_mode_object_unreference] OBJ ID: 34 (4)
> > > [drm:drm_ioctl] ret = -22
> > >
> > > With a bit more debugging, this is what's failing:
> > >
> > > ipu_plane_atomic_check:442: fail
> > > [drm:drm_crtc_helper_set_config] *ERROR* failed to set mode on [CRTC:24:crtc-0]
> > >
> > >         if (old_fb && (state->src_w != old_state->src_w ||
> > >                               state->src_h != old_state->src_h ||
> > >                               fb->pixel_format != old_fb->pixel_format)) {
> > >                 printk("%s:%d: fail\n", __func__, __LINE__);
> > >                 return -EINVAL;
> > >         }
> > >
> > > This test is stupid as it stands - it means we can _never_ change the
> > > format or size of _any_ plane, something that the old code allowed:
> > >
> > > -       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;
> > >
> > > This used to work because we'd call ipu_crtc_prepare()->ipu_fb_disable()
> > > before the mode set, which would clear ipu_plane->enabled, causing this
> > > test to be skipped.  However, in the new code, we merely test for the
> > > presence of the previous framebuffer, which is really not the same thing
> > > at all.
> > >
> > > The old functionality needs to be restored because significantly
> > > changing the displayed mode is something which must be supported with
> > > HDMI.
> >
> > Bypassing the above check also shows that:
> >
> >         if (old_fb && fb->pitches[0] != old_fb->pitches[0]) {
> >                 printk("%s:%d: fail\n", __func__, __LINE__);
> >                 return -EINVAL;
> >         }
> >
> > also fails.  Disabling this as well results in loss of sync on the
> > HDMI display, although the mode set now succeeds.
> >
> > The more I think about this, the more I come to one of two possible
> > conclusions:
> >
> > 1. These checks were not appropriate with the old code as we were
> >    always disabling the display channel prior to making changes.
> >
> >    We need atomic mode set to work in exactly the same way as the
> >    previous code: as a series of disable-modify-enable set of steps,
> >    so that we don't need these restrictive and regression causing
> >    checks.
> >
> > or
> >
> > 2. imx-drm has these particular restrictions which make it inappropriate
> >    to be converted to atomic mode set, as we always need to go through
> >    a disable-modify-enable series of steps - which means the atomic
> >    mode set changes for imx-drm need to be reverted back to this patch.
> >
> > I'm pretty sure (2) doesn't really apply, because I see no reason why
> > we can't disable the display channel while we reconfigure it.  That,
> > to me, seems to be an entirely reasonable thing to do.
>
> Already ddiscussed this at lenght on irc with Peter Senna. If you have
> plane update restrictions where you need to cycle the crtc completely,
> the right thing to do is to set crtc_state->mode_changed instead of return
> -EINVAL. The helper/core will then convert that into an EINVAL if
> userspace doesn't set ALLOW_MODESET. And since the helper function to map
> set_config to atomic does this, it should all just work.
>
> Note: There's about 3 such conditions in imx' atomic_check function which
> needs this change.

You also need to update the overall atomic_check to re-run the modeset
checks after the plane checks again (since plane checks can update
mode_changed). Peter Senna has the full diff, but apparently it doesn't
work either. So there's probably more bugs somewhere.
-Daniel
-- 
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

Re: [PATCH v3 03/10] drm/imx: atomic phase 1: Use transitional atomic CRTC and plane helpers

[Peter Senna Tschudin]

[-- Attachment #1: Type: text/plain, Size: 8070 bytes --]

On Sun, Aug 14, 2016 at 12:46:27PM +0200, Daniel Vetter wrote:
> On Sun, Aug 14, 2016 at 11:44:14AM +0200, Daniel Vetter wrote:
> > On Sat, Aug 13, 2016 at 12:29:47PM +0100, Russell King - ARM Linux wrote:
> > > On Sat, Aug 13, 2016 at 11:45:31AM +0100, Russell King - ARM Linux wrote:
> > > > On Sat, Aug 13, 2016 at 11:11:54AM +0100, Russell King - ARM Linux wrote:
> > > > > On Mon, Jul 04, 2016 at 03:40:32PM +0800, Liu Ying wrote:
> > > > > > 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>
> > > > >
> > > > > This patch causes a regression with my xorg ddx driver:
> > > > >
> > > > > [    29.850] (EE) armada(0): [drm] failed to set mode on crtc 24: Invalid argument
> > > > >
> > > > > I'm not sure why (yet).
> > > >
> > > > Hi,
> > > >
> > > > Enabling DRM debugging doesn't seem to provide much in the way of clues:
> > > >
> > > > [drm:drm_ioctl] pid=1015, dev=0xe200, auth=1, DRM_IOCTL_MODE_SETCRTC
> > > > [drm:drm_mode_setcrtc] [CRTC:24:crtc-0]
> > > > [drm:drm_mode_setcrtc] [CONNECTOR:34:HDMI-A-1]
> > > > [drm:drm_crtc_helper_set_config]
> > > > [drm:drm_crtc_helper_set_config] [CRTC:24:crtc-0] [FB:48] #connectors=1 (x y) (0 0)
> > > > [drm:drm_crtc_helper_set_config] [CONNECTOR:34:HDMI-A-1] to [CRTC:24:crtc-0]
> > > > [drm:drm_crtc_helper_set_config] attempting to set mode from userspace
> > > > [drm:drm_mode_debug_printmodeline] Modeline 49:"1920x1080" 0 148500 1920 2448 2492 2640 1080 1084 1089 1125 0x0 0x5
> > > > [drm:drm_crtc_helper_set_mode] [CRTC:24:crtc-0]
> > > > [drm:drm_vblank_off] crtc 0, vblank enabled 0, inmodeset 0
> > > > [drm:drm_update_vblank_count] updating vblank count on crtc 0: current=2, diff=0, hw=0 hw_last=0
> > > > [drm:drm_mode_object_reference] OBJ ID: 51 (1)
> > > > [drm:drm_mode_object_unreference] OBJ ID: 51 (2)
> > > > [drm:drm_atomic_set_mode_for_crtc] Set [MODE:1920x1080] for CRTC state ed81ee00
> > > > [drm:drm_mode_object_reference] OBJ ID: 47 (3)
> > > > [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
> > > > [drm:drm_mode_object_reference] OBJ ID: 48 (2)
> > > > [drm:drm_atomic_set_fb_for_plane] Set [FB:48] for plane state ed632d00
> > > > [drm:drm_mode_object_unreference] OBJ ID: 48 (3)
> > > > [drm:drm_mode_object_unreference] OBJ ID: 51 (1)
> > > > [drm:drm_crtc_helper_set_config] *ERROR* failed to set mode on [CRTC:24:crtc-0]
> > > > [drm:drm_crtc_helper_set_mode] [CRTC:24:crtc-0]
> > > > [drm:drm_mode_object_reference] OBJ ID: 52 (1)
> > > > [drm:drm_mode_object_unreference] OBJ ID: 52 (2)
> > > > [drm:drm_atomic_set_mode_for_crtc] Set [MODE:1920x1080] for CRTC state ed81f600
> > > > [drm:drm_mode_object_reference] OBJ ID: 47 (3)
> > > > [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
> > > > [drm:drm_mode_object_reference] OBJ ID: 47 (3)
> > > > [drm:drm_atomic_set_fb_for_plane] Set [FB:47] for plane state ed632d00
> > > > [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
> > > > [drm:drm_mode_object_unreference] OBJ ID: 52 (1)
> > > > [drm:drm_crtc_helper_set_mode] [ENCODER:33:TMDS-33] set [MODE:46:1920x1080]
> > > > [drm:drm_vblank_on] crtc 0, vblank enabled 0, inmodeset 1
> > > > [drm:drm_calc_timestamping_constants] crtc 24: hwmode: htotal 2640, vtotal 1125, vdisplay 1080
> > > > [drm:drm_calc_timestamping_constants] crtc 24: clock 148500 kHz framedur 20000000 linedur 17777
> > > > [drm:drm_mode_object_reference] OBJ ID: 47 (3)
> > > > [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
> > > > [drm:drm_mode_object_unreference] OBJ ID: 48 (2)
> > > > [drm:drm_mode_object_unreference] OBJ ID: 34 (4)
> > > > [drm:drm_ioctl] ret = -22
> > > >
> > > > With a bit more debugging, this is what's failing:
> > > >
> > > > ipu_plane_atomic_check:442: fail
> > > > [drm:drm_crtc_helper_set_config] *ERROR* failed to set mode on [CRTC:24:crtc-0]
> > > >
> > > >         if (old_fb && (state->src_w != old_state->src_w ||
> > > >                               state->src_h != old_state->src_h ||
> > > >                               fb->pixel_format != old_fb->pixel_format)) {
> > > >                 printk("%s:%d: fail\n", __func__, __LINE__);
> > > >                 return -EINVAL;
> > > >         }
> > > >
> > > > This test is stupid as it stands - it means we can _never_ change the
> > > > format or size of _any_ plane, something that the old code allowed:
> > > >
> > > > -       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;
> > > >
> > > > This used to work because we'd call ipu_crtc_prepare()->ipu_fb_disable()
> > > > before the mode set, which would clear ipu_plane->enabled, causing this
> > > > test to be skipped.  However, in the new code, we merely test for the
> > > > presence of the previous framebuffer, which is really not the same thing
> > > > at all.
> > > >
> > > > The old functionality needs to be restored because significantly
> > > > changing the displayed mode is something which must be supported with
> > > > HDMI.
> > >
> > > Bypassing the above check also shows that:
> > >
> > >         if (old_fb && fb->pitches[0] != old_fb->pitches[0]) {
> > >                 printk("%s:%d: fail\n", __func__, __LINE__);
> > >                 return -EINVAL;
> > >         }
> > >
> > > also fails.  Disabling this as well results in loss of sync on the
> > > HDMI display, although the mode set now succeeds.
> > >
> > > The more I think about this, the more I come to one of two possible
> > > conclusions:
> > >
> > > 1. These checks were not appropriate with the old code as we were
> > >    always disabling the display channel prior to making changes.
> > >
> > >    We need atomic mode set to work in exactly the same way as the
> > >    previous code: as a series of disable-modify-enable set of steps,
> > >    so that we don't need these restrictive and regression causing
> > >    checks.
> > >
> > > or
> > >
> > > 2. imx-drm has these particular restrictions which make it inappropriate
> > >    to be converted to atomic mode set, as we always need to go through
> > >    a disable-modify-enable series of steps - which means the atomic
> > >    mode set changes for imx-drm need to be reverted back to this patch.
> > >
> > > I'm pretty sure (2) doesn't really apply, because I see no reason why
> > > we can't disable the display channel while we reconfigure it.  That,
> > > to me, seems to be an entirely reasonable thing to do.
> >
> > Already ddiscussed this at lenght on irc with Peter Senna. If you have
> > plane update restrictions where you need to cycle the crtc completely,
> > the right thing to do is to set crtc_state->mode_changed instead of return
> > -EINVAL. The helper/core will then convert that into an EINVAL if
> > userspace doesn't set ALLOW_MODESET. And since the helper function to map
> > set_config to atomic does this, it should all just work.
> >
> > Note: There's about 3 such conditions in imx' atomic_check function which
> > needs this change.
> 
> You also need to update the overall atomic_check to re-run the modeset
> checks after the plane checks again (since plane checks can update
> mode_changed). Peter Senna has the full diff, but apparently it doesn't
> work either. So there's probably more bugs somewhere.

Patch attached. This patch change the output from black screen to a
buggy image that actually gets updated if I move the mouse, which means
that it doesn't fix the issue. Tested on next-20160812.

Not enabling the frame buffer emulation(Kconfig or commenting out the
call to drm_fbdev_cma_init()) solves the issue.


[-- Attachment #2: imx_drm_patch.patch --]
[-- Type: text/plain, Size: 2371 bytes --]

diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 9f7dafc..7de7126 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -171,10 +171,31 @@ static void imx_drm_output_poll_changed(struct drm_device *drm)
 	drm_fbdev_cma_hotplug_event(imxdrm->fbhelper);
 }
 
+static int imx_atomic_check(struct drm_device *dev,
+			    struct drm_atomic_state *state)
+{
+	int ret = 0;
+
+	ret = drm_atomic_helper_check_modeset(dev, state);
+	if (ret)
+		return ret;
+
+	ret = drm_atomic_helper_check_planes(dev, state);
+	if (ret)
+		return ret;
+
+	/* planes can set ->needs_modeset, once more to reach a stable state */
+	ret = drm_atomic_helper_check_modeset(dev, state);
+	if (ret)
+		return ret;
+
+	return ret;
+}
+
 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_check = imx_atomic_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 4ad67d0..b98581a 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -322,10 +322,10 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 	 * 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;
+	if (state->src_w != old_state->src_w ||
+	    state->src_h != old_state->src_h ||
+	    (old_fb && fb->pixel_format != old_fb->pixel_format))
+		crtc_state->mode_changed = true;
 
 	eba = drm_plane_state_to_eba(state);
 
@@ -336,7 +336,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 		return -EINVAL;
 
 	if (old_fb && fb->pitches[0] != old_fb->pitches[0])
-		return -EINVAL;
+		crtc_state->mode_changed = true;
 
 	switch (fb->pixel_format) {
 	case DRM_FORMAT_YUV420:
@@ -372,7 +372,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 			return -EINVAL;
 
 		if (old_fb && old_fb->pitches[1] != fb->pitches[1])
-			return -EINVAL;
+			crtc_state->mode_changed = true;
 	}
 
 	return 0;

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH v3 03/10] drm/imx: atomic phase 1: Use transitional atomic CRTC and plane helpers

[Ying Liu]

On Sun, Aug 14, 2016 at 7:43 PM, Peter Senna Tschudin
<peter.senna@gmail.com> wrote:
> On Sun, Aug 14, 2016 at 12:46:27PM +0200, Daniel Vetter wrote:
>> On Sun, Aug 14, 2016 at 11:44:14AM +0200, Daniel Vetter wrote:
>> > On Sat, Aug 13, 2016 at 12:29:47PM +0100, Russell King - ARM Linux wrote:
>> > > On Sat, Aug 13, 2016 at 11:45:31AM +0100, Russell King - ARM Linux wrote:
>> > > > On Sat, Aug 13, 2016 at 11:11:54AM +0100, Russell King - ARM Linux wrote:
>> > > > > On Mon, Jul 04, 2016 at 03:40:32PM +0800, Liu Ying wrote:
>> > > > > > 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>
>> > > > >
>> > > > > This patch causes a regression with my xorg ddx driver:
>> > > > >
>> > > > > [    29.850] (EE) armada(0): [drm] failed to set mode on crtc 24: Invalid argument
>> > > > >
>> > > > > I'm not sure why (yet).
>> > > >
>> > > > Hi,
>> > > >
>> > > > Enabling DRM debugging doesn't seem to provide much in the way of clues:
>> > > >
>> > > > [drm:drm_ioctl] pid=1015, dev=0xe200, auth=1, DRM_IOCTL_MODE_SETCRTC
>> > > > [drm:drm_mode_setcrtc] [CRTC:24:crtc-0]
>> > > > [drm:drm_mode_setcrtc] [CONNECTOR:34:HDMI-A-1]
>> > > > [drm:drm_crtc_helper_set_config]
>> > > > [drm:drm_crtc_helper_set_config] [CRTC:24:crtc-0] [FB:48] #connectors=1 (x y) (0 0)
>> > > > [drm:drm_crtc_helper_set_config] [CONNECTOR:34:HDMI-A-1] to [CRTC:24:crtc-0]
>> > > > [drm:drm_crtc_helper_set_config] attempting to set mode from userspace
>> > > > [drm:drm_mode_debug_printmodeline] Modeline 49:"1920x1080" 0 148500 1920 2448 2492 2640 1080 1084 1089 1125 0x0 0x5
>> > > > [drm:drm_crtc_helper_set_mode] [CRTC:24:crtc-0]
>> > > > [drm:drm_vblank_off] crtc 0, vblank enabled 0, inmodeset 0
>> > > > [drm:drm_update_vblank_count] updating vblank count on crtc 0: current=2, diff=0, hw=0 hw_last=0
>> > > > [drm:drm_mode_object_reference] OBJ ID: 51 (1)
>> > > > [drm:drm_mode_object_unreference] OBJ ID: 51 (2)
>> > > > [drm:drm_atomic_set_mode_for_crtc] Set [MODE:1920x1080] for CRTC state ed81ee00
>> > > > [drm:drm_mode_object_reference] OBJ ID: 47 (3)
>> > > > [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
>> > > > [drm:drm_mode_object_reference] OBJ ID: 48 (2)
>> > > > [drm:drm_atomic_set_fb_for_plane] Set [FB:48] for plane state ed632d00
>> > > > [drm:drm_mode_object_unreference] OBJ ID: 48 (3)
>> > > > [drm:drm_mode_object_unreference] OBJ ID: 51 (1)
>> > > > [drm:drm_crtc_helper_set_config] *ERROR* failed to set mode on [CRTC:24:crtc-0]
>> > > > [drm:drm_crtc_helper_set_mode] [CRTC:24:crtc-0]
>> > > > [drm:drm_mode_object_reference] OBJ ID: 52 (1)
>> > > > [drm:drm_mode_object_unreference] OBJ ID: 52 (2)
>> > > > [drm:drm_atomic_set_mode_for_crtc] Set [MODE:1920x1080] for CRTC state ed81f600
>> > > > [drm:drm_mode_object_reference] OBJ ID: 47 (3)
>> > > > [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
>> > > > [drm:drm_mode_object_reference] OBJ ID: 47 (3)
>> > > > [drm:drm_atomic_set_fb_for_plane] Set [FB:47] for plane state ed632d00
>> > > > [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
>> > > > [drm:drm_mode_object_unreference] OBJ ID: 52 (1)
>> > > > [drm:drm_crtc_helper_set_mode] [ENCODER:33:TMDS-33] set [MODE:46:1920x1080]
>> > > > [drm:drm_vblank_on] crtc 0, vblank enabled 0, inmodeset 1
>> > > > [drm:drm_calc_timestamping_constants] crtc 24: hwmode: htotal 2640, vtotal 1125, vdisplay 1080
>> > > > [drm:drm_calc_timestamping_constants] crtc 24: clock 148500 kHz framedur 20000000 linedur 17777
>> > > > [drm:drm_mode_object_reference] OBJ ID: 47 (3)
>> > > > [drm:drm_mode_object_unreference] OBJ ID: 47 (4)
>> > > > [drm:drm_mode_object_unreference] OBJ ID: 48 (2)
>> > > > [drm:drm_mode_object_unreference] OBJ ID: 34 (4)
>> > > > [drm:drm_ioctl] ret = -22
>> > > >
>> > > > With a bit more debugging, this is what's failing:
>> > > >
>> > > > ipu_plane_atomic_check:442: fail
>> > > > [drm:drm_crtc_helper_set_config] *ERROR* failed to set mode on [CRTC:24:crtc-0]
>> > > >
>> > > >         if (old_fb && (state->src_w != old_state->src_w ||
>> > > >                               state->src_h != old_state->src_h ||
>> > > >                               fb->pixel_format != old_fb->pixel_format)) {
>> > > >                 printk("%s:%d: fail\n", __func__, __LINE__);
>> > > >                 return -EINVAL;
>> > > >         }
>> > > >
>> > > > This test is stupid as it stands - it means we can _never_ change the
>> > > > format or size of _any_ plane, something that the old code allowed:
>> > > >
>> > > > -       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;
>> > > >
>> > > > This used to work because we'd call ipu_crtc_prepare()->ipu_fb_disable()
>> > > > before the mode set, which would clear ipu_plane->enabled, causing this
>> > > > test to be skipped.  However, in the new code, we merely test for the
>> > > > presence of the previous framebuffer, which is really not the same thing
>> > > > at all.
>> > > >
>> > > > The old functionality needs to be restored because significantly
>> > > > changing the displayed mode is something which must be supported with
>> > > > HDMI.
>> > >
>> > > Bypassing the above check also shows that:
>> > >
>> > >         if (old_fb && fb->pitches[0] != old_fb->pitches[0]) {
>> > >                 printk("%s:%d: fail\n", __func__, __LINE__);
>> > >                 return -EINVAL;
>> > >         }
>> > >
>> > > also fails.  Disabling this as well results in loss of sync on the
>> > > HDMI display, although the mode set now succeeds.
>> > >
>> > > The more I think about this, the more I come to one of two possible
>> > > conclusions:
>> > >
>> > > 1. These checks were not appropriate with the old code as we were
>> > >    always disabling the display channel prior to making changes.
>> > >
>> > >    We need atomic mode set to work in exactly the same way as the
>> > >    previous code: as a series of disable-modify-enable set of steps,
>> > >    so that we don't need these restrictive and regression causing
>> > >    checks.
>> > >
>> > > or
>> > >
>> > > 2. imx-drm has these particular restrictions which make it inappropriate
>> > >    to be converted to atomic mode set, as we always need to go through
>> > >    a disable-modify-enable series of steps - which means the atomic
>> > >    mode set changes for imx-drm need to be reverted back to this patch.
>> > >
>> > > I'm pretty sure (2) doesn't really apply, because I see no reason why
>> > > we can't disable the display channel while we reconfigure it.  That,
>> > > to me, seems to be an entirely reasonable thing to do.
>> >
>> > Already ddiscussed this at lenght on irc with Peter Senna. If you have
>> > plane update restrictions where you need to cycle the crtc completely,
>> > the right thing to do is to set crtc_state->mode_changed instead of return
>> > -EINVAL. The helper/core will then convert that into an EINVAL if
>> > userspace doesn't set ALLOW_MODESET. And since the helper function to map
>> > set_config to atomic does this, it should all just work.
>> >
>> > Note: There's about 3 such conditions in imx' atomic_check function which
>> > needs this change.
>>
>> You also need to update the overall atomic_check to re-run the modeset
>> checks after the plane checks again (since plane checks can update
>> mode_changed). Peter Senna has the full diff, but apparently it doesn't
>> work either. So there's probably more bugs somewhere.
>
> Patch attached. This patch change the output from black screen to a
> buggy image that actually gets updated if I move the mouse, which means
> that it doesn't fix the issue. Tested on next-20160812.
>
> Not enabling the frame buffer emulation(Kconfig or commenting out the
> call to drm_fbdev_cma_init()) solves the issue.
>

I've send a patch[1] to fix the issue by following the approach
suggested by Daniel Vetter.

Comments or Tested-by are welcome.

[1] http://www.spinics.net/lists/dri-devel/msg115488.html

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

[PATCH v3 04/10] drm/imx: atomic phase 2 step 1: Wire up state ->reset, ->duplicate and ->destroy

[Liu Ying]

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>
---
v2->v3:
* For dw-hdmi bridge driver, simply add the hooks to the legacy connector
  function structure instead of using the atomic version directly, because
  we are not yet ready to use the atomic version's ->dpms hook.

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       | 3 +++
 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(+)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 70b1f7d..dd5b21a 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1500,6 +1500,9 @@ static const struct drm_connector_funcs dw_hdmi_connector_funcs = {
 	.detect = dw_hdmi_connector_detect,
 	.destroy = dw_hdmi_connector_destroy,
 	.force = dw_hdmi_connector_force,
+	.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_funcs dw_hdmi_atomic_connector_funcs = {
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 7746418..b5a5173 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 59f2353..1a126fe 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

[PATCH v3 05/10] drm/imx: atomic phase 2 step 2: Track plane_state->fb correctly in ->page_flip

[Liu Ying]

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>
---
v2->v3:
* None.

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

[PATCH v3 06/10] drm/imx: Remove encoders' ->prepare callbacks

[Liu Ying]

The main task of imx encoders' ->prepare callbacks is to set bus_format,
bus_flags, di_vsync_pin and di_hsync_pin.  We may create a structure named
imx_encoder to cache them.  The atomic encoder callback ->disable may
replace ->prepare later, so let's remove ->prepare.

Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v2->v3:
* Newly introduced.
  The logic is picked from patch 07/10 in v2 to generate a self-contained
  patch.

 drivers/gpu/drm/imx/dw_hdmi-imx.c      |  18 +++---
 drivers/gpu/drm/imx/imx-drm-core.c     |  39 ------------
 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       |  38 ++++--------
 drivers/gpu/drm/imx/parallel-display.c |  56 +++++++++--------
 7 files changed, 133 insertions(+), 203 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 b5a5173..f6e44c2 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -85,45 +85,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);
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..f9d5d7c 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -67,10 +67,6 @@ struct ipu_crtc {
 	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)
@@ -321,6 +317,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 +328,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,20 +351,20 @@ 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);
 }
 
@@ -402,23 +402,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/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

[PATCH v3 07/10] drm/imx: atomic phase 3 step 1: Use atomic configuration

[Liu Ying]

Replacing drm_crtc_helper_set_config() by drm_atomic_helper_set_config()
and converting the suspend/resume operations to atomic make 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, change
the plane_funcs->update/disable_plane callbacks from the transitional
version to the atomic version.  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.  Page flip is also
switched to the atomic version.  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>
---
v2->v3:
* Newly introduced, but this contains some important logic in patch 06/10
  , 07/10 and 09/10 of v2 to support nonblock atomic commit(including page
  flip) - the ->set_config and ->update_plane/disable_plane hooks are updated
  to be pure atomic version, and ->atomic_check/atomic_commit hooks of
  drm_mode_config_funcs are wired up.

 drivers/gpu/drm/imx/imx-drm-core.c |  76 +++++++++++++-
 drivers/gpu/drm/imx/ipuv3-crtc.c   | 209 +++++--------------------------------
 drivers/gpu/drm/imx/ipuv3-plane.c  | 135 +++---------------------
 drivers/gpu/drm/imx/ipuv3-plane.h  |   2 -
 4 files changed, 114 insertions(+), 308 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index f6e44c2..f14ad2b 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 <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>
 #include <drm/drm_gem_cma_helper.h>
@@ -41,6 +45,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 {
@@ -169,6 +174,63 @@ 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,
+};
+
+static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state)
+{
+	struct drm_device *dev = state->dev;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *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(state, crtc, crtc_state, i) {
+		plane_state = crtc->primary->state;
+		if (plane_state->fb) {
+			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]);
+				}
+			}
+		}
+	}
+
+	drm_atomic_helper_commit_modeset_disables(dev, state);
+
+	drm_atomic_helper_commit_planes(dev, state, true);
+
+	drm_atomic_helper_commit_modeset_enables(dev, state);
+
+	drm_atomic_helper_commit_hw_done(state);
+
+	drm_atomic_helper_wait_for_vblanks(dev, state);
+
+	drm_atomic_helper_cleanup_planes(dev, state);
+}
+
+static struct drm_mode_config_helper_funcs imx_drm_mode_config_helpers = {
+	.atomic_commit_tail = imx_drm_atomic_commit_tail,
 };
 
 /*
@@ -210,6 +272,7 @@ static int imx_drm_driver_load(struct drm_device *drm, unsigned long flags)
 	drm->mode_config.max_width = 4096;
 	drm->mode_config.max_height = 4096;
 	drm->mode_config.funcs = &imx_drm_mode_config_funcs;
+	drm->mode_config.helper_private = &imx_drm_mode_config_helpers;
 
 	drm_mode_config_init(drm);
 
@@ -252,7 +315,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)) {
@@ -454,6 +516,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)
@@ -461,17 +524,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/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index f9d5d7c..3e82534 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,10 +43,6 @@ 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;
 };
 
@@ -75,34 +52,26 @@ 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);
 }
 
 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;
+	struct drm_crtc *crtc = &ipu_crtc->base;
 
 	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);
+	spin_lock_irq(&crtc->dev->event_lock);
+	if (crtc->state->event) {
+		drm_crtc_send_vblank_event(crtc, crtc->state->event);
+		crtc->state->event = NULL;
+	}
+	spin_unlock_irq(&crtc->dev->event_lock);
 }
 
 static void ipu_crtc_dpms(struct drm_crtc *crtc, int mode)
@@ -123,151 +92,21 @@ static void ipu_crtc_dpms(struct drm_crtc *crtc, int mode)
 	}
 }
 
-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_crtc_helper_set_config,
+	.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)
-{
-	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);
-	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;
 
 	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;
-	}
-
 	return IRQ_HANDLED;
 }
 
@@ -310,9 +149,26 @@ 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;
 }
 
+static void ipu_crtc_atomic_begin(struct drm_crtc *crtc,
+				  struct drm_crtc_state *old_crtc_state)
+{
+	spin_lock_irq(&crtc->dev->event_lock);
+	if (crtc->state->event) {
+		WARN_ON(drm_crtc_vblank_get(crtc));
+		drm_crtc_arm_vblank_event(crtc, crtc->state->event);
+		crtc->state->event = NULL;
+	}
+	spin_unlock_irq(&crtc->dev->event_lock);
+}
+
 static void ipu_crtc_mode_set_nofb(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -371,25 +227,17 @@ 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 = 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,
+	.atomic_begin = ipu_crtc_atomic_begin,
 };
 
 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;
@@ -508,8 +356,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:
@@ -554,7 +400,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]);
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 1a126fe..164333b 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>
@@ -55,122 +56,6 @@ 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)
-{
-	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);
-		if (!cma_obj[i]) {
-			DRM_DEBUG_KMS("plane %d entry is null.\n", i);
-			return -EFAULT;
-		}
-	}
-
-	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;
-
-	if (eba & 0x7) {
-		DRM_DEBUG_KMS("base address must be a multiple of 8.\n");
-		return -EINVAL;
-	}
-
-	if (fb->pitches[0] < 1 || fb->pitches[0] > 16384) {
-		DRM_DEBUG_KMS("pitches out of range.\n");
-		return -EINVAL;
-	}
-
-	if (fb->pitches[0] != old_fb->pitches[0]) {
-		DRM_DEBUG_KMS("pitches must not change while plane is enabled.\n");
-		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 = cma_obj[1]->paddr + fb->offsets[1] +
-		      fb->pitches[1] * y / 2 + x / 2 - eba;
-		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;
-		}
-
-		if ((ubo > 0xfffff8) || (vbo > 0xfffff8)) {
-			DRM_DEBUG_KMS("U/V buffer offsets must be positive and not larger than 0xfffff8.\n");
-			return -EINVAL;
-		}
-
-		if (old_ubo != ubo || old_vbo != vbo) {
-			DRM_DEBUG_KMS("U/V buffer offsets must not change while plane is enabled.\n");
-			return -EINVAL;
-		}
-
-		if (fb->pitches[1] != fb->pitches[2]) {
-			DRM_DEBUG_KMS("U/V pitches must be identical.\n");
-			return -EINVAL;
-		}
-
-		if (fb->pitches[1] < 1 || fb->pitches[1] > 16384) {
-			DRM_DEBUG_KMS("U/V pitches out of range.\n");
-			return -EINVAL;
-		}
-
-		if (old_fb->pitches[1] != fb->pitches[1]) {
-			DRM_DEBUG_KMS("U/V pitches must not change while plane is enabled.\n");
-			return -EINVAL;
-		}
-
-		dev_dbg(ipu_plane->base.dev->dev,
-			"phys = %pad %pad %pad, x = %d, y = %d",
-			&cma_obj[0]->paddr, &cma_obj[1]->paddr,
-			&cma_obj[2]->paddr, x, y);
-		break;
-	default:
-		dev_dbg(ipu_plane->base.dev->dev, "phys = %pad, x = %d, y = %d",
-			&cma_obj[0]->paddr, x, y);
-		break;
-	}
-
-	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;
-}
-
 static inline unsigned long
 drm_plane_state_to_eba(struct drm_plane_state *state)
 {
@@ -360,8 +245,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,
@@ -380,10 +265,18 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 
 	/* 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 */
-	if (!state->crtc->enabled)
+	if (!crtc_state->enable)
 		return -EINVAL;
 
 	/* no scaling */
@@ -391,8 +284,6 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 	    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/ipuv3-plane.h b/drivers/gpu/drm/imx/ipuv3-plane.h
index c51a44b..338b88a 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.h
+++ b/drivers/gpu/drm/imx/ipuv3-plane.h
@@ -37,8 +37,6 @@ 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);
 
-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);
 
-- 
2.7.4

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

[PATCH v3 08/10] drm/bridge: dw-hdmi: Remove the legacy drm_connector_funcs structure

[Liu Ying]

There is no one using the legacy drm_connector_funcs structure since
the imx-drm has been converted to atomic, so we may remove it.

Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v2->v3:
* Newly introduced to remove the legacy drm_connector_funcs structure
  step-by-step.

 drivers/gpu/drm/bridge/dw-hdmi.c | 22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index dd5b21a..77ab473 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1495,17 +1495,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,
-	.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_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,
@@ -1637,14 +1626,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);
 
-- 
2.7.4

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

[PATCH v3 09/10] drm/imx: atomic phase 3 step 2: Legacy callback fixups

[Liu Ying]

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>
---
v2->v3:
* Trivial change due to rebasing.

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       | 44 +++++-----------------------------
 drivers/gpu/drm/imx/parallel-display.c | 18 +++-----------
 5 files changed, 27 insertions(+), 82 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 3e82534..274b0e2 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -48,8 +48,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);
@@ -57,10 +58,10 @@ static void ipu_crtc_enable(struct ipu_crtc *ipu_crtc)
 	ipu_di_enable(ipu_crtc->di);
 }
 
-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);
-	struct drm_crtc *crtc = &ipu_crtc->base;
 
 	ipu_dc_disable_channel(ipu_crtc->dc);
 	ipu_di_disable(ipu_crtc->di);
@@ -74,24 +75,6 @@ static void ipu_crtc_disable(struct ipu_crtc *ipu_crtc)
 	spin_unlock_irq(&crtc->dev->event_lock);
 }
 
-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 const struct drm_crtc_funcs ipu_crtc_funcs = {
 	.set_config = drm_atomic_helper_set_config,
 	.destroy = drm_crtc_cleanup,
@@ -132,20 +115,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)
 {
@@ -225,13 +194,12 @@ 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,
 	.atomic_begin = ipu_crtc_atomic_begin,
+	.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

[PATCH v3 10/10] drm/imx: atomic phase 3 step 3: Advertise DRIVER_ATOMIC

[Liu Ying]

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

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

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 f14ad2b..9f7dafc 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -428,7 +428,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

Re: [PATCH v3 00/10] imx drm atomic mode setting conversion

[Daniel Vetter]

On Mon, Jul 04, 2016 at 03:40:29PM +0800, Liu Ying wrote:
> Hi,
> 
> This is the v3 patch set to convert imx drm into atomic mode setting.
> It takes 3 phases to achieve the goal.
> 
> v2->v3:
> * Rebase onto Daniel Vetter's open git branch topic/drm-misc so that
>   we may better support nonblock atomic commit with the aid from
>   drm atomic helper.
> * Remove dw-hdmi bridge driver's legacy drm_connector_funcs struture
>   step-by-step instead of doing that in patch 04/10 directly.
>   So, patch 08/10 in this set is newly introduced.

Assuming all my feedback has been addressed:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I'd just go ahead and send Dave Airlie a pull req for this, but hurry up
to make it into 4.8.
-Daniel

> 
> 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: Remove encoders' ->prepare callbacks
>   drm/imx: atomic phase 3 step 1: Use atomic configuration
>   drm/bridge: dw-hdmi: Remove the legacy drm_connector_funcs structure
>   drm/imx: atomic phase 3 step 2: Legacy callback fixups
>   drm/imx: atomic phase 3 step 3: 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     | 120 +++++---
>  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       | 370 ++++++----------------
>  drivers/gpu/drm/imx/ipuv3-plane.c      | 543 ++++++++++++++++-----------------
>  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, 592 insertions(+), 1028 deletions(-)
> 
> -- 
> 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

Re: [PATCH v3 00/10] imx drm atomic mode setting conversion

[Philipp Zabel]

Am Dienstag, den 12.07.2016, 14:51 +0200 schrieb Daniel Vetter:
> On Mon, Jul 04, 2016 at 03:40:29PM +0800, Liu Ying wrote:
> > Hi,
> > 
> > This is the v3 patch set to convert imx drm into atomic mode setting.
> > It takes 3 phases to achieve the goal.
> > 
> > v2->v3:
> > * Rebase onto Daniel Vetter's open git branch topic/drm-misc so that
> >   we may better support nonblock atomic commit with the aid from
> >   drm atomic helper.
> > * Remove dw-hdmi bridge driver's legacy drm_connector_funcs struture
> >   step-by-step instead of doing that in patch 04/10 directly.
> >   So, patch 08/10 in this set is newly introduced.
> 
> Assuming all my feedback has been addressed:
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> I'd just go ahead and send Dave Airlie a pull req for this, but hurry up
> to make it into 4.8.
> -Daniel

Will do, thanks.

regards
Philipp

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