Hello Andy,
I've got a question about your edk2-platforms commit 9df63499ea01 (i.e.,
this patch):
On 08/30/19 17:27, Leif Lindholm wrote:
> On Mon, Aug 19, 2019 at 02:32:00PM +0100, Andy Hayes wrote:
[...]
>> diff --git a/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Edid.c
>> b/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Edid.c
[...]
>> +EFI_STATUS
>> +DlReadEdid (
>> + IN USB_DISPLAYLINK_DEV* UsbDisplayLinkDev
>> + )
>> +{
[...]
>> + Status = EdidOverride->GetEdid (
>> + EdidOverride,
>> + &UsbDisplayLinkDev->Handle,
>> + &EdidAttributes,
>> + &EdidDataSize,
>> + &EdidDataBlock);
In UEFI v2.8, EFI_EDID_OVERRIDE_PROTOCOL_GET_EDID's "ChildHandle"
parameter is specified as follows, in *natural language*:
ChildHandle -- A child handle that represents a possible video output
device.
Accordingly, the function *prototype* should specify the parameter as:
IN EFI_HANDLE ChildHandle
However, the spec in fact mandates:
IN EFI_HANDLE *ChildHandle
there.
This is most likely a typo in the prototype, and should be fixed (so
that the prototype match the natural language description). I've
reported the following spec ticket about it:
https://mantis.uefi.org/mantis/view.php?id=2018
Here's the problem.
It looks like most GetEdid() calls in existence conform to the *spirit*
of the spec, and not to the *letter* thereof; in other words, they pass
an EFI_HANDLE for ChildHandle. Furthermore, the
EFI_EDID_OVERRIDE_PROTOCOL implementations underneath also conform to
the spirit, and not the letter of the spec, and they consume ChildHandle
as an EFI_HANDLE. (They do not de-reference ChildHandle for getting the
actual handle.)
In other words, fixing the typo in the spec would simply adopt existing
practice, and no code would have to be changed for spec conformance.
Except... the code quoted above.
The code above conforms to the *letter* of the spec, and passes an
(EFI_HANDLE*), as ChildHandle. If the typo is fixed in the spec, the
call site in DlReadEdid() would have to be modified.
Can you please confirm wheter:
- the EdidOverride->GetEdid() call is reached in practice (= it is not
dead code),
- the underlying EFI_EDID_OVERRIDE_PROTOCOL actually consumes
ChildHandle (for which it first has to de-reference it)?
My hope is that either the above call site is dead code en-bloc, or the
underlying protocol implementation ignores the ChildHandle parameter.
Then we can fix both the spec and the DlReadEdid() function.
Thanks!
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#48487): https://edk2.groups.io/g/devel/message/48487
Mute This Topic: https://groups.io/mt/33080721/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
