[PATCH xf86-video-amdgpu 0/5] Implementing non-legacy color management

[Leo Li]

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.
> 
>