Re: [edk2] Library refinement: OptionRomPkg/BltLib
Did you get a chance to review the commit message on caebd915 / r11520? On 2015-08-09 20:09:54, Ni, Ruiyu wrote: Jordan and Laszlo, I reviewed the OptionRomPkg/BltLib again and would like to discuss with you about the potential API refinement. I attached two versions of the refined BltLib.h. The common part of the two versions is: 1. BltLibGetSizes() is removed. Because the size information is actually passed from the library consumer to the library. Caller do not need to get the information from the library interface. Yeah. I guess you have a point here. 2. BltLibBufferToVideoEx and BltLibVideoToBltBufferEx are removed and the accordingly non-Ex version of APIs will be used for *Ex operation. Just for reducing the library APIs and avoid the different APIs have overlapped functionality. Part of the reason was to make simpler functions available for blit operations. I think for the common operations, it is nice to not have to add 3 extra zero parameters. 3. BltLibGopBlt() is removed Same reason as the above #2. I thought this would be convenient for a GOP driver to use to implement the GOP Blt function, but I suppose it is easy enough for a GOP driver to just have the switch in its Blt function. BltLib2.h contains another change. 4. BltLibConfigure() is removed and the frame buffer information will be passed through the four BltLib* APIs. Because it can remove the restriction to allow the library can be re-entry. Does that mean the library will have to check to see if the mode is different? OptionRomPkg/Library/FrameBufferBltLib/FrameBufferBltLib.c:BltLibConfigure does do some work. Similar to my comment above about #2, I liked that the current interface's BltLibBufferToVideo interface was pretty simple. With all these suggestions it will go from 5 parameters to 10 parameters. What's example use where re-entry would be important? -Jordan ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Library refinement: OptionRomPkg/BltLib
Laszlo, The reason I wanted to use the four APIs instead of only one BltLibGopBlt is if user just wants to fill video, he can supply less parameters. I am glad you have no concern to the refine proposal. Jordan, Any thought? Thanks, Ray -Original Message- From: Laszlo Ersek [mailto:ler...@redhat.com] Sent: Monday, August 10, 2015 8:35 PM To: Ni, Ruiyu ruiyu...@intel.com Cc: Justen, Jordan L jordan.l.jus...@intel.com; edk2-devel@lists.01.org edk2-de...@ml01.01.org Subject: Re: Library refinement: OptionRomPkg/BltLib On 08/10/15 05:09, Ni, Ruiyu wrote: Jordan and Laszlo, I reviewed the OptionRomPkg/BltLib again and would like to discuss with you about the potential API refinement. I attached two versions of the refined BltLib.h. The common part of the two versions is: 1. BltLibGetSizes() is removed. Because the size information is actually passed from the library consumer to the library. Caller do not need to get the information from the library interface. 2. BltLibBufferToVideoEx and BltLibVideoToBltBufferEx are removed and the accordingly non-Ex version of APIs will be used for *Ex operation. Just for reducing the library APIs and avoid the different APIs have overlapped functionality. 3. BltLibGopBlt() is removed Same reason as the above #2. BltLib2.h contains another change. 4. BltLibConfigure() is removed and the frame buffer information will be passed through the four BltLib* APIs. Because it can remove the restriction to allow the library can be re-entry. What's your thoughts about this? From the current (nine) BltLib APIs, OvmfPkg/QemuVideoDxe/Gop.c uses two: - BltLibConfigure() in EFI_GRAPHICS_OUTPUT_PROTOCOL.SetMode(), - BltLibGopBlt() in EFI_GRAPHICS_OUTPUT_PROTOCOL.Blt(). I think the first is easy to adapt to BltLib2.h; the mode information should simply be passed to the new (v2) BltLib APIs. The call we have right now can be deleted. Updating the second takes a bit more typing, but it is possible. In QemuVideoGraphicsOutputBlt() we already have a switch on BltOperation, but at the moment it is just passed to BltLibGopBlt(), for all four possible values. In order to adapt the driver to BltLib2.h, the four cases will have to call the four new APIs in BltLib2.h separately (passing in the mode info as well, see above). So, it's doable. Thanks Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Library refinement: OptionRomPkg/BltLib
Just drew a picture to show the current library APIs, V1 and V2 APIs, to help you understand my point. [cid:image001.png@01D0D37C.E78C2030] -Original Message- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ni, Ruiyu Sent: Monday, August 10, 2015 11:10 AM To: Justen, Jordan L jordan.l.jus...@intel.com; Laszlo Ersek ler...@redhat.com Cc: edk2-devel@lists.01.org Subject: [edk2] Library refinement: OptionRomPkg/BltLib Jordan and Laszlo, I reviewed the OptionRomPkg/BltLib again and would like to discuss with you about the potential API refinement. I attached two versions of the refined BltLib.h. The common part of the two versions is: 1. BltLibGetSizes() is removed. Because the size information is actually passed from the library consumer to the library. Caller do not need to get the information from the library interface. 2. BltLibBufferToVideoEx and BltLibVideoToBltBufferEx are removed and the accordingly non-Ex version of APIs will be used for *Ex operation. Just for reducing the library APIs and avoid the different APIs have overlapped functionality. 3. BltLibGopBlt() is removed Same reason as the above #2. BltLib2.h contains another change. 4. BltLibConfigure() is removed and the frame buffer information will be passed through the four BltLib* APIs. Because it can remove the restriction to allow the library can be re-entry. What's your thoughts about this? Regards, Ray ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] Library refinement: OptionRomPkg/BltLib
Jordan and Laszlo, I reviewed the OptionRomPkg/BltLib again and would like to discuss with you about the potential API refinement. I attached two versions of the refined BltLib.h. The common part of the two versions is: 1. BltLibGetSizes() is removed. Because the size information is actually passed from the library consumer to the library. Caller do not need to get the information from the library interface. 2. BltLibBufferToVideoEx and BltLibVideoToBltBufferEx are removed and the accordingly non-Ex version of APIs will be used for *Ex operation. Just for reducing the library APIs and avoid the different APIs have overlapped functionality. 3. BltLibGopBlt() is removed Same reason as the above #2. BltLib2.h contains another change. 4. BltLibConfigure() is removed and the frame buffer information will be passed through the four BltLib* APIs. Because it can remove the restriction to allow the library can be re-entry. What's your thoughts about this? Regards, Ray /** @file Library for performing video blt operations Copyright (c) 2009 - 2011, 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 http://opensource.org/licenses/bsd-license.php THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN AS IS BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. **/ #ifndef __BLT_LIB__ #define __BLT_LIB__ #include Protocol/GraphicsOutput.h /** Configure the BltLib for a frame-buffer @param[in] FrameBuffer Pointer to the start of the frame buffer @param[in] FrameBufferInfo Describes the frame buffer characteristics @retval EFI_INVALID_PARAMETER - Invalid parameter passed in @retval EFI_SUCCESS - Blt operation success **/ EFI_STATUS EFIAPI BltLibConfigure ( IN VOID *FrameBuffer, IN EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *FrameBufferInfo ); /** Performs a UEFI Graphics Output Protocol Blt Video Fill. @param[in] Color Color to fill the region with @param[in] DestinationX X location to start fill operation @param[in] DestinationY Y location to start fill operation @param[in] Width Width (in pixels) to fill @param[in] HeightHeight to fill @retval EFI_DEVICE_ERROR - A hardware error occured @retval EFI_INVALID_PARAMETER - Invalid parameter passed in @retval EFI_SUCCESS - Blt operation success **/ EFI_STATUS EFIAPI BltLibVideoFill ( IN EFI_GRAPHICS_OUTPUT_BLT_PIXEL *Color, IN UINTN DestinationX, IN UINTN DestinationY, IN UINTN Width, IN UINTN Height ); /** Performs a UEFI Graphics Output Protocol Blt Video to Buffer operation with extended parameters. @param[out] BltBuffer Output buffer for pixel color data @param[in] SourceX X location within video @param[in] SourceY Y location within video @param[in] DestinationX X location within BltBuffer @param[in] DestinationY Y location within BltBuffer @param[in] Width Width (in pixels) @param[in] HeightHeight @param[in] Delta Number of bytes in a row of BltBuffer @retval EFI_DEVICE_ERROR - A hardware error occured @retval EFI_INVALID_PARAMETER - Invalid parameter passed in @retval EFI_SUCCESS - Blt operation success **/ EFI_STATUS EFIAPI BltLibVideoToBltBuffer ( OUT EFI_GRAPHICS_OUTPUT_BLT_PIXEL *BltBuffer, IN UINTN SourceX, IN UINTN SourceY, IN UINTN DestinationX, IN UINTN DestinationY, IN UINTN Width, IN UINTN Height, IN UINTN Delta ); /** Performs a UEFI Graphics Output Protocol Blt Buffer to Video operation with extended parameters. @param[in] BltBuffer Output buffer for pixel color data @param[in] SourceX X location within BltBuffer @param[in] SourceY Y location within BltBuffer @param[in] DestinationX X location within video @param[in] DestinationY Y location within video @param[in] Width Width (in pixels) @param[in] HeightHeight @param[in] Delta Number of bytes in a row of BltBuffer @retval EFI_DEVICE_ERROR - A hardware error occured @retval EFI_INVALID_PARAMETER - Invalid parameter passed in @retval EFI_SUCCESS - Blt operation success **/ EFI_STATUS EFIAPI BltLibBufferToVideo ( IN EFI_GRAPHICS_OUTPUT_BLT_PIXEL *BltBuffer, IN UINTN SourceX, IN UINTN
