[PATCH v4 0/7] drm/imx: Add active plane reconfiguration support

[PATCH v4 0/7] drm/imx: Add active plane reconfiguration support

[Liu Ying]

This patch adds active plane reconfiguration support for imx-drm.
This may fixes some mode setting failure issues which were introduced
by imx-drm atomic conversion patch set.  The main idea is to disable the
plane in question in CRTC's atomic_disable operation and then the drm
atomic core will enable it again automatically.

v3->v4:
* Change the bool active_only parameter of commit_planes() to an uint32_t
  parameter named 'flags' and add two flags - DRM_PLANE_COMMIT_ACTIVE_ONLY
  and DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET.  This way, the drm atomic
  core is able to skip the atomic_disable call for the planes which are
  committed with the NO_DISABLE_AFTER_MODESET flag set.
* Fix the helper disable_planes_on_crtc(), which is needed for CRTC's
  atomic_disable callback to disable planes.
* Improve kernel-doc of CRTC's atomic_disable callback to address Daniel
  Vetter's comment.
* Do not wait for DMFC FIFO to clear to avoid timeout warning, as the
  precedure to disable display channel is changed slightly after the
  NO_DISABLE_AFTER_MODESET flag is used.

v2->v3:
* Disable all appropriate affected planes(when necessary) in CRTC's
  ->atomic_disable callback, but not in each plane's ->atomic_update callback,
  as suggested by Daniel Vetter.
* +Cc Lucas Stach, as he tested the patch v2.

v1->v2:
* Do not reject reconfiguring an active overlay plane.

Liu Ying (7):
  drm/atomic-helper: Add atomic_disable CRTC helper callback
  drm/atomic-helper: Disable appropriate planes in
    disable_planes_on_crtc()
  drm/atomic-helper: Add NO_DISABLE_AFTER_MODESET flag support for plane
    commit
  gpu: ipu-v3: Do not wait for DMFC FIFO to clear when disabling DMFC
    channel
  drm/imx: ipuv3-crtc: Use the callback ->atomic_disable instead of
    ->disable
  drm/imx: Use DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET flag
  drm/imx: Add active plane reconfiguration support

 drivers/gpu/drm/arm/malidp_drv.c             |  3 +-
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-
 drivers/gpu/drm/drm_atomic_helper.c          | 64 +++++++++++++++++++---------
 drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-
 drivers/gpu/drm/imx/imx-drm-core.c           | 30 ++++++++++++-
 drivers/gpu/drm/imx/ipuv3-crtc.c             |  8 +++-
 drivers/gpu/drm/imx/ipuv3-plane.c            | 21 ++++++---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c       |  6 ++-
 drivers/gpu/drm/msm/msm_atomic.c             |  2 +-
 drivers/gpu/drm/omapdrm/omap_drv.c           |  2 +-
 drivers/gpu/drm/rcar-du/rcar_du_kms.c        |  3 +-
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c   |  3 +-
 drivers/gpu/drm/sti/sti_drv.c                |  2 +-
 drivers/gpu/drm/tegra/drm.c                  |  3 +-
 drivers/gpu/drm/tilcdc/tilcdc_drv.c          |  2 +-
 drivers/gpu/drm/vc4/vc4_kms.c                |  2 +-
 drivers/gpu/drm/virtio/virtgpu_display.c     |  3 +-
 drivers/gpu/ipu-v3/ipu-dmfc.c                | 18 +-------
 include/drm/drm_atomic_helper.h              | 11 +++--
 include/drm/drm_modeset_helper_vtables.h     | 24 +++++++++++
 20 files changed, 146 insertions(+), 65 deletions(-)

-- 
2.7.4

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

[PATCH v4 1/7] drm/atomic-helper: Add atomic_disable CRTC helper callback

[Liu Ying]

Some display controllers need plane(s) to be disabled together with
the relevant CRTC, e.g., the IPUv3 display controller for imx-drm.
This patch adds atomic_disable CRTC helper callback so that
old_crtc_state(as a parameter of the callback) could be used
to get the active plane(s) of the old CRTC state for disable operation.

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Peter Senna Tschudin <peter.senna@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v3->v4:
* Improve kernel-doc of CRTC's atomic_disable callback to address Daniel
  Vetter's comment.

v3:
* Newly introduced in v3.

 drivers/gpu/drm/drm_atomic_helper.c      |  2 ++
 include/drm/drm_modeset_helper_vtables.h | 24 ++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 9abe0a2..254bdde 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -749,6 +749,8 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 		/* Right function depends upon target state. */
 		if (crtc->state->enable && funcs->prepare)
 			funcs->prepare(crtc);
+		else if (funcs->atomic_disable)
+			funcs->atomic_disable(crtc, old_crtc_state);
 		else if (funcs->disable)
 			funcs->disable(crtc);
 		else
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index 6c8d3da..10e449c 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -266,6 +266,8 @@ struct drm_crtc_helper_funcs {
 	 * disable anything at the CRTC level. To ensure that runtime PM
 	 * handling (using either DPMS or the new "ACTIVE" property) works
 	 * @disable must be the inverse of @enable for atomic drivers.
+	 * Atomic drivers should consider to use @atomic_disable instead of
+	 * this one.
 	 *
 	 * NOTE:
 	 *
@@ -391,6 +393,28 @@ struct drm_crtc_helper_funcs {
 	 */
 	void (*atomic_flush)(struct drm_crtc *crtc,
 			     struct drm_crtc_state *old_crtc_state);
+
+	/**
+	 * @atomic_disable:
+	 *
+	 * This callback should be used to disable the CRTC. With the atomic
+	 * drivers it is called after all encoders connected to this CRTC have
+	 * been shut off already using their own ->disable hook. If that
+	 * sequence is too simple drivers can just add their own hooks and call
+	 * it from this CRTC callback here by looping over all encoders
+	 * connected to it using for_each_encoder_on_crtc().
+	 *
+	 * This hook is used only by atomic helpers. Atomic drivers don't
+	 * need to implement it if there's no need to disable anything at the
+	 * CRTC level.
+	 *
+	 * Comparing to @disable, this one provides the additional input
+	 * parameter @old_crtc_state which could be used to access the old
+	 * state. Atomic drivers should consider to use this one instead
+	 * of @disable.
+	 */
+	void (*atomic_disable)(struct drm_crtc *crtc,
+			       struct drm_crtc_state *old_crtc_state);
 };
 
 /**
-- 
2.7.4

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

[PATCH v4 2/7] drm/atomic-helper: Disable appropriate planes in disable_planes_on_crtc()

[Liu Ying]

Currently, the helper drm_atomic_helper_disable_planes_on_crtc() calls
->atomic_disable for all planes _to be_ enabled on a particular CRTC.
This is obviously wrong for those planes which are not scanning out frames
when the helper is called.  Instead, it's sane to disable active planes
of old_crtc_state in the helper.

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Peter Senna Tschudin <peter.senna@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v4:
* Newly introduced in v4.

 drivers/gpu/drm/drm_atomic_helper.c | 16 +++++++++-------
 include/drm/drm_atomic_helper.h     |  5 +++--
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 254bdde..30c52a8 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1842,12 +1842,12 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_planes_on_crtc);
 
 /**
  * drm_atomic_helper_disable_planes_on_crtc - helper to disable CRTC's planes
- * @crtc: CRTC
+ * @old_crtc_state: atomic state object with the old CRTC state
  * @atomic: if set, synchronize with CRTC's atomic_begin/flush hooks
  *
  * Disables all planes associated with the given CRTC. This can be
- * used for instance in the CRTC helper disable callback to disable
- * all planes before shutting down the display pipeline.
+ * used for instance in the CRTC helper atomic_disable callback to disable
+ * all planes.
  *
  * If the atomic-parameter is set the function calls the CRTC's
  * atomic_begin hook before and atomic_flush hook after disabling the
@@ -1856,9 +1856,11 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_planes_on_crtc);
  * It is a bug to call this function without having implemented the
  * ->atomic_disable() plane hook.
  */
-void drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc *crtc,
-					      bool atomic)
+void
+drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc_state *old_crtc_state,
+					 bool atomic)
 {
+	struct drm_crtc *crtc = old_crtc_state->crtc;
 	const struct drm_crtc_helper_funcs *crtc_funcs =
 		crtc->helper_private;
 	struct drm_plane *plane;
@@ -1866,11 +1868,11 @@ void drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc *crtc,
 	if (atomic && crtc_funcs && crtc_funcs->atomic_begin)
 		crtc_funcs->atomic_begin(crtc, NULL);
 
-	drm_for_each_plane(plane, crtc->dev) {
+	drm_atomic_crtc_state_for_each_plane(plane, old_crtc_state) {
 		const struct drm_plane_helper_funcs *plane_funcs =
 			plane->helper_private;
 
-		if (plane->state->crtc != crtc || !plane_funcs)
+		if (!plane_funcs)
 			continue;
 
 		WARN_ON(!plane_funcs->atomic_disable);
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 5a02e49..1abf2c0 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -71,8 +71,9 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
 				      struct drm_atomic_state *old_state);
 void drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state);
-void drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc *crtc,
-					      bool atomic);
+void
+drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc_state *old_crtc_state,
+					 bool atomic);
 
 void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 				  bool stall);
-- 
2.7.4

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

[PATCH v4 3/7] drm/atomic-helper: Add NO_DISABLE_AFTER_MODESET flag support for plane commit

[Liu Ying]

Drivers may set the NO_DISABLE_AFTER_MODESET flag in the 'flags' parameter
of the helper drm_atomic_helper_commit_planes() if the relevant display
controllers(e.g., IPUv3 for imx-drm) require to disable a CRTC's planes
when the CRTC is disabled. The helper would skip the ->atomic_disable
call for a plane if the CRTC of the old plane state needs a modesetting
operation. Of course, the drivers need to disable the planes in their CRTC
disable callbacks since no one else would do that.

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Peter Senna Tschudin <peter.senna@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v4:
* Newly introduced in v4.

 drivers/gpu/drm/arm/malidp_drv.c             |  3 +-
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-
 drivers/gpu/drm/drm_atomic_helper.c          | 46 ++++++++++++++++++++--------
 drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-
 drivers/gpu/drm/imx/imx-drm-core.c           |  3 +-
 drivers/gpu/drm/mediatek/mtk_drm_drv.c       |  6 ++--
 drivers/gpu/drm/msm/msm_atomic.c             |  2 +-
 drivers/gpu/drm/omapdrm/omap_drv.c           |  2 +-
 drivers/gpu/drm/rcar-du/rcar_du_kms.c        |  3 +-
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c   |  3 +-
 drivers/gpu/drm/sti/sti_drv.c                |  2 +-
 drivers/gpu/drm/tegra/drm.c                  |  3 +-
 drivers/gpu/drm/tilcdc/tilcdc_drv.c          |  2 +-
 drivers/gpu/drm/vc4/vc4_kms.c                |  2 +-
 drivers/gpu/drm/virtio/virtgpu_display.c     |  3 +-
 include/drm/drm_atomic_helper.h              |  6 +++-
 16 files changed, 61 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 82171d2..c383d72 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -91,7 +91,8 @@ static void malidp_atomic_commit_tail(struct drm_atomic_state *state)
 
 	drm_atomic_helper_commit_modeset_disables(drm, state);
 	drm_atomic_helper_commit_modeset_enables(drm, state);
-	drm_atomic_helper_commit_planes(drm, state, true);
+	drm_atomic_helper_commit_planes(drm, state,
+					DRM_PLANE_COMMIT_ACTIVE_ONLY);
 
 	malidp_atomic_commit_hw_done(state);
 
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index d4a3d61..8e7483d 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -457,7 +457,7 @@ atmel_hlcdc_dc_atomic_complete(struct atmel_hlcdc_dc_commit *commit)
 
 	/* Apply the atomic update. */
 	drm_atomic_helper_commit_modeset_disables(dev, old_state);
-	drm_atomic_helper_commit_planes(dev, old_state, false);
+	drm_atomic_helper_commit_planes(dev, old_state, 0);
 	drm_atomic_helper_commit_modeset_enables(dev, old_state);
 
 	drm_atomic_helper_wait_for_vblanks(dev, old_state);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 30c52a8..cf19b6e 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1148,7 +1148,8 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks);
  *
  *     drm_atomic_helper_commit_modeset_enables(dev, state);
  *
- *     drm_atomic_helper_commit_planes(dev, state, true);
+ *     drm_atomic_helper_commit_planes(dev, state,
+ *                                     DRM_PLANE_COMMIT_ACTIVE_ONLY);
  *
  * for committing the atomic update to hardware.  See the kerneldoc entries for
  * these three functions for more details.
@@ -1159,7 +1160,7 @@ void drm_atomic_helper_commit_tail(struct drm_atomic_state *state)
 
 	drm_atomic_helper_commit_modeset_disables(dev, state);
 
-	drm_atomic_helper_commit_planes(dev, state, false);
+	drm_atomic_helper_commit_planes(dev, state, 0);
 
 	drm_atomic_helper_commit_modeset_enables(dev, state);
 
@@ -1678,7 +1679,7 @@ bool plane_crtc_active(struct drm_plane_state *state)
  * drm_atomic_helper_commit_planes - commit plane state
  * @dev: DRM device
  * @old_state: atomic state object with old state structures
- * @active_only: Only commit on active CRTC if set
+ * @flags: flags for committing plane state
  *
  * This function commits the new plane state using the plane and atomic helper
  * functions for planes and crtcs. It assumes that the atomic state has already
@@ -1698,25 +1699,33 @@ bool plane_crtc_active(struct drm_plane_state *state)
  * most drivers don't need to be immediately notified of plane updates for a
  * disabled CRTC.
  *
- * Unless otherwise needed, drivers are advised to set the @active_only
- * parameters to true in order not to receive plane update notifications related
- * to a disabled CRTC. This avoids the need to manually ignore plane updates in
+ * Unless otherwise needed, drivers are advised to set the ACTIVE_ONLY flag in
+ * @flags in order not to receive plane update notifications related to a
+ * disabled CRTC. This avoids the need to manually ignore plane updates in
  * driver code when the driver and/or hardware can't or just don't need to deal
  * with updates on disabled CRTCs, for example when supporting runtime PM.
- *
- * The drm_atomic_helper_commit() default implementation only sets @active_only
- * to false to most closely match the behaviour of the legacy helpers. This should
- * not be copied blindly by drivers.
+ * Drivers may set the NO_DISABLE_AFTER_MODESET flag in @flags if the relevant
+ * display controllers require to disable a CRTC's planes when the CRTC is
+ * disabled. This function would skip the ->atomic_disable call for a plane if
+ * the CRTC of the old plane state needs a modesetting operation. Of course,
+ * the drivers need to disable the planes in their CRTC disable callbacks
+ * since no one else would do that.
+ *
+ * The drm_atomic_helper_commit() default implementation doesn't set the
+ * ACTIVE_ONLY flag to most closely match the behaviour of the legacy helpers.
+ * This should not be copied blindly by drivers.
  */
 void drm_atomic_helper_commit_planes(struct drm_device *dev,
 				     struct drm_atomic_state *old_state,
-				     bool active_only)
+				     uint32_t flags)
 {
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *old_crtc_state;
 	struct drm_plane *plane;
 	struct drm_plane_state *old_plane_state;
 	int i;
+	bool active_only = flags & DRM_PLANE_COMMIT_ACTIVE_ONLY;
+	bool no_disable = flags & DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET;
 
 	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
@@ -1760,9 +1769,20 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 		/*
 		 * Special-case disabling the plane if drivers support it.
 		 */
-		if (disabling && funcs->atomic_disable)
+		if (disabling && funcs->atomic_disable) {
+			struct drm_crtc_state *crtc_state;
+
+			crtc_state = old_plane_state->crtc->state;
+			WARN_ON(!crtc_state);
+			if (!crtc_state)
+				continue;
+
+			if (drm_atomic_crtc_needs_modeset(crtc_state) &&
+			    no_disable)
+				continue;
+
 			funcs->atomic_disable(plane, old_plane_state);
-		else if (plane->state->crtc || disabling)
+		} else if (plane->state->crtc || disabling)
 			funcs->atomic_update(plane, old_plane_state);
 	}
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 877d2ef..486943e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -105,7 +105,7 @@ static void exynos_atomic_commit_complete(struct exynos_atomic_commit *commit)
 		atomic_inc(&exynos_crtc->pending_update);
 	}
 
-	drm_atomic_helper_commit_planes(dev, state, false);
+	drm_atomic_helper_commit_planes(dev, state, 0);
 
 	exynos_atomic_wait_for_commit(state);
 
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 438bac8..56dfc4c 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -193,7 +193,8 @@ static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state)
 
 	drm_atomic_helper_commit_modeset_disables(dev, state);
 
-	drm_atomic_helper_commit_planes(dev, state, true);
+	drm_atomic_helper_commit_planes(dev, state,
+					DRM_PLANE_COMMIT_ACTIVE_ONLY);
 
 	drm_atomic_helper_commit_modeset_enables(dev, state);
 
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 0e769ab..72c1ae4 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -70,13 +70,15 @@ static void mtk_atomic_complete(struct mtk_drm_private *private,
 	 *
 	 *     drm_atomic_helper_commit_modeset_disables(dev, state);
 	 *     drm_atomic_helper_commit_modeset_enables(dev, state);
-	 *     drm_atomic_helper_commit_planes(dev, state, true);
+	 *     drm_atomic_helper_commit_planes(dev, state,
+	 *                                     DRM_PLANE_COMMIT_ACTIVE_ONLY);
 	 *
 	 * See the kerneldoc entries for these three functions for more details.
 	 */
 	drm_atomic_helper_commit_modeset_disables(drm, state);
 	drm_atomic_helper_commit_modeset_enables(drm, state);
-	drm_atomic_helper_commit_planes(drm, state, true);
+	drm_atomic_helper_commit_planes(drm, state,
+					DRM_PLANE_COMMIT_ACTIVE_ONLY);
 
 	drm_atomic_helper_wait_for_vblanks(drm, state);
 
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 4a8a6f1..5df252c 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -118,7 +118,7 @@ static void complete_commit(struct msm_commit *c, bool async)
 
 	drm_atomic_helper_commit_modeset_disables(dev, state);
 
-	drm_atomic_helper_commit_planes(dev, state, false);
+	drm_atomic_helper_commit_planes(dev, state, 0);
 
 	drm_atomic_helper_commit_modeset_enables(dev, state);
 
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 3dd78f2..e1cfba5 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -96,7 +96,7 @@ static void omap_atomic_complete(struct omap_atomic_state_commit *commit)
 	dispc_runtime_get();
 
 	drm_atomic_helper_commit_modeset_disables(dev, old_state);
-	drm_atomic_helper_commit_planes(dev, old_state, false);
+	drm_atomic_helper_commit_planes(dev, old_state, 0);
 	drm_atomic_helper_commit_modeset_enables(dev, old_state);
 
 	omap_atomic_wait_for_completion(dev, old_state);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index f03eb55..bd9c3bb 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -257,7 +257,8 @@ static void rcar_du_atomic_complete(struct rcar_du_commit *commit)
 	/* Apply the atomic update. */
 	drm_atomic_helper_commit_modeset_disables(dev, old_state);
 	drm_atomic_helper_commit_modeset_enables(dev, old_state);
-	drm_atomic_helper_commit_planes(dev, old_state, true);
+	drm_atomic_helper_commit_planes(dev, old_state,
+					DRM_PLANE_COMMIT_ACTIVE_ONLY);
 
 	drm_atomic_helper_wait_for_vblanks(dev, old_state);
 
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index 55c5273..7ca8f34 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -233,7 +233,8 @@ rockchip_atomic_commit_tail(struct drm_atomic_state *state)
 
 	drm_atomic_helper_commit_modeset_enables(dev, state);
 
-	drm_atomic_helper_commit_planes(dev, state, true);
+	drm_atomic_helper_commit_planes(dev, state,
+					DRM_PLANE_COMMIT_ACTIVE_ONLY);
 
 	drm_atomic_helper_commit_hw_done(state);
 
diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
index f8311b2..7cd3804 100644
--- a/drivers/gpu/drm/sti/sti_drv.c
+++ b/drivers/gpu/drm/sti/sti_drv.c
@@ -178,7 +178,7 @@ static void sti_atomic_complete(struct sti_private *private,
 	 */
 
 	drm_atomic_helper_commit_modeset_disables(drm, state);
-	drm_atomic_helper_commit_planes(drm, state, false);
+	drm_atomic_helper_commit_planes(drm, state, 0);
 	drm_atomic_helper_commit_modeset_enables(drm, state);
 
 	drm_atomic_helper_wait_for_vblanks(drm, state);
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 755264d9d..4b9f1c7 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -57,7 +57,8 @@ static void tegra_atomic_complete(struct tegra_drm *tegra,
 
 	drm_atomic_helper_commit_modeset_disables(drm, state);
 	drm_atomic_helper_commit_modeset_enables(drm, state);
-	drm_atomic_helper_commit_planes(drm, state, true);
+	drm_atomic_helper_commit_planes(drm, state,
+					DRM_PLANE_COMMIT_ACTIVE_ONLY);
 
 	drm_atomic_helper_wait_for_vblanks(drm, state);
 
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 3404d24..4405e4b 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -118,7 +118,7 @@ static int tilcdc_commit(struct drm_device *dev,
 
 	drm_atomic_helper_commit_modeset_disables(dev, state);
 
-	drm_atomic_helper_commit_planes(dev, state, false);
+	drm_atomic_helper_commit_planes(dev, state, 0);
 
 	drm_atomic_helper_commit_modeset_enables(dev, state);
 
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 4ac894d..c1f65c6 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -44,7 +44,7 @@ vc4_atomic_complete_commit(struct vc4_commit *c)
 
 	drm_atomic_helper_commit_modeset_disables(dev, state);
 
-	drm_atomic_helper_commit_planes(dev, state, false);
+	drm_atomic_helper_commit_planes(dev, state, 0);
 
 	drm_atomic_helper_commit_modeset_enables(dev, state);
 
diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
index 4e192aa..7cf3678 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -338,7 +338,8 @@ static void vgdev_atomic_commit_tail(struct drm_atomic_state *state)
 
 	drm_atomic_helper_commit_modeset_disables(dev, state);
 	drm_atomic_helper_commit_modeset_enables(dev, state);
-	drm_atomic_helper_commit_planes(dev, state, true);
+	drm_atomic_helper_commit_planes(dev, state,
+					DRM_PLANE_COMMIT_ACTIVE_ONLY);
 
 	drm_atomic_helper_commit_hw_done(state);
 
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 1abf2c0..f866828 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -65,9 +65,13 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
 
 int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 				     struct drm_atomic_state *state);
+
+#define DRM_PLANE_COMMIT_ACTIVE_ONLY			BIT(0)
+#define DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET	BIT(1)
+
 void drm_atomic_helper_commit_planes(struct drm_device *dev,
 				     struct drm_atomic_state *state,
-				     bool active_only);
+				     uint32_t flags);
 void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
 				      struct drm_atomic_state *old_state);
 void drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state);
-- 
2.7.4

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

[PATCH v4 4/7] gpu: ipu-v3: Do not wait for DMFC FIFO to clear when disabling DMFC channel

[Liu Ying]

According to basic tests, it looks there is no issue if we don't wait for
DMFC FIFO to clear when disabling DMFC channel.  NXP BSP doesn't do that,
either.  This patch is needed to avoid the annoying warning caused by a
timeout on waiting for the FIFO to clear after we add the new
DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET flag to the imx-drm driver
which changes the procedure to disable display channel slightly.

Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Peter Senna Tschudin <peter.senna@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v4:
* Newly introduced in v4.

 drivers/gpu/ipu-v3/ipu-dmfc.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/gpu/ipu-v3/ipu-dmfc.c b/drivers/gpu/ipu-v3/ipu-dmfc.c
index 42705bb..a40f211 100644
--- a/drivers/gpu/ipu-v3/ipu-dmfc.c
+++ b/drivers/gpu/ipu-v3/ipu-dmfc.c
@@ -123,20 +123,6 @@ int ipu_dmfc_enable_channel(struct dmfc_channel *dmfc)
 }
 EXPORT_SYMBOL_GPL(ipu_dmfc_enable_channel);
 
-static void ipu_dmfc_wait_fifos(struct ipu_dmfc_priv *priv)
-{
-	unsigned long timeout = jiffies + msecs_to_jiffies(1000);
-
-	while ((readl(priv->base + DMFC_STAT) & 0x02fff000) != 0x02fff000) {
-		if (time_after(jiffies, timeout)) {
-			dev_warn(priv->dev,
-				 "Timeout waiting for DMFC FIFOs to clear\n");
-			break;
-		}
-		cpu_relax();
-	}
-}
-
 void ipu_dmfc_disable_channel(struct dmfc_channel *dmfc)
 {
 	struct ipu_dmfc_priv *priv = dmfc->priv;
@@ -145,10 +131,8 @@ void ipu_dmfc_disable_channel(struct dmfc_channel *dmfc)
 
 	priv->use_count--;
 
-	if (!priv->use_count) {
-		ipu_dmfc_wait_fifos(priv);
+	if (!priv->use_count)
 		ipu_module_disable(priv->ipu, IPU_CONF_DMFC_EN);
-	}
 
 	if (priv->use_count < 0)
 		priv->use_count = 0;
-- 
2.7.4

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

[PATCH v4 5/7] drm/imx: ipuv3-crtc: Use the callback ->atomic_disable instead of ->disable

[Liu Ying]

Now that the drm atomic core supports the callback ->atomic_disable,
we may replace the legacy one ->disable with it.

Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Peter Senna Tschudin <peter.senna@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v3->v4:
* None.

v3:
* Newly introduced in v3.

 drivers/gpu/drm/imx/ipuv3-crtc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index 6e1dc90..83c46bd 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -60,7 +60,8 @@ static void ipu_crtc_enable(struct drm_crtc *crtc)
 	ipu_di_enable(ipu_crtc->di);
 }
 
-static void ipu_crtc_disable(struct drm_crtc *crtc)
+static void ipu_crtc_atomic_disable(struct drm_crtc *crtc,
+				    struct drm_crtc_state *old_crtc_state)
 {
 	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
 	struct ipu_soc *ipu = dev_get_drvdata(ipu_crtc->dev->parent);
@@ -241,7 +242,7 @@ static const struct drm_crtc_helper_funcs ipu_helper_funcs = {
 	.mode_set_nofb = ipu_crtc_mode_set_nofb,
 	.atomic_check = ipu_crtc_atomic_check,
 	.atomic_begin = ipu_crtc_atomic_begin,
-	.disable = ipu_crtc_disable,
+	.atomic_disable = ipu_crtc_atomic_disable,
 	.enable = ipu_crtc_enable,
 };
 
-- 
2.7.4

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

[PATCH v4 6/7] drm/imx: Use DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET flag

[Liu Ying]

The IPUv3 display controller behind imx-drm needs all planes of
a CRTC be disabled when the CRTC is disabled.
The DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET flag reflects this
hardware requirement.  Let's use the flag for imx-drm.

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Peter Senna Tschudin <peter.senna@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v4:
* Newly introduced in v4, but some logic is picked from patch 3/3 in v3.

 drivers/gpu/drm/imx/imx-drm-core.c | 3 ++-
 drivers/gpu/drm/imx/ipuv3-crtc.c   | 3 +++
 drivers/gpu/drm/imx/ipuv3-plane.c  | 8 ++++++--
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 56dfc4c..99fff6c 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -194,7 +194,8 @@ static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state)
 	drm_atomic_helper_commit_modeset_disables(dev, state);
 
 	drm_atomic_helper_commit_planes(dev, state,
-					DRM_PLANE_COMMIT_ACTIVE_ONLY);
+				DRM_PLANE_COMMIT_ACTIVE_ONLY |
+				DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET);
 
 	drm_atomic_helper_commit_modeset_enables(dev, state);
 
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index 83c46bd..c3d5933 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -76,6 +76,9 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc,
 		crtc->state->event = NULL;
 	}
 	spin_unlock_irq(&crtc->dev->event_lock);
+
+	/* always disable planes on the CRTC */
+	drm_atomic_helper_disable_planes_on_crtc(old_crtc_state, true);
 }
 
 static void imx_drm_crtc_reset(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 4ad67d0..6a48147 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -392,8 +392,12 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
 	enum ipu_color_space ics;
 
 	if (old_state->fb) {
-		ipu_plane_atomic_set_base(ipu_plane, old_state);
-		return;
+		struct drm_crtc_state *crtc_state = state->crtc->state;
+
+		if (!drm_atomic_crtc_needs_modeset(crtc_state)) {
+			ipu_plane_atomic_set_base(ipu_plane, old_state);
+			return;
+		}
 	}
 
 	switch (ipu_plane->dp_flow) {
-- 
2.7.4

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

[PATCH v4 7/7] drm/imx: Add active plane reconfiguration support

[Liu Ying]

We don't support configuring active plane on-the-fly for imx-drm.
The relevant CRTC should be disabled before the plane configuration.
Of course, the plane itself should be disabled as well.

This patch adds active plane reconfiguration support by forcing CRTC
mode change in plane's ->atomic_check callback so that the CRTC
will be disabled before the plane configuration.

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Peter Senna Tschudin <peter.senna@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v4:
* Newly introduced in v4, but the logic is picked from patch 3/3 in v3.

 drivers/gpu/drm/imx/imx-drm-core.c | 26 +++++++++++++++++++++++++-
 drivers/gpu/drm/imx/ipuv3-plane.c  | 13 ++++++++-----
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 99fff6c..75f35ea 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -146,10 +146,34 @@ static void imx_drm_output_poll_changed(struct drm_device *drm)
 	drm_fbdev_cma_hotplug_event(imxdrm->fbhelper);
 }
 
+static int imx_drm_atomic_check(struct drm_device *dev,
+				struct drm_atomic_state *state)
+{
+	int ret;
+
+	ret = drm_atomic_helper_check_modeset(dev, state);
+	if (ret)
+		return ret;
+
+	ret = drm_atomic_helper_check_planes(dev, state);
+	if (ret)
+		return ret;
+
+	/*
+	 * Check modeset again in case crtc_state->mode_changed is
+	 * updated in plane's ->atomic_check callback.
+	 */
+	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_drm_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 6a48147..3c4f689 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -319,13 +319,16 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 		return -EINVAL;
 
 	/*
-	 * since we cannot touch active IDMAC channels, we do not support
-	 * resizing the enabled plane or changing its format
+	 * We support resizing active plane or changing its format by
+	 * forcing CRTC mode change in plane's ->atomic_check callback
+	 * and disabling all affected active planes in CRTC's ->atomic_disable
+	 * callback.  The planes will be reenabled in plane's ->atomic_update
+	 * callback.
 	 */
 	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;
+		crtc_state->mode_changed = true;
 
 	eba = drm_plane_state_to_eba(state);
 
@@ -336,7 +339,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 +375,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;
-- 
2.7.4

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

Re: [PATCH v4 3/7] drm/atomic-helper: Add NO_DISABLE_AFTER_MODESET flag support for plane commit

[Daniel Vetter]

On Fri, Aug 26, 2016 at 03:30:40PM +0800, Liu Ying wrote:
> Drivers may set the NO_DISABLE_AFTER_MODESET flag in the 'flags' parameter
> of the helper drm_atomic_helper_commit_planes() if the relevant display
> controllers(e.g., IPUv3 for imx-drm) require to disable a CRTC's planes
> when the CRTC is disabled. The helper would skip the ->atomic_disable
> call for a plane if the CRTC of the old plane state needs a modesetting
> operation. Of course, the drivers need to disable the planes in their CRTC
> disable callbacks since no one else would do that.
> 
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Peter Senna Tschudin <peter.senna@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Liu Ying <gnuiyl@gmail.com>

A few small nits below, otherwise looks good. I merged patches 1&2 into
drm-misc already.

Thanks, Daniel

> ---
> v4:
> * Newly introduced in v4.
> 
>  drivers/gpu/drm/arm/malidp_drv.c             |  3 +-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-
>  drivers/gpu/drm/drm_atomic_helper.c          | 46 ++++++++++++++++++++--------
>  drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-
>  drivers/gpu/drm/imx/imx-drm-core.c           |  3 +-
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c       |  6 ++--
>  drivers/gpu/drm/msm/msm_atomic.c             |  2 +-
>  drivers/gpu/drm/omapdrm/omap_drv.c           |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c        |  3 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c   |  3 +-
>  drivers/gpu/drm/sti/sti_drv.c                |  2 +-
>  drivers/gpu/drm/tegra/drm.c                  |  3 +-
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c          |  2 +-
>  drivers/gpu/drm/vc4/vc4_kms.c                |  2 +-
>  drivers/gpu/drm/virtio/virtgpu_display.c     |  3 +-
>  include/drm/drm_atomic_helper.h              |  6 +++-
>  16 files changed, 61 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 82171d2..c383d72 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -91,7 +91,8 @@ static void malidp_atomic_commit_tail(struct drm_atomic_state *state)
>  
>  	drm_atomic_helper_commit_modeset_disables(drm, state);
>  	drm_atomic_helper_commit_modeset_enables(drm, state);
> -	drm_atomic_helper_commit_planes(drm, state, true);
> +	drm_atomic_helper_commit_planes(drm, state,
> +					DRM_PLANE_COMMIT_ACTIVE_ONLY);
>  
>  	malidp_atomic_commit_hw_done(state);
>  
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index d4a3d61..8e7483d 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -457,7 +457,7 @@ atmel_hlcdc_dc_atomic_complete(struct atmel_hlcdc_dc_commit *commit)
>  
>  	/* Apply the atomic update. */
>  	drm_atomic_helper_commit_modeset_disables(dev, old_state);
> -	drm_atomic_helper_commit_planes(dev, old_state, false);
> +	drm_atomic_helper_commit_planes(dev, old_state, 0);
>  	drm_atomic_helper_commit_modeset_enables(dev, old_state);
>  
>  	drm_atomic_helper_wait_for_vblanks(dev, old_state);
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 30c52a8..cf19b6e 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1148,7 +1148,8 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks);
>   *
>   *     drm_atomic_helper_commit_modeset_enables(dev, state);
>   *
> - *     drm_atomic_helper_commit_planes(dev, state, true);
> + *     drm_atomic_helper_commit_planes(dev, state,
> + *                                     DRM_PLANE_COMMIT_ACTIVE_ONLY);
>   *
>   * for committing the atomic update to hardware.  See the kerneldoc entries for
>   * these three functions for more details.
> @@ -1159,7 +1160,7 @@ void drm_atomic_helper_commit_tail(struct drm_atomic_state *state)
>  
>  	drm_atomic_helper_commit_modeset_disables(dev, state);
>  
> -	drm_atomic_helper_commit_planes(dev, state, false);
> +	drm_atomic_helper_commit_planes(dev, state, 0);
>  
>  	drm_atomic_helper_commit_modeset_enables(dev, state);
>  
> @@ -1678,7 +1679,7 @@ bool plane_crtc_active(struct drm_plane_state *state)
>   * drm_atomic_helper_commit_planes - commit plane state
>   * @dev: DRM device
>   * @old_state: atomic state object with old state structures
> - * @active_only: Only commit on active CRTC if set
> + * @flags: flags for committing plane state
>   *
>   * This function commits the new plane state using the plane and atomic helper
>   * functions for planes and crtcs. It assumes that the atomic state has already
> @@ -1698,25 +1699,33 @@ bool plane_crtc_active(struct drm_plane_state *state)
>   * most drivers don't need to be immediately notified of plane updates for a
>   * disabled CRTC.
>   *
> - * Unless otherwise needed, drivers are advised to set the @active_only
> - * parameters to true in order not to receive plane update notifications related
> - * to a disabled CRTC. This avoids the need to manually ignore plane updates in
> + * Unless otherwise needed, drivers are advised to set the ACTIVE_ONLY flag in
> + * @flags in order not to receive plane update notifications related to a
> + * disabled CRTC. This avoids the need to manually ignore plane updates in
>   * driver code when the driver and/or hardware can't or just don't need to deal
>   * with updates on disabled CRTCs, for example when supporting runtime PM.
> - *

Please leave this newline here to start a new paragraph. I think that
makes the docs more readable.

> - * The drm_atomic_helper_commit() default implementation only sets @active_only
> - * to false to most closely match the behaviour of the legacy helpers. This should
> - * not be copied blindly by drivers.
> + * Drivers may set the NO_DISABLE_AFTER_MODESET flag in @flags if the relevant
> + * display controllers require to disable a CRTC's planes when the CRTC is
> + * disabled. This function would skip the ->atomic_disable call for a plane if
> + * the CRTC of the old plane state needs a modesetting operation. Of course,
> + * the drivers need to disable the planes in their CRTC disable callbacks
> + * since no one else would do that.
> + *
> + * The drm_atomic_helper_commit() default implementation doesn't set the
> + * ACTIVE_ONLY flag to most closely match the behaviour of the legacy helpers.
> + * This should not be copied blindly by drivers.
>   */
>  void drm_atomic_helper_commit_planes(struct drm_device *dev,
>  				     struct drm_atomic_state *old_state,
> -				     bool active_only)
> +				     uint32_t flags)
>  {
>  	struct drm_crtc *crtc;
>  	struct drm_crtc_state *old_crtc_state;
>  	struct drm_plane *plane;
>  	struct drm_plane_state *old_plane_state;
>  	int i;
> +	bool active_only = flags & DRM_PLANE_COMMIT_ACTIVE_ONLY;
> +	bool no_disable = flags & DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET;
>  
>  	for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
>  		const struct drm_crtc_helper_funcs *funcs;
> @@ -1760,9 +1769,20 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
>  		/*
>  		 * Special-case disabling the plane if drivers support it.
>  		 */
> -		if (disabling && funcs->atomic_disable)
> +		if (disabling && funcs->atomic_disable) {
> +			struct drm_crtc_state *crtc_state;
> +
> +			crtc_state = old_plane_state->crtc->state;
> +			WARN_ON(!crtc_state);
> +			if (!crtc_state)
> +				continue;

This is already enforced by the atomic core, I don't think there's any
point in checking for the crtc state here. Please remove the WARN_ON and
if () continue;

> +
> +			if (drm_atomic_crtc_needs_modeset(crtc_state) &&
> +			    no_disable)
> +				continue;
> +
>  			funcs->atomic_disable(plane, old_plane_state);
> -		else if (plane->state->crtc || disabling)
> +		} else if (plane->state->crtc || disabling)
>  			funcs->atomic_update(plane, old_plane_state);

Checkpatch says you should have {} for the else too.

>  	}
>  
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 877d2ef..486943e 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -105,7 +105,7 @@ static void exynos_atomic_commit_complete(struct exynos_atomic_commit *commit)
>  		atomic_inc(&exynos_crtc->pending_update);
>  	}
>  
> -	drm_atomic_helper_commit_planes(dev, state, false);
> +	drm_atomic_helper_commit_planes(dev, state, 0);
>  
>  	exynos_atomic_wait_for_commit(state);
>  
> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
> index 438bac8..56dfc4c 100644
> --- a/drivers/gpu/drm/imx/imx-drm-core.c
> +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> @@ -193,7 +193,8 @@ static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state)
>  
>  	drm_atomic_helper_commit_modeset_disables(dev, state);
>  
> -	drm_atomic_helper_commit_planes(dev, state, true);
> +	drm_atomic_helper_commit_planes(dev, state,
> +					DRM_PLANE_COMMIT_ACTIVE_ONLY);
>  
>  	drm_atomic_helper_commit_modeset_enables(dev, state);
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 0e769ab..72c1ae4 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -70,13 +70,15 @@ static void mtk_atomic_complete(struct mtk_drm_private *private,
>  	 *
>  	 *     drm_atomic_helper_commit_modeset_disables(dev, state);
>  	 *     drm_atomic_helper_commit_modeset_enables(dev, state);
> -	 *     drm_atomic_helper_commit_planes(dev, state, true);
> +	 *     drm_atomic_helper_commit_planes(dev, state,
> +	 *                                     DRM_PLANE_COMMIT_ACTIVE_ONLY);
>  	 *
>  	 * See the kerneldoc entries for these three functions for more details.
>  	 */
>  	drm_atomic_helper_commit_modeset_disables(drm, state);
>  	drm_atomic_helper_commit_modeset_enables(drm, state);
> -	drm_atomic_helper_commit_planes(drm, state, true);
> +	drm_atomic_helper_commit_planes(drm, state,
> +					DRM_PLANE_COMMIT_ACTIVE_ONLY);
>  
>  	drm_atomic_helper_wait_for_vblanks(drm, state);
>  
> diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
> index 4a8a6f1..5df252c 100644
> --- a/drivers/gpu/drm/msm/msm_atomic.c
> +++ b/drivers/gpu/drm/msm/msm_atomic.c
> @@ -118,7 +118,7 @@ static void complete_commit(struct msm_commit *c, bool async)
>  
>  	drm_atomic_helper_commit_modeset_disables(dev, state);
>  
> -	drm_atomic_helper_commit_planes(dev, state, false);
> +	drm_atomic_helper_commit_planes(dev, state, 0);
>  
>  	drm_atomic_helper_commit_modeset_enables(dev, state);
>  
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> index 3dd78f2..e1cfba5 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -96,7 +96,7 @@ static void omap_atomic_complete(struct omap_atomic_state_commit *commit)
>  	dispc_runtime_get();
>  
>  	drm_atomic_helper_commit_modeset_disables(dev, old_state);
> -	drm_atomic_helper_commit_planes(dev, old_state, false);
> +	drm_atomic_helper_commit_planes(dev, old_state, 0);
>  	drm_atomic_helper_commit_modeset_enables(dev, old_state);
>  
>  	omap_atomic_wait_for_completion(dev, old_state);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index f03eb55..bd9c3bb 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -257,7 +257,8 @@ static void rcar_du_atomic_complete(struct rcar_du_commit *commit)
>  	/* Apply the atomic update. */
>  	drm_atomic_helper_commit_modeset_disables(dev, old_state);
>  	drm_atomic_helper_commit_modeset_enables(dev, old_state);
> -	drm_atomic_helper_commit_planes(dev, old_state, true);
> +	drm_atomic_helper_commit_planes(dev, old_state,
> +					DRM_PLANE_COMMIT_ACTIVE_ONLY);
>  
>  	drm_atomic_helper_wait_for_vblanks(dev, old_state);
>  
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index 55c5273..7ca8f34 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -233,7 +233,8 @@ rockchip_atomic_commit_tail(struct drm_atomic_state *state)
>  
>  	drm_atomic_helper_commit_modeset_enables(dev, state);
>  
> -	drm_atomic_helper_commit_planes(dev, state, true);
> +	drm_atomic_helper_commit_planes(dev, state,
> +					DRM_PLANE_COMMIT_ACTIVE_ONLY);
>  
>  	drm_atomic_helper_commit_hw_done(state);
>  
> diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
> index f8311b2..7cd3804 100644
> --- a/drivers/gpu/drm/sti/sti_drv.c
> +++ b/drivers/gpu/drm/sti/sti_drv.c
> @@ -178,7 +178,7 @@ static void sti_atomic_complete(struct sti_private *private,
>  	 */
>  
>  	drm_atomic_helper_commit_modeset_disables(drm, state);
> -	drm_atomic_helper_commit_planes(drm, state, false);
> +	drm_atomic_helper_commit_planes(drm, state, 0);
>  	drm_atomic_helper_commit_modeset_enables(drm, state);
>  
>  	drm_atomic_helper_wait_for_vblanks(drm, state);
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index 755264d9d..4b9f1c7 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -57,7 +57,8 @@ static void tegra_atomic_complete(struct tegra_drm *tegra,
>  
>  	drm_atomic_helper_commit_modeset_disables(drm, state);
>  	drm_atomic_helper_commit_modeset_enables(drm, state);
> -	drm_atomic_helper_commit_planes(drm, state, true);
> +	drm_atomic_helper_commit_planes(drm, state,
> +					DRM_PLANE_COMMIT_ACTIVE_ONLY);
>  
>  	drm_atomic_helper_wait_for_vblanks(drm, state);
>  
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index 3404d24..4405e4b 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -118,7 +118,7 @@ static int tilcdc_commit(struct drm_device *dev,
>  
>  	drm_atomic_helper_commit_modeset_disables(dev, state);
>  
> -	drm_atomic_helper_commit_planes(dev, state, false);
> +	drm_atomic_helper_commit_planes(dev, state, 0);
>  
>  	drm_atomic_helper_commit_modeset_enables(dev, state);
>  
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index 4ac894d..c1f65c6 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -44,7 +44,7 @@ vc4_atomic_complete_commit(struct vc4_commit *c)
>  
>  	drm_atomic_helper_commit_modeset_disables(dev, state);
>  
> -	drm_atomic_helper_commit_planes(dev, state, false);
> +	drm_atomic_helper_commit_planes(dev, state, 0);
>  
>  	drm_atomic_helper_commit_modeset_enables(dev, state);
>  
> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
> index 4e192aa..7cf3678 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
> @@ -338,7 +338,8 @@ static void vgdev_atomic_commit_tail(struct drm_atomic_state *state)
>  
>  	drm_atomic_helper_commit_modeset_disables(dev, state);
>  	drm_atomic_helper_commit_modeset_enables(dev, state);
> -	drm_atomic_helper_commit_planes(dev, state, true);
> +	drm_atomic_helper_commit_planes(dev, state,
> +					DRM_PLANE_COMMIT_ACTIVE_ONLY);
>  
>  	drm_atomic_helper_commit_hw_done(state);
>  
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index 1abf2c0..f866828 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -65,9 +65,13 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
>  
>  int drm_atomic_helper_prepare_planes(struct drm_device *dev,
>  				     struct drm_atomic_state *state);
> +
> +#define DRM_PLANE_COMMIT_ACTIVE_ONLY			BIT(0)
> +#define DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET	BIT(1)
> +
>  void drm_atomic_helper_commit_planes(struct drm_device *dev,
>  				     struct drm_atomic_state *state,
> -				     bool active_only);
> +				     uint32_t flags);
>  void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
>  				      struct drm_atomic_state *old_state);
>  void drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state);
> -- 
> 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 v4 4/7] gpu: ipu-v3: Do not wait for DMFC FIFO to clear when disabling DMFC channel

[Philipp Zabel]

Am Freitag, den 26.08.2016, 15:30 +0800 schrieb Liu Ying:
> According to basic tests, it looks there is no issue if we don't wait for
> DMFC FIFO to clear when disabling DMFC channel.  NXP BSP doesn't do that,
> either.  This patch is needed to avoid the annoying warning caused by a
> timeout on waiting for the FIFO to clear after we add the new
> DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET flag to the imx-drm driver
> which changes the procedure to disable display channel slightly.

I suppose the reason this happens is that now DC/DI are disabled first,
so the DC can't drain the FIFO anymore. If the FIFO is properly reset
when reenabling the DMFC, this shouldn't have any ill effects.

regards
Philipp

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

Re: [PATCH v4 3/7] drm/atomic-helper: Add NO_DISABLE_AFTER_MODESET flag support for plane commit

[Ying Liu]

On Mon, Aug 29, 2016 at 4:25 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Aug 26, 2016 at 03:30:40PM +0800, Liu Ying wrote:
>> Drivers may set the NO_DISABLE_AFTER_MODESET flag in the 'flags' parameter
>> of the helper drm_atomic_helper_commit_planes() if the relevant display
>> controllers(e.g., IPUv3 for imx-drm) require to disable a CRTC's planes
>> when the CRTC is disabled. The helper would skip the ->atomic_disable
>> call for a plane if the CRTC of the old plane state needs a modesetting
>> operation. Of course, the drivers need to disable the planes in their CRTC
>> disable callbacks since no one else would do that.
>>
>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>> Cc: David Airlie <airlied@linux.ie>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Cc: Peter Senna Tschudin <peter.senna@gmail.com>
>> Cc: Lucas Stach <l.stach@pengutronix.de>
>> Signed-off-by: Liu Ying <gnuiyl@gmail.com>
>
> A few small nits below, otherwise looks good. I merged patches 1&2 into
> drm-misc already.

I'll address all your below comments and send v2 for this one separately.

Thanks.

Regards,
Liu Ying

>
> Thanks, Daniel
>
>> ---
>> v4:
>> * Newly introduced in v4.
>>
>>  drivers/gpu/drm/arm/malidp_drv.c             |  3 +-
>>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-
>>  drivers/gpu/drm/drm_atomic_helper.c          | 46 ++++++++++++++++++++--------
>>  drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-
>>  drivers/gpu/drm/imx/imx-drm-core.c           |  3 +-
>>  drivers/gpu/drm/mediatek/mtk_drm_drv.c       |  6 ++--
>>  drivers/gpu/drm/msm/msm_atomic.c             |  2 +-
>>  drivers/gpu/drm/omapdrm/omap_drv.c           |  2 +-
>>  drivers/gpu/drm/rcar-du/rcar_du_kms.c        |  3 +-
>>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c   |  3 +-
>>  drivers/gpu/drm/sti/sti_drv.c                |  2 +-
>>  drivers/gpu/drm/tegra/drm.c                  |  3 +-
>>  drivers/gpu/drm/tilcdc/tilcdc_drv.c          |  2 +-
>>  drivers/gpu/drm/vc4/vc4_kms.c                |  2 +-
>>  drivers/gpu/drm/virtio/virtgpu_display.c     |  3 +-
>>  include/drm/drm_atomic_helper.h              |  6 +++-
>>  16 files changed, 61 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
>> index 82171d2..c383d72 100644
>> --- a/drivers/gpu/drm/arm/malidp_drv.c
>> +++ b/drivers/gpu/drm/arm/malidp_drv.c
>> @@ -91,7 +91,8 @@ static void malidp_atomic_commit_tail(struct drm_atomic_state *state)
>>
>>       drm_atomic_helper_commit_modeset_disables(drm, state);
>>       drm_atomic_helper_commit_modeset_enables(drm, state);
>> -     drm_atomic_helper_commit_planes(drm, state, true);
>> +     drm_atomic_helper_commit_planes(drm, state,
>> +                                     DRM_PLANE_COMMIT_ACTIVE_ONLY);
>>
>>       malidp_atomic_commit_hw_done(state);
>>
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> index d4a3d61..8e7483d 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> @@ -457,7 +457,7 @@ atmel_hlcdc_dc_atomic_complete(struct atmel_hlcdc_dc_commit *commit)
>>
>>       /* Apply the atomic update. */
>>       drm_atomic_helper_commit_modeset_disables(dev, old_state);
>> -     drm_atomic_helper_commit_planes(dev, old_state, false);
>> +     drm_atomic_helper_commit_planes(dev, old_state, 0);
>>       drm_atomic_helper_commit_modeset_enables(dev, old_state);
>>
>>       drm_atomic_helper_wait_for_vblanks(dev, old_state);
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index 30c52a8..cf19b6e 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -1148,7 +1148,8 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks);
>>   *
>>   *     drm_atomic_helper_commit_modeset_enables(dev, state);
>>   *
>> - *     drm_atomic_helper_commit_planes(dev, state, true);
>> + *     drm_atomic_helper_commit_planes(dev, state,
>> + *                                     DRM_PLANE_COMMIT_ACTIVE_ONLY);
>>   *
>>   * for committing the atomic update to hardware.  See the kerneldoc entries for
>>   * these three functions for more details.
>> @@ -1159,7 +1160,7 @@ void drm_atomic_helper_commit_tail(struct drm_atomic_state *state)
>>
>>       drm_atomic_helper_commit_modeset_disables(dev, state);
>>
>> -     drm_atomic_helper_commit_planes(dev, state, false);
>> +     drm_atomic_helper_commit_planes(dev, state, 0);
>>
>>       drm_atomic_helper_commit_modeset_enables(dev, state);
>>
>> @@ -1678,7 +1679,7 @@ bool plane_crtc_active(struct drm_plane_state *state)
>>   * drm_atomic_helper_commit_planes - commit plane state
>>   * @dev: DRM device
>>   * @old_state: atomic state object with old state structures
>> - * @active_only: Only commit on active CRTC if set
>> + * @flags: flags for committing plane state
>>   *
>>   * This function commits the new plane state using the plane and atomic helper
>>   * functions for planes and crtcs. It assumes that the atomic state has already
>> @@ -1698,25 +1699,33 @@ bool plane_crtc_active(struct drm_plane_state *state)
>>   * most drivers don't need to be immediately notified of plane updates for a
>>   * disabled CRTC.
>>   *
>> - * Unless otherwise needed, drivers are advised to set the @active_only
>> - * parameters to true in order not to receive plane update notifications related
>> - * to a disabled CRTC. This avoids the need to manually ignore plane updates in
>> + * Unless otherwise needed, drivers are advised to set the ACTIVE_ONLY flag in
>> + * @flags in order not to receive plane update notifications related to a
>> + * disabled CRTC. This avoids the need to manually ignore plane updates in
>>   * driver code when the driver and/or hardware can't or just don't need to deal
>>   * with updates on disabled CRTCs, for example when supporting runtime PM.
>> - *
>
> Please leave this newline here to start a new paragraph. I think that
> makes the docs more readable.
>
>> - * The drm_atomic_helper_commit() default implementation only sets @active_only
>> - * to false to most closely match the behaviour of the legacy helpers. This should
>> - * not be copied blindly by drivers.
>> + * Drivers may set the NO_DISABLE_AFTER_MODESET flag in @flags if the relevant
>> + * display controllers require to disable a CRTC's planes when the CRTC is
>> + * disabled. This function would skip the ->atomic_disable call for a plane if
>> + * the CRTC of the old plane state needs a modesetting operation. Of course,
>> + * the drivers need to disable the planes in their CRTC disable callbacks
>> + * since no one else would do that.
>> + *
>> + * The drm_atomic_helper_commit() default implementation doesn't set the
>> + * ACTIVE_ONLY flag to most closely match the behaviour of the legacy helpers.
>> + * This should not be copied blindly by drivers.
>>   */
>>  void drm_atomic_helper_commit_planes(struct drm_device *dev,
>>                                    struct drm_atomic_state *old_state,
>> -                                  bool active_only)
>> +                                  uint32_t flags)
>>  {
>>       struct drm_crtc *crtc;
>>       struct drm_crtc_state *old_crtc_state;
>>       struct drm_plane *plane;
>>       struct drm_plane_state *old_plane_state;
>>       int i;
>> +     bool active_only = flags & DRM_PLANE_COMMIT_ACTIVE_ONLY;
>> +     bool no_disable = flags & DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET;
>>
>>       for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
>>               const struct drm_crtc_helper_funcs *funcs;
>> @@ -1760,9 +1769,20 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
>>               /*
>>                * Special-case disabling the plane if drivers support it.
>>                */
>> -             if (disabling && funcs->atomic_disable)
>> +             if (disabling && funcs->atomic_disable) {
>> +                     struct drm_crtc_state *crtc_state;
>> +
>> +                     crtc_state = old_plane_state->crtc->state;
>> +                     WARN_ON(!crtc_state);
>> +                     if (!crtc_state)
>> +                             continue;
>
> This is already enforced by the atomic core, I don't think there's any
> point in checking for the crtc state here. Please remove the WARN_ON and
> if () continue;
>
>> +
>> +                     if (drm_atomic_crtc_needs_modeset(crtc_state) &&
>> +                         no_disable)
>> +                             continue;
>> +
>>                       funcs->atomic_disable(plane, old_plane_state);
>> -             else if (plane->state->crtc || disabling)
>> +             } else if (plane->state->crtc || disabling)
>>                       funcs->atomic_update(plane, old_plane_state);
>
> Checkpatch says you should have {} for the else too.
>
>>       }
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> index 877d2ef..486943e 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> @@ -105,7 +105,7 @@ static void exynos_atomic_commit_complete(struct exynos_atomic_commit *commit)
>>               atomic_inc(&exynos_crtc->pending_update);
>>       }
>>
>> -     drm_atomic_helper_commit_planes(dev, state, false);
>> +     drm_atomic_helper_commit_planes(dev, state, 0);
>>
>>       exynos_atomic_wait_for_commit(state);
>>
>> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
>> index 438bac8..56dfc4c 100644
>> --- a/drivers/gpu/drm/imx/imx-drm-core.c
>> +++ b/drivers/gpu/drm/imx/imx-drm-core.c
>> @@ -193,7 +193,8 @@ static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state)
>>
>>       drm_atomic_helper_commit_modeset_disables(dev, state);
>>
>> -     drm_atomic_helper_commit_planes(dev, state, true);
>> +     drm_atomic_helper_commit_planes(dev, state,
>> +                                     DRM_PLANE_COMMIT_ACTIVE_ONLY);
>>
>>       drm_atomic_helper_commit_modeset_enables(dev, state);
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
>> index 0e769ab..72c1ae4 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
>> @@ -70,13 +70,15 @@ static void mtk_atomic_complete(struct mtk_drm_private *private,
>>        *
>>        *     drm_atomic_helper_commit_modeset_disables(dev, state);
>>        *     drm_atomic_helper_commit_modeset_enables(dev, state);
>> -      *     drm_atomic_helper_commit_planes(dev, state, true);
>> +      *     drm_atomic_helper_commit_planes(dev, state,
>> +      *                                     DRM_PLANE_COMMIT_ACTIVE_ONLY);
>>        *
>>        * See the kerneldoc entries for these three functions for more details.
>>        */
>>       drm_atomic_helper_commit_modeset_disables(drm, state);
>>       drm_atomic_helper_commit_modeset_enables(drm, state);
>> -     drm_atomic_helper_commit_planes(drm, state, true);
>> +     drm_atomic_helper_commit_planes(drm, state,
>> +                                     DRM_PLANE_COMMIT_ACTIVE_ONLY);
>>
>>       drm_atomic_helper_wait_for_vblanks(drm, state);
>>
>> diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
>> index 4a8a6f1..5df252c 100644
>> --- a/drivers/gpu/drm/msm/msm_atomic.c
>> +++ b/drivers/gpu/drm/msm/msm_atomic.c
>> @@ -118,7 +118,7 @@ static void complete_commit(struct msm_commit *c, bool async)
>>
>>       drm_atomic_helper_commit_modeset_disables(dev, state);
>>
>> -     drm_atomic_helper_commit_planes(dev, state, false);
>> +     drm_atomic_helper_commit_planes(dev, state, 0);
>>
>>       drm_atomic_helper_commit_modeset_enables(dev, state);
>>
>> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
>> index 3dd78f2..e1cfba5 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
>> @@ -96,7 +96,7 @@ static void omap_atomic_complete(struct omap_atomic_state_commit *commit)
>>       dispc_runtime_get();
>>
>>       drm_atomic_helper_commit_modeset_disables(dev, old_state);
>> -     drm_atomic_helper_commit_planes(dev, old_state, false);
>> +     drm_atomic_helper_commit_planes(dev, old_state, 0);
>>       drm_atomic_helper_commit_modeset_enables(dev, old_state);
>>
>>       omap_atomic_wait_for_completion(dev, old_state);
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> index f03eb55..bd9c3bb 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> @@ -257,7 +257,8 @@ static void rcar_du_atomic_complete(struct rcar_du_commit *commit)
>>       /* Apply the atomic update. */
>>       drm_atomic_helper_commit_modeset_disables(dev, old_state);
>>       drm_atomic_helper_commit_modeset_enables(dev, old_state);
>> -     drm_atomic_helper_commit_planes(dev, old_state, true);
>> +     drm_atomic_helper_commit_planes(dev, old_state,
>> +                                     DRM_PLANE_COMMIT_ACTIVE_ONLY);
>>
>>       drm_atomic_helper_wait_for_vblanks(dev, old_state);
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>> index 55c5273..7ca8f34 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>> @@ -233,7 +233,8 @@ rockchip_atomic_commit_tail(struct drm_atomic_state *state)
>>
>>       drm_atomic_helper_commit_modeset_enables(dev, state);
>>
>> -     drm_atomic_helper_commit_planes(dev, state, true);
>> +     drm_atomic_helper_commit_planes(dev, state,
>> +                                     DRM_PLANE_COMMIT_ACTIVE_ONLY);
>>
>>       drm_atomic_helper_commit_hw_done(state);
>>
>> diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
>> index f8311b2..7cd3804 100644
>> --- a/drivers/gpu/drm/sti/sti_drv.c
>> +++ b/drivers/gpu/drm/sti/sti_drv.c
>> @@ -178,7 +178,7 @@ static void sti_atomic_complete(struct sti_private *private,
>>        */
>>
>>       drm_atomic_helper_commit_modeset_disables(drm, state);
>> -     drm_atomic_helper_commit_planes(drm, state, false);
>> +     drm_atomic_helper_commit_planes(drm, state, 0);
>>       drm_atomic_helper_commit_modeset_enables(drm, state);
>>
>>       drm_atomic_helper_wait_for_vblanks(drm, state);
>> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
>> index 755264d9d..4b9f1c7 100644
>> --- a/drivers/gpu/drm/tegra/drm.c
>> +++ b/drivers/gpu/drm/tegra/drm.c
>> @@ -57,7 +57,8 @@ static void tegra_atomic_complete(struct tegra_drm *tegra,
>>
>>       drm_atomic_helper_commit_modeset_disables(drm, state);
>>       drm_atomic_helper_commit_modeset_enables(drm, state);
>> -     drm_atomic_helper_commit_planes(drm, state, true);
>> +     drm_atomic_helper_commit_planes(drm, state,
>> +                                     DRM_PLANE_COMMIT_ACTIVE_ONLY);
>>
>>       drm_atomic_helper_wait_for_vblanks(drm, state);
>>
>> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
>> index 3404d24..4405e4b 100644
>> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
>> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
>> @@ -118,7 +118,7 @@ static int tilcdc_commit(struct drm_device *dev,
>>
>>       drm_atomic_helper_commit_modeset_disables(dev, state);
>>
>> -     drm_atomic_helper_commit_planes(dev, state, false);
>> +     drm_atomic_helper_commit_planes(dev, state, 0);
>>
>>       drm_atomic_helper_commit_modeset_enables(dev, state);
>>
>> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
>> index 4ac894d..c1f65c6 100644
>> --- a/drivers/gpu/drm/vc4/vc4_kms.c
>> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
>> @@ -44,7 +44,7 @@ vc4_atomic_complete_commit(struct vc4_commit *c)
>>
>>       drm_atomic_helper_commit_modeset_disables(dev, state);
>>
>> -     drm_atomic_helper_commit_planes(dev, state, false);
>> +     drm_atomic_helper_commit_planes(dev, state, 0);
>>
>>       drm_atomic_helper_commit_modeset_enables(dev, state);
>>
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
>> index 4e192aa..7cf3678 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
>> @@ -338,7 +338,8 @@ static void vgdev_atomic_commit_tail(struct drm_atomic_state *state)
>>
>>       drm_atomic_helper_commit_modeset_disables(dev, state);
>>       drm_atomic_helper_commit_modeset_enables(dev, state);
>> -     drm_atomic_helper_commit_planes(dev, state, true);
>> +     drm_atomic_helper_commit_planes(dev, state,
>> +                                     DRM_PLANE_COMMIT_ACTIVE_ONLY);
>>
>>       drm_atomic_helper_commit_hw_done(state);
>>
>> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
>> index 1abf2c0..f866828 100644
>> --- a/include/drm/drm_atomic_helper.h
>> +++ b/include/drm/drm_atomic_helper.h
>> @@ -65,9 +65,13 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
>>
>>  int drm_atomic_helper_prepare_planes(struct drm_device *dev,
>>                                    struct drm_atomic_state *state);
>> +
>> +#define DRM_PLANE_COMMIT_ACTIVE_ONLY                 BIT(0)
>> +#define DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET    BIT(1)
>> +
>>  void drm_atomic_helper_commit_planes(struct drm_device *dev,
>>                                    struct drm_atomic_state *state,
>> -                                  bool active_only);
>> +                                  uint32_t flags);
>>  void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
>>                                     struct drm_atomic_state *old_state);
>>  void drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state);
>> --
>> 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 v4 4/7] gpu: ipu-v3: Do not wait for DMFC FIFO to clear when disabling DMFC channel

[Ying Liu]

On Mon, Aug 29, 2016 at 4:46 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Am Freitag, den 26.08.2016, 15:30 +0800 schrieb Liu Ying:
>> According to basic tests, it looks there is no issue if we don't wait for
>> DMFC FIFO to clear when disabling DMFC channel.  NXP BSP doesn't do that,
>> either.  This patch is needed to avoid the annoying warning caused by a
>> timeout on waiting for the FIFO to clear after we add the new
>> DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET flag to the imx-drm driver
>> which changes the procedure to disable display channel slightly.
>
> I suppose the reason this happens is that now DC/DI are disabled first,
> so the DC can't drain the FIFO anymore. If the FIFO is properly reset
> when reenabling the DMFC, this shouldn't have any ill effects.

I found the timeout warning issue by blanking the framebuffer.
Ofc, the framebuffer is supported by the fbdev emulation.
Before applying this patch set, the planes are not even disabled
when the framebuffer is blanked, that is to say, plane_funcs->
atomic_disable is not called - the CRTC is disabled alone.
After applying this patch set, the planes are always disabled
together with the CRTC.  And, yes, DC/DI are disabled first,
then the timeout warning happens.

Please note the warning happens when the planes are disabled
instead of reenabled.  So, I don't get your point by resetting
FIFO when _reenabling_  DMFC.  And, I don't see the way to
reset FIFO.

Regards,
Liu Ying

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

Re: [PATCH v4 4/7] gpu: ipu-v3: Do not wait for DMFC FIFO to clear when disabling DMFC channel

[Philipp Zabel]

Am Montag, den 29.08.2016, 17:36 +0800 schrieb Ying Liu:
> On Mon, Aug 29, 2016 at 4:46 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > Am Freitag, den 26.08.2016, 15:30 +0800 schrieb Liu Ying:
> >> According to basic tests, it looks there is no issue if we don't wait for
> >> DMFC FIFO to clear when disabling DMFC channel.  NXP BSP doesn't do that,
> >> either.  This patch is needed to avoid the annoying warning caused by a
> >> timeout on waiting for the FIFO to clear after we add the new
> >> DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET flag to the imx-drm driver
> >> which changes the procedure to disable display channel slightly.
> >
> > I suppose the reason this happens is that now DC/DI are disabled first,
> > so the DC can't drain the FIFO anymore. If the FIFO is properly reset
> > when reenabling the DMFC, this shouldn't have any ill effects.
> 
> I found the timeout warning issue by blanking the framebuffer.
> Ofc, the framebuffer is supported by the fbdev emulation.
> Before applying this patch set, the planes are not even disabled
> when the framebuffer is blanked, that is to say, plane_funcs->
> atomic_disable is not called - the CRTC is disabled alone.
> After applying this patch set, the planes are always disabled
> together with the CRTC.  And, yes, DC/DI are disabled first,
> then the timeout warning happens.
> 
> Please note the warning happens when the planes are disabled
> instead of reenabled.  So, I don't get your point by resetting
> FIFO when _reenabling_  DMFC.

If we disable the DMFC with data still in the FIFO and then reenable it
and the DC first reads the stale data from the FIFO, that should cause a
visible artifact in the first frame after reenabling the plane. If that
doesn't happen, I trust that the hardware resets the FIFO state
somewhere along the way.

> And, I don't see the way to reset FIFO.

We could reset the DMFC_WR memory after disabling the DMFC, but I'm not
sure this is necessary at all.

regards
Philipp

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

Re: [PATCH v4 4/7] gpu: ipu-v3: Do not wait for DMFC FIFO to clear when disabling DMFC channel

[Ying Liu]

On Mon, Aug 29, 2016 at 5:46 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Am Montag, den 29.08.2016, 17:36 +0800 schrieb Ying Liu:
>> On Mon, Aug 29, 2016 at 4:46 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
>> > Am Freitag, den 26.08.2016, 15:30 +0800 schrieb Liu Ying:
>> >> According to basic tests, it looks there is no issue if we don't wait for
>> >> DMFC FIFO to clear when disabling DMFC channel.  NXP BSP doesn't do that,
>> >> either.  This patch is needed to avoid the annoying warning caused by a
>> >> timeout on waiting for the FIFO to clear after we add the new
>> >> DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET flag to the imx-drm driver
>> >> which changes the procedure to disable display channel slightly.
>> >
>> > I suppose the reason this happens is that now DC/DI are disabled first,
>> > so the DC can't drain the FIFO anymore. If the FIFO is properly reset
>> > when reenabling the DMFC, this shouldn't have any ill effects.
>>
>> I found the timeout warning issue by blanking the framebuffer.
>> Ofc, the framebuffer is supported by the fbdev emulation.
>> Before applying this patch set, the planes are not even disabled
>> when the framebuffer is blanked, that is to say, plane_funcs->
>> atomic_disable is not called - the CRTC is disabled alone.
>> After applying this patch set, the planes are always disabled
>> together with the CRTC.  And, yes, DC/DI are disabled first,
>> then the timeout warning happens.
>>
>> Please note the warning happens when the planes are disabled
>> instead of reenabled.  So, I don't get your point by resetting
>> FIFO when _reenabling_  DMFC.
>
> If we disable the DMFC with data still in the FIFO and then reenable it
> and the DC first reads the stale data from the FIFO, that should cause a
> visible artifact in the first frame after reenabling the plane. If that
> doesn't happen, I trust that the hardware resets the FIFO state
> somewhere along the way.

Not sure if the hardware resets the FIFO automatically...
The NXP driver waits for some hardware status/irqs when disabling
the channels, but it doesn't wait for DMFC status.

>
>> And, I don't see the way to reset FIFO.
>
> We could reset the DMFC_WR memory after disabling the DMFC, but I'm not
> sure this is necessary at all.

You mean the register DMFC_WR_CHAN, for instance?
I still don't see how we could reset the FIFO.

Regards,
Liu Ying

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

Re: [PATCH v4 4/7] gpu: ipu-v3: Do not wait for DMFC FIFO to clear when disabling DMFC channel

[Philipp Zabel]

Am Montag, den 29.08.2016, 17:57 +0800 schrieb Ying Liu:
> On Mon, Aug 29, 2016 at 5:46 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > Am Montag, den 29.08.2016, 17:36 +0800 schrieb Ying Liu:
> >> On Mon, Aug 29, 2016 at 4:46 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> >> > Am Freitag, den 26.08.2016, 15:30 +0800 schrieb Liu Ying:
> >> >> According to basic tests, it looks there is no issue if we don't wait for
> >> >> DMFC FIFO to clear when disabling DMFC channel.  NXP BSP doesn't do that,
> >> >> either.  This patch is needed to avoid the annoying warning caused by a
> >> >> timeout on waiting for the FIFO to clear after we add the new
> >> >> DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET flag to the imx-drm driver
> >> >> which changes the procedure to disable display channel slightly.
> >> >
> >> > I suppose the reason this happens is that now DC/DI are disabled first,
> >> > so the DC can't drain the FIFO anymore. If the FIFO is properly reset
> >> > when reenabling the DMFC, this shouldn't have any ill effects.
> >>
> >> I found the timeout warning issue by blanking the framebuffer.
> >> Ofc, the framebuffer is supported by the fbdev emulation.
> >> Before applying this patch set, the planes are not even disabled
> >> when the framebuffer is blanked, that is to say, plane_funcs->
> >> atomic_disable is not called - the CRTC is disabled alone.
> >> After applying this patch set, the planes are always disabled
> >> together with the CRTC.  And, yes, DC/DI are disabled first,
> >> then the timeout warning happens.
> >>
> >> Please note the warning happens when the planes are disabled
> >> instead of reenabled.  So, I don't get your point by resetting
> >> FIFO when _reenabling_  DMFC.
> >
> > If we disable the DMFC with data still in the FIFO and then reenable it
> > and the DC first reads the stale data from the FIFO, that should cause a
> > visible artifact in the first frame after reenabling the plane. If that
> > doesn't happen, I trust that the hardware resets the FIFO state
> > somewhere along the way.
> 
> Not sure if the hardware resets the FIFO automatically...
> The NXP driver waits for some hardware status/irqs when disabling
> the channels, but it doesn't wait for DMFC status.
> 
> >
> >> And, I don't see the way to reset FIFO.
> >
> > We could reset the DMFC_WR memory after disabling the DMFC, but I'm not
> > sure this is necessary at all.
> 
> You mean the register DMFC_WR_CHAN, for instance?
> I still don't see how we could reset the FIFO.

I mean via IPU_MEM_RST. I'll send a patch to illustrate, but as I said,
I don't know if this is really useful.

regards
Philipp

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

Re: [PATCH v4 0/7] drm/imx: Add active plane reconfiguration support

[Philipp Zabel]

Am Freitag, den 26.08.2016, 15:30 +0800 schrieb Liu Ying:
> This patch adds active plane reconfiguration support for imx-drm.
> This may fixes some mode setting failure issues which were introduced
> by imx-drm atomic conversion patch set.  The main idea is to disable the
> plane in question in CRTC's atomic_disable operation and then the drm
> atomic core will enable it again automatically.

I have rebased onto drm-misc and picked up the remaining patches (4-7)

regards
Philipp

> v3->v4:
> * Change the bool active_only parameter of commit_planes() to an uint32_t
>   parameter named 'flags' and add two flags - DRM_PLANE_COMMIT_ACTIVE_ONLY
>   and DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET.  This way, the drm atomic
>   core is able to skip the atomic_disable call for the planes which are
>   committed with the NO_DISABLE_AFTER_MODESET flag set.
> * Fix the helper disable_planes_on_crtc(), which is needed for CRTC's
>   atomic_disable callback to disable planes.
> * Improve kernel-doc of CRTC's atomic_disable callback to address Daniel
>   Vetter's comment.
> * Do not wait for DMFC FIFO to clear to avoid timeout warning, as the
>   precedure to disable display channel is changed slightly after the
>   NO_DISABLE_AFTER_MODESET flag is used.
> 
> v2->v3:
> * Disable all appropriate affected planes(when necessary) in CRTC's
>   ->atomic_disable callback, but not in each plane's ->atomic_update callback,
>   as suggested by Daniel Vetter.
> * +Cc Lucas Stach, as he tested the patch v2.
> 
> v1->v2:
> * Do not reject reconfiguring an active overlay plane.
> 
> Liu Ying (7):
>   drm/atomic-helper: Add atomic_disable CRTC helper callback
>   drm/atomic-helper: Disable appropriate planes in
>     disable_planes_on_crtc()
>   drm/atomic-helper: Add NO_DISABLE_AFTER_MODESET flag support for plane
>     commit
>   gpu: ipu-v3: Do not wait for DMFC FIFO to clear when disabling DMFC
>     channel
>   drm/imx: ipuv3-crtc: Use the callback ->atomic_disable instead of
>     ->disable
>   drm/imx: Use DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET flag
>   drm/imx: Add active plane reconfiguration support
> 
>  drivers/gpu/drm/arm/malidp_drv.c             |  3 +-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  2 +-
>  drivers/gpu/drm/drm_atomic_helper.c          | 64 +++++++++++++++++++---------
>  drivers/gpu/drm/exynos/exynos_drm_drv.c      |  2 +-
>  drivers/gpu/drm/imx/imx-drm-core.c           | 30 ++++++++++++-
>  drivers/gpu/drm/imx/ipuv3-crtc.c             |  8 +++-
>  drivers/gpu/drm/imx/ipuv3-plane.c            | 21 ++++++---
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c       |  6 ++-
>  drivers/gpu/drm/msm/msm_atomic.c             |  2 +-
>  drivers/gpu/drm/omapdrm/omap_drv.c           |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c        |  3 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c   |  3 +-
>  drivers/gpu/drm/sti/sti_drv.c                |  2 +-
>  drivers/gpu/drm/tegra/drm.c                  |  3 +-
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c          |  2 +-
>  drivers/gpu/drm/vc4/vc4_kms.c                |  2 +-
>  drivers/gpu/drm/virtio/virtgpu_display.c     |  3 +-
>  drivers/gpu/ipu-v3/ipu-dmfc.c                | 18 +-------
>  include/drm/drm_atomic_helper.h              | 11 +++--
>  include/drm/drm_modeset_helper_vtables.h     | 24 +++++++++++
>  20 files changed, 146 insertions(+), 65 deletions(-)
> 


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

Re: [PATCH v4 0/7] drm/imx: Add active plane reconfiguration support

[Philipp Zabel]

Am Montag, den 29.08.2016, 12:53 +0200 schrieb Philipp Zabel:
> Am Freitag, den 26.08.2016, 15:30 +0800 schrieb Liu Ying:
> > This patch adds active plane reconfiguration support for imx-drm.
> > This may fixes some mode setting failure issues which were introduced
> > by imx-drm atomic conversion patch set.  The main idea is to disable the
> > plane in question in CRTC's atomic_disable operation and then the drm
> > atomic core will enable it again automatically.
> 
> I have rebased onto drm-misc and picked up the remaining patches (4-7)

Actually, since this is a regression and the new drm-misc patches won't
make it into 4.8, I'd be inclined to take the v2 patch as a fix for 4.8
and then apply the remaining patches as relative changes to that on top
of drm-misc instead.

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

Re: [PATCH v4 0/7] drm/imx: Add active plane reconfiguration support

[Daniel Vetter]

On Mon, Aug 29, 2016 at 4:59 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Am Montag, den 29.08.2016, 12:53 +0200 schrieb Philipp Zabel:
>> Am Freitag, den 26.08.2016, 15:30 +0800 schrieb Liu Ying:
>> > This patch adds active plane reconfiguration support for imx-drm.
>> > This may fixes some mode setting failure issues which were introduced
>> > by imx-drm atomic conversion patch set.  The main idea is to disable the
>> > plane in question in CRTC's atomic_disable operation and then the drm
>> > atomic core will enable it again automatically.
>>
>> I have rebased onto drm-misc and picked up the remaining patches (4-7)
>
> Actually, since this is a regression and the new drm-misc patches won't
> make it into 4.8, I'd be inclined to take the v2 patch as a fix for 4.8
> and then apply the remaining patches as relative changes to that on top
> of drm-misc instead.

Just apply the 4.8 to 4.8 and then once that's in an -rc do a
backmerge with -X ours and explain in the commit what you're doing. No
need to rework the patches once more to be incremental on top of the
4.8 fix.

At least that's what we're doing all the time in i915.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 0/7] drm/imx: Add active plane reconfiguration support

[Philipp Zabel]

Hi Dave,

Am Montag, den 29.08.2016, 17:44 +0200 schrieb Daniel Vetter:
> On Mon, Aug 29, 2016 at 4:59 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > Am Montag, den 29.08.2016, 12:53 +0200 schrieb Philipp Zabel:
> >> Am Freitag, den 26.08.2016, 15:30 +0800 schrieb Liu Ying:
> >> > This patch adds active plane reconfiguration support for imx-drm.
> >> > This may fixes some mode setting failure issues which were introduced
> >> > by imx-drm atomic conversion patch set.  The main idea is to disable the
> >> > plane in question in CRTC's atomic_disable operation and then the drm
> >> > atomic core will enable it again automatically.
> >>
> >> I have rebased onto drm-misc and picked up the remaining patches (4-7)
> >
> > Actually, since this is a regression and the new drm-misc patches won't
> > make it into 4.8, I'd be inclined to take the v2 patch as a fix for 4.8
> > and then apply the remaining patches as relative changes to that on top
> > of drm-misc instead.
> 
> Just apply the 4.8 to 4.8 and then once that's in an -rc do a
> backmerge with -X ours and explain in the commit what you're doing. No
> need to rework the patches once more to be incremental on top of the
> 4.8 fix.
> 
> At least that's what we're doing all the time in i915.
> -Daniel

Could you merge v4.8-rc3 into drm-next? After backmerging
drm-intel-fixes-2016-08-15 with -s ours, v4.8-rc3 just auto-merges
cleanly, but I assume I shouldn't do that inside an imx-drm pull
request.

Since imx-drm-fixes-2016-08-29 is based on v4.8-rc3, I'd otherwise have
to pull it in with that backmerge, to resolve the conflict between
commit c6c1f9bc798b ("drm/imx: Add active plane reconfiguration
support") that went into v4.8-rc5 and the newer version in imx-drm/next.

regards
Philipp

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

Re: [PATCH v4 0/7] drm/imx: Add active plane reconfiguration support

[Daniel Vetter]

On Wed, Sep 14, 2016 at 1:05 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Hi Dave,
>
> Am Montag, den 29.08.2016, 17:44 +0200 schrieb Daniel Vetter:
>> On Mon, Aug 29, 2016 at 4:59 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
>> > Am Montag, den 29.08.2016, 12:53 +0200 schrieb Philipp Zabel:
>> >> Am Freitag, den 26.08.2016, 15:30 +0800 schrieb Liu Ying:
>> >> > This patch adds active plane reconfiguration support for imx-drm.
>> >> > This may fixes some mode setting failure issues which were introduced
>> >> > by imx-drm atomic conversion patch set.  The main idea is to disable the
>> >> > plane in question in CRTC's atomic_disable operation and then the drm
>> >> > atomic core will enable it again automatically.
>> >>
>> >> I have rebased onto drm-misc and picked up the remaining patches (4-7)
>> >
>> > Actually, since this is a regression and the new drm-misc patches won't
>> > make it into 4.8, I'd be inclined to take the v2 patch as a fix for 4.8
>> > and then apply the remaining patches as relative changes to that on top
>> > of drm-misc instead.
>>
>> Just apply the 4.8 to 4.8 and then once that's in an -rc do a
>> backmerge with -X ours and explain in the commit what you're doing. No
>> need to rework the patches once more to be incremental on top of the
>> 4.8 fix.
>>
>> At least that's what we're doing all the time in i915.
>> -Daniel
>
> Could you merge v4.8-rc3 into drm-next? After backmerging
> drm-intel-fixes-2016-08-15 with -s ours, v4.8-rc3 just auto-merges
> cleanly, but I assume I shouldn't do that inside an imx-drm pull
> request.
>
> Since imx-drm-fixes-2016-08-29 is based on v4.8-rc3, I'd otherwise have
> to pull it in with that backmerge, to resolve the conflict between
> commit c6c1f9bc798b ("drm/imx: Add active plane reconfiguration
> support") that went into v4.8-rc5 and the newer version in imx-drm/next.

Yup, don't do backmerges in your driver tree which affect other
drivers. Those need to happen in Dave's main drm-next tree to avoid
confusion and chaos. I'll ping him about it.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel