* [PATCH 0/2] MdePkg: UiLib library
@ 2016-11-09 15:43 Felix Poludov
2016-11-10 1:53 ` Yao, Jiewen
0 siblings, 1 reply; 9+ messages in thread
From: Felix Poludov @ 2016-11-09 15:43 UTC (permalink / raw)
To: edk2-devel@lists.01.org
This series of two patches introduces new UiLib library.
1. New UiLib library class is added to MdePkg.
The library is intended for functions that produce UI elements.
The patch introduces two functions that produce a popup window:
UiCreatePopUp and UiCreatePopUpVaList.
2. An instance of UiLib library class is added to MdePkg.
Popup implementation is copied from UefiLib
(CreatePopUp function in UefiLib/Console.c).
3. MdePkg/UefiLib library is updated to implement CreatePopUp
by calling UiCreatePopUpVaList.
4. Platform DSC files are updated to add UiLib instance.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Felix Polyudov <felixp@ami.com>
AppPkg/AppPkg.dsc | 1 +
ArmPkg/ArmPkg.dsc | 1 +
BeagleBoardPkg/BeagleBoardPkg.dsc | 1 +
CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 1 +
CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 1 +
CryptoPkg/CryptoPkg.dsc | 1 +
DuetPkg/DuetPkgIa32.dsc | 1 +
DuetPkg/DuetPkgX64.dsc | 1 +
EdkCompatibilityPkg/EdkCompatibilityPkg.dsc | 1 +
EmbeddedPkg/EmbeddedPkg.dsc | 1 +
EmulatorPkg/EmulatorPkg.dsc | 1 +
FatPkg/FatPkg.dsc | 1 +
.../IntelFrameworkModulePkg.dsc | 1 +
IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc | 1 +
IntelFspWrapperPkg/IntelFspWrapperPkg.dsc | 1 +
MdeModulePkg/MdeModulePkg.dsc | 1 +
MdePkg/Include/Library/UiLib.h | 71 +++++
MdePkg/Library/UefiLib/Console.c | 252 +---------------
MdePkg/Library/UefiLib/UefiLib.inf | 1 +
MdePkg/Library/UefiLib/UefiLibInternal.h | 1 +
MdePkg/Library/UiLib/UiLib.c | 334 +++++++++++++++++++++
MdePkg/Library/UiLib/UiLib.inf | 38 +++
MdePkg/Library/UiLib/UiLib.uni | 21 ++
MdePkg/MdePkg.dec | 3 +
NetworkPkg/NetworkPkg.dsc | 1 +
Nt32Pkg/Nt32Pkg.dsc | 1 +
Omap35xxPkg/Omap35xxPkg.dsc | 1 +
OptionRomPkg/OptionRomPkg.dsc | 1 +
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
PcAtChipsetPkg/PcAtChipsetPkg.dsc | 1 +
PerformancePkg/PerformancePkg.dsc | 1 +
QuarkPlatformPkg/Quark.dsc | 1 +
QuarkPlatformPkg/QuarkMin.dsc | 1 +
QuarkSocPkg/QuarkSocPkg.dsc | 1 +
SecurityPkg/SecurityPkg.dsc | 1 +
ShellPkg/ShellPkg.dsc | 1 +
SourceLevelDebugPkg/SourceLevelDebugPkg.dsc | 1 +
StdLib/StdLib.dsc | 1 +
UefiCpuPkg/UefiCpuPkg.dsc | 1 +
Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 1 +
Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 1 +
Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 1 +
44 files changed, 506 insertions(+), 251 deletions(-)
create mode 100644 MdePkg/Include/Library/UiLib.h
create mode 100644 MdePkg/Library/UiLib/UiLib.c
create mode 100644 MdePkg/Library/UiLib/UiLib.inf
create mode 100644 MdePkg/Library/UiLib/UiLib.uni
Please consider the environment before printing this email.
The information contained in this message may be confidential and proprietary to American Megatrends, Inc. This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] MdePkg: UiLib library
2016-11-09 15:43 [PATCH 0/2] MdePkg: UiLib library Felix Poludov
@ 2016-11-10 1:53 ` Yao, Jiewen
2016-11-10 13:37 ` Felix Poludov
0 siblings, 1 reply; 9+ messages in thread
From: Yao, Jiewen @ 2016-11-10 1:53 UTC (permalink / raw)
To: Felix Poludov, edk2-devel@lists.01.org
HI Poludov
It is a good idea to have a new UiLib.
Both Mike and I give the suggestion to "You could define new lib and update all the code that uses the old API to use the new API."
IMHO, I do not think it is right way to let UefiLib depend on a UiLib.
We have plan to resolve it, but since you already have proposal to add UiLib. I hope it can be resolved here.
There is no real need to put UiLib into MdePkg. MdePkg is designed to hold EFI/PI speciation or industry standard.
In this case, I believe MdeModulePkg is a better place.
Thank you
Yao Jiewen
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Felix Poludov
> Sent: Wednesday, November 9, 2016 11:44 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 0/2] MdePkg: UiLib library
>
> This series of two patches introduces new UiLib library.
> 1. New UiLib library class is added to MdePkg.
> The library is intended for functions that produce UI elements.
> The patch introduces two functions that produce a popup window:
> UiCreatePopUp and UiCreatePopUpVaList.
> 2. An instance of UiLib library class is added to MdePkg.
> Popup implementation is copied from UefiLib
> (CreatePopUp function in UefiLib/Console.c).
> 3. MdePkg/UefiLib library is updated to implement CreatePopUp
> by calling UiCreatePopUpVaList.
> 4. Platform DSC files are updated to add UiLib instance.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Felix Polyudov <felixp@ami.com>
>
> AppPkg/AppPkg.dsc | 1 +
> ArmPkg/ArmPkg.dsc | 1 +
> BeagleBoardPkg/BeagleBoardPkg.dsc | 1 +
> CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 1 +
> CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 1 +
> CryptoPkg/CryptoPkg.dsc | 1 +
> DuetPkg/DuetPkgIa32.dsc | 1 +
> DuetPkg/DuetPkgX64.dsc | 1 +
> EdkCompatibilityPkg/EdkCompatibilityPkg.dsc | 1 +
> EmbeddedPkg/EmbeddedPkg.dsc | 1 +
> EmulatorPkg/EmulatorPkg.dsc | 1 +
> FatPkg/FatPkg.dsc | 1 +
> .../IntelFrameworkModulePkg.dsc | 1 +
> IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc | 1 +
> IntelFspWrapperPkg/IntelFspWrapperPkg.dsc | 1 +
> MdeModulePkg/MdeModulePkg.dsc | 1 +
> MdePkg/Include/Library/UiLib.h | 71 +++++
> MdePkg/Library/UefiLib/Console.c | 252
> +---------------
> MdePkg/Library/UefiLib/UefiLib.inf | 1 +
> MdePkg/Library/UefiLib/UefiLibInternal.h | 1 +
> MdePkg/Library/UiLib/UiLib.c | 334
> +++++++++++++++++++++
> MdePkg/Library/UiLib/UiLib.inf | 38 +++
> MdePkg/Library/UiLib/UiLib.uni | 21 ++
> MdePkg/MdePkg.dec | 3 +
> NetworkPkg/NetworkPkg.dsc | 1 +
> Nt32Pkg/Nt32Pkg.dsc | 1 +
> Omap35xxPkg/Omap35xxPkg.dsc | 1 +
> OptionRomPkg/OptionRomPkg.dsc | 1 +
> OvmfPkg/OvmfPkgIa32.dsc | 1 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> OvmfPkg/OvmfPkgX64.dsc | 1 +
> PcAtChipsetPkg/PcAtChipsetPkg.dsc | 1 +
> PerformancePkg/PerformancePkg.dsc | 1 +
> QuarkPlatformPkg/Quark.dsc | 1 +
> QuarkPlatformPkg/QuarkMin.dsc | 1 +
> QuarkSocPkg/QuarkSocPkg.dsc | 1 +
> SecurityPkg/SecurityPkg.dsc | 1 +
> ShellPkg/ShellPkg.dsc | 1 +
> SourceLevelDebugPkg/SourceLevelDebugPkg.dsc | 1 +
> StdLib/StdLib.dsc | 1 +
> UefiCpuPkg/UefiCpuPkg.dsc | 1 +
> Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 1 +
> Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 1 +
> Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 1 +
> 44 files changed, 506 insertions(+), 251 deletions(-)
> create mode 100644 MdePkg/Include/Library/UiLib.h
> create mode 100644 MdePkg/Library/UiLib/UiLib.c
> create mode 100644 MdePkg/Library/UiLib/UiLib.inf
> create mode 100644 MdePkg/Library/UiLib/UiLib.uni
>
> Please consider the environment before printing this email.
>
> The information contained in this message may be confidential and
> proprietary to American Megatrends, Inc. This communication is intended
> to be read only by the individual or entity to whom it is addressed or by their
> designee. If the reader of this message is not the intended recipient, you are
> on notice that any distribution of this message, in any form, is strictly
> prohibited. Please promptly notify the sender by reply e-mail or by
> telephone at 770-246-8600, and then delete or destroy all copies of the
> transmission.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] MdePkg: UiLib library
2016-11-10 1:53 ` Yao, Jiewen
@ 2016-11-10 13:37 ` Felix Poludov
2016-11-10 15:10 ` Gao, Liming
0 siblings, 1 reply; 9+ messages in thread
From: Felix Poludov @ 2016-11-10 13:37 UTC (permalink / raw)
To: Yao, Jiewen, edk2-devel@lists.01.org
Hi Jiewen,
Updating "all the code that uses the old API to use the new API" is not always possible.
We can certainly update all the EDKII packages; however, it is trickier with the non-open source packages.
For example, the Reference Code we are getting from a chipset vendor is using CreatePopUp.
Certainly, if we mark CreatePopUp deprecated (as you have suggested), the chances are, reference code will get eventually updated not to use the function.
However, it might take a significant amount of time.
Moreover, a project integrators that create board firmware by combining open-source and close-source packages coming from multiple sources(from multiple vendors),
would have to solve this issue again and again; whereas with my proposal, the issue is solved by upgrading EDKII code base (no need to touch third party packages).
Having said that, I do understand your concerns with keeping the library in MdePkg.
How about finding a middle ground between your proposal and my proposal?
We can keep UiLib instance in MdePkg, but keep only a minimalistic barebones implementation there.
We can also have another, more advanced, UiLib instance in the MdeModulePkg.
This technique is already used in EDKII.
For example, MdePkg defines DebugLib library class and provides BaseDebugLibSerialPort instance.
MdeModulePkg provides PeiDxeDebugLibReportStatusCode instance for the same library class.
Since "advanced" version of Popup is not currently available, there is no need to create new instance in MdeModulePkg.
However, once Eric is ready with his Popup-on-top-of-HII implementation, it can be added to a new UiLib instance in MdeModulePkg.
Also, if new functions are added to UiLib in the future, MdePkg UiLib instance can have a NULL implementation like many other libraries in the MdePkg.
What do you think?
Thanks
Felix
From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
Sent: Wednesday, November 09, 2016 8:54 PM
To: Felix Poludov; edk2-devel@lists.01.org
Subject: RE: [PATCH 0/2] MdePkg: UiLib library
HI Poludov
It is a good idea to have a new UiLib.
Both Mike and I give the suggestion to "You could define new lib and update all the code that uses the old API to use the new API."
IMHO, I do not think it is right way to let UefiLib depend on a UiLib.
We have plan to resolve it, but since you already have proposal to add UiLib. I hope it can be resolved here.
There is no real need to put UiLib into MdePkg. MdePkg is designed to hold EFI/PI speciation or industry standard.
In this case, I believe MdeModulePkg is a better place.
Thank you
Yao Jiewen
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Felix Poludov
> Sent: Wednesday, November 9, 2016 11:44 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 0/2] MdePkg: UiLib library
>
> This series of two patches introduces new UiLib library.
> 1. New UiLib library class is added to MdePkg.
> The library is intended for functions that produce UI elements.
> The patch introduces two functions that produce a popup window:
> UiCreatePopUp and UiCreatePopUpVaList.
> 2. An instance of UiLib library class is added to MdePkg.
> Popup implementation is copied from UefiLib
> (CreatePopUp function in UefiLib/Console.c).
> 3. MdePkg/UefiLib library is updated to implement CreatePopUp
> by calling UiCreatePopUpVaList.
> 4. Platform DSC files are updated to add UiLib instance.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Felix Polyudov <felixp@ami.com>
>
> AppPkg/AppPkg.dsc | 1 +
> ArmPkg/ArmPkg.dsc | 1 +
> BeagleBoardPkg/BeagleBoardPkg.dsc | 1 +
> CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 1 +
> CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 1 +
> CryptoPkg/CryptoPkg.dsc | 1 +
> DuetPkg/DuetPkgIa32.dsc | 1 +
> DuetPkg/DuetPkgX64.dsc | 1 +
> EdkCompatibilityPkg/EdkCompatibilityPkg.dsc | 1 +
> EmbeddedPkg/EmbeddedPkg.dsc | 1 +
> EmulatorPkg/EmulatorPkg.dsc | 1 +
> FatPkg/FatPkg.dsc | 1 +
> .../IntelFrameworkModulePkg.dsc | 1 +
> IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc | 1 +
> IntelFspWrapperPkg/IntelFspWrapperPkg.dsc | 1 +
> MdeModulePkg/MdeModulePkg.dsc | 1 +
> MdePkg/Include/Library/UiLib.h | 71 +++++
> MdePkg/Library/UefiLib/Console.c | 252
> +---------------
> MdePkg/Library/UefiLib/UefiLib.inf | 1 +
> MdePkg/Library/UefiLib/UefiLibInternal.h | 1 +
> MdePkg/Library/UiLib/UiLib.c | 334
> +++++++++++++++++++++
> MdePkg/Library/UiLib/UiLib.inf | 38 +++
> MdePkg/Library/UiLib/UiLib.uni | 21 ++
> MdePkg/MdePkg.dec | 3 +
> NetworkPkg/NetworkPkg.dsc | 1 +
> Nt32Pkg/Nt32Pkg.dsc | 1 +
> Omap35xxPkg/Omap35xxPkg.dsc | 1 +
> OptionRomPkg/OptionRomPkg.dsc | 1 +
> OvmfPkg/OvmfPkgIa32.dsc | 1 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> OvmfPkg/OvmfPkgX64.dsc | 1 +
> PcAtChipsetPkg/PcAtChipsetPkg.dsc | 1 +
> PerformancePkg/PerformancePkg.dsc | 1 +
> QuarkPlatformPkg/Quark.dsc | 1 +
> QuarkPlatformPkg/QuarkMin.dsc | 1 +
> QuarkSocPkg/QuarkSocPkg.dsc | 1 +
> SecurityPkg/SecurityPkg.dsc | 1 +
> ShellPkg/ShellPkg.dsc | 1 +
> SourceLevelDebugPkg/SourceLevelDebugPkg.dsc | 1 +
> StdLib/StdLib.dsc | 1 +
> UefiCpuPkg/UefiCpuPkg.dsc | 1 +
> Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 1 +
> Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 1 +
> Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 1 +
> 44 files changed, 506 insertions(+), 251 deletions(-)
> create mode 100644 MdePkg/Include/Library/UiLib.h
> create mode 100644 MdePkg/Library/UiLib/UiLib.c
> create mode 100644 MdePkg/Library/UiLib/UiLib.inf
> create mode 100644 MdePkg/Library/UiLib/UiLib.uni
>
> Please consider the environment before printing this email.
>
> The information contained in this message may be confidential and
> proprietary to American Megatrends, Inc. This communication is intended
> to be read only by the individual or entity to whom it is addressed or by their
> designee. If the reader of this message is not the intended recipient, you are
> on notice that any distribution of this message, in any form, is strictly
> prohibited. Please promptly notify the sender by reply e-mail or by
> telephone at 770-246-8600, and then delete or destroy all copies of the
> transmission.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
Please consider the environment before printing this email.
The information contained in this message may be confidential and proprietary to American Megatrends, Inc. This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] MdePkg: UiLib library
2016-11-10 13:37 ` Felix Poludov
@ 2016-11-10 15:10 ` Gao, Liming
2016-11-10 15:44 ` Felix Poludov
0 siblings, 1 reply; 9+ messages in thread
From: Gao, Liming @ 2016-11-10 15:10 UTC (permalink / raw)
To: Felix Poludov, Yao, Jiewen, edk2-devel@lists.01.org
Felix:
I think two major concerns are here.
1) UefiLib depends on UiLib
2) UiLib class is defined in MdePkg.
To resolve it, I have another idea.
1) Introduce new API RegisterCreatePopUpHandler() in UefiLib. Its input parameter is the handler function that is same to UiCreatePopUpVaList().
2) Update CreatePopup() to use the registered handler if new handler is registered. If no handler, CreatePopup() still uses current logic.
3) If platform wants to customize CreatePopup(), it can provide its library instance with library constructor only. Its library class is NULL, its library constructor() will consume RegisterCreatePopUpHandler() to register its handler.
This solution is compatible. It just updates UefiLib, doesn't need to update platform DSC files if the platforms don't enable the customized popup. It doesn't conflict with new UiLib design. New UiLib class introduce new APIs and update edk2 codes to use new one. For the driver uses old API, this way can provide the customization. For the driver uses new API, the different UiLib instances can provide the customization.
Thanks
Liming
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Felix Poludov
Sent: Thursday, November 10, 2016 9:38 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH 0/2] MdePkg: UiLib library
Hi Jiewen,
Updating "all the code that uses the old API to use the new API" is not always possible.
We can certainly update all the EDKII packages; however, it is trickier with the non-open source packages.
For example, the Reference Code we are getting from a chipset vendor is using CreatePopUp.
Certainly, if we mark CreatePopUp deprecated (as you have suggested), the chances are, reference code will get eventually updated not to use the function.
However, it might take a significant amount of time.
Moreover, a project integrators that create board firmware by combining open-source and close-source packages coming from multiple sources(from multiple vendors),
would have to solve this issue again and again; whereas with my proposal, the issue is solved by upgrading EDKII code base (no need to touch third party packages).
Having said that, I do understand your concerns with keeping the library in MdePkg.
How about finding a middle ground between your proposal and my proposal?
We can keep UiLib instance in MdePkg, but keep only a minimalistic barebones implementation there.
We can also have another, more advanced, UiLib instance in the MdeModulePkg.
This technique is already used in EDKII.
For example, MdePkg defines DebugLib library class and provides BaseDebugLibSerialPort instance.
MdeModulePkg provides PeiDxeDebugLibReportStatusCode instance for the same library class.
Since "advanced" version of Popup is not currently available, there is no need to create new instance in MdeModulePkg.
However, once Eric is ready with his Popup-on-top-of-HII implementation, it can be added to a new UiLib instance in MdeModulePkg.
Also, if new functions are added to UiLib in the future, MdePkg UiLib instance can have a NULL implementation like many other libraries in the MdePkg.
What do you think?
Thanks
Felix
From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
Sent: Wednesday, November 09, 2016 8:54 PM
To: Felix Poludov; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Subject: RE: [PATCH 0/2] MdePkg: UiLib library
HI Poludov
It is a good idea to have a new UiLib.
Both Mike and I give the suggestion to "You could define new lib and update all the code that uses the old API to use the new API."
IMHO, I do not think it is right way to let UefiLib depend on a UiLib.
We have plan to resolve it, but since you already have proposal to add UiLib. I hope it can be resolved here.
There is no real need to put UiLib into MdePkg. MdePkg is designed to hold EFI/PI speciation or industry standard.
In this case, I believe MdeModulePkg is a better place.
Thank you
Yao Jiewen
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Felix Poludov
> Sent: Wednesday, November 9, 2016 11:44 PM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Subject: [edk2] [PATCH 0/2] MdePkg: UiLib library
>
> This series of two patches introduces new UiLib library.
> 1. New UiLib library class is added to MdePkg.
> The library is intended for functions that produce UI elements.
> The patch introduces two functions that produce a popup window:
> UiCreatePopUp and UiCreatePopUpVaList.
> 2. An instance of UiLib library class is added to MdePkg.
> Popup implementation is copied from UefiLib
> (CreatePopUp function in UefiLib/Console.c).
> 3. MdePkg/UefiLib library is updated to implement CreatePopUp
> by calling UiCreatePopUpVaList.
> 4. Platform DSC files are updated to add UiLib instance.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Felix Polyudov
>
> AppPkg/AppPkg.dsc | 1 +
> ArmPkg/ArmPkg.dsc | 1 +
> BeagleBoardPkg/BeagleBoardPkg.dsc | 1 +
> CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 1 +
> CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 1 +
> CryptoPkg/CryptoPkg.dsc | 1 +
> DuetPkg/DuetPkgIa32.dsc | 1 +
> DuetPkg/DuetPkgX64.dsc | 1 +
> EdkCompatibilityPkg/EdkCompatibilityPkg.dsc | 1 +
> EmbeddedPkg/EmbeddedPkg.dsc | 1 +
> EmulatorPkg/EmulatorPkg.dsc | 1 +
> FatPkg/FatPkg.dsc | 1 +
> .../IntelFrameworkModulePkg.dsc | 1 +
> IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc | 1 +
> IntelFspWrapperPkg/IntelFspWrapperPkg.dsc | 1 +
> MdeModulePkg/MdeModulePkg.dsc | 1 +
> MdePkg/Include/Library/UiLib.h | 71 +++++
> MdePkg/Library/UefiLib/Console.c | 252
> +---------------
> MdePkg/Library/UefiLib/UefiLib.inf | 1 +
> MdePkg/Library/UefiLib/UefiLibInternal.h | 1 +
> MdePkg/Library/UiLib/UiLib.c | 334
> +++++++++++++++++++++
> MdePkg/Library/UiLib/UiLib.inf | 38 +++
> MdePkg/Library/UiLib/UiLib.uni | 21 ++
> MdePkg/MdePkg.dec | 3 +
> NetworkPkg/NetworkPkg.dsc | 1 +
> Nt32Pkg/Nt32Pkg.dsc | 1 +
> Omap35xxPkg/Omap35xxPkg.dsc | 1 +
> OptionRomPkg/OptionRomPkg.dsc | 1 +
> OvmfPkg/OvmfPkgIa32.dsc | 1 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> OvmfPkg/OvmfPkgX64.dsc | 1 +
> PcAtChipsetPkg/PcAtChipsetPkg.dsc | 1 +
> PerformancePkg/PerformancePkg.dsc | 1 +
> QuarkPlatformPkg/Quark.dsc | 1 +
> QuarkPlatformPkg/QuarkMin.dsc | 1 +
> QuarkSocPkg/QuarkSocPkg.dsc | 1 +
> SecurityPkg/SecurityPkg.dsc | 1 +
> ShellPkg/ShellPkg.dsc | 1 +
> SourceLevelDebugPkg/SourceLevelDebugPkg.dsc | 1 +
> StdLib/StdLib.dsc | 1 +
> UefiCpuPkg/UefiCpuPkg.dsc | 1 +
> Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 1 +
> Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 1 +
> Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 1 +
> 44 files changed, 506 insertions(+), 251 deletions(-)
> create mode 100644 MdePkg/Include/Library/UiLib.h
> create mode 100644 MdePkg/Library/UiLib/UiLib.c
> create mode 100644 MdePkg/Library/UiLib/UiLib.inf
> create mode 100644 MdePkg/Library/UiLib/UiLib.uni
>
> Please consider the environment before printing this email.
>
> The information contained in this message may be confidential and
> proprietary to American Megatrends, Inc. This communication is intended
> to be read only by the individual or entity to whom it is addressed or by their
> designee. If the reader of this message is not the intended recipient, you are
> on notice that any distribution of this message, in any form, is strictly
> prohibited. Please promptly notify the sender by reply e-mail or by
> telephone at 770-246-8600, and then delete or destroy all copies of the
> transmission.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
Please consider the environment before printing this email.
The information contained in this message may be confidential and proprietary to American Megatrends, Inc. This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] MdePkg: UiLib library
2016-11-10 15:10 ` Gao, Liming
@ 2016-11-10 15:44 ` Felix Poludov
2016-11-10 16:02 ` Gao, Liming
2016-11-10 16:04 ` Kinney, Michael D
0 siblings, 2 replies; 9+ messages in thread
From: Felix Poludov @ 2016-11-10 15:44 UTC (permalink / raw)
To: Gao, Liming, Yao, Jiewen, edk2-devel@lists.01.org
Liming,
With your proposal if a projects uses RegisterCreatePopUpHandler(), DSC file has to be updated to link popup handler NULL library
with all the modules that consume CreatePopup().
Since projects tend to evolve (new modules added, existing code changes), this sounds like a constant maintenance effort.
Project integrator would have to scan source tree after each commit to check if a new code that uses CretePopup has been added and if new DSC file modifications are required.
Thanks
Felix
From: Gao, Liming [mailto:liming.gao@intel.com]
Sent: Thursday, November 10, 2016 10:10 AM
To: Felix Poludov; Yao, Jiewen; edk2-devel@lists.01.org
Subject: RE: [PATCH 0/2] MdePkg: UiLib library
Felix:
I think two major concerns are here.
1) UefiLib depends on UiLib
2) UiLib class is defined in MdePkg.
To resolve it, I have another idea.
1) Introduce new API RegisterCreatePopUpHandler() in UefiLib. Its input parameter is the handler function that is same to UiCreatePopUpVaList().
2) Update CreatePopup() to use the registered handler if new handler is registered. If no handler, CreatePopup() still uses current logic.
3) If platform wants to customize CreatePopup(), it can provide its library instance with library constructor only. Its library class is NULL, its library constructor() will consume RegisterCreatePopUpHandler() to register its handler.
This solution is compatible. It just updates UefiLib, doesn't need to update platform DSC files if the platforms don't enable the customized popup. It doesn't conflict with new UiLib design. New UiLib class introduce new APIs and update edk2 codes to use new one. For the driver uses old API, this way can provide the customization. For the driver uses new API, the different UiLib instances can provide the customization.
Thanks
Liming
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Felix Poludov
Sent: Thursday, November 10, 2016 9:38 PM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Subject: Re: [edk2] [PATCH 0/2] MdePkg: UiLib library
Hi Jiewen,
Updating "all the code that uses the old API to use the new API" is not always possible.
We can certainly update all the EDKII packages; however, it is trickier with the non-open source packages.
For example, the Reference Code we are getting from a chipset vendor is using CreatePopUp.
Certainly, if we mark CreatePopUp deprecated (as you have suggested), the chances are, reference code will get eventually updated not to use the function.
However, it might take a significant amount of time.
Moreover, a project integrators that create board firmware by combining open-source and close-source packages coming from multiple sources(from multiple vendors),
would have to solve this issue again and again; whereas with my proposal, the issue is solved by upgrading EDKII code base (no need to touch third party packages).
Having said that, I do understand your concerns with keeping the library in MdePkg.
How about finding a middle ground between your proposal and my proposal?
We can keep UiLib instance in MdePkg, but keep only a minimalistic barebones implementation there.
We can also have another, more advanced, UiLib instance in the MdeModulePkg.
This technique is already used in EDKII.
For example, MdePkg defines DebugLib library class and provides BaseDebugLibSerialPort instance.
MdeModulePkg provides PeiDxeDebugLibReportStatusCode instance for the same library class.
Since "advanced" version of Popup is not currently available, there is no need to create new instance in MdeModulePkg.
However, once Eric is ready with his Popup-on-top-of-HII implementation, it can be added to a new UiLib instance in MdeModulePkg.
Also, if new functions are added to UiLib in the future, MdePkg UiLib instance can have a NULL implementation like many other libraries in the MdePkg.
What do you think?
Thanks
Felix
From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
Sent: Wednesday, November 09, 2016 8:54 PM
To: Felix Poludov; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Subject: RE: [PATCH 0/2] MdePkg: UiLib library
HI Poludov
It is a good idea to have a new UiLib.
Both Mike and I give the suggestion to "You could define new lib and update all the code that uses the old API to use the new API."
IMHO, I do not think it is right way to let UefiLib depend on a UiLib.
We have plan to resolve it, but since you already have proposal to add UiLib. I hope it can be resolved here.
There is no real need to put UiLib into MdePkg. MdePkg is designed to hold EFI/PI speciation or industry standard.
In this case, I believe MdeModulePkg is a better place.
Thank you
Yao Jiewen
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Felix Poludov
> Sent: Wednesday, November 9, 2016 11:44 PM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Subject: [edk2] [PATCH 0/2] MdePkg: UiLib library
>
> This series of two patches introduces new UiLib library.
> 1. New UiLib library class is added to MdePkg.
> The library is intended for functions that produce UI elements.
> The patch introduces two functions that produce a popup window:
> UiCreatePopUp and UiCreatePopUpVaList.
> 2. An instance of UiLib library class is added to MdePkg.
> Popup implementation is copied from UefiLib
> (CreatePopUp function in UefiLib/Console.c).
> 3. MdePkg/UefiLib library is updated to implement CreatePopUp
> by calling UiCreatePopUpVaList.
> 4. Platform DSC files are updated to add UiLib instance.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Felix Polyudov
>
> AppPkg/AppPkg.dsc | 1 +
> ArmPkg/ArmPkg.dsc | 1 +
> BeagleBoardPkg/BeagleBoardPkg.dsc | 1 +
> CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 1 +
> CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 1 +
> CryptoPkg/CryptoPkg.dsc | 1 +
> DuetPkg/DuetPkgIa32.dsc | 1 +
> DuetPkg/DuetPkgX64.dsc | 1 +
> EdkCompatibilityPkg/EdkCompatibilityPkg.dsc | 1 +
> EmbeddedPkg/EmbeddedPkg.dsc | 1 +
> EmulatorPkg/EmulatorPkg.dsc | 1 +
> FatPkg/FatPkg.dsc | 1 +
> .../IntelFrameworkModulePkg.dsc | 1 +
> IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc | 1 +
> IntelFspWrapperPkg/IntelFspWrapperPkg.dsc | 1 +
> MdeModulePkg/MdeModulePkg.dsc | 1 +
> MdePkg/Include/Library/UiLib.h | 71 +++++
> MdePkg/Library/UefiLib/Console.c | 252
> +---------------
> MdePkg/Library/UefiLib/UefiLib.inf | 1 +
> MdePkg/Library/UefiLib/UefiLibInternal.h | 1 +
> MdePkg/Library/UiLib/UiLib.c | 334
> +++++++++++++++++++++
> MdePkg/Library/UiLib/UiLib.inf | 38 +++
> MdePkg/Library/UiLib/UiLib.uni | 21 ++
> MdePkg/MdePkg.dec | 3 +
> NetworkPkg/NetworkPkg.dsc | 1 +
> Nt32Pkg/Nt32Pkg.dsc | 1 +
> Omap35xxPkg/Omap35xxPkg.dsc | 1 +
> OptionRomPkg/OptionRomPkg.dsc | 1 +
> OvmfPkg/OvmfPkgIa32.dsc | 1 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> OvmfPkg/OvmfPkgX64.dsc | 1 +
> PcAtChipsetPkg/PcAtChipsetPkg.dsc | 1 +
> PerformancePkg/PerformancePkg.dsc | 1 +
> QuarkPlatformPkg/Quark.dsc | 1 +
> QuarkPlatformPkg/QuarkMin.dsc | 1 +
> QuarkSocPkg/QuarkSocPkg.dsc | 1 +
> SecurityPkg/SecurityPkg.dsc | 1 +
> ShellPkg/ShellPkg.dsc | 1 +
> SourceLevelDebugPkg/SourceLevelDebugPkg.dsc | 1 +
> StdLib/StdLib.dsc | 1 +
> UefiCpuPkg/UefiCpuPkg.dsc | 1 +
> Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 1 +
> Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 1 +
> Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 1 +
> 44 files changed, 506 insertions(+), 251 deletions(-)
> create mode 100644 MdePkg/Include/Library/UiLib.h
> create mode 100644 MdePkg/Library/UiLib/UiLib.c
> create mode 100644 MdePkg/Library/UiLib/UiLib.inf
> create mode 100644 MdePkg/Library/UiLib/UiLib.uni
>
> Please consider the environment before printing this email.
>
> The information contained in this message may be confidential and
> proprietary to American Megatrends, Inc. This communication is intended
> to be read only by the individual or entity to whom it is addressed or by their
> designee. If the reader of this message is not the intended recipient, you are
> on notice that any distribution of this message, in any form, is strictly
> prohibited. Please promptly notify the sender by reply e-mail or by
> telephone at 770-246-8600, and then delete or destroy all copies of the
> transmission.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
Please consider the environment before printing this email.
The information contained in this message may be confidential and proprietary to American Megatrends, Inc. This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
Please consider the environment before printing this email.
The information contained in this message may be confidential and proprietary to American Megatrends, Inc. This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] MdePkg: UiLib library
2016-11-10 15:44 ` Felix Poludov
@ 2016-11-10 16:02 ` Gao, Liming
2016-11-10 16:04 ` Kinney, Michael D
1 sibling, 0 replies; 9+ messages in thread
From: Gao, Liming @ 2016-11-10 16:02 UTC (permalink / raw)
To: Felix Poludov, Yao, Jiewen, edk2-devel@lists.01.org
Yes. Quick way is to search CreatePopUp in build output map file. If the driver map includes this function, this driver will require to link NULL library instance. That means RegisterCreatePopUpHandler() and CreatePopUp() are both required in map file. You can develop one script to detect every map file to make sure they are both included or not. This script can be called at post build. If so, the developer can easily find any missing.
Thanks
Liming
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Felix Poludov
Sent: Thursday, November 10, 2016 11:44 PM
To: Gao, Liming <liming.gao@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH 0/2] MdePkg: UiLib library
Liming,
With your proposal if a projects uses RegisterCreatePopUpHandler(), DSC file has to be updated to link popup handler NULL library
with all the modules that consume CreatePopup().
Since projects tend to evolve (new modules added, existing code changes), this sounds like a constant maintenance effort.
Project integrator would have to scan source tree after each commit to check if a new code that uses CretePopup has been added and if new DSC file modifications are required.
Thanks
Felix
From: Gao, Liming [mailto:liming.gao@intel.com]
Sent: Thursday, November 10, 2016 10:10 AM
To: Felix Poludov; Yao, Jiewen; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Subject: RE: [PATCH 0/2] MdePkg: UiLib library
Felix:
I think two major concerns are here.
1) UefiLib depends on UiLib
2) UiLib class is defined in MdePkg.
To resolve it, I have another idea.
1) Introduce new API RegisterCreatePopUpHandler() in UefiLib. Its input parameter is the handler function that is same to UiCreatePopUpVaList().
2) Update CreatePopup() to use the registered handler if new handler is registered. If no handler, CreatePopup() still uses current logic.
3) If platform wants to customize CreatePopup(), it can provide its library instance with library constructor only. Its library class is NULL, its library constructor() will consume RegisterCreatePopUpHandler() to register its handler.
This solution is compatible. It just updates UefiLib, doesn't need to update platform DSC files if the platforms don't enable the customized popup. It doesn't conflict with new UiLib design. New UiLib class introduce new APIs and update edk2 codes to use new one. For the driver uses old API, this way can provide the customization. For the driver uses new API, the different UiLib instances can provide the customization.
Thanks
Liming
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Felix Poludov
Sent: Thursday, November 10, 2016 9:38 PM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Subject: Re: [edk2] [PATCH 0/2] MdePkg: UiLib library
Hi Jiewen,
Updating "all the code that uses the old API to use the new API" is not always possible.
We can certainly update all the EDKII packages; however, it is trickier with the non-open source packages.
For example, the Reference Code we are getting from a chipset vendor is using CreatePopUp.
Certainly, if we mark CreatePopUp deprecated (as you have suggested), the chances are, reference code will get eventually updated not to use the function.
However, it might take a significant amount of time.
Moreover, a project integrators that create board firmware by combining open-source and close-source packages coming from multiple sources(from multiple vendors),
would have to solve this issue again and again; whereas with my proposal, the issue is solved by upgrading EDKII code base (no need to touch third party packages).
Having said that, I do understand your concerns with keeping the library in MdePkg.
How about finding a middle ground between your proposal and my proposal?
We can keep UiLib instance in MdePkg, but keep only a minimalistic barebones implementation there.
We can also have another, more advanced, UiLib instance in the MdeModulePkg.
This technique is already used in EDKII.
For example, MdePkg defines DebugLib library class and provides BaseDebugLibSerialPort instance.
MdeModulePkg provides PeiDxeDebugLibReportStatusCode instance for the same library class.
Since "advanced" version of Popup is not currently available, there is no need to create new instance in MdeModulePkg.
However, once Eric is ready with his Popup-on-top-of-HII implementation, it can be added to a new UiLib instance in MdeModulePkg.
Also, if new functions are added to UiLib in the future, MdePkg UiLib instance can have a NULL implementation like many other libraries in the MdePkg.
What do you think?
Thanks
Felix
From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
Sent: Wednesday, November 09, 2016 8:54 PM
To: Felix Poludov; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Subject: RE: [PATCH 0/2] MdePkg: UiLib library
HI Poludov
It is a good idea to have a new UiLib.
Both Mike and I give the suggestion to "You could define new lib and update all the code that uses the old API to use the new API."
IMHO, I do not think it is right way to let UefiLib depend on a UiLib.
We have plan to resolve it, but since you already have proposal to add UiLib. I hope it can be resolved here.
There is no real need to put UiLib into MdePkg. MdePkg is designed to hold EFI/PI speciation or industry standard.
In this case, I believe MdeModulePkg is a better place.
Thank you
Yao Jiewen
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Felix Poludov
> Sent: Wednesday, November 9, 2016 11:44 PM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Subject: [edk2] [PATCH 0/2] MdePkg: UiLib library
>
> This series of two patches introduces new UiLib library.
> 1. New UiLib library class is added to MdePkg.
> The library is intended for functions that produce UI elements.
> The patch introduces two functions that produce a popup window:
> UiCreatePopUp and UiCreatePopUpVaList.
> 2. An instance of UiLib library class is added to MdePkg.
> Popup implementation is copied from UefiLib
> (CreatePopUp function in UefiLib/Console.c).
> 3. MdePkg/UefiLib library is updated to implement CreatePopUp
> by calling UiCreatePopUpVaList.
> 4. Platform DSC files are updated to add UiLib instance.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Felix Polyudov
>
> AppPkg/AppPkg.dsc | 1 +
> ArmPkg/ArmPkg.dsc | 1 +
> BeagleBoardPkg/BeagleBoardPkg.dsc | 1 +
> CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 1 +
> CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 1 +
> CryptoPkg/CryptoPkg.dsc | 1 +
> DuetPkg/DuetPkgIa32.dsc | 1 +
> DuetPkg/DuetPkgX64.dsc | 1 +
> EdkCompatibilityPkg/EdkCompatibilityPkg.dsc | 1 +
> EmbeddedPkg/EmbeddedPkg.dsc | 1 +
> EmulatorPkg/EmulatorPkg.dsc | 1 +
> FatPkg/FatPkg.dsc | 1 +
> .../IntelFrameworkModulePkg.dsc | 1 +
> IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc | 1 +
> IntelFspWrapperPkg/IntelFspWrapperPkg.dsc | 1 +
> MdeModulePkg/MdeModulePkg.dsc | 1 +
> MdePkg/Include/Library/UiLib.h | 71 +++++
> MdePkg/Library/UefiLib/Console.c | 252
> +---------------
> MdePkg/Library/UefiLib/UefiLib.inf | 1 +
> MdePkg/Library/UefiLib/UefiLibInternal.h | 1 +
> MdePkg/Library/UiLib/UiLib.c | 334
> +++++++++++++++++++++
> MdePkg/Library/UiLib/UiLib.inf | 38 +++
> MdePkg/Library/UiLib/UiLib.uni | 21 ++
> MdePkg/MdePkg.dec | 3 +
> NetworkPkg/NetworkPkg.dsc | 1 +
> Nt32Pkg/Nt32Pkg.dsc | 1 +
> Omap35xxPkg/Omap35xxPkg.dsc | 1 +
> OptionRomPkg/OptionRomPkg.dsc | 1 +
> OvmfPkg/OvmfPkgIa32.dsc | 1 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> OvmfPkg/OvmfPkgX64.dsc | 1 +
> PcAtChipsetPkg/PcAtChipsetPkg.dsc | 1 +
> PerformancePkg/PerformancePkg.dsc | 1 +
> QuarkPlatformPkg/Quark.dsc | 1 +
> QuarkPlatformPkg/QuarkMin.dsc | 1 +
> QuarkSocPkg/QuarkSocPkg.dsc | 1 +
> SecurityPkg/SecurityPkg.dsc | 1 +
> ShellPkg/ShellPkg.dsc | 1 +
> SourceLevelDebugPkg/SourceLevelDebugPkg.dsc | 1 +
> StdLib/StdLib.dsc | 1 +
> UefiCpuPkg/UefiCpuPkg.dsc | 1 +
> Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 1 +
> Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 1 +
> Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 1 +
> 44 files changed, 506 insertions(+), 251 deletions(-)
> create mode 100644 MdePkg/Include/Library/UiLib.h
> create mode 100644 MdePkg/Library/UiLib/UiLib.c
> create mode 100644 MdePkg/Library/UiLib/UiLib.inf
> create mode 100644 MdePkg/Library/UiLib/UiLib.uni
>
> Please consider the environment before printing this email.
>
> The information contained in this message may be confidential and
> proprietary to American Megatrends, Inc. This communication is intended
> to be read only by the individual or entity to whom it is addressed or by their
> designee. If the reader of this message is not the intended recipient, you are
> on notice that any distribution of this message, in any form, is strictly
> prohibited. Please promptly notify the sender by reply e-mail or by
> telephone at 770-246-8600, and then delete or destroy all copies of the
> transmission.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
Please consider the environment before printing this email.
The information contained in this message may be confidential and proprietary to American Megatrends, Inc. This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
Please consider the environment before printing this email.
The information contained in this message may be confidential and proprietary to American Megatrends, Inc. This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] MdePkg: UiLib library
2016-11-10 15:44 ` Felix Poludov
2016-11-10 16:02 ` Gao, Liming
@ 2016-11-10 16:04 ` Kinney, Michael D
2016-11-10 16:11 ` Felix Poludov
1 sibling, 1 reply; 9+ messages in thread
From: Kinney, Michael D @ 2016-11-10 16:04 UTC (permalink / raw)
To: Felix Poludov, Gao, Liming, Yao, Jiewen, edk2-devel@lists.01.org,
Kinney, Michael D
Felix,
The CONSTRUCTOR for the UiLib could do the registration.
Mike
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Felix
> Poludov
> Sent: Thursday, November 10, 2016 7:44 AM
> To: Gao, Liming <liming.gao@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; edk2-
> devel@lists.01.org
> Subject: Re: [edk2] [PATCH 0/2] MdePkg: UiLib library
>
> Liming,
>
> With your proposal if a projects uses RegisterCreatePopUpHandler(), DSC file has
> to be updated to link popup handler NULL library
> with all the modules that consume CreatePopup().
> Since projects tend to evolve (new modules added, existing code changes), this
> sounds like a constant maintenance effort.
> Project integrator would have to scan source tree after each commit to check if a
> new code that uses CretePopup has been added and if new DSC file modifications
> are required.
>
> Thanks
> Felix
>
> From: Gao, Liming [mailto:liming.gao@intel.com]
> Sent: Thursday, November 10, 2016 10:10 AM
> To: Felix Poludov; Yao, Jiewen; edk2-devel@lists.01.org
> Subject: RE: [PATCH 0/2] MdePkg: UiLib library
>
> Felix:
> I think two major concerns are here.
>
> 1) UefiLib depends on UiLib
>
> 2) UiLib class is defined in MdePkg.
>
> To resolve it, I have another idea.
>
> 1) Introduce new API RegisterCreatePopUpHandler() in UefiLib. Its input
> parameter is the handler function that is same to UiCreatePopUpVaList().
>
> 2) Update CreatePopup() to use the registered handler if new handler is
> registered. If no handler, CreatePopup() still uses current logic.
>
> 3) If platform wants to customize CreatePopup(), it can provide its library
> instance with library constructor only. Its library class is NULL, its library
> constructor() will consume RegisterCreatePopUpHandler() to register its handler.
> This solution is compatible. It just updates UefiLib, doesn't need to update
> platform DSC files if the platforms don't enable the customized popup. It doesn't
> conflict with new UiLib design. New UiLib class introduce new APIs and update
> edk2 codes to use new one. For the driver uses old API, this way can provide the
> customization. For the driver uses new API, the different UiLib instances can
> provide the customization.
>
> Thanks
> Liming
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Felix
> Poludov
> Sent: Thursday, November 10, 2016 9:38 PM
> To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; edk2-
> devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Subject: Re: [edk2] [PATCH 0/2] MdePkg: UiLib library
>
> Hi Jiewen,
>
> Updating "all the code that uses the old API to use the new API" is not always
> possible.
> We can certainly update all the EDKII packages; however, it is trickier with the
> non-open source packages.
> For example, the Reference Code we are getting from a chipset vendor is using
> CreatePopUp.
> Certainly, if we mark CreatePopUp deprecated (as you have suggested), the chances
> are, reference code will get eventually updated not to use the function.
> However, it might take a significant amount of time.
> Moreover, a project integrators that create board firmware by combining open-
> source and close-source packages coming from multiple sources(from multiple
> vendors),
> would have to solve this issue again and again; whereas with my proposal, the
> issue is solved by upgrading EDKII code base (no need to touch third party
> packages).
>
> Having said that, I do understand your concerns with keeping the library in
> MdePkg.
> How about finding a middle ground between your proposal and my proposal?
> We can keep UiLib instance in MdePkg, but keep only a minimalistic barebones
> implementation there.
> We can also have another, more advanced, UiLib instance in the MdeModulePkg.
> This technique is already used in EDKII.
> For example, MdePkg defines DebugLib library class and provides
> BaseDebugLibSerialPort instance.
> MdeModulePkg provides PeiDxeDebugLibReportStatusCode instance for the same
> library class.
>
> Since "advanced" version of Popup is not currently available, there is no need to
> create new instance in MdeModulePkg.
> However, once Eric is ready with his Popup-on-top-of-HII implementation, it can
> be added to a new UiLib instance in MdeModulePkg.
> Also, if new functions are added to UiLib in the future, MdePkg UiLib instance
> can have a NULL implementation like many other libraries in the MdePkg.
>
> What do you think?
>
> Thanks
> Felix
>
> From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
> Sent: Wednesday, November 09, 2016 8:54 PM
> To: Felix Poludov; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Subject: RE: [PATCH 0/2] MdePkg: UiLib library
>
> HI Poludov
> It is a good idea to have a new UiLib.
>
> Both Mike and I give the suggestion to "You could define new lib and update all
> the code that uses the old API to use the new API."
>
> IMHO, I do not think it is right way to let UefiLib depend on a UiLib.
> We have plan to resolve it, but since you already have proposal to add UiLib. I
> hope it can be resolved here.
>
> There is no real need to put UiLib into MdePkg. MdePkg is designed to hold EFI/PI
> speciation or industry standard.
> In this case, I believe MdeModulePkg is a better place.
>
> Thank you
> Yao Jiewen
>
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Felix Poludov
> > Sent: Wednesday, November 9, 2016 11:44 PM
> > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > Subject: [edk2] [PATCH 0/2] MdePkg: UiLib library
> >
> > This series of two patches introduces new UiLib library.
> > 1. New UiLib library class is added to MdePkg.
> > The library is intended for functions that produce UI elements.
> > The patch introduces two functions that produce a popup window:
> > UiCreatePopUp and UiCreatePopUpVaList.
> > 2. An instance of UiLib library class is added to MdePkg.
> > Popup implementation is copied from UefiLib
> > (CreatePopUp function in UefiLib/Console.c).
> > 3. MdePkg/UefiLib library is updated to implement CreatePopUp
> > by calling UiCreatePopUpVaList.
> > 4. Platform DSC files are updated to add UiLib instance.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Felix Polyudov
> >
> > AppPkg/AppPkg.dsc | 1 +
> > ArmPkg/ArmPkg.dsc | 1 +
> > BeagleBoardPkg/BeagleBoardPkg.dsc | 1 +
> > CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 1 +
> > CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 1 +
> > CryptoPkg/CryptoPkg.dsc | 1 +
> > DuetPkg/DuetPkgIa32.dsc | 1 +
> > DuetPkg/DuetPkgX64.dsc | 1 +
> > EdkCompatibilityPkg/EdkCompatibilityPkg.dsc | 1 +
> > EmbeddedPkg/EmbeddedPkg.dsc | 1 +
> > EmulatorPkg/EmulatorPkg.dsc | 1 +
> > FatPkg/FatPkg.dsc | 1 +
> > .../IntelFrameworkModulePkg.dsc | 1 +
> > IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc | 1 +
> > IntelFspWrapperPkg/IntelFspWrapperPkg.dsc | 1 +
> > MdeModulePkg/MdeModulePkg.dsc | 1 +
> > MdePkg/Include/Library/UiLib.h | 71 +++++
> > MdePkg/Library/UefiLib/Console.c | 252
> > +---------------
> > MdePkg/Library/UefiLib/UefiLib.inf | 1 +
> > MdePkg/Library/UefiLib/UefiLibInternal.h | 1 +
> > MdePkg/Library/UiLib/UiLib.c | 334
> > +++++++++++++++++++++
> > MdePkg/Library/UiLib/UiLib.inf | 38 +++
> > MdePkg/Library/UiLib/UiLib.uni | 21 ++
> > MdePkg/MdePkg.dec | 3 +
> > NetworkPkg/NetworkPkg.dsc | 1 +
> > Nt32Pkg/Nt32Pkg.dsc | 1 +
> > Omap35xxPkg/Omap35xxPkg.dsc | 1 +
> > OptionRomPkg/OptionRomPkg.dsc | 1 +
> > OvmfPkg/OvmfPkgIa32.dsc | 1 +
> > OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> > OvmfPkg/OvmfPkgX64.dsc | 1 +
> > PcAtChipsetPkg/PcAtChipsetPkg.dsc | 1 +
> > PerformancePkg/PerformancePkg.dsc | 1 +
> > QuarkPlatformPkg/Quark.dsc | 1 +
> > QuarkPlatformPkg/QuarkMin.dsc | 1 +
> > QuarkSocPkg/QuarkSocPkg.dsc | 1 +
> > SecurityPkg/SecurityPkg.dsc | 1 +
> > ShellPkg/ShellPkg.dsc | 1 +
> > SourceLevelDebugPkg/SourceLevelDebugPkg.dsc | 1 +
> > StdLib/StdLib.dsc | 1 +
> > UefiCpuPkg/UefiCpuPkg.dsc | 1 +
> > Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 1 +
> > Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 1 +
> > Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 1 +
> > 44 files changed, 506 insertions(+), 251 deletions(-)
> > create mode 100644 MdePkg/Include/Library/UiLib.h
> > create mode 100644 MdePkg/Library/UiLib/UiLib.c
> > create mode 100644 MdePkg/Library/UiLib/UiLib.inf
> > create mode 100644 MdePkg/Library/UiLib/UiLib.uni
> >
> > Please consider the environment before printing this email.
> >
> > The information contained in this message may be confidential and
> > proprietary to American Megatrends, Inc. This communication is intended
> > to be read only by the individual or entity to whom it is addressed or by their
> > designee. If the reader of this message is not the intended recipient, you are
> > on notice that any distribution of this message, in any form, is strictly
> > prohibited. Please promptly notify the sender by reply e-mail or by
> > telephone at 770-246-8600, and then delete or destroy all copies of the
> > transmission.
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > https://lists.01.org/mailman/listinfo/edk2-devel
>
> Please consider the environment before printing this email.
>
> The information contained in this message may be confidential and proprietary to
> American Megatrends, Inc. This communication is intended to be read only by the
> individual or entity to whom it is addressed or by their designee. If the reader
> of this message is not the intended recipient, you are on notice that any
> distribution of this message, in any form, is strictly prohibited. Please
> promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and
> then delete or destroy all copies of the transmission.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
>
> Please consider the environment before printing this email.
>
> The information contained in this message may be confidential and proprietary to
> American Megatrends, Inc. This communication is intended to be read only by the
> individual or entity to whom it is addressed or by their designee. If the reader
> of this message is not the intended recipient, you are on notice that any
> distribution of this message, in any form, is strictly prohibited. Please
> promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and
> then delete or destroy all copies of the transmission.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] MdePkg: UiLib library
2016-11-10 16:04 ` Kinney, Michael D
@ 2016-11-10 16:11 ` Felix Poludov
2016-11-10 16:18 ` Gao, Liming
0 siblings, 1 reply; 9+ messages in thread
From: Felix Poludov @ 2016-11-10 16:11 UTC (permalink / raw)
To: Kinney, Michael D, Gao, Liming, Yao, Jiewen,
edk2-devel@lists.01.org
Yes, but if I understand Liming's proposal correctly, there will be no dependency between UefiLib and UiLib (UefiLib will not use UiLib).
It means that just adding a constructor to UiLib will be insufficient.
The DSC file will still have to be updated to link either UiLib or a NULL lib proposed by Liming with all the CreatePopup consumers.
-----Original Message-----
From: Kinney, Michael D [mailto:michael.d.kinney@intel.com]
Sent: Thursday, November 10, 2016 11:05 AM
To: Felix Poludov; Gao, Liming; Yao, Jiewen; edk2-devel@lists.01.org; Kinney, Michael D
Subject: RE: [PATCH 0/2] MdePkg: UiLib library
Felix,
The CONSTRUCTOR for the UiLib could do the registration.
Mike
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Felix Poludov
> Sent: Thursday, November 10, 2016 7:44 AM
> To: Gao, Liming <liming.gao@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; edk2- devel@lists.01.org
> Subject: Re: [edk2] [PATCH 0/2] MdePkg: UiLib library
>
> Liming,
>
> With your proposal if a projects uses RegisterCreatePopUpHandler(),
> DSC file has to be updated to link popup handler NULL library with all
> the modules that consume CreatePopup().
> Since projects tend to evolve (new modules added, existing code
> changes), this sounds like a constant maintenance effort.
> Project integrator would have to scan source tree after each commit to
> check if a new code that uses CretePopup has been added and if new DSC
> file modifications are required.
>
> Thanks
> Felix
>
> From: Gao, Liming [mailto:liming.gao@intel.com]
> Sent: Thursday, November 10, 2016 10:10 AM
> To: Felix Poludov; Yao, Jiewen; edk2-devel@lists.01.org
> Subject: RE: [PATCH 0/2] MdePkg: UiLib library
>
> Felix:
> I think two major concerns are here.
>
> 1) UefiLib depends on UiLib
>
> 2) UiLib class is defined in MdePkg.
>
> To resolve it, I have another idea.
>
> 1) Introduce new API RegisterCreatePopUpHandler() in UefiLib. Its input
> parameter is the handler function that is same to UiCreatePopUpVaList().
>
> 2) Update CreatePopup() to use the registered handler if new handler is
> registered. If no handler, CreatePopup() still uses current logic.
>
> 3) If platform wants to customize CreatePopup(), it can provide its library
> instance with library constructor only. Its library class is NULL, its
> library
> constructor() will consume RegisterCreatePopUpHandler() to register its handler.
> This solution is compatible. It just updates UefiLib, doesn't need
> to update platform DSC files if the platforms don't enable the
> customized popup. It doesn't conflict with new UiLib design. New UiLib
> class introduce new APIs and update
> edk2 codes to use new one. For the driver uses old API, this way can
> provide the customization. For the driver uses new API, the different
> UiLib instances can provide the customization.
>
> Thanks
> Liming
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Felix Poludov
> Sent: Thursday, November 10, 2016 9:38 PM
> To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>;
> edk2- devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Subject: Re: [edk2] [PATCH 0/2] MdePkg: UiLib library
>
> Hi Jiewen,
>
> Updating "all the code that uses the old API to use the new API" is
> not always possible.
> We can certainly update all the EDKII packages; however, it is
> trickier with the non-open source packages.
> For example, the Reference Code we are getting from a chipset vendor
> is using CreatePopUp.
> Certainly, if we mark CreatePopUp deprecated (as you have suggested),
> the chances are, reference code will get eventually updated not to use the function.
> However, it might take a significant amount of time.
> Moreover, a project integrators that create board firmware by
> combining open- source and close-source packages coming from multiple
> sources(from multiple vendors), would have to solve this issue again
> and again; whereas with my proposal, the issue is solved by upgrading
> EDKII code base (no need to touch third party packages).
>
> Having said that, I do understand your concerns with keeping the
> library in MdePkg.
> How about finding a middle ground between your proposal and my proposal?
> We can keep UiLib instance in MdePkg, but keep only a minimalistic
> barebones implementation there.
> We can also have another, more advanced, UiLib instance in the MdeModulePkg.
> This technique is already used in EDKII.
> For example, MdePkg defines DebugLib library class and provides
> BaseDebugLibSerialPort instance.
> MdeModulePkg provides PeiDxeDebugLibReportStatusCode instance for the
> same library class.
>
> Since "advanced" version of Popup is not currently available, there is
> no need to create new instance in MdeModulePkg.
> However, once Eric is ready with his Popup-on-top-of-HII
> implementation, it can be added to a new UiLib instance in MdeModulePkg.
> Also, if new functions are added to UiLib in the future, MdePkg UiLib
> instance can have a NULL implementation like many other libraries in the MdePkg.
>
> What do you think?
>
> Thanks
> Felix
>
> From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
> Sent: Wednesday, November 09, 2016 8:54 PM
> To: Felix Poludov;
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Subject: RE: [PATCH 0/2] MdePkg: UiLib library
>
> HI Poludov
> It is a good idea to have a new UiLib.
>
> Both Mike and I give the suggestion to "You could define new lib and
> update all the code that uses the old API to use the new API."
>
> IMHO, I do not think it is right way to let UefiLib depend on a UiLib.
> We have plan to resolve it, but since you already have proposal to add
> UiLib. I hope it can be resolved here.
>
> There is no real need to put UiLib into MdePkg. MdePkg is designed to
> hold EFI/PI speciation or industry standard.
> In this case, I believe MdeModulePkg is a better place.
>
> Thank you
> Yao Jiewen
>
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
> > Of Felix Poludov
> > Sent: Wednesday, November 9, 2016 11:44 PM
> > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > Subject: [edk2] [PATCH 0/2] MdePkg: UiLib library
> >
> > This series of two patches introduces new UiLib library.
> > 1. New UiLib library class is added to MdePkg.
> > The library is intended for functions that produce UI elements.
> > The patch introduces two functions that produce a popup window:
> > UiCreatePopUp and UiCreatePopUpVaList.
> > 2. An instance of UiLib library class is added to MdePkg.
> > Popup implementation is copied from UefiLib (CreatePopUp function in
> > UefiLib/Console.c).
> > 3. MdePkg/UefiLib library is updated to implement CreatePopUp by
> > calling UiCreatePopUpVaList.
> > 4. Platform DSC files are updated to add UiLib instance.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Felix Polyudov
> >
> > AppPkg/AppPkg.dsc | 1 +
> > ArmPkg/ArmPkg.dsc | 1 +
> > BeagleBoardPkg/BeagleBoardPkg.dsc | 1 +
> > CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 1 +
> > CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 1 +
> > CryptoPkg/CryptoPkg.dsc | 1 + DuetPkg/DuetPkgIa32.dsc | 1 +
> > DuetPkg/DuetPkgX64.dsc | 1 +
> > EdkCompatibilityPkg/EdkCompatibilityPkg.dsc | 1 +
> > EmbeddedPkg/EmbeddedPkg.dsc | 1 + EmulatorPkg/EmulatorPkg.dsc | 1 +
> > FatPkg/FatPkg.dsc | 1 + .../IntelFrameworkModulePkg.dsc | 1 +
> > IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc | 1 +
> > IntelFspWrapperPkg/IntelFspWrapperPkg.dsc | 1 +
> > MdeModulePkg/MdeModulePkg.dsc | 1 + MdePkg/Include/Library/UiLib.h |
> > 71 +++++ MdePkg/Library/UefiLib/Console.c | 252
> > +---------------
> > MdePkg/Library/UefiLib/UefiLib.inf | 1 +
> > MdePkg/Library/UefiLib/UefiLibInternal.h | 1 +
> > MdePkg/Library/UiLib/UiLib.c | 334
> > +++++++++++++++++++++
> > MdePkg/Library/UiLib/UiLib.inf | 38 +++
> > MdePkg/Library/UiLib/UiLib.uni | 21 ++ MdePkg/MdePkg.dec | 3 +
> > NetworkPkg/NetworkPkg.dsc | 1 + Nt32Pkg/Nt32Pkg.dsc | 1 +
> > Omap35xxPkg/Omap35xxPkg.dsc | 1 + OptionRomPkg/OptionRomPkg.dsc | 1
> > + OvmfPkg/OvmfPkgIa32.dsc | 1 + OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> > OvmfPkg/OvmfPkgX64.dsc | 1 + PcAtChipsetPkg/PcAtChipsetPkg.dsc | 1 +
> > PerformancePkg/PerformancePkg.dsc | 1 + QuarkPlatformPkg/Quark.dsc |
> > 1 + QuarkPlatformPkg/QuarkMin.dsc | 1 + QuarkSocPkg/QuarkSocPkg.dsc
> > | 1 + SecurityPkg/SecurityPkg.dsc | 1 + ShellPkg/ShellPkg.dsc | 1 +
> > SourceLevelDebugPkg/SourceLevelDebugPkg.dsc | 1 + StdLib/StdLib.dsc
> > | 1 + UefiCpuPkg/UefiCpuPkg.dsc | 1 +
> > Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 1 +
> > Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 1 +
> > Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 1 +
> > 44 files changed, 506 insertions(+), 251 deletions(-) create mode
> > 100644 MdePkg/Include/Library/UiLib.h create mode 100644
> > MdePkg/Library/UiLib/UiLib.c create mode 100644
> > MdePkg/Library/UiLib/UiLib.inf create mode 100644
> > MdePkg/Library/UiLib/UiLib.uni
> >
> > Please consider the environment before printing this email.
> >
> > The information contained in this message may be confidential and
> > proprietary to American Megatrends, Inc. This communication is
> > intended to be read only by the individual or entity to whom it is
> > addressed or by their designee. If the reader of this message is not
> > the intended recipient, you are on notice that any distribution of
> > this message, in any form, is strictly prohibited. Please promptly
> > notify the sender by reply e-mail or by telephone at 770-246-8600,
> > and then delete or destroy all copies of the transmission.
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > https://lists.01.org/mailman/listinfo/edk2-devel
>
> Please consider the environment before printing this email.
>
> The information contained in this message may be confidential and
> proprietary to American Megatrends, Inc. This communication is
> intended to be read only by the individual or entity to whom it is
> addressed or by their designee. If the reader of this message is not
> the intended recipient, you are on notice that any distribution of
> this message, in any form, is strictly prohibited. Please promptly
> notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
>
> Please consider the environment before printing this email.
>
> The information contained in this message may be confidential and
> proprietary to American Megatrends, Inc. This communication is
> intended to be read only by the individual or entity to whom it is
> addressed or by their designee. If the reader of this message is not
> the intended recipient, you are on notice that any distribution of
> this message, in any form, is strictly prohibited. Please promptly
> notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
Please consider the environment before printing this email.
The information contained in this message may be confidential and proprietary to American Megatrends, Inc. This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] MdePkg: UiLib library
2016-11-10 16:11 ` Felix Poludov
@ 2016-11-10 16:18 ` Gao, Liming
0 siblings, 0 replies; 9+ messages in thread
From: Gao, Liming @ 2016-11-10 16:18 UTC (permalink / raw)
To: Felix Poludov, Kinney, Michael D, Yao, Jiewen,
edk2-devel@lists.01.org
Right. To customize CreatePopup(), new library instance and platform dsc are both updated. Not to customize Createpopup(), there is nothing to do.
From: Felix Poludov [mailto:Felixp@ami.com]
Sent: Friday, November 11, 2016 12:11 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
Subject: RE: [PATCH 0/2] MdePkg: UiLib library
Yes, but if I understand Liming's proposal correctly, there will be no dependency between UefiLib and UiLib (UefiLib will not use UiLib).
It means that just adding a constructor to UiLib will be insufficient.
The DSC file will still have to be updated to link either UiLib or a NULL lib proposed by Liming with all the CreatePopup consumers.
-----Original Message-----
From: Kinney, Michael D [mailto:michael.d.kinney@intel.com]
Sent: Thursday, November 10, 2016 11:05 AM
To: Felix Poludov; Gao, Liming; Yao, Jiewen; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Kinney, Michael D
Subject: RE: [PATCH 0/2] MdePkg: UiLib library
Felix,
The CONSTRUCTOR for the UiLib could do the registration.
Mike
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Felix Poludov
> Sent: Thursday, November 10, 2016 7:44 AM
> To: Gao, Liming ; Yao, Jiewen
> ; edk2- devel@lists.01.org<mailto:devel@lists.01.org>
> Subject: Re: [edk2] [PATCH 0/2] MdePkg: UiLib library
>
> Liming,
>
> With your proposal if a projects uses RegisterCreatePopUpHandler(),
> DSC file has to be updated to link popup handler NULL library with all
> the modules that consume CreatePopup().
> Since projects tend to evolve (new modules added, existing code
> changes), this sounds like a constant maintenance effort.
> Project integrator would have to scan source tree after each commit to
> check if a new code that uses CretePopup has been added and if new DSC
> file modifications are required.
>
> Thanks
> Felix
>
> From: Gao, Liming [mailto:liming.gao@intel.com]
> Sent: Thursday, November 10, 2016 10:10 AM
> To: Felix Poludov; Yao, Jiewen; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Subject: RE: [PATCH 0/2] MdePkg: UiLib library
>
> Felix:
> I think two major concerns are here.
>
> 1) UefiLib depends on UiLib
>
> 2) UiLib class is defined in MdePkg.
>
> To resolve it, I have another idea.
>
> 1) Introduce new API RegisterCreatePopUpHandler() in UefiLib. Its input
> parameter is the handler function that is same to UiCreatePopUpVaList().
>
> 2) Update CreatePopup() to use the registered handler if new handler is
> registered. If no handler, CreatePopup() still uses current logic.
>
> 3) If platform wants to customize CreatePopup(), it can provide its library
> instance with library constructor only. Its library class is NULL, its
> library
> constructor() will consume RegisterCreatePopUpHandler() to register its handler.
> This solution is compatible. It just updates UefiLib, doesn't need
> to update platform DSC files if the platforms don't enable the
> customized popup. It doesn't conflict with new UiLib design. New UiLib
> class introduce new APIs and update
> edk2 codes to use new one. For the driver uses old API, this way can
> provide the customization. For the driver uses new API, the different
> UiLib instances can provide the customization.
>
> Thanks
> Liming
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Felix Poludov
> Sent: Thursday, November 10, 2016 9:38 PM
> To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>;
> edk2- devel@lists.01.org<mailto:devel@lists.01.org>
> Subject: Re: [edk2] [PATCH 0/2] MdePkg: UiLib library
>
> Hi Jiewen,
>
> Updating "all the code that uses the old API to use the new API" is
> not always possible.
> We can certainly update all the EDKII packages; however, it is
> trickier with the non-open source packages.
> For example, the Reference Code we are getting from a chipset vendor
> is using CreatePopUp.
> Certainly, if we mark CreatePopUp deprecated (as you have suggested),
> the chances are, reference code will get eventually updated not to use the function.
> However, it might take a significant amount of time.
> Moreover, a project integrators that create board firmware by
> combining open- source and close-source packages coming from multiple
> sources(from multiple vendors), would have to solve this issue again
> and again; whereas with my proposal, the issue is solved by upgrading
> EDKII code base (no need to touch third party packages).
>
> Having said that, I do understand your concerns with keeping the
> library in MdePkg.
> How about finding a middle ground between your proposal and my proposal?
> We can keep UiLib instance in MdePkg, but keep only a minimalistic
> barebones implementation there.
> We can also have another, more advanced, UiLib instance in the MdeModulePkg.
> This technique is already used in EDKII.
> For example, MdePkg defines DebugLib library class and provides
> BaseDebugLibSerialPort instance.
> MdeModulePkg provides PeiDxeDebugLibReportStatusCode instance for the
> same library class.
>
> Since "advanced" version of Popup is not currently available, there is
> no need to create new instance in MdeModulePkg.
> However, once Eric is ready with his Popup-on-top-of-HII
> implementation, it can be added to a new UiLib instance in MdeModulePkg.
> Also, if new functions are added to UiLib in the future, MdePkg UiLib
> instance can have a NULL implementation like many other libraries in the MdePkg.
>
> What do you think?
>
> Thanks
> Felix
>
> From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
> Sent: Wednesday, November 09, 2016 8:54 PM
> To: Felix Poludov;
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Subject: RE: [PATCH 0/2] MdePkg: UiLib library
>
> HI Poludov
> It is a good idea to have a new UiLib.
>
> Both Mike and I give the suggestion to "You could define new lib and
> update all the code that uses the old API to use the new API."
>
> IMHO, I do not think it is right way to let UefiLib depend on a UiLib.
> We have plan to resolve it, but since you already have proposal to add
> UiLib. I hope it can be resolved here.
>
> There is no real need to put UiLib into MdePkg. MdePkg is designed to
> hold EFI/PI speciation or industry standard.
> In this case, I believe MdeModulePkg is a better place.
>
> Thank you
> Yao Jiewen
>
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
> > Of Felix Poludov
> > Sent: Wednesday, November 9, 2016 11:44 PM
> > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > Subject: [edk2] [PATCH 0/2] MdePkg: UiLib library
> >
> > This series of two patches introduces new UiLib library.
> > 1. New UiLib library class is added to MdePkg.
> > The library is intended for functions that produce UI elements.
> > The patch introduces two functions that produce a popup window:
> > UiCreatePopUp and UiCreatePopUpVaList.
> > 2. An instance of UiLib library class is added to MdePkg.
> > Popup implementation is copied from UefiLib (CreatePopUp function in
> > UefiLib/Console.c).
> > 3. MdePkg/UefiLib library is updated to implement CreatePopUp by
> > calling UiCreatePopUpVaList.
> > 4. Platform DSC files are updated to add UiLib instance.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Felix Polyudov
> >
> > AppPkg/AppPkg.dsc | 1 +
> > ArmPkg/ArmPkg.dsc | 1 +
> > BeagleBoardPkg/BeagleBoardPkg.dsc | 1 +
> > CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 1 +
> > CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 1 +
> > CryptoPkg/CryptoPkg.dsc | 1 + DuetPkg/DuetPkgIa32.dsc | 1 +
> > DuetPkg/DuetPkgX64.dsc | 1 +
> > EdkCompatibilityPkg/EdkCompatibilityPkg.dsc | 1 +
> > EmbeddedPkg/EmbeddedPkg.dsc | 1 + EmulatorPkg/EmulatorPkg.dsc | 1 +
> > FatPkg/FatPkg.dsc | 1 + .../IntelFrameworkModulePkg.dsc | 1 +
> > IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc | 1 +
> > IntelFspWrapperPkg/IntelFspWrapperPkg.dsc | 1 +
> > MdeModulePkg/MdeModulePkg.dsc | 1 + MdePkg/Include/Library/UiLib.h |
> > 71 +++++ MdePkg/Library/UefiLib/Console.c | 252
> > +---------------
> > MdePkg/Library/UefiLib/UefiLib.inf | 1 +
> > MdePkg/Library/UefiLib/UefiLibInternal.h | 1 +
> > MdePkg/Library/UiLib/UiLib.c | 334
> > +++++++++++++++++++++
> > MdePkg/Library/UiLib/UiLib.inf | 38 +++
> > MdePkg/Library/UiLib/UiLib.uni | 21 ++ MdePkg/MdePkg.dec | 3 +
> > NetworkPkg/NetworkPkg.dsc | 1 + Nt32Pkg/Nt32Pkg.dsc | 1 +
> > Omap35xxPkg/Omap35xxPkg.dsc | 1 + OptionRomPkg/OptionRomPkg.dsc | 1
> > + OvmfPkg/OvmfPkgIa32.dsc | 1 + OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> > OvmfPkg/OvmfPkgX64.dsc | 1 + PcAtChipsetPkg/PcAtChipsetPkg.dsc | 1 +
> > PerformancePkg/PerformancePkg.dsc | 1 + QuarkPlatformPkg/Quark.dsc |
> > 1 + QuarkPlatformPkg/QuarkMin.dsc | 1 + QuarkSocPkg/QuarkSocPkg.dsc
> > | 1 + SecurityPkg/SecurityPkg.dsc | 1 + ShellPkg/ShellPkg.dsc | 1 +
> > SourceLevelDebugPkg/SourceLevelDebugPkg.dsc | 1 + StdLib/StdLib.dsc
> > | 1 + UefiCpuPkg/UefiCpuPkg.dsc | 1 +
> > Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 1 +
> > Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 1 +
> > Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 1 +
> > 44 files changed, 506 insertions(+), 251 deletions(-) create mode
> > 100644 MdePkg/Include/Library/UiLib.h create mode 100644
> > MdePkg/Library/UiLib/UiLib.c create mode 100644
> > MdePkg/Library/UiLib/UiLib.inf create mode 100644
> > MdePkg/Library/UiLib/UiLib.uni
> >
> > Please consider the environment before printing this email.
> >
> > The information contained in this message may be confidential and
> > proprietary to American Megatrends, Inc. This communication is
> > intended to be read only by the individual or entity to whom it is
> > addressed or by their designee. If the reader of this message is not
> > the intended recipient, you are on notice that any distribution of
> > this message, in any form, is strictly prohibited. Please promptly
> > notify the sender by reply e-mail or by telephone at 770-246-8600,
> > and then delete or destroy all copies of the transmission.
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > https://lists.01.org/mailman/listinfo/edk2-devel
>
> Please consider the environment before printing this email.
>
> The information contained in this message may be confidential and
> proprietary to American Megatrends, Inc. This communication is
> intended to be read only by the individual or entity to whom it is
> addressed or by their designee. If the reader of this message is not
> the intended recipient, you are on notice that any distribution of
> this message, in any form, is strictly prohibited. Please promptly
> notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
>
> Please consider the environment before printing this email.
>
> The information contained in this message may be confidential and
> proprietary to American Megatrends, Inc. This communication is
> intended to be read only by the individual or entity to whom it is
> addressed or by their designee. If the reader of this message is not
> the intended recipient, you are on notice that any distribution of
> this message, in any form, is strictly prohibited. Please promptly
> notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
Please consider the environment before printing this email.
The information contained in this message may be confidential and proprietary to American Megatrends, Inc. This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-11-10 16:18 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-09 15:43 [PATCH 0/2] MdePkg: UiLib library Felix Poludov
2016-11-10 1:53 ` Yao, Jiewen
2016-11-10 13:37 ` Felix Poludov
2016-11-10 15:10 ` Gao, Liming
2016-11-10 15:44 ` Felix Poludov
2016-11-10 16:02 ` Gao, Liming
2016-11-10 16:04 ` Kinney, Michael D
2016-11-10 16:11 ` Felix Poludov
2016-11-10 16:18 ` Gao, Liming
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox