[PATCH xf86-video-amdgpu 10/13] Push staged color properties when DPMS state toggles On

[Michel Dänzer]

On 2018-05-17 11:44 PM, Leo Li wrote:
> On 2018-05-16 01:10 PM, Michel Dänzer wrote:
>> On 2018-05-03 08:31 PM, sunpeng.li at amd.com wrote:
>>> From: "Leo (Sunpeng) Li" <sunpeng.li at amd.com>
>>>
>>> This will persist color management properties on a CRTC across DPMS
>>> state changes.
>>>
>>> Signed-off-by: Leo (Sunpeng) Li <sunpeng.li at amd.com>
>>> ---
>>>   src/drmmode_display.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
>>> index 45c582c..06ae902 100644
>>> --- a/src/drmmode_display.c
>>> +++ b/src/drmmode_display.c
>>> @@ -1294,6 +1294,7 @@ drmmode_do_crtc_dpms(xf86CrtcPtr crtc, int mode)
>>>       AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
>>>       CARD64 ust;
>>>       int ret;
>>> +    int i;
>>>         if (drmmode_crtc->dpms_mode == DPMSModeOn && mode !=
>>> DPMSModeOn) {
>>>           uint32_t seq;
>>> @@ -1341,6 +1342,11 @@ drmmode_do_crtc_dpms(xf86CrtcPtr crtc, int mode)
>>>               drmmode_crtc->interpolated_vblanks += delta_seq;
>>>           }
>>>   +        for (i = 0; i < CM_NUM_PROPS; i++) {
>>> +            if (i == CM_GAMMA_LUT_SIZE || i == CM_DEGAMMA_LUT_SIZE)
>>> +                continue;
>>> +            drmmode_crtc_push_cm_prop(crtc, i);
>>> +        }
>>>       }
>>>       drmmode_crtc->dpms_mode = mode;
>>>   }
>>>
>>
>> This and patch 11 smell like workarounds for a kernel issue. The kernel
>> should preserve the property values regardless of DPMS state.
>>
>> This probably explains something I just discovered: the legacy gamma LUT
>> becomes ineffective after turning a CRTC off and on again with DC,
>> whereas it's preserved without DC.
> 
> That's indeed a kernel issue, will look into it. This patch can be
> dropped once the kernel persists the properties across dpms.
> 
> In terms of Patch 11, which persists the properties across hotplugs, is
> it even a valid use-case? I tested with i915 and amdgpu non-dc drivers.
> Both don't seem to persist legacy gamma across hotplugs, or xrandr
> --output --off/--auto.

It persists for me (without this series) with amdgpu without DC on
xrandr --output --off/--auto, or explicitly moving an output between
different CRTCs. Those are definitely expected.

Not sure about hotplug; it's possible that something explicitly
re-initializes the legacy gamma in that case. But other than that, each
CRTC should preserve its values unless they are explicitly modified.


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