Re: [PATCH] drm/i915: dirty fb operation flushsing frontbuffer

Daniel Vetter
2015-07-09T15:22:28Z

On Thu, Jul 09, 2015 at 10:04:20AM -0300, Paulo Zanoni wrote:
> 2015-07-08 20:22 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
> > Let's do a frontbuffer flush on dirty fb.
> > To be used for DIRTYFB drm ioctl.
> >
> > This patch solves the biggest PSR known issue, that is
> > missed screen updates during boot, mainly when there is a splash
> > screen involved like Plymouth.
> >
> > Previously PSR was being invalidated by fbdev and Plymounth
> > was taking control with PSR yet invalidated and could get screen
> > updates normally. However with some atomic modeset changes
> > Pymouth modeset over ioctl was now causing frontbuffer flushes
> > making PSR gets back to work while it cannot track the
> > screen updates and exit properly.
> >
> > By adding this flush on dirtyfb we properly track frontbuffer
> > writes and properly exit PSR.
> >
> > Actually all mmap_wc users should call this dirty callback
> > in order to have a proper frontbuffer tracking.
> >
> > In the future it can be extended to return 0 if the whole
> > screen has being flushed or the number of rects flushed
> > as Chris suggested.
> >
> > v2: Remove ORIGIN_FB_DIRTY and use ORIGIN_GTT instead since dirty
> > callback is just called after few screen updates and not on
> > everyone as pointed by Daniel.
> >
> > v3: Use flush instead of invalidate since flush means
> > invalidate + flush and dirty means drawn had finished and
> > it can be flushed.
> >
> > v4: Remove PSR from subject since it is purely frontbuffer tracking
> > change and that can be useful for FBC as well.
> >
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > 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_display.c | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 0efa455..1d67958 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14388,9 +14388,27 @@ static int intel_user_framebuffer_create_handle(struct drm_framebuffer *fb,
> > return drm_gem_handle_create(file, &obj->base, handle);
> > }
> >
> > +static int intel_user_framebuffer_dirty(struct drm_framebuffer *fb,
> > + struct drm_file *file,
> > + unsigned flags, unsigned color,
> > + struct drm_clip_rect *clips,
> > + unsigned num_clips)
>
> We still have all those useless white spaces above. Only the tabs are
> needed. I guess Daniel can remove these when applying the patch, so:
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Merged patches 2&3 from this series, thanks.
-Daniel

>
> > +{
> > + struct drm_device *dev = fb->dev;
> > + struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
> > + struct drm_i915_gem_object *obj = intel_fb->obj;
> > +
> > + mutex_lock(&dev->struct_mutex);
> > + intel_fb_obj_flush(obj, false, ORIGIN_GTT);
> > + mutex_unlock(&dev->struct_mutex);
> > +
> > + return 0;
> > +}
> > +
> > static const struct drm_framebuffer_funcs intel_fb_funcs = {
> > .destroy = intel_user_framebuffer_destroy,
> > .create_handle = intel_user_framebuffer_create_handle,
> > + .dirty = intel_user_framebuffer_dirty,
> > };
> >
> > static
> > --
> > 2.1.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni

--
Daniel Vetter
Software Engineer, Intel Corporation