Re: [Intel-gfx] [PATCH v2] drm/i915/psr: HW tracking for cursor moves to fix lags.

[Pandiyan, Dhinakaran]

On Tue, 2018-02-13 at 22:59 +, Chris Wilson wrote:
> Quoting Pandiyan, Dhinakaran (2018-02-13 22:45:55)
> > 
> > 
> > 
> > On Tue, 2018-02-13 at 22:15 +, Chris Wilson wrote:
> > > Quoting Pandiyan, Dhinakaran (2018-02-13 22:10:48)
> > > > 
> > > > 
> > > > 
> > > > On Tue, 2018-02-13 at 21:54 +, 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 
> > > > > > Cc: Ville Syrjälä 
> > > > > > Cc: Chris Wilson 
> > > > > > Signed-off-by: Dhinakaran Pandiyan 
> > > > > > ---
> > > > > >  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, 

Re: [Intel-gfx] [PATCH v2] drm/i915/psr: HW tracking for cursor moves to fix lags.

[Chris Wilson]

Quoting Pandiyan, Dhinakaran (2018-02-13 22:45:55)
> 
> 
> 
> On Tue, 2018-02-13 at 22:15 +, Chris Wilson wrote:
> > Quoting Pandiyan, Dhinakaran (2018-02-13 22:10:48)
> > > 
> > > 
> > > 
> > > On Tue, 2018-02-13 at 21:54 +, 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 
> > > > > Cc: Ville Syrjälä 
> > > > > Cc: Chris Wilson 
> > > > > Signed-off-by: Dhinakaran Pandiyan 
> > > > > ---
> > > > >  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 

Re: [Intel-gfx] [PATCH v2] drm/i915/psr: HW tracking for cursor moves to fix lags.

[Pandiyan, Dhinakaran]

On Tue, 2018-02-13 at 22:15 +, Chris Wilson wrote:
> Quoting Pandiyan, Dhinakaran (2018-02-13 22:10:48)
> > 
> > 
> > 
> > On Tue, 2018-02-13 at 21:54 +, 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 
> > > > Cc: Ville Syrjälä 
> > > > Cc: Chris Wilson 
> > > > Signed-off-by: Dhinakaran Pandiyan 
> > > > ---
> > > >  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"



-DK


> -Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org

Re: [Intel-gfx] [PATCH v2] drm/i915/psr: HW tracking for cursor moves to fix lags.

[Chris Wilson]

Quoting Pandiyan, Dhinakaran (2018-02-13 22:10:48)
> 
> 
> 
> On Tue, 2018-02-13 at 21:54 +, 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 
> > > Cc: Ville Syrjälä 
> > > Cc: Chris Wilson 
> > > Signed-off-by: Dhinakaran Pandiyan 
> > > ---
> > >  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.
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.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915/psr: HW tracking for cursor moves to fix lags.

[Pandiyan, Dhinakaran]

On Tue, 2018-02-13 at 21:54 +, 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 
> > Cc: Ville Syrjälä 
> > Cc: Chris Wilson 
> > Signed-off-by: Dhinakaran Pandiyan 
> > ---
> >  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.




> > +   intel_fb_obj_flush(obj, ORIGIN_PINNEDFB);
> ___
> 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

Re: [Intel-gfx] [PATCH v2] drm/i915/psr: HW tracking for cursor moves to fix lags.

[Chris Wilson]

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 
> Cc: Ville Syrjälä 
> Cc: Chris Wilson 
> Signed-off-by: Dhinakaran Pandiyan 
> ---
>  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)?

> +   intel_fb_obj_flush(obj, ORIGIN_PINNEDFB);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx