Re: [PATCH] EmbeddedPkg/DtPlatformDxe: remove /chosen/stdout-path on GOP registration

[Leif Lindholm <leif.lindholm@linaro.org>]

On Thu, Oct 19, 2017 at 08:21:41PM +0100, Ard Biesheuvel wrote:
> The presence of a /chosen/stdout-path property will force Linux to use
> the serial port as the primary console, even if a graphical console is
> available as well. But the presence of the Graphics Output Protocol (GOP)
> is a strong indication that the user may prefer to use his keyboard and
> mouse rather than his terminal emulator to interact with the system, so
> let's remove /chosen/stdout-path as soon as a GOP instance is registered.

This may be the sensible thing to do, but something just doesn't feel
right.

Also, whatever we do here, we should try to mirror in ACPI with SPCR.

Are we guaranteed to always want to disable serial console if there is
a graphics adapter?

I still dream of a world in which I can run consplitter up to the
point where an OS takes over, and an OS installer will listen to input
from both sources and register its selection from there.

But if we can't have that, can we have a menu setting to select preference?
The default can be a platform-specific Pcd.

/
    Leif

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c   | 41 ++++++++++++++++++++
>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf |  4 ++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
> index 1014be2281d4..4f9ac8090fac 100644
> --- a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
> @@ -12,6 +12,7 @@
>  *
>  **/
>  
> +#include <libfdt.h>
>  #include <Library/BaseLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/DevicePathLib.h>
> @@ -54,6 +55,9 @@ STATIC HII_VENDOR_DEVICE_PATH     mDtPlatformDxeVendorDevicePath = {
>    }
>  };
>  
> +STATIC EFI_EVENT                  mRegisterGopEvent;
> +STATIC VOID                       *mGopRegistration;
> +
>  STATIC
>  EFI_STATUS
>  InstallHiiPages (
> @@ -89,6 +93,32 @@ InstallHiiPages (
>    return EFI_SUCCESS;
>  }
>  
> +STATIC
> +VOID
> +OnRegisterGop (
> +  IN EFI_EVENT  Event,
> +  IN VOID       *Dtb
> +  )
> +{
> +  INT32     Node;
> +  INT32     Error;
> +
> +  DEBUG ((DEBUG_INFO,
> +    "%a: a GOP has been registered, removing /chosen/stdout-path from DT\n",
> +    __FUNCTION__));
> +
> +  Node = fdt_path_offset (Dtb, "/chosen");
> +  if (Node < 0) {
> +    return;
> +  }
> +
> +  Error = fdt_delprop (Dtb, Node, "stdout-path");
> +  if (Error) {
> +    DEBUG ((DEBUG_INFO, "%a: Failed to delete 'stdout-path' property: %a\n",
> +      __FUNCTION__, fdt_strerror (Error)));
> +  }
> +}
> +
>  /**
>    The entry point for DtPlatformDxe driver.
>  
> @@ -181,6 +211,17 @@ DtPlatformDxeEntryPoint (
>          __FUNCTION__));
>        goto FreeDtb;
>      }
> +
> +    Status = gBS->CreateEvent (EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
> +                    OnRegisterGop, Dtb, &mRegisterGopEvent);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_ERROR, "%a: failed to create event - %r\n",
> +        __FUNCTION__, Status));
> +    }
> +
> +    Status = gBS->RegisterProtocolNotify (&gEfiGraphicsOutputProtocolGuid,
> +                    mRegisterGopEvent, &mGopRegistration);
> +    ASSERT_EFI_ERROR (Status);
>    } else {
>      ASSERT (FALSE);
>    }
> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
> index 45dfd9088bf0..6b331294df8a 100644
> --- a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
> @@ -41,6 +41,7 @@ [LibraryClasses]
>    BaseLib
>    DebugLib
>    DtPlatformDtbLoaderLib
> +  FdtLib
>    HiiLib
>    MemoryAllocationLib
>    UefiBootServicesTableLib
> @@ -53,6 +54,9 @@ [Guids]
>    gEdkiiPlatformHasAcpiGuid
>    gFdtTableGuid
>  
> +[Protocols]
> +  gEfiGraphicsOutputProtocolGuid
> +
>  [Depex]
>    gEfiVariableArchProtocolGuid        AND
>    gEfiVariableWriteArchProtocolGuid
> -- 
> 2.11.0
>