Re: [PATCH v2 1/5] drm: add legacy support for using degamma for gamma

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Dec 02, 2020 at 01:38:42PM +0100, Daniel Vetter wrote:
> On Wed, Dec 2, 2020 at 12:52 PM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >
> > On 30/11/2020 16:10, Daniel Vetter wrote:
> >
> > > The thing is, the legacy helpers should be able to pull off what userspace
> > > needs to do when it's using atomic anyway. Hard-coding information in the
> > > kernel means we have a gap here. Hence imo legacy helpers doing the right
> > > thing in all reasonable cases is imo better.
> > >
> > > In many cases I think we should even go further, and ditch driver ability
> > > to overwrite legacy helper hooks like this. I thought we'd need that
> > > flexibility for legacy userspace being incompatible in awkward ways, but
> > > wasn't ever really needed. Worse, many drivers forget to wire up the
> > > compat hooks.
> > >
> > > tldr, imo right thing to do here:
> > > - move legacy gamma function from helpers into core code
> > > - call it unconditionally for all atomic drivers (if there's no legacy
> > >   drivers using the hook left then we can outright remove it)
> > > - make sure it dtrt in all cases
> >
> > There are atomic drivers which have their custom gamma_set function. I guess they don't support
> > atomic color mgmt, but do support (legacy) gamma.
> 
> Hm yeah, but it's this kind of feature disparity which is why I think
> we should at least try to unify more.
> 
> > We could make the core code call the gamma legacy helper automatically for atomic drivers that don't
> > have gamma_set defined but do have GAMMA_LUT or DEGAMMA_LUT. But the gamma_set function is called
> > also in a few places from drm_fb_helper.c, so this code wouldn't be fully inside drm_color_mgmt.c.
> >
> > Or we could just change drm_atomic_helper_legacy_gamma_set() to do the right thing, depending on
> > GAMMA_LUT & DEGAMMA_LUT existence.
> 
> Yeah that would be at least better than pushing more decisions onto
> drivers as hard-coding. I still think that maybe just automatically
> calling the helper when either a GAMMA or DEGAMMA lut is set up would
> be better.

BTW I have some gamma related stuff here
git://github.com/vsyrjala/linux.git fb_helper_c8_lut_4

which tries to fix some fb_helper gamma stuff, and I'm also
getting rid of the gamma_store stuff for the leacy uapi for
drivers which implement the fancier color management stuff.
In fact I just threw out the helper thing entirely and made
the core directly call the right stuff. Not sure if that
would be helpful, harmful or just meh here.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel