Re: [edk2-devel] [External] Re: [PATCH v1 1/1] Drivers/DisplayLink/DisplayLinkPkg DisplayLinkGop
That's right, the only (current) request was index 0 - that is why it didn't show up. It was a refactoring error. It was picked up when we ported some of the changes back into our "closed source" version of the driver and the unit tests failed. Thanks for pushing this. From: Leif Lindholm Sent: 17 September 2019 16:28 To: Andy Hayes Cc: devel@edk2.groups.io; Ard Biesheuvel Subject: [External] Re: [PATCH v1 1/1] Drivers/DisplayLink/DisplayLinkPkg DisplayLinkGop On Wed, Sep 11, 2019 at 07:42:03AM +0000, Andy Hayes wrote: > Corrected initialisation of one of data structures used to transmit USB > control messages. Mistake had no practical effects but fixing to be on safe > side. So, was the only request used index 0? Or why didn't this cause an issue? Nevertheless, a clear fix. > Cc: Leif Lindholm mailto:leif.lindh...@linaro.org>> > Cc: Ard Biesheuvel > mailto:ard.biesheu...@linaro.org>> > Signed-off-by: Andy Hayes > mailto:andy.ha...@displaylink.com>> Reviewed-by: Leif Lindholm mailto:leif.lindh...@linaro.org>> Pushed as 958aaf600728. / Leif > --- > Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c > b/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c > index 252293da39d4..9871ab0378ce 100644 > --- a/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c > +++ b/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c > @@ -107,7 +107,7 @@ DlUsbSendControlWriteMessage ( > UINT32 UsbStatus; > EFI_USB_DEVICE_REQUEST UsbRequest; > > - ZeroMem (, sizeof (Request)); > + ZeroMem (, sizeof (UsbRequest)); > UsbRequest.RequestType = USB_REQ_TYPE_VENDOR | USB_TARGET_INTERFACE; > UsbRequest.Index = Device->InterfaceDescriptor.InterfaceNumber; > UsbRequest.Request = Request; > -- > 1.8.3.1<http://1.8.3.1> > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47382): https://edk2.groups.io/g/devel/message/47382 Mute This Topic: https://groups.io/mt/34177235/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v1 0/1] DisplayLink GOP driver USB msg initialisation
Corrected initialisation of one of data structures used to transmit USB control messages. Mistake had no practical effects but fixing to be on safe side. https://github.com/andy-hayes/edk2-platforms Cc: Leif Lindholm Cc: Ard Biesheuvel Andy Hayes (1): Drivers/DisplayLink/DisplayLinkPkg DisplayLinkGop Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 1.8.3.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47111): https://edk2.groups.io/g/devel/message/47111 Mute This Topic: https://groups.io/mt/34101032/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v1 1/1] Drivers/DisplayLink/DisplayLinkPkg DisplayLinkGop
Corrected initialisation of one of data structures used to transmit USB control messages. Mistake had no practical effects but fixing to be on safe side. Cc: Leif Lindholm Cc: Ard Biesheuvel Signed-off-by: Andy Hayes --- Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c b/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c index 252293da39d4..9871ab0378ce 100644 --- a/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c +++ b/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c @@ -107,7 +107,7 @@ DlUsbSendControlWriteMessage ( UINT32 UsbStatus; EFI_USB_DEVICE_REQUEST UsbRequest; - ZeroMem (, sizeof (Request)); + ZeroMem (, sizeof (UsbRequest)); UsbRequest.RequestType = USB_REQ_TYPE_VENDOR | USB_TARGET_INTERFACE; UsbRequest.Index = Device->InterfaceDescriptor.InterfaceNumber; UsbRequest.Request = Request; -- 1.8.3.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47110): https://edk2.groups.io/g/devel/message/47110 Mute This Topic: https://groups.io/mt/34092678/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [edk2-platforms] [PATCH v1 0/1] DisplayLink GOP driver USB msg initialisation
Corrected initialisation of one of data structures used to transmit USB control messages. Mistake had no practical effects but fixing to be on safe side. https://github.com/andy-hayes/edk2-platforms Cc: Leif Lindholm Cc: Ard Biesheuvel Andy Hayes (1): Drivers/DisplayLink/DisplayLinkPkg DisplayLinkGop Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 1.8.3.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47076): https://edk2.groups.io/g/devel/message/47076 Mute This Topic: https://groups.io/mt/34092677/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v1 1/1] Drivers/DisplayLink/DisplayLinkPkg DisplayLinkGop
Corrected initialisation of one of data structures used to transmit USB control messages. Mistake had no practical effects but fixing to be on safe side. Cc: Leif Lindholm Cc: Ard Biesheuvel Signed-off-by: Andy Hayes --- Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c b/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c index 252293da39d4..9871ab0378ce 100644 --- a/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c +++ b/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c @@ -107,7 +107,7 @@ DlUsbSendControlWriteMessage ( UINT32 UsbStatus; EFI_USB_DEVICE_REQUEST UsbRequest; - ZeroMem (, sizeof (Request)); + ZeroMem (, sizeof (UsbRequest)); UsbRequest.RequestType = USB_REQ_TYPE_VENDOR | USB_TARGET_INTERFACE; UsbRequest.Index = Device->InterfaceDescriptor.InterfaceNumber; UsbRequest.Request = Request; -- 1.8.3.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47077): https://edk2.groups.io/g/devel/message/47077 Mute This Topic: https://groups.io/mt/34092678/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 0/1] Added GOP driver for DisplayLink-based Universal USB Docks to edk2-platforms
On Thu, Aug 15, 2019 at 10:15:48AM +, Andy Hayes wrote: > Thanks very much for the very quick feedback from both of you - I > really appreciate it. I'll fix the issues that you pointed out and > re-submit the patch. Thanks! > I've been testing the build with VisualStudio 2015 and GCC 7.3. (VS > for X64, GCC5 for IA32/X64/ARM/AARCH64) I had a bit of trouble > getting the clang build to work but I'll try it again to make sure > that I don't introduce any other errors. Yeah, the compilers (especially GCC/clang) are getting pickier My baseline today is Clang 7 and GCC 8.3. Testing with Clang 8 and GCC9 is definitely becoming relevant, but could in most cases replace testing with GCC 8. > The way that the C structures are initialized - what other compilers > need to be tested to make sure there is sufficient support for this > syntax? (I believe the syntax is a C99 thing). Well, in this case, the "problem" is that the compiler generates calls to "known to be there" libc helper functions. Which aren't for us. On the IA32/X64 side, these rules have been put in place. On the ARM/AARCH64 side, we have ArmPkg/Library/CompilerIntrinsicsLib, which on (32-bit) ARM was a fundamental necessity for other reasons. So testing with a particular compiler version may or may not trigger the issue, and seemingly unrelated future code changes may nudge the compiler into doing something else instead. So it's not exactly foolproof. > I know that there is a big list in tools.def, but realistically > which ones do you have to be sure work correctly, to cover the > majority of users? It's easy enough to cover all of the versions of > GCC and CLANG for, say, X86/X64/ARM/AARCH64, but how far back with > Visual Studio versions do I need to go? I don't expect everyone to test everything - only to test more than one option and fix things as reported. Unless it's core code. It *would* be perfetcly valid (though less than ideal) to have external device drivers not supported by the full set of toolchains. Of course, making your code build with multiple toolchains frequently flush out latent bugs, so... In this case, the changes were all pretty trivial, so I would prefer to see them implemented. We have some cleanup work to do on the cross-compilation aspect in general for !VS. While it can be nudged into working, it works differently on ARM and X64. Personally, I worry mostly about compatibility with the toolchain versions included in the latest stable version of Debian (since that's the distribution people tend to complain about lagging behind :). From Debian Buster, we finally have cross-compilation symmetry AARCH64<->X64. For something like UDK, we *should* probably worry about all versions included in all LTS Linux distro releases, but... > > I am curious how this driver interacts with the ConSplitter when > > displays are hot added and hot removed. I see a reference to a > > feature that appears to sync the GOP content between frame buffers > > when a display is hot added. I would be better if the common code in > > the ConSplitter handled these types of operations instead of putting > > that code in individual GOP drivers. I have added Ray to this thread > > who may have ideas on how to handle these hot add events. > > I think the code that you are looking at that syncs GOP content > might be a debug feature, rather than a hotplug handler. We found > that some platforms only render to the first GOP device that they > find, so I added the ability to build a version of the driver (using > the build flag COPY_PIXELS_FROM_PRIMARY_GOP_DEVICE) that > "steals" the pixels from another GOP framebuffer and displays them > on our output, to check that our driver fundamentally works on that > platform. This code is excluded in a normal build. Most platforms > are well-behaved and render to all GOP devices correctly. Best Regards, Leif > In general we've found that hotplug of our device works fine, > without us having to do anything special in the driver. Note that > the system will initially see this as a USB hotplug rather than a > display hotplug. For various reasons we decided not to support > display hotplug (i.e. plugging and unplugging displays into out > device once our device is plugged in to USB and initialized) in the > driver, mainly to keep things simple. > > Thanks again for your help, > > Andy Hayes > > From: Kinney, Michael D > mailto:michael.d.kin...@intel.com>> > Sent: 14 August 2019 16:23 > To: Andy Hayes > mailto:andy.ha...@displaylink.com>>; > devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray > mailto:ray...@intel.com>>; Kinney, Michael D > mailto:michael.d.kin...@intel.com>> > Cc: Leif Lindholm mailto:leif.lindh...@linaro.org>> > Subject
