[igt-dev] [PATCH i-g-t 2/2] tooks/vbt_decode: Print PSR2 training pattern durations.

[Dhinakaran Pandiyan]

On Thu, 2019-07-18 at 14:35 +0300, Ville Syrjälä wrote:
> On Wed, Jul 17, 2019 at 05:43:31PM -0700, Dhinakaran Pandiyan wrote:
> > There is a new field for PSR2 training pattern duration in VBT versions
> > > = 226, decode that.
> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > ---
> >  tools/intel_vbt_decode.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
> > index 38eccc48..4004762c 100644
> > --- a/tools/intel_vbt_decode.c
> > +++ b/tools/intel_vbt_decode.c
> > @@ -944,11 +944,13 @@ static void dump_psr(struct context *context,
> >  {
> >  	const struct bdb_psr *psr_block = block->data;
> >  	int i;
> > +	int psr2_tp_time;
> 
> uint32_t
> 
> >  
> >  	/* The same block ID was used for something else before? */
> >  	if (context->bdb->version < 165)
> >  		return;
> >  
> > +	psr2_tp_time = psr_block->psr2_tp2_tp3_wakeup_time;
> >  	for (i = 0; i < 16; i++) {
> >  		const struct psr_table *psr = &psr_block->psr_table[i];
> >  
> > @@ -987,6 +989,15 @@ static void dump_psr(struct context *context,
> >  		printf("\t\tTP2/TP3 wakeup time: %d usec (0x%x)\n",
> >  		       psr->tp2_tp3_wakeup_time * 100,
> >  		       psr->tp2_tp3_wakeup_time);
> > +
> > +		if (context->bdb->version >= 226) {
> > +			int index;
> > +			static const int psr2_tp_times[] = {500, 100, 2500, 5};
> 
> Everthing fits into uint16_t.
Do we want a fixed-width type for values that are not read from/written to HW?
I changed it in the second version anyway. 
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Oh, and the psr1 parsing doesn't seem to be following this silly
> new style of putting the hw register value directly into the vbt.
> Would be nice to fix that too. Actually, since the spec is a mess
> and it's not 100% clear which VBTs might follow which style maybe
> we should in fact decode both ways?
I thought of fixing it, but left it alone because it is a mess as you said.
Are suggesting something like this - "TP2/TP3 wakeup time: 200 usec/2500 usec (0x02)"?
I feel the ambiguity makes decoding useless as it requires someone to go read
the driver code to know what will get programmed. Might as well avoid decoding and
just print the raw value. The other option is to replicate the platform checks used in
the driver.

-DK

> 
> > +
> > +			index = (psr2_tp_time >> (i * 2)) & 0x3;
> > +			printf("\t\tPSR2 TP2/TP3 wakeup time: %d usec (0x%x)\n",
> > +			       psr2_tp_times[index], index);
> > +		}
> >  	}
> >  }
> >  
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
>