On 2015-08-17 19:42:04, Ni, Ruiyu wrote:
> 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.

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?

> 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.

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
  );

> 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.

Maybe a better name for your new driver is PeiHobGopDxe?

My point was that with the video mode setting lib, we could probably
produce a GOP driver for most video devices just by customizing the
two libraries. In that case, I think the GraphicsOutputDxe name makes
sense.

-Jordan

> -----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>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to