* [PATCH 09/25] drm/atmel: Use drm_atomic_helper_commit [not found] <20200707201229.472834-1-daniel.vetter@ffwll.ch> @ 2020-07-07 20:12 ` Daniel Vetter 2020-07-07 20:37 ` Sam Ravnborg 2020-07-07 21:31 ` [PATCH] " Daniel Vetter 2020-07-07 20:12 ` [PATCH 10/25] drm/imx: Annotate dma-fence critical section in commit path Daniel Vetter 1 sibling, 2 replies; 5+ messages in thread From: Daniel Vetter @ 2020-07-07 20:12 UTC (permalink / raw) To: DRI Development Cc: Alexandre Belloni, Boris Brezillon, linux-rdma, Daniel Vetter, Intel Graphics Development, Ludovic Desroches, Daniel Vetter, Sam Ravnborg, linux-arm-kernel One of these drivers that predates the nonblocking support in helpers, and hand-rolled its own thing. Entirely not anything specific here, we can just delete it all and replace it with the helper version. Could also perhaps use the drm_mode_config_helper_suspend/resume stuff, for another few lines deleted. But I'm not looking at that stuff, I'm just going through all the atomic commit functions and make sure they have properly annotated dma-fence critical sections everywhere. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Boris Brezillon <bbrezillon@kernel.org> Cc: Nicolas Ferre <nicolas.ferre@microchip.com> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> Cc: Ludovic Desroches <ludovic.desroches@microchip.com> Cc: linux-arm-kernel@lists.infradead.org --- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 96 +------------------- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 4 - 2 files changed, 1 insertion(+), 99 deletions(-) diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c index 871293d1aeeb..9ec156e98f06 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c @@ -557,103 +557,10 @@ static irqreturn_t atmel_hlcdc_dc_irq_handler(int irq, void *data) return IRQ_HANDLED; } -struct atmel_hlcdc_dc_commit { - struct work_struct work; - struct drm_device *dev; - struct drm_atomic_state *state; -}; - -static void -atmel_hlcdc_dc_atomic_complete(struct atmel_hlcdc_dc_commit *commit) -{ - struct drm_device *dev = commit->dev; - struct atmel_hlcdc_dc *dc = dev->dev_private; - struct drm_atomic_state *old_state = commit->state; - - /* Apply the atomic update. */ - drm_atomic_helper_commit_modeset_disables(dev, old_state); - drm_atomic_helper_commit_planes(dev, old_state, 0); - drm_atomic_helper_commit_modeset_enables(dev, old_state); - - drm_atomic_helper_wait_for_vblanks(dev, old_state); - - drm_atomic_helper_cleanup_planes(dev, old_state); - - drm_atomic_state_put(old_state); - - /* Complete the commit, wake up any waiter. */ - spin_lock(&dc->commit.wait.lock); - dc->commit.pending = false; - wake_up_all_locked(&dc->commit.wait); - spin_unlock(&dc->commit.wait.lock); - - kfree(commit); -} - -static void atmel_hlcdc_dc_atomic_work(struct work_struct *work) -{ - struct atmel_hlcdc_dc_commit *commit = - container_of(work, struct atmel_hlcdc_dc_commit, work); - - atmel_hlcdc_dc_atomic_complete(commit); -} - -static int atmel_hlcdc_dc_atomic_commit(struct drm_device *dev, - struct drm_atomic_state *state, - bool async) -{ - struct atmel_hlcdc_dc *dc = dev->dev_private; - struct atmel_hlcdc_dc_commit *commit; - int ret; - - ret = drm_atomic_helper_prepare_planes(dev, state); - if (ret) - return ret; - - /* Allocate the commit object. */ - commit = kzalloc(sizeof(*commit), GFP_KERNEL); - if (!commit) { - ret = -ENOMEM; - goto error; - } - - INIT_WORK(&commit->work, atmel_hlcdc_dc_atomic_work); - commit->dev = dev; - commit->state = state; - - spin_lock(&dc->commit.wait.lock); - ret = wait_event_interruptible_locked(dc->commit.wait, - !dc->commit.pending); - if (ret == 0) - dc->commit.pending = true; - spin_unlock(&dc->commit.wait.lock); - - if (ret) - goto err_free; - - /* 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); - else - atmel_hlcdc_dc_atomic_complete(commit); - - return 0; - -err_free: - kfree(commit); -error: - drm_atomic_helper_cleanup_planes(dev, state); - return ret; -} - static const struct drm_mode_config_funcs mode_config_funcs = { .fb_create = drm_gem_fb_create, .atomic_check = drm_atomic_helper_check, - .atomic_commit = atmel_hlcdc_dc_atomic_commit, + .atomic_commit = drm_atomic_helper_commit, }; static int atmel_hlcdc_dc_modeset_init(struct drm_device *dev) @@ -716,7 +623,6 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev) if (!dc->wq) return -ENOMEM; - init_waitqueue_head(&dc->commit.wait); dc->desc = match->data; dc->hlcdc = dev_get_drvdata(dev->dev->parent); dev->dev_private = dc; diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h index 469d4507e576..9367a3747a3a 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h @@ -346,10 +346,6 @@ struct atmel_hlcdc_dc { u32 imr; struct drm_atomic_state *state; } suspend; - struct { - wait_queue_head_t wait; - bool pending; - } commit; }; extern struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats; -- 2.27.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 09/25] drm/atmel: Use drm_atomic_helper_commit 2020-07-07 20:12 ` [PATCH 09/25] drm/atmel: Use drm_atomic_helper_commit Daniel Vetter @ 2020-07-07 20:37 ` Sam Ravnborg 2020-07-07 21:31 ` [PATCH] " Daniel Vetter 1 sibling, 0 replies; 5+ messages in thread From: Sam Ravnborg @ 2020-07-07 20:37 UTC (permalink / raw) To: Daniel Vetter Cc: Alexandre Belloni, Boris Brezillon, linux-rdma, Intel Graphics Development, DRI Development, Ludovic Desroches, Daniel Vetter, linux-arm-kernel Hi Daniel. On Tue, Jul 07, 2020 at 10:12:13PM +0200, Daniel Vetter wrote: > One of these drivers that predates the nonblocking support in helpers, > and hand-rolled its own thing. Entirely not anything specific here, we > can just delete it all and replace it with the helper version. > > Could also perhaps use the drm_mode_config_helper_suspend/resume > stuff, for another few lines deleted. But I'm not looking at that > stuff, I'm just going through all the atomic commit functions and make > sure they have properly annotated dma-fence critical sections > everywhere. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: Boris Brezillon <bbrezillon@kernel.org> > Cc: Nicolas Ferre <nicolas.ferre@microchip.com> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> > Cc: Ludovic Desroches <ludovic.desroches@microchip.com> > Cc: linux-arm-kernel@lists.infradead.org Looks good, nice to see all this code deleted! This more or less matches what I had concluded. But.. atmel_hlcdc_dc.wq is no longer used - so can also be deleted. This will delete even more code - good. I will try to test the patch in the weekend. Will get back if I suceed doing so. Sam > --- > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 96 +------------------- > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 4 - > 2 files changed, 1 insertion(+), 99 deletions(-) > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > index 871293d1aeeb..9ec156e98f06 100644 > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > @@ -557,103 +557,10 @@ static irqreturn_t atmel_hlcdc_dc_irq_handler(int irq, void *data) > return IRQ_HANDLED; > } > > -struct atmel_hlcdc_dc_commit { > - struct work_struct work; > - struct drm_device *dev; > - struct drm_atomic_state *state; > -}; > - > -static void > -atmel_hlcdc_dc_atomic_complete(struct atmel_hlcdc_dc_commit *commit) > -{ > - struct drm_device *dev = commit->dev; > - struct atmel_hlcdc_dc *dc = dev->dev_private; > - struct drm_atomic_state *old_state = commit->state; > - > - /* Apply the atomic update. */ > - drm_atomic_helper_commit_modeset_disables(dev, old_state); > - drm_atomic_helper_commit_planes(dev, old_state, 0); > - drm_atomic_helper_commit_modeset_enables(dev, old_state); > - > - drm_atomic_helper_wait_for_vblanks(dev, old_state); > - > - drm_atomic_helper_cleanup_planes(dev, old_state); > - > - drm_atomic_state_put(old_state); > - > - /* Complete the commit, wake up any waiter. */ > - spin_lock(&dc->commit.wait.lock); > - dc->commit.pending = false; > - wake_up_all_locked(&dc->commit.wait); > - spin_unlock(&dc->commit.wait.lock); > - > - kfree(commit); > -} > - > -static void atmel_hlcdc_dc_atomic_work(struct work_struct *work) > -{ > - struct atmel_hlcdc_dc_commit *commit = > - container_of(work, struct atmel_hlcdc_dc_commit, work); > - > - atmel_hlcdc_dc_atomic_complete(commit); > -} > - > -static int atmel_hlcdc_dc_atomic_commit(struct drm_device *dev, > - struct drm_atomic_state *state, > - bool async) > -{ > - struct atmel_hlcdc_dc *dc = dev->dev_private; > - struct atmel_hlcdc_dc_commit *commit; > - int ret; > - > - ret = drm_atomic_helper_prepare_planes(dev, state); > - if (ret) > - return ret; > - > - /* Allocate the commit object. */ > - commit = kzalloc(sizeof(*commit), GFP_KERNEL); > - if (!commit) { > - ret = -ENOMEM; > - goto error; > - } > - > - INIT_WORK(&commit->work, atmel_hlcdc_dc_atomic_work); > - commit->dev = dev; > - commit->state = state; > - > - spin_lock(&dc->commit.wait.lock); > - ret = wait_event_interruptible_locked(dc->commit.wait, > - !dc->commit.pending); > - if (ret == 0) > - dc->commit.pending = true; > - spin_unlock(&dc->commit.wait.lock); > - > - if (ret) > - goto err_free; > - > - /* 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); > - else > - atmel_hlcdc_dc_atomic_complete(commit); > - > - return 0; > - > -err_free: > - kfree(commit); > -error: > - drm_atomic_helper_cleanup_planes(dev, state); > - return ret; > -} > - > static const struct drm_mode_config_funcs mode_config_funcs = { > .fb_create = drm_gem_fb_create, > .atomic_check = drm_atomic_helper_check, > - .atomic_commit = atmel_hlcdc_dc_atomic_commit, > + .atomic_commit = drm_atomic_helper_commit, > }; > > static int atmel_hlcdc_dc_modeset_init(struct drm_device *dev) > @@ -716,7 +623,6 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev) > if (!dc->wq) > return -ENOMEM; > > - init_waitqueue_head(&dc->commit.wait); > dc->desc = match->data; > dc->hlcdc = dev_get_drvdata(dev->dev->parent); > dev->dev_private = dc; > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h > index 469d4507e576..9367a3747a3a 100644 > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h > @@ -346,10 +346,6 @@ struct atmel_hlcdc_dc { > u32 imr; > struct drm_atomic_state *state; > } suspend; > - struct { > - wait_queue_head_t wait; > - bool pending; > - } commit; > }; > > extern struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats; > -- > 2.27.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] drm/atmel: Use drm_atomic_helper_commit 2020-07-07 20:12 ` [PATCH 09/25] drm/atmel: Use drm_atomic_helper_commit Daniel Vetter 2020-07-07 20:37 ` Sam Ravnborg @ 2020-07-07 21:31 ` Daniel Vetter 2020-07-14 9:55 ` Sam Ravnborg 1 sibling, 1 reply; 5+ messages in thread From: Daniel Vetter @ 2020-07-07 21:31 UTC (permalink / raw) To: DRI Development Cc: Alexandre Belloni, Boris Brezillon, linux-rdma, Daniel Vetter, Intel Graphics Development, Ludovic Desroches, Daniel Vetter, Sam Ravnborg, linux-arm-kernel One of these drivers that predates the nonblocking support in helpers, and hand-rolled its own thing. Entirely not anything specific here, we can just delete it all and replace it with the helper version. Could also perhaps use the drm_mode_config_helper_suspend/resume stuff, for another few lines deleted. But I'm not looking at that stuff, I'm just going through all the atomic commit functions and make sure they have properly annotated dma-fence critical sections everywhere. v2: - Also delete the workqueue (Sam) - drop the @commit kerneldoc, I missed that one. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Boris Brezillon <bbrezillon@kernel.org> Cc: Nicolas Ferre <nicolas.ferre@microchip.com> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> Cc: Ludovic Desroches <ludovic.desroches@microchip.com> Cc: linux-arm-kernel@lists.infradead.org --- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 107 +------------------ drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 7 -- 2 files changed, 2 insertions(+), 112 deletions(-) diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c index 871293d1aeeb..03984932d174 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c @@ -557,103 +557,10 @@ static irqreturn_t atmel_hlcdc_dc_irq_handler(int irq, void *data) return IRQ_HANDLED; } -struct atmel_hlcdc_dc_commit { - struct work_struct work; - struct drm_device *dev; - struct drm_atomic_state *state; -}; - -static void -atmel_hlcdc_dc_atomic_complete(struct atmel_hlcdc_dc_commit *commit) -{ - struct drm_device *dev = commit->dev; - struct atmel_hlcdc_dc *dc = dev->dev_private; - struct drm_atomic_state *old_state = commit->state; - - /* Apply the atomic update. */ - drm_atomic_helper_commit_modeset_disables(dev, old_state); - drm_atomic_helper_commit_planes(dev, old_state, 0); - drm_atomic_helper_commit_modeset_enables(dev, old_state); - - drm_atomic_helper_wait_for_vblanks(dev, old_state); - - drm_atomic_helper_cleanup_planes(dev, old_state); - - drm_atomic_state_put(old_state); - - /* Complete the commit, wake up any waiter. */ - spin_lock(&dc->commit.wait.lock); - dc->commit.pending = false; - wake_up_all_locked(&dc->commit.wait); - spin_unlock(&dc->commit.wait.lock); - - kfree(commit); -} - -static void atmel_hlcdc_dc_atomic_work(struct work_struct *work) -{ - struct atmel_hlcdc_dc_commit *commit = - container_of(work, struct atmel_hlcdc_dc_commit, work); - - atmel_hlcdc_dc_atomic_complete(commit); -} - -static int atmel_hlcdc_dc_atomic_commit(struct drm_device *dev, - struct drm_atomic_state *state, - bool async) -{ - struct atmel_hlcdc_dc *dc = dev->dev_private; - struct atmel_hlcdc_dc_commit *commit; - int ret; - - ret = drm_atomic_helper_prepare_planes(dev, state); - if (ret) - return ret; - - /* Allocate the commit object. */ - commit = kzalloc(sizeof(*commit), GFP_KERNEL); - if (!commit) { - ret = -ENOMEM; - goto error; - } - - INIT_WORK(&commit->work, atmel_hlcdc_dc_atomic_work); - commit->dev = dev; - commit->state = state; - - spin_lock(&dc->commit.wait.lock); - ret = wait_event_interruptible_locked(dc->commit.wait, - !dc->commit.pending); - if (ret == 0) - dc->commit.pending = true; - spin_unlock(&dc->commit.wait.lock); - - if (ret) - goto err_free; - - /* 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); - else - atmel_hlcdc_dc_atomic_complete(commit); - - return 0; - -err_free: - kfree(commit); -error: - drm_atomic_helper_cleanup_planes(dev, state); - return ret; -} - static const struct drm_mode_config_funcs mode_config_funcs = { .fb_create = drm_gem_fb_create, .atomic_check = drm_atomic_helper_check, - .atomic_commit = atmel_hlcdc_dc_atomic_commit, + .atomic_commit = drm_atomic_helper_commit, }; static int atmel_hlcdc_dc_modeset_init(struct drm_device *dev) @@ -712,11 +619,6 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev) if (!dc) return -ENOMEM; - dc->wq = alloc_ordered_workqueue("atmel-hlcdc-dc", 0); - if (!dc->wq) - return -ENOMEM; - - init_waitqueue_head(&dc->commit.wait); dc->desc = match->data; dc->hlcdc = dev_get_drvdata(dev->dev->parent); dev->dev_private = dc; @@ -724,7 +626,7 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev) ret = clk_prepare_enable(dc->hlcdc->periph_clk); if (ret) { dev_err(dev->dev, "failed to enable periph_clk\n"); - goto err_destroy_wq; + return ret; } pm_runtime_enable(dev->dev); @@ -761,9 +663,6 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev) pm_runtime_disable(dev->dev); clk_disable_unprepare(dc->hlcdc->periph_clk); -err_destroy_wq: - destroy_workqueue(dc->wq); - return ret; } @@ -771,7 +670,6 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev) { struct atmel_hlcdc_dc *dc = dev->dev_private; - flush_workqueue(dc->wq); drm_kms_helper_poll_fini(dev); drm_atomic_helper_shutdown(dev); drm_mode_config_cleanup(dev); @@ -784,7 +682,6 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev) pm_runtime_disable(dev->dev); clk_disable_unprepare(dc->hlcdc->periph_clk); - destroy_workqueue(dc->wq); } static int atmel_hlcdc_dc_irq_postinstall(struct drm_device *dev) diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h index 469d4507e576..5b5c774e0edf 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h @@ -331,9 +331,7 @@ struct atmel_hlcdc_dc_desc { * @crtc: CRTC provided by the display controller * @planes: instantiated planes * @layers: active HLCDC layers - * @wq: display controller workqueue * @suspend: used to store the HLCDC state when entering suspend - * @commit: used for async commit handling */ struct atmel_hlcdc_dc { const struct atmel_hlcdc_dc_desc *desc; @@ -341,15 +339,10 @@ struct atmel_hlcdc_dc { struct atmel_hlcdc *hlcdc; struct drm_crtc *crtc; struct atmel_hlcdc_layer *layers[ATMEL_HLCDC_MAX_LAYERS]; - struct workqueue_struct *wq; struct { u32 imr; struct drm_atomic_state *state; } suspend; - struct { - wait_queue_head_t wait; - bool pending; - } commit; }; extern struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats; -- 2.27.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/atmel: Use drm_atomic_helper_commit 2020-07-07 21:31 ` [PATCH] " Daniel Vetter @ 2020-07-14 9:55 ` Sam Ravnborg 0 siblings, 0 replies; 5+ messages in thread From: Sam Ravnborg @ 2020-07-14 9:55 UTC (permalink / raw) To: Daniel Vetter Cc: Alexandre Belloni, Boris Brezillon, linux-rdma, Intel Graphics Development, DRI Development, Ludovic Desroches, Daniel Vetter, linux-arm-kernel Hi Daniel. On Tue, Jul 07, 2020 at 11:31:37PM +0200, Daniel Vetter wrote: > One of these drivers that predates the nonblocking support in helpers, > and hand-rolled its own thing. Entirely not anything specific here, we > can just delete it all and replace it with the helper version. > > Could also perhaps use the drm_mode_config_helper_suspend/resume > stuff, for another few lines deleted. But I'm not looking at that > stuff, I'm just going through all the atomic commit functions and make > sure they have properly annotated dma-fence critical sections > everywhere. > > v2: > - Also delete the workqueue (Sam) > - drop the @commit kerneldoc, I missed that one. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: Boris Brezillon <bbrezillon@kernel.org> > Cc: Nicolas Ferre <nicolas.ferre@microchip.com> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> > Cc: Ludovic Desroches <ludovic.desroches@microchip.com> > Cc: linux-arm-kernel@lists.infradead.org I did succeed getttign my board operational. But based on reading the code: Reviewed-by: Sam Ravnborg <sam@ravnborg.org> I assume you will apply it. Sam > --- > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 107 +------------------ > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 7 -- > 2 files changed, 2 insertions(+), 112 deletions(-) > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > index 871293d1aeeb..03984932d174 100644 > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > @@ -557,103 +557,10 @@ static irqreturn_t atmel_hlcdc_dc_irq_handler(int irq, void *data) > return IRQ_HANDLED; > } > > -struct atmel_hlcdc_dc_commit { > - struct work_struct work; > - struct drm_device *dev; > - struct drm_atomic_state *state; > -}; > - > -static void > -atmel_hlcdc_dc_atomic_complete(struct atmel_hlcdc_dc_commit *commit) > -{ > - struct drm_device *dev = commit->dev; > - struct atmel_hlcdc_dc *dc = dev->dev_private; > - struct drm_atomic_state *old_state = commit->state; > - > - /* Apply the atomic update. */ > - drm_atomic_helper_commit_modeset_disables(dev, old_state); > - drm_atomic_helper_commit_planes(dev, old_state, 0); > - drm_atomic_helper_commit_modeset_enables(dev, old_state); > - > - drm_atomic_helper_wait_for_vblanks(dev, old_state); > - > - drm_atomic_helper_cleanup_planes(dev, old_state); > - > - drm_atomic_state_put(old_state); > - > - /* Complete the commit, wake up any waiter. */ > - spin_lock(&dc->commit.wait.lock); > - dc->commit.pending = false; > - wake_up_all_locked(&dc->commit.wait); > - spin_unlock(&dc->commit.wait.lock); > - > - kfree(commit); > -} > - > -static void atmel_hlcdc_dc_atomic_work(struct work_struct *work) > -{ > - struct atmel_hlcdc_dc_commit *commit = > - container_of(work, struct atmel_hlcdc_dc_commit, work); > - > - atmel_hlcdc_dc_atomic_complete(commit); > -} > - > -static int atmel_hlcdc_dc_atomic_commit(struct drm_device *dev, > - struct drm_atomic_state *state, > - bool async) > -{ > - struct atmel_hlcdc_dc *dc = dev->dev_private; > - struct atmel_hlcdc_dc_commit *commit; > - int ret; > - > - ret = drm_atomic_helper_prepare_planes(dev, state); > - if (ret) > - return ret; > - > - /* Allocate the commit object. */ > - commit = kzalloc(sizeof(*commit), GFP_KERNEL); > - if (!commit) { > - ret = -ENOMEM; > - goto error; > - } > - > - INIT_WORK(&commit->work, atmel_hlcdc_dc_atomic_work); > - commit->dev = dev; > - commit->state = state; > - > - spin_lock(&dc->commit.wait.lock); > - ret = wait_event_interruptible_locked(dc->commit.wait, > - !dc->commit.pending); > - if (ret == 0) > - dc->commit.pending = true; > - spin_unlock(&dc->commit.wait.lock); > - > - if (ret) > - goto err_free; > - > - /* 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); > - else > - atmel_hlcdc_dc_atomic_complete(commit); > - > - return 0; > - > -err_free: > - kfree(commit); > -error: > - drm_atomic_helper_cleanup_planes(dev, state); > - return ret; > -} > - > static const struct drm_mode_config_funcs mode_config_funcs = { > .fb_create = drm_gem_fb_create, > .atomic_check = drm_atomic_helper_check, > - .atomic_commit = atmel_hlcdc_dc_atomic_commit, > + .atomic_commit = drm_atomic_helper_commit, > }; > > static int atmel_hlcdc_dc_modeset_init(struct drm_device *dev) > @@ -712,11 +619,6 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev) > if (!dc) > return -ENOMEM; > > - dc->wq = alloc_ordered_workqueue("atmel-hlcdc-dc", 0); > - if (!dc->wq) > - return -ENOMEM; > - > - init_waitqueue_head(&dc->commit.wait); > dc->desc = match->data; > dc->hlcdc = dev_get_drvdata(dev->dev->parent); > dev->dev_private = dc; > @@ -724,7 +626,7 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev) > ret = clk_prepare_enable(dc->hlcdc->periph_clk); > if (ret) { > dev_err(dev->dev, "failed to enable periph_clk\n"); > - goto err_destroy_wq; > + return ret; > } > > pm_runtime_enable(dev->dev); > @@ -761,9 +663,6 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev) > pm_runtime_disable(dev->dev); > clk_disable_unprepare(dc->hlcdc->periph_clk); > > -err_destroy_wq: > - destroy_workqueue(dc->wq); > - > return ret; > } > > @@ -771,7 +670,6 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev) > { > struct atmel_hlcdc_dc *dc = dev->dev_private; > > - flush_workqueue(dc->wq); > drm_kms_helper_poll_fini(dev); > drm_atomic_helper_shutdown(dev); > drm_mode_config_cleanup(dev); > @@ -784,7 +682,6 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev) > > pm_runtime_disable(dev->dev); > clk_disable_unprepare(dc->hlcdc->periph_clk); > - destroy_workqueue(dc->wq); > } > > static int atmel_hlcdc_dc_irq_postinstall(struct drm_device *dev) > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h > index 469d4507e576..5b5c774e0edf 100644 > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h > @@ -331,9 +331,7 @@ struct atmel_hlcdc_dc_desc { > * @crtc: CRTC provided by the display controller > * @planes: instantiated planes > * @layers: active HLCDC layers > - * @wq: display controller workqueue > * @suspend: used to store the HLCDC state when entering suspend > - * @commit: used for async commit handling > */ > struct atmel_hlcdc_dc { > const struct atmel_hlcdc_dc_desc *desc; > @@ -341,15 +339,10 @@ struct atmel_hlcdc_dc { > struct atmel_hlcdc *hlcdc; > struct drm_crtc *crtc; > struct atmel_hlcdc_layer *layers[ATMEL_HLCDC_MAX_LAYERS]; > - struct workqueue_struct *wq; > struct { > u32 imr; > struct drm_atomic_state *state; > } suspend; > - struct { > - wait_queue_head_t wait; > - bool pending; > - } commit; > }; > > extern struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats; > -- > 2.27.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 10/25] drm/imx: Annotate dma-fence critical section in commit path [not found] <20200707201229.472834-1-daniel.vetter@ffwll.ch> 2020-07-07 20:12 ` [PATCH 09/25] drm/atmel: Use drm_atomic_helper_commit Daniel Vetter @ 2020-07-07 20:12 ` Daniel Vetter 1 sibling, 0 replies; 5+ messages in thread From: Daniel Vetter @ 2020-07-07 20:12 UTC (permalink / raw) To: DRI Development Cc: Fabio Estevam, Pengutronix Kernel Team, linux-rdma, Daniel Vetter, Intel Graphics Development, NXP Linux Team, Philipp Zabel, Daniel Vetter, Shawn Guo, Sascha Hauer, linux-arm-kernel drm_atomic_helper_commit_hw_done() is the last thing (no plane cleanup apparrently), so it's the entire function. And a nice comment explaining why thw wait_for_flip_done is ahead, unlike the usual sequence. Aside, I think since the atomic helpers do track plane disabling now separately this might no longer be a real problem since: commit 21a01abbe32a3cbeb903378a24e504bfd9fe0648 Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Date: Mon Sep 4 12:48:37 2017 +0200 drm/atomic: Fix freeing connector/plane state too early by tracking commits, v3. Plus the subsequent bugfixes of course, this was tricky to get right. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Philipp Zabel <p.zabel@pengutronix.de> Cc: Shawn Guo <shawnguo@kernel.org> Cc: Sascha Hauer <s.hauer@pengutronix.de> Cc: Pengutronix Kernel Team <kernel@pengutronix.de> Cc: Fabio Estevam <festevam@gmail.com> Cc: NXP Linux Team <linux-imx@nxp.com> Cc: linux-arm-kernel@lists.infradead.org --- drivers/gpu/drm/imx/imx-drm-core.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c index 36037b2e6564..8c209bd36780 100644 --- a/drivers/gpu/drm/imx/imx-drm-core.c +++ b/drivers/gpu/drm/imx/imx-drm-core.c @@ -80,6 +80,7 @@ static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state) struct drm_plane_state *old_plane_state, *new_plane_state; bool plane_disabling = false; int i; + bool fence_cookie = dma_fence_begin_signalling(); drm_atomic_helper_commit_modeset_disables(dev, state); @@ -110,6 +111,7 @@ static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state) } drm_atomic_helper_commit_hw_done(state); + dma_fence_end_signalling(fence_cookie); } static const struct drm_mode_config_helper_funcs imx_drm_mode_config_helpers = { -- 2.27.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-07-14 9:57 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20200707201229.472834-1-daniel.vetter@ffwll.ch> 2020-07-07 20:12 ` [PATCH 09/25] drm/atmel: Use drm_atomic_helper_commit Daniel Vetter 2020-07-07 20:37 ` Sam Ravnborg 2020-07-07 21:31 ` [PATCH] " Daniel Vetter 2020-07-14 9:55 ` Sam Ravnborg 2020-07-07 20:12 ` [PATCH 10/25] drm/imx: Annotate dma-fence critical section in commit path Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).