[PATCH 1/1] drm: Add dirty_rects atomic blob property for drm_plane

[Thomas Hellstrom]

Hi, Lukasz!

(Sorry for top-posting).

We have Deepak from our team working on the same subject. I think he's 
in over the holidays so I'll add him to the CC list.

Adding damage to the plane state is, IMO the correct way to do it. 
However, from your proposal it's not clear whether damage is given in 
the plane-, crtc- or framebuffer coordinates. The last conclusion from 
our email thread discussion was that they should be given in framebuffer 
coordinates with helpers to compute plane coordinates or crtc 
coordinates. The reason being that it's easier for user-space apps to 
send damage that way, and again, we have the full information that we 
can clip and scale if necessary. Most drivers probably want the damage 
in clipped plane- or crtc coordinates. Helpers could for example be in 
the form of region iterators.

Full (multi-rect) damage regions are OK with us, although we should keep 
in mind that we won't be able to compute region unions in the kernel 
(yet?). Implying: Should we forbid overlapping rects at the interface 
level or should we just recommend rects not to be overlapping? The 
former would be pretty hard to enforce efficiently.

Another thing we should think about is how to use this interface for the 
legacy "dirtyfb" call. Probably we need to clear the damage property on 
each state-update, or set a flag that "this is a dirtyfb state update".

IMO we should also have as an end goal of this work to have gnome-shell 
on drm sending damage regions on page-flip, which means either porting 
gnome-shell to atomic or set up a new legacy page-flip-with-atomic ioctl.

/Thomas


On 12/21/2017 12:10 PM, Lukasz Spintzyk wrote:
> Change-Id: I63dce004f8d3c5dc6a7c71070f1fab0707286ea5
> Signed-off-by: Lukasz Spintzyk <lukasz.spintzyk at displaylink.com>
> ---
>   drivers/gpu/drm/drm_atomic.c      | 10 ++++++++++
>   drivers/gpu/drm/drm_mode_config.c |  6 ++++++
>   drivers/gpu/drm/drm_plane.c       |  1 +
>   include/drm/drm_mode_config.h     |  5 +++++
>   include/drm/drm_plane.h           |  3 +++
>   5 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index b76d49218cf1..cd3b4ed7b04c 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -759,6 +759,14 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>   		state->rotation = val;
>   	} else if (property == plane->zpos_property) {
>   		state->zpos = val;
> +	} else if (property == config->dirty_rects_property) {
> +		bool replaced = false;
> +		int ret = drm_atomic_replace_property_blob_from_id(dev,
> +					&state->dirty_blob,
> +					val,
> +					-1,
> +					&replaced);
> +		return ret;
>   	} else if (plane->funcs->atomic_set_property) {
>   		return plane->funcs->atomic_set_property(plane, state,
>   				property, val);
> @@ -818,6 +826,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>   		*val = state->rotation;
>   	} else if (property == plane->zpos_property) {
>   		*val = state->zpos;
> +	} else if (property == config->dirty_rects_property) {
> +		*val = (state->dirty_blob) ? state->dirty_blob->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_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index bc5c46306b3d..d5f1021c6ece 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -293,6 +293,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,
> +			"DIRTY_RECTS", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.dirty_rects_property = prop;
> +
>   	prop = drm_property_create_bool(dev, DRM_MODE_PROP_ATOMIC,
>   			"ACTIVE");
>   	if (!prop)
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 37a93cdffb4a..add110f025e5 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -258,6 +258,7 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
>   		drm_object_attach_property(&plane->base, config->prop_src_y, 0);
>   		drm_object_attach_property(&plane->base, config->prop_src_w, 0);
>   		drm_object_attach_property(&plane->base, config->prop_src_h, 0);
> +		drm_object_attach_property(&plane->base, config->dirty_rects_property, 0);
>   	}
>   
>   	if (config->allow_fb_modifiers)
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index e5f3b43014e1..65f64eb04c0c 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -599,6 +599,11 @@ struct drm_mode_config {
>   	 * &drm_crtc.
>   	 */
>   	struct drm_property *prop_crtc_id;
> +	/**
> +	 * @dirty_rects_property: Optional plane property to mark damaged
> +	 * regions on the plane framebuffer.
> +	 */
> +	struct drm_property *dirty_rects_property;
>   	/**
>   	 * @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 8185e3468a23..7d45b164ccce 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -131,6 +131,9 @@ struct drm_plane_state {
>   	 */
>   	struct drm_crtc_commit *commit;
>   
> +	/* Optional blob property with damaged regions. */
> +	struct drm_property_blob *dirty_blob;
> +
>   	struct drm_atomic_state *state;
>   };
>