Re: [Intel-gfx] [PATCH 03/11] drm/i915/bios: Use the copy of the LFP data table always

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 17 Mar 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently get_lvds_fp_timing() still returns a pointer to the original
> data block rather than our copy. Let's convert the data pointer offsets
> to be relative to the data block rather than the whole BDB. With that
> we can make get_lvds_fp_timing() return a pointer to the copy.

Ugh, so just as I R-b'd the previous patch... I realize it's all broken
without this, right? It does pointer arithmetics between bdb header and
the allocated bdb for ptrs?

Do we want a broken step?

BR,
Jani.

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c | 56 ++++++++++++++++++++---
>  1 file changed, 49 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index ff04514eb3b7..777339f5dd79 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -138,6 +138,44 @@ find_raw_section(const void *_bdb, enum bdb_block_id section_id)
>  	return NULL;
>  }
>  
> +/*
> + * Offset from the start of BDB to the start of the
> + * block data (just past the block header).
> + */
> +static u32 block_offset(const void *bdb, enum bdb_block_id section_id)
> +{
> +	const void *block;
> +
> +	block = find_raw_section(bdb, section_id);
> +	if (!block)
> +		return 0;
> +
> +	return block - bdb;
> +}
> +
> +/* make the data table offsets relative to the data block */
> +static bool fixup_lfp_data_ptrs(const void *bdb, void *ptrs_block)
> +{
> +	struct bdb_lvds_lfp_data_ptrs *ptrs = ptrs_block;
> +	u32 offset;
> +	int i;
> +
> +	offset = block_offset(bdb, BDB_LVDS_LFP_DATA);
> +
> +	for (i = 0; i < 16; i++) {
> +		if (ptrs->ptr[i].fp_timing.offset < offset ||
> +		    ptrs->ptr[i].dvo_timing.offset < offset ||
> +		    ptrs->ptr[i].panel_pnp_id.offset < offset)
> +			return false;
> +
> +		ptrs->ptr[i].fp_timing.offset -= offset;
> +		ptrs->ptr[i].dvo_timing.offset -= offset;
> +		ptrs->ptr[i].panel_pnp_id.offset -= offset;
> +	}
> +
> +	return true;
> +}
> +
>  static const void *
>  find_section(struct drm_i915_private *i915,
>  	     const void *bdb, enum bdb_block_id section_id,
> @@ -174,6 +212,13 @@ find_section(struct drm_i915_private *i915,
>  	drm_dbg_kms(&i915->drm, "Found BDB block %d (size %zu, min size %zu)\n",
>  		    section_id, block_size, min_size);
>  
> +	if (section_id == BDB_LVDS_LFP_DATA_PTRS &&
> +	    !fixup_lfp_data_ptrs(bdb, entry->data + 3)) {
> +		drm_err(&i915->drm, "VBT has malformed LFP data table pointers\n");
> +		kfree(entry);
> +		return NULL;
> +	}
> +
>  	list_add(&entry->node, &i915->vbt.bdb_blocks);
>  
>  	return entry->data + 3;
> @@ -255,22 +300,19 @@ get_lvds_dvo_timing(const struct bdb_lvds_lfp_data *lvds_lfp_data,
>   * this function may return NULL if the corresponding entry is invalid
>   */
>  static const struct lvds_fp_timing *
> -get_lvds_fp_timing(const struct bdb_header *bdb,
> -		   const struct bdb_lvds_lfp_data *data,
> +get_lvds_fp_timing(const struct bdb_lvds_lfp_data *data,
>  		   const struct bdb_lvds_lfp_data_ptrs *ptrs,
>  		   int index)
>  {
> -	size_t data_ofs = (const u8 *)data - (const u8 *)bdb;
>  	u16 data_size = ((const u16 *)data)[-1]; /* stored in header */
>  	size_t ofs;
>  
>  	if (index >= ARRAY_SIZE(ptrs->ptr))
>  		return NULL;
>  	ofs = ptrs->ptr[index].fp_timing.offset;
> -	if (ofs < data_ofs ||
> -	    ofs + sizeof(struct lvds_fp_timing) > data_ofs + data_size)
> +	if (ofs + sizeof(struct lvds_fp_timing) > data_size)
>  		return NULL;
> -	return (const struct lvds_fp_timing *)((const u8 *)bdb + ofs);
> +	return (const struct lvds_fp_timing *)((const u8 *)data + ofs);
>  }
>  
>  /* Parse general panel options */
> @@ -373,7 +415,7 @@ parse_lfp_panel_dtd(struct drm_i915_private *i915,
>  		    "Found panel mode in BIOS VBT legacy lfp table:\n");
>  	drm_mode_debug_printmodeline(panel_fixed_mode);
>  
> -	fp_timing = get_lvds_fp_timing(bdb, lvds_lfp_data,
> +	fp_timing = get_lvds_fp_timing(lvds_lfp_data,
>  				       lvds_lfp_data_ptrs,
>  				       panel_type);
>  	if (fp_timing) {

-- 
Jani Nikula, Intel Open Source Graphics Center