On 2018-04-09 10:10 AM, Michel Dänzer wrote: > > Hi Leo, > > > apologies for the late follow-up; I was on vacation and then backlogged. No worries, thanks for the review :) > > > On 2018-03-26 10:00 PM, sunpeng.li at amd.com wrote: >> From: "Leo (Sunpeng) Li" <sunpeng.li at amd.com> >> >> These patches will enable modification of non-legacy color management >> properties via xrandr. >> >> On top of the current legacy gamma, DRM allows the setting of three color >> management tables: the degamma LUT, the color transform matrix (CTM), and the >> regamma LUT. To user land, all of them are stored as DRM blobs, and are >> referenced by CRTC properties via their blob IDs. >> >> Therefore, in order to allow setting color management via xrandr, we have to: >> >> 1. Enable modification of CRTC properties via xrandr >> 2. Allow configuring and changing DRM blob properties via their IDs >> 3. Ensure compatability with legacy gamma >> >> The first three patches does the above, while the last two does some >> refactoring work to remove repetative code. >> >> A note to reviewers, I'm a little unclear on whether this woks when one CRTC is >> connected to multiple outputs. I expect that changing a CRTC property via one >> of its outputs will update for that output only, since randr still understands >> it as an "output property". In whic case, there needs to be a v2. > > Yes, I suspect so. > >> However, I'm not sure how I can setup a test for this. Let me know if you have tips. > > Something like > > xrandr --output DVI-D-1 --off --output DVI-D-2 --off > xrandr --output DVI-D-1 --crtc 0 --mode 1920x1080 --output DVI-D-2 > --crtc 0 --mode 1920x1080 > > and then verify with xrandr --verbose that both outputs are actually > using the same CRTC. Note that I'm getting an error on the second step > when trying this right now, so there may be something preventing using > the same CRTC for multiple outputs. But AFAIK at least in theory it's > possible. I'll give this a shot. Leo > > > I will follow up with some comments on individual patches. > >
