| Message ID | 20181011001657.1715-5-drawat@vmware.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,01/18] drm: Add a new plane property to send damage during plane update | expand |
On Wed, Oct 10, 2018 at 05:16:44PM -0700, Deepak Rawat wrote: > From: Rob Clark <robdclark@gmail.com> > > Add an atomic helper to implement dirtyfb support. This is needed to > support DSI command-mode panels with x11 userspace (ie. when we can't > rely on pageflips to trigger a flush to the panel). > > v2: Modified the helper to use plane fb_damage_clips property and > removed plane_state::dirty flag. > > v3: > - Use uapi drm_mode_rect. > - Support annotate flags. > > v4: Correct kernel doc. > > Cc: ville.syrjala@linux.intel.com > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Pekka Paalanen <ppaalanen@gmail.com> > Cc: Daniel Stone <daniel@fooishbar.org> > Signed-off-by: Rob Clark <robdclark@gmail.com> > Signed-off-by: Deepak Rawat <drawat@vmware.com> > --- > drivers/gpu/drm/drm_damage_helper.c | 124 ++++++++++++++++++++++++++++ > include/drm/drm_damage_helper.h | 4 + > 2 files changed, 128 insertions(+) > > diff --git a/drivers/gpu/drm/drm_damage_helper.c b/drivers/gpu/drm/drm_damage_helper.c > index e80fa6e08a99..fdb5a072d7e9 100644 > --- a/drivers/gpu/drm/drm_damage_helper.c > +++ b/drivers/gpu/drm/drm_damage_helper.c > @@ -26,6 +26,7 @@ > * > * Authors: > * Deepak Rawat <drawat@vmware.com> > + * Rob Clark <robdclark@gmail.com> > * > **************************************************************************/ > > @@ -70,6 +71,21 @@ > * rectangles clipped to &drm_plane_state.src. > */ > > +static void convert_clip_rect_to_rect(const struct drm_clip_rect *src, > + struct drm_mode_rect *dest, > + uint32_t num_clips, uint32_t src_inc) > +{ > + while (num_clips > 0) { > + dest->x1 = src->x1; > + dest->y1 = src->y1; > + dest->x2 = src->x2; > + dest->y2 = src->y2; > + src += src_inc; > + dest++; > + num_clips--; > + } > +} > + > /** > * drm_plane_enable_fb_damage_clips - Enables plane fb damage clips property. > * @plane: Plane on which to enable damage clips property. > @@ -123,6 +139,114 @@ void drm_atomic_helper_check_plane_damage(struct drm_atomic_state *state, > } > EXPORT_SYMBOL(drm_atomic_helper_check_plane_damage); > > +/** > + * drm_atomic_helper_dirtyfb - Helper for dirtyfb. > + * @fb: DRM framebuffer. > + * @file_priv: Drm file for the ioctl call. > + * @flags: Dirty fb annotate flags. > + * @color: Color for annotate fill. > + * @clips: Dirty region. > + * @num_clips: Count of clip in clips. > + * > + * A helper to implement &drm_framebuffer_funcs::dirty using damage interface s/::/./ to make the kerneldoc work. > + * during plane update. If num_clips is 0 then this helper will do a full plane > + * update. This is the same behaviour expected by dirtyfb ioctl(). I think usually we all-uppercase DIRTFB IOCTL, but this is also ok I think. With this addressed: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > + * > + * Note that this helper is blocking implementation. This is because current > + * user of this helper is vmwgfx which needs blocking. > + * > + * Return: Zero on success, negative errno on failure. > + */ > +int drm_atomic_helper_dirtyfb(struct drm_framebuffer *fb, > + struct drm_file *file_priv, unsigned flags, > + unsigned color, struct drm_clip_rect *clips, > + unsigned num_clips) > +{ > + struct drm_modeset_acquire_ctx ctx; > + struct drm_property_blob *damage = NULL; > + struct drm_mode_rect *rects = NULL; > + struct drm_atomic_state *state; > + struct drm_plane *plane; > + int ret = 0; > + > + /* > + * When called from ioctl, we are interruptable, but not when called > + * internally (ie. defio worker) > + */ > + drm_modeset_acquire_init(&ctx, > + file_priv ? DRM_MODESET_ACQUIRE_INTERRUPTIBLE : 0); > + > + state = drm_atomic_state_alloc(fb->dev); > + if (!state) { > + ret = -ENOMEM; > + goto out; > + } > + state->acquire_ctx = &ctx; > + > + if (clips) { > + uint32_t inc = 1; > + > + if (flags & DRM_MODE_FB_DIRTY_ANNOTATE_COPY) { > + inc = 2; > + num_clips /= 2; > + } > + > + rects = kcalloc(num_clips, sizeof(*rects), GFP_KERNEL); > + if (!rects) { > + ret = -ENOMEM; > + goto out; > + } > + > + convert_clip_rect_to_rect(clips, rects, num_clips, inc); > + damage = drm_property_create_blob(fb->dev, > + num_clips * sizeof(*rects), > + rects); > + if (IS_ERR(damage)) { > + ret = PTR_ERR(damage); > + damage = NULL; > + goto out; > + } > + } > + > +retry: > + drm_for_each_plane(plane, fb->dev) { > + struct drm_plane_state *plane_state; > + > + if (plane->state->fb != fb) > + continue; > + > + plane_state = drm_atomic_get_plane_state(state, plane); > + if (IS_ERR(plane_state)) { > + ret = PTR_ERR(plane_state); > + goto out; > + } > + > + drm_property_replace_blob(&plane_state->fb_damage_clips, > + damage); > + } > + > + ret = drm_atomic_commit(state); > + > +out: > + if (ret == -EDEADLK) { > + drm_atomic_state_clear(state); > + ret = drm_modeset_backoff(&ctx); > + if (!ret) > + goto retry; > + } > + > + drm_property_blob_put(damage); > + kfree(rects); > + drm_atomic_state_put(state); > + > + drm_modeset_drop_locks(&ctx); > + drm_modeset_acquire_fini(&ctx); > + > + return ret; > + > +} > +EXPORT_SYMBOL(drm_atomic_helper_dirtyfb); > + > /** > * drm_atomic_helper_damage_iter_init - Initialize the damage iterator. > * @iter: The iterator to initialize. > diff --git a/include/drm/drm_damage_helper.h b/include/drm/drm_damage_helper.h > index b9a0d05fb093..82a7128c9f97 100644 > --- a/include/drm/drm_damage_helper.h > +++ b/include/drm/drm_damage_helper.h > @@ -67,6 +67,10 @@ struct drm_atomic_helper_damage_iter { > void drm_plane_enable_fb_damage_clips(struct drm_plane *plane); > void drm_atomic_helper_check_plane_damage(struct drm_atomic_state *state, > struct drm_plane_state *plane_state); > +int drm_atomic_helper_dirtyfb(struct drm_framebuffer *fb, > + struct drm_file *file_priv, unsigned flags, > + unsigned color, struct drm_clip_rect *clips, > + unsigned num_clips); > void > drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter, > const struct drm_plane_state *old_state, > -- > 2.17.1 >
On Thu, Oct 11, 2018 at 06:15:47PM +0200, Daniel Vetter wrote: > On Wed, Oct 10, 2018 at 05:16:44PM -0700, Deepak Rawat wrote: > > From: Rob Clark <robdclark@gmail.com> > > > > Add an atomic helper to implement dirtyfb support. This is needed to > > support DSI command-mode panels with x11 userspace (ie. when we can't > > rely on pageflips to trigger a flush to the panel). > > > > v2: Modified the helper to use plane fb_damage_clips property and > > removed plane_state::dirty flag. > > > > v3: > > - Use uapi drm_mode_rect. > > - Support annotate flags. > > > > v4: Correct kernel doc. > > > > Cc: ville.syrjala@linux.intel.com > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: Pekka Paalanen <ppaalanen@gmail.com> > > Cc: Daniel Stone <daniel@fooishbar.org> > > Signed-off-by: Rob Clark <robdclark@gmail.com> > > Signed-off-by: Deepak Rawat <drawat@vmware.com> > > --- > > drivers/gpu/drm/drm_damage_helper.c | 124 ++++++++++++++++++++++++++++ > > include/drm/drm_damage_helper.h | 4 + > > 2 files changed, 128 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_damage_helper.c b/drivers/gpu/drm/drm_damage_helper.c > > index e80fa6e08a99..fdb5a072d7e9 100644 > > --- a/drivers/gpu/drm/drm_damage_helper.c > > +++ b/drivers/gpu/drm/drm_damage_helper.c > > @@ -26,6 +26,7 @@ > > * > > * Authors: > > * Deepak Rawat <drawat@vmware.com> > > + * Rob Clark <robdclark@gmail.com> > > * > > **************************************************************************/ > > > > @@ -70,6 +71,21 @@ > > * rectangles clipped to &drm_plane_state.src. > > */ > > > > +static void convert_clip_rect_to_rect(const struct drm_clip_rect *src, > > + struct drm_mode_rect *dest, > > + uint32_t num_clips, uint32_t src_inc) > > +{ > > + while (num_clips > 0) { > > + dest->x1 = src->x1; > > + dest->y1 = src->y1; > > + dest->x2 = src->x2; > > + dest->y2 = src->y2; > > + src += src_inc; > > + dest++; > > + num_clips--; > > + } > > +} > > + > > /** > > * drm_plane_enable_fb_damage_clips - Enables plane fb damage clips property. > > * @plane: Plane on which to enable damage clips property. > > @@ -123,6 +139,114 @@ void drm_atomic_helper_check_plane_damage(struct drm_atomic_state *state, > > } > > EXPORT_SYMBOL(drm_atomic_helper_check_plane_damage); > > > > +/** > > + * drm_atomic_helper_dirtyfb - Helper for dirtyfb. > > + * @fb: DRM framebuffer. > > + * @file_priv: Drm file for the ioctl call. > > + * @flags: Dirty fb annotate flags. > > + * @color: Color for annotate fill. > > + * @clips: Dirty region. > > + * @num_clips: Count of clip in clips. > > + * > > + * A helper to implement &drm_framebuffer_funcs::dirty using damage interface > > s/::/./ to make the kerneldoc work. > > > + * during plane update. If num_clips is 0 then this helper will do a full plane > > + * update. This is the same behaviour expected by dirtyfb ioctl(). > > I think usually we all-uppercase DIRTFB IOCTL, but this is also ok I > think. > > With this addressed: > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > + * > > + * Note that this helper is blocking implementation. This is because current > > + * user of this helper is vmwgfx which needs blocking. We also know other drivers block here too. I think you could reword this to not be vmwgfx-specific, e.g. "This is what current drivers and userspace expect in their DIRTYFB IOCTL implementation, as a way to rate-limit userspace and make sure its rendering doesn't get ahead of uploading new data too much." -Daniel > > + * > > + * Return: Zero on success, negative errno on failure. > > + */ > > +int drm_atomic_helper_dirtyfb(struct drm_framebuffer *fb, > > + struct drm_file *file_priv, unsigned flags, > > + unsigned color, struct drm_clip_rect *clips, > > + unsigned num_clips) > > +{ > > + struct drm_modeset_acquire_ctx ctx; > > + struct drm_property_blob *damage = NULL; > > + struct drm_mode_rect *rects = NULL; > > + struct drm_atomic_state *state; > > + struct drm_plane *plane; > > + int ret = 0; > > + > > + /* > > + * When called from ioctl, we are interruptable, but not when called > > + * internally (ie. defio worker) > > + */ > > + drm_modeset_acquire_init(&ctx, > > + file_priv ? DRM_MODESET_ACQUIRE_INTERRUPTIBLE : 0); > > + > > + state = drm_atomic_state_alloc(fb->dev); > > + if (!state) { > > + ret = -ENOMEM; > > + goto out; > > + } > > + state->acquire_ctx = &ctx; > > + > > + if (clips) { > > + uint32_t inc = 1; > > + > > + if (flags & DRM_MODE_FB_DIRTY_ANNOTATE_COPY) { > > + inc = 2; > > + num_clips /= 2; > > + } > > + > > + rects = kcalloc(num_clips, sizeof(*rects), GFP_KERNEL); > > + if (!rects) { > > + ret = -ENOMEM; > > + goto out; > > + } > > + > > + convert_clip_rect_to_rect(clips, rects, num_clips, inc); > > + damage = drm_property_create_blob(fb->dev, > > + num_clips * sizeof(*rects), > > + rects); > > + if (IS_ERR(damage)) { > > + ret = PTR_ERR(damage); > > + damage = NULL; > > + goto out; > > + } > > + } > > + > > +retry: > > + drm_for_each_plane(plane, fb->dev) { > > + struct drm_plane_state *plane_state; > > + > > + if (plane->state->fb != fb) > > + continue; > > + > > + plane_state = drm_atomic_get_plane_state(state, plane); > > + if (IS_ERR(plane_state)) { > > + ret = PTR_ERR(plane_state); > > + goto out; > > + } > > + > > + drm_property_replace_blob(&plane_state->fb_damage_clips, > > + damage); > > + } > > + > > + ret = drm_atomic_commit(state); > > + > > +out: > > + if (ret == -EDEADLK) { > > + drm_atomic_state_clear(state); > > + ret = drm_modeset_backoff(&ctx); > > + if (!ret) > > + goto retry; > > + } > > + > > + drm_property_blob_put(damage); > > + kfree(rects); > > + drm_atomic_state_put(state); > > + > > + drm_modeset_drop_locks(&ctx); > > + drm_modeset_acquire_fini(&ctx); > > + > > + return ret; > > + > > +} > > +EXPORT_SYMBOL(drm_atomic_helper_dirtyfb); > > + > > /** > > * drm_atomic_helper_damage_iter_init - Initialize the damage iterator. > > * @iter: The iterator to initialize. > > diff --git a/include/drm/drm_damage_helper.h b/include/drm/drm_damage_helper.h > > index b9a0d05fb093..82a7128c9f97 100644 > > --- a/include/drm/drm_damage_helper.h > > +++ b/include/drm/drm_damage_helper.h > > @@ -67,6 +67,10 @@ struct drm_atomic_helper_damage_iter { > > void drm_plane_enable_fb_damage_clips(struct drm_plane *plane); > > void drm_atomic_helper_check_plane_damage(struct drm_atomic_state *state, > > struct drm_plane_state *plane_state); > > +int drm_atomic_helper_dirtyfb(struct drm_framebuffer *fb, > > + struct drm_file *file_priv, unsigned flags, > > + unsigned color, struct drm_clip_rect *clips, > > + unsigned num_clips); > > void > > drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter, > > const struct drm_plane_state *old_state, > > -- > > 2.17.1 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/drm_damage_helper.c b/drivers/gpu/drm/drm_damage_helper.c index e80fa6e08a99..fdb5a072d7e9 100644 --- a/drivers/gpu/drm/drm_damage_helper.c +++ b/drivers/gpu/drm/drm_damage_helper.c @@ -26,6 +26,7 @@ * * Authors: * Deepak Rawat <drawat@vmware.com> + * Rob Clark <robdclark@gmail.com> * **************************************************************************/ @@ -70,6 +71,21 @@ * rectangles clipped to &drm_plane_state.src. */ +static void convert_clip_rect_to_rect(const struct drm_clip_rect *src, + struct drm_mode_rect *dest, + uint32_t num_clips, uint32_t src_inc) +{ + while (num_clips > 0) { + dest->x1 = src->x1; + dest->y1 = src->y1; + dest->x2 = src->x2; + dest->y2 = src->y2; + src += src_inc; + dest++; + num_clips--; + } +} + /** * drm_plane_enable_fb_damage_clips - Enables plane fb damage clips property. * @plane: Plane on which to enable damage clips property. @@ -123,6 +139,114 @@ void drm_atomic_helper_check_plane_damage(struct drm_atomic_state *state, } EXPORT_SYMBOL(drm_atomic_helper_check_plane_damage); +/** + * drm_atomic_helper_dirtyfb - Helper for dirtyfb. + * @fb: DRM framebuffer. + * @file_priv: Drm file for the ioctl call. + * @flags: Dirty fb annotate flags. + * @color: Color for annotate fill. + * @clips: Dirty region. + * @num_clips: Count of clip in clips. + * + * A helper to implement &drm_framebuffer_funcs::dirty using damage interface + * during plane update. If num_clips is 0 then this helper will do a full plane + * update. This is the same behaviour expected by dirtyfb ioctl(). + * + * Note that this helper is blocking implementation. This is because current + * user of this helper is vmwgfx which needs blocking. + * + * Return: Zero on success, negative errno on failure. + */ +int drm_atomic_helper_dirtyfb(struct drm_framebuffer *fb, + struct drm_file *file_priv, unsigned flags, + unsigned color, struct drm_clip_rect *clips, + unsigned num_clips) +{ + struct drm_modeset_acquire_ctx ctx; + struct drm_property_blob *damage = NULL; + struct drm_mode_rect *rects = NULL; + struct drm_atomic_state *state; + struct drm_plane *plane; + int ret = 0; + + /* + * When called from ioctl, we are interruptable, but not when called + * internally (ie. defio worker) + */ + drm_modeset_acquire_init(&ctx, + file_priv ? DRM_MODESET_ACQUIRE_INTERRUPTIBLE : 0); + + state = drm_atomic_state_alloc(fb->dev); + if (!state) { + ret = -ENOMEM; + goto out; + } + state->acquire_ctx = &ctx; + + if (clips) { + uint32_t inc = 1; + + if (flags & DRM_MODE_FB_DIRTY_ANNOTATE_COPY) { + inc = 2; + num_clips /= 2; + } + + rects = kcalloc(num_clips, sizeof(*rects), GFP_KERNEL); + if (!rects) { + ret = -ENOMEM; + goto out; + } + + convert_clip_rect_to_rect(clips, rects, num_clips, inc); + damage = drm_property_create_blob(fb->dev, + num_clips * sizeof(*rects), + rects); + if (IS_ERR(damage)) { + ret = PTR_ERR(damage); + damage = NULL; + goto out; + } + } + +retry: + drm_for_each_plane(plane, fb->dev) { + struct drm_plane_state *plane_state; + + if (plane->state->fb != fb) + continue; + + plane_state = drm_atomic_get_plane_state(state, plane); + if (IS_ERR(plane_state)) { + ret = PTR_ERR(plane_state); + goto out; + } + + drm_property_replace_blob(&plane_state->fb_damage_clips, + damage); + } + + ret = drm_atomic_commit(state); + +out: + if (ret == -EDEADLK) { + drm_atomic_state_clear(state); + ret = drm_modeset_backoff(&ctx); + if (!ret) + goto retry; + } + + drm_property_blob_put(damage); + kfree(rects); + drm_atomic_state_put(state); + + drm_modeset_drop_locks(&ctx); + drm_modeset_acquire_fini(&ctx); + + return ret; + +} +EXPORT_SYMBOL(drm_atomic_helper_dirtyfb); + /** * drm_atomic_helper_damage_iter_init - Initialize the damage iterator. * @iter: The iterator to initialize. diff --git a/include/drm/drm_damage_helper.h b/include/drm/drm_damage_helper.h index b9a0d05fb093..82a7128c9f97 100644 --- a/include/drm/drm_damage_helper.h +++ b/include/drm/drm_damage_helper.h @@ -67,6 +67,10 @@ struct drm_atomic_helper_damage_iter { void drm_plane_enable_fb_damage_clips(struct drm_plane *plane); void drm_atomic_helper_check_plane_damage(struct drm_atomic_state *state, struct drm_plane_state *plane_state); +int drm_atomic_helper_dirtyfb(struct drm_framebuffer *fb, + struct drm_file *file_priv, unsigned flags, + unsigned color, struct drm_clip_rect *clips, + unsigned num_clips); void drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter, const struct drm_plane_state *old_state,