> pa@xxxxxxxxx (Pierre Asselin) writes:
>
>> After careful reading of the comments in f35cd3fa7729, would this
>> be an appropriate fix ? Does it still address all the issues raised
>> by Thomas ?
>>
>> (not tested)
>>
>> diff --git a/drivers/firmware/sysfb_simplefb.c
>> b/drivers/firmware/sysfb_simplefb.c
>> index 82c64cb9f531..9f5299d54732 100644
>> --- a/drivers/firmware/sysfb_simplefb.c
>> +++ b/drivers/firmware/sysfb_simplefb.c
>> @@ -56,10 +56,11 @@ __init bool sysfb_parse_mode(const struct
>> screen_info *si,
>> * don't specify alpha channels.
>> */
>> if (si->lfb_depth > 8) {
>> - bits_per_pixel = max(max3(si->red_size + si->red_pos,
>> + bits_per_pixel = max3(max3(si->red_size + si->red_pos,
>> si->green_size + si->green_pos,
>> si->blue_size + si->blue_pos),
>> - si->rsvd_size + si->rsvd_pos);
>> + si->rsvd_size + si->rsvd_pos,
>> + si->lfb_depth);
> I would defer to Thomas but personally I don't like it. Seems to me that
> this is getting too complicated just to workaround buggy BIOS that are not
> reporting consistent information about their firmware-provided
> framebuffer.
Okay, but remember, this is a regression. The buggy BIOSes were there
the whole time and the old code that matched f->bits_per_pixel against
si->lfb_depth used to work against these buggy BIOSes.
And is it a bug, or is it an underspecified standard ? "These bits aren't
reserved, we just ignore them" or some such.
My reading of Thomas' comments in the code was that sometimes lfb_depth
was reported too small but never too large ? But I'm not sure. It's
true in my two cases.
> I would either trust the pixel channel information (what Thomas patch did)
> + my patch to calculate the stride (since we can't trust the line lenght
> which is based on the reported depth) + a DMI table for broken machines.
> But that will only work if your systems are the exception and not a common
> issue, otherwise then Thomas' approach won't work if there are too many
> buggy BIOS out there.
The laptop is ancient but the Dell tower is maybe 4 years old. The
BIOS is 09/11/2019 according to dmidecode, and the most recent for
this box.
> Another option is to do the opposite, not calculate a BPP using the pixel
> and then use that value to calculate a new stride, but instead assume that
> the lfb_linelength is correct and use that to calculate the BPP.
Or reject (some) inconsistencies in the struct screen_info and return
false, so the kernel falls back to e.g. vesa ?
> Something like the following patch, which should also fix your regression
> and may be enough to address Thomas' concerns of inconsistent depths too?
I'll try that patch.
