On Thu, 2021-09-23 at 18:49 +0200, Lukasz Majczak wrote: > With patch "drm/i915/vbt: Fix backlight parsing for VBT 234+" > the size of bdb_lfp_backlight_data structure has been increased, > causing if-statement in the parse_lfp_backlight function > that comapres this structure size to the one retrieved from BDB, > always to fail for older revisions. > This patch calculates expected size of the structure for a given > BDB version and compares it with the value gathered from BDB. > Tested on Chromebook Pixelbook (Nocturne) (reports bdb->version = 221) Fixes: d381baad29b4 ("drm/i915/vbt: Fix backlight parsing for VBT 234+") > > Tested-by: Lukasz Majczak <lma@xxxxxxxxxxxx> > Signed-off-by: Lukasz Majczak <lma@xxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/display/intel_bios.c | 11 +++++++++-- > drivers/gpu/drm/i915/display/intel_vbt_defs.h | 5 +++++ > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c > index 3c25926092de..90eae6da12e0 100644 > --- a/drivers/gpu/drm/i915/display/intel_bios.c > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > @@ -428,6 +428,7 @@ parse_lfp_backlight(struct drm_i915_private *i915, > const struct lfp_backlight_data_entry *entry; > int panel_type = i915->vbt.panel_type; > u16 level; > + size_t exp_size; > > backlight_data = find_section(bdb, BDB_LVDS_BACKLIGHT); > if (!backlight_data) > @@ -450,9 +451,15 @@ parse_lfp_backlight(struct drm_i915_private *i915, > return; > } > > + if (bdb->version <= 234) > + exp_size = EXP_BDB_LFP_BL_DATA_SIZE_REV_234; > + else if (bdb->version > 234 && bdb->version <= 236) > + exp_size = EXP_BDB_LFP_BL_DATA_SIZE_REV_236; > + else > + exp_size = sizeof(struct bdb_lfp_backlight_data); Usually we go by the newest(IP version, platform...) to the oldest: if (bdb->version >= 236) exp_size = sizeof(struct bdb_lfp_backlight_data); else if (bdb->version >= 234) exp_size = offsetof(struct bdb_lfp_backlight_data, brightness_precision_bits); else exp_size = offsetof(struct bdb_lfp_backlight_data, brightness_level); backlight_control was added in version 191 so no need to set exp_size for older versions. > + > i915->vbt.backlight.type = INTEL_BACKLIGHT_DISPLAY_DDI; > - if (bdb->version >= 191 && > - get_blocksize(backlight_data) >= sizeof(*backlight_data)) { > + if (bdb->version >= 191 && get_blocksize(backlight_data) >= exp_size) { > const struct lfp_backlight_control_method *method; > > method = &backlight_data->backlight_control[panel_type]; > diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h > index 330077c2e588..ba9990e5983c 100644 > --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h > +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h > @@ -814,6 +814,11 @@ struct lfp_brightness_level { > u16 reserved; > } __packed; > > +#define EXP_BDB_LFP_BL_DATA_SIZE_REV_234 \ > + offsetof(struct bdb_lfp_backlight_data, brightness_level) version 234 starts at brightness_level but the size of 234 data must be included to the size, so it should be: offsetof(struct bdb_lfp_backlight_data, brightness_precision_bits). > +#define EXP_BDB_LFP_BL_DATA_SIZE_REV_236 \ > + offsetof(struct bdb_lfp_backlight_data, brightness_precision_bits) > + > struct bdb_lfp_backlight_data { > u8 entry_size; > struct lfp_backlight_data_entry data[16];