On Thu, Apr 18, 2019 at 04:11:58PM +0300, Ville Syrjälä wrote: > On Thu, Apr 18, 2019 at 09:13:04AM +0200, Daniel Vetter wrote: > > On Wed, Apr 17, 2019 at 02:57:31PM +0300, Ville Syrjälä wrote: > > > On Wed, Apr 17, 2019 at 09:28:19AM +0200, Daniel Vetter wrote: > > > > On Fri, Apr 12, 2019 at 03:50:56PM +0530, Uma Shankar wrote: > > > > > This series adds support for programmable gamma modes and > > > > > exposes a property interface for the same. Also added, > > > > > support for multi segment gamma mode introduced in ICL+ > > > > > > > > > > It creates GAMMA_MODE property interface. This is an enum > > > > > property with values as blob_id's and exposes > > > > > the various gamma modes supported and the lut ranges Getting the > > > > > blob id in userspace, user can get the mode supported and > > > > > also the range of gamma mode supported with number of lut > > > > > coefficients. It can then set one of the modes using this > > > > > enum property. > > > > > > > > > > Lut values will be sent through already available GAMMA_LUT > > > > > blob property. > > > > > > > > > > It also introduces a CLIENT CAP for advanced GAMMA_MODE. > > > > > This is for user to set the and use advance gamma mode and older > > > > > userspace can continue using the legacy paths. > > > > > > > > > > v2: Used Ville's design and approach to define the interfaces. > > > > > Addressed Matt Roper's review feedback and re-ordered the > > > > > patches. > > > > > > > > > > v3: Converged to 1 property interface and introduced a Client cap > > > > > as suggested by Ville. Fixed review comments received. > > > > > > > > > > Uma Shankar (5): > > > > > drm/i915/icl: Add register definitions for Multi Segmented gamma > > > > > drm/i915/icl: Add support for multi segmented gamma mode > > > > > drm/i915: Attach gamma mode property > > > > > drm: Add Client Cap for advance gamma mode > > > > > drm/i915: Enable advance gamma mode > > > > > > > > > > Ville Syrjälä (2): > > > > > drm: Add gamma mode property > > > > > drm/i915: Define color lut range structure > > > > > > > > Bunch of higher level comments after some internal discussions: > > > > > > > > - we need the userspace for this, can't design new uapi without involving > > > > the compositor folks for hdr. > > > > > > > > - single property doesn't work: Once userspace has set it, the old blob > > > > property with the list of all options is gone. We need one read-only > > > > property for the list of options, plus a 2nd property that userspace can > > > > set. This is a general rule for more complex properties, where the usual > > > > property metadata isn't enough to describe the possible options. > > > > > > I guess no one understood my blob_enum idea? It's an enum where each > > > possible value is a blob. The only thing that changes is the current > > > value (which can only point to one of the enumerated blobs). > > > > Uh yes that's not clear at all, and if we do go with this, I guess we > > should have a pile of core code to make sure it validates and is > > consistent. > > > > >> > - no caps for properties. Yes that gives us a theoretical problem, no in > > > > practice it doesn't matter, since people don't even care enough to make > > > > e.g. fbdev resetting work today for everything. Long form discussion, > > > > see here: > > > > > > > > https://blog.ffwll.ch/2016/01/vt-switching-with-atomic-modeset.html > > > > > > > > Nothing happened in this area ever since I typed this up, so I guess > > > > it's really not a real-world concern. > > > > > > > > - Simplest path forward would be if we accept different LUT sizes than the > > > > one advertised (we already do that for legacy gamma, and this is > > > > officially what we had in mind too), and the kernel automatically picks > > > > the best lut configuration. Will be somewhat awkard for the > > > > multi-segment lut, but would decouple the uapi discussion a bit. > > > > > > It'll be ridiculously wasteful. IIRC we need a LUT with 32768 entries, > > > and then ~98% of those gets thrown away and never programmed to the > > > hardware. > > > > Yeah it's a few MB, not that awesome really ... > > > > > > - Frankly the uapi proposed looks like fake generic - it tries to model > > > > all possibilities in a generic way, when really userspace needs to have > > > > special code for special pipelines. > > > > > > I think it can be used pretty easily. Userspace just has to decide > > > whether it wants a straight up LUT or whether an interpolated curve > > > is enough, and how much precision it needs. For x11 the logic would > > > be simple enough: 1. look for straight up LUT with num_entries >= 1<<bpc, > > > if that isn't found fall back to an interpolated curve with >= 1<<bpc > > > precision, and finally just fall back to whatever gives the best > > > results I suppose. > > > > Hm, there's also a bunch more defines about mirroring and non-negative and > > other stuff that I have no idea how userspace should use it. I do think > > some "here's the possible configs for color management" thing is needed, > > I'm not sure userspace can do much with all the details provided in the > > current series. > > The negative values would represent out of gamut colors, which > can definitely happen with xvYCC. Looks like the v4l folks > also considered this in their transfer func docs [1], which > are specifying the formulas extended for <0.0 values. > > Also based on my chat with Ilia on irc at some point I got the > impression that nv hardware may have gamma LUTs which support > negative values without this mirroring trick. Hence I wanted to > make all the numbers signed rather than unsigned. > > We could of course leave a bunch of these advanced things > undefined until an actual use case comes around. But I wanted to > include it all in my initial proposal so that we could be more > confident that we're not painting ourselves in a corner that > would require yet another uapi to escape. Hm good point. I just feel like with just one driver (at least in kms) it might be a bit early to go all in on color manager v2.0. From a kms ecosystem pov we're still pretty busy rolling out the first one. But if we need it now, I guess we'll need it now ... > [1] https://www.linuxtv.org/downloads/v4l-dvb-apis-old/ch02s06.html > > -- > Ville Syrjälä > Intel > --------------------------------------------------------------------- > Intel Finland Oy > Registered Address: PL 281, 00181 Helsinki > Business Identity Code: 0357606 - 4 > Domiciled in Helsinki > > This e-mail and any attachments may contain confidential material for > the sole use of the intended recipient(s). Any review or distribution > by others is strictly prohibited. If you are not the intended > recipient, please contact the sender and delete all copies. Wrong mail address :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
