[PATCH 1/2] drm/i915: VLV/CHV PSR needs to exit PSR on every flush.

[PATCH 1/2] drm/i915: VLV/CHV PSR needs to exit PSR on every flush.

[Rodrigo Vivi]

ON these platforms we don't have hardware tracking working for any case.
So we need to fake this on software by forcing psr to exit on every
flush.

Manual tests indicated this was needed.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index dd0e6e0..afb8b8c 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -620,13 +620,11 @@ void intel_psr_flush(struct drm_device *dev,
 
 	/*
 	 * On Valleyview and Cherryview we don't use hardware tracking so
-	 * sprite plane updates or cursor moves don't result in a PSR
+	 * 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 (!HAS_DDI(dev) &&
-	    ((frontbuffer_bits & INTEL_FRONTBUFFER_SPRITE(pipe)) ||
-	     (frontbuffer_bits & INTEL_FRONTBUFFER_CURSOR(pipe))))
+	if (!HAS_DDI(dev))
 		intel_psr_exit(dev);
 
 	if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.

[Rodrigo Vivi]

Since active function on VLV immediately activate PSR let's give more
time for idleness.

v2: Rebase over intel_psr.c and fix typo.
v3: Revival: Manual tests indicated that this is needed. With a short delay there is a huge
    risk of getting blank screens when planes are being enabled.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index afb8b8c..a6028b6 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -597,6 +597,12 @@ void intel_psr_flush(struct drm_device *dev,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc;
 	enum pipe pipe;
+	/* On HSW/BDW Hardware controls idle_frames to go to PSR entry state
+	 * However on VLV we go to PSR active state with psr work. So let's
+	 * wait more time. The main reason is to give more time when primary
+	 * plane is getting enabled avoiding blank screens.
+	 */
+	int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 5000);
 
 	mutex_lock(&dev_priv->psr.lock);
 	if (!dev_priv->psr.enabled) {
@@ -629,7 +635,7 @@ void intel_psr_flush(struct drm_device *dev,
 
 	if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
 		schedule_delayed_work(&dev_priv->psr.work,
-				      msecs_to_jiffies(100));
+				      msecs_to_jiffies(delay));
 	mutex_unlock(&dev_priv->psr.lock);
 }
 
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  364/364              364/364
ILK                 -1              366/366              365/366
SNB                 -1              450/450              449/450
IVB              +17                 481/498              498/498
BYT                                  289/289              289/289
HSW                 -1              564/564              563/564
BDW                                  417/417              417/417
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*ILK  igt_gem_fenced_exec_thrash_no-spare-fences-busy-interruptible      PASS(1, M37)      DMESG_WARN(1, M37)
 SNB  igt_kms_force_connector      NRUN(5, M35M22)PASS(1, M35)      NRUN(1, M35)
 IVB  igt_kms_3d      DMESG_WARN(1, M34)PASS(15, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-128x128-onscreen      NSPT(1, M34)PASS(15, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-128x128-random      NSPT(1, M34)PASS(15, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-128x128-sliding      NSPT(1, M34)PASS(15, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-256x256-offscreen      NSPT(1, M34)PASS(15, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-256x256-onscreen      NSPT(1, M34)PASS(15, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-256x256-sliding      NSPT(1, M34)PASS(15, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-64x64-offscreen      NSPT(1, M34)PASS(15, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-64x64-onscreen      NSPT(1, M34)PASS(15, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-64x64-random      NSPT(1, M34)PASS(15, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-64x64-sliding      NSPT(1, M34)PASS(15, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-size-change      NSPT(1, M34)PASS(15, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_fence_pin_leak      NSPT(1, M34)PASS(15, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip      NSPT(1, M34)PASS(15, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip      NSPT(1, M34)PASS(15, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_rotation_crc_primary-rotation      NSPT(1, M34)PASS(15, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_rotation_crc_sprite-rotation      NSPT(1, M34)PASS(15, M4M34M21)      PASS(1, M34)
 HSW  igt_kms_force_connector      NRUN(5, M40M19M20)PASS(1, M40)      NRUN(1, M19)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.

[Daniel Vetter]

On Fri, Dec 05, 2014 at 08:40:42PM -0500, Rodrigo Vivi wrote:
> Since active function on VLV immediately activate PSR let's give more
> time for idleness.
> 
> v2: Rebase over intel_psr.c and fix typo.
> v3: Revival: Manual tests indicated that this is needed. With a short delay there is a huge
>     risk of getting blank screens when planes are being enabled.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index afb8b8c..a6028b6 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -597,6 +597,12 @@ void intel_psr_flush(struct drm_device *dev,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_crtc *crtc;
>  	enum pipe pipe;
> +	/* On HSW/BDW Hardware controls idle_frames to go to PSR entry state
> +	 * However on VLV we go to PSR active state with psr work. So let's
> +	 * wait more time. The main reason is to give more time when primary
> +	 * plane is getting enabled avoiding blank screens.
> +	 */
> +	int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 5000);

Same question as before: Shouldn't we look at vbt perhaps like on ddi
platforms to compute a reasonable delay? 5s is kinda not reasonable I
think ;-)
-Daniel

>  
>  	mutex_lock(&dev_priv->psr.lock);
>  	if (!dev_priv->psr.enabled) {
> @@ -629,7 +635,7 @@ void intel_psr_flush(struct drm_device *dev,
>  
>  	if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
>  		schedule_delayed_work(&dev_priv->psr.work,
> -				      msecs_to_jiffies(100));
> +				      msecs_to_jiffies(delay));
>  	mutex_unlock(&dev_priv->psr.lock);
>  }
>  
> -- 
> 1.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.

[Rodrigo Vivi]

On Mon, Dec 8, 2014 at 1:35 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Dec 05, 2014 at 08:40:42PM -0500, Rodrigo Vivi wrote:
>> Since active function on VLV immediately activate PSR let's give more
>> time for idleness.
>>
>> v2: Rebase over intel_psr.c and fix typo.
>> v3: Revival: Manual tests indicated that this is needed. With a short delay there is a huge
>>     risk of getting blank screens when planes are being enabled.
>>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_psr.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>> index afb8b8c..a6028b6 100644
>> --- a/drivers/gpu/drm/i915/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>> @@ -597,6 +597,12 @@ void intel_psr_flush(struct drm_device *dev,
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>>       struct drm_crtc *crtc;
>>       enum pipe pipe;
>> +     /* On HSW/BDW Hardware controls idle_frames to go to PSR entry state
>> +      * However on VLV we go to PSR active state with psr work. So let's
>> +      * wait more time. The main reason is to give more time when primary
>> +      * plane is getting enabled avoiding blank screens.
>> +      */
>> +     int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 5000);
>
> Same question as before: Shouldn't we look at vbt perhaps like on ddi
> platforms to compute a reasonable delay? 5s is kinda not reasonable I
> think ;-)

I agree 5s is an eternity here... to be honest I was using 1 sec here
but when sending the patch I reused the one I had sent before without
noticing it was 5.
Anyway, what kind of value at vbt do you suggest? If we use the tp1
wake for instance that is 50 here in multiple of 100 it will end up in
5 sec again.

Another thing that I notice today is that I also got that sink crc
issue on BDW. So it isn't only on VLV/CHV that we are unable to use
sink crc for test. I still believe that is the link fully off that is
making us to be unable to get panel crc when in psr.

But there is hope. I tested different values here and if we use 400 ms
I could use automated kms_psr_sink_crc here... with 300 and bellow it
fails.
The issue is that I'm not sure if it depends on different panel so
I'll test this value on other platforms and different panels.

Do you think 500ms reasonable to apply for all platforms in order to
keep automated tests and everything working well?

What do you think?

Thanks,
Rodrigo.

> -Daniel
>
>>
>>       mutex_lock(&dev_priv->psr.lock);
>>       if (!dev_priv->psr.enabled) {
>> @@ -629,7 +635,7 @@ void intel_psr_flush(struct drm_device *dev,
>>
>>       if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
>>               schedule_delayed_work(&dev_priv->psr.work,
>> -                                   msecs_to_jiffies(100));
>> +                                   msecs_to_jiffies(delay));
>>       mutex_unlock(&dev_priv->psr.lock);
>>  }
>>
>> --
>> 1.9.3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.

[Daniel Vetter]

On Fri, Dec 12, 2014 at 07:16:34PM -0800, Rodrigo Vivi wrote:
> On Mon, Dec 8, 2014 at 1:35 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Fri, Dec 05, 2014 at 08:40:42PM -0500, Rodrigo Vivi wrote:
> >> Since active function on VLV immediately activate PSR let's give more
> >> time for idleness.
> >>
> >> v2: Rebase over intel_psr.c and fix typo.
> >> v3: Revival: Manual tests indicated that this is needed. With a short delay there is a huge
> >>     risk of getting blank screens when planes are being enabled.
> >>
> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >> Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_psr.c | 8 +++++++-
> >>  1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> >> index afb8b8c..a6028b6 100644
> >> --- a/drivers/gpu/drm/i915/intel_psr.c
> >> +++ b/drivers/gpu/drm/i915/intel_psr.c
> >> @@ -597,6 +597,12 @@ void intel_psr_flush(struct drm_device *dev,
> >>       struct drm_i915_private *dev_priv = dev->dev_private;
> >>       struct drm_crtc *crtc;
> >>       enum pipe pipe;
> >> +     /* On HSW/BDW Hardware controls idle_frames to go to PSR entry state
> >> +      * However on VLV we go to PSR active state with psr work. So let's
> >> +      * wait more time. The main reason is to give more time when primary
> >> +      * plane is getting enabled avoiding blank screens.
> >> +      */
> >> +     int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 5000);
> >
> > Same question as before: Shouldn't we look at vbt perhaps like on ddi
> > platforms to compute a reasonable delay? 5s is kinda not reasonable I
> > think ;-)
> 
> I agree 5s is an eternity here... to be honest I was using 1 sec here
> but when sending the patch I reused the one I had sent before without
> noticing it was 5.
> Anyway, what kind of value at vbt do you suggest? If we use the tp1
> wake for instance that is 50 here in multiple of 100 it will end up in
> 5 sec again.

Yeah that's the one I was thinking off. So maybe 5s is indeed the right
value, but we should compute it from vbt?

> Another thing that I notice today is that I also got that sink crc
> issue on BDW. So it isn't only on VLV/CHV that we are unable to use
> sink crc for test. I still believe that is the link fully off that is
> making us to be unable to get panel crc when in psr.

Hm, maybe we need a special mode to disallow link shutdown for the psr
test? Then we could run all the functional correctness tests in that mode
(psr still works the same wrt frontbuffer tracking). Ofc that means we
need a new set of tests to exercise full psr (including link shutdown)
against e.g. suspend/resume and stuff to make sure that still works. But
those are then only doing modesets, s/r and won't check crcs. So
additional tests on top of what we have.

> But there is hope. I tested different values here and if we use 400 ms
> I could use automated kms_psr_sink_crc here... with 300 and bellow it
> fails.
> The issue is that I'm not sure if it depends on different panel so
> I'll test this value on other platforms and different panels.
> 
> Do you think 500ms reasonable to apply for all platforms in order to
> keep automated tests and everything working well?
> 
> What do you think?

Imo if vbt provides a delay, we should follow that. Even when it's
horrible, who knows what kind of broken panels we'll see ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx