On Fri, Apr 16, 2021 at 2:53 PM Takashi Iwai <tiwai at 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-tiwai at
suse.de/
> BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1095700
> Signed-off-by: Takashi Iwai <tiwai at 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-tiwai at
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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch