[Intel-gfx] [PATCH v2 6/6] drm/i915/debugfs: Print PSR selective update status register values

[Souza, Jose]

On Wed, 2019-01-09 at 18:11 -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2019-01-03 at 06:21 -0800, José Roberto de Souza wrote:
> > The value of this registers will be used to test if PSR2 is doing
> > selective update and if the number of blocks match with the
> > expected.
> > 
> > v2:
> > - Using new macros
> > - Changed the string output
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 32 +++++++++++++++++++++++++
> > ----
> >  1 file changed, 28 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 5ebf0e647ac7..4e92078bc65d 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2621,10 +2621,34 @@ static int i915_edp_psr_status(struct
> > seq_file *m, void *data)
> >  		seq_printf(m, "Performance counter: %u\n", val);
> >  	}
> >  
> > -	if ((psr->debug & I915_PSR_DEBUG_IRQ) && !psr->psr2_enabled) {
> > -		seq_printf(m, "Last attempted entry at: %lld\n",
> > -			   psr->last_entry_attempt);
> > -		seq_printf(m, "Last exit at: %lld\n", psr->last_exit);
> > +	if (!psr->psr2_enabled) {
> > +		if (psr->debug & I915_PSR_DEBUG_IRQ) {
> > +			seq_printf(m, "Last attempted entry at:
> > %lld\n",
> > +				   psr->last_entry_attempt);
> > +			seq_printf(m, "Last exit at: %lld\n", psr-
> > > last_exit);
> > +		}
> > +	} else {
> > +		u8 frame;
> > +
> > +		seq_puts(m, "Frame:\tPSR2 SU blocks:\n");
> > +
> > +		for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; frame++)
> > {
> > +			u32 su_blocks;
> > +
> > +			/*
> > +			 * Avoid register reads as each register
> > contains more
> > +			 * than one frame value
> > +			 */
> I don't think the comment is needed, but if you want to retain it I
> suggest explicitly saying each register contains data for three
> frames?

The last one have only 2 frames.

> 
> Not sure if it matters, how about completing all the three register
> reads before printing so that we reduce the chances of crossing a
> frame
> boundary between register reads?

Hum sounds good, I will do that.

> 
> > +			if ((frame % 3) == 0)
> > +				val = I915_READ(PSR2_SU_STATUS(frame));
> > +
> > +			su_blocks = val & PSR2_SU_STATUS_MASK(frame);
> > +			su_blocks = su_blocks >>
> > PSR2_SU_STATUS_SHIFT(frame);
> > +			/* Only printing frames with SU blocks */
> > +			if (!su_blocks)
> > +				continue;
> Why not print zero if that's the number of blocks updated in a frame?

I think 8 frames with value zero are not worth to print everytime and
IGT will only care about the frame 0 but I don't see any problem in
printing it.


> 
> > +			seq_printf(m, "%d\t%d\n", frame, su_blocks);
> > +		}
> >  	}
> >  
> >  unlock:
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20190110/a2b86709/attachment.sig>