From dri-devel Thu Jul 20 09:20:40 2023 From: Simon Ser Date: Thu, 20 Jul 2023 09:20:40 +0000 To: dri-devel Subject: Re: [PATCH v5 2/9] drm/atomic: Add support for mouse hotspots Message-Id: X-MARC-Message: https://marc.info/?l=dri-devel&m=168984477104110 On Wednesday, July 19th, 2023 at 03:42, Zack Rusin wrote: > From: Zack Rusin >=20 > Atomic modesetting code lacked support for specifying mouse cursor > hotspots. The legacy kms DRM_IOCTL_MODE_CURSOR2 had support for setting > the hotspot but the functionality was not implemented in the new atomic > paths. >=20 > Due to the lack of hotspots in the atomic paths userspace compositors > completely disable atomic modesetting for drivers that require it (i.e. > all paravirtualized drivers). >=20 > This change adds hotspot properties to the atomic codepaths throughtout > the DRM core and will allow enabling atomic modesetting for virtualized > drivers in the userspace. >=20 > Signed-off-by: Zack Rusin > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: David Airlie > Cc: Daniel Vetter > Reviewed-by: Javier Martinez Canillas > --- > drivers/gpu/drm/drm_atomic_state_helper.c | 14 +++++++ > drivers/gpu/drm/drm_atomic_uapi.c | 20 +++++++++ > drivers/gpu/drm/drm_plane.c | 50 +++++++++++++++++++++++ > include/drm/drm_plane.h | 14 +++++++ > 4 files changed, 98 insertions(+) >=20 > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/= drm_atomic_state_helper.c > index 784e63d70a42..54975de44a0e 100644 > --- a/drivers/gpu/drm/drm_atomic_state_helper.c > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c > @@ -275,6 +275,20 @@ void __drm_atomic_helper_plane_state_reset(struct dr= m_plane_state *plane_state, > =09=09=09plane_state->normalized_zpos =3D val; > =09=09} > =09} > + > +=09if (plane->hotspot_x_property) { > +=09=09if (!drm_object_property_get_default_value(&plane->base, > +=09=09=09=09=09=09=09 plane->hotspot_x_property, > +=09=09=09=09=09=09=09 &val)) > +=09=09=09plane_state->hotspot_x =3D val; > +=09} > + > +=09if (plane->hotspot_y_property) { > +=09=09if (!drm_object_property_get_default_value(&plane->base, > +=09=09=09=09=09=09=09 plane->hotspot_y_property, > +=09=09=09=09=09=09=09 &val)) > +=09=09=09plane_state->hotspot_y =3D val; > +=09} > } > EXPORT_SYMBOL(__drm_atomic_helper_plane_state_reset); >=20 > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atom= ic_uapi.c > index 98d3b10c08ae..07a7b3f18df2 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -593,6 +593,22 @@ static int drm_atomic_plane_set_property(struct drm_= plane *plane, > =09} else if (plane->funcs->atomic_set_property) { > =09=09return plane->funcs->atomic_set_property(plane, state, > =09=09=09=09property, val); > +=09} else if (property =3D=3D plane->hotspot_x_property) { > +=09=09if (plane->type !=3D DRM_PLANE_TYPE_CURSOR) { > +=09=09=09drm_dbg_atomic(plane->dev, > +=09=09=09=09 "[PLANE:%d:%s] is not a cursor plane: 0x%llx\n", > +=09=09=09=09 plane->base.id, plane->name, val); > +=09=09=09return -EINVAL; > +=09=09} Hm, it sounds a bit weird to catch this case here. Wouldn't it be a driver = bug to attach the hotspot props to a plane which isn't a cursor? Wouldn't it ma= ke more sense to WARN in drm_plane_create_hotspot_properties() if a driver attempts to do so? > +=09=09state->hotspot_x =3D val; > +=09} else if (property =3D=3D plane->hotspot_y_property) { > +=09=09if (plane->type !=3D DRM_PLANE_TYPE_CURSOR) { > +=09=09=09drm_dbg_atomic(plane->dev, > +=09=09=09=09 "[PLANE:%d:%s] is not a cursor plane: 0x%llx\n", > +=09=09=09=09 plane->base.id, plane->name, val); > +=09=09=09return -EINVAL; > +=09=09} > +=09=09state->hotspot_y =3D val; > =09} else { > =09=09drm_dbg_atomic(plane->dev, > =09=09=09 "[PLANE:%d:%s] unknown property [PROP:%d:%s]\n", > @@ -653,6 +669,10 @@ drm_atomic_plane_get_property(struct drm_plane *plan= e, > =09=09*val =3D state->scaling_filter; > =09} else if (plane->funcs->atomic_get_property) { > =09=09return plane->funcs->atomic_get_property(plane, state, property, v= al); > +=09} else if (property =3D=3D plane->hotspot_x_property) { > +=09=09*val =3D state->hotspot_x; > +=09} else if (property =3D=3D plane->hotspot_y_property) { > +=09=09*val =3D state->hotspot_y; > =09} else { > =09=09drm_dbg_atomic(dev, > =09=09=09 "[PLANE:%d:%s] unknown property [PROP:%d:%s]\n", > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > index c6bbb0c209f4..eaca367bdc7e 100644 > --- a/drivers/gpu/drm/drm_plane.c > +++ b/drivers/gpu/drm/drm_plane.c > @@ -230,6 +230,47 @@ static int create_in_format_blob(struct drm_device *= dev, struct drm_plane *plane > =09return 0; > } >=20 > +/** > + * drm_plane_create_hotspot_properties - creates the mouse hotspot > + * properties and attaches them to the given cursor plane > + * > + * @plane: drm cursor plane > + * > + * This function enables the mouse hotspot property on a given > + * cursor plane. > + * > + * RETURNS: > + * Zero for success or -errno > + */ > +static int drm_plane_create_hotspot_properties(struct drm_plane *plane) > +{ > +=09struct drm_property *prop_x; > +=09struct drm_property *prop_y; > + > +=09drm_WARN_ON(plane->dev, > +=09=09 !drm_core_check_feature(plane->dev, > +=09=09=09=09=09 DRIVER_CURSOR_HOTSPOT)); > + > +=09prop_x =3D drm_property_create_signed_range(plane->dev, 0, "HOTSPOT_X= ", > +=09=09=09=09=09=09 INT_MIN, INT_MAX); This is an atomic-only property, the DRM_MODE_PROP_ATOMIC flag should be pa= ssed. > +=09if (IS_ERR(prop_x)) > +=09=09return PTR_ERR(prop_x); > + > +=09prop_y =3D drm_property_create_signed_range(plane->dev, 0, "HOTSPOT_Y= ", > +=09=09=09=09=09=09 INT_MIN, INT_MAX); > +=09if (IS_ERR(prop_y)) { > +=09=09drm_property_destroy(plane->dev, prop_x); > +=09=09return PTR_ERR(prop_y); > +=09} > + > +=09drm_object_attach_property(&plane->base, prop_x, 0); > +=09drm_object_attach_property(&plane->base, prop_y, 0); > +=09plane->hotspot_x_property =3D prop_x; > +=09plane->hotspot_y_property =3D prop_y; > + > +=09return 0; > +}