Hello everyone, any update on this issue? I can see that the old/custom wait-for-vblank code is still in place. Andrzej mentioned that this patch is quick/dirty, but isn't using DRM core functionality actually the better approach? With best wishes, Tobias Andrzej Hajda wrote: > Exynos DRM devices update their registers at vblank time. Exynos-DRM uses > custom mechanism to wait for vblank. This mechanism is error prone - > variables are not updated atomically. As a result in certain circumstances > user space can try to free buffers which are still in use by hardware, > in such cases IOMMU can throw OOPS. > The patch instead of fixing the mechanism replaces it with drm core helper. > > Signed-off-by: Andrzej Hajda <a.hajda@xxxxxxxxxxx> > --- > Hi Tobias, > > This is a quick/dirty patch just for checking if it solves your issue. > Successfully tested on decon5433/dsi/i80 path. > > Please verify if it helps in your case. > > The patch is based on exynos-drm-next and > "drm/exynos: fix cancel page flip code" [1]. > > [1]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/53801 > > Regards > Andrzej > --- > drivers/gpu/drm/exynos/exynos_drm_crtc.c | 16 +----------- > drivers/gpu/drm/exynos/exynos_drm_drv.c | 44 +------------------------------- > drivers/gpu/drm/exynos/exynos_drm_drv.h | 2 -- > 3 files changed, 2 insertions(+), 60 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > index 785ffa6..5b6845b 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > @@ -134,8 +134,6 @@ struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev, > exynos_crtc->ops = ops; > exynos_crtc->ctx = ctx; > > - init_waitqueue_head(&exynos_crtc->wait_update); > - > crtc = &exynos_crtc->base; > > private->crtc[pipe] = crtc; > @@ -175,13 +173,6 @@ void exynos_drm_crtc_disable_vblank(struct drm_device *dev, unsigned int pipe) > exynos_crtc->ops->disable_vblank(exynos_crtc); > } > > -void exynos_drm_crtc_wait_pending_update(struct exynos_drm_crtc *exynos_crtc) > -{ > - wait_event_timeout(exynos_crtc->wait_update, > - (atomic_read(&exynos_crtc->pending_update) == 0), > - msecs_to_jiffies(50)); > -} > - > void exynos_drm_crtc_finish_update(struct exynos_drm_crtc *exynos_crtc, > struct exynos_drm_plane *exynos_plane) > { > @@ -190,9 +181,6 @@ void exynos_drm_crtc_finish_update(struct exynos_drm_crtc *exynos_crtc, > > exynos_plane->pending_fb = NULL; > > - if (atomic_dec_and_test(&exynos_crtc->pending_update)) > - wake_up(&exynos_crtc->wait_update); > - > spin_lock_irqsave(&crtc->dev->event_lock, flags); > if (exynos_crtc->event) > drm_crtc_send_vblank_event(crtc, exynos_crtc->event); > @@ -235,10 +223,8 @@ void exynos_drm_crtc_cancel_page_flip(struct drm_crtc *crtc, > spin_lock_irqsave(&crtc->dev->event_lock, flags); > > e = exynos_crtc->event; > - if (e && e->base.file_priv == file) { > + if (e && e->base.file_priv == file) > exynos_crtc->event = NULL; > - atomic_dec(&exynos_crtc->pending_update); > - } > > spin_unlock_irqrestore(&crtc->dev->event_lock, flags); > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c > index 0281b30..cc96e85 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c > @@ -45,37 +45,11 @@ struct exynos_atomic_commit { > u32 crtcs; > }; > > -static void exynos_atomic_wait_for_commit(struct drm_atomic_state *state) > -{ > - struct drm_crtc_state *crtc_state; > - struct drm_crtc *crtc; > - int i, ret; > - > - for_each_crtc_in_state(state, crtc, crtc_state, i) { > - struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc); > - > - if (!crtc->state->enable) > - continue; > - > - ret = drm_crtc_vblank_get(crtc); > - if (ret) > - continue; > - > - exynos_drm_crtc_wait_pending_update(exynos_crtc); > - drm_crtc_vblank_put(crtc); > - } > -} > - > static void exynos_atomic_commit_complete(struct exynos_atomic_commit *commit) > { > struct drm_device *dev = commit->dev; > struct exynos_drm_private *priv = dev->dev_private; > struct drm_atomic_state *state = commit->state; > - struct drm_plane *plane; > - struct drm_crtc *crtc; > - struct drm_plane_state *plane_state; > - struct drm_crtc_state *crtc_state; > - int i; > > drm_atomic_helper_commit_modeset_disables(dev, state); > > @@ -89,25 +63,9 @@ static void exynos_atomic_commit_complete(struct exynos_atomic_commit *commit) > * have the relevant clocks enabled to perform the update. > */ > > - for_each_crtc_in_state(state, crtc, crtc_state, i) { > - struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc); > - > - atomic_set(&exynos_crtc->pending_update, 0); > - } > - > - for_each_plane_in_state(state, plane, plane_state, i) { > - struct exynos_drm_crtc *exynos_crtc = > - to_exynos_crtc(plane->crtc); > - > - if (!plane->crtc) > - continue; > - > - atomic_inc(&exynos_crtc->pending_update); > - } > - > drm_atomic_helper_commit_planes(dev, state, false); > > - exynos_atomic_wait_for_commit(state); > + drm_atomic_helper_wait_for_vblanks(dev, state); > > drm_atomic_helper_cleanup_planes(dev, state); > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h > index 561925f..0bb3766 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h > @@ -174,8 +174,6 @@ struct exynos_drm_crtc { > enum exynos_drm_output_type type; > unsigned int pipe; > struct drm_pending_vblank_event *event; > - wait_queue_head_t wait_update; > - atomic_t pending_update; > const struct exynos_drm_crtc_ops *ops; > void *ctx; > struct exynos_drm_clk *pipe_clk; > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
