[SeaBIOS] Re: [PATCH v2 2/2] Add basic MXM 3.0 interrupt support

[Kevin O'Connor]

On Fri, Feb 23, 2024 at 08:55:56PM +0000, Riku Viitanen via SeaBIOS wrote:
> VGAROMs on MXM graphics cards need certain int15h functions present,
> to function properly.
>
> On HP EliteBook 8560w with coreboot and Quadro 2000M, this warning
> is displayed for 30 seconds, making every boot extremely slow:
>
> ERROR: Valid MXM Structure not found
>    POST halted for 30 seconds, P-state limited to P10...
>
> This patch implements the minimum functionality to get rid of it: the
> functions 0 and 1, version 3.0 of the specification [1]. Older version
> 2.1 is not implemented.
>
> Functions are enabled only if romfile "mxm-30-sis" exists. These functios
> are specific to the MXM revision, but not the board or GPU. Some boards
> might require more of the optional functions to be implemented, however.
>
> Function 0 returns information about supported functions and revision.
>
> Function 1 returns a pointer to a MXM configuration structure in low
>
> memory. This is copied from romfile "mxm-30-sis".
>
> TEST: HP EliteBook 8560w boots without error and delay, graphics work.
>
> [1]: MXM Graphics Module Software Specification Version 3.0, Revision 1.1
> [2]: https://codeberg.org/Riku_V/mxmdump/
> Signed-off-by: Riku Viitanen
>
> src/optionroms.c |  8 ++++++
>  src/vgahooks.c   | 70 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/vgahooks.h   |  8 ++++++
>  3 files changed, 86 insertions(+)
>  create mode 100644 src/vgahooks.h
>
> diff --git a/src/optionroms.c b/src/optionroms.c
> index e906ab97..4e6a1ad1 100644
> --- a/src/optionroms.c
> +++ b/src/optionroms.c
> @@ -22,6 +22,7 @@
>  #include "string.h" // memset
>  #include "util.h" // get_pnp_offset
>  #include "tcgbios.h" // tpm_*
> +#include "vgahooks.h" // MXM30SIS
>
> static int EnforceChecksum, S3ResumeVga, RunPCIroms;
>
> @@ -463,6 +464,13 @@ vgarom_setup(void)
>      RunPCIroms = romfile_loadint("etc/pci-optionrom-exec", 2);
>      ScreenAndDebug = romfile_loadint("etc/screen-and-debug", 1);
> // Load MXM 3.0 System Information Structure, if it exists
> void *mxm_sis = romfile_loadfile_low("mxm-30-sis", NULL);
> if (mxm_sis)
>    MXM30SIS = (u32)mxm_sis;
> else
>    MXM30SIS = 0;

Thanks.  We can certainly add support for "mxm" to seabios.  It seems
odd to me that this patch modifies the "generic" optionrom code.  Is
there a reason the code is checking for "mxm-30-sis" from
optionsroms.c instead of from the existing vgahook_setup() code in
vgahooks.c?

Similarly, is there a reason patch 1 modifies the generic
src/romfile.c code to add a new romfile_loadfile_low() instead of
having mxm_setup() call romfile_loadfile(), malloc_low(), and
memcpy()?

Cheers,
-Kevin