[Intel-gfx] [PATCH] drm/i915: Make sure we invalidate frontbuffer on fbcon.

[Daniel Vetter]

On Mon, Mar 09, 2015 at 05:57:07PM -0700, Rodrigo Vivi wrote:
> There are some cases like suspend/resume or dpms off/on sequences
> that can flush frontbuffer bits. In these cases features that relies
> on frontbuffer tracking can start working and user can stop getting
> screen updates on fbcon having impression the system is frozen.
> 
> So, let's make sure we also invalidate frontbuffer on fbdev blank.
> 
> v2: Daniel was right, backtrace didn't show other path than this blank
> one so let's make sure frontbuffer bits gets invalidate here instead of
> on random write operations that doesn't garantee we track all frontbuffer
> writes.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_fbdev.c | 36 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 234a699..324b160 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -71,6 +71,40 @@ static int intel_fbdev_set_par(struct fb_info *info)
>  	return ret;
>  }
>  
> +static int intel_fbdev_blank(int blank, struct fb_info *info)
> +{
> +	struct drm_fb_helper *fb_helper = info->par;
> +	struct intel_fbdev *ifbdev =
> +		container_of(fb_helper, struct intel_fbdev, helper);
> +	int ret;
> +
> +	ret = drm_fb_helper_blank(blank, info);
> +
> +	if (ret == 0) {
> +		/*
> +		 * FIXME: After dpms off/on sequence on fbdev frontbuffer bits
> +		 * gets flushed so, let's set to gtt domain again when
> +		 * restoring the panel, at least while we don't have a
> +		 * propper solution.
> +		  */

No FIXME required, this imo _is_ the right fix. But we need a FIXME about
the panic locking inversion instead.

> +		mutex_lock(&fb_helper->dev->struct_mutex);
> +
> +		/*
> +		 * There are some cases that can flush frontbuffer bits
> +		 * while we are still on console. Like when planes gets
> +		 * enabled/disabled on blank/unblank. So, let's be sure
> +		 * the fb obj gets invalidated when touching blank function.
> +		 * It is already on gtt domain, so we just invalidate it
> +		 * making sure components trusting frontbuffer tracking
> +		 * still gets invalidate after blank/unblank sequence.
> +		 */

Since this mirros fbdev_set_par and we don't have a comment there I think
this one isn't needed. The commit message explains it all well already.

Merged the patche with the code comments exchanged, thanks.
-Daniel

> +		intel_fb_obj_invalidate(ifbdev->fb->obj, NULL, ORIGIN_GTT);
> +		mutex_unlock(&fb_helper->dev->struct_mutex);
> +	}
> +
> +	return ret;
> +}
> +
>  static struct fb_ops intelfb_ops = {
>  	.owner = THIS_MODULE,
>  	.fb_check_var = drm_fb_helper_check_var,
> @@ -79,7 +113,7 @@ static struct fb_ops intelfb_ops = {
>  	.fb_copyarea = cfb_copyarea,
>  	.fb_imageblit = cfb_imageblit,
>  	.fb_pan_display = drm_fb_helper_pan_display,
> -	.fb_blank = drm_fb_helper_blank,
> +	.fb_blank = intel_fbdev_blank,
>  	.fb_setcmap = drm_fb_helper_setcmap,
>  	.fb_debug_enter = drm_fb_helper_debug_enter,
>  	.fb_debug_leave = drm_fb_helper_debug_leave,
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch