[PATCH] Disable CRTCs when disabling the outputs

[Michel Dänzer]

On 23.06.2015 16:45, Piotr Redlewski wrote:
> 
> 2015-06-23 8:28 GMT+02:00 Michel Dänzer <michel at daenzer.net
> <mailto:michel at daenzer.net>>:
> 
>     On 23.06.2015 04:19, Piotr Redlewski wrote:
>     >
>     > 2015-06-22 12:01 GMT+02:00 Piotr Redlewski <predlewski at gmail.com <mailto:predlewski at gmail.com>
>     > <mailto:predlewski at gmail.com <mailto:predlewski at gmail.com>>>:
>     >
>     >     2015-06-22 9:49 GMT+02:00 Michel Dänzer <michel at daenzer.net <mailto:michel at daenzer.net>
>     >     <mailto:michel at daenzer.net <mailto:michel at daenzer.net>>>:
>     >
>     >         On 21.06.2015 23 <tel:21.06.2015%2023> <tel:21.06.2015%2023>:30, Piotr
>     Redlewski wrote:
>     >         ​​
>     >         drmmode_crtc_dpms(xf86CrtcPtr crtc, int mode)
>     >         >  {
>     >         > -     /* Nothing to do. drmmode_do_crtc_dpms() is called
>     as appropriate */
>     >         > +     drmmode_crtc_private_ptr drmmode_crtc =
>     crtc->driver_private;
>     >         > +     drmmode_ptr drmmode = drmmode_crtc->drmmode;
>     >         > +
>     >         > +     /* Disable unused CRTCs */
>     >         > +     if (!crtc->enabled && mode != DPMSModeOn) {
>     >
>     >         Maybe this should be
>     >
>     >                 if (!crtc->enabled || mode != DPMSModeOn) {
>     >
>     >         or can it actually happen that
>     >         ​​
>     >         !crtc->enabled and mode == DPMSModeOn?
>     >
>     >     ​Just wanted to be safe here, but I think you are right. I'll send
>     >     v2 later today.
>     >
>     > ​I wanted to test changes before sending v2 and I stumbled on one
>     issue.
>     > Everything works as expected when xrandr is used, but problem starts
>     > when screensaver
>     > ​comes into play. X calls
>     > drmmode_crtc_dpms() to on/off screensaver, so when it is supposed
>     to be
>     > enabled, function is called with mode=DPMSModeOff and we
>     > call drmModeSetCrtc() to set crtc's mode to NULL. Problem is when X
>     > wants to disable screensaver: drmmode_crtc_dpms() is called with
>     > mode=DPMSModeOn and... nothing happens as there is no logic for this
>     > scenario - we should call drmModeSetCrtc() to set valid mode, but we
>     > don't know what this mode should be.
> 
>     Isn't crtc->mode valid at that point?
> 
> ​I don't think so, but I'll double check it​

If it's not valid, I'd expect drmmode_set_mode_major() to get called by
the X server.


>     If it's not, maybe we can store the mode in
>     ​​
>     drmmode_crtc and use that
>     instead.
> 
> ​crtc mode is one thing, but we need other parameters as well: x/y crtc
> offsets,
> buffer id​, connectors list - do you think that these also can be stored in 
> ​
> drmmode_crtc
> ​?​

Again I'm not sure how those can not be valid in the xf86CrtcRec when
the X server calls drmmode_crtc_dpms() but not drmmode_set_mode_major().


>     > ​Because of the situation with the screensaver I think that original
>     > version of the patch is ok (it solves the problem with CRTC staying
>     > enabled when using xrandr to disable the output).
> 
>     It would be nice to get the same effect for DPMS off though,
>     wouldn't it?
> 
> ​Of course, if I find a way to implement it I'll do it.

Thanks. If you want to tackle that in a followup patch, we can probably
just use if (!crtc->enabled) in this patch?


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