Re: [Intel-gfx] [PATCH v3 08/22] drm/i915/bios: Generate LFP data table pointers if the VBT lacks them

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Apr 07, 2022 at 07:53:13PM +0300, Jani Nikula wrote:
> On Wed, 06 Apr 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Modern VBTs no longer contain the LFP data table pointers
> > block (41). We are expecting to have one in order to be able
> > to parse the LFP data block (42), so let's make one up.
> >
> > Since the fp_timing table has variable size we must somehow
> > determine its size. Rather than just hardcode it we look for
> > the terminator bytes (0xffff) to figure out where each table
> > entry starts. dvo_timing, panel_pnp_id, and panel_name are
> > expected to have fixed size.
> >
> > This has been observed on various machines, eg. TGL with BDB
> > version 240, CML with BDB version 231, etc. The most recent
> > VBT I've observed that still had block 41 had BDB version
> > 228. So presumably the cutoff (if an exact cutoff even exists)
> > is somewhere around BDB version 229-231.
> >
> > v2: kfree the thing we allocated, not the thing+3 bytes
> > v3: Do the debugprint only if we found the LFP data block
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_bios.c | 136 +++++++++++++++++++++-
> >  1 file changed, 135 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> > index 8b118c54314d..d32091dad1b0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> > @@ -310,16 +310,146 @@ static bool fixup_lfp_data_ptrs(const void *bdb, void *ptrs_block)
> >  	return validate_lfp_data_ptrs(bdb, ptrs);
> >  }
> >  
> > +static const void *find_fp_timing_terminator(const u8 *data, int size)
> > +{
> > +	int i;
> > +
> > +	if (!data)
> > +		return NULL;
> > +
> > +	for (i = 0; i < size - 1; i++) {
> > +		if (data[i] == 0xff && data[i+1] == 0xff)
> > +			return &data[i];
> > +	}
> > +
> > +	return NULL;
> > +}
> > +
> > +static int make_lfp_data_ptr(struct lvds_lfp_data_ptr_table *table,
> > +			     int table_size, int total_size)
> > +{
> > +	if (total_size < table_size)
> > +		return total_size;
> > +
> > +	table->table_size = table_size;
> > +	table->offset = total_size - table_size;
> > +
> > +	return total_size - table_size;
> > +}
> > +
> > +static void next_lfp_data_ptr(struct lvds_lfp_data_ptr_table *next,
> > +			      const struct lvds_lfp_data_ptr_table *prev,
> > +			      int size)
> > +{
> > +	next->table_size = prev->table_size;
> > +	next->offset = prev->offset + size;
> > +}
> > +
> > +static void *generate_lfp_data_ptrs(struct drm_i915_private *i915,
> > +				    const void *bdb)
> > +{
> > +	int i, size, table_size, block_size, offset;
> > +	const void *t0, *t1, *block;
> > +	struct bdb_lvds_lfp_data_ptrs *ptrs;
> > +	void *ptrs_block;
> > +
> > +	block = find_raw_section(bdb, BDB_LVDS_LFP_DATA);
> > +	if (!block)
> > +		return NULL;
> > +
> > +	drm_dbg_kms(&i915->drm, "Generating LFP data table pointers\n");
> > +
> > +	block_size = get_blocksize(block);
> > +
> > +	size = block_size;
> > +	t0 = find_fp_timing_terminator(block, size);
> > +
> > +	size -= t0 - block - 2;
> > +	t1 = find_fp_timing_terminator(t0 + 2, size);
> 
> Need to NULL check t0 before using it.

I had the null check in find_fp_timing_terminator() but the +2
here does mean that it doesn't actually work. I'll move the check here.

-- 
Ville Syrjälä
Intel