[PATCH 1/5] drm: Propagate error from connector dpms function in drm_fb_helper_blank()

[Daniel Vetter]

On Wed, Jul 26, 2017 at 10:56:32PM +0200, Takashi Iwai 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 properly for
> blanking the screen.  However, it turned out that the console blank
> doesn't work at all on KVM/QEMU when DRM driver is used: most of the
> relevant drivers (bochs, qxl, and virtio) just ignore DPMS, and even
> cirrus driver doesn't work because the DPMS register bits the driver
> fiddles with are also ignored by KVM/QEMU.
> 
> 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 change the drm_fb_helper_dpms() to give back an
> error code returned from the connector dpms callback, so that the
> error is propagated to drm_fb_helper_blank().  After this change, each
> driver needs just to return an error to fall back to the generic fbcon
> blank mode.
> 
> Signed-off-by: Takashi Iwai <tiwai at suse.de>

This needs to be rebased onto -next, this code changed a lot.
-Daniel

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 30 ++++++++++++++++++++----------
>  1 file changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 574af01d3ce9..db31747ae598 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -581,21 +581,22 @@ static struct sysrq_key_op sysrq_drm_fb_helper_restore_op = {
>  static struct sysrq_key_op sysrq_drm_fb_helper_restore_op = { };
>  #endif
>  
> -static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
> +static int drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
>  {
>  	struct drm_fb_helper *fb_helper = info->par;
>  	struct drm_device *dev = fb_helper->dev;
>  	struct drm_crtc *crtc;
>  	struct drm_connector *connector;
>  	int i, j;
> +	int ret = 0;
>  
>  	/*
>  	 * For each CRTC in this fb, turn the connectors on/off.
>  	 */
>  	drm_modeset_lock_all(dev);
>  	if (!drm_fb_helper_is_bound(fb_helper)) {
> -		drm_modeset_unlock_all(dev);
> -		return;
> +		ret = -ENODEV;
> +		goto out;
>  	}
>  
>  	for (i = 0; i < fb_helper->crtc_count; i++) {
> @@ -607,12 +608,16 @@ static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
>  		/* Walk the connectors & encoders on this fb turning them on/off */
>  		drm_fb_helper_for_each_connector(fb_helper, j) {
>  			connector = fb_helper->connector_info[j]->connector;
> -			connector->funcs->dpms(connector, dpms_mode);
> +			ret = connector->funcs->dpms(connector, dpms_mode);
> +			if (ret < 0)
> +				goto out;
>  			drm_object_property_set_value(&connector->base,
>  				dev->mode_config.dpms_property, dpms_mode);
>  		}
>  	}
> + out:
>  	drm_modeset_unlock_all(dev);
> +	return ret;
>  }
>  
>  /**
> @@ -622,32 +627,37 @@ static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
>   */
>  int drm_fb_helper_blank(int blank, struct fb_info *info)
>  {
> +	int dpms_mode;
> +
>  	if (oops_in_progress)
>  		return -EBUSY;
>  
>  	switch (blank) {
>  	/* Display: On; HSync: On, VSync: On */
>  	case FB_BLANK_UNBLANK:
> -		drm_fb_helper_dpms(info, DRM_MODE_DPMS_ON);
> +		dpms_mode = DRM_MODE_DPMS_ON;
>  		break;
>  	/* Display: Off; HSync: On, VSync: On */
>  	case FB_BLANK_NORMAL:
> -		drm_fb_helper_dpms(info, DRM_MODE_DPMS_STANDBY);
> +		dpms_mode = DRM_MODE_DPMS_STANDBY;
>  		break;
>  	/* Display: Off; HSync: Off, VSync: On */
>  	case FB_BLANK_HSYNC_SUSPEND:
> -		drm_fb_helper_dpms(info, DRM_MODE_DPMS_STANDBY);
> +		dpms_mode = DRM_MODE_DPMS_STANDBY;
>  		break;
>  	/* Display: Off; HSync: On, VSync: Off */
>  	case FB_BLANK_VSYNC_SUSPEND:
> -		drm_fb_helper_dpms(info, DRM_MODE_DPMS_SUSPEND);
> +		dpms_mode = DRM_MODE_DPMS_SUSPEND;
>  		break;
>  	/* Display: Off; HSync: Off, VSync: Off */
>  	case FB_BLANK_POWERDOWN:
> -		drm_fb_helper_dpms(info, DRM_MODE_DPMS_OFF);
> +		dpms_mode = DRM_MODE_DPMS_OFF;
>  		break;
> +	default:
> +		return 0; /* ignored */
>  	}
> -	return 0;
> +
> +	return drm_fb_helper_dpms(info, dpms_mode);
>  }
>  EXPORT_SYMBOL(drm_fb_helper_blank);
>  
> -- 
> 2.13.3
> 
> _______________________________________________
> 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