On Mon, Apr 08, 2019 at 03:59:51PM +0000, Shankar, Uma wrote: > > > >-----Original Message----- > >From: dri-devel [mailto:dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of Ville > >Syrjälä > >Sent: Monday, April 8, 2019 9:15 PM > >To: Shankar, Uma <uma.shankar@xxxxxxxxx> > >Cc: dcastagna@xxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri- > >devel@xxxxxxxxxxxxxxxxxxxxx; seanpaul@xxxxxxxxxxxx; Syrjala, Ville > ><ville.syrjala@xxxxxxxxx>; Lankhorst, Maarten <maarten.lankhorst@xxxxxxxxx> > >Subject: Re: [v2 0/7] Add Multi Segment Gamma Support > > > >On Mon, Apr 08, 2019 at 03:40:39PM +0000, Shankar, Uma wrote: > >> > >> > >> >-----Original Message----- > >> >From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] > >> >Sent: Monday, April 8, 2019 8:27 PM > >> >To: Shankar, Uma <uma.shankar@xxxxxxxxx> > >> >Cc: dcastagna@xxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri- > >> >devel@xxxxxxxxxxxxxxxxxxxxx; seanpaul@xxxxxxxxxxxx; Syrjala, Ville > >> ><ville.syrjala@xxxxxxxxx>; Lankhorst, Maarten > >> ><maarten.lankhorst@xxxxxxxxx> > >> >Subject: Re: [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@xxxxxxxxxxxxxxx] > >> >> >Sent: Monday, April 8, 2019 6:01 PM > >> >> >To: Shankar, Uma <uma.shankar@xxxxxxxxx> > >> >> >Cc: dcastagna@xxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri- > >> >> >devel@xxxxxxxxxxxxxxxxxxxxx; seanpaul@xxxxxxxxxxxx; Syrjala, Ville > >> >> ><ville.syrjala@xxxxxxxxx>; Lankhorst, Maarten > >> >> ><maarten.lankhorst@xxxxxxxxx> > >> >> >Subject: Re: [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@xxxxxxxxxxxxxxxxxxxxx] > >> >> >> >On Behalf Of Ville Syrjälä > >> >> >> >Sent: Friday, April 5, 2019 9:42 PM > >> >> >> >To: Shankar, Uma <uma.shankar@xxxxxxxxx> > >> >> >> >Cc: dcastagna@xxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; > >> >> >> >dri- devel@xxxxxxxxxxxxxxxxxxxxx; seanpaul@xxxxxxxxxxxx; > >> >> >> >Syrjala, Ville <ville.syrjala@xxxxxxxxx>; Lankhorst, Maarten > >> >> >> ><maarten.lankhorst@xxxxxxxxx> > >> >> >> >Subject: Re: [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. > > Oh ok. So do you suggest that we add some kind of flag to be set by user, based on > which we take either legacy or advance_gamma path. Is my understanding correct ? Yeah, just another client cap. I can't immediately think of a nicer way to extend the precision. -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
