Re: [PATCH 12/12] drm/i915/debugfs: Print how many blocks were sent in a selective update

["Souza, Jose" <jose.souza@intel.com>]

On Thu, 2018-03-22 at 16:46 -0700, Rodrigo Vivi wrote:
> On Thu, Mar 22, 2018 at 02:48:48PM -0700, José Roberto de Souza
> wrote:
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 40
> > ++++++++++++++++++++++++++++++++++++-
> >  drivers/gpu/drm/i915/i915_reg.h     | 17 ++++++++++++++++
> >  2 files changed, 56 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 3182e9a7cc5d..20985584cc0f 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2678,6 +2678,43 @@ static void psr_event_exit_sprintf(struct
> > seq_file *m, u32 val,
> >  		seq_puts(m, "\tPSR disabled\n");
> >  }
> >  
> > +static void psr2_su_blocks_sprintf(struct seq_file *m,
> > +				   struct drm_i915_private
> > *dev_priv)
> > +{
> > +	u32 val;
> > +	u16 su;
> > +
> > +	val = I915_READ(EDP_PSR2_SU_STATUS);
> > +	su = val >>
> > EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_FRAME_N_SHIFT;
> > +	su &= EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_MASK;
> > +	seq_printf(m, "\tSU blocks in frame N: %d\n", su);
> > +	su = val >>
> > EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_FRAME_N_MINUS_1_SHIFT;
> > +	su &= EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_MASK;
> > +	seq_printf(m, "\tSU blocks in frame N-1: %d\n", su);
> > +	su = val >>
> > EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_FRAME_N_MINUS_2_SHIFT;
> > +	su &= EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_MASK;
> > +	seq_printf(m, "\tSU blocks in frame N-2: %d\n", su);
> > +
> > +	val = I915_READ(EDP_PSR2_SU_STATUS2);
> > +	su = val >>
> > EDP_PSR2_SU_STATUS2_NUM_SU_BLOCKS_FRAME_N_MINUS_3_SHIFT;
> > +	su &= EDP_PSR2_SU_STATUS2_NUM_SU_BLOCKS_MASK;
> > +	seq_printf(m, "\tSU blocks in frame N-3: %d\n", su);
> > +	su = val >>
> > EDP_PSR2_SU_STATUS2_NUM_SU_BLOCKS_FRAME_N_MINUS_4_SHIFT;
> > +	su &= EDP_PSR2_SU_STATUS2_NUM_SU_BLOCKS_MASK;
> > +	seq_printf(m, "\tSU blocks in frame N-4: %d\n", su);
> > +	su = val >>
> > EDP_PSR2_SU_STATUS2_NUM_SU_BLOCKS_FRAME_N_MINUS_5_SHIFT;
> > +	su &= EDP_PSR2_SU_STATUS2_NUM_SU_BLOCKS_MASK;
> > +	seq_printf(m, "\tSU blocks in frame N-5: %d\n", su);
> > +
> > +	val = I915_READ(EDP_PSR2_SU_STATUS3);
> > +	su = val >>
> > EDP_PSR2_SU_STATUS3_NUM_SU_BLOCKS_FRAME_N_MINUS_6_SHIFT;
> > +	su &= EDP_PSR2_SU_STATUS3_NUM_SU_BLOCKS_MASK;
> > +	seq_printf(m, "\tSU blocks in frame N-6: %d\n", su);
> > +	su = val >>
> > EDP_PSR2_SU_STATUS3_NUM_SU_BLOCKS_FRAME_N_MINUS_7_SHIFT;
> > +	su &= EDP_PSR2_SU_STATUS3_NUM_SU_BLOCKS_MASK;
> > +	seq_printf(m, "\tSU blocks in frame N-7: %d\n", su);
> > +}
> > +
> >  static int i915_edp_psr_status(struct seq_file *m, void *data)
> >  {
> >  	struct drm_i915_private *dev_priv = node_to_i915(m-
> > >private);
> > @@ -2766,8 +2803,9 @@ static int i915_edp_psr_status(struct
> > seq_file *m, void *data)
> >  	if (dev_priv->psr.psr2_enabled) {
> >  		u32 psr2 = I915_READ(EDP_PSR2_STATUS);
> >  
> > -		seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
> > +		seq_printf(m, "EDP_PSR2_STATUS: 0x%x [%s]\n",
> >  			   psr2, psr2_live_status(psr2));
> > +		psr2_su_blocks_sprintf(m, dev_priv);
> >  	}
> >  
> >  	if (dev_priv->psr.enabled) {
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 45f7703a9ee6..18af3e8fd4b6 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3929,6 +3929,23 @@ enum {
> >  #define EDP_PSR2_STATUS_STATE_MASK     (0xf<<28)
> >  #define EDP_PSR2_STATUS_STATE_SHIFT    28
> >  
> > +#define EDP_PSR2_SU_STATUS					
> >   _MMIO(0x6F914)
> > +#define  EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_MASK			
> >   0x3FF
> > +#define  EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_FRAME_N_SHIFT		
> >   0
> > +#define  EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_FRAME_N_MINUS_1_SHIFT	
> >   10
> > +#define  EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_FRAME_N_MINUS_2_SHIFT	
> >   20
> > +
> > +#define EDP_PSR2_SU_STATUS2					
> >   _MMIO(0x6F918)
> > +#define  EDP_PSR2_SU_STATUS2_NUM_SU_BLOCKS_MASK			
> >   0x3FF
> > +#define  EDP_PSR2_SU_STATUS2_NUM_SU_BLOCKS_FRAME_N_MINUS_3_SHIFT  
> > 0
> > +#define  EDP_PSR2_SU_STATUS2_NUM_SU_BLOCKS_FRAME_N_MINUS_4_SHIFT  
> > 10
> > +#define  EDP_PSR2_SU_STATUS2_NUM_SU_BLOCKS_FRAME_N_MINUS_5_SHIFT  
> > 20
> > +
> > +#define EDP_PSR2_SU_STATUS3					
> >   _MMIO(0x6F91C)
> > +#define  EDP_PSR2_SU_STATUS3_NUM_SU_BLOCKS_MASK			
> >   0x3FF
> > +#define  EDP_PSR2_SU_STATUS3_NUM_SU_BLOCKS_FRAME_N_MINUS_6_SHIFT  
> > 0
> > +#define  EDP_PSR2_SU_STATUS3_NUM_SU_BLOCKS_FRAME_N_MINUS_7_SHIFT  
> > 10
> > +
> 
> Couldn't we define it unified as:
> +#define EDP_PSR2_SU_STATUS(frame)				_MM
> IO(0x6F914 + 4 * frame / 3)
> +#define  EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_MASK			
>   0x3FF
> +#define  EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_FRAME_SHIFT(frame)	
> (frame % 3 * 10)

Oh way better, thanks.
It also saved a lot of line when priting:

static void psr2_su_blocks_sprintf(struct seq_file *m,
				   struct drm_i915_private *dev_priv)
{
	unsigned i;

	for (i = 0; i < 8; i++) {
		u32 val = I915_READ(EDP_PSR2_SU_STATUS(i));
		val = val >>
EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_FRAME_SHIFT(i);
		val &= EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_MASK;
		seq_printf(m, "\tSU blocks in frame N-%d: %d\n", i,
val);
	}
}

> 
> >  /* VGA port control */
> >  #define ADPA			_MMIO(0x61100)
> >  #define PCH_ADPA                _MMIO(0xe1100)
> > -- 
> > 2.16.2
> > 
> > _______________________________________________
> > 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