* [PATCH] OvmfPkg/Gop: clear the screen to black in SetMode()
@ 2018-03-13 10:05 Ruiyu Ni
2018-03-13 13:09 ` Laszlo Ersek
0 siblings, 1 reply; 3+ messages in thread
From: Ruiyu Ni @ 2018-03-13 10:05 UTC (permalink / raw)
To: edk2-devel; +Cc: Jordan Justen, Laszlo Ersek, Ard Biesheuvel
Today's implementation forgot to clear the screen to black in
SetMode(). It causes SCT SetMode() test fails.
The patch adds the clear screen operation in SetMode() to fix
the SCT failure.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
OvmfPkg/QemuVideoDxe/Gop.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
index 512fd27acb..f573f7011a 100644
--- a/OvmfPkg/QemuVideoDxe/Gop.c
+++ b/OvmfPkg/QemuVideoDxe/Gop.c
@@ -1,7 +1,7 @@
/** @file
Graphics Output Protocol functions for the QEMU video controller.
- Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -196,9 +196,10 @@ Routine Description:
--*/
{
- QEMU_VIDEO_PRIVATE_DATA *Private;
- QEMU_VIDEO_MODE_DATA *ModeData;
- RETURN_STATUS Status;
+ QEMU_VIDEO_PRIVATE_DATA *Private;
+ QEMU_VIDEO_MODE_DATA *ModeData;
+ RETURN_STATUS Status;
+ EFI_GRAPHICS_OUTPUT_BLT_PIXEL Black;
Private = QEMU_VIDEO_PRIVATE_DATA_FROM_GRAPHICS_OUTPUT_THIS (This);
@@ -271,7 +272,19 @@ Routine Description:
}
ASSERT (Status == RETURN_SUCCESS);
- return EFI_SUCCESS;
+ //
+ // Per UEFI Spec, need to clear the visible portions of the output display to black.
+ //
+ ZeroMem (&Black, sizeof (Black));
+ return FrameBufferBlt (
+ Private->FrameBufferBltConfigure,
+ &Black,
+ EfiBltVideoFill,
+ 0, 0,
+ 0, 0,
+ This->Mode->Info->HorizontalResolution, This->Mode->Info->VerticalResolution,
+ 0
+ );
}
EFI_STATUS
--
2.16.1.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] OvmfPkg/Gop: clear the screen to black in SetMode()
2018-03-13 10:05 [PATCH] OvmfPkg/Gop: clear the screen to black in SetMode() Ruiyu Ni
@ 2018-03-13 13:09 ` Laszlo Ersek
2018-03-13 14:09 ` Ni, Ruiyu
0 siblings, 1 reply; 3+ messages in thread
From: Laszlo Ersek @ 2018-03-13 13:09 UTC (permalink / raw)
To: Ruiyu Ni, edk2-devel; +Cc: Jordan Justen, Ard Biesheuvel
Hi Ray,
On 03/13/18 11:05, Ruiyu Ni wrote:
> Today's implementation forgot to clear the screen to black in
> SetMode(). It causes SCT SetMode() test fails.
>
> The patch adds the clear screen operation in SetMode() to fix
> the SCT failure.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> OvmfPkg/QemuVideoDxe/Gop.c | 23 ++++++++++++++++++-----
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
> index 512fd27acb..f573f7011a 100644
> --- a/OvmfPkg/QemuVideoDxe/Gop.c
> +++ b/OvmfPkg/QemuVideoDxe/Gop.c
> @@ -1,7 +1,7 @@
> /** @file
> Graphics Output Protocol functions for the QEMU video controller.
>
> - Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> @@ -196,9 +196,10 @@ Routine Description:
>
> --*/
> {
> - QEMU_VIDEO_PRIVATE_DATA *Private;
> - QEMU_VIDEO_MODE_DATA *ModeData;
> - RETURN_STATUS Status;
> + QEMU_VIDEO_PRIVATE_DATA *Private;
> + QEMU_VIDEO_MODE_DATA *ModeData;
> + RETURN_STATUS Status;
> + EFI_GRAPHICS_OUTPUT_BLT_PIXEL Black;
>
> Private = QEMU_VIDEO_PRIVATE_DATA_FROM_GRAPHICS_OUTPUT_THIS (This);
>
> @@ -271,7 +272,19 @@ Routine Description:
> }
> ASSERT (Status == RETURN_SUCCESS);
>
> - return EFI_SUCCESS;
> + //
> + // Per UEFI Spec, need to clear the visible portions of the output display to black.
> + //
> + ZeroMem (&Black, sizeof (Black));
> + return FrameBufferBlt (
> + Private->FrameBufferBltConfigure,
> + &Black,
> + EfiBltVideoFill,
> + 0, 0,
> + 0, 0,
> + This->Mode->Info->HorizontalResolution, This->Mode->Info->VerticalResolution,
> + 0
> + );
> }
>
> EFI_STATUS
>
Thanks for the patch!
What would you think of the following update: FrameBufferBlt() is
documented to return RETURN_INVALID_PARAMETER for "Invalid parameter
were passed in", and RETURN_SUCCESS otherwise ("The Blt operation was
performed successfully"). I suggest that we keep the "return
EFI_SUCCESS" in the end, just ASSERT() that FrameBufferBlt() succeeds.
(Even if there was a third possibility, i.e. for FrameBufferBlt() to
fail for a different reason, I think that should still not report that
SetMode() failed.)
If you agree with the idea, please update the patch before pushing it.
If you disagree with it, I don't insist.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] OvmfPkg/Gop: clear the screen to black in SetMode()
2018-03-13 13:09 ` Laszlo Ersek
@ 2018-03-13 14:09 ` Ni, Ruiyu
0 siblings, 0 replies; 3+ messages in thread
From: Ni, Ruiyu @ 2018-03-13 14:09 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@lists.01.org; +Cc: Justen, Jordan L, Ard Biesheuvel
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo
> Ersek
> Sent: Tuesday, March 13, 2018 9:10 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>
> Subject: Re: [edk2] [PATCH] OvmfPkg/Gop: clear the screen to black in SetMode()
>
> Hi Ray,
>
> On 03/13/18 11:05, Ruiyu Ni wrote:
> > Today's implementation forgot to clear the screen to black in
> > SetMode(). It causes SCT SetMode() test fails.
> >
> > The patch adds the clear screen operation in SetMode() to fix the SCT
> > failure.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> > OvmfPkg/QemuVideoDxe/Gop.c | 23 ++++++++++++++++++-----
> > 1 file changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
> > index 512fd27acb..f573f7011a 100644
> > --- a/OvmfPkg/QemuVideoDxe/Gop.c
> > +++ b/OvmfPkg/QemuVideoDxe/Gop.c
> > @@ -1,7 +1,7 @@
> > /** @file
> > Graphics Output Protocol functions for the QEMU video controller.
> >
> > - Copyright (c) 2007 - 2017, Intel Corporation. All rights
> > reserved.<BR>
> > + Copyright (c) 2007 - 2018, Intel Corporation. All rights
> > + reserved.<BR>
> >
> > This program and the accompanying materials
> > are licensed and made available under the terms and conditions of
> > the BSD License @@ -196,9 +196,10 @@ Routine Description:
> >
> > --*/
> > {
> > - QEMU_VIDEO_PRIVATE_DATA *Private;
> > - QEMU_VIDEO_MODE_DATA *ModeData;
> > - RETURN_STATUS Status;
> > + QEMU_VIDEO_PRIVATE_DATA *Private;
> > + QEMU_VIDEO_MODE_DATA *ModeData;
> > + RETURN_STATUS Status;
> > + EFI_GRAPHICS_OUTPUT_BLT_PIXEL Black;
> >
> > Private = QEMU_VIDEO_PRIVATE_DATA_FROM_GRAPHICS_OUTPUT_THIS
> (This);
> >
> > @@ -271,7 +272,19 @@ Routine Description:
> > }
> > ASSERT (Status == RETURN_SUCCESS);
> >
> > - return EFI_SUCCESS;
> > + //
> > + // Per UEFI Spec, need to clear the visible portions of the output display to
> black.
> > + //
> > + ZeroMem (&Black, sizeof (Black));
> > + return FrameBufferBlt (
> > + Private->FrameBufferBltConfigure,
> > + &Black,
> > + EfiBltVideoFill,
> > + 0, 0,
> > + 0, 0,
> > + This->Mode->Info->HorizontalResolution, This->Mode->Info-
> >VerticalResolution,
> > + 0
> > + );
> > }
> >
> > EFI_STATUS
> >
>
> Thanks for the patch!
>
> What would you think of the following update: FrameBufferBlt() is documented
> to return RETURN_INVALID_PARAMETER for "Invalid parameter were passed in",
> and RETURN_SUCCESS otherwise ("The Blt operation was performed
> successfully"). I suggest that we keep the "return EFI_SUCCESS" in the end, just
> ASSERT() that FrameBufferBlt() succeeds.
>
> (Even if there was a third possibility, i.e. for FrameBufferBlt() to fail for a
> different reason, I think that should still not report that
> SetMode() failed.)
I agree!
>
> If you agree with the idea, please update the patch before pushing it.
> If you disagree with it, I don't insist.
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
> Thanks
> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-03-13 14:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-13 10:05 [PATCH] OvmfPkg/Gop: clear the screen to black in SetMode() Ruiyu Ni
2018-03-13 13:09 ` Laszlo Ersek
2018-03-13 14:09 ` Ni, Ruiyu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox