2015-11-13 22:56 GMT-02:00 Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>:
> 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@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
> 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@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
