| Message ID | 20190103142107.18304-6-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/6] drm/i915/psr: Allow PSR2 to be enabled when debugfs asks | expand |
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@intel.com> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@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? 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? > + 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? > + seq_printf(m, "%d\t%d\n", frame, su_blocks); > + } > } > > unlock:
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@intel.com> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@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:
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 + */ + 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; + seq_printf(m, "%d\t%d\n", frame, su_blocks); + } } unlock:
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@intel.com> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 32 +++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-)