[PATCH v3 01/18] drm: Add a new plane property to send damage during plane update
Thomas Hellstrom
thellstrom at vmware.com
Mon Oct 15 13:15:28 UTC 2018
With some nitpicks sent out previously
Reviewed-by: Thomas Hellstrom <thellstrom at vmware.com>
On 10/11/2018 02:16 AM, Deepak Rawat wrote:
> From: Lukasz Spintzyk <lukasz.spintzyk at displaylink.com>
>
> FB_DAMAGE_CLIPS is an optional plane property to mark damaged regions
> on the plane in framebuffer coordinates of the framebuffer attached to
> the plane.
>
> The layout of blob data is simply an array of "struct drm_mode_rect".
> Unlike plane src coordinates, damage clips are not in 16.16 fixed point.
> As plane src in framebuffer cannot be negative so are damage clips. In
> damage clip, x1/y1 are inclusive and x2/y2 are exclusive.
>
> This patch also exports the kernel internal drm_rect to userspace as
> drm_mode_rect. This is because "struct drm_clip_rect" is not sufficient
> to represent damage for current plane size.
>
> Driver which are interested in enabling FB_DAMAGE_CLIPS property for a
> plane should enable this property using drm_plane_enable_damage_clips.
>
> v2:
> - Input validation on damage clips against framebuffer size.
> - Doc update, other minor changes.
>
> Cc: ville.syrjala at linux.intel.com
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Pekka Paalanen <ppaalanen at gmail.com>
> Cc: Daniel Stone <daniel at fooishbar.org>
> Signed-off-by: Lukasz Spintzyk <lukasz.spintzyk at displaylink.com>
> Signed-off-by: Deepak Rawat <drawat at vmware.com>
> ---
> Documentation/gpu/drm-kms.rst | 12 +++++
> drivers/gpu/drm/Makefile | 3 +-
> drivers/gpu/drm/drm_atomic.c | 22 ++++++++
> drivers/gpu/drm/drm_atomic_helper.c | 3 ++
> drivers/gpu/drm/drm_atomic_uapi.c | 13 +++++
> drivers/gpu/drm/drm_damage_helper.c | 83 +++++++++++++++++++++++++++++
> drivers/gpu/drm/drm_mode_config.c | 6 +++
> include/drm/drm_damage_helper.h | 39 ++++++++++++++
> include/drm/drm_mode_config.h | 9 ++++
> include/drm/drm_plane.h | 40 ++++++++++++++
> include/uapi/drm/drm_mode.h | 19 +++++++
> 11 files changed, 248 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/drm_damage_helper.c
> create mode 100644 include/drm/drm_damage_helper.h
>
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 4b1501b4835b..6c3e89e324f8 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -554,6 +554,18 @@ Plane Composition Properties
> .. kernel-doc:: drivers/gpu/drm/drm_blend.c
> :export:
>
> +FB_DAMAGE_CLIPS
> +~~~~~~~~~~~~~~~
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_damage_helper.c
> + :doc: overview
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_damage_helper.c
> + :export:
> +
> +.. kernel-doc:: include/drm/drm_damage_helper.h
> + :internal:
> +
> Color Management Properties
> ---------------------------
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index bc6a16a3c36e..2ed4c66ca849 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -36,7 +36,8 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
> drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
> drm_kms_helper_common.o drm_dp_dual_mode_helper.o \
> drm_simple_kms_helper.o drm_modeset_helper.o \
> - drm_scdc_helper.o drm_gem_framebuffer_helper.o
> + drm_scdc_helper.o drm_gem_framebuffer_helper.o \
> + drm_damage_helper.o
>
> drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
> drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 2870ae205237..ff488b47b5bb 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -516,6 +516,8 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
> struct drm_plane_state *state)
> {
> unsigned int fb_width, fb_height;
> + struct drm_mode_rect *clips;
> + uint32_t num_clips;
> int ret;
>
> /* either *both* CRTC and FB must be set, or neither */
> @@ -585,6 +587,26 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
> return -ENOSPC;
> }
>
> + clips = drm_plane_get_damage_clips(state);
> + num_clips = drm_plane_get_damage_clips_count(state);
> +
> + /* Make sure damage clips are valid and inside the fb. */
> + while (num_clips > 0) {
> + if (clips->x1 >= clips->x2 ||
> + clips->y1 >= clips->y2 ||
> + clips->x1 < 0 ||
> + clips->y1 < 0 ||
> + clips->x2 > fb_width ||
> + clips->y2 > fb_height) {
> + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] invalid damage clip %d %d %d %d\n",
> + plane->base.id, plane->name, clips->x1,
> + clips->y1, clips->x2, clips->y2);
> + return -EINVAL;
> + }
> + clips++;
> + num_clips--;
> + }
> +
> if (plane_switching_crtc(state->state, plane, state)) {
> DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
> plane->base.id, plane->name);
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index e49b22381048..35395577ca86 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3613,6 +3613,7 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>
> state->fence = NULL;
> state->commit = NULL;
> + state->fb_damage_clips = NULL;
> }
> EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
>
> @@ -3657,6 +3658,8 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
>
> if (state->commit)
> drm_crtc_commit_put(state->commit);
> +
> + drm_property_blob_put(state->fb_damage_clips);
> }
> EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index d5b7f315098c..b83b96fe887e 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -513,6 +513,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
> {
> struct drm_device *dev = plane->dev;
> struct drm_mode_config *config = &dev->mode_config;
> + bool replaced = false;
> + int ret;
>
> if (property == config->prop_fb_id) {
> struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL, val);
> @@ -566,6 +568,14 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
> state->color_encoding = val;
> } else if (property == plane->color_range_property) {
> state->color_range = val;
> + } else if (property == config->prop_fb_damage_clips) {
> + ret = drm_atomic_replace_property_blob_from_id(dev,
> + &state->fb_damage_clips,
> + val,
> + -1,
> + sizeof(struct drm_rect),
> + &replaced);
> + return ret;
> } else if (plane->funcs->atomic_set_property) {
> return plane->funcs->atomic_set_property(plane, state,
> property, val);
> @@ -621,6 +631,9 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> *val = state->color_encoding;
> } else if (property == plane->color_range_property) {
> *val = state->color_range;
> + } else if (property == config->prop_fb_damage_clips) {
> + *val = (state->fb_damage_clips) ?
> + state->fb_damage_clips->base.id : 0;
> } else if (plane->funcs->atomic_get_property) {
> return plane->funcs->atomic_get_property(plane, state, property, val);
> } else {
> diff --git a/drivers/gpu/drm/drm_damage_helper.c b/drivers/gpu/drm/drm_damage_helper.c
> new file mode 100644
> index 000000000000..8dc906a489a9
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_damage_helper.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/**************************************************************************
> + *
> + * Copyright (c) 2018 VMware, Inc., Palo Alto, CA., USA
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * Authors:
> + * Deepak Rawat <drawat at vmware.com>
> + *
> + **************************************************************************/
> +
> +#include <drm/drm_damage_helper.h>
> +
> +/**
> + * DOC: overview
> + *
> + * FB_DAMAGE_CLIPS is an optional plane property which provides a means to
> + * specify a list of damage rectangles on a plane in framebuffer coordinates of
> + * the framebuffer attached to the plane. In current context damage is the area
> + * of plane framebuffer that has changed since last plane update (also called
> + * page-flip), irrespective of whether currently attached framebuffer is same as
> + * framebuffer attached during last plane update or not.
> + *
> + * FB_DAMAGE_CLIPS is a hint to kernel which could be helpful for some drivers
> + * to optimize internally especially for virtual devices where each framebuffer
> + * change needs to be transmitted over network, usb, etc.
> + *
> + * Since FB_DAMAGE_CLIPS is a hint so it is an optional property. User-space can
> + * ignore damage clips property and in that case driver will do a full plane
> + * update. In case damage clips are provided then it is guaranteed that the area
> + * inside damage clips will be updated to plane. For efficiency driver can do
> + * full update or can update more than specified in damage clips. Since driver
> + * is free to read more, user-space must always render the entire visible
> + * framebuffer. Otherwise there can be corruptions. Also, if a user-space
> + * provides damage clips which doesn't encompass the actual damage to
> + * framebuffer (since last plane update) can result in incorrect rendering.
> + *
> + * FB_DAMAGE_CLIPS is a blob property with the layout of blob data is simply an
> + * array of &drm_mode_rect. Unlike plane &drm_plane_state.src coordinates,
> + * damage clips are not in 16.16 fixed point. Similar to plane src in
> + * framebuffer, damage clips cannot be negative. In damage clip, x1/y1 are
> + * inclusive and x2/y2 are exclusive. While kernel does not error for overlapped
> + * damage clips, it is strongly discouraged.
> + *
> + * Drivers that are interested in damage interface for plane should enable
> + * FB_DAMAGE_CLIPS property by calling drm_plane_enable_fb_damage_clips().
> + */
> +
> +/**
> + * drm_plane_enable_fb_damage_clips - Enables plane fb damage clips property.
> + * @plane: Plane on which to enable damage clips property.
> + *
> + * This function lets driver to enable the damage clips property on a plane.
> + */
> +void drm_plane_enable_fb_damage_clips(struct drm_plane *plane)
> +{
> + struct drm_device *dev = plane->dev;
> + struct drm_mode_config *config = &dev->mode_config;
> +
> + drm_object_attach_property(&plane->base, config->prop_fb_damage_clips,
> + 0);
> +}
> +EXPORT_SYMBOL(drm_plane_enable_fb_damage_clips);
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index ee80788f2c40..05cd5e9857e4 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -297,6 +297,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
> return -ENOMEM;
> dev->mode_config.prop_crtc_id = prop;
>
> + prop = drm_property_create(dev, DRM_MODE_PROP_BLOB, "FB_DAMAGE_CLIPS",
> + 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.prop_fb_damage_clips = prop;
> +
> prop = drm_property_create_bool(dev, DRM_MODE_PROP_ATOMIC,
> "ACTIVE");
> if (!prop)
> diff --git a/include/drm/drm_damage_helper.h b/include/drm/drm_damage_helper.h
> new file mode 100644
> index 000000000000..4947c614fff9
> --- /dev/null
> +++ b/include/drm/drm_damage_helper.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR MIT */
> +/**************************************************************************
> + *
> + * Copyright (c) 2018 VMware, Inc., Palo Alto, CA., USA
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * Authors:
> + * Deepak Rawat <drawat at vmware.com>
> + *
> + **************************************************************************/
> +
> +#ifndef DRM_DAMAGE_HELPER_H_
> +#define DRM_DAMAGE_HELPER_H_
> +
> +#include <drm/drm_atomic_helper.h>
> +
> +void drm_plane_enable_fb_damage_clips(struct drm_plane *plane);
> +
> +#endif
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 928e4172a0bb..073649ba516f 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -627,6 +627,15 @@ struct drm_mode_config {
> * &drm_crtc.
> */
> struct drm_property *prop_crtc_id;
> + /**
> + * @prop_fb_damage_clips: Optional plane property to mark damaged
> + * regions on the plane in framebuffer coordinates of the framebuffer
> + * attached to the plane.
> + *
> + * The layout of blob data is simply an array of &drm_mode_rect. Unlike
> + * plane src coordinates, damage clips are not in 16.16 fixed point.
> + */
> + struct drm_property *prop_fb_damage_clips;
> /**
> * @prop_active: Default atomic CRTC property to control the active
> * state, which is the simplified implementation for DPMS in atomic
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 0a0834bef8bd..66f187e4ecfc 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -173,6 +173,16 @@ struct drm_plane_state {
> */
> enum drm_color_range color_range;
>
> + /**
> + * @fb_damage_clips:
> + *
> + * Blob representing damage (area in plane framebuffer that changed
> + * 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.
> + */
> + struct drm_property_blob *fb_damage_clips;
> +
> /** @src: clipped source coordinates of the plane (in 16.16) */
> /** @dst: clipped destination coordinates of the plane */
> struct drm_rect src, dst;
> @@ -798,5 +808,35 @@ static inline struct drm_plane *drm_plane_find(struct drm_device *dev,
> #define drm_for_each_plane(plane, dev) \
> list_for_each_entry(plane, &(dev)->mode_config.plane_list, head)
>
> +/**
> + * 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;
> +}
> +
> +/**
> + * drm_plane_get_damage_clips - Returns damage clips.
> + * @state: Plane state.
> + *
> + * Note that this function returns uapi type &drm_mode_rect.
> + *
> + * 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);
> +}
>
> #endif
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index d3e0fe31efc5..a439c2e67896 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -888,6 +888,25 @@ struct drm_mode_revoke_lease {
> __u32 lessee_id;
> };
>
> +/**
> + * struct drm_mode_rect - Two dimensional rectangle.
> + * @x1: Horizontal starting coordinate (inclusive).
> + * @y1: Vertical starting coordinate (inclusive).
> + * @x2: Horizontal ending coordinate (exclusive).
> + * @y2: Vertical ending coordinate (exclusive).
> + *
> + * With drm subsystem using struct drm_rect to manage rectangular area this
> + * export it to user-space.
> + *
> + * Currently used by drm_mode_atomic blob property FB_DAMAGE_CLIPS.
> + */
> +struct drm_mode_rect {
> + __s32 x1;
> + __s32 y1;
> + __s32 x2;
> + __s32 y2;
> +};
> +
> #if defined(__cplusplus)
> }
> #endif
More information about the dri-devel
mailing list