Re: [edk2] [Patch 6/8] OptionRomPkg/OvmfPkg: Remove BltLib::BltConfigure API

2015-08-26 Thread Laszlo Ersek 
On 08/24/15 08:15, Ni, Ruiyu wrote:
 Jordan,
 The mail is too long so let me summarize in below: (I changed all
 API name to remove Lib)
 1. Have four or six BLT APIs?
   four: BltVideoFill BltVideoToVideo BltVideoToBuffer BltBufferToVideo
   six:  BltVideoFill BltVideoToVideo BltVideoToBuffer BltBufferToVideo
 BltVideoToBufferEx BltBufferToVideoEx
   I prefer 4 because it aligns to the GOP protocol 4 BLT operations
   and the parameters are also aligned.
   You prefer 6 for easy calling.
 
 Laszlo and Mike,
   any ideas?

No particular preference on my part.

Thanks
Laszlo

 
 
 
 2. BltConfigure/BltFreeConfiguration
2.a. I agree to use the handle to hold the shift/mask. But I need to
 see whether there is any concerns from Mike.
2.b. I suggest to use VOID * instead of EFI_HANDLE. Let's wait for
 Andrew/Mike's input.
 
 3. VideoModeSetLib for more abstraction
I agree to either change the driver name as you suggested, either in
the other way to have a more generic GOP driver. Do you know any
graphics driver developers? Can you ask them if the more generic GOP
driver will satisfy their needs?
 
 Thanks,
 Ray

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch 6/8] OptionRomPkg/OvmfPkg: Remove BltLib::BltConfigure API

2015-08-23 Thread Jordan Justen 
On 2015-08-23 20:09:23, Ni, Ruiyu wrote:
 Jordan,
 reply embedded in the mail.
 
 Mike,
 Jordan proposed to add a new parameter Configuration in BltConfigure, 
 instead of removing this API, to eliminate the global variables in 
 library implementation. I think it's also a good solution and can 
 benefit the library performance. What's your thoughts?
 EFI_STATUS
 EFIAPI
 BltConfigure (
 IN  VOID *FrameBuffer,
 IN  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *FrameBufferInfo,
 OUT EFI_HANDLE   *Configuration
 );

I also had considered:

EFI_STATUS
EFIAPI
BltLibConfigureGop (
IN  EFI_GRAPHICS_OUTPUT_PROTOCOL *Gop,
OUT EFI_HANDLE   *Configuration
);

Which would either configure based on the current GOP settings, or in
the case of GopBltLib, it could just save the pointer to the GOP
protocol and call the protocol Blt function.

 EFI_STATUS
 EFIAPI
 BltFreeConfiguration (
 IN  EFI_HANDLE  Configuration
 );
 
 
 On 2015/8/24 6:28, Jordan Justen wrote:
  Previously you said to Laszlo in regard to keeping the BltLibVideoFill
  function: 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.
 
  This is exactly why I think the current BltLibVideoToBltBuffer and
  BltLibBufferToVideo are useful. They have less parameters, and should
  be useful in a case where the buffer is sized exactly to the blit
  operation.
 
  It is also very easy to implement in the library by just calling the
  other 'Ex' functions.
 
  If you just want to avoid overlapping functions, then why not just
  keep *only* the BltLibGopBlt function?
 
 True we can choose *only keep the BltLibGopBlt function, instead of
 keeping the current four functions. We can assume the interfaces of
 GOP protocol defined in spec were carefully reviewed, then, why not
 just align to these interfaces. It's a little bit confusing when the
 protocol implementation of BltVideoToBltBuffer operation calls a
 BltVideoToBltBuffer*Ex* library API. After all for the additional
 parameters, caller can simply pass 0.

I was not serious about the suggestion to just have the single blit
function. My preference would be to leave all of the current
functions.

One of my goals for BltLib was that it would make it easier for code
to use the blit operations. Think of it similar to HobLib. The Build*
functions in that library make it easier to create HOBs.

But, I thought the BltLibGopBlt function was nice for a GOP driver to
use, so I added it to the library interface.

I do agree that BltLibConfigure was not the greatest design. I wanted
to make it easier to wrapper a GOP protocol as I mentioned above.

If we only have a single blit function, then it makes GopBltLib
pointless, whereas I was hoping that the BltLib interface could make
calling the GOP protocol a little easier and with more readable code.

  Q2: If removing BltConfigure, does that mean the library will have
  to check to see if the mode is different? what's the benefit?
 
  After removing BltConfigure(), the library will calculate the
  shift/mask every time when doing the BLT operations. The benefit is
  to make the library stateless so it doesn't contain any state
  information so it can be called from different threads/CPUs.
 
  I don't think the GOP protocol blit is reentrant, so I'm still not
  sure about where the requirement comes from.
 
 A GOP implementation can choose to utilize all the processors to fill 
 the frame buffer simultaneously for good performance. I don't know 
 whether there is any such GOP MP implementation but we'd better not to 
 restrict it from the API interface level.

Hmm. I'm not convinced it is necessary. Do you know who originally
proposed that this would be a requirement?

I guess it would be nice for a multi-screen/framebuffer situation to
not have to call BltLibConfigure to blit to all of the screens.

If we add the hidden 'Configuration' struct mentioned above, I guess
it seems reasonable.

  But, I did have another idea. What if BltLibConfigure has an output
  EFI_HANDLE, and each BltLib function has this handle as it's first
  parameter. Then we only need 1 parameter to each function, and it can
  encapsulate any data needed for that configuration. For example, it
  can still calculate the shifts and masks just once.
 
  Since the returned handle would likely be an allocated structure, I
  think we should also then add a BltLibFreeConfiguration.
 
  In other words:
 
  EFI_STATUS
  EFIAPI
  BltLibConfigure (
 IN  VOID *FrameBuffer,
 IN  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *FrameBufferInfo,
 OUT EFI_HANDLE   *Configuration
 );
 
  EFI_STATUS
  EFIAPI
  BltLibFreeConfiguration (
 IN  EFI_HANDLE  Configuration
 );
 
 I agree with your idea, with just a little modification. Do not use

Re: [edk2] [Patch 6/8] OptionRomPkg/OvmfPkg: Remove BltLib::BltConfigure API

2015-08-17 Thread Ni, Ruiyu 
Jordan,
Sorry I missed your previous questions. Let me try to answer all your questions 
in this email.
Q1: Why merging Ex and non-Ex APIs? Providing Non-Ex APIs was to make simpler 
functions available for blt operations.
Merging the Ex and non-EX APIs is to avoid the library APIs provide overlapped 
functionalities. And it aligns to the interfaces defined in GOP protocol.

Q2: If removing BltConfigure, does that mean the library will have to check to 
see if the mode is different? what's the benefit?
After removing BltConfigure(), the library will calculate the shift/mask every 
time when doing the BLT operations. The benefit is to make the library 
stateless so it doesn't contain any state information so it can be called from 
different threads/CPUs.
For example strtok_r() is the stateless and reentrant version of strtok().
char *strtok(char *str, const char *delim);
char *strtok_r(char *str, const char *delim, char **saveptr);

Q3: What if we design a VideoModeSetLib?
GraphicsOutputDxe + null_VideoModeSetLib + BltLib - current GraphicsOutputDxe
GraphicsOutputDxe + qemu_VideoModeSetLib + BltLib - current QemuVideoDxe

Null_VideoModeSetLib only allows the video in the fixed mode supplied from PEI 
HOB
Qemu_VideoModeSetLib allows the video in several modes for QEMU.

Is my understanding correct?
Do you have more usage scenarios about the VideoModeSetLib? For now I only see 
two and only two, considering the design complexity introduced by 
VideoModeSetLib, leave the QemuVideoDxe driver as is make people easier to 
understand.

Thanks,
Ray

-Original Message-
From: Justen, Jordan L 
Sent: Tuesday, August 18, 2015 1:18 AM
To: Ni, Ruiyu ruiyu...@intel.com; edk2-devel@lists.01.org
Cc: Ni, Ruiyu ruiyu...@intel.com; Laszlo Ersek ler...@redhat.com
Subject: Re: [Patch 6/8] OptionRomPkg/OvmfPkg: Remove BltLib::BltConfigure API

On 2015-08-17 06:45:27, Ruiyu Ni wrote:
 The BltConfigure() API caches the video frame buffer meta data which
 forbids the library to be implemented to support re-entry.

How does this help? GraphicsOutputDxe will set a mode, and then use
BltLib with that mode.

I already asked this, and I had some other questions:

http://article.gmane.org/gmane.comp.bios.edk2.devel/1209

-Jordan

 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Ruiyu Ni ruiyu...@intel.com
 Cc: Laszlo Ersek ler...@redhat.com
 Cc: Jordan Justen jordan.l.jus...@intel.com
 ---
  .../Application/BltLibSample/BltLibSample.c| 129 +++---
  OptionRomPkg/Include/Library/BltLib.h  | 151 +++---
  .../Library/FrameBufferBltLib/FrameBufferBltLib.c  | 514 
 +++--
  OptionRomPkg/Library/GopBltLib/GopBltLib.c | 289 ++--
  OvmfPkg/QemuVideoDxe/Gop.c |  13 +-
  5 files changed, 576 insertions(+), 520 deletions(-)
 
 diff --git a/OptionRomPkg/Application/BltLibSample/BltLibSample.c 
 b/OptionRomPkg/Application/BltLibSample/BltLibSample.c
 index 333b054..3c7e392 100644
 --- a/OptionRomPkg/Application/BltLibSample/BltLibSample.c
 +++ b/OptionRomPkg/Application/BltLibSample/BltLibSample.c
 @@ -18,6 +18,7 @@
  #include Library/UefiLib.h
  #include Library/UefiApplicationEntryPoint.h
  #include Library/UefiBootServicesTableLib.h
 +#include Library/MemoryAllocationLib.h
  
  
  UINT64
 @@ -68,8 +69,8 @@ Rand32 (
  
  VOID
  TestFills (
 -  UINT32 HorizontalResolution,
 -  UINT32 VerticalResolution
 +  IN  VOID *FrameBuffer,
 +  IN  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *FrameBufferInfo
)
  {
EFI_GRAPHICS_OUTPUT_BLT_PIXEL  Color;
 @@ -79,44 +80,45 @@ TestFills (
UINTN  W;
UINTN  H;
  
 -  for (Loop = 0; Loop  1; Loop++) {
 -W = HorizontalResolution - (Rand32 () % HorizontalResolution);
 -H = VerticalResolution - (Rand32 () % VerticalResolution);
 -if (W != HorizontalResolution) {
 -  X = Rand32 () % (HorizontalResolution - W);
 +  for (Loop = 0; Loop  1000; Loop++) {
 +W = FrameBufferInfo-HorizontalResolution - (Rand32 () % 
 FrameBufferInfo-HorizontalResolution);
 +H = FrameBufferInfo-VerticalResolution - (Rand32 () % 
 FrameBufferInfo-VerticalResolution);
 +if (W != FrameBufferInfo-HorizontalResolution) {
 +  X = Rand32 () % (FrameBufferInfo-HorizontalResolution - W);
  } else {
X = 0;
  }
 -if (H != VerticalResolution) {
 -  Y = Rand32 () % (VerticalResolution - H);
 +if (H != FrameBufferInfo-VerticalResolution) {
 +  Y = Rand32 () % (FrameBufferInfo-VerticalResolution - H);
  } else {
Y = 0;
  }
  *(UINT32*) (Color) = Rand32 ()  0xff;
 -BltVideoFill (Color, X, Y, W, H);
 +BltVideoFill (FrameBuffer, FrameBufferInfo, Color, X, Y, W, H);
}
  }
  
  
  VOID
  TestColor1 (
 -  UINT32 HorizontalResolution,
 -  UINT32 VerticalResolution
 +  IN  VOID

[edk2] [Patch 6/8] OptionRomPkg/OvmfPkg: Remove BltLib::BltConfigure API

2015-08-17 Thread Ruiyu Ni 
The BltConfigure() API caches the video frame buffer meta data which
forbids the library to be implemented to support re-entry.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni ruiyu...@intel.com
Cc: Laszlo Ersek ler...@redhat.com
Cc: Jordan Justen jordan.l.jus...@intel.com
---
 .../Application/BltLibSample/BltLibSample.c| 129 +++---
 OptionRomPkg/Include/Library/BltLib.h  | 151 +++---
 .../Library/FrameBufferBltLib/FrameBufferBltLib.c  | 514 +++--
 OptionRomPkg/Library/GopBltLib/GopBltLib.c | 289 ++--
 OvmfPkg/QemuVideoDxe/Gop.c |  13 +-
 5 files changed, 576 insertions(+), 520 deletions(-)

diff --git a/OptionRomPkg/Application/BltLibSample/BltLibSample.c 
b/OptionRomPkg/Application/BltLibSample/BltLibSample.c
index 333b054..3c7e392 100644
--- a/OptionRomPkg/Application/BltLibSample/BltLibSample.c
+++ b/OptionRomPkg/Application/BltLibSample/BltLibSample.c
@@ -18,6 +18,7 @@
 #include Library/UefiLib.h
 #include Library/UefiApplicationEntryPoint.h
 #include Library/UefiBootServicesTableLib.h
+#include Library/MemoryAllocationLib.h
 
 
 UINT64
@@ -68,8 +69,8 @@ Rand32 (
 
 VOID
 TestFills (
-  UINT32 HorizontalResolution,
-  UINT32 VerticalResolution
+  IN  VOID *FrameBuffer,
+  IN  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *FrameBufferInfo
   )
 {
   EFI_GRAPHICS_OUTPUT_BLT_PIXEL  Color;
@@ -79,44 +80,45 @@ TestFills (
   UINTN  W;
   UINTN  H;
 
-  for (Loop = 0; Loop  1; Loop++) {
-W = HorizontalResolution - (Rand32 () % HorizontalResolution);
-H = VerticalResolution - (Rand32 () % VerticalResolution);
-if (W != HorizontalResolution) {
-  X = Rand32 () % (HorizontalResolution - W);
+  for (Loop = 0; Loop  1000; Loop++) {
+W = FrameBufferInfo-HorizontalResolution - (Rand32 () % 
FrameBufferInfo-HorizontalResolution);
+H = FrameBufferInfo-VerticalResolution - (Rand32 () % 
FrameBufferInfo-VerticalResolution);
+if (W != FrameBufferInfo-HorizontalResolution) {
+  X = Rand32 () % (FrameBufferInfo-HorizontalResolution - W);
 } else {
   X = 0;
 }
-if (H != VerticalResolution) {
-  Y = Rand32 () % (VerticalResolution - H);
+if (H != FrameBufferInfo-VerticalResolution) {
+  Y = Rand32 () % (FrameBufferInfo-VerticalResolution - H);
 } else {
   Y = 0;
 }
 *(UINT32*) (Color) = Rand32 ()  0xff;
-BltVideoFill (Color, X, Y, W, H);
+BltVideoFill (FrameBuffer, FrameBufferInfo, Color, X, Y, W, H);
   }
 }
 
 
 VOID
 TestColor1 (
-  UINT32 HorizontalResolution,
-  UINT32 VerticalResolution
+  IN  VOID *FrameBuffer,
+  IN  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *FrameBufferInfo
   )
 {
-  EFI_GRAPHICS_OUTPUT_BLT_PIXEL  Color;
+  EFI_GRAPHICS_OUTPUT_BLT_PIXEL  *BltBuffer;
   UINTN  X;
   UINTN  Y;
 
-  *(UINT32*) (Color) = 0;
+  BltBuffer = AllocatePool (sizeof (*BltBuffer) * 
FrameBufferInfo-HorizontalResolution);
+  ASSERT (BltBuffer != NULL);
 
-  for (Y = 0; Y  VerticalResolution; Y++) {
-for (X = 0; X  HorizontalResolution; X++) {
-  Color.Red =   (UINT8) ((X * 0x100) / HorizontalResolution);
-  Color.Green = (UINT8) ((Y * 0x100) / VerticalResolution);
-  Color.Blue =  (UINT8) ((Y * 0x100) / VerticalResolution);
-  BltVideoFill (Color, X, Y, 1, 1);
+  for (Y = 0; Y  FrameBufferInfo-VerticalResolution; Y++) {
+for (X = 0; X  FrameBufferInfo-HorizontalResolution; X++) {
+  BltBuffer[X].Red =   (UINT8) ((X * 0x100) / 
FrameBufferInfo-HorizontalResolution);
+  BltBuffer[X].Green = (UINT8) ((Y * 0x100) / 
FrameBufferInfo-VerticalResolution);
+  BltBuffer[X].Blue =  (UINT8) ((Y * 0x100) / 
FrameBufferInfo-VerticalResolution);
 }
+BltBufferToVideo (FrameBuffer, FrameBufferInfo, BltBuffer, 0, 0, 0, Y, 
FrameBufferInfo-HorizontalResolution, 1, 0);
   }
 }
 
@@ -176,8 +178,8 @@ GetTriColor (
 
 VOID
 TestColor (
-  UINT32 HorizontalResolution,
-  UINT32 VerticalResolution
+  IN  VOID *FrameBuffer,
+  IN  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *FrameBufferInfo
   )
 {
   EFI_GRAPHICS_OUTPUT_BLT_PIXEL  Color;
@@ -187,34 +189,38 @@ TestColor (
   UINTN  LineWidth, TriWidth;
   UINTN  TriHeight;
   UINT32 ColorDist;
+  EFI_GRAPHICS_OUTPUT_BLT_PIXEL  *BltBuffer;
 
   *(UINT32*) (Color) = 0;
-  BltVideoFill (Color, 0, 0, HorizontalResolution, VerticalResolution);
+  BltVideoFill (FrameBuffer, FrameBufferInfo, Color, 0, 0, 
FrameBufferInfo-HorizontalResolution, FrameBufferInfo-VerticalResolution);
 
   TriWidth = (UINTN) DivU64x32 (
-   MultU64x32 (11547005, (UINT32)

Re: [edk2] [Patch 6/8] OptionRomPkg/OvmfPkg: Remove BltLib::BltConfigure API

2015-08-17 Thread Jordan Justen 
On 2015-08-17 06:45:27, Ruiyu Ni wrote:
 The BltConfigure() API caches the video frame buffer meta data which
 forbids the library to be implemented to support re-entry.

How does this help? GraphicsOutputDxe will set a mode, and then use
BltLib with that mode.

I already asked this, and I had some other questions:

http://article.gmane.org/gmane.comp.bios.edk2.devel/1209

-Jordan

 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Ruiyu Ni ruiyu...@intel.com
 Cc: Laszlo Ersek ler...@redhat.com
 Cc: Jordan Justen jordan.l.jus...@intel.com
 ---
  .../Application/BltLibSample/BltLibSample.c| 129 +++---
  OptionRomPkg/Include/Library/BltLib.h  | 151 +++---
  .../Library/FrameBufferBltLib/FrameBufferBltLib.c  | 514 
 +++--
  OptionRomPkg/Library/GopBltLib/GopBltLib.c | 289 ++--
  OvmfPkg/QemuVideoDxe/Gop.c |  13 +-
  5 files changed, 576 insertions(+), 520 deletions(-)
 
 diff --git a/OptionRomPkg/Application/BltLibSample/BltLibSample.c 
 b/OptionRomPkg/Application/BltLibSample/BltLibSample.c
 index 333b054..3c7e392 100644
 --- a/OptionRomPkg/Application/BltLibSample/BltLibSample.c
 +++ b/OptionRomPkg/Application/BltLibSample/BltLibSample.c
 @@ -18,6 +18,7 @@
  #include Library/UefiLib.h
  #include Library/UefiApplicationEntryPoint.h
  #include Library/UefiBootServicesTableLib.h
 +#include Library/MemoryAllocationLib.h
  
  
  UINT64
 @@ -68,8 +69,8 @@ Rand32 (
  
  VOID
  TestFills (
 -  UINT32 HorizontalResolution,
 -  UINT32 VerticalResolution
 +  IN  VOID *FrameBuffer,
 +  IN  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *FrameBufferInfo
)
  {
EFI_GRAPHICS_OUTPUT_BLT_PIXEL  Color;
 @@ -79,44 +80,45 @@ TestFills (
UINTN  W;
UINTN  H;
  
 -  for (Loop = 0; Loop  1; Loop++) {
 -W = HorizontalResolution - (Rand32 () % HorizontalResolution);
 -H = VerticalResolution - (Rand32 () % VerticalResolution);
 -if (W != HorizontalResolution) {
 -  X = Rand32 () % (HorizontalResolution - W);
 +  for (Loop = 0; Loop  1000; Loop++) {
 +W = FrameBufferInfo-HorizontalResolution - (Rand32 () % 
 FrameBufferInfo-HorizontalResolution);
 +H = FrameBufferInfo-VerticalResolution - (Rand32 () % 
 FrameBufferInfo-VerticalResolution);
 +if (W != FrameBufferInfo-HorizontalResolution) {
 +  X = Rand32 () % (FrameBufferInfo-HorizontalResolution - W);
  } else {
X = 0;
  }
 -if (H != VerticalResolution) {
 -  Y = Rand32 () % (VerticalResolution - H);
 +if (H != FrameBufferInfo-VerticalResolution) {
 +  Y = Rand32 () % (FrameBufferInfo-VerticalResolution - H);
  } else {
Y = 0;
  }
  *(UINT32*) (Color) = Rand32 ()  0xff;
 -BltVideoFill (Color, X, Y, W, H);
 +BltVideoFill (FrameBuffer, FrameBufferInfo, Color, X, Y, W, H);
}
  }
  
  
  VOID
  TestColor1 (
 -  UINT32 HorizontalResolution,
 -  UINT32 VerticalResolution
 +  IN  VOID *FrameBuffer,
 +  IN  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *FrameBufferInfo
)
  {
 -  EFI_GRAPHICS_OUTPUT_BLT_PIXEL  Color;
 +  EFI_GRAPHICS_OUTPUT_BLT_PIXEL  *BltBuffer;
UINTN  X;
UINTN  Y;
  
 -  *(UINT32*) (Color) = 0;
 +  BltBuffer = AllocatePool (sizeof (*BltBuffer) * 
 FrameBufferInfo-HorizontalResolution);
 +  ASSERT (BltBuffer != NULL);
  
 -  for (Y = 0; Y  VerticalResolution; Y++) {
 -for (X = 0; X  HorizontalResolution; X++) {
 -  Color.Red =   (UINT8) ((X * 0x100) / HorizontalResolution);
 -  Color.Green = (UINT8) ((Y * 0x100) / VerticalResolution);
 -  Color.Blue =  (UINT8) ((Y * 0x100) / VerticalResolution);
 -  BltVideoFill (Color, X, Y, 1, 1);
 +  for (Y = 0; Y  FrameBufferInfo-VerticalResolution; Y++) {
 +for (X = 0; X  FrameBufferInfo-HorizontalResolution; X++) {
 +  BltBuffer[X].Red =   (UINT8) ((X * 0x100) / 
 FrameBufferInfo-HorizontalResolution);
 +  BltBuffer[X].Green = (UINT8) ((Y * 0x100) / 
 FrameBufferInfo-VerticalResolution);
 +  BltBuffer[X].Blue =  (UINT8) ((Y * 0x100) / 
 FrameBufferInfo-VerticalResolution);
  }
 +BltBufferToVideo (FrameBuffer, FrameBufferInfo, BltBuffer, 0, 0, 0, Y, 
 FrameBufferInfo-HorizontalResolution, 1, 0);
}
  }
  
 @@ -176,8 +178,8 @@ GetTriColor (
  
  VOID
  TestColor (
 -  UINT32 HorizontalResolution,
 -  UINT32 VerticalResolution
 +  IN  VOID *FrameBuffer,
 +  IN  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *FrameBufferInfo
)
  {
EFI_GRAPHICS_OUTPUT_BLT_PIXEL  Color;
 @@ -187,34 +189,38 @@ TestColor (
UINTN  LineWidth, TriWidth;
UINTN  TriHeight;
UINT32 ColorDist;
 +