On Thu, 10 Sep 2015, Deepak M <[email protected]> wrote:
> Currently the field in bdb header which indicates the VBT size
> is of 2 bytes, but there are some cases where VBT size exceeds
> 64KB in which case this field may not contain the correct VBT size.
> So its better to get the VBT size from the mailbox3 if
> VBT is not present in the mailbox4 of opregion.
>
> v2: - Use opregion filed from dev_priv struct instead of creating
>       a new field in dev_priv (Jani)
>     - Have vbt_size field vaild in all scenarios (Jani)
>     - rebase
>
> Signed-off-by: Deepak M <[email protected]>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |    2 ++
>  drivers/gpu/drm/i915/intel_bios.c     |   42 
> +++++++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_opregion.c |    6 +++++
>  3 files changed, 32 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 507d57a..91ccbc6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1777,6 +1777,8 @@ struct drm_i915_private {
>       u32 pm_rps_events;
>       u32 pipestat_irq_mask[I915_MAX_PIPES];
>  
> +     u32 vbt_size;

No. dev_priv is not a random area to throw things into. The place you're
looking for is dev_priv->opregion, i.e. struct intel_opregion.

> +
>       struct i915_hotplug hotplug;
>       struct i915_fbc fbc;
>       struct i915_drrs drrs;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
> b/drivers/gpu/drm/i915/intel_bios.c
> index 1932a86..34a1042 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -37,17 +37,19 @@
>  static int panel_type;
>  
>  static const void *
> -find_section(const void *_bdb, int section_id)
> +find_section(struct drm_i915_private *dev_priv,
> +             const void *_bdb, int section_id)
>  {
>       const struct bdb_header *bdb = _bdb;
>       const u8 *base = _bdb;
>       int index = 0;
> -     u16 total, current_size;
> +     u32 total, current_size;
>       u8 current_id;
>  
>       /* skip to first section */
>       index += bdb->header_size;
> -     total = bdb->bdb_size;
> +
> +     total = dev_priv->vbt_size;

vbt_size != bdb_size. See below.

>  
>       /* walk the sections looking for section_id */
>       while (index + 3 < total) {
> @@ -179,7 +181,7 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
>       struct drm_display_mode *panel_fixed_mode;
>       int drrs_mode;
>  
> -     lvds_options = find_section(bdb, BDB_LVDS_OPTIONS);
> +     lvds_options = find_section(dev_priv, bdb, BDB_LVDS_OPTIONS);
>       if (!lvds_options)
>               return;
>  
> @@ -211,11 +213,12 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
>               break;
>       }
>  
> -     lvds_lfp_data = find_section(bdb, BDB_LVDS_LFP_DATA);
> +     lvds_lfp_data = find_section(dev_priv, bdb, BDB_LVDS_LFP_DATA);
>       if (!lvds_lfp_data)
>               return;
>  
> -     lvds_lfp_data_ptrs = find_section(bdb, BDB_LVDS_LFP_DATA_PTRS);
> +     lvds_lfp_data_ptrs = find_section(dev_priv, bdb,
> +                                     BDB_LVDS_LFP_DATA_PTRS);
>       if (!lvds_lfp_data_ptrs)
>               return;
>  
> @@ -257,7 +260,7 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv,
>       const struct bdb_lfp_backlight_data *backlight_data;
>       const struct bdb_lfp_backlight_data_entry *entry;
>  
> -     backlight_data = find_section(bdb, BDB_LVDS_BACKLIGHT);
> +     backlight_data = find_section(dev_priv, bdb, BDB_LVDS_BACKLIGHT);
>       if (!backlight_data)
>               return;
>  
> @@ -305,14 +308,15 @@ parse_sdvo_panel_data(struct drm_i915_private *dev_priv,
>       if (index == -1) {
>               const struct bdb_sdvo_lvds_options *sdvo_lvds_options;
>  
> -             sdvo_lvds_options = find_section(bdb, BDB_SDVO_LVDS_OPTIONS);
> +             sdvo_lvds_options = find_section(dev_priv, bdb,
> +                                             BDB_SDVO_LVDS_OPTIONS);
>               if (!sdvo_lvds_options)
>                       return;
>  
>               index = sdvo_lvds_options->panel_type;
>       }
>  
> -     dvo_timing = find_section(bdb, BDB_SDVO_PANEL_DTDS);
> +     dvo_timing = find_section(dev_priv, bdb, BDB_SDVO_PANEL_DTDS);
>       if (!dvo_timing)
>               return;
>  
> @@ -349,7 +353,7 @@ parse_general_features(struct drm_i915_private *dev_priv,
>       struct drm_device *dev = dev_priv->dev;
>       const struct bdb_general_features *general;
>  
> -     general = find_section(bdb, BDB_GENERAL_FEATURES);
> +     general = find_section(dev_priv, bdb, BDB_GENERAL_FEATURES);
>       if (general) {
>               dev_priv->vbt.int_tv_support = general->int_tv_support;
>               dev_priv->vbt.int_crt_support = general->int_crt_support;
> @@ -374,7 +378,7 @@ parse_general_definitions(struct drm_i915_private 
> *dev_priv,
>  {
>       const struct bdb_general_definitions *general;
>  
> -     general = find_section(bdb, BDB_GENERAL_DEFINITIONS);
> +     general = find_section(dev_priv, bdb, BDB_GENERAL_DEFINITIONS);
>       if (general) {
>               u16 block_size = get_blocksize(general);
>               if (block_size >= sizeof(*general)) {
> @@ -405,7 +409,7 @@ parse_sdvo_device_mapping(struct drm_i915_private 
> *dev_priv,
>       int i, child_device_num, count;
>       u16     block_size;
>  
> -     p_defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
> +     p_defs = find_section(dev_priv, bdb, BDB_GENERAL_DEFINITIONS);
>       if (!p_defs) {
>               DRM_DEBUG_KMS("No general definition block is found, unable to 
> construct sdvo mapping.\n");
>               return;
> @@ -491,7 +495,7 @@ parse_driver_features(struct drm_i915_private *dev_priv,
>  {
>       const struct bdb_driver_features *driver;
>  
> -     driver = find_section(bdb, BDB_DRIVER_FEATURES);
> +     driver = find_section(dev_priv, bdb, BDB_DRIVER_FEATURES);
>       if (!driver)
>               return;
>  
> @@ -519,7 +523,7 @@ parse_edp(struct drm_i915_private *dev_priv, const struct 
> bdb_header *bdb)
>       const struct edp_power_seq *edp_pps;
>       const struct edp_link_params *edp_link_params;
>  
> -     edp = find_section(bdb, BDB_EDP);
> +     edp = find_section(dev_priv, bdb, BDB_EDP);
>       if (!edp) {
>               if (dev_priv->vbt.edp_support)
>                       DRM_DEBUG_KMS("No eDP BDB found but eDP panel 
> supported.\n");
> @@ -630,7 +634,7 @@ parse_psr(struct drm_i915_private *dev_priv, const struct 
> bdb_header *bdb)
>       const struct bdb_psr *psr;
>       const struct psr_table *psr_table;
>  
> -     psr = find_section(bdb, BDB_PSR);
> +     psr = find_section(dev_priv, bdb, BDB_PSR);
>       if (!psr) {
>               DRM_DEBUG_KMS("No PSR BDB found.\n");
>               return;
> @@ -768,7 +772,7 @@ parse_mipi(struct drm_i915_private *dev_priv, const 
> struct bdb_header *bdb)
>       /* Parse #52 for panel index used from panel_type already
>        * parsed
>        */
> -     start = find_section(bdb, BDB_MIPI_CONFIG);
> +     start = find_section(dev_priv, bdb, BDB_MIPI_CONFIG);
>       if (!start) {
>               DRM_DEBUG_KMS("No MIPI config BDB found");
>               return;
> @@ -799,7 +803,7 @@ parse_mipi(struct drm_i915_private *dev_priv, const 
> struct bdb_header *bdb)
>       dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
>  
>       /* Check if we have sequence block as well */
> -     sequence = find_section(bdb, BDB_MIPI_SEQUENCE);
> +     sequence = find_section(dev_priv, bdb, BDB_MIPI_SEQUENCE);
>       if (!sequence) {
>               DRM_DEBUG_KMS("No MIPI Sequence found, parsing complete\n");
>               return;
> @@ -1077,7 +1081,7 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>       u8 expected_size;
>       u16 block_size;
>  
> -     p_defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
> +     p_defs = find_section(dev_priv, bdb, BDB_GENERAL_DEFINITIONS);
>       if (!p_defs) {
>               DRM_DEBUG_KMS("No general definition block is found, no devices 
> defined.\n");
>               return;
> @@ -1280,6 +1284,8 @@ intel_parse_bios(struct drm_device *dev)
>                       pci_unmap_rom(pdev, bios);
>                       return -1;
>               }
> +
> +             dev_priv->vbt_size = bdb->bdb_size;
>       }
>  
>       /* Grab useful general definitions */
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
> b/drivers/gpu/drm/i915/intel_opregion.c
> index 3a43db9..c2f1a4a 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -793,6 +793,12 @@ int intel_opregion_setup(struct drm_device *dev)
>               goto err_vbt;
>       }
>  
> +     /* Assigning the vbt_size based on the VBT location */
> +     if (opregion->header->opregion_ver >= 2 && opregion->asle->rvda)
> +             dev_priv->vbt_size = opregion->asle->rvds;

IIUC this one includes VBT Header...

> +     else
> +             dev_priv->vbt_size = bdb->bdb_size;

...and this one doesn't, so one of them will be wrong in the size checks
in find_section.

If you have a variable called "vbt_size" it better include all of VBT,
and not just part of it.


BR,
Jani.


> +
>       dev_priv->bdb_start = bdb;
>       opregion->vbt = vbt_base;
>  
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to