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

From: Leo Li <sunpeng.li-5C7GfCeVMHo@public.gmane.org>
To: "Michel Dänzer" <michel-otUistvHUpPR7s880joybQ@public.gmane.org>
Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH xf86-video-amdgpu 0/5] Implementing non-legacy color management
Date: Tue, 10 Apr 2018 14:02:44 -0400	[thread overview]
Message-ID: <cec69aeb-1006-d41e-b63c-9b22f17d46b9@amd.com> (raw)
In-Reply-To: <53e9ea09-8b20-70fd-b7ef-c0219d46bdcc-otUistvHUpPR7s880joybQ@public.gmane.org>

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@amd.com wrote:
>> From: "Leo (Sunpeng) Li" <sunpeng.li@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.
> 
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx