From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukasz Spintzyk Subject: Re: [RFC 1/3] drm: Add DAMAGE_CLIPS property to plane Date: Tue, 10 Apr 2018 10:10:40 +0200 Message-ID: <0e7c70bc-6587-41ea-263a-68018bcb7db2@displaylink.com> References: <1522885748-67122-1-git-send-email-drawat@vmware.com> <1522885748-67122-2-git-send-email-drawat@vmware.com> Mime-Version: 1.0 Content-Type: text/plain; charset=WINDOWS-1252; format=flowed Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <1522885748-67122-2-git-send-email-drawat@vmware.com> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Deepak Rawat , "dri-devel@lists.freedesktop.org" , "thellstrom@vmware.com" , "syeh@vmware.com" Cc: "linux-graphics-maintainer@vmware.com" , "daniel@ffwll.ch" , "ville.syrjala@linux.intel.com" , "noralf@tronnes.org" , "robdclark@gmail.com" , "gustavo@padovan.org" , "maarten.lankhorst@linux.intel.com" , "seanpaul@chromium.org" , "airlied@linux.ie" , "linux-kernel@vger.kernel.org" List-Id: dri-devel@lists.freedesktop.org On 05/04/2018 01:49, Deepak Rawat wrote: > From: Lukasz Spintzyk > > 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 with maximum > array size limited by DRM_MODE_FB_DIRTY_MAX_CLIPS. Unlike plane src > coordinates, damage clips are not in 16.16 fixed point. > > Damage clips are a hint to kernel as which area of framebuffer has > changed since last page-flip. This should be helpful for some drivers > especially for virtual devices where each framebuffer change needs to > be transmitted over network, usb, etc. > > Driver which are interested in enabling DAMAGE_CLIPS property for a > plane should enable this property using drm_plane_enable_damage_clips. > > Signed-off-by: Lukasz Spintzyk > Signed-off-by: Deepak Rawat > --- > drivers/gpu/drm/drm_atomic.c | 42 ++++++++++++++++++++++++++++++= +++++++ > drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ > drivers/gpu/drm/drm_mode_config.c | 5 +++++ > drivers/gpu/drm/drm_plane.c | 12 +++++++++++ > include/drm/drm_mode_config.h | 15 +++++++++++++ > include/drm/drm_plane.h | 16 ++++++++++++++ > include/uapi/drm/drm_mode.h | 15 +++++++++++++ > 7 files changed, 109 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 7d25c42..9226d24 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -669,6 +669,40 @@ static void drm_atomic_crtc_print_state(struct drm_p= rinter *p, > } > =20 > /** > + * drm_atomic_set_damage_for_plane - sets the damage clips property to p= lane > + * @state: plane state > + * @blob: damage clips in framebuffer coordinates > + * > + * Returns: > + * > + * Zero on success, error code on failure. > + */ > +static int drm_atomic_set_damage_for_plane(struct drm_plane_state *state= , > +=09=09=09=09=09 struct drm_property_blob *blob) > +{ > +=09if (blob =3D=3D state->damage_clips) > +=09=09return 0; > + > +=09drm_property_blob_put(state->damage_clips); > +=09state->damage_clips =3D NULL; > + > +=09if (blob) { > +=09=09uint32_t count =3D blob->length/sizeof(struct drm_rect); > + > +=09=09if (count > DRM_MODE_FB_DIRTY_MAX_CLIPS) > +=09=09=09return -EINVAL; > + > +=09=09state->damage_clips =3D drm_property_blob_get(blob); > +=09=09state->num_clips =3D count; > +=09} else { > +=09=09state->damage_clips =3D NULL; > +=09=09state->num_clips =3D 0; > +=09} > + > +=09return 0; > +} > + > +/** > * drm_atomic_get_plane_state - get plane state > * @state: global atomic state object > * @plane: plane to get state object for > @@ -793,6 +827,12 @@ static int drm_atomic_plane_set_property(struct drm_= plane *plane, > =09=09state->color_encoding =3D val; > =09} else if (property =3D=3D plane->color_range_property) { > =09=09state->color_range =3D val; > +=09} else if (property =3D=3D config->prop_damage_clips) { > +=09=09struct drm_property_blob *blob =3D > +=09=09=09drm_property_lookup_blob(dev, val); > +=09=09int ret =3D drm_atomic_set_damage_for_plane(state, blob); > +=09=09drm_property_blob_put(blob); > +=09=09return ret; > =09} else if (plane->funcs->atomic_set_property) { > =09=09return plane->funcs->atomic_set_property(plane, state, > =09=09=09=09property, val); > @@ -856,6 +896,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane= , > =09=09*val =3D state->color_encoding; > =09} else if (property =3D=3D plane->color_range_property) { > =09=09*val =3D state->color_range; > +=09} else if (property =3D=3D config->prop_damage_clips) { > +=09=09*val =3D (state->damage_clips) ? state->damage_clips->base.id : 0; > =09} else if (plane->funcs->atomic_get_property) { > =09=09return plane->funcs->atomic_get_property(plane, state, property, = val); > =09} else { > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_at= omic_helper.c > index c356545..55b44e3 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -3506,6 +3506,8 @@ void __drm_atomic_helper_plane_duplicate_state(stru= ct drm_plane *plane, > =20 > =09state->fence =3D NULL; > =09state->commit =3D NULL; > +=09state->damage_clips =3D NULL; > +=09state->num_clips =3D 0; > } > EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state); > =20 > @@ -3550,6 +3552,8 @@ void __drm_atomic_helper_plane_destroy_state(struct= drm_plane_state *state) > =20 > =09if (state->commit) > =09=09drm_crtc_commit_put(state->commit); > + > +=09drm_property_blob_put(state->damage_clips); > } > EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state); > =20 > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode= _config.c > index e5c6533..e93b127 100644 > --- a/drivers/gpu/drm/drm_mode_config.c > +++ b/drivers/gpu/drm/drm_mode_config.c > @@ -293,6 +293,11 @@ static int drm_mode_create_standard_properties(struc= t drm_device *dev) > =09=09return -ENOMEM; > =09dev->mode_config.prop_crtc_id =3D prop; > =20 > +=09prop =3D drm_property_create(dev, DRM_MODE_PROP_BLOB, "DAMAGE_CLIPS",= 0); > +=09if (!prop) > +=09=09return -ENOMEM; > +=09dev->mode_config.prop_damage_clips =3D prop; > + > =09prop =3D drm_property_create_bool(dev, DRM_MODE_PROP_ATOMIC, > =09=09=09"ACTIVE"); > =09if (!prop) > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > index 6d2a6e4..071221b 100644 > --- a/drivers/gpu/drm/drm_plane.c > +++ b/drivers/gpu/drm/drm_plane.c > @@ -1101,3 +1101,15 @@ int drm_mode_page_flip_ioctl(struct drm_device *de= v, > =20 > =09return ret; > } > + > +/** > + * drm_plane_enable_damage_clips - enable damage clips property > + * @plane: plane on which this property to enable. > + */ > +void drm_plane_enable_damage_clips(struct drm_plane *plane) > +{ > +=09struct drm_device *dev =3D plane->dev; > +=09struct drm_mode_config *config =3D &dev->mode_config; > + > +=09drm_object_attach_property(&plane->base, config->prop_damage_clips, 0= ); > +} > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.= h > index 7569f22..d8767da 100644 > --- a/include/drm/drm_mode_config.h > +++ b/include/drm/drm_mode_config.h > @@ -628,6 +628,21 @@ struct drm_mode_config { > =09 */ > =09struct drm_property *prop_crtc_id; > =09/** > +=09 * @prop_damage_clips: Optional plane property to mark damaged region= s > +=09 * on the plane in framebuffer coordinates of the framebuffer attache= d > +=09 * to the plane. > +=09 * > +=09 * The layout of blob data is simply an array of drm_mode_rect with > +=09 * maximum array size limited by DRM_MODE_FB_DIRTY_MAX_CLIPS. Unlike > +=09 * plane src coordinates, damage clips are not in 16.16 fixed point. > +=09 * > +=09 * Damage clips are a hint to kernel as which area of framebuffer has > +=09 * changed since last page-flip. This should be helpful > +=09 * for some drivers especially for virtual devices where each > +=09 * framebuffer change needs to be transmitted over network, usb, etc. > +=09 */ > +=09struct drm_property *prop_damage_clips; > +=09/** > =09 * @prop_active: Default atomic CRTC property to control the active > =09 * state, which is the simplified implementation for DPMS in atomic > =09 * drivers. > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h > index f7bf4a4..9f24548 100644 > --- a/include/drm/drm_plane.h > +++ b/include/drm/drm_plane.h > @@ -146,6 +146,21 @@ struct drm_plane_state { > =09 */ > =09struct drm_crtc_commit *commit; > =20 > +=09/* > +=09 * @damage_clips > +=09 * > +=09 * blob property with damage as array of drm_rect in framebuffer > +=09 * coodinates. > +=09 */ > +=09struct drm_property_blob *damage_clips; > + > +=09/* > +=09 * @num_clips > +=09 * > +=09 * Number of drm_rect in @damage_clips. > +=09 */ > +=09uint32_t num_clips; > + > =09struct drm_atomic_state *state; > }; > =20 > @@ -611,6 +626,7 @@ int drm_plane_init(struct drm_device *dev, > =09=09 const uint32_t *formats, unsigned int format_count, > =09=09 bool is_primary); > void drm_plane_cleanup(struct drm_plane *plane); > +void drm_plane_enable_damage_clips(struct drm_plane *plane); > =20 > /** > * drm_plane_index - find the index of a registered plane > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index 50bcf42..0ad0d5b 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -873,6 +873,21 @@ struct drm_mode_revoke_lease { > =09__u32 lessee_id; > }; > =20 > +/** > + * struct drm_mode_rect - two dimensional rectangle drm_rect exported to > + * user-space. > + * @x1: horizontal starting coordinate (inclusive) > + * @y1: vertical starting coordinate (inclusive) > + * @x2: horizontal ending coordinate (exclusive) > + * @y2: vertical ending coordinate (exclusive) > + */ > +struct drm_mode_rect { > +=09__s32 x1; > +=09__s32 y1; > +=09__s32 x2; > +=09__s32 y2; > +}; I wonder why we can't use move 'struct drm_rect'=A0 definition from=20 'include/drm/drm_rect.h' and include 'uapi/drm/drm_mode.h' in private header=20 'include/drm/drm_rect.h'. Is there any general rule that disallows it? > + > #if defined(__cplusplus) > } > #endif