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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
