| Message ID | 20190529154635.2659-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | efifb: BGRT: Add check for new BGRT status field rotation bits | expand |
On Wed, 29 May 2019 at 17:46, Hans de Goede <hdegoede@redhat.com> wrote: > > Starting with ACPI 6.2 bits 1 and 2 of the BGRT status field are no longer > reserved. These bits are now used to indicate if the image needs to be > rotated before being displayed. > > The efifb code does not support rotating the image before copying it to > the screen. > > This commit adds a check for these new bits and if they are set leaves the > fb contents as is instead of trying to use the un-rotated BGRT image. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > drivers/video/fbdev/efifb.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c > index 9f39f0c360e0..dfa8dd47d19d 100644 > --- a/drivers/video/fbdev/efifb.c > +++ b/drivers/video/fbdev/efifb.c > @@ -169,6 +169,11 @@ static void efifb_show_boot_graphics(struct fb_info *info) > return; > } > > + if (bgrt_tab.status & 0x06) { > + pr_info("efifb: BGRT rotation bits set, not showing boot graphics\n"); > + return; > + } > + > /* Avoid flashing the logo if we're going to print std probe messages */ > if (console_loglevel > CONSOLE_LOGLEVEL_QUIET) > return; > -- > 2.21.0 >
On Mon, 10 Jun 2019 at 17:12, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On Wed, 29 May 2019 at 17:46, Hans de Goede <hdegoede@redhat.com> wrote: > > > > Starting with ACPI 6.2 bits 1 and 2 of the BGRT status field are no longer > > reserved. These bits are now used to indicate if the image needs to be > > rotated before being displayed. > > > > The efifb code does not support rotating the image before copying it to > > the screen. > > > > This commit adds a check for these new bits and if they are set leaves the > > fb contents as is instead of trying to use the un-rotated BGRT image. > > > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > BTW should we make sure that this patch and the efi-bgrt patch get merged at the same time? I guess the net result is just that we get rid of some error in the log, but a rotated BMP will be ignored otherwise. Or is it relevant for userland in some other way?
Hi, On 11-06-19 16:04, Ard Biesheuvel wrote: > On Mon, 10 Jun 2019 at 17:12, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> >> On Wed, 29 May 2019 at 17:46, Hans de Goede <hdegoede@redhat.com> wrote: >>> >>> Starting with ACPI 6.2 bits 1 and 2 of the BGRT status field are no longer >>> reserved. These bits are now used to indicate if the image needs to be >>> rotated before being displayed. >>> >>> The efifb code does not support rotating the image before copying it to >>> the screen. >>> >>> This commit adds a check for these new bits and if they are set leaves the >>> fb contents as is instead of trying to use the un-rotated BGRT image. >>> >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> >> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> > > BTW should we make sure that this patch and the efi-bgrt patch get > merged at the same time? The 2 patches are related but merging them at the same time is not necessary. > I guess the net result is just that we get > rid of some error in the log, but a rotated BMP will be ignored > otherwise. Right, worse case (if the bmp fits pre-rotation) it will be displayed rotated. Note on the one machine I'm aware of which uses these bits the bmp does not fit pre-rotation, so we end up triggering: error: memunmap(bgrt_image); pr_warn("efifb: Ignoring BGRT: unexpected or invalid BMP data\n"); } Which this patch replaces with hitting: if (bgrt_tab.status & 0x06) { pr_info("efifb: BGRT rotation bits set, not showing boot graphics\n"); return; } Instead. So at least on the one machine I know of this is 99% cosmetic. > Or is it relevant for userland in some other way? No. Regards, Hans
On Tue, 11 Jun 2019 at 16:24, Hans de Goede <hdegoede@redhat.com> wrote: > > Hi, > > On 11-06-19 16:04, Ard Biesheuvel wrote: > > On Mon, 10 Jun 2019 at 17:12, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > >> > >> On Wed, 29 May 2019 at 17:46, Hans de Goede <hdegoede@redhat.com> wrote: > >>> > >>> Starting with ACPI 6.2 bits 1 and 2 of the BGRT status field are no longer > >>> reserved. These bits are now used to indicate if the image needs to be > >>> rotated before being displayed. > >>> > >>> The efifb code does not support rotating the image before copying it to > >>> the screen. > >>> > >>> This commit adds a check for these new bits and if they are set leaves the > >>> fb contents as is instead of trying to use the un-rotated BGRT image. > >>> > >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > >> > >> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> > > > > BTW should we make sure that this patch and the efi-bgrt patch get > > merged at the same time? > > The 2 patches are related but merging them at the same time is not > necessary. > > > I guess the net result is just that we get > > rid of some error in the log, but a rotated BMP will be ignored > > otherwise. > > Right, worse case (if the bmp fits pre-rotation) it will be displayed > rotated. Note on the one machine I'm aware of which uses these bits > the bmp does not fit pre-rotation, so we end up triggering: > > error: > memunmap(bgrt_image); > pr_warn("efifb: Ignoring BGRT: unexpected or invalid BMP data\n"); > } > Doesn't that mean we may now end up breaking 'quiet', by exchanging a pr_notice() in the efi-bgrt driver for a pr_warn() in this one? > Which this patch replaces with hitting: > > if (bgrt_tab.status & 0x06) { > pr_info("efifb: BGRT rotation bits set, not showing boot graphics\n"); > return; > } > > Instead. So at least on the one machine I know of this is 99% cosmetic. > > > Or is it relevant for userland in some other way? > > No. > > Regards, > > Hans
Hi, On 11-06-19 16:37, Ard Biesheuvel wrote: > On Tue, 11 Jun 2019 at 16:24, Hans de Goede <hdegoede@redhat.com> wrote: >> >> Hi, >> >> On 11-06-19 16:04, Ard Biesheuvel wrote: >>> On Mon, 10 Jun 2019 at 17:12, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >>>> >>>> On Wed, 29 May 2019 at 17:46, Hans de Goede <hdegoede@redhat.com> wrote: >>>>> >>>>> Starting with ACPI 6.2 bits 1 and 2 of the BGRT status field are no longer >>>>> reserved. These bits are now used to indicate if the image needs to be >>>>> rotated before being displayed. >>>>> >>>>> The efifb code does not support rotating the image before copying it to >>>>> the screen. >>>>> >>>>> This commit adds a check for these new bits and if they are set leaves the >>>>> fb contents as is instead of trying to use the un-rotated BGRT image. >>>>> >>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>> >>>> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>>> >>> >>> BTW should we make sure that this patch and the efi-bgrt patch get >>> merged at the same time? >> >> The 2 patches are related but merging them at the same time is not >> necessary. >> >>> I guess the net result is just that we get >>> rid of some error in the log, but a rotated BMP will be ignored >>> otherwise. >> >> Right, worse case (if the bmp fits pre-rotation) it will be displayed >> rotated. Note on the one machine I'm aware of which uses these bits >> the bmp does not fit pre-rotation, so we end up triggering: >> >> error: >> memunmap(bgrt_image); >> pr_warn("efifb: Ignoring BGRT: unexpected or invalid BMP data\n"); >> } >> > > Doesn't that mean we may now end up breaking 'quiet', by exchanging a > pr_notice() in the efi-bgrt driver for a pr_warn() in this one? quiet has only logged pr_err and more severe for as long as I can remember, so notice / warn does not matter for quiet. Also for flickerfree boot I've made the quiet cut-off configurable (CONFIG_CONSOLE_LOGLEVEL_QUIET) and in Fedora at least we set it to only show messages at KERN_CRIT and more severe levels, since there are simply too many false-positive pr_err messages in the kernel and I quickly got tired of the whack-a-mole game. Regards, Hans
On 6/11/19 4:24 PM, Hans de Goede wrote: > Hi, > > On 11-06-19 16:04, Ard Biesheuvel wrote: >> On Mon, 10 Jun 2019 at 17:12, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >>> >>> On Wed, 29 May 2019 at 17:46, Hans de Goede <hdegoede@redhat.com> wrote: >>>> >>>> Starting with ACPI 6.2 bits 1 and 2 of the BGRT status field are no longer >>>> reserved. These bits are now used to indicate if the image needs to be >>>> rotated before being displayed. >>>> >>>> The efifb code does not support rotating the image before copying it to >>>> the screen. >>>> >>>> This commit adds a check for these new bits and if they are set leaves the >>>> fb contents as is instead of trying to use the un-rotated BGRT image. >>>> >>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>> >>> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>> >> >> BTW should we make sure that this patch and the efi-bgrt patch get >> merged at the same time? > > The 2 patches are related but merging them at the same time is not > necessary. > >> I guess the net result is just that we get >> rid of some error in the log, but a rotated BMP will be ignored >> otherwise. > > Right, worse case (if the bmp fits pre-rotation) it will be displayed > rotated. Note on the one machine I'm aware of which uses these bits > the bmp does not fit pre-rotation, so we end up triggering: > > error: > memunmap(bgrt_image); > pr_warn("efifb: Ignoring BGRT: unexpected or invalid BMP data\n"); > } > > Which this patch replaces with hitting: > > if (bgrt_tab.status & 0x06) { > pr_info("efifb: BGRT rotation bits set, not showing boot graphics\n"); > return; > } > > Instead. So at least on the one machine I know of this is 99% cosmetic. > >> Or is it relevant for userland in some other way? > > No. > > Regards, > > Hans Patch queued for v5.3, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c index 9f39f0c360e0..dfa8dd47d19d 100644 --- a/drivers/video/fbdev/efifb.c +++ b/drivers/video/fbdev/efifb.c @@ -169,6 +169,11 @@ static void efifb_show_boot_graphics(struct fb_info *info) return; } + if (bgrt_tab.status & 0x06) { + pr_info("efifb: BGRT rotation bits set, not showing boot graphics\n"); + return; + } + /* Avoid flashing the logo if we're going to print std probe messages */ if (console_loglevel > CONSOLE_LOGLEVEL_QUIET) return;
Starting with ACPI 6.2 bits 1 and 2 of the BGRT status field are no longer reserved. These bits are now used to indicate if the image needs to be rotated before being displayed. The efifb code does not support rotating the image before copying it to the screen. This commit adds a check for these new bits and if they are set leaves the fb contents as is instead of trying to use the un-rotated BGRT image. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/video/fbdev/efifb.c | 5 +++++ 1 file changed, 5 insertions(+)