| Message ID | 20190313000532.7087-15-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | R-Car DU display writeback support | expand |
On Wed, Mar 13, 2019 at 02:05:28AM +0200, Laurent Pinchart wrote: > As writeback jobs contain a framebuffer, drivers may need to prepare and > cleanup them the same way they can prepare and cleanup framebuffers for > planes. Add two new optional connector helper operations, > .prepare_writeback_job() and .cleanup_writeback_job() to support this. > > The job prepare operation is called from > drm_atomic_helper_prepare_planes() to avoid a new atomic commit helper > that would need to be called by all drivers not using > drm_atomic_helper_commit(). The job cleanup operation is called from the > existing drm_writeback_cleanup_job() function, invoked both when > destroying the job as part of a aborted commit, or when the job > completes. > > The drm_writeback_job structure is extended with a priv field to let > drivers store per-job data, such as mappings related to the writeback > framebuffer. > > For internal plumbing reasons the drm_writeback_job structure needs to > store a back-pointer to the drm_writeback_connector. To avoid pushing > too much writeback-specific knowledge to drm_atomic_uapi.c, create a > drm_writeback_set_fb() function, move the writeback job setup code > there, and set the connector backpointer. The prepare_signaling() > function doesn't need to allocate writeback jobs and can ignore > connectors without a job, as it is called after the writeback jobs are > allocated to store framebuffers, and a writeback fence with a > framebuffer is an invalid configuration that gets rejected by the commit > check. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com> Best regards, Liviu > --- > Changes since v5: > > - Export drm_writeback_prepare_job() > - Check for .prepare_writeback_job() in drm_writeback_prepare_job() > --- > drivers/gpu/drm/drm_atomic_helper.c | 11 ++++++ > drivers/gpu/drm/drm_atomic_uapi.c | 31 +++++------------ > drivers/gpu/drm/drm_writeback.c | 44 ++++++++++++++++++++++++ > include/drm/drm_modeset_helper_vtables.h | 7 ++++ > include/drm/drm_writeback.h | 28 ++++++++++++++- > 5 files changed, 97 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 6fe2303fccd9..70a4886c6e65 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2245,10 +2245,21 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_cleanup_done); > int drm_atomic_helper_prepare_planes(struct drm_device *dev, > struct drm_atomic_state *state) > { > + struct drm_connector *connector; > + struct drm_connector_state *new_conn_state; > struct drm_plane *plane; > struct drm_plane_state *new_plane_state; > int ret, i, j; > > + for_each_new_connector_in_state(state, connector, new_conn_state, i) { > + if (!new_conn_state->writeback_job) > + continue; > + > + ret = drm_writeback_prepare_job(new_conn_state->writeback_job); > + if (ret < 0) > + return ret; > + } > + > for_each_new_plane_in_state(state, plane, new_plane_state, i) { > const struct drm_plane_helper_funcs *funcs; > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c > index c40889888a16..e802152a01ad 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -647,28 +647,15 @@ drm_atomic_plane_get_property(struct drm_plane *plane, > return 0; > } > > -static struct drm_writeback_job * > -drm_atomic_get_writeback_job(struct drm_connector_state *conn_state) > -{ > - WARN_ON(conn_state->connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK); > - > - if (!conn_state->writeback_job) > - conn_state->writeback_job = > - kzalloc(sizeof(*conn_state->writeback_job), GFP_KERNEL); > - > - return conn_state->writeback_job; > -} > - > static int drm_atomic_set_writeback_fb_for_connector( > struct drm_connector_state *conn_state, > struct drm_framebuffer *fb) > { > - struct drm_writeback_job *job = > - drm_atomic_get_writeback_job(conn_state); > - if (!job) > - return -ENOMEM; > + int ret; > > - drm_framebuffer_assign(&job->fb, fb); > + ret = drm_writeback_set_fb(conn_state, fb); > + if (ret < 0) > + return ret; > > if (fb) > DRM_DEBUG_ATOMIC("Set [FB:%d] for connector state %p\n", > @@ -1158,19 +1145,17 @@ static int prepare_signaling(struct drm_device *dev, > > for_each_new_connector_in_state(state, conn, conn_state, i) { > struct drm_writeback_connector *wb_conn; > - struct drm_writeback_job *job; > struct drm_out_fence_state *f; > struct dma_fence *fence; > s32 __user *fence_ptr; > > + if (!conn_state->writeback_job) > + continue; > + > fence_ptr = get_out_fence_for_connector(state, conn); > if (!fence_ptr) > continue; > > - job = drm_atomic_get_writeback_job(conn_state); > - if (!job) > - return -ENOMEM; > - > f = krealloc(*fence_state, sizeof(**fence_state) * > (*num_fences + 1), GFP_KERNEL); > if (!f) > @@ -1192,7 +1177,7 @@ static int prepare_signaling(struct drm_device *dev, > return ret; > } > > - job->out_fence = fence; > + conn_state->writeback_job->out_fence = fence; > } > > /* > diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c > index 1b497d3530b5..79ac014701c8 100644 > --- a/drivers/gpu/drm/drm_writeback.c > +++ b/drivers/gpu/drm/drm_writeback.c > @@ -239,6 +239,43 @@ int drm_writeback_connector_init(struct drm_device *dev, > } > EXPORT_SYMBOL(drm_writeback_connector_init); > > +int drm_writeback_set_fb(struct drm_connector_state *conn_state, > + struct drm_framebuffer *fb) > +{ > + WARN_ON(conn_state->connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK); > + > + if (!conn_state->writeback_job) { > + conn_state->writeback_job = > + kzalloc(sizeof(*conn_state->writeback_job), GFP_KERNEL); > + if (!conn_state->writeback_job) > + return -ENOMEM; > + > + conn_state->writeback_job->connector = > + drm_connector_to_writeback(conn_state->connector); > + } > + > + drm_framebuffer_assign(&conn_state->writeback_job->fb, fb); > + return 0; > +} > + > +int drm_writeback_prepare_job(struct drm_writeback_job *job) > +{ > + struct drm_writeback_connector *connector = job->connector; > + const struct drm_connector_helper_funcs *funcs = > + connector->base.helper_private; > + int ret; > + > + if (funcs->prepare_writeback_job) { > + ret = funcs->prepare_writeback_job(connector, job); > + if (ret < 0) > + return ret; > + } > + > + job->prepared = true; > + return 0; > +} > +EXPORT_SYMBOL(drm_writeback_prepare_job); > + > /** > * drm_writeback_queue_job - Queue a writeback job for later signalling > * @wb_connector: The writeback connector to queue a job on > @@ -275,6 +312,13 @@ EXPORT_SYMBOL(drm_writeback_queue_job); > > void drm_writeback_cleanup_job(struct drm_writeback_job *job) > { > + struct drm_writeback_connector *connector = job->connector; > + const struct drm_connector_helper_funcs *funcs = > + connector->base.helper_private; > + > + if (job->prepared && funcs->cleanup_writeback_job) > + funcs->cleanup_writeback_job(connector, job); > + > if (job->fb) > drm_framebuffer_put(job->fb); > > diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h > index 61142aa0ab23..73d03fe66799 100644 > --- a/include/drm/drm_modeset_helper_vtables.h > +++ b/include/drm/drm_modeset_helper_vtables.h > @@ -49,6 +49,8 @@ > */ > > enum mode_set_atomic; > +struct drm_writeback_connector; > +struct drm_writeback_job; > > /** > * struct drm_crtc_helper_funcs - helper operations for CRTCs > @@ -989,6 +991,11 @@ struct drm_connector_helper_funcs { > */ > void (*atomic_commit)(struct drm_connector *connector, > struct drm_connector_state *state); > + > + int (*prepare_writeback_job)(struct drm_writeback_connector *connector, > + struct drm_writeback_job *job); > + void (*cleanup_writeback_job)(struct drm_writeback_connector *connector, > + struct drm_writeback_job *job); > }; > > /** > diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h > index 47662c362743..777c14c847f0 100644 > --- a/include/drm/drm_writeback.h > +++ b/include/drm/drm_writeback.h > @@ -79,6 +79,20 @@ struct drm_writeback_connector { > }; > > struct drm_writeback_job { > + /** > + * @connector: > + * > + * Back-pointer to the writeback connector associated with the job > + */ > + struct drm_writeback_connector *connector; > + > + /** > + * @prepared: > + * > + * Set when the job has been prepared with drm_writeback_prepare_job() > + */ > + bool prepared; > + > /** > * @cleanup_work: > * > @@ -98,7 +112,7 @@ struct drm_writeback_job { > * @fb: > * > * Framebuffer to be written to by the writeback connector. Do not set > - * directly, use drm_atomic_set_writeback_fb_for_connector() > + * directly, use drm_writeback_set_fb() > */ > struct drm_framebuffer *fb; > > @@ -108,6 +122,13 @@ struct drm_writeback_job { > * Fence which will signal once the writeback has completed > */ > struct dma_fence *out_fence; > + > + /** > + * @priv: > + * > + * Driver-private data > + */ > + void *priv; > }; > > static inline struct drm_writeback_connector * > @@ -122,6 +143,11 @@ int drm_writeback_connector_init(struct drm_device *dev, > const struct drm_encoder_helper_funcs *enc_helper_funcs, > const u32 *formats, int n_formats); > > +int drm_writeback_set_fb(struct drm_connector_state *conn_state, > + struct drm_framebuffer *fb); > + > +int drm_writeback_prepare_job(struct drm_writeback_job *job); > + > void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector, > struct drm_connector_state *conn_state); > > -- > Regards, > > Laurent Pinchart >
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 6fe2303fccd9..70a4886c6e65 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2245,10 +2245,21 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_cleanup_done); int drm_atomic_helper_prepare_planes(struct drm_device *dev, struct drm_atomic_state *state) { + struct drm_connector *connector; + struct drm_connector_state *new_conn_state; struct drm_plane *plane; struct drm_plane_state *new_plane_state; int ret, i, j; + for_each_new_connector_in_state(state, connector, new_conn_state, i) { + if (!new_conn_state->writeback_job) + continue; + + ret = drm_writeback_prepare_job(new_conn_state->writeback_job); + if (ret < 0) + return ret; + } + for_each_new_plane_in_state(state, plane, new_plane_state, i) { const struct drm_plane_helper_funcs *funcs; diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index c40889888a16..e802152a01ad 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -647,28 +647,15 @@ drm_atomic_plane_get_property(struct drm_plane *plane, return 0; } -static struct drm_writeback_job * -drm_atomic_get_writeback_job(struct drm_connector_state *conn_state) -{ - WARN_ON(conn_state->connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK); - - if (!conn_state->writeback_job) - conn_state->writeback_job = - kzalloc(sizeof(*conn_state->writeback_job), GFP_KERNEL); - - return conn_state->writeback_job; -} - static int drm_atomic_set_writeback_fb_for_connector( struct drm_connector_state *conn_state, struct drm_framebuffer *fb) { - struct drm_writeback_job *job = - drm_atomic_get_writeback_job(conn_state); - if (!job) - return -ENOMEM; + int ret; - drm_framebuffer_assign(&job->fb, fb); + ret = drm_writeback_set_fb(conn_state, fb); + if (ret < 0) + return ret; if (fb) DRM_DEBUG_ATOMIC("Set [FB:%d] for connector state %p\n", @@ -1158,19 +1145,17 @@ static int prepare_signaling(struct drm_device *dev, for_each_new_connector_in_state(state, conn, conn_state, i) { struct drm_writeback_connector *wb_conn; - struct drm_writeback_job *job; struct drm_out_fence_state *f; struct dma_fence *fence; s32 __user *fence_ptr; + if (!conn_state->writeback_job) + continue; + fence_ptr = get_out_fence_for_connector(state, conn); if (!fence_ptr) continue; - job = drm_atomic_get_writeback_job(conn_state); - if (!job) - return -ENOMEM; - f = krealloc(*fence_state, sizeof(**fence_state) * (*num_fences + 1), GFP_KERNEL); if (!f) @@ -1192,7 +1177,7 @@ static int prepare_signaling(struct drm_device *dev, return ret; } - job->out_fence = fence; + conn_state->writeback_job->out_fence = fence; } /* diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c index 1b497d3530b5..79ac014701c8 100644 --- a/drivers/gpu/drm/drm_writeback.c +++ b/drivers/gpu/drm/drm_writeback.c @@ -239,6 +239,43 @@ int drm_writeback_connector_init(struct drm_device *dev, } EXPORT_SYMBOL(drm_writeback_connector_init); +int drm_writeback_set_fb(struct drm_connector_state *conn_state, + struct drm_framebuffer *fb) +{ + WARN_ON(conn_state->connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK); + + if (!conn_state->writeback_job) { + conn_state->writeback_job = + kzalloc(sizeof(*conn_state->writeback_job), GFP_KERNEL); + if (!conn_state->writeback_job) + return -ENOMEM; + + conn_state->writeback_job->connector = + drm_connector_to_writeback(conn_state->connector); + } + + drm_framebuffer_assign(&conn_state->writeback_job->fb, fb); + return 0; +} + +int drm_writeback_prepare_job(struct drm_writeback_job *job) +{ + struct drm_writeback_connector *connector = job->connector; + const struct drm_connector_helper_funcs *funcs = + connector->base.helper_private; + int ret; + + if (funcs->prepare_writeback_job) { + ret = funcs->prepare_writeback_job(connector, job); + if (ret < 0) + return ret; + } + + job->prepared = true; + return 0; +} +EXPORT_SYMBOL(drm_writeback_prepare_job); + /** * drm_writeback_queue_job - Queue a writeback job for later signalling * @wb_connector: The writeback connector to queue a job on @@ -275,6 +312,13 @@ EXPORT_SYMBOL(drm_writeback_queue_job); void drm_writeback_cleanup_job(struct drm_writeback_job *job) { + struct drm_writeback_connector *connector = job->connector; + const struct drm_connector_helper_funcs *funcs = + connector->base.helper_private; + + if (job->prepared && funcs->cleanup_writeback_job) + funcs->cleanup_writeback_job(connector, job); + if (job->fb) drm_framebuffer_put(job->fb); diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index 61142aa0ab23..73d03fe66799 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -49,6 +49,8 @@ */ enum mode_set_atomic; +struct drm_writeback_connector; +struct drm_writeback_job; /** * struct drm_crtc_helper_funcs - helper operations for CRTCs @@ -989,6 +991,11 @@ struct drm_connector_helper_funcs { */ void (*atomic_commit)(struct drm_connector *connector, struct drm_connector_state *state); + + int (*prepare_writeback_job)(struct drm_writeback_connector *connector, + struct drm_writeback_job *job); + void (*cleanup_writeback_job)(struct drm_writeback_connector *connector, + struct drm_writeback_job *job); }; /** diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h index 47662c362743..777c14c847f0 100644 --- a/include/drm/drm_writeback.h +++ b/include/drm/drm_writeback.h @@ -79,6 +79,20 @@ struct drm_writeback_connector { }; struct drm_writeback_job { + /** + * @connector: + * + * Back-pointer to the writeback connector associated with the job + */ + struct drm_writeback_connector *connector; + + /** + * @prepared: + * + * Set when the job has been prepared with drm_writeback_prepare_job() + */ + bool prepared; + /** * @cleanup_work: * @@ -98,7 +112,7 @@ struct drm_writeback_job { * @fb: * * Framebuffer to be written to by the writeback connector. Do not set - * directly, use drm_atomic_set_writeback_fb_for_connector() + * directly, use drm_writeback_set_fb() */ struct drm_framebuffer *fb; @@ -108,6 +122,13 @@ struct drm_writeback_job { * Fence which will signal once the writeback has completed */ struct dma_fence *out_fence; + + /** + * @priv: + * + * Driver-private data + */ + void *priv; }; static inline struct drm_writeback_connector * @@ -122,6 +143,11 @@ int drm_writeback_connector_init(struct drm_device *dev, const struct drm_encoder_helper_funcs *enc_helper_funcs, const u32 *formats, int n_formats); +int drm_writeback_set_fb(struct drm_connector_state *conn_state, + struct drm_framebuffer *fb); + +int drm_writeback_prepare_job(struct drm_writeback_job *job); + void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector, struct drm_connector_state *conn_state);
As writeback jobs contain a framebuffer, drivers may need to prepare and cleanup them the same way they can prepare and cleanup framebuffers for planes. Add two new optional connector helper operations, .prepare_writeback_job() and .cleanup_writeback_job() to support this. The job prepare operation is called from drm_atomic_helper_prepare_planes() to avoid a new atomic commit helper that would need to be called by all drivers not using drm_atomic_helper_commit(). The job cleanup operation is called from the existing drm_writeback_cleanup_job() function, invoked both when destroying the job as part of a aborted commit, or when the job completes. The drm_writeback_job structure is extended with a priv field to let drivers store per-job data, such as mappings related to the writeback framebuffer. For internal plumbing reasons the drm_writeback_job structure needs to store a back-pointer to the drm_writeback_connector. To avoid pushing too much writeback-specific knowledge to drm_atomic_uapi.c, create a drm_writeback_set_fb() function, move the writeback job setup code there, and set the connector backpointer. The prepare_signaling() function doesn't need to allocate writeback jobs and can ignore connectors without a job, as it is called after the writeback jobs are allocated to store framebuffers, and a writeback fence with a framebuffer is an invalid configuration that gets rejected by the commit check. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- Changes since v5: - Export drm_writeback_prepare_job() - Check for .prepare_writeback_job() in drm_writeback_prepare_job() --- drivers/gpu/drm/drm_atomic_helper.c | 11 ++++++ drivers/gpu/drm/drm_atomic_uapi.c | 31 +++++------------ drivers/gpu/drm/drm_writeback.c | 44 ++++++++++++++++++++++++ include/drm/drm_modeset_helper_vtables.h | 7 ++++ include/drm/drm_writeback.h | 28 ++++++++++++++- 5 files changed, 97 insertions(+), 24 deletions(-)