On Wed, Sep 05, 2018 at 04:38:49PM -0700, Deepak Rawat wrote:
> With fb_damage_clips blob property in drm_plane_state, this patch adds
> helper iterator to traverse the damage clips that lie inside plane src.
> Iterator will return full plane src as damage in case need full plane
> update or damage is not specified.
>
> Signed-off-by: Deepak Rawat <drawat@xxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_damage_helper.c | 93 +++++++++++++++++++++++++++++
> include/drm/drm_damage_helper.h | 20 +++++++
> 2 files changed, 113 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_damage_helper.c b/drivers/gpu/drm/drm_damage_helper.c
> index 3e7de5afe7f6..7d70f6001889 100644
> --- a/drivers/gpu/drm/drm_damage_helper.c
> +++ b/drivers/gpu/drm/drm_damage_helper.c
> @@ -30,6 +30,7 @@
> **************************************************************************/
>
> #include <drm/drmP.h>
> +#include <drm/drm_atomic.h>
> #include <drm/drm_damage_helper.h>
>
> /**
> @@ -75,6 +76,11 @@
> * While kernel does not error for overlapped damage clips, it is discouraged.
> */
>
> +static int convert_fixed_to_32(int fixed)
> +{
> + return ((fixed >> 15) & 1) + (fixed >> 16);
> +}
> +
> /**
> * drm_plane_enable_fb_damage_clips - enables plane fb damage clips property
> * @plane: plane on which to enable damage clips property
> @@ -90,3 +96,90 @@ void drm_plane_enable_fb_damage_clips(struct drm_plane *plane)
> 0);
> }
> EXPORT_SYMBOL(drm_plane_enable_fb_damage_clips);
> +
> +/**
> + * drm_atomic_helper_damage_iter_init - initialize the damage iterator
> + * @iter: The iterator to initialize.
> + * @old_state: old plane state for validation.
> + * @new_state: plane state from which to iterate the damage clips.
> + *
> + * Initialize an iterator that clip framebuffer damage in plane fb_damage_clips
> + * blob to plane src clip. The iterator returns full plane src in case needing
> + * full update e.g. during full modeset.
> + *
> + * With this helper iterator, drivers which enabled fb_damage_clips property can
> + * iterate over the damage clips that falls inside plane src during plane
> + * update.
> + *
> + * Returns: 0 on success and negative error code on error. If an error code is
> + * returned then it means the plane state shouldn't update with attached fb.
> + */
> +int
> +drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
> + const struct drm_plane_state *old_state,
> + const struct drm_plane_state *new_state)
> +{
> + if (!new_state || !new_state->crtc || !new_state->fb)
> + return -EINVAL;
> +
> + if (!new_state->visible)
> + return -EINVAL;
Can't we handle this by iterating 0 damage rects instead? Would make the
code a bit cleaner I think.
> +
> + memset(iter, 0, sizeof(*iter));
> + iter->clips = drm_plane_get_damage_clips(new_state);
> + iter->num_clips = drm_plane_get_damage_clips_count(new_state);
> +
> + if (!iter->clips)
> + iter->full_update = true;
> +
> + if (!drm_rect_equals(&new_state->src, &old_state->src))
> + iter->full_update = true;
> +
> + iter->plane_src.x1 = convert_fixed_to_32(new_state->src.x1);
> + iter->plane_src.y1 = convert_fixed_to_32(new_state->src.y1);
> + iter->plane_src.x2 = convert_fixed_to_32(new_state->src.x2);
> + iter->plane_src.y2 = convert_fixed_to_32(new_state->src.y2);
I think you want to clip with the clipped rectangles here, not with the
ones userspace provides.
Also I think you're rounding is wrong here - I think you need to round
down for x/y1, and round up for x/y2 to make sure you catch all the
pixels?
Unit tests for this, in the form of a drm selftest (like we have for
drm_mm.c already, but for kms helpers) would be perfect. Much easier to
review a testcase than do bitmath in my head :-)
> +
> + if (iter->full_update) {
> + iter->clips = 0;
> + iter->curr_clip = 0;
> + iter->num_clips = 0;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_damage_iter_init);
> +
> +/**
> + * drm_atomic_helper_damage_iter_next - advance the damage iterator
> + * @iter: The iterator to advance.
> + * @rect: Return a rectangle in fb coordinate clipped to plane src.
> + *
> + * Returns: true if the output is valid, false if we've reached the end of the
> + * rectangle list.
> + */
> +bool
> +drm_atomic_helper_damage_iter_next(struct drm_atomic_helper_damage_iter *iter,
> + struct drm_rect *rect)
> +{
> + bool ret = false;
> +
> + if (iter->full_update) {
> + *rect = iter->plane_src;
> + iter->full_update = false;
> + return true;
> + }
> +
> + while (iter->curr_clip < iter->num_clips) {
> + *rect = iter->clips[iter->curr_clip];
> + iter->curr_clip++;
> +
> + if (drm_rect_intersect(rect, &iter->plane_src)) {
> + ret = true;
> + break;
> + }
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_damage_iter_next);
> diff --git a/include/drm/drm_damage_helper.h b/include/drm/drm_damage_helper.h
> index 217694e9168c..f1282b459a4f 100644
> --- a/include/drm/drm_damage_helper.h
> +++ b/include/drm/drm_damage_helper.h
> @@ -32,6 +32,19 @@
> #ifndef DRM_DAMAGE_HELPER_H_
> #define DRM_DAMAGE_HELPER_H_
>
> +/**
> + * struct drm_atomic_helper_damage_iter - damage clip iterator
> + *
> + * This iterator tracks state needed to walk the list of damage clips.
> + */
> +struct drm_atomic_helper_damage_iter {
> + const struct drm_rect *clips;
> + struct drm_rect plane_src;
> + uint32_t num_clips;
> + uint32_t curr_clip;
> + bool full_update;
> +};
> +
> /**
> * drm_plane_get_damage_clips_count - returns damage clips count
> * @state: Plane state
> @@ -59,5 +72,12 @@ drm_plane_get_damage_clips(const struct drm_plane_state *state)
> }
>
> void drm_plane_enable_fb_damage_clips(struct drm_plane *plane);
> +int
> +drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
> + const struct drm_plane_state *old_state,
> + const struct drm_plane_state *new_state);
I think a for_each_damage macro would be sweet on top of these here. But
easy to add later on.
-Daniel
> +bool
> +drm_atomic_helper_damage_iter_next(struct drm_atomic_helper_damage_iter *iter,
> + struct drm_rect *rect);
>
> #endif
> --
> 2.17.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
