Re: [PATCH xf86-video-amdgpu 08/13] Set driver-private CRTC's dpms mode on disable

["Michel Dänzer" <michel-otUistvHUpPR7s880joybQ@public.gmane.org>]

On 2018-05-17 11:43 PM, Leo Li wrote:
> On 2018-05-16 01:09 PM, Michel Dänzer wrote:
>> On 2018-05-03 08:31 PM, sunpeng.li@amd.com wrote:
>>> From: "Leo (Sunpeng) Li" <sunpeng.li@amd.com>
>>>
>>> The dpms_mode flag on the driver-private CRTC was not being set when
>>> it's DPMS state is set to off. This causes some problems when toggling
>>> it back on, as some conditionals check this flag.
>>>
>>> Signed-off-by: Leo (Sunpeng) Li <sunpeng.li@amd.com>
>>> ---
>>>   src/drmmode_display.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
>>> index 2b38a71..f86f99a 100644
>>> --- a/src/drmmode_display.c
>>> +++ b/src/drmmode_display.c
>>> @@ -347,6 +347,7 @@ drmmode_crtc_dpms(xf86CrtcPtr crtc, int mode)
>>>           drmModeSetCrtc(pAMDGPUEnt->fd,
>>> drmmode_crtc->mode_crtc->crtc_id,
>>>                      0, 0, 0, NULL, 0, NULL);
>>>           drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->fb, NULL);
>>> +        drmmode_crtc->dpms_mode = mode;
>>>       } else if (drmmode_crtc->dpms_mode != DPMSModeOn)
>>>           crtc->funcs->set_mode_major(crtc, &crtc->mode, crtc->rotation,
>>>                           crtc->x, crtc->y);
>>>
>>
>> drmmode_crtc->dpms_mode is updated in drmmode_do_crtc_dpms. I'm a bit
>> worried that doing it here as well might cause subtle breakage. Is this
>> related to patches 10 & 11, or can you describe the scenario that
>> prompted you to make this change?
>>
> 
> After reading your response to patch 10, and the cover letter, I think
> this patch could be dropped. See my reply to patch 10 for details.
> 
> I originally had this change for the `xrandr --output --off` case.
> Unlike xset dpms off, it wasn't triggering drmmode_do_crtc_dpms, and
> therefore not setting the dpms_mode. This causes drmmode_do_crtc_dpms to
> skip over a section of the code when output is turned back on, where
> patch 10 resides in.

Indeed, that's a good catch, thanks. However, it's better to make sure
drmmode_do_crtc_dpms is called in this case as well, please take a look
at https://patchwork.freedesktop.org/patch/224016/ .


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx