Re: [PATCH] drm/i915/dmc: Accept symbolic link in firmware name

Imre Deak
2016-07-11T13:24:54Z

On ma, 2016-07-11 at 13:55 +0100, chris@chris-wilson.co.uk wrote:
> On Mon, Jul 11, 2016 at 03:45:21PM +0300, Imre Deak wrote:
> > On ma, 2016-07-11 at 13:39 +0100, chris@chris-wilson.co.uk wrote:
> > > On Mon, Jul 11, 2016 at 02:23:48PM +0300, Imre Deak wrote:
> > > > On to, 2016-07-07 at 17:57 +0300, Mika Kuoppala wrote:
> > > > > "Vivi, Rodrigo" <rodrigo.vivi@intel.com> writes:
> > > > >
> > > > > > Nak.
> > > > > >
> > > > > > I don't intend to update the symbolic links on linux-
> > > > > > firmware.git
> > > > > > repository anymore so if we receive a new minor version
> > > > > > update
> > > > > > we
> > > > > > are
> > > > > > not going to load.
> > > > > >
> > > > > > I was the one advocating in the favor for the symbolic link
> > > > > > flexibility
> > > > > > but I lost the discussions for the stability and validation
> > > > > > etc.
> > > > > >
> > > > >
> > > > > And I was one advocating in favor of getting rid of symlink.
> > > > > But
> > > > > the
> > > > > filename versioning is superfluous as the contents has the
> > > > > version
> > > > > info
> > > > > which we can solely rely to not run something we dont want.
> > > > >
> > > > > So I am not sure what we lose in stability and validation
> > > > > front
> > > > > with the strict version check.
> > > >
> > > > Bisection is more cumbersome with a symlink.
> > >
> > > Did you miss a without there? Because when bisecting the kernel
> > > it's
> > > harder without the symlink as the build breaks otherwise and the
> > > runtime
> > > is not bisectable either.
> >
> > No, I meant when bisecting we also want to load the proper fw
> > version
> > for each bisect commit point. So using exact filenames we'll
> > automatically load the proper firmware file for a given commit,
> > whereas
> > with symlinks we have to adjust the symlink at each bisect step.
>
> You mean you have to adjust the kernel build at each point. That is
> the
> root of the problem as we do not have deferred firmware loading.

Yes. Looking at the bug now: it's true that using CONFIG_EXTRA_FIRMWARE
you have to update this config option at each bisect point, but you
would have to anyway update the symlink in that case too.

> And then you get random changes in the firmare whilst bisecting the
> kernel.

What do you mean random? During bisecting we want to load the firmware
version that was used with a particular commit. With a symlink pointing
to the wrong firmware file for a given commit, we'd fail loading the
firmware due to the version check and hide/introduce bugs for that
commit.

--Imre