2015-07-07 20:28 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>:
> Since flush actually means invalidate + flush we need to force psr
> exit on PSR flush.
>
> On Core platforms there is no way to do the sw tracking so we
There is no way to do the _HW_ tracking?
> simulate it by fully disable psr and reschedule a enable back.
> So a good idea is to minimize sequential disable/enable in cases we
> know that HW tracking like when flush has been originated by a flip.
> Also flip had just invalidated it already.
>
> It also uses origin to minimize the a bit the amount of
> disable/enabled, mainly when flip already had invalidated.
>
> With this patch in place it is possible to do a flush on dirty areas
> properly in a following patch.
>
> Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 3 +-
> drivers/gpu/drm/i915/intel_frontbuffer.c | 2 +-
> drivers/gpu/drm/i915/intel_psr.c | 51 +++++++++++++++++++++-----------
> 3 files changed, 36 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 1f3f786..c5005f2 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1329,7 +1329,8 @@ void intel_psr_disable(struct intel_dp *intel_dp);
> void intel_psr_invalidate(struct drm_device *dev,
> unsigned frontbuffer_bits);
> void intel_psr_flush(struct drm_device *dev,
> - unsigned frontbuffer_bits);
> + unsigned frontbuffer_bits,
> + enum fb_op_origin origin);
> void intel_psr_init(struct drm_device *dev);
> void intel_psr_single_frame_update(struct drm_device *dev,
> unsigned frontbuffer_bits);
> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
> index cb5a6f0..e73d2ff0 100644
> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
> @@ -128,7 +128,7 @@ void intel_frontbuffer_flush(struct drm_device *dev,
> return;
>
> intel_edp_drrs_flush(dev, frontbuffer_bits);
> - intel_psr_flush(dev, frontbuffer_bits);
> + intel_psr_flush(dev, frontbuffer_bits, origin);
> intel_fbc_flush(dev_priv, frontbuffer_bits);
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index d79ba58..dd174ae 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -680,6 +680,7 @@ void intel_psr_invalidate(struct drm_device *dev,
> * intel_psr_flush - Flush PSR
> * @dev: DRM device
> * @frontbuffer_bits: frontbuffer plane tracking bits
> + * @origin: which operation caused the flush
> *
> * Since the hardware frontbuffer tracking has gaps we need to integrate
> * with the software frontbuffer tracking. This function gets called every
> @@ -689,7 +690,7 @@ void intel_psr_invalidate(struct drm_device *dev,
> * Dirty frontbuffers relevant to PSR are tracked in busy_frontbuffer_bits.
> */
> void intel_psr_flush(struct drm_device *dev,
> - unsigned frontbuffer_bits)
> + unsigned frontbuffer_bits, enum fb_op_origin origin)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_crtc *crtc;
> @@ -707,24 +708,38 @@ void intel_psr_flush(struct drm_device *dev,
> frontbuffer_bits &= INTEL_FRONTBUFFER_ALL_MASK(pipe);
> dev_priv->psr.busy_frontbuffer_bits &= ~frontbuffer_bits;
>
> - /*
> - * On Haswell sprite plane updates don't result in a psr invalidating
> - * signal in the hardware. Which means we need to manually fake this in
> - * software for all flushes, not just when we've seen a preceding
> - * invalidation through frontbuffer rendering.
> - */
> - if (IS_HASWELL(dev) &&
> - (frontbuffer_bits & INTEL_FRONTBUFFER_SPRITE(pipe)))
> - intel_psr_exit(dev);
> + if (HAS_DDI(dev)) {
> + /*
> + * By definition every flush should mean invalidate + flush,
> + * however on core platforms let's minimize the
> + * disable/re-enable so we can avoid the invalidate when flip
> + * originated the flush.
> + */
> + if (frontbuffer_bits && origin != ORIGIN_FLIP)
> + intel_psr_exit(dev);
>
> - /*
> - * On Valleyview and Cherryview we don't use hardware tracking so
> - * any plane updates or cursor moves don't result in a PSR
> - * invalidating. Which means we need to manually fake this in
> - * software for all flushes, not just when we've seen a preceding
> - * invalidation through frontbuffer rendering. */
> - if (frontbuffer_bits && !HAS_DDI(dev))
> - intel_psr_exit(dev);
> + /*
> + * On Haswell sprite plane updates don't result in a psr
> + * invalidating signal in the hardware. Which means we need
> + * to manually fake this in software for all flushes, not just
> + * when we've seen a preceding invalidation through
> + * frontbuffer rendering.
> + */
> + if (IS_HASWELL(dev) &&
> + (frontbuffer_bits & INTEL_FRONTBUFFER_SPRITE(pipe)))
> + intel_psr_exit(dev);
With this, we'll call intel_psr_exit() twice for sprites. It seems we
never get ORIGIN_FLIP for sprites, so you can just kill these lines.
Everything else looks correct.
> +
> + } else {
> + /*
> + * On Valleyview and Cherryview we don't use hardware tracking
> + * so any plane updates or cursor moves don't result in a PSR
> + * invalidating. Which means we need to manually fake this in
> + * software for all flushes, not just when we've seen a
> + * preceding invalidation through frontbuffer rendering.
> + */
> + if (frontbuffer_bits)
> + intel_psr_exit(dev);
> + }
>
> if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
> schedule_delayed_work(&dev_priv->psr.work,
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
