Re: [RFC 2/3] drm: Add helper iterator functions to iterate over plane damage.

[Sinclair Yeh]

On Wed, Apr 04, 2018 at 04:49:07PM -0700, Deepak Rawat wrote:
> With damage property in drm_plane_state, this patch adds helper iterator
> to traverse the damage clips. Iterator will return the damage rectangles
> in framebuffer, plane or crtc coordinates as need by driver
> implementation.
> 
> Signed-off-by: Deepak Rawat <drawat@xxxxxxxxxx>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 122 ++++++++++++++++++++++++++++++++++++
>  include/drm/drm_atomic_helper.h     |  39 ++++++++++++
>  2 files changed, 161 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 55b44e3..355b514 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3865,3 +3865,125 @@ void __drm_atomic_helper_private_obj_duplicate_state(struct drm_private_obj *obj
>  	memcpy(state, obj->state, sizeof(*state));
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_private_obj_duplicate_state);
> +
> +/**
> + * drm_atomic_helper_damage_iter_init - initialize the damage iterator
> + * @iter: The iterator to initialize.
> + * @type: Coordinate type caller is interested in.
> + * @state: plane_state from which to iterate the damage clips.
> + * @hdisplay: Width of crtc on which plane is scanned out.
> + * @vdisplay: Height of crtc on which plane is scanned out.
> + *
> + * Initialize an iterator that is used to translate and clip a set of damage
> + * rectangles in framebuffer coordinates to plane and crtc coordinates. The type
> + * argument specify which type of coordinate to iterate in.
> + *
> + * Returns: 0 on success and negative error code on error. If an error code is
> + * returned then it means the plane state should not update.
> + */
> +int
> +drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
> +				   enum drm_atomic_helper_damage_clip_type type,
> +				   const struct drm_plane_state *state,
> +				   uint32_t hdisplay, uint32_t vdisplay)
> +{
> +	if (!state || !state->crtc || !state->fb)
> +		return -EINVAL;
> +
> +	memset(iter, 0, sizeof(*iter));
> +	iter->clips = (struct drm_rect *)state->damage_clips->data;
> +	iter->num_clips = state->num_clips;
> +	iter->type = type;
> +
> +	/*
> +	 * Full update in case of scaling or rotation. In future support for
> +	 * scaling/rotating damage clips can be added
> +	 */
> +	if (state->crtc_w != (state->src_w >> 16) ||
> +	    state->crtc_h != state->src_h >> 16 || state->rotation != 0) {
> +		iter->curr_clip = iter->num_clips;
> +		return 0;
> +	}
> +
> +	iter->fb_src.x1 = 0;
> +	iter->fb_src.y1 = 0;
> +	iter->fb_src.x2 = state->fb->width;
> +	iter->fb_src.y2 = state->fb->height;
> +
> +	iter->plane_src.x1 = state->src_x >> 16;
> +	iter->plane_src.y1 = state->src_y >> 16;
> +	iter->plane_src.x2 = iter->plane_src.x1 + (state->src_w >> 16);
> +	iter->plane_src.y2 = iter->plane_src.y1 + (state->src_h >> 16);
> +	iter->translate_plane_x = -iter->plane_src.x1;
> +	iter->translate_plane_y = -iter->plane_src.y1;
> +
> +	/* Clip plane src rect to fb dimensions */
> +	drm_rect_intersect(&iter->plane_src, &iter->fb_src);
> +
> +	iter->crtc_src.x1 = 0;
> +	iter->crtc_src.y1 = 0;
> +	iter->crtc_src.x2 = hdisplay;
> +	iter->crtc_src.y2 = vdisplay;
> +	iter->translate_crtc_x = -(iter->plane_src.x1 - state->crtc_x);
> +	iter->translate_crtc_x = -(iter->plane_src.y1 - state->crtc_y);
                           ^
I believe you mean translate_crtc_y here


> +
> +	/* Clip crtc src rect to plane dimensions */
> +	drm_rect_translate(&iter->crtc_src, -iter->translate_crtc_x,
> +			   -iter->translate_crtc_x);
Also here                            ^