"Pierre Asselin" <pa@xxxxxxxxx> writes: >> pa@xxxxxxxxx (Pierre Asselin) writes: [...] >>> - 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 Yes, I agree that is a regression and has to be fixed. I'm just arguing against this particular fix. > 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 (mis?)understood that it could be too small as well. But that's why I prefer Thomas to chime in before agreeing on any path forward. But he is in vacation this week I believe. >> 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. > I see. >> 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 ? > That a reasonable approach too. But better if we can make simpledrm work too since many distros have dropped all the fbdev drivers in favor of it. >> 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. > Thanks for all your information and testing with this bug! -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
