On Fri, Apr 16, 2021 at 2:53 PM Takashi Iwai <ti...@suse.de> wrote: > > Currently the DRM fbcon helper for console blank, > drm_fb_helper_blank(), simply calls drm_fb_helper_dpms() and always > returns zero, supposing the driver dealing with DPMS or atomic > crtc->active flip to handle blanking the screen. It works on most of > devices, but broken on most of KVM/QEMU graphics: bochs, qxl and > cirrus drivers just ignore crtc->active state change as blanking (or > cirrus ignoring DPMS). In practice, when you run like > % setterm --blank force > on a VT console, the screen freezes without actually blanking. > > A simple fix for this problem would be not to rely on DPMS but let > fbcon performs the generic blank code. This can be achieved just by > returning an error from drm_fb_helper_blank(). > > In this patch, we add a flag, no_dpms_blank, to drm_fb_helper for > indicating that the driver doesn't handle blank via DPMS or > crtc->active flip. When this flag is set, drm_fb_helper_blank() > simply returns an error, so that fbcon falls back to its generic blank > handler. The flag is set to both bochs and qxl drivers in this patch, > while cirrus is left untouched as it's declared as to-be-deprecated. > > Link: https://lore.kernel.org/dri-devel/20170726205636.19144-1-ti...@suse.de/ > BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1095700 > Signed-off-by: Takashi Iwai <ti...@suse.de>
Uh we're supposed to fix these drivers to actually blank (scan out black, worst case), not paper over it in higher levels. Atomic kms is meant to be a somewhat useful abstraction. So now fbcon blanks, but your desktop still just freezes. > --- > > Here I whip a dead horse again, revisiting the long-standing issue > stated in the previous patch set in 2017: > https://lore.kernel.org/dri-devel/20170726205636.19144-1-ti...@suse.de/ > > I thought to refresh the previous patch set at first, but it seems > invalid for the atomic modeset case. And for the atomic, it's even > more difficult to propagate the return from the bottom to up. > So I ended up with this approach as it's much simpler. Yeah that's because atomic assume you can at least blank your screen to black. -Daniel > But if there is any better way (even simpler or more robust), I'd > happily rewrite, too. > > --- > drivers/gpu/drm/bochs/bochs_drv.c | 3 +++ > drivers/gpu/drm/drm_fb_helper.c | 5 +++++ > drivers/gpu/drm/qxl/qxl_drv.c | 3 +++ > include/drm/drm_fb_helper.h | 8 ++++++++ > 4 files changed, 19 insertions(+) > > diff --git a/drivers/gpu/drm/bochs/bochs_drv.c > b/drivers/gpu/drm/bochs/bochs_drv.c > index b469624fe40d..816899a266ff 100644 > --- a/drivers/gpu/drm/bochs/bochs_drv.c > +++ b/drivers/gpu/drm/bochs/bochs_drv.c > @@ -132,6 +132,9 @@ static int bochs_pci_probe(struct pci_dev *pdev, > goto err_unload; > > drm_fbdev_generic_setup(dev, 32); > + if (dev->fb_helper) > + dev->fb_helper->no_dpms_blank = true; > + > return ret; > > err_unload: > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index f6baa2046124..b892f02ff2f1 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -332,9 +332,14 @@ static void drm_fb_helper_dpms(struct fb_info *info, int > dpms_mode) > */ > int drm_fb_helper_blank(int blank, struct fb_info *info) > { > + struct drm_fb_helper *fb_helper = info->par; > + > if (oops_in_progress) > return -EBUSY; > > + if (fb_helper->no_dpms_blank) > + return -EINVAL; > + > switch (blank) { > /* Display: On; HSync: On, VSync: On */ > case FB_BLANK_UNBLANK: > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c > index 1864467f1063..58ecfaeed7c1 100644 > --- a/drivers/gpu/drm/qxl/qxl_drv.c > +++ b/drivers/gpu/drm/qxl/qxl_drv.c > @@ -120,6 +120,9 @@ qxl_pci_probe(struct pci_dev *pdev, const struct > pci_device_id *ent) > goto modeset_cleanup; > > drm_fbdev_generic_setup(&qdev->ddev, 32); > + if (qdev->fb_helper) > + qdev->fb_helper->no_dpms_blank = true; > + > return 0; > > modeset_cleanup: > diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h > index 3b273f9ca39a..151be4219c32 100644 > --- a/include/drm/drm_fb_helper.h > +++ b/include/drm/drm_fb_helper.h > @@ -176,6 +176,14 @@ struct drm_fb_helper { > */ > bool deferred_setup; > > + /** > + * @no_dpms_blank: > + * > + * A flag indicating that the driver doesn't support blanking. > + * Then fbcon core code falls back to its generic handler. > + */ > + bool no_dpms_blank; > + > /** > * @preferred_bpp: > * > -- > 2.26.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
