[PATCH xf86-video-amdgpu 00/13] Enabling Color Management - Round 2

[Michel Dänzer]

On 2018-05-24 10:29 PM, Leo Li wrote:
> On 2018-05-18 04:10 AM, Michel Dänzer wrote:
>> On 2018-05-17 11:43 PM, Leo Li wrote:
>>> On 2018-05-16 01:06 PM, Michel Dänzer wrote:
>>>> On 2018-05-03 08:31 PM, sunpeng.li at amd.com wrote:
>>>>>
>>>>> 3. The three color management properties (Degamma LUT, Color
>>>>> Transform Matrix
>>>>>      (CTM), and Gamma LUT) are hard-coded into the DDX driver, to be
>>>>> listed (as
>>>>>      disabled) regardless of whether a CRTC is attached on the output,
>>>>> or whether
>>>>>      the kernel driver supports it.
>>>>>
>>>>>       * If kernel driver does not support color management, the
>>>>> properties will
>>>>>         remain disabled. A `xrandr --set` will then error.
>>>>
>>>> Is it really useful to expose these properties to clients if the kernel
>>>> doesn't support them?
>>>>
>>>
>>> I left them exposed mainly for simplicity. I can see how it would
>>> confuse a client.
>>>
>>> It should be simpler to hide these properties once the color property
>>> IDs are cached somewhere (maybe on the AMDGPUInfo struct?)
>>
>> drmmode_crtc_private_rec seems better.
>>
> 
> Doesn't that mean we're caching duplicate DRM property IDs on each CRTC
> object? I think we only need to cache one copy.
> 
> Looking at DRM code, the IDs identify DRM property "types", not the
> actual property data, and are created during kernel driver load. Storing
> one copy is enough, since the types are the same regardless of CRTC.
> 
> I was thinking we can fetch these id's in drmmode_pre_init because of
> that, but I'm not sure of the implications. Wouldn't that be better?

If the IDs are the same for all CRTCs, they should be stored in struct
drmmode_rec.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer