On Tue, 2018-02-13 at 22:59 +0000, Chris Wilson wrote:
> Quoting Pandiyan, Dhinakaran (2018-02-13 22:45:55)
> >
> >
> >
> > On Tue, 2018-02-13 at 22:15 +0000, Chris Wilson wrote:
> > > Quoting Pandiyan, Dhinakaran (2018-02-13 22:10:48)
> > > >
> > > >
> > > >
> > > > On Tue, 2018-02-13 at 21:54 +0000, Chris Wilson wrote:
> > > > > Quoting Dhinakaran Pandiyan (2018-02-13 21:46:13)
> > > > > > DRM_IOCTL_MODE_CURSOR results in a frontbuffer flush before the
> > > > > > cursor
> > > > > > plane MMIOs are written to. But this flush is not necessary for PSR
> > > > > > as
> > > > > > hardware tracking takes care of exiting PSR when the MMIO's are
> > > > > > written.
> > > > > >
> > > > > > Introduce a new fb_op_origin enum to differentiate flushes due to a
> > > > > > BO
> > > > > > being pinned from those originating due to a dirty fbdev buffer.
> > > > > > Now, this
> > > > > > enum can be ignored in psr_flush and psr_invalidate.
> > > > > >
> > > > > > v2: Update comment in i915_gem_object_pin_to_display_plane. (Chris)
> > > > > >
> > > > > > Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
> > > > > > Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > > > > > Cc: Chris Wilson <ch...@chris-wilson.co.uk>
> > > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com>
> > > > > > ---
> > > > > > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > > > > > drivers/gpu/drm/i915/i915_gem.c | 11 +++++++++--
> > > > > > drivers/gpu/drm/i915/intel_psr.c | 6 ++++--
> > > > > > 3 files changed, 14 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > > > > b/drivers/gpu/drm/i915/i915_drv.h
> > > > > > index 81886b74c750..3bf6c6ec0509 100644
> > > > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > > > @@ -637,6 +637,7 @@ enum fb_op_origin {
> > > > > > ORIGIN_CS,
> > > > > > ORIGIN_FLIP,
> > > > > > ORIGIN_DIRTYFB,
> > > > > > + ORIGIN_PINNEDFB,
> > > > > > };
> > > > > >
> > > > > > struct intel_fbc {
> > > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > > > > > b/drivers/gpu/drm/i915/i915_gem.c
> > > > > > index fc68b35854df..405acf3562de 100644
> > > > > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > > > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > > > > @@ -4139,9 +4139,16 @@ 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() */
> > > > > > + /* Treat this as an end-of-frame, like
> > > > > > intel_user_framebuffer_dirty() to
> > > > > > + * flush the caches.
> > > > > > + */
> > > > > > __i915_gem_object_flush_for_display(obj);
> > > > > > - intel_fb_obj_flush(obj, ORIGIN_DIRTYFB);
> > > > > > +
> > > > > > + /* Features like PSR might want to rely on HW to do the
> > > > > > frontbuffer
> > > > > > + * flush, pass origin as ORIGIN_PINNEDFB rather than
> > > > > > ORIGIN_DIRTYFB
> > > > > > + * so that their flush implementations can handle it
> > > > > > accordingly.
> > > > > > + */
> > > > >
> > > > > So why it is different? Why can't the dirtyfb ioctl benefit from HW,
> > > > > which the
> > > > > application is meant to call every frame to *all* dirty framebuffers
> > > > > (which include cursors in atomic)?
> > > > >
> > > >
> > > > Because the hardware requires a write to one of the pipe registers. When
> > > > applications write to the buffer via fbdev, it doesn't lead to pipe MMIO
> > > > write and hence does not benefit from HW triggered PSR exit.
> > >
> > > Somewhere you have to have that explanation, that you rely on a
> > > subsequent mmioflip of the framebuffer to trigger the frontbuffer flush.
> >
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index 405acf3562de..c669fef5d388 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -4071,9 +4071,13 @@ 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 and this may not lead to the buffer being scanned out if
> > + * intel_fb_obj_flush() consumers ignore ORIGIN_PINNEDFB. This is
> > useful because
> > + * features like PSR can delegate the flush to HW, which gets triggered
> > when
> > + * flip or cursor move MMIO's are written to.
> > */
> > struct i915_vma *
> > i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
> >
> >
> > Like this?
> >
> >
> >
> > > That probably also deserves lifting out of pin_to_display_plane as
> > > currently there's no requirement that pin_to_display is followed by a
> > > flip.
> >
> > Does pin_to_display imply ready to scan out? And I didn't get the part
> > about "lifting out of pin_to_display_plane"
>
> Nothing about flushing the GEM caches for coherency with the display
> engine imply how it will be used. Since PINNEDFB only works if the
> caller follows it with a mmioflip, it should be moved to the caller or
> that information passed in.
>
> So how is PINNEDFB different from FLIP? Once you move it to the callers,
> doesn't it just reduce to FLIP in the cases where you want to treat it
> as FLIP and DIRTYFB in the others?
Good point, let me revisit the idea of reusing FLIP now that I have a
slightly better understanding of how this is supposed to work.
-DK
> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
