* [PATCH 01/12] drm/nouveau: Fix error handling in nv50_disp_atomic_commit
@ 2017-07-19 7:40 ` Maarten Lankhorst
0 siblings, 0 replies; 11+ messages in thread
From: Maarten Lankhorst @ 2017-07-19 7:40 UTC (permalink / raw)
To: intel-gfx-trybot-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, # v4 . 10+,
Maarten Lankhorst, Ben Skeggs,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
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>
Link: http://patchwork.freedesktop.org/patch/msgid/20170711143314.2148-2-maarten.lankhorst@linux.intel.com
Reviewed-by: Sean Paul <seanpaul@chromium.org>
[mlankhorst: Use if (ret) to remove the goto in success case.]
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
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 5f71e304022e..c1e7307a2538 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -4120,7 +4120,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) {
@@ -4148,7 +4148,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;
@@ -4160,6 +4160,9 @@ nv50_disp_atomic_commit(struct drm_device *dev,
drm->have_disp_power_ref = false;
}
+err_cleanup:
+ if (ret)
+ drm_atomic_helper_cleanup_planes(dev, state);
done:
pm_runtime_put_autosuspend(dev->dev);
return ret;
--
2.11.0
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 01/12] drm/nouveau: Fix error handling in nv50_disp_atomic_commit @ 2017-07-19 7:40 ` Maarten Lankhorst 0 siblings, 0 replies; 11+ messages in thread From: Maarten Lankhorst @ 2017-07-19 7:40 UTC (permalink / raw) To: intel-gfx-trybot Cc: Maarten Lankhorst, Ben Skeggs, dri-devel, nouveau, # v4 . 10+ 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> Link: http://patchwork.freedesktop.org/patch/msgid/20170711143314.2148-2-maarten.lankhorst@linux.intel.com Reviewed-by: Sean Paul <seanpaul@chromium.org> [mlankhorst: Use if (ret) to remove the goto in success case.] Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- 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 5f71e304022e..c1e7307a2538 100644 --- a/drivers/gpu/drm/nouveau/nv50_display.c +++ b/drivers/gpu/drm/nouveau/nv50_display.c @@ -4120,7 +4120,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) { @@ -4148,7 +4148,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; @@ -4160,6 +4160,9 @@ nv50_disp_atomic_commit(struct drm_device *dev, drm->have_disp_power_ref = false; } +err_cleanup: + if (ret) + drm_atomic_helper_cleanup_planes(dev, state); done: pm_runtime_put_autosuspend(dev->dev); return ret; -- 2.11.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 02/12] drm/atomic: Change drm_atomic_helper_swap_state to return an error. 2017-07-19 7:40 ` Maarten Lankhorst @ 2017-07-19 7:40 ` Maarten Lankhorst -1 siblings, 0 replies; 11+ messages in thread From: Maarten Lankhorst @ 2017-07-19 7:40 UTC (permalink / raw) To: intel-gfx-trybot; +Cc: intel-gfx, linux-kernel, dri-devel 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> Link: http://patchwork.freedesktop.org/patch/msgid/20170711143314.2148-3-maarten.lankhorst@linux.intel.com Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> [mlankhorst: Fix typos in swap_state documentation (seanpaul)] Reviewed-by: Sean Paul <seanpaul@chromium.org>] --- drivers/gpu/drm/drm_atomic_helper.c | 26 ++++++++++++++++++-------- include/drm/drm_atomic_helper.h | 3 +-- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 2f675112e225..8d8221a128d0 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); @@ -2228,14 +2232,14 @@ EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes); /** * drm_atomic_helper_swap_state - store atomic state into current sw state * @state: atomic state - * @stall: stall for proceeding commits + * @stall: stall for preceeding commits * * This function stores the atomic state into the current state pointers in all * driver objects. It should be called after all failing steps have been done * and succeeded, but before the actual hardware state is committed. * * For cleanup and error recovery the current state for all changed objects will - * be swaped into @state. + * be swapped into @state. * * With that sequence it fits perfectly into the plane prepare/cleanup sequence: * @@ -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; @@ -2339,6 +2347,8 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state, state->private_objs[i].state = old_obj_state; obj->state = new_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 7db3438ff735..547480692470 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -86,8 +86,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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 02/12] drm/atomic: Change drm_atomic_helper_swap_state to return an error. @ 2017-07-19 7:40 ` Maarten Lankhorst 0 siblings, 0 replies; 11+ messages in thread From: Maarten Lankhorst @ 2017-07-19 7:40 UTC (permalink / raw) To: intel-gfx-trybot; +Cc: Maarten Lankhorst, dri-devel, linux-kernel, intel-gfx 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> Link: http://patchwork.freedesktop.org/patch/msgid/20170711143314.2148-3-maarten.lankhorst@linux.intel.com Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> [mlankhorst: Fix typos in swap_state documentation (seanpaul)] Reviewed-by: Sean Paul <seanpaul@chromium.org>] --- drivers/gpu/drm/drm_atomic_helper.c | 26 ++++++++++++++++++-------- include/drm/drm_atomic_helper.h | 3 +-- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 2f675112e225..8d8221a128d0 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); @@ -2228,14 +2232,14 @@ EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes); /** * drm_atomic_helper_swap_state - store atomic state into current sw state * @state: atomic state - * @stall: stall for proceeding commits + * @stall: stall for preceeding commits * * This function stores the atomic state into the current state pointers in all * driver objects. It should be called after all failing steps have been done * and succeeded, but before the actual hardware state is committed. * * For cleanup and error recovery the current state for all changed objects will - * be swaped into @state. + * be swapped into @state. * * With that sequence it fits perfectly into the plane prepare/cleanup sequence: * @@ -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; @@ -2339,6 +2347,8 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state, state->private_objs[i].state = old_obj_state; obj->state = new_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 7db3438ff735..547480692470 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -86,8 +86,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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
[parent not found: <20170719074052.2993-1-maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* [PATCH 03/12] drm/nouveau: Handle drm_atomic_helper_swap_state failure [not found] ` <20170719074052.2993-1-maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> @ 2017-07-19 7:40 ` Maarten Lankhorst 2017-07-19 7:40 ` [PATCH 07/12] drm/msm: " Maarten Lankhorst 2017-07-19 7:40 ` [PATCH 08/12] drm/tegra: " Maarten Lankhorst 2 siblings, 0 replies; 11+ messages in thread From: Maarten Lankhorst @ 2017-07-19 7:40 UTC (permalink / raw) To: intel-gfx-trybot-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Maarten Lankhorst, Ben Skeggs 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 Link: http://patchwork.freedesktop.org/patch/msgid/20170711143314.2148-4-maarten.lankhorst@linux.intel.com Reviewed-by: Sean Paul <seanpaul@chromium.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 c1e7307a2538..747c99c1e474 100644 --- a/drivers/gpu/drm/nouveau/nv50_display.c +++ b/drivers/gpu/drm/nouveau/nv50_display.c @@ -4123,6 +4123,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); @@ -4136,7 +4140,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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 07/12] drm/msm: Handle drm_atomic_helper_swap_state failure [not found] ` <20170719074052.2993-1-maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 2017-07-19 7:40 ` [PATCH 03/12] drm/nouveau: Handle drm_atomic_helper_swap_state failure Maarten Lankhorst @ 2017-07-19 7:40 ` Maarten Lankhorst [not found] ` <20170719074052.2993-7-maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 2017-07-19 7:40 ` [PATCH 08/12] drm/tegra: " Maarten Lankhorst 2 siblings, 1 reply; 11+ messages in thread From: Maarten Lankhorst @ 2017-07-19 7:40 UTC (permalink / raw) To: intel-gfx-trybot-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Rob Clark, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, 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 Link: http://patchwork.freedesktop.org/patch/msgid/20170711143314.2148-8-maarten.lankhorst@linux.intel.com Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Reviewed-by: Sean Paul <seanpaul@chromium.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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
[parent not found: <20170719074052.2993-7-maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: [PATCH 07/12] drm/msm: Handle drm_atomic_helper_swap_state failure [not found] ` <20170719074052.2993-7-maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> @ 2017-07-19 9:24 ` Archit Taneja [not found] ` <2d51335c-c1d1-3a01-002a-900bbe77e509-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 0 siblings, 1 reply; 11+ messages in thread From: Archit Taneja @ 2017-07-19 9:24 UTC (permalink / raw) To: Maarten Lankhorst, intel-gfx-trybot-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Rob Clark, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On 07/19/2017 01:10 PM, 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. > Applied #2/12 and tried this out. Tested-by: Archit Taneja <architt@codeaurora.org> > 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 > Link: http://patchwork.freedesktop.org/patch/msgid/20170711143314.2148-8-maarten.lankhorst@linux.intel.com > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > Reviewed-by: Sean Paul <seanpaul@chromium.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; > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <2d51335c-c1d1-3a01-002a-900bbe77e509-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* Re: [PATCH 07/12] drm/msm: Handle drm_atomic_helper_swap_state failure [not found] ` <2d51335c-c1d1-3a01-002a-900bbe77e509-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> @ 2017-07-19 10:07 ` Maarten Lankhorst 0 siblings, 0 replies; 11+ messages in thread From: Maarten Lankhorst @ 2017-07-19 10:07 UTC (permalink / raw) To: Archit Taneja, intel-gfx-trybot-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Rob Clark, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Op 19-07-17 om 11:24 schreef Archit Taneja: > > > On 07/19/2017 01:10 PM, 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. >> > > Applied #2/12 and tried this out. > > Tested-by: Archit Taneja <architt@codeaurora.org> Thanks, applied series. I accidentally sent this out to everyone instead of trybot only, but applying your t-b to the commit. :) _______________________________________________ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 08/12] drm/tegra: Handle drm_atomic_helper_swap_state failure [not found] ` <20170719074052.2993-1-maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 2017-07-19 7:40 ` [PATCH 03/12] drm/nouveau: Handle drm_atomic_helper_swap_state failure Maarten Lankhorst 2017-07-19 7:40 ` [PATCH 07/12] drm/msm: " Maarten Lankhorst @ 2017-07-19 7:40 ` Maarten Lankhorst 2 siblings, 0 replies; 11+ messages in thread From: Maarten Lankhorst @ 2017-07-19 7:40 UTC (permalink / raw) To: intel-gfx-trybot-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Cc: Maarten Lankhorst, Thierry Reding, Jonathan Hunter, linux-tegra-u79uwXL29TY76Z2rM5mHXA 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 Link: http://patchwork.freedesktop.org/patch/msgid/20170711143314.2148-9-maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA@public.gmane.org Reviewed-by: Daniel Vetter <daniel.vetter-/w4YWyX8dFk@public.gmane.org> Reviewed-by: Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 06/12] drm/mediatek: Handle drm_atomic_helper_swap_state failure 2017-07-19 7:40 ` Maarten Lankhorst @ 2017-07-19 7:40 ` Maarten Lankhorst -1 siblings, 0 replies; 11+ messages in thread From: Maarten Lankhorst @ 2017-07-19 7:40 UTC (permalink / raw) To: intel-gfx-trybot Cc: CK Hu, linux-mediatek, Maarten Lankhorst, linux-arm-kernel, Philipp Zabel 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 Link: http://patchwork.freedesktop.org/patch/msgid/20170711143314.2148-7-maarten.lankhorst@linux.intel.com Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> Reviewed-by: Sean Paul <seanpaul@chromium.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 be0741638f94..b2596f35104b 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 06/12] drm/mediatek: Handle drm_atomic_helper_swap_state failure @ 2017-07-19 7:40 ` Maarten Lankhorst 0 siblings, 0 replies; 11+ messages in thread From: Maarten Lankhorst @ 2017-07-19 7:40 UTC (permalink / raw) To: linux-arm-kernel 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 Link: http://patchwork.freedesktop.org/patch/msgid/20170711143314.2148-7-maarten.lankhorst at linux.intel.com Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> Reviewed-by: Sean Paul <seanpaul@chromium.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 be0741638f94..b2596f35104b 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-07-19 10:07 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-19 7:40 [PATCH 01/12] drm/nouveau: Fix error handling in nv50_disp_atomic_commit Maarten Lankhorst
2017-07-19 7:40 ` Maarten Lankhorst
2017-07-19 7:40 ` [PATCH 02/12] drm/atomic: Change drm_atomic_helper_swap_state to return an error Maarten Lankhorst
2017-07-19 7:40 ` Maarten Lankhorst
[not found] ` <20170719074052.2993-1-maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-07-19 7:40 ` [PATCH 03/12] drm/nouveau: Handle drm_atomic_helper_swap_state failure Maarten Lankhorst
2017-07-19 7:40 ` [PATCH 07/12] drm/msm: " Maarten Lankhorst
[not found] ` <20170719074052.2993-7-maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-07-19 9:24 ` Archit Taneja
[not found] ` <2d51335c-c1d1-3a01-002a-900bbe77e509-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2017-07-19 10:07 ` Maarten Lankhorst
2017-07-19 7:40 ` [PATCH 08/12] drm/tegra: " Maarten Lankhorst
2017-07-19 7:40 ` [PATCH 06/12] drm/mediatek: " Maarten Lankhorst
2017-07-19 7:40 ` Maarten Lankhorst
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.