Re: [PATCH i-g-t] tools/intel_bios_reader: Dump PSR block from VBT

Ville Syrjälä
2016-09-13T10:00:12Z

On Tue, Sep 13, 2016 at 12:51:05PM +0300, Jani Nikula wrote:
> On Mon, 12 Sep 2016, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Decode the PSR block (9) from VBT. Looks like the same block ID may have
> > been used for something else in the past, so a version check is also
> > needed.
> >
> > The wakeup times part is still up in the air due to the spec not knowing
> > what it's saying, but let's do something that makes at least some sense
> > given the VBTs currently out there in the wild.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > tools/intel_bios.h | 15 +++++++++++++
> > tools/intel_bios_reader.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 69 insertions(+)
> >
> > diff --git a/tools/intel_bios.h b/tools/intel_bios.h
> > index 925bb06cd110..324ef4a759b4 100644
> > --- a/tools/intel_bios.h
> > +++ b/tools/intel_bios.h
> > @@ -63,6 +63,7 @@ struct bdb_header {
> > #define BDB_SWF_IO 7
> > #define BDB_SWF_MMIO 8
> > #define BDB_DOT_CLOCK_TABLE 9
> > +#define BDB_PSR 9
> > #define BDB_MODE_REMOVAL_TABLE 10
> > #define BDB_CHILD_DEVICE_TABLE 11
> > #define BDB_DRIVER_FEATURES 12
> > @@ -691,6 +692,20 @@ struct bdb_edp { /* 155 */
> > struct edp_full_link_params full_link_params[16]; /* 199 */
> > } __attribute__ ((packed));
> >
> > +struct psr_params {
> > + uint8_t full_link:1;
> > + uint8_t require_aux_to_wakeup:1;
> > + uint8_t rsvd1:6;
> > + uint8_t idle_frames:4;
> > + uint8_t lines_to_wait:3;
> > + uint8_t rsvd2:1;
> > + uint16_t tp1_wakeup_time;
> > + uint16_t tp2_tp3_wakeup_time;
> > +} __attribute__ ((packed));
> > +
> > +struct bdb_psr {
> > + struct psr_params psr[16];
> > +} __attribute__ ((packed));
> >
> > /* Block 52 contains MiPi Panel info
> > * 6 such enteries will there. Index into correct
> > diff --git a/tools/intel_bios_reader.c b/tools/intel_bios_reader.c
> > index e3daa7424580..3df36293c81c 100644
> > --- a/tools/intel_bios_reader.c
> > +++ b/tools/intel_bios_reader.c
> > @@ -907,6 +907,55 @@ static void dump_edp(struct context *context,
> > }
> > }
> >
> > +static void dump_psr(struct context *context,
> > + const struct bdb_block *block)
> > +{
> > + const struct bdb_psr *psr = block->data;
> > + int i;
> > +
> > + /* The same block ID was used for something else before? */
> > + if (context->bdb->version < 165)
> > + return;
>
> *sigh*
>
> > +
> > + for (i = 0; i < 16; i++) {
> > + if (i != context->panel_type && !context->dump_all_panel_types)
> > + continue;
> > +
> > + printf("\tPanel %d%s\n", i, context->panel_type == i ? " *" : "");
> > +
> > + printf("\t\tFull link: %s\n", YESNO(psr->psr[i].full_link));
> > + printf("\t\tRequire AUX to wakeup: %s\n", YESNO(psr->psr[i].require_aux_to_wakeup));
> > +
> > + switch (psr->psr[i].lines_to_wait) {
> > + case 0:
> > + case 1:
> > + printf("\t\tLines to wait before link standby: %d\n",
> > + psr->psr[i].lines_to_wait);
> > + break;
> > + case 2:
> > + case 3:
> > + printf("\t\tLines to wait before link standby: %d\n",
> > + 1 << psr->psr[i].lines_to_wait);
> > + break;
> > + default:
> > + printf("\t\tLines to wait before link standby: (unknown) (0x%x)\n",
> > + psr->psr[i].lines_to_wait);
> > + break;
> > + }
> > +
> > + printf("\t\tIdle frames to for PSR enable: %d\n",
> > + psr->psr[i].idle_frames);
> > +
> > + printf("\t\tTP1 wakeup time: %d usec (0x%x)\n",
> > + psr->psr[i].tp1_wakeup_time * 100,
> > + psr->psr[i].tp1_wakeup_time);
> > +
> > + printf("\t\tTP2/TP3 wakeup time: %d usec (0x%x)\n",
> > + psr->psr[i].tp1_wakeup_time * 100,
> > + psr->psr[i].tp1_wakeup_time);
>
> Do we want the alternative interpretation here too?
>
> 0 = 500 usec
>
> 1 = 100 usec
>
> 2 = 2.5 msec
>
> 3 = 0 usec (Skip TP)

So far I've not come across a VBT where that interpretation would work.
Based on that I think printing the raw value in addition to the decoded
value is good enough for now.

> Otherwise LGTM.
>
>
> BR,
> Jani.
>
>
>
> > + }
> > +}
> > +
> > static void
> > print_detail_timing_data(const struct lvds_dvo_timing2 *dvo_timing)
> > {
> > @@ -1519,6 +1568,11 @@ struct dumper dumpers[] = {
> > .dump = dump_edp,
> > },
> > {
> > + .id = BDB_PSR,
> > + .name = "PSR block",
> > + .dump = dump_psr,
> > + },
> > + {
> > .id = BDB_MIPI_CONFIG,
> > .name = "MIPI configuration block",
> > .dump = dump_mipi_config,
>
> --
> Jani Nikula, Intel Open Source Technology Center

--
Ville Syrjälä
Intel OTC