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

Paulo Zanoni
2015-07-09T13:04:21Z

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>

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