On Wed, 5 May 2021 at 23:01, Rob Clark <robdclark@xxxxxxxxx> wrote:
>
> On Wed, May 5, 2021 at 12:35 PM John Stultz <john.stultz@xxxxxxxxxx> wrote:
> >
> > On Wed, May 5, 2021 at 8:04 AM Rob Clark <robdclark@xxxxxxxxx> wrote:
> > > On Tue, May 4, 2021 at 11:37 PM John Stultz <john.stultz@xxxxxxxxxx> wrote:
> > > > Hey Dmitry, Bjorn,
> > > > I wanted to raise a regression I caught in the merge window on db845c.
> > > >
> > > > I was seeing troubles with audio and while there are a few other
> > > > pending fixes needed, they did not seem to work for me. So I spent
> > > > some time bisecting things down and found the problematic commit was
> > > > 7443ff06da45 ("arm64: dts: sdm845-db845c: make firmware filenames
> > > > follow linux-firmware").
> > > >
> > > > It seems for systems using the old firmware filenames, this will break
> > > > dependent devices on adsp_pas and cdsp_pas nodes.
> > > >
> > > > Now, obviously updating the firmware files in userland should resolve
> > > > this, but it adds the complexity that we can't just replace the
> > > > firmware files because older LTS kernels will look for the old names,
> > > > while newer kernels will look for the new names. We can add both files
> > > > to the system images, but then there is some confusion on which
> > > > version of the firmware files are being used where.
> > > >
> > > > So yes, we should align with linux-firmware file names, but I think
> > > > more care is needed for this sort of thing as it has the potential to
> > > > break folks, and this isn't the first time around we've had similar
> > > > firmware name changes break us.
> > > >
> > > > So I'm working on fixing this by including both filenames in userland,
> > > > so we probably don't need a revert here, but *please* maybe take more
> > > > care on this sort of change.
> > > >
> > >
> > > It is rather more difficult than you think, because if you try the
> > > wrong path you could end up waiting with a timeout.. we have
> > > shenanigans to work around that for gpu fw in drm/msm to avoid this
> > > sort of regression with people using downstream firmware trees. I'd
> > > like to rip that out at some point, but I suppose doing so would be
> > > problematic for folks doing upstream kernel on android devices.
> > >
> > > Maybe there is some way to add support to simultaneously
> > > request_firmware for two different paths at the same time.. not sure
> > > how that would work from the PoV of the usermode helper path.
> > >
> > > It really is a lot of pain to deal with downstream firmware layout..
> >
> > Yeah, but on other platforms, the kernel has to meet and deal with the
> > hardware, firmware and userland as it exists in the world.
> > It would be quite a thing if an upstream kernel change required a new
> > BIOS update which then made the system incompatible with the most
> > recent LTS.
>
> Does downstream exist? :-P
>
> We first ran into this issue with GPU firmware when it was originally
> pushed to linux-firmware. Upstream linux-firmware wanted it moved to
> qcom subdirectory. At this point, now upstream and downstream
> firmware are incompatible in their directory layout. You can't really
> expect the kernel to support the downstream layout but not the
> upstream layout. If the upstream kernel has to choose, it will go
> with the upstream linux-firmware layout.
>
> Seriously though, the extra 60sec timeout delays is not an option. I
> don't really see any good option other than somehow teaching
> request_firmware how to look for two (or multiple) paths at the same
> time. I'm not sure (a) if that is possible (ie. without breaking
> compatibility with usermode helper), and (b) how others would feel
> about adding more complexity to the firmware loader to deal with
> downstream firmware trees. If others are open to that idea, I'm all
> for it.
>
> And tbf, the BIOS update example is a *bit* of a stretch, since that
> is often not something owners of the hw can do. In this case, it is
> just a matter of putting some symlinks in the downstream /lib/firmware
> to make it work with both cases.
In fact we had this in place for a while for RB5 (qrb5165, sm8250),
when there was no official firmware release.
--
With best wishes
Dmitry
