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

Ville Syrjälä
2019-04-16T15:06:27Z

On Mon, Apr 15, 2019 at 09:20:55PM +0200, Daniel Vetter wrote:
> On Mon, Apr 15, 2019 at 4:14 PM Lankhorst, Maarten
> <maarten.lankhorst@intel.com> wrote:
> >
> > mån 2019-04-15 klockan 19:26 +0530 skrev Sharma, Shashank:
> > > > -----Original Message-----
> > > > From: Lankhorst, Maarten
> > > > Sent: Monday, April 15, 2019 4:28 PM
> > > > To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedeskt
> > > > op.org; dri-
> > > > devel@lists.freedesktop.org
> > > > Cc: Syrjala, Ville <ville.syrjala@intel.com>; emil.l.velikov@gmail.
> > > > com;
> > > > sam@ravnborg.org; Roper, Matthew D <matthew.d.roper@intel.com>;
> > > > seanpaul@chromium.org; brian.starkey@arm.com; dcastagna@chromium.or
> > > > g;
> > > > Sharma, Shashank <shashank.sharma@intel.com>
> > > > Subject: Re: [v3 6/7] drm: Add Client Cap for advance gamma mode
> > > >
> > > > 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.
> > > >
> > > > ~Maarten
> > >
> > > Hey Maarten,
> > > In that case, what do you suggest should be the right way to do this
> > > ?
> > >
> > > @Ville, any comments here ?
> > >
> > I would say drop this patch, and just enable segmented gamma
> > unconditionally, it's not the first property that can cause trouble
> > when not understood.
>
> Yeah, thus far we went with "new properties should have the old
> behaviour as default, no cap/flag needed".

That's a given. But it doesn't help with the mixed userspace when one of
them doesn't know how to reset it back to default.

> If you mix old&new
> userspace and stuff starts looking funny, that's not a regression imo.
> Also, it's a very uncommon use-case.

I was mainly thinking of eg. wayland vs. X type of cases where the user
might want to switch. If the wayland compositor uses the new fancy gamma
thing but the xserver doesn't then this won't work anymore. But I guess
easier solution is to just add the relevant knowledge to the ddx. Which
should probably be a hard requirement for landing this.

>
> Wrt reset to default: fbdev emulation should do that for anything
> that's too fancy, which is generally enough for the "developer of
> compositors" use case. That part might be missing in the gamma/ctm
> support in general I think.

Probably. IIRC even running X at 8bpp and then swithcing back to
fbcon leaves you with a gargbled palette.

--
Ville Syrjälä
Intel