On 2018-04-11 04:39 AM, Michel Dänzer wrote:
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...@amd.com wrote:
From: "Leo (Sunpeng) Li" <sunpeng...@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?
Hmm, combining LUTs could be feasible, but I don't think that's the
right approach.
LUTs can be combined through composition h(x) = f(g(x)), with some
interpolation involved. Once combined, it can be set using the
non-legacy API, since that'll likely have a larger LUT size.
But I think what you've mentioned raises a bigger issue of color
management conflicts in general. It doesn't have to be redshift vs
monitor correction, what if there more than 2 applications contending to
set gamma? xrandr's brightness already conflicts with redshift, and so
does some apps running on WINE. Ultimately, any (legacy or not) gamma
set requests are unified into one CRTC property in DRM, and they will
all conflict if not managed. I don't think combining two LUTs will help
here.
For the small example at hand, the ideal solution is to have redshift
use the color transformation matrix (CTM), which will be exposed as part
of the non-legacy color API. It'll need modification of redshift, but at
least it won't conflict with anything gamma related.
----
Jumping back on some patch 1 topics:
Are there ways to get arbitrarily (no more than 4096 elements) sized
arrays from a client, to the DDX driver? Otherwise, I'm not sure if
there's another way to expose these properties, short of modifying the
RandR interface. I agree, it would definitely be nicer for clients to
not worry about DRM blobs at all.
Leo
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
