Re: [PATCH 2/4] efi/libstub: detect panel-id

[Ard Biesheuvel]

On Wed, 3 Jul 2019 at 19:41, Rob Clark <robdclark@xxxxxxxxx> wrote:
>
> On Wed, Jul 3, 2019 at 9:33 AM Leif Lindholm <leif.lindholm@xxxxxxxxxx> wrote:
> >
> > On Tue, Jul 02, 2019 at 03:48:48PM -0700, Rob Clark wrote:
> > > > > There is one kernel, and there
> > > > > are N distro's, so debugging a users "I don't get a screen at boot"
> > > > > problem because their distro missed some shim patch really just
> > > > > doesn't seem like a headache I want to have.
> > > >
> > > > The distros should not need to be aware *at all* of the hacks required
> > > > to disguise these platforms as DT platforms.
> > > >
> > > > If they do, they're already device-specific installers and have
> > > > already accepted the logistical/support nightmare.
> > >
> > > I guess I'm not *against* a DT loader shim populating the panel-id
> > > over into /chosen.. I had it in mind as a backup plan.  Ofc still need
> > > to get dt folks to buy into /chosen/panel-id but for DT boot I think
> > > that is the best option.  (At least the /chosen/panel-id approach
> > > doesn't require the shim to be aware of how the panel is wired up to
> > > dsi controller and whether their is a bridge in between, and that
> > > short of thing, so the panel-id approach seems more maintainable that
> > > other options.)
> >
> > I am leaning like Ard towards preferring a configuration table though.
>
> Ok, if you want the DT loader to propagate UEFIDisplayInfo to a config
> table, I can update the drm parts of my patchset to look for that in
> addition to /chosen/panel-id
>
> > That removes the question of no runtime services (needing to manually
> > cache things, at least until EBBR 1.2 (?) is out and in use), and
> > means we don't have to use different paths for DT and ACPI. Now we
> > have UEFI in U-Boot, do we really need to worry about the non-UEFI
> > case?
>
> I've mixed feelings about requiring UEFI..  I definitely want to give
> qcom an incentive to turn on GOP and full UEFI boot for future android
> devices.  OTOH there are quite a few devices out there that aren't
> UEFI boot.  But I guess if drm falls back to /chosen/panel-id we are
> covered.
>
> > > I am a bit fearful of problems arising from different distros and
> > > users using different versions of shim, and how to manage that.  I
> > > guess if somehow "shim thing" was part of the kernel, there would by
> > > one less moving part...
> >
> > Sure, but that's insurance against bindings changing
> > non-backwards-compatibly - which there are ways to prevent, and which
> > streamlining the design for really isn't the way to discourage...
> >
> > Distros have no need to worry about the DT loader - the whole point of
> > it is to remove the need for the distro to worry about anything other
> > than getting the required drivers in.
>
> I'm a bit more concerned about DT loader getting into the business of
> DT fixup..  I guess if we don't do that, it is less of a concern.  But
> if we relied on it to fixup DT for installed panel, we could probably
> make it work semi-generically on existing devices that have bridge and
> panel wired up same way.  But seems like some of the 835 laptops have
> bridge hooked up as child of dsi bus instead.  And someday we could
> see devices using dsi directly, etc.
>
> (It would be really nice to see DT loader able to pick the correct
> .dtb based on smbios tables tho ;-).. but maybe different topic)
>

I think this is the only sane way of doing things: the DT loader,
which is tied much more closely to the platform, does whatever it
needs to do to infer from UEFI variables and/or ACPI or SMBIOS tables
which bundled DT it installs, and whether/how it needs to fix things
up. This indeed constitutes a moving part separate from the OS, but
this is the only way that scales. Getting DTs for these devices into
distros is *not* what we should be doing.

> > > I'd know if user had kernel vX.Y.Z they'd be
> > > good to go vs not.  But *also* depending on a new-enough version of a
> > > shim, where the version # is probably not easily apparent to the end
> > > user, sounds a bit scary from the "all the things that can go wrong"
> > > point of view.  Maybe I'm paranoid, but I'm a bit worried about how to
> > > manage that.
> >
> > Until the hardware abstractions provided by the system firmware (ACPI)
> > is supported, these platforms are not going to be appropriate for
> > end users anyway. No matter how many not-quite-upstream hacks distros
> > include, they won't be able to support the next minor spin that comes
> > off the production line and is no longer compatible with existing DTs.
>
> yeah, that will be a problem.. and also switching to older kernel
> after upgrading when in-flight dt bindings evolve.  Having one less
> moving part would be nice.
>

The whole point of the discussion we have been having for years is
that for production use cases, we should not be dealing with evolving
in-flight DT binding in the first place. If we ship a DT loader with a
certain version of the binding and there is a need to change it, we
can only do so if we retain support for the old binding as well.

> Maybe if adding a config table for UEFIDisplayInfo, you could also add
> one for DT loader version, so (at least if user is able to get far
> enough to get dmesg) we could see that more easily?
>

I'd prefer it if the DT loader were in charge of creating the
UEFIDisplayInfo config tables, but given that we'll need to deal with
platforms that don't implement runtime variable services, it is
something I would be able to live with in the stub.

In summary, working around platform limitations that prevent us from
delivering an accurate DT to the OS should preferably be addressed in
a platform specific pre-OS component. I haven't had the chance to look
at leif's code yet, but from what I understand, we have pretty much
what we need with the exception of the panel/gop variable handling,
no?