Re: [Intel-gfx] [PATCH 1/3] drm/plane: remove drm_helper_get_plane_damage_clips

["Souza, Jose" <jose.souza@intel.com>]

On Wed, 2021-07-21 at 15:30 +0200, Daniel Vetter wrote:
> It's not used. Drivers should instead use the helpers anyway.
> 
> Currently both vbox and i915 hand-roll this and it's not the greatest.
> vbox looks buggy, and i915 does a bit much that helpers would take
> care of I think.
> 
> Also improve the kerneldocs while we're at it.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_damage_helper.c |  2 +-
>  include/drm/drm_damage_helper.h     | 17 -----------------
>  include/drm/drm_plane.h             | 10 +++++++---
>  include/drm/drm_rect.h              |  3 +++
>  4 files changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_damage_helper.c b/drivers/gpu/drm/drm_damage_helper.c
> index 3a4126dc2520..eb69b7123af5 100644
> --- a/drivers/gpu/drm/drm_damage_helper.c
> +++ b/drivers/gpu/drm/drm_damage_helper.c
> @@ -282,7 +282,7 @@ drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
>  	if (!state || !state->crtc || !state->fb || !state->visible)
>  		return;
>  
> -	iter->clips = drm_helper_get_plane_damage_clips(state);
> +	iter->clips = (struct drm_rect *)drm_plane_get_damage_clips(state);
>  	iter->num_clips = drm_plane_get_damage_clips_count(state);
>  
>  	/* Round down for x1/y1 and round up for x2/y2 to catch all pixels */
> diff --git a/include/drm/drm_damage_helper.h b/include/drm/drm_damage_helper.h
> index 40c34a5bf149..1ae8bce6a5ce 100644
> --- a/include/drm/drm_damage_helper.h
> +++ b/include/drm/drm_damage_helper.h
> @@ -82,21 +82,4 @@ bool drm_atomic_helper_damage_merged(const struct drm_plane_state *old_state,
>  				     struct drm_plane_state *state,
>  				     struct drm_rect *rect);
>  
> -/**
> - * drm_helper_get_plane_damage_clips - Returns damage clips in &drm_rect.
> - * @state: Plane state.
> - *
> - * Returns plane damage rectangles in internal &drm_rect. Currently &drm_rect
> - * can be obtained by simply typecasting &drm_mode_rect. This is because both
> - * are signed 32 and during drm_atomic_check_only() it is verified that damage
> - * clips are inside fb.
> - *
> - * Return: Clips in plane fb_damage_clips blob property.
> - */
> -static inline struct drm_rect *
> -drm_helper_get_plane_damage_clips(const struct drm_plane_state *state)
> -{
> -	return (struct drm_rect *)drm_plane_get_damage_clips(state);
> -}
> -
>  #endif
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 1294610e84f4..7f7d5148310c 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -186,6 +186,9 @@ struct drm_plane_state {
>  	 * since last plane update) as an array of &drm_mode_rect in framebuffer
>  	 * coodinates of the attached framebuffer. Note that unlike plane src,
>  	 * damage clips are not in 16.16 fixed point.
> +	 *
> +	 * See drm_plane_get_damage_clips() and
> +	 * drm_plane_get_damage_clips_count() for accessing these.
>  	 */
>  	struct drm_property_blob *fb_damage_clips;
>  
> @@ -914,9 +917,10 @@ drm_plane_get_damage_clips_count(const struct drm_plane_state *state)
>   * drm_plane_get_damage_clips - Returns damage clips.
>   * @state: Plane state.
>   *
> - * Note that this function returns uapi type &drm_mode_rect. Drivers might
> - * instead be interested in internal &drm_rect which can be obtained by calling
> - * drm_helper_get_plane_damage_clips().
> + * Note that this function returns uapi type &drm_mode_rect. Drivers might want
> + * to use the helper functions drm_atomic_helper_damage_iter_init() and
> + * drm_atomic_helper_damage_iter_next() or drm_atomic_helper_damage_merged() if
> + * the driver can only handle a single damage region at most.
>   *
>   * Return: Damage clips in plane fb_damage_clips blob property.
>   */
> diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
> index 39f2deee709c..6f6e19bd4dac 100644
> --- a/include/drm/drm_rect.h
> +++ b/include/drm/drm_rect.h
> @@ -39,6 +39,9 @@
>   * @x2: horizontal ending coordinate (exclusive)
>   * @y1: vertical starting coordinate (inclusive)
>   * @y2: vertical ending coordinate (exclusive)
> + *
> + * Note that this must match the layout of struct drm_mode_rect or the damage
> + * helpers like drm_atomic_helper_damage_iter_init() break.
>   */
>  struct drm_rect {
>  	int x1, y1, x2, y2;

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx