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

Jani Nikula
2016-09-13T09:51:07Z

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)

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