[Intel-gfx] [v2 0/7] Add Multi Segment Gamma Support
Ville Syrjälä
ville.syrjala at linux.intel.com
Mon Apr 8 15:45:05 UTC 2019
On Mon, Apr 08, 2019 at 03:40:39PM +0000, Shankar, Uma wrote:
>
>
> >-----Original Message-----
> >From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> >Sent: Monday, April 8, 2019 8:27 PM
> >To: Shankar, Uma <uma.shankar at intel.com>
> >Cc: dcastagna at chromium.org; intel-gfx at lists.freedesktop.org; dri-
> >devel at lists.freedesktop.org; seanpaul at chromium.org; Syrjala, Ville
> ><ville.syrjala at intel.com>; Lankhorst, Maarten <maarten.lankhorst at intel.com>
> >Subject: Re: [Intel-gfx] [v2 0/7] Add Multi Segment Gamma Support
> >
> >On Mon, Apr 08, 2019 at 02:40:51PM +0000, Shankar, Uma wrote:
> >>
> >>
> >> >-----Original Message-----
> >> >From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> >> >Sent: Monday, April 8, 2019 6:01 PM
> >> >To: Shankar, Uma <uma.shankar at intel.com>
> >> >Cc: dcastagna at chromium.org; intel-gfx at lists.freedesktop.org; dri-
> >> >devel at lists.freedesktop.org; seanpaul at chromium.org; Syrjala, Ville
> >> ><ville.syrjala at intel.com>; Lankhorst, Maarten
> >> ><maarten.lankhorst at intel.com>
> >> >Subject: Re: [Intel-gfx] [v2 0/7] Add Multi Segment Gamma Support
> >> >
> >> >On Mon, Apr 08, 2019 at 12:26:23PM +0000, Shankar, Uma wrote:
> >> >>
> >> >>
> >> >> >-----Original Message-----
> >> >> >From: dri-devel [mailto:dri-devel-bounces at lists.freedesktop.org]
> >> >> >On Behalf Of Ville Syrjälä
> >> >> >Sent: Friday, April 5, 2019 9:42 PM
> >> >> >To: Shankar, Uma <uma.shankar at intel.com>
> >> >> >Cc: dcastagna at chromium.org; intel-gfx at lists.freedesktop.org; dri-
> >> >> >devel at lists.freedesktop.org; seanpaul at chromium.org; Syrjala, Ville
> >> >> ><ville.syrjala at intel.com>; Lankhorst, Maarten
> >> >> ><maarten.lankhorst at intel.com>
> >> >> >Subject: Re: [Intel-gfx] [v2 0/7] Add Multi Segment Gamma Support
> >> >> >
> >> >> >On Mon, Apr 01, 2019 at 11:00:04PM +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 2 property interfaces :
> >> >> >> 1. GAMMA_MODE_CAPS: This is immutable property and exposes the
> >> >> >> various gamma modes supported and the lut ranges. This is an
> >> >> >> enum property with element as blob id. 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.
> >> >> >>
> >> >> >> 2. GAMMA_MODE: This is for user to set the gamma mode and send
> >> >> >> the lut values for that particular mode.
> >> >> >
> >> >> >I think we should just go for the BLOB_ENUM prop type instead.
> >> >> >Then the possible values and the current value are all part of the same prop.
> >> >>
> >> >> Hi Ville,
> >> >> With the current approach, we have enum property with values as
> >> >> blob_ids (representing platform capabilities). This should not get
> >> >> modified and needs to be immutable.
> >> >
> >> >That's not quite what we want. We want to let the user modify the
> >> >current value so that they can actually select the gamma mode.
> >> >Otherwise we need yet another prop for it, or we have to deduce it
> >> >from the LUT size (that apporach would actually work for i915 but may
> >> >not work for other drivers/hardware).
> >> >
> >> >>
> >> >> Userspace can query the property and get the blob using the blob_ids.
> >> >> Thereby getting all the platform capabilities.
> >> >>
> >> >> Now to set the LUT values, he can use another blob property and
> >> >> pass the luts. This is inline to how gamma/degamma is implemented
> >> >> where we have one immutable LUT_SIZE property (indicating number of
> >> >> luts) and another blob property for actual lut values..
> >>
> >> Hi Ville,
> >> Just to clarify and clear some doubts :)
> >>
> >> We should be able to set the gamma mode using the blob enum value.
> >> Userspace will get the list enum vals (blob ids with mode name embedded) and
> >select one and do a setprop to set a mode.
> >> Driver will get the blob_id and will be able to get the mode to be
> >> set. So exposing capabilities and setting the mode should be possible with this one
> >property. I hope my understanding is correct.
> >>
> >> Now to send the actual blob values to be set, we need to use some
> >> other property interface. Should we use the currently available "gamma blob
> >(gamma_lut_property)" property to send the lut values.
> >> The challenge there is that it currently uses 16 bit lut values struct
> >> drm_color_lut {
> >> __u16 red;
> >> __u16 green;
> >> __u16 blue;
> >> __u16 reserved;
> >> };
> >> which is not sufficient for HDR usecases. Or should we need a new property for
> >advance lut/extended lut like below:
> >> https://patchwork.freedesktop.org/patch/294732/?series=30875&rev=7
> >>
> >> What do you suggest ?
> >
> >I was thinking that we might get away with reusing the current props and just change
> >the interpretation of the data when gamma_mode is set. But I'm not sure that's going
> >to work out so well if one client sets this up and then another client comes along that
> >doesn't understand the new props at all. But even with separate props I think we
> >might still end up in a mess because the new client wouldn't know how to unset the
> >higher precision LUT before setting up the old style prop and the kernel would then
> >refuse the operation with with both props being set.
> >
> >So I think we might need a client cap for this which simply changes how the data in
> >the existing props is represented. So internally we could always store things in the
> >new high precision format, but we'd convert to/from the old format when dealing
> >with an older client.
>
> We could also say that if a legacy gamma_mode_property is set (which will be used by
> legacy apps or apps not aware of new interface), in driver we will simply unset the
> earlier gamma_mode and fallback to legacy mode (whatever it was for a particular
> platform). This way we should be able to deal with this situation and an explicit unset
> may not be needed. What do you think ?
I don't want (non-immutable) properties that magically change values.
That way lies madness.
>
> >>
> >> Note: I have currently used 16bit values only to get the feedback on the approach.
> >>
> >> Regards,
> >> Uma Shankar
> >>
> >> >>
> >> >> >>
> >> >> >> v2: Used Ville's design and approach to define the interfaces.
> >> >> >> Addressed Matt Roper's review feedback and re-ordered the patches.
> >> >> >>
> >> >> >> Uma Shankar (5):
> >> >> >> drm: Add gamma mode property
> >> >> >> drm/i915/icl: Add register definitions for Multi Segmented gamma
> >> >> >> drm/i915/icl: Add support for multi segmented gamma mode
> >> >> >> drm/i915: Add gamma mode caps property
> >> >> >> drm/i915: Attach gamma mode property
> >> >> >>
> >> >> >> Ville Syrjälä (2):
> >> >> >> drm: Add gamma mode caps property
> >> >> >> drm/i915: Define color lut range structure
> >> >> >>
> >> >> >> drivers/gpu/drm/drm_atomic_uapi.c | 13 +
> >> >> >> drivers/gpu/drm/drm_color_mgmt.c | 110 +++++++++
> >> >> >> drivers/gpu/drm/i915/i915_reg.h | 17 ++
> >> >> >> drivers/gpu/drm/i915/intel_color.c | 465
> >> >> >++++++++++++++++++++++++++++++++++-
> >> >> >> drivers/gpu/drm/i915/intel_display.c | 5 +
> >> >> >> include/drm/drm_color_mgmt.h | 11 +
> >> >> >> include/drm/drm_crtc.h | 17 ++
> >> >> >> include/drm/drm_mode_config.h | 10 +
> >> >> >> include/uapi/drm/drm_mode.h | 49 ++++
> >> >> >> 9 files changed, 690 insertions(+), 7 deletions(-)
> >> >> >>
> >> >> >> --
> >> >> >> 1.9.1
> >> >> >>
> >> >> >> _______________________________________________
> >> >> >> Intel-gfx mailing list
> >> >> >> Intel-gfx at lists.freedesktop.org
> >> >> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> >> >
> >> >> >--
> >> >> >Ville Syrjälä
> >> >> >Intel
> >> >> >_______________________________________________
> >> >> >dri-devel mailing list
> >> >> >dri-devel at lists.freedesktop.org
> >> >> >https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >> >
> >> >--
> >> >Ville Syrjälä
> >> >Intel
> >
> >--
> >Ville Syrjälä
> >Intel
--
Ville Syrjälä
Intel
More information about the Intel-gfx
mailing list