On Fri, Jul 23, 2021 at 10:34:56AM +0200, Daniel Vetter wrote:
> There's two stages of manual upload/invalidate displays:
> - just handling dirtyfb and uploading the entire fb all the time
> - looking at damage clips
>
> In the latter case we support it through fbdev emulation (with
> fb_defio), atomic property, and with the dirtfy clip rects.
>
> Make sure at least the atomic property is set up as the main official
> interface for this. Ideally we'd also check that
> drm_atomic_helper_dirtyfb() is used and that fbdev defio is set up,
> but that's quite a bit harder to do. Ideas very much welcome.
>
> From a cursor audit drivers seem to be getting this right mostly, but
> better to make sure. At least no one is bypassing the accessor
> function.
>
> v2:
> - use drm_warn_once with a meaningful warning string (José)
> - don't splat in the atomic check code for everyone (intel-gfx-ci)
v2 got rid of the false positive noise, going to push the series to
drm-misc-next.
-Daniel
>
> Reviewed-by: José Roberto de Souza <jose.souza@xxxxxxxxx> (v1)
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx>
> Cc: José Roberto de Souza <jose.souza@xxxxxxxxx>
> Cc: Hans de Goede <hdegoede@xxxxxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Cc: Maxime Ripard <mripard@xxxxxxxxxx>
> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
> Cc: David Airlie <airlied@xxxxxxxx>
> Cc: Daniel Vetter <daniel@xxxxxxxx>
> ---
> drivers/gpu/drm/drm_atomic.c | 2 +-
> drivers/gpu/drm/drm_crtc_internal.h | 2 ++
> drivers/gpu/drm/drm_plane.c | 50 +++++++++++++++++++++++++++++
> include/drm/drm_plane.h | 36 +++------------------
> 4 files changed, 57 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index d820423fac32..c85dcfd69158 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -660,7 +660,7 @@ static int drm_atomic_plane_check(const struct drm_plane_state *old_plane_state,
> return -ENOSPC;
> }
>
> - clips = drm_plane_get_damage_clips(new_plane_state);
> + clips = __drm_plane_get_damage_clips(new_plane_state);
> num_clips = drm_plane_get_damage_clips_count(new_plane_state);
>
> /* Make sure damage clips are valid and inside the fb. */
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
> index 1ca51addb589..edb772947cb4 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -262,6 +262,8 @@ int drm_plane_register_all(struct drm_device *dev);
> void drm_plane_unregister_all(struct drm_device *dev);
> int drm_plane_check_pixel_format(struct drm_plane *plane,
> u32 format, u64 modifier);
> +struct drm_mode_rect *
> +__drm_plane_get_damage_clips(const struct drm_plane_state *state);
>
> /* drm_bridge.c */
> void drm_bridge_detach(struct drm_bridge *bridge);
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index b373958ecb30..f61315b61174 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -1397,6 +1397,56 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
> return ret;
> }
>
> +/**
> + * drm_plane_get_damage_clips_count - Returns damage clips count.
> + * @state: Plane state.
> + *
> + * Simple helper to get the number of &drm_mode_rect clips set by user-space
> + * during plane update.
> + *
> + * Return: Number of clips in plane fb_damage_clips blob property.
> + */
> +unsigned int
> +drm_plane_get_damage_clips_count(const struct drm_plane_state *state)
> +{
> + return (state && state->fb_damage_clips) ?
> + state->fb_damage_clips->length/sizeof(struct drm_mode_rect) : 0;
> +}
> +EXPORT_SYMBOL(drm_plane_get_damage_clips_count);
> +
> +struct drm_mode_rect *
> +__drm_plane_get_damage_clips(const struct drm_plane_state *state)
> +{
> + return (struct drm_mode_rect *)((state && state->fb_damage_clips) ?
> + state->fb_damage_clips->data : NULL);
> +}
> +
> +/**
> + * drm_plane_get_damage_clips - Returns damage clips.
> + * @state: Plane state.
> + *
> + * 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.
> + */
> +struct drm_mode_rect *
> +drm_plane_get_damage_clips(const struct drm_plane_state *state)
> +{
> + struct drm_device *dev = state->plane->dev;
> + struct drm_mode_config *config = &dev->mode_config;
> +
> + /* check that drm_plane_enable_fb_damage_clips() was called */
> + if (!drm_mode_obj_find_prop_id(&state->plane->base,
> + config->prop_fb_damage_clips->base.id))
> + drm_warn_once(dev, "drm_plane_enable_fb_damage_clips() not called\n");
> +
> + return __drm_plane_get_damage_clips(state);
> +}
> +EXPORT_SYMBOL(drm_plane_get_damage_clips);
> +
> struct drm_property *
> drm_create_scaling_filter_prop(struct drm_device *dev,
> unsigned int supported_filters)
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 7f7d5148310c..a2684aab8372 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -897,39 +897,11 @@ static inline struct drm_plane *drm_plane_find(struct drm_device *dev,
>
> bool drm_any_plane_has_format(struct drm_device *dev,
> u32 format, u64 modifier);
> -/**
> - * drm_plane_get_damage_clips_count - Returns damage clips count.
> - * @state: Plane state.
> - *
> - * Simple helper to get the number of &drm_mode_rect clips set by user-space
> - * during plane update.
> - *
> - * Return: Number of clips in plane fb_damage_clips blob property.
> - */
> -static inline unsigned int
> -drm_plane_get_damage_clips_count(const struct drm_plane_state *state)
> -{
> - return (state && state->fb_damage_clips) ?
> - state->fb_damage_clips->length/sizeof(struct drm_mode_rect) : 0;
> -}
> +unsigned int
> +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 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.
> - */
> -static inline struct drm_mode_rect *
> -drm_plane_get_damage_clips(const struct drm_plane_state *state)
> -{
> - return (struct drm_mode_rect *)((state && state->fb_damage_clips) ?
> - state->fb_damage_clips->data : NULL);
> -}
> +struct drm_mode_rect *
> +drm_plane_get_damage_clips(const struct drm_plane_state *state);
>
> int drm_plane_create_scaling_filter_property(struct drm_plane *plane,
> unsigned int supported_filters);
> --
> 2.32.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
