On 2018-04-10 08:02 PM, Leo Li wrote: > On 2018-04-09 11:03 AM, Michel Dänzer wrote: >> On 2018-03-26 10:00 PM, sunpeng.li at amd.com wrote: >>> From: "Leo (Sunpeng) Li" <sunpeng.li at amd.com> >>> >>> In cases where CRTC properties are updated without going through >>> RRChangeOutputProperty, we don't update the properties in user land. >>> >>> Consider setting legacy gamma. It doesn't go through >>> RRChangeOutputProperty, but modifies the CRTC's color management >>> properties. Unless they are updated, the user properties will remain >>> stale. >> >> Can you describe a bit more how the legacy gamma and the new properties >> interact? >> > > Sure thing, I'll include this in the message for v2: > > In kernel, the legacy set gamma interface is essentially an adapter to > the non-legacy set properties interface. In the end, they both set the > same property to a DRM property blob, which contains the gamma lookup > table. The key difference between them is how this blob is created. > > For legacy gamma, the kernel takes 3 arrays from user-land, and creates > the blob using them. Note that a blob is identified by it's blob_id. > > For non-legacy gamma, the kernel takes a blob_id from user-land that > references the blob. This means user-land is responsible for creating > the blob. > > From the perspective of RandR, this presents some problems. Since both > paths modify the same property, RandR must keep the reported property > value up-to-date with which ever path is used: > > 1. Legacy gamma via > xrandr --output <output_here> --gamma x:x:x > 2. Non-legacy color properties via > xrandr --output <output_here> --set GAMMA_LUT <blob_id> > > Keeping the value up-to-date isn't a problem for 2, since RandR updates > it for us as part of changing output properties. > > But if 1 is used, the property blob is created within kernel, and RandR > is unaware of the new blob_id. To update it, we need to ask kernel about > it. > > --- continue with rest of message --- >> >>> Therefore, add a function to update user CRTC properties by querying >>> DRM, >>> and call it whenever legacy gamma is changed. >> >> Note that drmmode_crtc_gamma_do_set is called from >> drmmode_set_mode_major, i.e. on every modeset or under some >> circumstances when a DRI3 client stops page flipping. >> > > The property will have to be updated each time the legacy set gamma > ioctl is called, since a new blob (with a new blob_id) is created each > time. > > Not sure if this is a good idea, but perhaps we can have a flag that > explicitly enable one or the other, depending on user preference? A > user-only property with something like: > > 0: Use legacy gamma, calls to change non-legacy properties are ignored. > 1: Use non-legacy, calls to legacy gamma will be ignored. > > On 0, we can remove/disable all non-legacy properties from the property > list, and avoid having to update them. On 1, we'll enable the > properties, and won't have to update them either since legacy gamma is > "disabled". It has the added benefit of avoiding unexpected legacy gamma > sets when using non-legacy, and vice versa. Hmm. So either legacy or non-legacy clients won't work at all, or they'll step on each other's toes, clobbering the HW gamma LUT from each other. I'm afraid neither of those alternatives sound like a good user experience to me. Consider on the one hand something like Night Light / redshift, using legacy APIs to adjust colour temperature to the time of day. On the other hand, another client using the non-legacy API for say fine-tuning of a display's advanced gamma capabilities. Ideally, in this case the gamma LUTs from the legacy and non-legacy APIs should be combined, such that the hardware LUT reflects both the colour temperature set by Night Light / refshift and the fine-tuning set by the non-legacy client. Is that feasible? If not, can you explain a little why? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
