[Intel-gfx] [PATCH] drm/i915: PSR: Let's rely more on frontbuffer tracking.

[Paulo Zanoni]

2015-11-13 22:56 GMT-02:00 Rodrigo Vivi <rodrigo.vivi at intel.com>:
> The ultimate goal here is to remove the dependency we
> currently have on audio driver power to get PSR working.
>
> With audio driver runtime PM disabled the Hardware tracking
> believes graphics is fully active and prevent PSR Entry, or
> in other words continuously exit PSR.
>
> When we introduced PSR we let LPSP masked allowing us
> to get PSR independtly from the audio runtime PM. However
> in one of the attempts to get PSR enabled by default one
> user reported one specific case where he would miss
> screen updates if scrolling the firefox in a Gnome
> environment when i915 runtime pm was enabled. So for
> this specific case that (I could never create an i-g-t
> test case) we decided to remove the LPSP mask and
> let HW tracking taking care of this case. So, we
> started depending on audio driver again.

Thanks for the better commit message, but I still think there's a huge
gap between the paragraph above and the paragraph below. What is still
not clear to me is: what is the relationship between the LPSP mask
problem mentioned above and the automatic page flip tracking mentioned
below? In other words: why not relying on the automatic HW tracking
solves the bug?

Also, did you do any measurements on how this patch affects PC state
residency on the affected platforms? I expect to see some delta here.

The patch itself looks fine. And we can always look into re-adding the
HW piece after we get the current bugs sorted.

With those two explained in the commit message:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

>
> An alternative solution that makes us indepent and also
> solve this case is to fully rely on our frontbuffer
> tracking that is really mature right now.
>
> From the frontbuffer tracking perspective the flush means
> invalidate and flush. But without this patch for HSW, BDW
> and SKL we just do the invalidate part when the flush wasn't
> originated by a page flip because we were trusting the HW
> tracking for the flip case, that is exactly the case with
> firefox scrolling on gnome.
>
> So let's rely more on frontbuffer tracking and do the
> invalidation regardless the origin as expected and for
> all platforms.
>
> v2: Improve commit message as suggested by Paulo.
>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 22 +++-------------------
>  1 file changed, 3 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index e5b3fce..b0e343c 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -711,25 +711,9 @@ void intel_psr_flush(struct drm_device *dev,
>         frontbuffer_bits &= INTEL_FRONTBUFFER_ALL_MASK(pipe);
>         dev_priv->psr.busy_frontbuffer_bits &= ~frontbuffer_bits;
>
> -       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);
> -       } 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.
> -                */
> -               if (frontbuffer_bits)
> -                       intel_psr_exit(dev);
> -       }
> +       /* By definition flush = invalidate + flush */
> +       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.4.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni