Re: [Intel-gfx] [v3 6/7] drm: Add Client Cap for advance gamma mode

Ville Syrjälä
2019-04-15T12:43:57Z

On Mon, Apr 15, 2019 at 10:57:52AM +0000, Lankhorst, Maarten wrote:
> fre 2019-04-12 klockan 15:51 +0530 skrev Uma Shankar:
> > Introduced a client cap for advance cap mode
> > capability. Userspace should set this to get
> > to be able to use the new gamma_mode property.
> >
> > If this is not set, driver will work in legacy
> > mode.
> >
> > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>
> Nack, this doesn't seem like a sensible idea. We already guard it
> behind the gamma mode property. Userspace shouldn't set the gamma mode
> to a value it doesn't understand.

Old userspace wouldn't know what a gamma_mode prop is. While a client
cap isn't an entirely satisfactory solution I can't think of a better
way at the moment.

Well, maybe the old "hey kernel, please reset all my props to some
sane default" idea could be another way to deal with this sort of stuff.

>
> ~Maarten
>
> > drivers/gpu/drm/drm_atomic_uapi.c | 3 +++
> > drivers/gpu/drm/drm_ioctl.c | 5 +++++
> > include/drm/drm_atomic.h | 1 +
> > include/drm/drm_crtc.h | 7 +++++++
> > include/drm/drm_file.h | 8 ++++++++
> > include/uapi/drm/drm.h | 2 ++
> > 6 files changed, 26 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index d85e0c9..590c87a 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -974,6 +974,8 @@ int drm_atomic_set_property(struct
> > drm_atomic_state *state,
> > break;
> > }
> >
> > + crtc_state->advance_gamma_mode_active =
> > + state-
> > >advance_gamma_mode_active;
> > ret = drm_atomic_crtc_set_property(crtc,
> > crtc_state, prop, prop_value);
> > break;
> > @@ -1297,6 +1299,7 @@ int drm_mode_atomic_ioctl(struct drm_device
> > *dev,
> > drm_modeset_acquire_init(&ctx,
> > DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
> > state->acquire_ctx = &ctx;
> > state->allow_modeset = !!(arg->flags &
> > DRM_MODE_ATOMIC_ALLOW_MODESET);
> > + state->advance_gamma_mode_active = file_priv-
> > >advance_gamma_mode_active;
> >
> > retry:
> > copied_objs = 0;
> > diff --git a/drivers/gpu/drm/drm_ioctl.c
> > b/drivers/gpu/drm/drm_ioctl.c
> > index d337f16..e593a4c 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -348,6 +348,11 @@ static int drm_getcap(struct drm_device *dev,
> > void *data, struct drm_file *file_
> > return -EINVAL;
> > file_priv->writeback_connectors = req->value;
> > break;
> > + case DRM_CLIENT_CAP_ADVANCE_GAMMA_MODES:
> > + if (req->value > 1)
> > + return -EINVAL;
> > + file_priv->advance_gamma_mode_active = req->value;
> > + break;
> > default:
> > return -EINVAL;
> > }
> > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> > index 824a5ed..02c1a68 100644
> > --- a/include/drm/drm_atomic.h
> > +++ b/include/drm/drm_atomic.h
> > @@ -338,6 +338,7 @@ struct drm_atomic_state {
> > * states.
> > */
> > bool duplicated : 1;
> > + bool advance_gamma_mode_active : 1;
> > struct __drm_planes_state *planes;
> > struct __drm_crtcs_state *crtcs;
> > int num_connector;
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index f789359..f11dc25 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -170,6 +170,11 @@ struct drm_crtc_state {
> > bool color_mgmt_changed : 1;
> >
> > /**
> > + * This is to indicate advance gamma mode support
> > + */
> > + bool advance_gamma_mode_active : 1;
> > +
> > + /**
> > * @no_vblank:
> > *
> > * Reflects the ability of a CRTC to send VBLANK events.
> > This state
> > @@ -952,6 +957,8 @@ struct drm_crtc {
> > */
> > bool enabled;
> >
> > + bool advance_gamma_mode_active : 1;
> > +
> > /**
> > * @mode:
> > *
> > diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> > index 6710b61..b5aa24e 100644
> > --- a/include/drm/drm_file.h
> > +++ b/include/drm/drm_file.h
> > @@ -201,6 +201,14 @@ struct drm_file {
> > bool writeback_connectors;
> >
> > /**
> > + * This is to enable advance gamma modes using
> > + * gamma_mode property
> > + *
> > + * True if client understands advance gamma
> > + */
> > + bool advance_gamma_mode_active : 1;
> > +
> > + /**
> > * @is_master:
> > *
> > * This client is the creator of @master. Protected by
> > struct
> > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> > index 236b01a..abef966 100644
> > --- a/include/uapi/drm/drm.h
> > +++ b/include/uapi/drm/drm.h
> > @@ -696,6 +696,8 @@ struct drm_get_cap {
> > */
> > #define DRM_CLIENT_CAP_WRITEBACK_CONNECTORS 5
> >
> > +#define DRM_CLIENT_CAP_ADVANCE_GAMMA_MODES 6
> > +
> > /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
> > struct drm_set_client_cap {
> > __u64 capability;

> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org