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

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Jul 07, 2015 at 04:28:53PM -0700, Rodrigo Vivi wrote:
> 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.
> 
> 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 3c2425f..9a60d15 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14383,9 +14383,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)
> +{
> +	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);

There's some good discussion going on in some internal thread about what
to do with wc mmaps. The idea is to use the dirty ioctl to flush them, but
at least for fbc we already filter ORIGIN_GTT out, which means fbc won't
see the dirtyfb flushes for cpu wc mmaps. Should we have a ORIGIN_DIRTYFB,
which essentially just means "any kind of cpu frontbuffer rendering"?

Otoh we can make this change when Paulo implements the wc-mmap for
frontbuffers stuff.
-Daniel

> +
> +	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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx