Re: [PATCH] drm/i915: fbdev_set_par reliably invalidating frontbuffer

From: "Vivi, Rodrigo" <rodrigo.vivi@intel.com>
To: "daniel@ffwll.ch" <daniel@ffwll.ch>
Cc: "daniel.vetter@ffwll.ch" <daniel.vetter@ffwll.ch>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Zanoni, Paulo R" <paulo.r.zanoni@intel.com>
Subject: Re: [PATCH] drm/i915: fbdev_set_par reliably invalidating frontbuffer
Date: Thu, 9 Jul 2015 20:40:22 +0000	[thread overview]
Message-ID: <1436474764.2893.10.camel@intel.com> (raw)
In-Reply-To: <20150709195628.GK7410@phenom.ffwll.local>

On Thu, 2015-07-09 at 21:56 +0200, Daniel Vetter wrote:
> On Thu, Jul 09, 2015 at 03:46:17PM -0300, Paulo Zanoni wrote:
> > 2015-07-09 13:56 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
> > > fbdev_set_par is called when fbcon is taking over control.
> > > In the past frontbuffer was being invalidated on
> > > set_to_gtt_domain, but it moved to set_domain fixing that case,
> > > but left this behind.
> > >
> > > Commit that changed this fixing set_domain case was:
> > >
> > > commit 031b698a77a70a6c394568034437b5486a44e868
> > > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Date:   Fri Jun 26 19:35:16 2015 +0200
> > >
> > >     drm/i915: Unconditionally do fb tracking invalidate in set_domain
> > >
> > > Since we are also invalidating in other fbdev cases this one
> > > was masked here. At least until now that I found this corner
> > > case: On boot with plymouth doing a splash screen
> > > when returning to the console frontbuffer wans't being invalidated
> > > causing missed screen updates with PSR enabled.
> > >
> > > So this patch fixes this issue.
> > >
> > > v2: Make invalidate directly and unconditionally and
> > >     fix commit message indicating the set_domain fix
> > >     as pointed out by Daniel.
> > > v3: Remove unecessary if(obj) added by mistake
> > 
> > The commit message is a little confusing. Also, I'm not 100% sure of
> > this since I'm not the fbcon specialist, but apparently this commit is
> > needed for the FBC test i just submitted as a reply to patch 7. So:
> 
> I clarified the commit message a bit and pulled in all the remaining
> patches from this series.

Thank you very much Paulo and Daniel.
I'll do the igt changes that Paulo had requested.

> 
> Thanks, Daniel
> > 
> > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > 
> > >
> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_fbdev.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > > index 2a1724e..25ce7b6 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > > @@ -63,8 +63,7 @@ static int intel_fbdev_set_par(struct fb_info *info)
> > >                  * now until we solve this for real.
> > >                  */
> > >                 mutex_lock(&fb_helper->dev->struct_mutex);
> > > -               ret = i915_gem_object_set_to_gtt_domain(ifbdev->fb->obj,
> > > -                                                       true);
> > > +               intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT);
> > >                 mutex_unlock(&fb_helper->dev->struct_mutex);
> > >         }
> > >
> > > --
> > > 2.1.0
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > 
> > 
> > -- 
> > Paulo Zanoni
> 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx