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@xxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel