Quoting Dhinakaran Pandiyan (2018-02-16 04:33:18)
> i915_gem_obj_pin_to_display() calls frontbuffer_flush with origin set to
> DIRTYFB. The callers however are at a vantage point to decide if hardware
> frontbuffer tracking can do the flush for us. For example, legacy cursor
> updates, like flips, write to MMIO registers, which then triggers PSR flush
> by the hardware. Moving frontbuffer_flush out will enable us to skip a
> software initiated flush by setting origin to FLIP. Thanks to Chris for the
> idea.
>
> Cc: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Cc: Paulo Zanoni <paulo.r.zan...@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 9 ++++-----
> drivers/gpu/drm/i915/intel_display.c | 13 ++++++++++---
> drivers/gpu/drm/i915/intel_fbdev.c | 6 +++---
> drivers/gpu/drm/i915/intel_overlay.c | 1 +
> 4 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index fc68b35854df..22b598e1e0b9 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4071,9 +4071,10 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev,
> void *data,
> }
>
> /*
> - * Prepare buffer for display plane (scanout, cursors, etc).
> - * Can be called from an uninterruptible phase (modesetting) and allows
> - * any flushes to be pipelined (for pageflips).
> + * Prepare buffer for display plane (scanout, cursors, etc). Can be called
> from
> + * an uninterruptible phase (modesetting) and allows any flushes to be
> pipelined
> + * (for pageflips). We only flush the caches while preparing the buffer for
> + * display, the callers are responsible for frontbuffer flush.
> */
> struct i915_vma *
> i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
> @@ -4139,9 +4140,7 @@ i915_gem_object_pin_to_display_plane(struct
> drm_i915_gem_object *obj,
>
> vma->display_alignment = max_t(u64, vma->display_alignment,
> alignment);
>
> - /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty()
> */
> __i915_gem_object_flush_for_display(obj);
> - intel_fb_obj_flush(obj, ORIGIN_DIRTYFB);
>
> /* It should now be out of any other write domains, and we can update
> * the domain values for our changes.
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 286a9591d179..2b70714ead0f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2806,10 +2806,15 @@ intel_find_initial_plane_obj(struct intel_crtc
> *intel_crtc,
> return;
>
> valid_fb:
> + obj = intel_fb_obj(fb);
> mutex_lock(&dev->struct_mutex);
> intel_state->vma =
> intel_pin_and_fence_fb_obj(fb, primary->state->rotation);
> +
> + if (!IS_ERR(intel_state->vma))
> + intel_fb_obj_flush(obj, ORIGIN_DIRTYFB);
The fb_obj_flush doesn't need to be inside the lock, so you can do it
after the normal error testing (or just do it since the flush doesn't
depend on having the vma bound).
> mutex_unlock(&dev->struct_mutex);
> +
> if (IS_ERR(intel_state->vma)) {
> DRM_ERROR("failed to pin boot fb on pipe %d: %li\n",
> intel_crtc->pipe, PTR_ERR(intel_state->vma));
> @@ -12713,10 +12717,12 @@ intel_prepare_plane_fb(struct drm_plane *plane,
> struct i915_vma *vma;
>
> vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
> - if (!IS_ERR(vma))
> + if (!IS_ERR(vma)) {
> to_intel_plane_state(new_state)->vma = vma;
> - else
> + intel_fb_obj_flush(obj, ORIGIN_DIRTYFB);
> + } else {
> ret = PTR_ERR(vma);
Would
if (IS_ERR(vma)) {
ret = PTR_ERR(vma);
break;
}
intel_fb_obj_flush(obj, ORIGIN_DIRTYFB);
to_intel_plane_state(new_state)->vma = vma;
look neater here?
Lgtm,
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
