diff mbox series

[02/14] drm: add helper iterator functions for plane fb_damage_clips blob

Message ID 20180905233901.2321-3-drawat@vmware.com (mailing list archive)
State New, archived
Headers show
Series plane update with damage | expand

Commit Message

Deepak Singh Rawat Sept. 5, 2018, 11:38 p.m. UTC
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@vmware.com>
---
 drivers/gpu/drm/drm_damage_helper.c | 93 +++++++++++++++++++++++++++++
 include/drm/drm_damage_helper.h     | 20 +++++++
 2 files changed, 113 insertions(+)

Comments

Daniel Vetter Sept. 6, 2018, 7:51 a.m. UTC | #1
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@vmware.com>
> ---
>  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
>
Deepak Singh Rawat Sept. 6, 2018, 9:44 p.m. UTC | #2
> >  #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.

Agreed.

> 
> > +
> > +	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.

new_state->src.x1 is the clipped one, clipped in the function
drm_atomic_helper_check_plane_state. Or am I missing something ?

> 
> 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 :-)

Sure I will work on a unit test case for this scenario and work on round up
of plane_src.

> 
> > +
> > +	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
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ff
> wll.ch&amp;data=02%7C01%7Cdrawat%40vmware.com%7C117739774a7341f
> d8c0208d613cd83d5%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C
> 636718170756943488&amp;sdata=tNK2zTqJrq8U5U4W96wp49FZ8LAB9fl2WV
> UixbOSWsU%3D&amp;reserved=0
Daniel Vetter Sept. 7, 2018, 7:41 p.m. UTC | #3
On Thu, Sep 06, 2018 at 09:44:25PM +0000, Deepak Singh Rawat wrote:
> 
> > >  #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.
> 
> Agreed.
> 
> > 
> > > +
> > > +	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.
> 
> new_state->src.x1 is the clipped one, clipped in the function
> drm_atomic_helper_check_plane_state. Or am I missing something ?

You're right, I misremembered. This looks correct. Would be good to
explain this in the kerneldoc, and that you must call
drm_atomic_helper_check_plane_state() first before calling this.

Cheers, Daniel


> > 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 :-)
> 
> Sure I will work on a unit test case for this scenario and work on round up
> of plane_src.
> 
> > 
> > > +
> > > +	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
> > https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ff
> > wll.ch&amp;data=02%7C01%7Cdrawat%40vmware.com%7C117739774a7341f
> > d8c0208d613cd83d5%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C
> > 636718170756943488&amp;sdata=tNK2zTqJrq8U5U4W96wp49FZ8LAB9fl2WV
> > UixbOSWsU%3D&amp;reserved=0
diff mbox series

Patch

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;
+
+	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);
+
+	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);
+bool
+drm_atomic_helper_damage_iter_next(struct drm_atomic_helper_damage_iter *iter,
+				   struct drm_rect *rect);
 
 #endif