[PATCH xf86-video-amdgpu 2/5] Hook up CRTC color management functions

[Leo Li]

On 2018-04-11 04:48 AM, Michel Dänzer wrote:
> On 2018-04-10 08:02 PM, Leo Li wrote:
>> On 2018-04-09 11:03 AM, Michel Dänzer wrote:
>>> On 2018-03-26 10:00 PM, sunpeng.li at amd.com wrote:
>>>> From: "Leo (Sunpeng) Li" <sunpeng.li at amd.com>
>>>>
>>>> The functions insert into the output resource creation, and property
>>>> change functions. CRTC destroy is also hooked-up for proper cleanup of
>>>> the CRTC property list.
>>>>
>>>> Signed-off-by: Leo (Sunpeng) Li <sunpeng.li at amd.com>
>>>
>>> [...]
>>>
>>>> @@ -1933,6 +1933,9 @@ static void
>>>> drmmode_output_create_resources(xf86OutputPtr output)
>>>>                }
>>>>            }
>>>>        }
>>>> +
>>>> +    if (output->crtc)
>>>> +        drmmode_crtc_create_resources(output->crtc, output);
>>>
>>> output->crtc is only non-NULL here for outputs which are enabled at Xorg
>>> startup; other outputs won't have the new properties.
>>
>> Is it necessary to have the CRTC properties on a output if the CRTC is
>> disabled for that output?
> 
> The set of properties exposed by an output should be constant throughout
> its lifetime.
> 

I see.

I'm not sure what the standard is for listing 'disabled' properties like
this. What do clients expect? Should the properties be configured as 0
and immutable until a valid CRTC is attached?

This would mean that DDX has to know the list of available CRTC
properties before a CRTC is available. I guess we can hard-code a list
of expected properties from kernel DRM, if that's not of concern. And if
they don't exist because of version differences, we can just leave them
disabled.

> 
>> I've tested hot-plugging with this, and the properties do initialize on
>> hot-plug.
> 
> I suspect you were testing something like DP MST, where the output is
> created dynamically on hot-plug. For "static" outputs such as
> DVI/HDMI/VGA/LVDS (and "normal" (e)DP), drmmode_output_create_resources
> is only called once during Xorg initialization, and output->crtc is NULL
> at that point unless the output is enabled from the beginning (which is
> only the case if either the output is connected at that point, or is
> explicitly configured in xorg.conf to be on).
> 

Yep, I was using a DP... This definitely should be fixed.

Leo

>