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. -Daniel -Daniel > > To me this feels like the pixel > > modifier discussion all over, where we had multi-year discussions on > > trying to describe everything in generic terms or just have fairly > > opaque enumeration of special cases. Both approaches have been tried. > > For this I'm leaning towards the opaque color pipeline description for > > the more fancy stuff. > > > > Either way, settling on the right uapi will take some time, and will > > need a pile of people to be involved. > > > > Cheers, Daniel > > > > > > > > drivers/gpu/drm/drm_atomic_uapi.c | 8 + > > > drivers/gpu/drm/drm_color_mgmt.c | 77 ++++ > > > drivers/gpu/drm/drm_ioctl.c | 5 + > > > drivers/gpu/drm/i915/i915_reg.h | 17 + > > > drivers/gpu/drm/i915/intel_color.c | 735 ++++++++++++++++++++++++++++++++++- > > > drivers/gpu/drm/i915/intel_display.c | 3 + > > > include/drm/drm_atomic.h | 1 + > > > include/drm/drm_color_mgmt.h | 8 + > > > include/drm/drm_crtc.h | 17 + > > > include/drm/drm_file.h | 8 + > > > include/drm/drm_mode_config.h | 6 + > > > include/uapi/drm/drm.h | 2 + > > > include/uapi/drm/drm_mode.h | 38 ++ > > > 13 files changed, 918 insertions(+), 7 deletions(-) > > > > > > -- > > > 1.9.1 > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@xxxxxxxxxxxxxxxxxxxxx > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > -- > 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. > -- 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
