Re: [PATCH v3 1/8] drm/i915/skl: Add support to load SKL CSR firmware

Imre Deak
2015-04-13T17:15:32Z

On Mon, 2015-04-13 at 18:02 +0100, Damien Lespiau wrote:
> On Mon, Apr 13, 2015 at 07:52:54PM +0300, Imre Deak wrote:
> > On Mon, 2015-04-13 at 17:34 +0100, Damien Lespiau wrote:
> > > On Mon, Apr 13, 2015 at 03:54:02PM +0530, Animesh Manna wrote:
> > > > diff --git a/drivers/gpu/drm/i915/intel_csr.h b/drivers/gpu/drm/i915/intel_csr.h
> > > > new file mode 100644
> > > > index 0000000..c2a5a53
> > > > --- /dev/null
> > > > +++ b/drivers/gpu/drm/i915/intel_csr.h
> > > > +
> > >
> > > [...]
> > >
> > > > +#define I915_CSR_SKL "i915/dmc_gen9.bin"
> > >
> > > I'm guessing the BXT DMC firwmare will be different from SKL's? Remember
> > > that one of the requirements is to be able to have the same OS image
> > > boot on both SKL and BXT. That means the firmware names have to be
> > > different for SKL and BXT. We probably should have skl in the name here.
> > >
> > > Also, we need to be able to be able to support "Interface" versions. Ie.
> > > if the firmware interface changes in such a way a different loading code
> > > is needed, we need to be able to have both firmware on the disk so both
> > > an old kernel and a new kernel can work with the same user space.
> > >
> > > Right now, the naming scheme of the firmware does have a version on it,
> > > hopefully that's this "Interface" version.
> > >
> > > Thoughts?
> >
> > Yes, the above file name looks incorrect. "dmc_gen9.bin" was the name
> > for the firmware image with the old layout. This patchset adds support
> > for the new firmware layout starting from ver 1, while the old layout
> > doesn't need to be supported. In an earlier version of this patch the
> > filename was changed to "i915/skl_dmc_ver1.bin" not sure why that change
> > got dropped. I think we could just use this latter name.
>
> First public firmware is already v4 [1], no idea is the version bumps
> are actual API/interface changes.

Ok, I haven't seen that. One question is if we need to support multiple
interface versions or just the latest one. I would say only the latest
one (for each platform) and so I915_CSR_SKL should be this latest
firmware image filename, in this case i915/skl_dmc_ver4.bin.

--Imre