| Message ID | 20190313000532.7087-13-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | R-Car DU display writeback support | expand |
Hi Laurent, On 13/03/2019 00:05, Laurent Pinchart wrote: > The drm_writeback_queue_job() function takes ownership of the passed job > and requires the caller to manually set the connector state > writeback_job pointer to NULL. To simplify drivers and avoid errors > (such as the missing NULL set in the vc4 driver), pass the connector > state pointer to the function instead of the job pointer, and set the > writeback_job pointer to NULL internally. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > Reviewed-by: Brian Starkey <brian.starkey@arm.com> > Acked-by: Eric Anholt <eric@anholt.net> > Acked-by: Liviu Dudau <liviu.dudau@arm.com> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > --- > drivers/gpu/drm/arm/malidp_mw.c | 3 +-- > drivers/gpu/drm/drm_writeback.c | 15 ++++++++++----- > drivers/gpu/drm/vc4/vc4_txp.c | 2 +- > include/drm/drm_writeback.h | 2 +- > 4 files changed, 13 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c > index 041a64dc7167..87627219ce3b 100644 > --- a/drivers/gpu/drm/arm/malidp_mw.c > +++ b/drivers/gpu/drm/arm/malidp_mw.c > @@ -252,8 +252,7 @@ void malidp_mw_atomic_commit(struct drm_device *drm, > &mw_state->addrs[0], > mw_state->format); > > - drm_writeback_queue_job(mw_conn, conn_state->writeback_job); > - conn_state->writeback_job = NULL; > + drm_writeback_queue_job(mw_conn, conn_state); > hwdev->hw->enable_memwrite(hwdev, mw_state->addrs, > mw_state->pitches, mw_state->n_planes, > fb->width, fb->height, mw_state->format, > diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c > index c20e6fe00cb3..338b993d7c9f 100644 > --- a/drivers/gpu/drm/drm_writeback.c > +++ b/drivers/gpu/drm/drm_writeback.c > @@ -242,11 +242,12 @@ EXPORT_SYMBOL(drm_writeback_connector_init); > /** > * drm_writeback_queue_job - Queue a writeback job for later signalling > * @wb_connector: The writeback connector to queue a job on > - * @job: The job to queue > + * @conn_state: The connector state containing the job to queue > * > - * This function adds a job to the job_queue for a writeback connector. It > - * should be considered to take ownership of the writeback job, and so any other > - * references to the job must be cleared after calling this function. > + * This function adds the job contained in @conn_state to the job_queue for a > + * writeback connector. It takes ownership of the writeback job and sets the > + * @conn_state->writeback_job to NULL, and so no access to the job may be > + * performed by the caller after this function returns. > * > * Drivers must ensure that for a given writeback connector, jobs are queued in > * exactly the same order as they will be completed by the hardware (and > @@ -258,10 +259,14 @@ EXPORT_SYMBOL(drm_writeback_connector_init); > * See also: drm_writeback_signal_completion() > */ > void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector, > - struct drm_writeback_job *job) > + struct drm_connector_state *conn_state) > { > + struct drm_writeback_job *job; > unsigned long flags; > > + job = conn_state->writeback_job; > + conn_state->writeback_job = NULL; > + > spin_lock_irqsave(&wb_connector->job_lock, flags); > list_add_tail(&job->list_entry, &wb_connector->job_queue); > spin_unlock_irqrestore(&wb_connector->job_lock, flags); > diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c > index aa279b5b0de7..5dabd91f2d7e 100644 > --- a/drivers/gpu/drm/vc4/vc4_txp.c > +++ b/drivers/gpu/drm/vc4/vc4_txp.c > @@ -327,7 +327,7 @@ static void vc4_txp_connector_atomic_commit(struct drm_connector *conn, > > TXP_WRITE(TXP_DST_CTRL, ctrl); > > - drm_writeback_queue_job(&txp->connector, conn_state->writeback_job); > + drm_writeback_queue_job(&txp->connector, conn_state); > } > > static const struct drm_connector_helper_funcs vc4_txp_connector_helper_funcs = { > diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h > index 23df9d463003..47662c362743 100644 > --- a/include/drm/drm_writeback.h > +++ b/include/drm/drm_writeback.h > @@ -123,7 +123,7 @@ int drm_writeback_connector_init(struct drm_device *dev, > const u32 *formats, int n_formats); > > void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector, > - struct drm_writeback_job *job); > + struct drm_connector_state *conn_state); > > void drm_writeback_cleanup_job(struct drm_writeback_job *job); > >
diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c index 041a64dc7167..87627219ce3b 100644 --- a/drivers/gpu/drm/arm/malidp_mw.c +++ b/drivers/gpu/drm/arm/malidp_mw.c @@ -252,8 +252,7 @@ void malidp_mw_atomic_commit(struct drm_device *drm, &mw_state->addrs[0], mw_state->format); - drm_writeback_queue_job(mw_conn, conn_state->writeback_job); - conn_state->writeback_job = NULL; + drm_writeback_queue_job(mw_conn, conn_state); hwdev->hw->enable_memwrite(hwdev, mw_state->addrs, mw_state->pitches, mw_state->n_planes, fb->width, fb->height, mw_state->format, diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c index c20e6fe00cb3..338b993d7c9f 100644 --- a/drivers/gpu/drm/drm_writeback.c +++ b/drivers/gpu/drm/drm_writeback.c @@ -242,11 +242,12 @@ EXPORT_SYMBOL(drm_writeback_connector_init); /** * drm_writeback_queue_job - Queue a writeback job for later signalling * @wb_connector: The writeback connector to queue a job on - * @job: The job to queue + * @conn_state: The connector state containing the job to queue * - * This function adds a job to the job_queue for a writeback connector. It - * should be considered to take ownership of the writeback job, and so any other - * references to the job must be cleared after calling this function. + * This function adds the job contained in @conn_state to the job_queue for a + * writeback connector. It takes ownership of the writeback job and sets the + * @conn_state->writeback_job to NULL, and so no access to the job may be + * performed by the caller after this function returns. * * Drivers must ensure that for a given writeback connector, jobs are queued in * exactly the same order as they will be completed by the hardware (and @@ -258,10 +259,14 @@ EXPORT_SYMBOL(drm_writeback_connector_init); * See also: drm_writeback_signal_completion() */ void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector, - struct drm_writeback_job *job) + struct drm_connector_state *conn_state) { + struct drm_writeback_job *job; unsigned long flags; + job = conn_state->writeback_job; + conn_state->writeback_job = NULL; + spin_lock_irqsave(&wb_connector->job_lock, flags); list_add_tail(&job->list_entry, &wb_connector->job_queue); spin_unlock_irqrestore(&wb_connector->job_lock, flags); diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c index aa279b5b0de7..5dabd91f2d7e 100644 --- a/drivers/gpu/drm/vc4/vc4_txp.c +++ b/drivers/gpu/drm/vc4/vc4_txp.c @@ -327,7 +327,7 @@ static void vc4_txp_connector_atomic_commit(struct drm_connector *conn, TXP_WRITE(TXP_DST_CTRL, ctrl); - drm_writeback_queue_job(&txp->connector, conn_state->writeback_job); + drm_writeback_queue_job(&txp->connector, conn_state); } static const struct drm_connector_helper_funcs vc4_txp_connector_helper_funcs = { diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h index 23df9d463003..47662c362743 100644 --- a/include/drm/drm_writeback.h +++ b/include/drm/drm_writeback.h @@ -123,7 +123,7 @@ int drm_writeback_connector_init(struct drm_device *dev, const u32 *formats, int n_formats); void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector, - struct drm_writeback_job *job); + struct drm_connector_state *conn_state); void drm_writeback_cleanup_job(struct drm_writeback_job *job);