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 fixes it by comparing a total size of all fileds from the structure (present before the change) with the value gathered from BDB. Tested on Chromebook Pixelbook (Nocturne) (reports bdb->version = 221)
Cc: stable@vger.kernel.org # 5.4+ Tested-by: Lukasz Majczak lma@semihalf.com Signed-off-by: Lukasz Majczak lma@semihalf.com --- drivers/gpu/drm/i915/display/intel_bios.c | 4 +++- drivers/gpu/drm/i915/display/intel_vbt_defs.h | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index 3c25926092de..052a19b455d1 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -452,7 +452,9 @@ parse_lfp_backlight(struct drm_i915_private *i915,
i915->vbt.backlight.type = INTEL_BACKLIGHT_DISPLAY_DDI; if (bdb->version >= 191 && - get_blocksize(backlight_data) >= sizeof(*backlight_data)) { + get_blocksize(backlight_data) >= (sizeof(backlight_data->entry_size) + + sizeof(backlight_data->data) + + sizeof(backlight_data->level))) { 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..fff456bf8783 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;
+/* + * Changing struct bdb_lfp_backlight_data might affect its + * size comparation to the value hold in BDB. + * (e.g. in parse_lfp_backlight()) + */ struct bdb_lfp_backlight_data { u8 entry_size; struct lfp_backlight_data_entry data[16];
On Mon, 20 Sep 2021, Lukasz Majczak lma@semihalf.com 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 fixes it by comparing a total size of all fileds from the structure (present before the change) with the value gathered from BDB. Tested on Chromebook Pixelbook (Nocturne) (reports bdb->version = 221)
Cc: stable@vger.kernel.org # 5.4+
Not a review of the patch, I'll leave that to José, but 5.4 is not right.
The commit is d381baad29b4 ("drm/i915/vbt: Fix backlight parsing for VBT 234+") which was merged in v5.11 and AFAICT has not been backported to any stable kernel.
So this would be:
Fixes: d381baad29b4 ("drm/i915/vbt: Fix backlight parsing for VBT 234+") Cc: stable@vger.kernel.org # v5.11+
BR, Jani.
Tested-by: Lukasz Majczak lma@semihalf.com Signed-off-by: Lukasz Majczak lma@semihalf.com
drivers/gpu/drm/i915/display/intel_bios.c | 4 +++- drivers/gpu/drm/i915/display/intel_vbt_defs.h | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index 3c25926092de..052a19b455d1 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -452,7 +452,9 @@ parse_lfp_backlight(struct drm_i915_private *i915, i915->vbt.backlight.type = INTEL_BACKLIGHT_DISPLAY_DDI; if (bdb->version >= 191 &&
get_blocksize(backlight_data) >= sizeof(*backlight_data)) {
get_blocksize(backlight_data) >= (sizeof(backlight_data->entry_size) +sizeof(backlight_data->data) +sizeof(backlight_data->level))) {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..fff456bf8783 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; +/*
- Changing struct bdb_lfp_backlight_data might affect its
- size comparation to the value hold in BDB.
- (e.g. in parse_lfp_backlight())
- */
struct bdb_lfp_backlight_data { u8 entry_size; struct lfp_backlight_data_entry data[16];
On Mon, 2021-09-20 at 16:11 +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 fixes it by comparing a total size of all fileds from the structure (present before the change) with the value gathered from BDB. Tested on Chromebook Pixelbook (Nocturne) (reports bdb->version = 221)
Cc: stable@vger.kernel.org # 5.4+ Tested-by: Lukasz Majczak lma@semihalf.com Signed-off-by: Lukasz Majczak lma@semihalf.com
drivers/gpu/drm/i915/display/intel_bios.c | 4 +++- drivers/gpu/drm/i915/display/intel_vbt_defs.h | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index 3c25926092de..052a19b455d1 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -452,7 +452,9 @@ parse_lfp_backlight(struct drm_i915_private *i915, i915->vbt.backlight.type = INTEL_BACKLIGHT_DISPLAY_DDI; if (bdb->version >= 191 &&
get_blocksize(backlight_data) >= sizeof(*backlight_data)) {
get_blocksize(backlight_data) >= (sizeof(backlight_data->entry_size) +sizeof(backlight_data->data) +sizeof(backlight_data->level))) {
Missing sizeof(backlight_data->backlight_control) but this is getting very verbose. Would be better have a expected size variable set each version set in the beginning of this function.
something like: switch (bdb->version) { case 191: expected_size = x; break; case 234: expected_size = x; break; case 236: default: expected_size = x; }
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..fff456bf8783 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; +/*
- Changing struct bdb_lfp_backlight_data might affect its
- size comparation to the value hold in BDB.
- (e.g. in parse_lfp_backlight())
- */
This is true for all the blocks so I don't think we need this comment.
struct bdb_lfp_backlight_data { u8 entry_size; struct lfp_backlight_data_entry data[16];
pon., 20 wrz 2021 o 22:47 Souza, Jose jose.souza@intel.com napisał(a):
On Mon, 2021-09-20 at 16:11 +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 fixes it by comparing a total size of all fileds from the structure (present before the change) with the value gathered from BDB. Tested on Chromebook Pixelbook (Nocturne) (reports bdb->version = 221)
Cc: stable@vger.kernel.org # 5.4+ Tested-by: Lukasz Majczak lma@semihalf.com Signed-off-by: Lukasz Majczak lma@semihalf.com
drivers/gpu/drm/i915/display/intel_bios.c | 4 +++- drivers/gpu/drm/i915/display/intel_vbt_defs.h | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index 3c25926092de..052a19b455d1 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -452,7 +452,9 @@ parse_lfp_backlight(struct drm_i915_private *i915,
i915->vbt.backlight.type = INTEL_BACKLIGHT_DISPLAY_DDI; if (bdb->version >= 191 &&
get_blocksize(backlight_data) >= sizeof(*backlight_data)) {
get_blocksize(backlight_data) >= (sizeof(backlight_data->entry_size) +sizeof(backlight_data->data) +sizeof(backlight_data->level))) {Missing sizeof(backlight_data->backlight_control) but this is getting very verbose. Would be better have a expected size variable set each version set in the beginning of this function.
something like: switch (bdb->version) { case 191: expected_size = x; break; case 234: expected_size = x; break; case 236: default: expected_size = x; }
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..fff456bf8783 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;
+/*
- Changing struct bdb_lfp_backlight_data might affect its
- size comparation to the value hold in BDB.
- (e.g. in parse_lfp_backlight())
- */
This is true for all the blocks so I don't think we need this comment.
struct bdb_lfp_backlight_data { u8 entry_size; struct lfp_backlight_data_entry data[16];
Hi Jose, Jani
Jani - you are right - I was working on 5.4 with a backported patch - I'm sorry for this confusion.
Jose,
Regarding expected_size, I couldn't find documentation that could described this structure size changes among revisions, so all I could do is to do an educated guess, basing on comments at this structure, like:
(gdb) ptype /o struct bdb_lfp_backlight_data /* offset | size */ type = struct bdb_lfp_backlight_data { /* 0 | 1 */ u8 entry_size; /* 1 | 96 */ struct lfp_backlight_data_entry data[16]; /* 97 | 16 */ u8 level[16]; /* 113 | 16 */ struct lfp_backlight_control_method backlight_control[16]; /* 129 | 64 */ struct lfp_brightness_level brightness_level[16]; /* 234+ */ /* 193 | 64 */ struct lfp_brightness_level brightness_min_level[16]; /* 234+ */ /* 257 | 16 */ u8 brightness_precision_bits[16]; /* 236+ */
/* total size (bytes): 273 */ }
if (revision <= 234) expected_size = 129; else if (revision > 234 && revision <=236) expected_size = 257; else /* revision > 236 */ expected_size = 273;
Is this approach ok? Otherwise I think I would need help from you to get exact numbers for each revision...
Best regards, Lukasz
linux-stable-mirror@lists.linaro.org
-
Jani Nikula -
Lukasz Majczak -
Souza, Jose