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@xxxxxxxxx> > Cc: Sam Ravnborg <sam@xxxxxxxxxxxx> > Cc: Boris Brezillon <bbrezillon@xxxxxxxxxx> > Cc: Nicolas Ferre <nicolas.ferre@xxxxxxxxxxxxx> > Cc: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx> > Cc: Ludovic Desroches <ludovic.desroches@xxxxxxxxxxxxx> > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx 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@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel