* [PATCH 0/3] MdeModulePkg/FrameBufferBltLib: Use dynamic allocated line buffer
@ 2017-01-23 6:11 Ruiyu Ni
2017-01-23 6:11 ` [PATCH 1/3] MdeModulePkg/FrameBufferBltLib: Refine ConfigurePixelBitMaskFormat Ruiyu Ni
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Ruiyu Ni @ 2017-01-23 6:11 UTC (permalink / raw)
To: edk2-devel
The patches optimized the FrameBufferBltLib to use less memory than old implementation.
QemuVideoDxe driver needs to be updated to avoid hang due to wrong assumption.
Ruiyu Ni (3):
MdeModulePkg/FrameBufferBltLib: Refine ConfigurePixelBitMaskFormat
MdeModulePkg/FrameBufferBltLib: Use dynamic allocated line buffer
OvmfPkg/QemuVideoDxe: Frame buffer config size may change in new mode
.../Library/FrameBufferBltLib/FrameBufferBltLib.c | 95 +++++++++++++---------
OvmfPkg/QemuVideoDxe/Gop.c | 47 ++++++-----
2 files changed, 81 insertions(+), 61 deletions(-)
--
2.9.0.windows.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/3] MdeModulePkg/FrameBufferBltLib: Refine ConfigurePixelBitMaskFormat 2017-01-23 6:11 [PATCH 0/3] MdeModulePkg/FrameBufferBltLib: Use dynamic allocated line buffer Ruiyu Ni @ 2017-01-23 6:11 ` Ruiyu Ni 2017-01-23 8:36 ` Tian, Feng 2017-01-23 6:11 ` [PATCH 2/3] MdeModulePkg/FrameBufferBltLib: Use dynamic allocated line buffer Ruiyu Ni 2017-01-23 6:11 ` [PATCH 3/3] OvmfPkg/QemuVideoDxe: Frame buffer config size may change in new mode Ruiyu Ni 2 siblings, 1 reply; 7+ messages in thread From: Ruiyu Ni @ 2017-01-23 6:11 UTC (permalink / raw) To: edk2-devel; +Cc: Feng Tian https://bugzilla.tianocore.org/show_bug.cgi?id=339 The patch refines ConfigurePixelBitMaskFormat() to prepare the enhancement in next commit: Enhance this library to use dynamic allocated line buffer to reduce memory usage of frame buffer configure. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Feng Tian <feng.tian@intel.com> --- .../Library/FrameBufferBltLib/FrameBufferBltLib.c | 71 +++++++++++++--------- 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c b/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c index c9bb206..5f6eddc 100644 --- a/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c +++ b/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c @@ -1,7 +1,7 @@ /** @file FrameBufferBltLib - Library to perform blt operations on a frame buffer. - Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.<BR> + Copyright (c) 2007 - 2017, 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 which accompanies this distribution. The full text of the license may be found at @@ -30,8 +30,8 @@ struct FRAME_BUFFER_CONFIGURE { UINT8 *FrameBuffer; EFI_GRAPHICS_PIXEL_FORMAT PixelFormat; EFI_PIXEL_BITMASK PixelMasks; - INTN PixelShl[4]; // R-G-B-Rsvd - INTN PixelShr[4]; // R-G-B-Rsvd + INT8 PixelShl[4]; // R-G-B-Rsvd + INT8 PixelShr[4]; // R-G-B-Rsvd }; CONST EFI_PIXEL_BITMASK mRgbPixelMasks = { @@ -45,43 +45,47 @@ CONST EFI_PIXEL_BITMASK mBgrPixelMasks = { /** Initialize the bit mask in frame buffer configure. - @param Configure The frame buffer configure. - @param BitMask The bit mask of pixel. + @param BitMask The bit mask of pixel. + @param BytesPerPixel Size in bytes of pixel. + @param PixelShl Left shift array. + @param PixelShr Right shift array. **/ VOID -ConfigurePixelBitMaskFormat ( - IN FRAME_BUFFER_CONFIGURE *Configure, - IN CONST EFI_PIXEL_BITMASK *BitMask +FrameBufferBltLibConfigurePixelFormat ( + IN CONST EFI_PIXEL_BITMASK *BitMask, + OUT UINTN *BytesPerPixel, + OUT INT8 *PixelShl, + OUT INT8 *PixelShr ) { - UINTN Loop; + UINT8 Index; UINT32 *Masks; UINT32 MergedMasks; + ASSERT (BytesPerPixel != NULL); + MergedMasks = 0; Masks = (UINT32*) BitMask; - for (Loop = 0; Loop < 3; Loop++) { - ASSERT ((Loop == 3) || (Masks[Loop] != 0)); - ASSERT ((MergedMasks & Masks[Loop]) == 0); - Configure->PixelShl[Loop] = HighBitSet32 (Masks[Loop]) - 23 + (Loop * 8); - if (Configure->PixelShl[Loop] < 0) { - Configure->PixelShr[Loop] = -Configure->PixelShl[Loop]; - Configure->PixelShl[Loop] = 0; + for (Index = 0; Index < 3; Index++) { + ASSERT ((MergedMasks & Masks[Index]) == 0); + + PixelShl[Index] = (INT8) HighBitSet32 (Masks[Index]) - 23 + (Index * 8); + if (PixelShl[Index] < 0) { + PixelShr[Index] = -PixelShl[Index]; + PixelShl[Index] = 0; } else { - Configure->PixelShr[Loop] = 0; + PixelShr[Index] = 0; } - MergedMasks = (UINT32) (MergedMasks | Masks[Loop]); - DEBUG ((EFI_D_VERBOSE, "%d: shl:%d shr:%d mask:%x\n", Loop, - Configure->PixelShl[Loop], Configure->PixelShr[Loop], Masks[Loop])); + DEBUG ((DEBUG_INFO, "%d: shl:%d shr:%d mask:%x\n", Index, + PixelShl[Index], PixelShr[Index], Masks[Index])); + + MergedMasks = (UINT32) (MergedMasks | Masks[Index]); } MergedMasks = (UINT32) (MergedMasks | Masks[3]); ASSERT (MergedMasks != 0); - Configure->BytesPerPixel = (UINTN) ((HighBitSet32 (MergedMasks) + 7) / 8); - - DEBUG ((EFI_D_VERBOSE, "Bytes per pixel: %d\n", Configure->BytesPerPixel)); - - CopyMem (&Configure->PixelMasks, BitMask, sizeof (*BitMask)); + *BytesPerPixel = (UINTN) ((HighBitSet32 (MergedMasks) + 7) / 8); + DEBUG ((DEBUG_INFO, "Bytes per pixel: %d\n", *BytesPerPixel)); } /** @@ -110,6 +114,11 @@ FrameBufferBltConfigure ( IN OUT UINTN *ConfigureSize ) { + CONST EFI_PIXEL_BITMASK *BitMask; + UINTN BytesPerPixel; + INT8 PixelShl[4]; + INT8 PixelShr[4]; + if (ConfigureSize == NULL) { return RETURN_INVALID_PARAMETER; } @@ -125,15 +134,15 @@ FrameBufferBltConfigure ( switch (FrameBufferInfo->PixelFormat) { case PixelRedGreenBlueReserved8BitPerColor: - ConfigurePixelBitMaskFormat (Configure, &mRgbPixelMasks); + BitMask = &mRgbPixelMasks; break; case PixelBlueGreenRedReserved8BitPerColor: - ConfigurePixelBitMaskFormat (Configure, &mBgrPixelMasks); + BitMask = &mBgrPixelMasks; break; case PixelBitMask: - ConfigurePixelBitMaskFormat (Configure, &(FrameBufferInfo->PixelInformation)); + BitMask = &FrameBufferInfo->PixelInformation; break; case PixelBltOnly: @@ -145,6 +154,12 @@ FrameBufferBltConfigure ( return RETURN_INVALID_PARAMETER; } + FrameBufferBltLibConfigurePixelFormat (BitMask, &BytesPerPixel, PixelShl, PixelShr); + + CopyMem (&Configure->PixelMasks, BitMask, sizeof (*BitMask)); + CopyMem (Configure->PixelShl, PixelShl, sizeof (PixelShl)); + CopyMem (Configure->PixelShr, PixelShr, sizeof (PixelShr)); + Configure->BytesPerPixel = BytesPerPixel; Configure->PixelFormat = FrameBufferInfo->PixelFormat; Configure->FrameBuffer = (UINT8*) FrameBuffer; Configure->WidthInPixels = (UINTN) FrameBufferInfo->HorizontalResolution; -- 2.9.0.windows.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] MdeModulePkg/FrameBufferBltLib: Refine ConfigurePixelBitMaskFormat 2017-01-23 6:11 ` [PATCH 1/3] MdeModulePkg/FrameBufferBltLib: Refine ConfigurePixelBitMaskFormat Ruiyu Ni @ 2017-01-23 8:36 ` Tian, Feng 0 siblings, 0 replies; 7+ messages in thread From: Tian, Feng @ 2017-01-23 8:36 UTC (permalink / raw) To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Tian, Feng Reviewed-by: Feng Tian <feng.tian@intel.com> Thanks Feng -----Original Message----- From: Ni, Ruiyu Sent: Monday, January 23, 2017 2:12 PM To: edk2-devel@lists.01.org Cc: Tian, Feng <feng.tian@intel.com> Subject: [PATCH 1/3] MdeModulePkg/FrameBufferBltLib: Refine ConfigurePixelBitMaskFormat https://bugzilla.tianocore.org/show_bug.cgi?id=339 The patch refines ConfigurePixelBitMaskFormat() to prepare the enhancement in next commit: Enhance this library to use dynamic allocated line buffer to reduce memory usage of frame buffer configure. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Feng Tian <feng.tian@intel.com> --- .../Library/FrameBufferBltLib/FrameBufferBltLib.c | 71 +++++++++++++--------- 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c b/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c index c9bb206..5f6eddc 100644 --- a/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c +++ b/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c @@ -1,7 +1,7 @@ /** @file FrameBufferBltLib - Library to perform blt operations on a frame buffer. - Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.<BR> + Copyright (c) 2007 - 2017, 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 which accompanies this distribution. The full text of the license may be found at @@ -30,8 +30,8 @@ struct FRAME_BUFFER_CONFIGURE { UINT8 *FrameBuffer; EFI_GRAPHICS_PIXEL_FORMAT PixelFormat; EFI_PIXEL_BITMASK PixelMasks; - INTN PixelShl[4]; // R-G-B-Rsvd - INTN PixelShr[4]; // R-G-B-Rsvd + INT8 PixelShl[4]; // R-G-B-Rsvd + INT8 PixelShr[4]; // R-G-B-Rsvd }; CONST EFI_PIXEL_BITMASK mRgbPixelMasks = { @@ -45,43 +45,47 @@ CONST EFI_PIXEL_BITMASK mBgrPixelMasks = { /** Initialize the bit mask in frame buffer configure. - @param Configure The frame buffer configure. - @param BitMask The bit mask of pixel. + @param BitMask The bit mask of pixel. + @param BytesPerPixel Size in bytes of pixel. + @param PixelShl Left shift array. + @param PixelShr Right shift array. **/ VOID -ConfigurePixelBitMaskFormat ( - IN FRAME_BUFFER_CONFIGURE *Configure, - IN CONST EFI_PIXEL_BITMASK *BitMask +FrameBufferBltLibConfigurePixelFormat ( + IN CONST EFI_PIXEL_BITMASK *BitMask, + OUT UINTN *BytesPerPixel, + OUT INT8 *PixelShl, + OUT INT8 *PixelShr ) { - UINTN Loop; + UINT8 Index; UINT32 *Masks; UINT32 MergedMasks; + ASSERT (BytesPerPixel != NULL); + MergedMasks = 0; Masks = (UINT32*) BitMask; - for (Loop = 0; Loop < 3; Loop++) { - ASSERT ((Loop == 3) || (Masks[Loop] != 0)); - ASSERT ((MergedMasks & Masks[Loop]) == 0); - Configure->PixelShl[Loop] = HighBitSet32 (Masks[Loop]) - 23 + (Loop * 8); - if (Configure->PixelShl[Loop] < 0) { - Configure->PixelShr[Loop] = -Configure->PixelShl[Loop]; - Configure->PixelShl[Loop] = 0; + for (Index = 0; Index < 3; Index++) { + ASSERT ((MergedMasks & Masks[Index]) == 0); + + PixelShl[Index] = (INT8) HighBitSet32 (Masks[Index]) - 23 + (Index * 8); + if (PixelShl[Index] < 0) { + PixelShr[Index] = -PixelShl[Index]; + PixelShl[Index] = 0; } else { - Configure->PixelShr[Loop] = 0; + PixelShr[Index] = 0; } - MergedMasks = (UINT32) (MergedMasks | Masks[Loop]); - DEBUG ((EFI_D_VERBOSE, "%d: shl:%d shr:%d mask:%x\n", Loop, - Configure->PixelShl[Loop], Configure->PixelShr[Loop], Masks[Loop])); + DEBUG ((DEBUG_INFO, "%d: shl:%d shr:%d mask:%x\n", Index, + PixelShl[Index], PixelShr[Index], Masks[Index])); + + MergedMasks = (UINT32) (MergedMasks | Masks[Index]); } MergedMasks = (UINT32) (MergedMasks | Masks[3]); ASSERT (MergedMasks != 0); - Configure->BytesPerPixel = (UINTN) ((HighBitSet32 (MergedMasks) + 7) / 8); - - DEBUG ((EFI_D_VERBOSE, "Bytes per pixel: %d\n", Configure->BytesPerPixel)); - - CopyMem (&Configure->PixelMasks, BitMask, sizeof (*BitMask)); + *BytesPerPixel = (UINTN) ((HighBitSet32 (MergedMasks) + 7) / 8); + DEBUG ((DEBUG_INFO, "Bytes per pixel: %d\n", *BytesPerPixel)); } /** @@ -110,6 +114,11 @@ FrameBufferBltConfigure ( IN OUT UINTN *ConfigureSize ) { + CONST EFI_PIXEL_BITMASK *BitMask; + UINTN BytesPerPixel; + INT8 PixelShl[4]; + INT8 PixelShr[4]; + if (ConfigureSize == NULL) { return RETURN_INVALID_PARAMETER; } @@ -125,15 +134,15 @@ FrameBufferBltConfigure ( switch (FrameBufferInfo->PixelFormat) { case PixelRedGreenBlueReserved8BitPerColor: - ConfigurePixelBitMaskFormat (Configure, &mRgbPixelMasks); + BitMask = &mRgbPixelMasks; break; case PixelBlueGreenRedReserved8BitPerColor: - ConfigurePixelBitMaskFormat (Configure, &mBgrPixelMasks); + BitMask = &mBgrPixelMasks; break; case PixelBitMask: - ConfigurePixelBitMaskFormat (Configure, &(FrameBufferInfo->PixelInformation)); + BitMask = &FrameBufferInfo->PixelInformation; break; case PixelBltOnly: @@ -145,6 +154,12 @@ FrameBufferBltConfigure ( return RETURN_INVALID_PARAMETER; } + FrameBufferBltLibConfigurePixelFormat (BitMask, &BytesPerPixel, + PixelShl, PixelShr); + + CopyMem (&Configure->PixelMasks, BitMask, sizeof (*BitMask)); + CopyMem (Configure->PixelShl, PixelShl, sizeof (PixelShl)); + CopyMem (Configure->PixelShr, PixelShr, sizeof (PixelShr)); + Configure->BytesPerPixel = BytesPerPixel; Configure->PixelFormat = FrameBufferInfo->PixelFormat; Configure->FrameBuffer = (UINT8*) FrameBuffer; Configure->WidthInPixels = (UINTN) FrameBufferInfo->HorizontalResolution; -- 2.9.0.windows.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] MdeModulePkg/FrameBufferBltLib: Use dynamic allocated line buffer 2017-01-23 6:11 [PATCH 0/3] MdeModulePkg/FrameBufferBltLib: Use dynamic allocated line buffer Ruiyu Ni 2017-01-23 6:11 ` [PATCH 1/3] MdeModulePkg/FrameBufferBltLib: Refine ConfigurePixelBitMaskFormat Ruiyu Ni @ 2017-01-23 6:11 ` Ruiyu Ni 2017-01-23 8:36 ` Tian, Feng 2017-01-23 6:11 ` [PATCH 3/3] OvmfPkg/QemuVideoDxe: Frame buffer config size may change in new mode Ruiyu Ni 2 siblings, 1 reply; 7+ messages in thread From: Ruiyu Ni @ 2017-01-23 6:11 UTC (permalink / raw) To: edk2-devel; +Cc: Feng Tian https://bugzilla.tianocore.org/show_bug.cgi?id=339 The patch uses dynamic allocated line buffer to reduce memory usage of frame buffer configure. (Original implementation uses 0x4000 bytes for line buffer.) Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Feng Tian <feng.tian@intel.com> --- .../Library/FrameBufferBltLib/FrameBufferBltLib.c | 24 +++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c b/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c index 5f6eddc..011d9c5 100644 --- a/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c +++ b/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c @@ -26,12 +26,12 @@ struct FRAME_BUFFER_CONFIGURE { UINTN BytesPerPixel; UINTN WidthInPixels; UINTN Height; - UINT8 LineBuffer[SIZE_4KB * sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL)]; UINT8 *FrameBuffer; EFI_GRAPHICS_PIXEL_FORMAT PixelFormat; EFI_PIXEL_BITMASK PixelMasks; INT8 PixelShl[4]; // R-G-B-Rsvd INT8 PixelShr[4]; // R-G-B-Rsvd + UINT8 LineBuffer[0]; }; CONST EFI_PIXEL_BITMASK mRgbPixelMasks = { @@ -123,15 +123,6 @@ FrameBufferBltConfigure ( return RETURN_INVALID_PARAMETER; } - if (*ConfigureSize < sizeof (FRAME_BUFFER_CONFIGURE)) { - *ConfigureSize = sizeof (FRAME_BUFFER_CONFIGURE); - return RETURN_BUFFER_TOO_SMALL; - } - - if (Configure == NULL) { - return RETURN_INVALID_PARAMETER; - } - switch (FrameBufferInfo->PixelFormat) { case PixelRedGreenBlueReserved8BitPerColor: BitMask = &mRgbPixelMasks; @@ -156,6 +147,17 @@ FrameBufferBltConfigure ( FrameBufferBltLibConfigurePixelFormat (BitMask, &BytesPerPixel, PixelShl, PixelShr); + if (*ConfigureSize < sizeof (FRAME_BUFFER_CONFIGURE) + + FrameBufferInfo->HorizontalResolution * BytesPerPixel) { + *ConfigureSize = sizeof (FRAME_BUFFER_CONFIGURE) + + FrameBufferInfo->HorizontalResolution * BytesPerPixel; + return RETURN_BUFFER_TOO_SMALL; + } + + if (Configure == NULL) { + return RETURN_INVALID_PARAMETER; + } + CopyMem (&Configure->PixelMasks, BitMask, sizeof (*BitMask)); CopyMem (Configure->PixelShl, PixelShl, sizeof (PixelShl)); CopyMem (Configure->PixelShr, PixelShr, sizeof (PixelShr)); @@ -166,8 +168,6 @@ FrameBufferBltConfigure ( Configure->Height = (UINTN) FrameBufferInfo->VerticalResolution; Configure->WidthInBytes = Configure->WidthInPixels * Configure->BytesPerPixel; - ASSERT (Configure->WidthInBytes < sizeof (Configure->LineBuffer)); - return RETURN_SUCCESS; } -- 2.9.0.windows.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] MdeModulePkg/FrameBufferBltLib: Use dynamic allocated line buffer 2017-01-23 6:11 ` [PATCH 2/3] MdeModulePkg/FrameBufferBltLib: Use dynamic allocated line buffer Ruiyu Ni @ 2017-01-23 8:36 ` Tian, Feng 0 siblings, 0 replies; 7+ messages in thread From: Tian, Feng @ 2017-01-23 8:36 UTC (permalink / raw) To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Tian, Feng Reviewed-by: Feng Tian <feng.tian@intel.com> Thanks Feng -----Original Message----- From: Ni, Ruiyu Sent: Monday, January 23, 2017 2:12 PM To: edk2-devel@lists.01.org Cc: Tian, Feng <feng.tian@intel.com> Subject: [PATCH 2/3] MdeModulePkg/FrameBufferBltLib: Use dynamic allocated line buffer https://bugzilla.tianocore.org/show_bug.cgi?id=339 The patch uses dynamic allocated line buffer to reduce memory usage of frame buffer configure. (Original implementation uses 0x4000 bytes for line buffer.) Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Feng Tian <feng.tian@intel.com> --- .../Library/FrameBufferBltLib/FrameBufferBltLib.c | 24 +++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c b/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c index 5f6eddc..011d9c5 100644 --- a/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c +++ b/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c @@ -26,12 +26,12 @@ struct FRAME_BUFFER_CONFIGURE { UINTN BytesPerPixel; UINTN WidthInPixels; UINTN Height; - UINT8 LineBuffer[SIZE_4KB * sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL)]; UINT8 *FrameBuffer; EFI_GRAPHICS_PIXEL_FORMAT PixelFormat; EFI_PIXEL_BITMASK PixelMasks; INT8 PixelShl[4]; // R-G-B-Rsvd INT8 PixelShr[4]; // R-G-B-Rsvd + UINT8 LineBuffer[0]; }; CONST EFI_PIXEL_BITMASK mRgbPixelMasks = { @@ -123,15 +123,6 @@ FrameBufferBltConfigure ( return RETURN_INVALID_PARAMETER; } - if (*ConfigureSize < sizeof (FRAME_BUFFER_CONFIGURE)) { - *ConfigureSize = sizeof (FRAME_BUFFER_CONFIGURE); - return RETURN_BUFFER_TOO_SMALL; - } - - if (Configure == NULL) { - return RETURN_INVALID_PARAMETER; - } - switch (FrameBufferInfo->PixelFormat) { case PixelRedGreenBlueReserved8BitPerColor: BitMask = &mRgbPixelMasks; @@ -156,6 +147,17 @@ FrameBufferBltConfigure ( FrameBufferBltLibConfigurePixelFormat (BitMask, &BytesPerPixel, PixelShl, PixelShr); + if (*ConfigureSize < sizeof (FRAME_BUFFER_CONFIGURE) + + FrameBufferInfo->HorizontalResolution * BytesPerPixel) { + *ConfigureSize = sizeof (FRAME_BUFFER_CONFIGURE) + + FrameBufferInfo->HorizontalResolution * BytesPerPixel; + return RETURN_BUFFER_TOO_SMALL; + } + + if (Configure == NULL) { + return RETURN_INVALID_PARAMETER; + } + CopyMem (&Configure->PixelMasks, BitMask, sizeof (*BitMask)); CopyMem (Configure->PixelShl, PixelShl, sizeof (PixelShl)); CopyMem (Configure->PixelShr, PixelShr, sizeof (PixelShr)); @@ -166,8 +168,6 @@ FrameBufferBltConfigure ( Configure->Height = (UINTN) FrameBufferInfo->VerticalResolution; Configure->WidthInBytes = Configure->WidthInPixels * Configure->BytesPerPixel; - ASSERT (Configure->WidthInBytes < sizeof (Configure->LineBuffer)); - return RETURN_SUCCESS; } -- 2.9.0.windows.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] OvmfPkg/QemuVideoDxe: Frame buffer config size may change in new mode 2017-01-23 6:11 [PATCH 0/3] MdeModulePkg/FrameBufferBltLib: Use dynamic allocated line buffer Ruiyu Ni 2017-01-23 6:11 ` [PATCH 1/3] MdeModulePkg/FrameBufferBltLib: Refine ConfigurePixelBitMaskFormat Ruiyu Ni 2017-01-23 6:11 ` [PATCH 2/3] MdeModulePkg/FrameBufferBltLib: Use dynamic allocated line buffer Ruiyu Ni @ 2017-01-23 6:11 ` Ruiyu Ni 2017-01-23 10:45 ` Laszlo Ersek 2 siblings, 1 reply; 7+ messages in thread From: Ruiyu Ni @ 2017-01-23 6:11 UTC (permalink / raw) To: edk2-devel; +Cc: Jordan Justen, Laszlo Ersek https://bugzilla.tianocore.org/show_bug.cgi?id=339 The patch removes the assumption in QemuVideoDxe driver that it wrongly assumes the frame buffer configure size is the same in different video modes. The assumption is true in old FrameBufferBltLib but is false in new implementation. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> --- OvmfPkg/QemuVideoDxe/Gop.c | 47 +++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c index 5485ba3..359e921 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 - 2016, Intel Corporation. All rights reserved.<BR> + Copyright (c) 2007 - 2017, 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 @@ -189,30 +189,35 @@ Routine Description: QemuVideoCompleteModeData (Private, This->Mode); // - // Allocate when using first time. + // Re-initialize the frame buffer configure when mode changes. // - if (Private->FrameBufferBltConfigure == NULL) { - Status = FrameBufferBltConfigure ( - (VOID*) (UINTN) This->Mode->FrameBufferBase, - This->Mode->Info, - Private->FrameBufferBltConfigure, - &Private->FrameBufferBltConfigureSize - ); - ASSERT (Status == RETURN_BUFFER_TOO_SMALL); + Status = FrameBufferBltConfigure ( + (VOID*) (UINTN) This->Mode->FrameBufferBase, + This->Mode->Info, + Private->FrameBufferBltConfigure, + &Private->FrameBufferBltConfigureSize + ); + if (Status == RETURN_BUFFER_TOO_SMALL) { + // + // Frame buffer configure may be larger in new mode. + // + if (Private->FrameBufferBltConfigure != NULL) { + FreePool (Private->FrameBufferBltConfigure); + } Private->FrameBufferBltConfigure = AllocatePool (Private->FrameBufferBltConfigureSize); - } + ASSERT (Private->FrameBufferBltConfigure != NULL); - // - // Create the configuration for FrameBufferBltLib - // - ASSERT (Private->FrameBufferBltConfigure != NULL); - Status = FrameBufferBltConfigure ( - (VOID*) (UINTN) This->Mode->FrameBufferBase, - This->Mode->Info, - Private->FrameBufferBltConfigure, - &Private->FrameBufferBltConfigureSize - ); + // + // Create the configuration for FrameBufferBltLib + // + Status = FrameBufferBltConfigure ( + (VOID*) (UINTN) This->Mode->FrameBufferBase, + This->Mode->Info, + Private->FrameBufferBltConfigure, + &Private->FrameBufferBltConfigureSize + ); + } ASSERT (Status == RETURN_SUCCESS); return EFI_SUCCESS; -- 2.9.0.windows.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] OvmfPkg/QemuVideoDxe: Frame buffer config size may change in new mode 2017-01-23 6:11 ` [PATCH 3/3] OvmfPkg/QemuVideoDxe: Frame buffer config size may change in new mode Ruiyu Ni @ 2017-01-23 10:45 ` Laszlo Ersek 0 siblings, 0 replies; 7+ messages in thread From: Laszlo Ersek @ 2017-01-23 10:45 UTC (permalink / raw) To: Ruiyu Ni, edk2-devel; +Cc: Jordan Justen On 01/23/17 07:11, Ruiyu Ni wrote: > https://bugzilla.tianocore.org/show_bug.cgi?id=339 > > The patch removes the assumption in QemuVideoDxe driver that it > wrongly assumes the frame buffer configure size is the same in > different video modes. > The assumption is true in old FrameBufferBltLib but is false in > new implementation. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> > Cc: Jordan Justen <jordan.l.justen@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > --- > OvmfPkg/QemuVideoDxe/Gop.c | 47 +++++++++++++++++++++++++--------------------- > 1 file changed, 26 insertions(+), 21 deletions(-) Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks! Laszlo > diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c > index 5485ba3..359e921 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 - 2016, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2007 - 2017, 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 > @@ -189,30 +189,35 @@ Routine Description: > QemuVideoCompleteModeData (Private, This->Mode); > > // > - // Allocate when using first time. > + // Re-initialize the frame buffer configure when mode changes. > // > - if (Private->FrameBufferBltConfigure == NULL) { > - Status = FrameBufferBltConfigure ( > - (VOID*) (UINTN) This->Mode->FrameBufferBase, > - This->Mode->Info, > - Private->FrameBufferBltConfigure, > - &Private->FrameBufferBltConfigureSize > - ); > - ASSERT (Status == RETURN_BUFFER_TOO_SMALL); > + Status = FrameBufferBltConfigure ( > + (VOID*) (UINTN) This->Mode->FrameBufferBase, > + This->Mode->Info, > + Private->FrameBufferBltConfigure, > + &Private->FrameBufferBltConfigureSize > + ); > + if (Status == RETURN_BUFFER_TOO_SMALL) { > + // > + // Frame buffer configure may be larger in new mode. > + // > + if (Private->FrameBufferBltConfigure != NULL) { > + FreePool (Private->FrameBufferBltConfigure); > + } > Private->FrameBufferBltConfigure = > AllocatePool (Private->FrameBufferBltConfigureSize); > - } > + ASSERT (Private->FrameBufferBltConfigure != NULL); > > - // > - // Create the configuration for FrameBufferBltLib > - // > - ASSERT (Private->FrameBufferBltConfigure != NULL); > - Status = FrameBufferBltConfigure ( > - (VOID*) (UINTN) This->Mode->FrameBufferBase, > - This->Mode->Info, > - Private->FrameBufferBltConfigure, > - &Private->FrameBufferBltConfigureSize > - ); > + // > + // Create the configuration for FrameBufferBltLib > + // > + Status = FrameBufferBltConfigure ( > + (VOID*) (UINTN) This->Mode->FrameBufferBase, > + This->Mode->Info, > + Private->FrameBufferBltConfigure, > + &Private->FrameBufferBltConfigureSize > + ); > + } > ASSERT (Status == RETURN_SUCCESS); > > return EFI_SUCCESS; > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-01-23 10:45 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-01-23 6:11 [PATCH 0/3] MdeModulePkg/FrameBufferBltLib: Use dynamic allocated line buffer Ruiyu Ni 2017-01-23 6:11 ` [PATCH 1/3] MdeModulePkg/FrameBufferBltLib: Refine ConfigurePixelBitMaskFormat Ruiyu Ni 2017-01-23 8:36 ` Tian, Feng 2017-01-23 6:11 ` [PATCH 2/3] MdeModulePkg/FrameBufferBltLib: Use dynamic allocated line buffer Ruiyu Ni 2017-01-23 8:36 ` Tian, Feng 2017-01-23 6:11 ` [PATCH 3/3] OvmfPkg/QemuVideoDxe: Frame buffer config size may change in new mode Ruiyu Ni 2017-01-23 10:45 ` Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox