[Intel-gfx] [PATCH 03/13] drm/i915: VBT Parsing for the PSR Feature Block for HSW

[Paulo Zanoni]

2013/6/12 Rodrigo Vivi <rodrigo.vivi at gmail.com>:
> From: Shobhit Kumar <shobhit.kumar at intel.com>
>
> Parse and store useful information in i915_dev_private
>
> v2: Add to new vbt struct and call them psr_*
> v3: Fix comment and variable name
>
> Signed-off-by: Shobhit Kumar <shobhit.kumar at intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  7 +++++++
>  drivers/gpu/drm/i915/intel_bios.c | 30 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_bios.h | 20 +++++++++++++++++++-
>  3 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 87f7f88..dd459a5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -951,6 +951,13 @@ struct intel_vbt_data {
>         int edp_bpp;
>         struct edp_power_seq edp_pps;
>
> +       /* eDP PSR*/
> +       u8 psr_full_link_state;
> +       u8 psr_wait_lines;
> +       u8 psr_idle_frames;
> +       u16 psr_wakeup_tp1;
> +       u16 psr_wakeup_tp2_tp3;
> +
>         int crt_ddc_pin;
>
>         int child_dev_num;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 53f2bed..99c6788 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -383,6 +383,35 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
>         }
>  }
>
> +
> +static void
> +parse_edp_psr(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
> +{
> +       struct bdb_psr_features *psr;
> +       struct bdb_lvds_options *lvds_opts;
> +       int index = 0;
> +       lvds_opts = find_section(bdb, BDB_LVDS_OPTIONS);
> +       if (!lvds_opts) {
> +               DRM_DEBUG_KMS("No LVDS Options block found.\n");
> +               return;

We already parse BDB_LVDS_OPTIONS inside parse_lfp_panel_data(). I
wonder if we can reuse that parse somehow.


> +       }
> +
> +       index = lvds_opts->panel_type;
> +
> +       psr = find_section(bdb, BDB_PSR_FEATURES);

Don't we need to check for VBT version >= 155 here before proceeding?
Actually the docs are confusing, they say this block comes from
version 155, but all the bits inside it come from version 165... Which
one is the correct one? It seems 165 is the "safer" option at least...


> +       if (!psr) {
> +               DRM_DEBUG_KMS("No PSR feature block found.\n");
> +               return;
> +       }
> +
> +       dev_priv->vbt.psr_full_link_state = psr[index].link_disable;
> +       dev_priv->vbt.psr_wait_lines = psr[index].wait_lines;
> +       dev_priv->vbt.psr_idle_frames = psr[index].idle_frames;
> +       dev_priv->vbt.psr_wakeup_tp1 = psr[index].wakeup_tp1;
> +       dev_priv->vbt.psr_wakeup_tp2_tp3 = psr[index].wakeup_tp2_tp3;
> +}
> +
> +
>  static void
>  parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
>                           struct bdb_header *bdb)
> @@ -745,6 +774,7 @@ intel_parse_bios(struct drm_device *dev)
>         parse_device_mapping(dev_priv, bdb);
>         parse_driver_features(dev_priv, bdb);
>         parse_edp(dev_priv, bdb);
> +       parse_edp_psr(dev_priv, bdb);
>
>         if (bios)
>                 pci_unmap_rom(pdev, bios);
> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
> index e088d6f..c883b87 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -80,7 +80,7 @@ struct vbios_data {
>  #define BDB_EXT_MMIO_REGS        6
>  #define BDB_SWF_IO               7
>  #define BDB_SWF_MMIO             8
> -#define BDB_DOT_CLOCK_TABLE      9

This is probably from some older VBT version. That's why I suggested
checking the version above. I guess we shouldn't remove the old one,
but we should add some comments explaining this.


> +#define BDB_PSR_FEATURES         9
>  #define BDB_MODE_REMOVAL_TABLE  10
>  #define BDB_CHILD_DEVICE_TABLE  11
>  #define BDB_DRIVER_FEATURES     12
> @@ -265,6 +265,24 @@ struct bdb_lvds_options {
>         u8 rsvd4;
>  } __attribute__((packed));
>
> +struct bdb_psr_features {
> +       /* Feature bits */
> +       u8 link_disable:1;

Considering that 1 means "Enable", I'd rename this to "full_link_enable".


> +       u8 require_aux:1;
> +       u8 rsvd1:6;
> +
> +       /* Wait times */
> +       u8 idle_frames:4;
> +       u8 wait_lines:3;
> +       u8 rsvd2:1;
> +
> +       /* TP1 wakeup time */
> +       u16 wakeup_tp1;
> +
> +       /* TP2 TP3 wakeup time */
> +       u16 wakeup_tp2_tp3;
> +} __attribute__((packed));
> +
>  /* LFP pointer table contains entries to the struct below */
>  struct bdb_lvds_lfp_data_ptr {
>         u16 fp_timing_offset; /* offsets are from start of bdb */
> --
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni