Re: [Intel-gfx] [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written

2018-04-25 Thread Souza, Jose
On Tue, 2018-04-24 at 17:16 -0700, Dhinakaran Pandiyan wrote:
> 
> 
> On Tue, 2018-04-24 at 14:20 -0700, Rodrigo Vivi wrote:
> > On Mon, Apr 23, 2018 at 05:42:40PM -0700, Souza, Jose wrote:
> > > On Fri, 2018-04-20 at 15:57 -0700, Rodrigo Vivi wrote:
> > > > On Fri, Apr 20, 2018 at 03:27:56PM -0700, José Roberto de Souza
> > > > wrote:
> > > > > Any write in any display register was causing HW to exit PSR,
> > > > > masking it to allow more power savings. Writes to pipe
> > > > > related
> > > > > registers will still cause HW to exit PSR.
> > > > > This is already masked for PSR2.
> > > > 
> > > > This seems a good idea indeed with the test case on
> > > > perspective.
> > > 
> > > what test cases are thinking? the current ones already do pages
> > > flips
> > > that will only touch the pipe related registers.
> > > 
> > > > 
> > > > But it needs more tests to make sure it doesn't break
> > > > "Display WA #0884: all"
> > > 
> > > I just tested the WA #0884 and it still causes PSR to exit. I
> > > have
> > > added a new debugfs that when read it writes to
> > > 'I915_WRITE(CURSURFLIVE(pipe), 0);' causing a PSR exit.
> > 
> > Interesting. Thanks a lot for checking this.
> > I guess we are safe then. DK?!
> > 
> 
> Not sure why it works though, let's give it a try.
> 
> It might be worth adding more information about test platform and how
> was this was tested in the commit message. Since this is not clearly
> documented in bspec, someone is going read this in a few months and
> think the code does not make any sense.
> 
> Reviewed-by: Dhinakaran Pandiyan 

Done

> 
> 
> > > 
> > > > 
> > > > Or we might need to revert that patch before moving with this
> > > > idea.
> > > > 
> > > > > 
> > > > > Bspec: 7721 and 8042
> > > > > 
> > > > > Cc: Rodrigo Vivi 
> > > > > Cc: Dhinakaran Pandiyan 
> > > > > Signed-off-by: José Roberto de Souza 
> > > > > ---
> > > > > 
> > > > > New patch in this series.
> > > > > 
> > > > >  drivers/gpu/drm/i915/intel_psr.c | 3 ++-
> > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > > index 0938df48107a..c907282dc82d 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > > @@ -712,7 +712,8 @@ static void hsw_psr_enable_source(struct
> > > > > intel_dp *intel_dp,
> > > > >   I915_WRITE(EDP_PSR_DEBUG,
> > > > >  EDP_PSR_DEBUG_MASK_MEMUP |
> > > > >  EDP_PSR_DEBUG_MASK_HPD |
> > > > > -EDP_PSR_DEBUG_MASK_LPSP);
> > > > > +EDP_PSR_DEBUG_MASK_LPSP |
> > > > > +EDP_PSR_DEBUG_MASK_DISP_REG_WRITE
> > > > > );
> > > > >   }
> > > > >  }
> > > > >  
> > > > > -- 
> > > > > 2.17.0
> > > > > 
> > 
> > ___
> > 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 v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written

2018-04-24 Thread Dhinakaran Pandiyan



On Tue, 2018-04-24 at 14:20 -0700, Rodrigo Vivi wrote:
> On Mon, Apr 23, 2018 at 05:42:40PM -0700, Souza, Jose wrote:
> > On Fri, 2018-04-20 at 15:57 -0700, Rodrigo Vivi wrote:
> > > On Fri, Apr 20, 2018 at 03:27:56PM -0700, José Roberto de Souza
> > > wrote:
> > > > Any write in any display register was causing HW to exit PSR,
> > > > masking it to allow more power savings. Writes to pipe related
> > > > registers will still cause HW to exit PSR.
> > > > This is already masked for PSR2.
> > > 
> > > This seems a good idea indeed with the test case on perspective.
> > 
> > what test cases are thinking? the current ones already do pages flips
> > that will only touch the pipe related registers.
> > 
> > > 
> > > But it needs more tests to make sure it doesn't break
> > > "Display WA #0884: all"
> > 
> > I just tested the WA #0884 and it still causes PSR to exit. I have
> > added a new debugfs that when read it writes to
> > 'I915_WRITE(CURSURFLIVE(pipe), 0);' causing a PSR exit.
> 
> Interesting. Thanks a lot for checking this.
> I guess we are safe then. DK?!
> 

Not sure why it works though, let's give it a try.

It might be worth adding more information about test platform and how
was this was tested in the commit message. Since this is not clearly
documented in bspec, someone is going read this in a few months and
think the code does not make any sense.

Reviewed-by: Dhinakaran Pandiyan 


> > 
> > > 
> > > Or we might need to revert that patch before moving with this idea.
> > > 
> > > > 
> > > > Bspec: 7721 and 8042
> > > > 
> > > > Cc: Rodrigo Vivi 
> > > > Cc: Dhinakaran Pandiyan 
> > > > Signed-off-by: José Roberto de Souza 
> > > > ---
> > > > 
> > > > New patch in this series.
> > > > 
> > > >  drivers/gpu/drm/i915/intel_psr.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > index 0938df48107a..c907282dc82d 100644
> > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > @@ -712,7 +712,8 @@ static void hsw_psr_enable_source(struct
> > > > intel_dp *intel_dp,
> > > > I915_WRITE(EDP_PSR_DEBUG,
> > > >EDP_PSR_DEBUG_MASK_MEMUP |
> > > >EDP_PSR_DEBUG_MASK_HPD |
> > > > -  EDP_PSR_DEBUG_MASK_LPSP);
> > > > +  EDP_PSR_DEBUG_MASK_LPSP |
> > > > +  EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
> > > > }
> > > >  }
> > > >  
> > > > -- 
> > > > 2.17.0
> > > > 
> ___
> 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 v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written

2018-04-24 Thread Rodrigo Vivi
On Mon, Apr 23, 2018 at 05:42:40PM -0700, Souza, Jose wrote:
> On Fri, 2018-04-20 at 15:57 -0700, Rodrigo Vivi wrote:
> > On Fri, Apr 20, 2018 at 03:27:56PM -0700, José Roberto de Souza
> > wrote:
> > > Any write in any display register was causing HW to exit PSR,
> > > masking it to allow more power savings. Writes to pipe related
> > > registers will still cause HW to exit PSR.
> > > This is already masked for PSR2.
> > 
> > This seems a good idea indeed with the test case on perspective.
> 
> what test cases are thinking? the current ones already do pages flips
> that will only touch the pipe related registers.
> 
> > 
> > But it needs more tests to make sure it doesn't break
> > "Display WA #0884: all"
> 
> I just tested the WA #0884 and it still causes PSR to exit. I have
> added a new debugfs that when read it writes to
> 'I915_WRITE(CURSURFLIVE(pipe), 0);' causing a PSR exit.

Interesting. Thanks a lot for checking this.
I guess we are safe then. DK?!

> 
> > 
> > Or we might need to revert that patch before moving with this idea.
> > 
> > > 
> > > Bspec: 7721 and 8042
> > > 
> > > Cc: Rodrigo Vivi 
> > > Cc: Dhinakaran Pandiyan 
> > > Signed-off-by: José Roberto de Souza 
> > > ---
> > > 
> > > New patch in this series.
> > > 
> > >  drivers/gpu/drm/i915/intel_psr.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index 0938df48107a..c907282dc82d 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -712,7 +712,8 @@ static void hsw_psr_enable_source(struct
> > > intel_dp *intel_dp,
> > >   I915_WRITE(EDP_PSR_DEBUG,
> > >  EDP_PSR_DEBUG_MASK_MEMUP |
> > >  EDP_PSR_DEBUG_MASK_HPD |
> > > -EDP_PSR_DEBUG_MASK_LPSP);
> > > +EDP_PSR_DEBUG_MASK_LPSP |
> > > +EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
> > >   }
> > >  }
> > >  
> > > -- 
> > > 2.17.0
> > > 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written

2018-04-23 Thread Souza, Jose
On Fri, 2018-04-20 at 15:57 -0700, Rodrigo Vivi wrote:
> On Fri, Apr 20, 2018 at 03:27:56PM -0700, José Roberto de Souza
> wrote:
> > Any write in any display register was causing HW to exit PSR,
> > masking it to allow more power savings. Writes to pipe related
> > registers will still cause HW to exit PSR.
> > This is already masked for PSR2.
> 
> This seems a good idea indeed with the test case on perspective.

what test cases are thinking? the current ones already do pages flips
that will only touch the pipe related registers.

> 
> But it needs more tests to make sure it doesn't break
> "Display WA #0884: all"

I just tested the WA #0884 and it still causes PSR to exit. I have
added a new debugfs that when read it writes to
'I915_WRITE(CURSURFLIVE(pipe), 0);' causing a PSR exit.

> 
> Or we might need to revert that patch before moving with this idea.
> 
> > 
> > Bspec: 7721 and 8042
> > 
> > Cc: Rodrigo Vivi 
> > Cc: Dhinakaran Pandiyan 
> > Signed-off-by: José Roberto de Souza 
> > ---
> > 
> > New patch in this series.
> > 
> >  drivers/gpu/drm/i915/intel_psr.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 0938df48107a..c907282dc82d 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -712,7 +712,8 @@ static void hsw_psr_enable_source(struct
> > intel_dp *intel_dp,
> > I915_WRITE(EDP_PSR_DEBUG,
> >EDP_PSR_DEBUG_MASK_MEMUP |
> >EDP_PSR_DEBUG_MASK_HPD |
> > -  EDP_PSR_DEBUG_MASK_LPSP);
> > +  EDP_PSR_DEBUG_MASK_LPSP |
> > +  EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
> > }
> >  }
> >  
> > -- 
> > 2.17.0
> > 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written

2018-04-20 Thread Rodrigo Vivi
On Fri, Apr 20, 2018 at 03:27:56PM -0700, José Roberto de Souza wrote:
> Any write in any display register was causing HW to exit PSR,
> masking it to allow more power savings. Writes to pipe related
> registers will still cause HW to exit PSR.
> This is already masked for PSR2.
> 
> Bspec: 7721 and 8042
> 
> Cc: Rodrigo Vivi 
> Cc: Dhinakaran Pandiyan 
> Signed-off-by: José Roberto de Souza 
> ---
> 
> New patch in this series.
> 
>  drivers/gpu/drm/i915/intel_psr.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> b/drivers/gpu/drm/i915/intel_psr.c
> index 0938df48107a..c907282dc82d 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -712,7 +712,8 @@ static void hsw_psr_enable_source(struct intel_dp 
> *intel_dp,
>   I915_WRITE(EDP_PSR_DEBUG,
>  EDP_PSR_DEBUG_MASK_MEMUP |
>  EDP_PSR_DEBUG_MASK_HPD |
> -EDP_PSR_DEBUG_MASK_LPSP);
> +EDP_PSR_DEBUG_MASK_LPSP |
> +EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);

What about only setting this bit before debugfs reads
and unseting it after that?


>   }
>  }
>  
> -- 
> 2.17.0
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written

2018-04-20 Thread Rodrigo Vivi
On Fri, Apr 20, 2018 at 03:27:56PM -0700, José Roberto de Souza wrote:
> Any write in any display register was causing HW to exit PSR,
> masking it to allow more power savings. Writes to pipe related
> registers will still cause HW to exit PSR.
> This is already masked for PSR2.

This seems a good idea indeed with the test case on perspective.

But it needs more tests to make sure it doesn't break
"Display WA #0884: all"

Or we might need to revert that patch before moving with this idea.

> 
> Bspec: 7721 and 8042
> 
> Cc: Rodrigo Vivi 
> Cc: Dhinakaran Pandiyan 
> Signed-off-by: José Roberto de Souza 
> ---
> 
> New patch in this series.
> 
>  drivers/gpu/drm/i915/intel_psr.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> b/drivers/gpu/drm/i915/intel_psr.c
> index 0938df48107a..c907282dc82d 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -712,7 +712,8 @@ static void hsw_psr_enable_source(struct intel_dp 
> *intel_dp,
>   I915_WRITE(EDP_PSR_DEBUG,
>  EDP_PSR_DEBUG_MASK_MEMUP |
>  EDP_PSR_DEBUG_MASK_HPD |
> -EDP_PSR_DEBUG_MASK_LPSP);
> +EDP_PSR_DEBUG_MASK_LPSP |
> +EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
>   }
>  }
>  
> -- 
> 2.17.0
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written

2018-04-20 Thread José Roberto de Souza
Any write in any display register was causing HW to exit PSR,
masking it to allow more power savings. Writes to pipe related
registers will still cause HW to exit PSR.
This is already masked for PSR2.

Bspec: 7721 and 8042

Cc: Rodrigo Vivi 
Cc: Dhinakaran Pandiyan 
Signed-off-by: José Roberto de Souza 
---

New patch in this series.

 drivers/gpu/drm/i915/intel_psr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 0938df48107a..c907282dc82d 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -712,7 +712,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
I915_WRITE(EDP_PSR_DEBUG,
   EDP_PSR_DEBUG_MASK_MEMUP |
   EDP_PSR_DEBUG_MASK_HPD |
-  EDP_PSR_DEBUG_MASK_LPSP);
+  EDP_PSR_DEBUG_MASK_LPSP |
+  EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
}
 }
 
-- 
2.17.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx