On Wed, Feb 26, 2014 at 1:59 PM, Laszlo Ersek <ler...@redhat.com> wrote: > On 02/26/14 20:16, Jordan Justen wrote: >> Commonly a platform will have a PlatformDxe driver. It has kind of >> surprised me how long OVMF has lived without one. One of the main >> things that lived in the PlatformDxe driver is the setup form data. >> >> So, rather than PlatformConfigDxe, could you add PlatformDxe? > > As long as this idea covers only the renaming / relocating of the > driver, sure. But, in that case, would you mind if I squashed the > relevant patches? Their cumulative diffstat is now > > OvmfPkg/OvmfPkgIa32.dsc | 2 + > OvmfPkg/OvmfPkgIa32.fdf | 1 + > OvmfPkg/OvmfPkgIa32X64.dsc | 2 + > OvmfPkg/OvmfPkgIa32X64.fdf | 1 + > OvmfPkg/OvmfPkgX64.dsc | 2 + > OvmfPkg/OvmfPkgX64.fdf | 1 + > OvmfPkg/PlatformConfigDxe/PlatformConfig.c | 730 ++++++++++++++++++ > OvmfPkg/PlatformConfigDxe/PlatformConfig.h | 42 + > OvmfPkg/PlatformConfigDxe/PlatformConfig.inf | 59 ++ > OvmfPkg/PlatformConfigDxe/PlatformConfig.uni | Bin 0 -> 3322 bytes > .../PlatformConfigDxe/PlatformConfigForms.vfr | 74 ++ > 11 files changed, 914 insertions(+) > > Basically it stays completely in PlatformConfigDxe. > > I'm asking because threading a rename through such a rebase is a big > pain. Too bad I'd lose the nice (I hope!) structure of these patches, > ie. the progress and the thought process. Perhaps if I re-enable rename > detection temporarily in my repo, it could work.
I think that would work. (Reenabling rename detection.) I don't have that rename detection disabled, and a quick git rebase test seemed to show it should work. >> PlatformDxe can also look at the setup variable and set the resolution >> PCDs. (Rather than PlatformBds.) >> >> At that point, I think we could (at least for now) fold >> PlatformConfigLib into PlatformDxe. > > I think this would not work. Consider the following relative dependencies: > > (1) the resolution preference must be read from the NvVar, and the PCDs > must be set for GraphicsConsoleDxe, before the consoles are connected. > Patch v2 04/19 does this at the last minute, just before > PlatformBdsConnectConsole(). > > PCDs --> PlatformBdsConnectConsole() > > (2) The platfom config driver installs its HII stuff in its entry point > (it must, it is a device-independent driver). The contents of the > drop-down list is part of the HII stuff. In order to populate the > drop-down list with the selectable resolutions, it needs the GOP, so > there's a Depex on the GOP. PlatformDxe should not have a depex on GOP, but it could have a GOP callback. For a more complicated version, I think the setup data could be published very early, and the form could be updated once GOP is available. I think the form could even have a callback into PlatformDxe when the form is displayed allowing resolution population at that time. We should also make sure that -vga none does not cause major breakage. -Jordan > The GOP interface is installed in the video > driver's binding start function. > > video binding start --> platform config entry point > > (3) The platform driver can't do anything, including set any PCDs (your > proposal), before its entry point is invoked. > > platform config entry point --> PCDs > > (4) The video driver can be loaded whenever, but its binding start > function will be called no sooner than BDS tries to connect the VGA card. > > PlatformBdsConnectConsole() --> video binding start > > If you add these up, they build a circular total dependency: > > PCDs > --> PlatformBdsConnectConsole() > --> video binding start > --> platform config entry point > --> PCDs > > In practice the cycle would be entered at PlatformBdsConnectConsole(), > and the PCDs would be loaded/set from the NvVar preference too late, at > each boot. > > Thanks, > Laszlo ------------------------------------------------------------------------------ Flow-based real-time traffic analytics software. Cisco certified tool. Monitor traffic, SLAs, QoS, Medianet, WAAS etc. with NetFlow Analyzer Customize your own dashboards, set traffic alerts and generate reports. Network behavioral analysis & security monitoring. All-in-one tool. http://pubads.g.doubleclick.net/gampad/clk?id=126839071&iu=/4140/ostg.clktrk _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
