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
);
EFI_STATUS
EFIAPI
BltFreeConfiguration (
IN EFI_HANDLE Configuration
);
On 2015/8/24 6:28, Jordan Justen wrote:
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?
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.
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.
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
EFI_HANDLE but directly use VOID * because EFI_HANDLE is a concept used
in DXE Core protocol/handle database. See below
SimpleTextInEx::RegisterKeystrokeNotify() also uses VOID * as NotifyHandle.
typedef
EFI_STATUS
(EFIAPI *EFI_REGISTER_KEYSTROKE_NOTIFY) (
IN EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL *This,
IN EFI_KEY_DATA *KeyData,
IN EFI_KEY_NOTIFY_FUNCTION KeyNotificationFunction,
OUT VOID **NotifyHandle
);
The benefit of using a handle is it can avoid the shift/mask being
re-calculated every time someone calling the library BLT functions.
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.
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?
-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
