Hi Andy, Thanks for the contribution. Is this patch for the edk2-platform repo? The email subject can help clarify the intended repo.
Also, we prefer the patches to be provided inline instead of an attachment. You can use the 'git send-email' feature to do this. Here are a couple links to useful wikis. https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers I also noticed looking at the patch file that there are some inconsistencies with the EDK II C Coding Standards. Most are around function headers and indents. Here is a link to the latest version: https://www.gitbook.com/download/pdf/book/edk2-docs/edk-ii-c-coding-standards-specification I see a few #if 0 and commented out lines of code and TODO comments. Can you please clean these up and if needed add Tianocore Bugzillas for future enhancements/work items for this driver. https://bugzilla.tianocore.org/ I also noticed that you are using named fields to initialize C structures: STATIC CONST struct VideoMode ModeData[] = { + { + // 640 x 480 @ 60Hz + .Reserved2 = 2, + .PixelClock = 2517, + .HActive = 640, + .HBlanking = 160, + .HSyncOffset = 16, + .HSyncWidth = 96, I do like this style because it has better self documentation of the field being initialized to a value. However, I do not see this style in the rest of the EDK II sources, so I am concerned that we may have required compilers for the EDK II that may not support this syntax. If we find all the supported compilers do support this syntax, this would be a great addition to the EDK II C Coding Standards. 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. Thanks, Mike From: Andy Hayes [mailto:andy.ha...@displaylink.com] Sent: Wednesday, August 14, 2019 7:44 AM To: devel@edk2.groups.io Cc: Leif Lindholm <leif.lindh...@linaro.org>; Kinney, Michael D <michael.d.kin...@intel.com> Subject: [PATCH v1 0/1] Added GOP driver for DisplayLink-based Universal USB Docks to edk2-platforms >From 4a42eb997aeb3699217b40bf3bc47dec56458847 Mon Sep 17 00:00:00 2001 From: "Andy Hayes" < andy.ha...@displaylink.com<mailto:andy.ha...@displaylink.com> > Date: Wed, 14 Aug 2019 15:30:20 +0100 Subject: [PATCH v1 0/1] Added GOP graphics driver for DisplayLink-based Universal USB Docks to edk2-platforms andy.ha...@displaylink.com<mailto:andy.ha...@displaylink.com> (1): Added GOP driver for USB Docks which use DisplayLink chips. .../DisplayLinkPkg/DisplayLinkPkg.dsc | 61 + .../DisplayLinkGop/DisplayLinkGopDxe.inf | 63 + .../DisplayLinkPkg/DisplayLinkGop/Edid.h | 129 ++ .../DisplayLinkGop/UsbDescriptors.h | 109 ++ .../DisplayLinkGop/UsbDisplayLink.h | 284 +++++ .../DisplayLinkGop/CapabilityDescriptor.c | 137 ++ .../DisplayLinkGop/ComponentName.c | 235 ++++ .../DisplayLinkPkg/DisplayLinkGop/Edid.c | 598 +++++++++ .../DisplayLinkPkg/DisplayLinkGop/Gop.c | 587 +++++++++ .../DisplayLinkGop/UsbDescriptors.c | 144 +++ .../DisplayLinkGop/UsbDisplayLink.c | 1109 +++++++++++++++++ .../DisplayLinkGop/UsbTransfer.c | 180 +++ .../DisplayLinkGop/VideoModes.c | 254 ++++ Drivers/DisplayLink/DisplayLinkPkg/ReadMe.md | 77 ++ Maintainers.txt | 5 + 15 files changed, 3972 insertions(+) create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkPkg.dsc create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/DisplayLinkGopDxe.inf create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Edid.h create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDescriptors.h create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDisplayLink.h create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/CapabilityDescriptor.c create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/ComponentName.c create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Edid.c create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Gop.c create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDescriptors.c create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDisplayLink.c create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/VideoModes.c create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/ReadMe.md -- 2.17.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#45616): https://edk2.groups.io/g/devel/message/45616 Mute This Topic: https://groups.io/mt/32864415/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
