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

[Leo Li <sunpeng.li-5C7GfCeVMHo@public.gmane.org>]

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@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?

Leo

> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx