[Intel-gfx] [PATCH 2/3] drm/i915: Remove PSR Perf Counter for SKL+

[Vivi, Rodrigo]

On Fri, 2015-11-20 at 06:01 +0000, Jindal, Sonika wrote:
> Hmm, please help me understand more about it.

Well, PSR is working because we are seeing PC10 with screen on. But the
registers are zeroed. Regarding it is restored properly or not.

I can send a v2 with a much better commit message and explanation now I
know it better after talking to HW guys.

> As per bspec this is a RW register.
> Also, restoring the register values, isn't it part of the firmware's 
> job?

Oh  indeed. I'm sorry for my confusion.

> Do we have any communication from HW team on this?

Today when I was explaining them what I was seeing I realized that this
register is reset to 0 when entering DC5/6. Even if restored properly
on exit, we would still see 0 on the full idle scenario and people
would think PSR is not working. So I still want to remove that.

> 
> Regards,
> Sonika
> 
> -----Original Message-----
> From: Rodrigo Vivi [mailto:rodrigo.vivi at gmail.com] 
> Sent: Friday, November 20, 2015 12:01 AM
> To: Jindal, Sonika
> Cc: R, Durgadoss; Vivi, Rodrigo; intel-gfx at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 2/3] drm/i915: Remove PSR Perf 
> Counter for SKL+
> 
> On Wed, Nov 18, 2015 at 11:45 PM, Jindal, Sonika <
> sonika.jindal at intel.com> wrote:
> > Hi Rodrigo,
> > 
> > Which platform have you observed this issue on?
> 
> Skylake and Kabylake
> 
> > This looks really strange.
> 
> It is expected actually.
> On DC5/6 states all read-only registers are reset and cannot be 
> restored. including this counter.
> 
> > Have you checked whether we are able to enter PSR at sink side or 
> > not in such cases?
> > Are we sure we are not entering PSR? I mean the PSR_STATE register 
> > says it correctly?
> 
> I'm sure PSR is working well in entry state with links off and Perf 
> counter goes to zero when DC5/6 states enters.
> So this patch just removes the counter to avoid confusion. People 
> would think PSR isn't work, when it is.
> 
> > 
> > Regards,
> > Sonika
> > 
> > -----Original Message-----
> > From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On 
> > Behalf Of R, Durgadoss
> > Sent: Thursday, November 19, 2015 11:39 AM
> > To: Vivi, Rodrigo; intel-gfx at lists.freedesktop.org
> > Cc: Vivi, Rodrigo
> > Subject: Re: [Intel-gfx] [PATCH 2/3] drm/i915: Remove PSR Perf 
> > Counter 
> > for SKL+
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] 
> > > On 
> > > Behalf Of Rodrigo Vivi
> > > Sent: Thursday, November 19, 2015 6:10 AM
> > > To: intel-gfx at lists.freedesktop.org
> > > Cc: Vivi, Rodrigo
> > > Subject: [Intel-gfx] [PATCH 2/3] drm/i915: Remove PSR Perf 
> > > Counter for
> > > SKL+
> > > 
> > > Whenever DMC firmware put the HW into DC State a bunch of 
> > > registers 
> > > including this perf counter is reset to 0 and never restored.
> > > 
> > > So, even with PSR active and working we could still read
> > > "Performance_Counter: 0" what will misslead people to believe PSR 
> > > is 
> > > broken.
> > > 
> > > So, it is better to remove this counter information while we 
> > > don't 
> > > have a better way to track PSR residency.
> > 
> > Agreed..
> > 
> > Reviewed-by: Durgadoss R <durgadoss.r at intel.com>
> > 
> > Thanks,
> > Durga
> > 
> > > 
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_debugfs.c | 7 +++++--
> > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 038d5c6..71e1666 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -2580,8 +2580,11 @@ static int i915_edp_psr_status(struct 
> > > seq_file *m, void *data)
> > >               }
> > >       seq_puts(m, "\n");
> > > 
> > > -      /* CHV PSR has no kind of performance counter */
> > > -      if (HAS_DDI(dev)) {
> > > +      /*
> > > +       * VLV/CHV PSR has no kind of performance counter
> > > +       * SKL+ Perf counter is reset to 0 everytime DC state is 
> > > entered
> > > +       */
> > > +      if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> > >               psrperf = I915_READ(EDP_PSR_PERF_CNT) &
> > >                       EDP_PSR_PERF_CNT_MASK;
> > > 
> > > --
> > > 2.4.3
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br