[PATCH v2 00/12] drm/atomic: Make drm_atomic_helper_swap_state waiting interruptible.

[PATCH v2 00/12] drm/atomic: Make drm_atomic_helper_swap_state waiting interruptible.

[Maarten Lankhorst]

drm_atomic_helper_swap_state could previously never fail, in order to still
continue it would set a limit of 10s on waiting for previous updates to complete,
then it moved forward. This can be very bad when ignoring previous updates,
because the hw state and sw state may get out of sync when for example a modeset
is ignored.

By converting the swap_state to interruptible and handling the error in each driver,
we fix this issue because if a update takes forever it can be aborted through signals.

Changes since first version:
- Split out driver conversions.
- Fix nouveau error handling first.

Maarten Lankhorst (12):
  drm/nouveau: Fix error handling in nv50_disp_atomic_commit
  drm/atomic: Change drm_atomic_helper_swap_state to return an error.
  drm/nouveau: Handle drm_atomic_helper_swap_state failure
  drm/atmel-hlcdc: Handle drm_atomic_helper_swap_state failure
  drm/i915: Handle drm_atomic_helper_swap_state failure
  drm/mediatek: Handle drm_atomic_helper_swap_state failure
  drm/msm: Handle drm_atomic_helper_swap_state failure
  drm/tegra: Handle drm_atomic_helper_swap_state failure
  drm/tilcdc: Handle drm_atomic_helper_swap_state failure
  drm/vc4: Handle drm_atomic_helper_swap_state failure
  drm/atomic: Add __must_check to drm_atomic_helper_swap_state.
  drm/atomic: Allow drm_atomic_helper_swap_state to fail

 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 13 ++++++-----
 drivers/gpu/drm/drm_atomic_helper.c          | 35 +++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_display.c         | 10 +++++++-
 drivers/gpu/drm/mediatek/mtk_drm_drv.c       |  7 +++++-
 drivers/gpu/drm/msm/msm_atomic.c             | 16 ++++++-------
 drivers/gpu/drm/nouveau/nv50_display.c       | 12 +++++++---
 drivers/gpu/drm/tegra/drm.c                  |  7 +++++-
 drivers/gpu/drm/tilcdc/tilcdc_drv.c          |  6 ++++-
 drivers/gpu/drm/vc4/vc4_kms.c                |  2 +-
 include/drm/drm_atomic_helper.h              |  4 ++--
 10 files changed, 75 insertions(+), 37 deletions(-)

-- 
2.11.0

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

[PATCH v2 01/12] drm/nouveau: Fix error handling in nv50_disp_atomic_commit

[Maarten Lankhorst]

Make it more clear that post commit return ret is really return 0,

and add a missing drm_atomic_helper_cleanup_planes when
drm_atomic_helper_wait_for_fences fails.

Fixes: 839ca903f12e ("drm/nouveau/kms/nv50: transition to atomic interfaces internally")
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: dri-devel@lists.freedesktop.org
Cc: nouveau@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v4.10+
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/nouveau/nv50_display.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index 42a85c14aea0..8fb55b96c40f 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -4119,7 +4119,7 @@ nv50_disp_atomic_commit(struct drm_device *dev,
 	if (!nonblock) {
 		ret = drm_atomic_helper_wait_for_fences(dev, state, true);
 		if (ret)
-			goto done;
+			goto err_cleanup;
 	}
 
 	for_each_plane_in_state(state, plane, plane_state, i) {
@@ -4147,7 +4147,7 @@ nv50_disp_atomic_commit(struct drm_device *dev,
 		if (crtc->state->enable) {
 			if (!drm->have_disp_power_ref) {
 				drm->have_disp_power_ref = true;
-				return ret;
+				return 0;
 			}
 			active = true;
 			break;
@@ -4158,7 +4158,10 @@ nv50_disp_atomic_commit(struct drm_device *dev,
 		pm_runtime_put_autosuspend(dev->dev);
 		drm->have_disp_power_ref = false;
 	}
+	goto done;
 
+err_cleanup:
+	drm_atomic_helper_cleanup_planes(dev, state);
 done:
 	pm_runtime_put_autosuspend(dev->dev);
 	return ret;
-- 
2.11.0

[PATCH v2 02/12] drm/atomic: Change drm_atomic_helper_swap_state to return an error.

We want to change swap_state to wait indefinitely, but to do this
swap_state should wait interruptibly. This requires propagating
the error to each driver.

Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 22 ++++++++++++++++------
 include/drm/drm_atomic_helper.h     |  3 +--
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 667ec97d4efb..bfb98fbd0e0e 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1510,10 +1510,8 @@ int drm_atomic_helper_commit(struct drm_device *dev,
 
 	if (!nonblock) {
 		ret = drm_atomic_helper_wait_for_fences(dev, state, true);
-		if (ret) {
-			drm_atomic_helper_cleanup_planes(dev, state);
-			return ret;
-		}
+		if (ret)
+			goto err;
 	}
 
 	/*
@@ -1522,7 +1520,9 @@ int drm_atomic_helper_commit(struct drm_device *dev,
 	 * the software side now.
 	 */
 
-	drm_atomic_helper_swap_state(state, true);
+	ret = drm_atomic_helper_swap_state(state, true);
+	if (ret)
+		goto err;
 
 	/*
 	 * Everything below can be run asynchronously without the need to grab
@@ -1551,6 +1551,10 @@ int drm_atomic_helper_commit(struct drm_device *dev,
 		commit_tail(state);
 
 	return 0;
+
+err:
+	drm_atomic_helper_cleanup_planes(dev, state);
+	return ret;
 }
 EXPORT_SYMBOL(drm_atomic_helper_commit);
 
@@ -2254,8 +2258,12 @@ EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
  * the &drm_plane.state, &drm_crtc.state or &drm_connector.state pointer. With
  * the current atomic helpers this is almost always the case, since the helpers
  * don't pass the right state structures to the callbacks.
+ *
+ * Returns:
+ *
+ * Always returns 0, cannot fail yet.
  */
-void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
+int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 				  bool stall)
 {
 	int i;
@@ -2332,6 +2340,8 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 
 	__for_each_private_obj(state, obj, obj_state, i, funcs)
 		funcs->swap_state(obj, &state->private_objs[i].obj_state);
+
+	return 0;
 }
 EXPORT_SYMBOL(drm_atomic_helper_swap_state);
 
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index dd196cc0afd7..37c9534ff691 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -84,8 +84,7 @@ 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);
+int drm_atomic_helper_swap_state(struct drm_atomic_state *state, bool stall);
 
 /* nonblocking commit helpers */
 int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
-- 
2.11.0

[PATCH v2 02/12] drm/atomic: Change drm_atomic_helper_swap_state to return an error.

[Maarten Lankhorst]

We want to change swap_state to wait indefinitely, but to do this
swap_state should wait interruptibly. This requires propagating
the error to each driver.

Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 22 ++++++++++++++++------
 include/drm/drm_atomic_helper.h     |  3 +--
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 667ec97d4efb..bfb98fbd0e0e 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1510,10 +1510,8 @@ int drm_atomic_helper_commit(struct drm_device *dev,
 
 	if (!nonblock) {
 		ret = drm_atomic_helper_wait_for_fences(dev, state, true);
-		if (ret) {
-			drm_atomic_helper_cleanup_planes(dev, state);
-			return ret;
-		}
+		if (ret)
+			goto err;
 	}
 
 	/*
@@ -1522,7 +1520,9 @@ int drm_atomic_helper_commit(struct drm_device *dev,
 	 * the software side now.
 	 */
 
-	drm_atomic_helper_swap_state(state, true);
+	ret = drm_atomic_helper_swap_state(state, true);
+	if (ret)
+		goto err;
 
 	/*
 	 * Everything below can be run asynchronously without the need to grab
@@ -1551,6 +1551,10 @@ int drm_atomic_helper_commit(struct drm_device *dev,
 		commit_tail(state);
 
 	return 0;
+
+err:
+	drm_atomic_helper_cleanup_planes(dev, state);
+	return ret;
 }
 EXPORT_SYMBOL(drm_atomic_helper_commit);
 
@@ -2254,8 +2258,12 @@ EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
  * the &drm_plane.state, &drm_crtc.state or &drm_connector.state pointer. With
  * the current atomic helpers this is almost always the case, since the helpers
  * don't pass the right state structures to the callbacks.
+ *
+ * Returns:
+ *
+ * Always returns 0, cannot fail yet.
  */
-void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
+int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 				  bool stall)
 {
 	int i;
@@ -2332,6 +2340,8 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 
 	__for_each_private_obj(state, obj, obj_state, i, funcs)
 		funcs->swap_state(obj, &state->private_objs[i].obj_state);
+
+	return 0;
 }
 EXPORT_SYMBOL(drm_atomic_helper_swap_state);
 
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index dd196cc0afd7..37c9534ff691 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -84,8 +84,7 @@ 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);
+int drm_atomic_helper_swap_state(struct drm_atomic_state *state, bool stall);
 
 /* nonblocking commit helpers */
 int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 03/12] drm/nouveau: Handle drm_atomic_helper_swap_state failure

[Maarten Lankhorst]

drm_atomic_helper_swap_state() will be changed to interruptible waiting
in the next few commits, so all drivers have to be changed to handling
failure.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: nouveau@lists.freedesktop.org
---
 drivers/gpu/drm/nouveau/nv50_display.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index 8fb55b96c40f..3bfea54ffd7c 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -4122,6 +4122,10 @@ nv50_disp_atomic_commit(struct drm_device *dev,
 			goto err_cleanup;
 	}
 
+	ret = drm_atomic_helper_swap_state(state, true);
+	if (ret)
+		goto err_cleanup;
+
 	for_each_plane_in_state(state, plane, plane_state, i) {
 		struct nv50_wndw_atom *asyw = nv50_wndw_atom(plane_state);
 		struct nv50_wndw *wndw = nv50_wndw(plane);
@@ -4135,7 +4139,6 @@ nv50_disp_atomic_commit(struct drm_device *dev,
 		}
 	}
 
-	drm_atomic_helper_swap_state(state, true);
 	drm_atomic_state_get(state);
 
 	if (nonblock)
-- 
2.11.0

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH v2 04/12] drm/atmel-hlcdc: Handle drm_atomic_helper_swap_state failure

[Maarten Lankhorst]

drm_atomic_helper_swap_state() will be changed to interruptible waiting
in the next few commits, so all drivers have to be changed to handling
failure.

Atmel tracks pending commits through dc->commit.pending, so it can
ignore the changes by setting stall = false. We never return failure in
this case, so make failure a BUG_ON.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 516d9547d331..64f54dc7dd68 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -539,14 +539,13 @@ static int atmel_hlcdc_dc_atomic_commit(struct drm_device *dev,
 		dc->commit.pending = true;
 	spin_unlock(&dc->commit.wait.lock);
 
-	if (ret) {
-		kfree(commit);
-		goto error;
-	}
+	if (ret)
+		goto err_free;
 
-	/* Swap the state, this is the point of no return. */
-	drm_atomic_helper_swap_state(state, true);
+	/* We have our own synchronization through the commit lock. */
+	BUG_ON(drm_atomic_helper_swap_state(state, false) < 0);
 
+	/* Swap state succeeded, this is the point of no return. */
 	drm_atomic_state_get(state);
 	if (async)
 		queue_work(dc->wq, &commit->work);
@@ -555,6 +554,8 @@ static int atmel_hlcdc_dc_atomic_commit(struct drm_device *dev,
 
 	return 0;
 
+err_free:
+	kfree(commit);
 error:
 	drm_atomic_helper_cleanup_planes(dev, state);
 	return ret;
-- 
2.11.0

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

[PATCH v2 05/12] drm/i915: Handle drm_atomic_helper_swap_state failure

[Maarten Lankhorst]

drm_atomic_helper_swap_state() will be changed to interruptible waiting
in the next few commits, so all drivers have to be changed to handling
failure.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4e03ca6c946f..da5d49407594 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13271,7 +13271,15 @@ static int intel_atomic_commit(struct drm_device *dev,
 	if (INTEL_GEN(dev_priv) < 9)
 		state->legacy_cursor_update = false;
 
-	drm_atomic_helper_swap_state(state, true);
+	ret = drm_atomic_helper_swap_state(state, true);
+	if (ret) {
+		i915_sw_fence_commit(&intel_state->commit_ready);
+
+		mutex_lock(&dev->struct_mutex);
+		drm_atomic_helper_cleanup_planes(dev, state);
+		mutex_unlock(&dev->struct_mutex);
+		return ret;
+	}
 	dev_priv->wm.distrust_bios_wm = false;
 	intel_shared_dpll_swap_state(state);
 	intel_atomic_track_fbs(state);
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 06/12] drm/mediatek: Handle drm_atomic_helper_swap_state failure

[Maarten Lankhorst]

drm_atomic_helper_swap_state() will be changed to interruptible waiting
in the next few commits, so all drivers have to be changed to handling
failure.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: CK Hu <ck.hu@mediatek.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-mediatek@lists.infradead.org
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 56f802d0a51c..9a130c84c861 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -109,7 +109,12 @@ static int mtk_atomic_commit(struct drm_device *drm,
 	mutex_lock(&private->commit.lock);
 	flush_work(&private->commit.work);
 
-	drm_atomic_helper_swap_state(state, true);
+	ret = drm_atomic_helper_swap_state(state, true);
+	if (ret) {
+		mutex_unlock(&private->commit.lock);
+		drm_atomic_helper_cleanup_planes(drm, state);
+		return ret;
+	}
 
 	drm_atomic_state_get(state);
 	if (async)
-- 
2.11.0

[PATCH v2 06/12] drm/mediatek: Handle drm_atomic_helper_swap_state failure

[Maarten Lankhorst]

drm_atomic_helper_swap_state() will be changed to interruptible waiting
in the next few commits, so all drivers have to be changed to handling
failure.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: CK Hu <ck.hu@mediatek.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-mediatek at lists.infradead.org
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 56f802d0a51c..9a130c84c861 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -109,7 +109,12 @@ static int mtk_atomic_commit(struct drm_device *drm,
 	mutex_lock(&private->commit.lock);
 	flush_work(&private->commit.work);
 
-	drm_atomic_helper_swap_state(state, true);
+	ret = drm_atomic_helper_swap_state(state, true);
+	if (ret) {
+		mutex_unlock(&private->commit.lock);
+		drm_atomic_helper_cleanup_planes(drm, state);
+		return ret;
+	}
 
 	drm_atomic_state_get(state);
 	if (async)
-- 
2.11.0

[PATCH v2 07/12] drm/msm: Handle drm_atomic_helper_swap_state failure

[Maarten Lankhorst]

drm_atomic_helper_swap_state() will be changed to interruptible waiting
in the next few commits, so all drivers have to be changed to handling
failure.

MSM has its own busy tracking, which means the swap_state call can be
done with stall = false, in which case it should never return an error.
Handle failure with BUG_ON for this reason.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
---
 drivers/gpu/drm/msm/msm_atomic.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 9633a68b14d7..badfa8717317 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -232,20 +232,18 @@ int msm_atomic_commit(struct drm_device *dev,
 	 * mark our set of crtc's as busy:
 	 */
 	ret = start_atomic(dev->dev_private, c->crtc_mask);
-	if (ret) {
-		kfree(c);
-		goto error;
-	}
+	if (ret)
+		goto err_free;
+
+	BUG_ON(drm_atomic_helper_swap_state(state, false) < 0);
 
 	/*
 	 * This is the point of no return - everything below never fails except
 	 * when the hw goes bonghits. Which means we can commit the new state on
 	 * the software side now.
+	 *
+	 * swap driver private state while still holding state_lock
 	 */
-
-	drm_atomic_helper_swap_state(state, true);
-
-	/* swap driver private state while still holding state_lock */
 	if (to_kms_state(state)->state)
 		priv->kms->funcs->swap_state(priv->kms, state);
 
@@ -275,6 +273,8 @@ int msm_atomic_commit(struct drm_device *dev,
 
 	return 0;
 
+err_free:
+	kfree(c);
 error:
 	drm_atomic_helper_cleanup_planes(dev, state);
 	return ret;
-- 
2.11.0

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 08/12] drm/tegra: Handle drm_atomic_helper_swap_state failure

[Maarten Lankhorst]

drm_atomic_helper_swap_state() will be changed to interruptible waiting
in the next few commits, so all drivers have to be changed to handling
failure.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Jonathan Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 drivers/gpu/drm/tegra/drm.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index ad3d124a972d..3ba659a5940d 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -100,7 +100,12 @@ static int tegra_atomic_commit(struct drm_device *drm,
 	 * the software side now.
 	 */
 
-	drm_atomic_helper_swap_state(state, true);
+	err = drm_atomic_helper_swap_state(state, true);
+	if (err) {
+		mutex_unlock(&tegra->commit.lock);
+		drm_atomic_helper_cleanup_planes(drm, state);
+		return err;
+	}
 
 	drm_atomic_state_get(state);
 	if (nonblock)
-- 
2.11.0

[PATCH v2 09/12] drm/tilcdc: Handle drm_atomic_helper_swap_state failure

[Maarten Lankhorst]

drm_atomic_helper_swap_state() will be changed to interruptible waiting
in the next few commits, so all drivers have to be changed to handling
failure.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Jyri Sarha <jsarha@ti.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index d67e18983a7d..049d2f5a1ee4 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -108,7 +108,11 @@ static int tilcdc_commit(struct drm_device *dev,
 	if (ret)
 		return ret;
 
-	drm_atomic_helper_swap_state(state, true);
+	ret = drm_atomic_helper_swap_state(state, true);
+	if (ret) {
+		drm_atomic_helper_cleanup_planes(dev, state);
+		return ret;
+	}
 
 	/*
 	 * Everything below can be run asynchronously without the need to grab
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 10/12] drm/vc4: Handle drm_atomic_helper_swap_state failure

[Maarten Lankhorst]

drm_atomic_helper_swap_state() will be changed to interruptible waiting
in the next few commits, so all drivers have to be changed to handling
failure.

VC4 has its own nonblocking modeset tracking through the vc4->async_modeset
semaphore, so it doesn't need to stall in swap_state. Pass stall = false
and BUG_ON when it returns an error. This should never happen for !stall.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_kms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 27edae427025..aeec6e8703d2 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -126,7 +126,7 @@ static int vc4_atomic_commit(struct drm_device *dev,
 	 * the software side now.
 	 */
 
-	drm_atomic_helper_swap_state(state, true);
+	BUG_ON(drm_atomic_helper_swap_state(state, false) < 0);
 
 	/*
 	 * Everything below can be run asynchronously without the need to grab
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 11/12] drm/atomic: Add __must_check to drm_atomic_helper_swap_state.

[Maarten Lankhorst]

Now that all drivers check the return value, convert swap_state to
__must_check. This is done separately to force build warnings if we
missed a driver.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 include/drm/drm_atomic_helper.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 37c9534ff691..8871741fa723 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -84,7 +84,8 @@ void
 drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc_state *old_crtc_state,
 					 bool atomic);
 
-int drm_atomic_helper_swap_state(struct drm_atomic_state *state, bool stall);
+int __must_check drm_atomic_helper_swap_state(struct drm_atomic_state *state,
+					      bool stall);
 
 /* nonblocking commit helpers */
 int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 12/12] drm/atomic: Allow drm_atomic_helper_swap_state to fail

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index bfb98fbd0e0e..05a22aeffbb0 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2261,13 +2261,13 @@ EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
  *
  * Returns:
  *
- * Always returns 0, cannot fail yet.
+ * Returns 0 on success. Can return -ERESTARTSYS when @stall is true and the
+ * waiting for the previous commits has been interrupted.
  */
 int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 				  bool stall)
 {
-	int i;
-	long ret;
+	int i, ret;
 	struct drm_connector *connector;
 	struct drm_connector_state *old_conn_state, *new_conn_state;
 	struct drm_crtc *crtc;
@@ -2290,12 +2290,11 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 			if (!commit)
 				continue;
 
-			ret = wait_for_completion_timeout(&commit->hw_done,
-							  10*HZ);
-			if (ret == 0)
-				DRM_ERROR("[CRTC:%d:%s] hw_done timed out\n",
-					  crtc->base.id, crtc->name);
+			ret = wait_for_completion_interruptible(&commit->hw_done);
 			drm_crtc_commit_put(commit);
+
+			if (ret)
+				return ret;
 		}
 	}
 
-- 
2.11.0

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

✗ Fi.CI.BAT: warning for drm/atomic: Make drm_atomic_helper_swap_state waiting interruptible.

[Patchwork]

== Series Details ==

Series: drm/atomic: Make drm_atomic_helper_swap_state waiting interruptible.
URL   : https://patchwork.freedesktop.org/series/27112/
State : warning

== Summary ==

Series 27112v1 drm/atomic: Make drm_atomic_helper_swap_state waiting interruptible.
https://patchwork.freedesktop.org/api/1.0/series/27112/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-7560u) fdo#100125
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> SKIP       (fi-skl-x1585l)

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:441s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:425s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:360s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:531s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:503s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:496s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:484s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:598s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:441s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:413s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:417s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:499s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:474s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:466s
fi-kbl-7560u     total:279  pass:268  dwarn:1   dfail:0   fail:0   skip:10  time:575s
fi-kbl-r         total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  time:585s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:565s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:455s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:581s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:465s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:475s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:435s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:471s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:537s
fi-snb-2600      total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  time:402s

85c472b33a364770b4c4af0caa8b8a828e0c0498 drm-tip: 2017y-07m-11d-14h-04m-44s UTC integration manifest
1c85249 drm/atomic: Allow drm_atomic_helper_swap_state to fail
36cbc4a drm/atomic: Add __must_check to drm_atomic_helper_swap_state.
2556605 drm/vc4: Handle drm_atomic_helper_swap_state failure
7b4b37d drm/tilcdc: Handle drm_atomic_helper_swap_state failure
91a3e3b drm/tegra: Handle drm_atomic_helper_swap_state failure
acee233 drm/msm: Handle drm_atomic_helper_swap_state failure
aea9b47 drm/mediatek: Handle drm_atomic_helper_swap_state failure
99cd129 drm/i915: Handle drm_atomic_helper_swap_state failure
a44c115 drm/atmel-hlcdc: Handle drm_atomic_helper_swap_state failure
161e33e drm/nouveau: Handle drm_atomic_helper_swap_state failure
ac069bf drm/atomic: Change drm_atomic_helper_swap_state to return an error.
4c8ed6a drm/nouveau: Fix error handling in nv50_disp_atomic_commit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5163/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 06/12] drm/mediatek: Handle drm_atomic_helper_swap_state failure

[Philipp Zabel]

On Tue, 2017-07-11 at 16:33 +0200, Maarten Lankhorst wrote:
> drm_atomic_helper_swap_state() will be changed to interruptible waiting
> in the next few commits, so all drivers have to be changed to handling
> failure.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: CK Hu <ck.hu@mediatek.com>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-mediatek@lists.infradead.org
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 56f802d0a51c..9a130c84c861 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -109,7 +109,12 @@ static int mtk_atomic_commit(struct drm_device *drm,
>  	mutex_lock(&private->commit.lock);
>  	flush_work(&private->commit.work);
>  
> -	drm_atomic_helper_swap_state(state, true);
> +	ret = drm_atomic_helper_swap_state(state, true);
> +	if (ret) {
> +		mutex_unlock(&private->commit.lock);
> +		drm_atomic_helper_cleanup_planes(drm, state);
> +		return ret;
> +	}
>  
>  	drm_atomic_state_get(state);
>  	if (async)

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

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

[PATCH v2 06/12] drm/mediatek: Handle drm_atomic_helper_swap_state failure

[Philipp Zabel]

On Tue, 2017-07-11 at 16:33 +0200, Maarten Lankhorst wrote:
> drm_atomic_helper_swap_state() will be changed to interruptible waiting
> in the next few commits, so all drivers have to be changed to handling
> failure.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: CK Hu <ck.hu@mediatek.com>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-mediatek at lists.infradead.org
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 56f802d0a51c..9a130c84c861 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -109,7 +109,12 @@ static int mtk_atomic_commit(struct drm_device *drm,
>  	mutex_lock(&private->commit.lock);
>  	flush_work(&private->commit.work);
>  
> -	drm_atomic_helper_swap_state(state, true);
> +	ret = drm_atomic_helper_swap_state(state, true);
> +	if (ret) {
> +		mutex_unlock(&private->commit.lock);
> +		drm_atomic_helper_cleanup_planes(drm, state);
> +		return ret;
> +	}
>  
>  	drm_atomic_state_get(state);
>  	if (async)

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

Re: [Intel-gfx] [PATCH v2 01/12] drm/nouveau: Fix error handling in nv50_disp_atomic_commit

[Daniel Vetter]

On Tue, Jul 11, 2017 at 04:33:03PM +0200, Maarten Lankhorst wrote:
> Make it more clear that post commit return ret is really return 0,
> 
> and add a missing drm_atomic_helper_cleanup_planes when
> drm_atomic_helper_wait_for_fences fails.
> 
> Fixes: 839ca903f12e ("drm/nouveau/kms/nv50: transition to atomic interfaces internally")
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: nouveau@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v4.10+
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/nouveau/nv50_display.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
> index 42a85c14aea0..8fb55b96c40f 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -4119,7 +4119,7 @@ nv50_disp_atomic_commit(struct drm_device *dev,
>  	if (!nonblock) {
>  		ret = drm_atomic_helper_wait_for_fences(dev, state, true);
>  		if (ret)
> -			goto done;
> +			goto err_cleanup;
>  	}
>  
>  	for_each_plane_in_state(state, plane, plane_state, i) {
> @@ -4147,7 +4147,7 @@ nv50_disp_atomic_commit(struct drm_device *dev,
>  		if (crtc->state->enable) {
>  			if (!drm->have_disp_power_ref) {
>  				drm->have_disp_power_ref = true;
> -				return ret;
> +				return 0;
>  			}
>  			active = true;
>  			break;
> @@ -4158,7 +4158,10 @@ nv50_disp_atomic_commit(struct drm_device *dev,
>  		pm_runtime_put_autosuspend(dev->dev);
>  		drm->have_disp_power_ref = false;
>  	}
> +	goto done;
>  

A bit convoluted, I prefer to wrap these in if (ret) to make them only run
for the failure case.

Either way: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> +err_cleanup:
> +	drm_atomic_helper_cleanup_planes(dev, state);
>  done:
>  	pm_runtime_put_autosuspend(dev->dev);
>  	return ret;
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [Intel-gfx] [PATCH v2 02/12] drm/atomic: Change drm_atomic_helper_swap_state to return an error.

[Daniel Vetter]

On Tue, Jul 11, 2017 at 04:33:04PM +0200, Maarten Lankhorst wrote:
> We want to change swap_state to wait indefinitely, but to do this
> swap_state should wait interruptibly. This requires propagating
> the error to each driver.
> 
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 22 ++++++++++++++++------
>  include/drm/drm_atomic_helper.h     |  3 +--
>  2 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 667ec97d4efb..bfb98fbd0e0e 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1510,10 +1510,8 @@ int drm_atomic_helper_commit(struct drm_device *dev,
>  
>  	if (!nonblock) {
>  		ret = drm_atomic_helper_wait_for_fences(dev, state, true);
> -		if (ret) {
> -			drm_atomic_helper_cleanup_planes(dev, state);
> -			return ret;
> -		}
> +		if (ret)
> +			goto err;
>  	}
>  
>  	/*
> @@ -1522,7 +1520,9 @@ int drm_atomic_helper_commit(struct drm_device *dev,
>  	 * the software side now.
>  	 */
>  
> -	drm_atomic_helper_swap_state(state, true);
> +	ret = drm_atomic_helper_swap_state(state, true);
> +	if (ret)
> +		goto err;
>  
>  	/*
>  	 * Everything below can be run asynchronously without the need to grab
> @@ -1551,6 +1551,10 @@ int drm_atomic_helper_commit(struct drm_device *dev,
>  		commit_tail(state);
>  
>  	return 0;
> +
> +err:
> +	drm_atomic_helper_cleanup_planes(dev, state);
> +	return ret;
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_commit);
>  
> @@ -2254,8 +2258,12 @@ EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
>   * the &drm_plane.state, &drm_crtc.state or &drm_connector.state pointer. With
>   * the current atomic helpers this is almost always the case, since the helpers
>   * don't pass the right state structures to the callbacks.
> + *
> + * Returns:
> + *
> + * Always returns 0, cannot fail yet.
>   */
> -void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
> +int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
>  				  bool stall)
>  {
>  	int i;
> @@ -2332,6 +2340,8 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
>  
>  	__for_each_private_obj(state, obj, obj_state, i, funcs)
>  		funcs->swap_state(obj, &state->private_objs[i].obj_state);
> +
> +	return 0;
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_swap_state);
>  
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index dd196cc0afd7..37c9534ff691 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -84,8 +84,7 @@ 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);
> +int drm_atomic_helper_swap_state(struct drm_atomic_state *state, bool stall);
>  
>  /* nonblocking commit helpers */
>  int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [Intel-gfx] [PATCH v2 02/12] drm/atomic: Change drm_atomic_helper_swap_state to return an error.

[Sean Paul]

On Fri, Jul 14, 2017 at 04:30:30PM +0200, Daniel Vetter wrote:
> On Tue, Jul 11, 2017 at 04:33:04PM +0200, Maarten Lankhorst wrote:
> > We want to change swap_state to wait indefinitely, but to do this
> > swap_state should wait interruptibly. This requires propagating
> > the error to each driver.
> > 
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: intel-gfx@lists.freedesktop.org
> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 22 ++++++++++++++++------
> >  include/drm/drm_atomic_helper.h     |  3 +--
> >  2 files changed, 17 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index 667ec97d4efb..bfb98fbd0e0e 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -1510,10 +1510,8 @@ int drm_atomic_helper_commit(struct drm_device *dev,
> >  
> >  	if (!nonblock) {
> >  		ret = drm_atomic_helper_wait_for_fences(dev, state, true);
> > -		if (ret) {
> > -			drm_atomic_helper_cleanup_planes(dev, state);
> > -			return ret;
> > -		}
> > +		if (ret)
> > +			goto err;
> >  	}
> >  
> >  	/*
> > @@ -1522,7 +1520,9 @@ int drm_atomic_helper_commit(struct drm_device *dev,
> >  	 * the software side now.
> >  	 */
> >  
> > -	drm_atomic_helper_swap_state(state, true);
> > +	ret = drm_atomic_helper_swap_state(state, true);
> > +	if (ret)
> > +		goto err;
> >  
> >  	/*
> >  	 * Everything below can be run asynchronously without the need to grab
> > @@ -1551,6 +1551,10 @@ int drm_atomic_helper_commit(struct drm_device *dev,
> >  		commit_tail(state);
> >  
> >  	return 0;
> > +
> > +err:
> > +	drm_atomic_helper_cleanup_planes(dev, state);
> > +	return ret;
> >  }
> >  EXPORT_SYMBOL(drm_atomic_helper_commit);
> >  
> > @@ -2254,8 +2258,12 @@ EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
> >   * the &drm_plane.state, &drm_crtc.state or &drm_connector.state pointer. With
> >   * the current atomic helpers this is almost always the case, since the helpers
> >   * don't pass the right state structures to the callbacks.

While you're changing this comment... would you mind s/proceeding/preceding/ and
s/swaped/swapped/ ?

Otherwise, 

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> > + *
> > + * Returns:
> > + *
> > + * Always returns 0, cannot fail yet.
> >   */
> > -void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
> > +int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
> >  				  bool stall)
> >  {
> >  	int i;
> > @@ -2332,6 +2340,8 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
> >  
> >  	__for_each_private_obj(state, obj, obj_state, i, funcs)
> >  		funcs->swap_state(obj, &state->private_objs[i].obj_state);
> > +
> > +	return 0;
> >  }
> >  EXPORT_SYMBOL(drm_atomic_helper_swap_state);
> >  
> > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> > index dd196cc0afd7..37c9534ff691 100644
> > --- a/include/drm/drm_atomic_helper.h
> > +++ b/include/drm/drm_atomic_helper.h
> > @@ -84,8 +84,7 @@ 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);
> > +int drm_atomic_helper_swap_state(struct drm_atomic_state *state, bool stall);
> >  
> >  /* nonblocking commit helpers */
> >  int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
> > -- 
> > 2.11.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> 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

-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [PATCH v2 00/12] drm/atomic: Make drm_atomic_helper_swap_state waiting interruptible.

[Sean Paul]

On Tue, Jul 11, 2017 at 04:33:02PM +0200, Maarten Lankhorst wrote:
> drm_atomic_helper_swap_state could previously never fail, in order to still
> continue it would set a limit of 10s on waiting for previous updates to complete,
> then it moved forward. This can be very bad when ignoring previous updates,
> because the hw state and sw state may get out of sync when for example a modeset
> is ignored.
> 
> By converting the swap_state to interruptible and handling the error in each driver,
> we fix this issue because if a update takes forever it can be aborted through signals.
> 
> Changes since first version:
> - Split out driver conversions.
> - Fix nouveau error handling first.
> 
> Maarten Lankhorst (12):
>   drm/nouveau: Fix error handling in nv50_disp_atomic_commit
>   drm/atomic: Change drm_atomic_helper_swap_state to return an error.
>   drm/nouveau: Handle drm_atomic_helper_swap_state failure
>   drm/atmel-hlcdc: Handle drm_atomic_helper_swap_state failure
>   drm/i915: Handle drm_atomic_helper_swap_state failure
>   drm/mediatek: Handle drm_atomic_helper_swap_state failure
>   drm/msm: Handle drm_atomic_helper_swap_state failure
>   drm/tegra: Handle drm_atomic_helper_swap_state failure
>   drm/tilcdc: Handle drm_atomic_helper_swap_state failure
>   drm/vc4: Handle drm_atomic_helper_swap_state failure
>   drm/atomic: Add __must_check to drm_atomic_helper_swap_state.
>   drm/atomic: Allow drm_atomic_helper_swap_state to fail

Hi Maarten,
The set looks good to me. The core changes make sense and seem like a good step
forward, and the driver changes seem like they do the right thing.

Normally, I'd suggest waiting a week or so for maintainer acks to trickle in,
but since this is fixing behavior in i915, it makes sense to expedite it.
Perhaps ping individuals on IRC?

For the whole set,
Reviewed-by: Sean Paul <seanpaul@chromium.org>

Sean

> 
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 13 ++++++-----
>  drivers/gpu/drm/drm_atomic_helper.c          | 35 +++++++++++++++++-----------
>  drivers/gpu/drm/i915/intel_display.c         | 10 +++++++-
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c       |  7 +++++-
>  drivers/gpu/drm/msm/msm_atomic.c             | 16 ++++++-------
>  drivers/gpu/drm/nouveau/nv50_display.c       | 12 +++++++---
>  drivers/gpu/drm/tegra/drm.c                  |  7 +++++-
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c          |  6 ++++-
>  drivers/gpu/drm/vc4/vc4_kms.c                |  2 +-
>  include/drm/drm_atomic_helper.h              |  4 ++--
>  10 files changed, 75 insertions(+), 37 deletions(-)
> 
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 07/12] drm/msm: Handle drm_atomic_helper_swap_state failure

[Daniel Vetter]

On Tue, Jul 11, 2017 at 04:33:09PM +0200, Maarten Lankhorst wrote:
> drm_atomic_helper_swap_state() will be changed to interruptible waiting
> in the next few commits, so all drivers have to be changed to handling
> failure.
> 
> MSM has its own busy tracking, which means the swap_state call can be
> done with stall = false, in which case it should never return an error.
> Handle failure with BUG_ON for this reason.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> ---
>  drivers/gpu/drm/msm/msm_atomic.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
> index 9633a68b14d7..badfa8717317 100644
> --- a/drivers/gpu/drm/msm/msm_atomic.c
> +++ b/drivers/gpu/drm/msm/msm_atomic.c
> @@ -232,20 +232,18 @@ int msm_atomic_commit(struct drm_device *dev,
>  	 * mark our set of crtc's as busy:
>  	 */
>  	ret = start_atomic(dev->dev_private, c->crtc_mask);
> -	if (ret) {
> -		kfree(c);
> -		goto error;
> -	}
> +	if (ret)
> +		goto err_free;
> +
> +	BUG_ON(drm_atomic_helper_swap_state(state, false) < 0);

Hm, not sure we want to do this, makes switching msm to the nonblocking
helpers a bit more tricky. And the got err_free thing looks like leftovers
from an old version. But it's all correct.

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

>  
>  	/*
>  	 * This is the point of no return - everything below never fails except
>  	 * when the hw goes bonghits. Which means we can commit the new state on
>  	 * the software side now.
> +	 *
> +	 * swap driver private state while still holding state_lock
>  	 */
> -
> -	drm_atomic_helper_swap_state(state, true);
> -
> -	/* swap driver private state while still holding state_lock */
>  	if (to_kms_state(state)->state)
>  		priv->kms->funcs->swap_state(priv->kms, state);
>  
> @@ -275,6 +273,8 @@ int msm_atomic_commit(struct drm_device *dev,
>  
>  	return 0;
>  
> +err_free:
> +	kfree(c);
>  error:
>  	drm_atomic_helper_cleanup_planes(dev, state);
>  	return ret;
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [Intel-gfx] [PATCH v2 08/12] drm/tegra: Handle drm_atomic_helper_swap_state failure

[Daniel Vetter]

On Tue, Jul 11, 2017 at 04:33:10PM +0200, Maarten Lankhorst wrote:
> drm_atomic_helper_swap_state() will be changed to interruptible waiting
> in the next few commits, so all drivers have to be changed to handling
> failure.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Cc: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Jonathan Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Reviewed-by: Daniel Vetter <daniel.vetter-/w4YWyX8dFk@public.gmane.org>

> ---
>  drivers/gpu/drm/tegra/drm.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index ad3d124a972d..3ba659a5940d 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -100,7 +100,12 @@ static int tegra_atomic_commit(struct drm_device *drm,
>  	 * the software side now.
>  	 */
>  
> -	drm_atomic_helper_swap_state(state, true);
> +	err = drm_atomic_helper_swap_state(state, true);
> +	if (err) {
> +		mutex_unlock(&tegra->commit.lock);
> +		drm_atomic_helper_cleanup_planes(drm, state);
> +		return err;
> +	}
>  
>  	drm_atomic_state_get(state);
>  	if (nonblock)
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [Intel-gfx] [PATCH v2 10/12] drm/vc4: Handle drm_atomic_helper_swap_state failure

[Daniel Vetter]

On Tue, Jul 11, 2017 at 04:33:12PM +0200, Maarten Lankhorst wrote:
> drm_atomic_helper_swap_state() will be changed to interruptible waiting
> in the next few commits, so all drivers have to be changed to handling
> failure.
> 
> VC4 has its own nonblocking modeset tracking through the vc4->async_modeset
> semaphore, so it doesn't need to stall in swap_state. Pass stall = false
> and BUG_ON when it returns an error. This should never happen for !stall.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Eric Anholt <eric@anholt.net>

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

> ---
>  drivers/gpu/drm/vc4/vc4_kms.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index 27edae427025..aeec6e8703d2 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -126,7 +126,7 @@ static int vc4_atomic_commit(struct drm_device *dev,
>  	 * the software side now.
>  	 */
>  
> -	drm_atomic_helper_swap_state(state, true);
> +	BUG_ON(drm_atomic_helper_swap_state(state, false) < 0);
>  
>  	/*
>  	 * Everything below can be run asynchronously without the need to grab
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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 v2 11/12] drm/atomic: Add __must_check to drm_atomic_helper_swap_state.

[Daniel Vetter]

On Tue, Jul 11, 2017 at 04:33:13PM +0200, Maarten Lankhorst wrote:
> Now that all drivers check the return value, convert swap_state to
> __must_check. This is done separately to force build warnings if we
> missed a driver.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Maybe squash in with the next commit, seems a bit silly to split this out.

Either way: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  include/drm/drm_atomic_helper.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index 37c9534ff691..8871741fa723 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -84,7 +84,8 @@ void
>  drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc_state *old_crtc_state,
>  					 bool atomic);
>  
> -int drm_atomic_helper_swap_state(struct drm_atomic_state *state, bool stall);
> +int __must_check drm_atomic_helper_swap_state(struct drm_atomic_state *state,
> +					      bool stall);
>  
>  /* nonblocking commit helpers */
>  int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH v2 12/12] drm/atomic: Allow drm_atomic_helper_swap_state to fail

[Daniel Vetter]

On Tue, Jul 11, 2017 at 04:33:14PM +0200, Maarten Lankhorst wrote:
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index bfb98fbd0e0e..05a22aeffbb0 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2261,13 +2261,13 @@ EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
>   *
>   * Returns:
>   *
> - * Always returns 0, cannot fail yet.
> + * Returns 0 on success. Can return -ERESTARTSYS when @stall is true and the
> + * waiting for the previous commits has been interrupted.
>   */

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

>  int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
>  				  bool stall)
>  {
> -	int i;
> -	long ret;
> +	int i, ret;
>  	struct drm_connector *connector;
>  	struct drm_connector_state *old_conn_state, *new_conn_state;
>  	struct drm_crtc *crtc;
> @@ -2290,12 +2290,11 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
>  			if (!commit)
>  				continue;
>  
> -			ret = wait_for_completion_timeout(&commit->hw_done,
> -							  10*HZ);
> -			if (ret == 0)
> -				DRM_ERROR("[CRTC:%d:%s] hw_done timed out\n",
> -					  crtc->base.id, crtc->name);
> +			ret = wait_for_completion_interruptible(&commit->hw_done);
>  			drm_crtc_commit_put(commit);
> +
> +			if (ret)
> +				return ret;
>  		}
>  	}
>  
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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 v2 00/12] drm/atomic: Make drm_atomic_helper_swap_state waiting interruptible.

[Maarten Lankhorst]

Op 14-07-17 om 17:01 schreef Sean Paul:
> On Tue, Jul 11, 2017 at 04:33:02PM +0200, Maarten Lankhorst wrote:
>> drm_atomic_helper_swap_state could previously never fail, in order to still
>> continue it would set a limit of 10s on waiting for previous updates to complete,
>> then it moved forward. This can be very bad when ignoring previous updates,
>> because the hw state and sw state may get out of sync when for example a modeset
>> is ignored.
>>
>> By converting the swap_state to interruptible and handling the error in each driver,
>> we fix this issue because if a update takes forever it can be aborted through signals.
>>
>> Changes since first version:
>> - Split out driver conversions.
>> - Fix nouveau error handling first.
>>
>> Maarten Lankhorst (12):
>>   drm/nouveau: Fix error handling in nv50_disp_atomic_commit
>>   drm/atomic: Change drm_atomic_helper_swap_state to return an error.
>>   drm/nouveau: Handle drm_atomic_helper_swap_state failure
>>   drm/atmel-hlcdc: Handle drm_atomic_helper_swap_state failure
>>   drm/i915: Handle drm_atomic_helper_swap_state failure
>>   drm/mediatek: Handle drm_atomic_helper_swap_state failure
>>   drm/msm: Handle drm_atomic_helper_swap_state failure
>>   drm/tegra: Handle drm_atomic_helper_swap_state failure
>>   drm/tilcdc: Handle drm_atomic_helper_swap_state failure
>>   drm/vc4: Handle drm_atomic_helper_swap_state failure
>>   drm/atomic: Add __must_check to drm_atomic_helper_swap_state.
>>   drm/atomic: Allow drm_atomic_helper_swap_state to fail
> Hi Maarten,
> The set looks good to me. The core changes make sense and seem like a good step
> forward, and the driver changes seem like they do the right thing.
>
> Normally, I'd suggest waiting a week or so for maintainer acks to trickle in,
> but since this is fixing behavior in i915, it makes sense to expedite it.
> Perhaps ping individuals on IRC?
>
> For the whole set,
> Reviewed-by: Sean Paul <seanpaul@chromium.org>
Series applied, thanks for review. :)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 09/12] drm/tilcdc: Handle drm_atomic_helper_swap_state failure

[Jyri Sarha]

On 07/11/17 17:33, Maarten Lankhorst wrote:
> drm_atomic_helper_swap_state() will be changed to interruptible waiting
> in the next few commits, so all drivers have to be changed to handling
> failure.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>

This has probably been applied already, but here is my

Acked-by: Jyri Sarha <jsarha@ti.com>

just in case it is not.

> ---
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index d67e18983a7d..049d2f5a1ee4 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -108,7 +108,11 @@ static int tilcdc_commit(struct drm_device *dev,
>  	if (ret)
>  		return ret;
>  
> -	drm_atomic_helper_swap_state(state, true);
> +	ret = drm_atomic_helper_swap_state(state, true);
> +	if (ret) {
> +		drm_atomic_helper_cleanup_planes(dev, state);
> +		return ret;
> +	}
>  
>  	/*
>  	 * Everything below can be run asynchronously without the need to grab
> 

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

Re: [PATCH v2 09/12] drm/tilcdc: Handle drm_atomic_helper_swap_state failure

[Maarten Lankhorst]

Op 27-07-17 om 09:49 schreef Jyri Sarha:
> On 07/11/17 17:33, Maarten Lankhorst wrote:
>> drm_atomic_helper_swap_state() will be changed to interruptible waiting
>> in the next few commits, so all drivers have to be changed to handling
>> failure.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Jyri Sarha <jsarha@ti.com>
>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> This has probably been applied already, but here is my
>
> Acked-by: Jyri Sarha <jsarha@ti.com>
>
> just in case it is not.
Yeah it has been applied. :)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel