Re: [Intel-gfx] [PATCH 2/2] drm/i915/dmc: Use unversioned firmware paths

2022-12-28 Thread Gustavo Sousa
On Wed, Dec 28, 2022 at 07:04:46PM -0300, Vivi, Rodrigo wrote:
> On Wed, 2022-12-28 at 19:00 -0300, Gustavo Sousa wrote:
> > On Fri, Dec 23, 2022 at 10:36:05AM -0500, Rodrigo Vivi wrote:
> > > On Fri, Dec 23, 2022 at 08:51:59AM -0300, Gustavo Sousa wrote:
> > > > On Thu, Dec 22, 2022 at 04:52:21PM -0800, Lucas De Marchi wrote:
> > > > > On Thu, Dec 22, 2022 at 07:12:08PM -0300, Gustavo Sousa wrote:
> > > > > > On Wed, Dec 21, 2022 at 04:23:45PM -0800, Lucas De Marchi
> > > > > > wrote:
> > > > > > > On Wed, Dec 21, 2022 at 12:26:26PM +0200, Jani Nikula
> > > > > > > wrote:
> > > > > > > > On Tue, 20 Dec 2022, Gustavo Sousa
> > > > > > > >  wrote:
> > > > > > > > > As we do not require specific versions anymore, change
> > > > > > > > > the convention
> > > > > > > > > for blob filenames to stop using version numbers. This
> > > > > > > > > simplifies code
> > > > > > > > > maintenance, since we do not need to keep updating blob
> > > > > > > > > paths for new
> > > > > > > > > DMC releases, and also makes DMC loading compatible
> > > > > > > > > with systems that do
> > > > > > > > > not have the latest firmware release.
> > > > > > > > > 
> > > > > > > > > References:
> > > > > > > > > https://lore.kernel.org/r/y3081s7c5ksut...@intel.com
> > > > > > > > > Signed-off-by: Gustavo Sousa 
> > > > > > > > > ---
> > > > > > > > >  drivers/gpu/drm/i915/display/intel_dmc.c | 98
> > > > > > > > > 
> > > > > > > > >  1 file changed, 82 insertions(+), 16 deletions(-)
> > > > > > > > > 
> > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > > > > > > b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > > > > > > index 4124b3d37110..b11f0f451dd7 100644
> > > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > > > > > > @@ -42,51 +42,70 @@
> > > > > > > > >  #define DMC_VERSION_MAJOR(version) ((version) >>
> > > > > > > > > 16)
> > > > > > > > >  #define DMC_VERSION_MINOR(version) ((version) &
> > > > > > > > > 0x)
> > > > > > > > > 
> > > > > > > > > -#define DMC_PATH(platform, major, minor) \
> > > > > > > > > -   "i915/"  \
> > > > > > > > > -   __stringify(platform) "_dmc_ver" \
> > > > > > > > > -   __stringify(major) "_"   \
> > > > > > > > > +#define DMC_PATH(platform) \
> > > > > > > > > +   "i915/" __stringify(platform) "_dmc.bin"
> > > > > > > > > +
> > > > > > > > > +/*
> > > > > > > > > + * New DMC additions should not use this. This is used
> > > > > > > > > solely to remain
> > > > > > > > > + * compatible with systems that have not yet updated
> > > > > > > > > DMC blobs to use
> > > > > > > > > + * unversioned file names.
> > > > > > > > > + */
> > > > > > > > > +#define DMC_LEGACY_PATH(platform, major, minor) \
> > > > > > > > > +   "i915/" \
> > > > > > > > > +   __stringify(platform) "_dmc_ver"\
> > > > > > > > > +   __stringify(major) "_"  \
> > > > > > > > > __stringify(minor) ".bin"
> > > > > > > > > 
> > > > > > > > >  #define DISPLAY_VER13_DMC_MAX_FW_SIZE  0x2
> > > > > > > > > 
> > > > > > > > >  #define
> > > > > > > > > DISPLAY_VER12_DMC_MAX_FW_SIZE  ICL_DMC_MAX_FW_SIZE
> > > > > > > > > 
> > > > > > > > > -#define DG2_DMC_PATH   DMC_PATH(dg2,
> > > > > > > > > 2, 08)
> > > > > > > > > +#define DG2_DMC_PATH   DMC_PATH(dg2)
> > > > > > > > > +#define
> > > > > > > > > DG2_DMC_LEGACY_PATHDMC_LEGACY_PATH(dg2, 2,
> > > > > > > > > 08)
> > > > > > > > >  MODULE_FIRMWARE(DG2_DMC_PATH);
> > > > > > > 
> > > > > > > We have an issue here.  Previously `modinfo --
> > > > > > > field=firmware i915`
> > > > > > > would report i915/dg2_dmc_ver2_08.bin. Now it's going to
> > > > > > > report
> > > > > > > i915/dg2_dmc.bin
> > > > > > > 
> > > > > > > that modinfo call is what distros use to bundle the
> > > > > > > firmware blobs in
> > > > > > > the initrd. It may also be used for creating package
> > > > > > > dependendies.
> > > > > > > 
> > > > > > > If we do this, unless they have updated their linux-
> > > > > > > firmware
> > > > > > > packages, the initrd will be left without the firmware.
> > > > > > > Just checked
> > > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linu
> > > > > > > x-firmware.git main
> > > > > > > and we still don't have the unversioned firmware there.
> > > > > > 
> > > > > > Interesting. Thanks for the explanation!
> > > > > > 
> > > > > > I think one way of approaching the issue would be to
> > > > > > synchronize the process:
> > > > > > 
> > > > > > 1. Work toward getting approval for the patch (i.e. r-b);
> > > > > > 2. With the approval, send a PR to linux-firmware with the
> > > > > > necessary changes;
> > > > > > 3. After the linux-firmware PR is merged, the patch could be
> > > > > > integraged.
> > > > > > 
> > > > > > I think that would stil

Re: [Intel-gfx] [PATCH 2/2] drm/i915/dmc: Use unversioned firmware paths

2022-12-28 Thread Vivi, Rodrigo
On Wed, 2022-12-28 at 19:00 -0300, Gustavo Sousa wrote:
> On Fri, Dec 23, 2022 at 10:36:05AM -0500, Rodrigo Vivi wrote:
> > On Fri, Dec 23, 2022 at 08:51:59AM -0300, Gustavo Sousa wrote:
> > > On Thu, Dec 22, 2022 at 04:52:21PM -0800, Lucas De Marchi wrote:
> > > > On Thu, Dec 22, 2022 at 07:12:08PM -0300, Gustavo Sousa wrote:
> > > > > On Wed, Dec 21, 2022 at 04:23:45PM -0800, Lucas De Marchi
> > > > > wrote:
> > > > > > On Wed, Dec 21, 2022 at 12:26:26PM +0200, Jani Nikula
> > > > > > wrote:
> > > > > > > On Tue, 20 Dec 2022, Gustavo Sousa
> > > > > > >  wrote:
> > > > > > > > As we do not require specific versions anymore, change
> > > > > > > > the convention
> > > > > > > > for blob filenames to stop using version numbers. This
> > > > > > > > simplifies code
> > > > > > > > maintenance, since we do not need to keep updating blob
> > > > > > > > paths for new
> > > > > > > > DMC releases, and also makes DMC loading compatible
> > > > > > > > with systems that do
> > > > > > > > not have the latest firmware release.
> > > > > > > > 
> > > > > > > > References:
> > > > > > > > https://lore.kernel.org/r/y3081s7c5ksut...@intel.com
> > > > > > > > Signed-off-by: Gustavo Sousa 
> > > > > > > > ---
> > > > > > > >  drivers/gpu/drm/i915/display/intel_dmc.c | 98
> > > > > > > > 
> > > > > > > >  1 file changed, 82 insertions(+), 16 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > > > > > b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > > > > > index 4124b3d37110..b11f0f451dd7 100644
> > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > > > > > @@ -42,51 +42,70 @@
> > > > > > > >  #define DMC_VERSION_MAJOR(version) ((version) >>
> > > > > > > > 16)
> > > > > > > >  #define DMC_VERSION_MINOR(version) ((version) &
> > > > > > > > 0x)
> > > > > > > > 
> > > > > > > > -#define DMC_PATH(platform, major, minor) \
> > > > > > > > -   "i915/"  \
> > > > > > > > -   __stringify(platform) "_dmc_ver" \
> > > > > > > > -   __stringify(major) "_"   \
> > > > > > > > +#define DMC_PATH(platform) \
> > > > > > > > +   "i915/" __stringify(platform) "_dmc.bin"
> > > > > > > > +
> > > > > > > > +/*
> > > > > > > > + * New DMC additions should not use this. This is used
> > > > > > > > solely to remain
> > > > > > > > + * compatible with systems that have not yet updated
> > > > > > > > DMC blobs to use
> > > > > > > > + * unversioned file names.
> > > > > > > > + */
> > > > > > > > +#define DMC_LEGACY_PATH(platform, major, minor) \
> > > > > > > > +   "i915/" \
> > > > > > > > +   __stringify(platform) "_dmc_ver"\
> > > > > > > > +   __stringify(major) "_"  \
> > > > > > > > __stringify(minor) ".bin"
> > > > > > > > 
> > > > > > > >  #define DISPLAY_VER13_DMC_MAX_FW_SIZE  0x2
> > > > > > > > 
> > > > > > > >  #define
> > > > > > > > DISPLAY_VER12_DMC_MAX_FW_SIZE  ICL_DMC_MAX_FW_SIZE
> > > > > > > > 
> > > > > > > > -#define DG2_DMC_PATH   DMC_PATH(dg2,
> > > > > > > > 2, 08)
> > > > > > > > +#define DG2_DMC_PATH   DMC_PATH(dg2)
> > > > > > > > +#define
> > > > > > > > DG2_DMC_LEGACY_PATHDMC_LEGACY_PATH(dg2, 2,
> > > > > > > > 08)
> > > > > > > >  MODULE_FIRMWARE(DG2_DMC_PATH);
> > > > > > 
> > > > > > We have an issue here.  Previously `modinfo --
> > > > > > field=firmware i915`
> > > > > > would report i915/dg2_dmc_ver2_08.bin. Now it's going to
> > > > > > report
> > > > > > i915/dg2_dmc.bin
> > > > > > 
> > > > > > that modinfo call is what distros use to bundle the
> > > > > > firmware blobs in
> > > > > > the initrd. It may also be used for creating package
> > > > > > dependendies.
> > > > > > 
> > > > > > If we do this, unless they have updated their linux-
> > > > > > firmware
> > > > > > packages, the initrd will be left without the firmware.
> > > > > > Just checked
> > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linu
> > > > > > x-firmware.git main
> > > > > > and we still don't have the unversioned firmware there.
> > > > > 
> > > > > Interesting. Thanks for the explanation!
> > > > > 
> > > > > I think one way of approaching the issue would be to
> > > > > synchronize the process:
> > > > > 
> > > > > 1. Work toward getting approval for the patch (i.e. r-b);
> > > > > 2. With the approval, send a PR to linux-firmware with the
> > > > > necessary changes;
> > > > > 3. After the linux-firmware PR is merged, the patch could be
> > > > > integraged.
> > > > > 
> > > > > I think that would still apply if going with your proposal on
> > > > > your next comment.
> > > > > 
> > > > > > 
> > > > > > > > 
> > > > > > > > -#define ADLP_DMC_PATH  DMC_PATH(adlp,
> > > > > > > > 2, 16)
> > > > > > > > +#define ADLP_DMC_PATH  DMC_PATH(adl

Re: [Intel-gfx] [PATCH 2/2] drm/i915/dmc: Use unversioned firmware paths

2022-12-28 Thread Gustavo Sousa
On Fri, Dec 23, 2022 at 10:36:05AM -0500, Rodrigo Vivi wrote:
> On Fri, Dec 23, 2022 at 08:51:59AM -0300, Gustavo Sousa wrote:
> > On Thu, Dec 22, 2022 at 04:52:21PM -0800, Lucas De Marchi wrote:
> > > On Thu, Dec 22, 2022 at 07:12:08PM -0300, Gustavo Sousa wrote:
> > > > On Wed, Dec 21, 2022 at 04:23:45PM -0800, Lucas De Marchi wrote:
> > > > > On Wed, Dec 21, 2022 at 12:26:26PM +0200, Jani Nikula wrote:
> > > > > > On Tue, 20 Dec 2022, Gustavo Sousa  wrote:
> > > > > > > As we do not require specific versions anymore, change the 
> > > > > > > convention
> > > > > > > for blob filenames to stop using version numbers. This simplifies 
> > > > > > > code
> > > > > > > maintenance, since we do not need to keep updating blob paths for 
> > > > > > > new
> > > > > > > DMC releases, and also makes DMC loading compatible with systems 
> > > > > > > that do
> > > > > > > not have the latest firmware release.
> > > > > > >
> > > > > > > References: https://lore.kernel.org/r/y3081s7c5ksut...@intel.com
> > > > > > > Signed-off-by: Gustavo Sousa 
> > > > > > > ---
> > > > > > >  drivers/gpu/drm/i915/display/intel_dmc.c | 98 
> > > > > > > 
> > > > > > >  1 file changed, 82 insertions(+), 16 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c 
> > > > > > > b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > > > > index 4124b3d37110..b11f0f451dd7 100644
> > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > > > > @@ -42,51 +42,70 @@
> > > > > > >  #define DMC_VERSION_MAJOR(version)   ((version) >> 16)
> > > > > > >  #define DMC_VERSION_MINOR(version)   ((version) & 0x)
> > > > > > >
> > > > > > > -#define DMC_PATH(platform, major, minor) \
> > > > > > > - "i915/"  \
> > > > > > > - __stringify(platform) "_dmc_ver" \
> > > > > > > - __stringify(major) "_"   \
> > > > > > > +#define DMC_PATH(platform) \
> > > > > > > + "i915/" __stringify(platform) "_dmc.bin"
> > > > > > > +
> > > > > > > +/*
> > > > > > > + * New DMC additions should not use this. This is used solely to 
> > > > > > > remain
> > > > > > > + * compatible with systems that have not yet updated DMC blobs 
> > > > > > > to use
> > > > > > > + * unversioned file names.
> > > > > > > + */
> > > > > > > +#define DMC_LEGACY_PATH(platform, major, minor) \
> > > > > > > + "i915/" \
> > > > > > > + __stringify(platform) "_dmc_ver"\
> > > > > > > + __stringify(major) "_"  \
> > > > > > >   __stringify(minor) ".bin"
> > > > > > >
> > > > > > >  #define DISPLAY_VER13_DMC_MAX_FW_SIZE0x2
> > > > > > >
> > > > > > >  #define DISPLAY_VER12_DMC_MAX_FW_SIZEICL_DMC_MAX_FW_SIZE
> > > > > > >
> > > > > > > -#define DG2_DMC_PATH DMC_PATH(dg2, 2, 08)
> > > > > > > +#define DG2_DMC_PATH DMC_PATH(dg2)
> > > > > > > +#define DG2_DMC_LEGACY_PATH  DMC_LEGACY_PATH(dg2, 2, 
> > > > > > > 08)
> > > > > > >  MODULE_FIRMWARE(DG2_DMC_PATH);
> > > > > 
> > > > > We have an issue here.  Previously `modinfo --field=firmware i915`
> > > > > would report i915/dg2_dmc_ver2_08.bin. Now it's going to report
> > > > > i915/dg2_dmc.bin
> > > > > 
> > > > > that modinfo call is what distros use to bundle the firmware blobs in
> > > > > the initrd. It may also be used for creating package dependendies.
> > > > > 
> > > > > If we do this, unless they have updated their linux-firmware
> > > > > packages, the initrd will be left without the firmware.
> > > > > Just checked
> > > > > git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
> > > > >  main
> > > > > and we still don't have the unversioned firmware there.
> > > > 
> > > > Interesting. Thanks for the explanation!
> > > > 
> > > > I think one way of approaching the issue would be to synchronize the 
> > > > process:
> > > > 
> > > > 1. Work toward getting approval for the patch (i.e. r-b);
> > > > 2. With the approval, send a PR to linux-firmware with the necessary 
> > > > changes;
> > > > 3. After the linux-firmware PR is merged, the patch could be integraged.
> > > > 
> > > > I think that would still apply if going with your proposal on your next 
> > > > comment.
> > > > 
> > > > > 
> > > > > > >
> > > > > > > -#define ADLP_DMC_PATHDMC_PATH(adlp, 2, 16)
> > > > > > > +#define ADLP_DMC_PATHDMC_PATH(adlp)
> > > > > > > +#define ADLP_DMC_LEGACY_PATH DMC_LEGACY_PATH(adlp, 
> > > > > > > 2, 16)
> > > > > > >  MODULE_FIRMWARE(ADLP_DMC_PATH);
> > > > > > >
> > > > > > > -#define ADLS_DMC_PATHDMC_PATH(adls, 2, 01)
> > > > > > > +#define ADLS_DMC_PATHDMC_PATH(adls)
> > > > > > > +#define ADLS_DMC_LEGACY_PATH DMC_LEGACY_PATH(adls, 
> > > > > > > 2, 01)
> > > > > > >  MODULE_FIRMWARE(ADLS_DMC_PATH);
> > > > > > >
> > > > > > > -

Re: [Intel-gfx] [PATCH 2/2] drm/i915/dmc: Use unversioned firmware paths

2022-12-23 Thread Rodrigo Vivi
On Fri, Dec 23, 2022 at 08:51:59AM -0300, Gustavo Sousa wrote:
> On Thu, Dec 22, 2022 at 04:52:21PM -0800, Lucas De Marchi wrote:
> > On Thu, Dec 22, 2022 at 07:12:08PM -0300, Gustavo Sousa wrote:
> > > On Wed, Dec 21, 2022 at 04:23:45PM -0800, Lucas De Marchi wrote:
> > > > On Wed, Dec 21, 2022 at 12:26:26PM +0200, Jani Nikula wrote:
> > > > > On Tue, 20 Dec 2022, Gustavo Sousa  wrote:
> > > > > > As we do not require specific versions anymore, change the 
> > > > > > convention
> > > > > > for blob filenames to stop using version numbers. This simplifies 
> > > > > > code
> > > > > > maintenance, since we do not need to keep updating blob paths for 
> > > > > > new
> > > > > > DMC releases, and also makes DMC loading compatible with systems 
> > > > > > that do
> > > > > > not have the latest firmware release.
> > > > > >
> > > > > > References: https://lore.kernel.org/r/y3081s7c5ksut...@intel.com
> > > > > > Signed-off-by: Gustavo Sousa 
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/display/intel_dmc.c | 98 
> > > > > > 
> > > > > >  1 file changed, 82 insertions(+), 16 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c 
> > > > > > b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > > > index 4124b3d37110..b11f0f451dd7 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > > > @@ -42,51 +42,70 @@
> > > > > >  #define DMC_VERSION_MAJOR(version) ((version) >> 16)
> > > > > >  #define DMC_VERSION_MINOR(version) ((version) & 0x)
> > > > > >
> > > > > > -#define DMC_PATH(platform, major, minor) \
> > > > > > -   "i915/"  \
> > > > > > -   __stringify(platform) "_dmc_ver" \
> > > > > > -   __stringify(major) "_"   \
> > > > > > +#define DMC_PATH(platform) \
> > > > > > +   "i915/" __stringify(platform) "_dmc.bin"
> > > > > > +
> > > > > > +/*
> > > > > > + * New DMC additions should not use this. This is used solely to 
> > > > > > remain
> > > > > > + * compatible with systems that have not yet updated DMC blobs to 
> > > > > > use
> > > > > > + * unversioned file names.
> > > > > > + */
> > > > > > +#define DMC_LEGACY_PATH(platform, major, minor) \
> > > > > > +   "i915/" \
> > > > > > +   __stringify(platform) "_dmc_ver"\
> > > > > > +   __stringify(major) "_"  \
> > > > > > __stringify(minor) ".bin"
> > > > > >
> > > > > >  #define DISPLAY_VER13_DMC_MAX_FW_SIZE  0x2
> > > > > >
> > > > > >  #define DISPLAY_VER12_DMC_MAX_FW_SIZE  ICL_DMC_MAX_FW_SIZE
> > > > > >
> > > > > > -#define DG2_DMC_PATH   DMC_PATH(dg2, 2, 08)
> > > > > > +#define DG2_DMC_PATH   DMC_PATH(dg2)
> > > > > > +#define DG2_DMC_LEGACY_PATHDMC_LEGACY_PATH(dg2, 2, 
> > > > > > 08)
> > > > > >  MODULE_FIRMWARE(DG2_DMC_PATH);
> > > > 
> > > > We have an issue here.  Previously `modinfo --field=firmware i915`
> > > > would report i915/dg2_dmc_ver2_08.bin. Now it's going to report
> > > > i915/dg2_dmc.bin
> > > > 
> > > > that modinfo call is what distros use to bundle the firmware blobs in
> > > > the initrd. It may also be used for creating package dependendies.
> > > > 
> > > > If we do this, unless they have updated their linux-firmware
> > > > packages, the initrd will be left without the firmware.
> > > > Just checked
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
> > > >  main
> > > > and we still don't have the unversioned firmware there.
> > > 
> > > Interesting. Thanks for the explanation!
> > > 
> > > I think one way of approaching the issue would be to synchronize the 
> > > process:
> > > 
> > > 1. Work toward getting approval for the patch (i.e. r-b);
> > > 2. With the approval, send a PR to linux-firmware with the necessary 
> > > changes;
> > > 3. After the linux-firmware PR is merged, the patch could be integraged.
> > > 
> > > I think that would still apply if going with your proposal on your next 
> > > comment.
> > > 
> > > > 
> > > > > >
> > > > > > -#define ADLP_DMC_PATH  DMC_PATH(adlp, 2, 16)
> > > > > > +#define ADLP_DMC_PATH  DMC_PATH(adlp)
> > > > > > +#define ADLP_DMC_LEGACY_PATH   DMC_LEGACY_PATH(adlp, 
> > > > > > 2, 16)
> > > > > >  MODULE_FIRMWARE(ADLP_DMC_PATH);
> > > > > >
> > > > > > -#define ADLS_DMC_PATH  DMC_PATH(adls, 2, 01)
> > > > > > +#define ADLS_DMC_PATH  DMC_PATH(adls)
> > > > > > +#define ADLS_DMC_LEGACY_PATH   DMC_LEGACY_PATH(adls, 
> > > > > > 2, 01)
> > > > > >  MODULE_FIRMWARE(ADLS_DMC_PATH);
> > > > > >
> > > > > > -#define DG1_DMC_PATH   DMC_PATH(dg1, 2, 02)
> > > > > > +#define DG1_DMC_PATH   DMC_PATH(dg1)
> > > > > > +#define DG1_DMC_LEGACY_PATHDMC_LEGACY_PATH(dg1, 2, 
> > > > > > 02)
> > > > > >  

Re: [Intel-gfx] [PATCH 2/2] drm/i915/dmc: Use unversioned firmware paths

2022-12-23 Thread Gustavo Sousa
On Thu, Dec 22, 2022 at 04:52:21PM -0800, Lucas De Marchi wrote:
> On Thu, Dec 22, 2022 at 07:12:08PM -0300, Gustavo Sousa wrote:
> > On Wed, Dec 21, 2022 at 04:23:45PM -0800, Lucas De Marchi wrote:
> > > On Wed, Dec 21, 2022 at 12:26:26PM +0200, Jani Nikula wrote:
> > > > On Tue, 20 Dec 2022, Gustavo Sousa  wrote:
> > > > > As we do not require specific versions anymore, change the convention
> > > > > for blob filenames to stop using version numbers. This simplifies code
> > > > > maintenance, since we do not need to keep updating blob paths for new
> > > > > DMC releases, and also makes DMC loading compatible with systems that 
> > > > > do
> > > > > not have the latest firmware release.
> > > > >
> > > > > References: https://lore.kernel.org/r/y3081s7c5ksut...@intel.com
> > > > > Signed-off-by: Gustavo Sousa 
> > > > > ---
> > > > >  drivers/gpu/drm/i915/display/intel_dmc.c | 98 
> > > > > 
> > > > >  1 file changed, 82 insertions(+), 16 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c 
> > > > > b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > > index 4124b3d37110..b11f0f451dd7 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > > @@ -42,51 +42,70 @@
> > > > >  #define DMC_VERSION_MAJOR(version)   ((version) >> 16)
> > > > >  #define DMC_VERSION_MINOR(version)   ((version) & 0x)
> > > > >
> > > > > -#define DMC_PATH(platform, major, minor) \
> > > > > - "i915/"  \
> > > > > - __stringify(platform) "_dmc_ver" \
> > > > > - __stringify(major) "_"   \
> > > > > +#define DMC_PATH(platform) \
> > > > > + "i915/" __stringify(platform) "_dmc.bin"
> > > > > +
> > > > > +/*
> > > > > + * New DMC additions should not use this. This is used solely to 
> > > > > remain
> > > > > + * compatible with systems that have not yet updated DMC blobs to use
> > > > > + * unversioned file names.
> > > > > + */
> > > > > +#define DMC_LEGACY_PATH(platform, major, minor) \
> > > > > + "i915/" \
> > > > > + __stringify(platform) "_dmc_ver"\
> > > > > + __stringify(major) "_"  \
> > > > >   __stringify(minor) ".bin"
> > > > >
> > > > >  #define DISPLAY_VER13_DMC_MAX_FW_SIZE0x2
> > > > >
> > > > >  #define DISPLAY_VER12_DMC_MAX_FW_SIZEICL_DMC_MAX_FW_SIZE
> > > > >
> > > > > -#define DG2_DMC_PATH DMC_PATH(dg2, 2, 08)
> > > > > +#define DG2_DMC_PATH DMC_PATH(dg2)
> > > > > +#define DG2_DMC_LEGACY_PATH  DMC_LEGACY_PATH(dg2, 2, 08)
> > > > >  MODULE_FIRMWARE(DG2_DMC_PATH);
> > > 
> > > We have an issue here.  Previously `modinfo --field=firmware i915`
> > > would report i915/dg2_dmc_ver2_08.bin. Now it's going to report
> > > i915/dg2_dmc.bin
> > > 
> > > that modinfo call is what distros use to bundle the firmware blobs in
> > > the initrd. It may also be used for creating package dependendies.
> > > 
> > > If we do this, unless they have updated their linux-firmware
> > > packages, the initrd will be left without the firmware.
> > > Just checked
> > > git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git 
> > > main
> > > and we still don't have the unversioned firmware there.
> > 
> > Interesting. Thanks for the explanation!
> > 
> > I think one way of approaching the issue would be to synchronize the 
> > process:
> > 
> > 1. Work toward getting approval for the patch (i.e. r-b);
> > 2. With the approval, send a PR to linux-firmware with the necessary 
> > changes;
> > 3. After the linux-firmware PR is merged, the patch could be integraged.
> > 
> > I think that would still apply if going with your proposal on your next 
> > comment.
> > 
> > > 
> > > > >
> > > > > -#define ADLP_DMC_PATHDMC_PATH(adlp, 2, 16)
> > > > > +#define ADLP_DMC_PATHDMC_PATH(adlp)
> > > > > +#define ADLP_DMC_LEGACY_PATH DMC_LEGACY_PATH(adlp, 2, 16)
> > > > >  MODULE_FIRMWARE(ADLP_DMC_PATH);
> > > > >
> > > > > -#define ADLS_DMC_PATHDMC_PATH(adls, 2, 01)
> > > > > +#define ADLS_DMC_PATHDMC_PATH(adls)
> > > > > +#define ADLS_DMC_LEGACY_PATH DMC_LEGACY_PATH(adls, 2, 01)
> > > > >  MODULE_FIRMWARE(ADLS_DMC_PATH);
> > > > >
> > > > > -#define DG1_DMC_PATH DMC_PATH(dg1, 2, 02)
> > > > > +#define DG1_DMC_PATH DMC_PATH(dg1)
> > > > > +#define DG1_DMC_LEGACY_PATH  DMC_LEGACY_PATH(dg1, 2, 02)
> > > > >  MODULE_FIRMWARE(DG1_DMC_PATH);
> > > > >
> > > > > -#define RKL_DMC_PATH DMC_PATH(rkl, 2, 03)
> > > > > +#define RKL_DMC_PATH DMC_PATH(rkl)
> > > > > +#define RKL_DMC_LEGACY_PATH  DMC_LEGACY_PATH(rkl, 2, 03)
> > > > >  MODULE_FIRMWARE(RKL_DMC_PATH);
> > > > >
> > > > > -#define TGL_DMC_PATH DMC_PATH(tgl, 2, 12)
> > > > > +#

Re: [Intel-gfx] [PATCH 2/2] drm/i915/dmc: Use unversioned firmware paths

2022-12-22 Thread Lucas De Marchi

On Thu, Dec 22, 2022 at 07:12:08PM -0300, Gustavo Sousa wrote:

On Wed, Dec 21, 2022 at 04:23:45PM -0800, Lucas De Marchi wrote:

On Wed, Dec 21, 2022 at 12:26:26PM +0200, Jani Nikula wrote:
> On Tue, 20 Dec 2022, Gustavo Sousa  wrote:
> > As we do not require specific versions anymore, change the convention
> > for blob filenames to stop using version numbers. This simplifies code
> > maintenance, since we do not need to keep updating blob paths for new
> > DMC releases, and also makes DMC loading compatible with systems that do
> > not have the latest firmware release.
> >
> > References: https://lore.kernel.org/r/y3081s7c5ksut...@intel.com
> > Signed-off-by: Gustavo Sousa 
> > ---
> >  drivers/gpu/drm/i915/display/intel_dmc.c | 98 
> >  1 file changed, 82 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c 
b/drivers/gpu/drm/i915/display/intel_dmc.c
> > index 4124b3d37110..b11f0f451dd7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> > @@ -42,51 +42,70 @@
> >  #define DMC_VERSION_MAJOR(version)   ((version) >> 16)
> >  #define DMC_VERSION_MINOR(version)   ((version) & 0x)
> >
> > -#define DMC_PATH(platform, major, minor) \
> > - "i915/"\
> > - __stringify(platform) "_dmc_ver" \
> > - __stringify(major) "_" \
> > +#define DMC_PATH(platform) \
> > + "i915/" __stringify(platform) "_dmc.bin"
> > +
> > +/*
> > + * New DMC additions should not use this. This is used solely to remain
> > + * compatible with systems that have not yet updated DMC blobs to use
> > + * unversioned file names.
> > + */
> > +#define DMC_LEGACY_PATH(platform, major, minor) \
> > + "i915/"   \
> > + __stringify(platform) "_dmc_ver"  \
> > + __stringify(major) "_"\
> >   __stringify(minor) ".bin"
> >
> >  #define DISPLAY_VER13_DMC_MAX_FW_SIZE0x2
> >
> >  #define DISPLAY_VER12_DMC_MAX_FW_SIZEICL_DMC_MAX_FW_SIZE
> >
> > -#define DG2_DMC_PATH DMC_PATH(dg2, 2, 08)
> > +#define DG2_DMC_PATH DMC_PATH(dg2)
> > +#define DG2_DMC_LEGACY_PATH  DMC_LEGACY_PATH(dg2, 2, 08)
> >  MODULE_FIRMWARE(DG2_DMC_PATH);

We have an issue here.  Previously `modinfo --field=firmware i915`
would report i915/dg2_dmc_ver2_08.bin. Now it's going to report
i915/dg2_dmc.bin

that modinfo call is what distros use to bundle the firmware blobs in
the initrd. It may also be used for creating package dependendies.

If we do this, unless they have updated their linux-firmware
packages, the initrd will be left without the firmware.
Just checked
git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git main
and we still don't have the unversioned firmware there.


Interesting. Thanks for the explanation!

I think one way of approaching the issue would be to synchronize the process:

1. Work toward getting approval for the patch (i.e. r-b);
2. With the approval, send a PR to linux-firmware with the necessary changes;
3. After the linux-firmware PR is merged, the patch could be integraged.

I think that would still apply if going with your proposal on your next comment.



> >
> > -#define ADLP_DMC_PATHDMC_PATH(adlp, 2, 16)
> > +#define ADLP_DMC_PATHDMC_PATH(adlp)
> > +#define ADLP_DMC_LEGACY_PATH DMC_LEGACY_PATH(adlp, 2, 16)
> >  MODULE_FIRMWARE(ADLP_DMC_PATH);
> >
> > -#define ADLS_DMC_PATHDMC_PATH(adls, 2, 01)
> > +#define ADLS_DMC_PATHDMC_PATH(adls)
> > +#define ADLS_DMC_LEGACY_PATH DMC_LEGACY_PATH(adls, 2, 01)
> >  MODULE_FIRMWARE(ADLS_DMC_PATH);
> >
> > -#define DG1_DMC_PATH DMC_PATH(dg1, 2, 02)
> > +#define DG1_DMC_PATH DMC_PATH(dg1)
> > +#define DG1_DMC_LEGACY_PATH  DMC_LEGACY_PATH(dg1, 2, 02)
> >  MODULE_FIRMWARE(DG1_DMC_PATH);
> >
> > -#define RKL_DMC_PATH DMC_PATH(rkl, 2, 03)
> > +#define RKL_DMC_PATH DMC_PATH(rkl)
> > +#define RKL_DMC_LEGACY_PATH  DMC_LEGACY_PATH(rkl, 2, 03)
> >  MODULE_FIRMWARE(RKL_DMC_PATH);
> >
> > -#define TGL_DMC_PATH DMC_PATH(tgl, 2, 12)
> > +#define TGL_DMC_PATH DMC_PATH(tgl)
> > +#define TGL_DMC_LEGACY_PATH  DMC_LEGACY_PATH(tgl, 2, 12)
> >  MODULE_FIRMWARE(TGL_DMC_PATH);
> >
> > -#define ICL_DMC_PATH DMC_PATH(icl, 1, 09)
> > +#define ICL_DMC_PATH DMC_PATH(icl)
> > +#define ICL_DMC_LEGACY_PATH  DMC_LEGACY_PATH(icl, 1, 09)
> >  #define ICL_DMC_MAX_FW_SIZE  0x6000
> >  MODULE_FIRMWARE(ICL_DMC_PATH);
> >
> > -#define GLK_DMC_PATH DMC_PATH(glk, 1, 04)
> > +#define GLK_DMC_PATH DMC_PATH(glk)
> > +#define GLK_DMC_LEGACY_PATH  DMC_LEGACY_PATH(glk, 1, 04)
> >  #define GLK_DMC_MAX_FW_SIZE

Re: [Intel-gfx] [PATCH 2/2] drm/i915/dmc: Use unversioned firmware paths

2022-12-22 Thread Gustavo Sousa
On Wed, Dec 21, 2022 at 04:23:45PM -0800, Lucas De Marchi wrote:
> On Wed, Dec 21, 2022 at 12:26:26PM +0200, Jani Nikula wrote:
> > On Tue, 20 Dec 2022, Gustavo Sousa  wrote:
> > > As we do not require specific versions anymore, change the convention
> > > for blob filenames to stop using version numbers. This simplifies code
> > > maintenance, since we do not need to keep updating blob paths for new
> > > DMC releases, and also makes DMC loading compatible with systems that do
> > > not have the latest firmware release.
> > > 
> > > References: https://lore.kernel.org/r/y3081s7c5ksut...@intel.com
> > > Signed-off-by: Gustavo Sousa 
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_dmc.c | 98 
> > >  1 file changed, 82 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c 
> > > b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > index 4124b3d37110..b11f0f451dd7 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > @@ -42,51 +42,70 @@
> > >  #define DMC_VERSION_MAJOR(version)   ((version) >> 16)
> > >  #define DMC_VERSION_MINOR(version)   ((version) & 0x)
> > > 
> > > -#define DMC_PATH(platform, major, minor) \
> > > - "i915/"  \
> > > - __stringify(platform) "_dmc_ver" \
> > > - __stringify(major) "_"   \
> > > +#define DMC_PATH(platform) \
> > > + "i915/" __stringify(platform) "_dmc.bin"
> > > +
> > > +/*
> > > + * New DMC additions should not use this. This is used solely to remain
> > > + * compatible with systems that have not yet updated DMC blobs to use
> > > + * unversioned file names.
> > > + */
> > > +#define DMC_LEGACY_PATH(platform, major, minor) \
> > > + "i915/" \
> > > + __stringify(platform) "_dmc_ver"\
> > > + __stringify(major) "_"  \
> > >   __stringify(minor) ".bin"
> > > 
> > >  #define DISPLAY_VER13_DMC_MAX_FW_SIZE0x2
> > > 
> > >  #define DISPLAY_VER12_DMC_MAX_FW_SIZEICL_DMC_MAX_FW_SIZE
> > > 
> > > -#define DG2_DMC_PATH DMC_PATH(dg2, 2, 08)
> > > +#define DG2_DMC_PATH DMC_PATH(dg2)
> > > +#define DG2_DMC_LEGACY_PATH  DMC_LEGACY_PATH(dg2, 2, 08)
> > >  MODULE_FIRMWARE(DG2_DMC_PATH);
> 
> We have an issue here.  Previously `modinfo --field=firmware i915`
> would report i915/dg2_dmc_ver2_08.bin. Now it's going to report
> i915/dg2_dmc.bin
> 
> that modinfo call is what distros use to bundle the firmware blobs in
> the initrd. It may also be used for creating package dependendies.
> 
> If we do this, unless they have updated their linux-firmware
> packages, the initrd will be left without the firmware.
> Just checked
> git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git main
> and we still don't have the unversioned firmware there.

Interesting. Thanks for the explanation!

I think one way of approaching the issue would be to synchronize the process:

1. Work toward getting approval for the patch (i.e. r-b);
2. With the approval, send a PR to linux-firmware with the necessary changes;
3. After the linux-firmware PR is merged, the patch could be integraged.

I think that would still apply if going with your proposal on your next comment.

> 
> > > 
> > > -#define ADLP_DMC_PATHDMC_PATH(adlp, 2, 16)
> > > +#define ADLP_DMC_PATHDMC_PATH(adlp)
> > > +#define ADLP_DMC_LEGACY_PATH DMC_LEGACY_PATH(adlp, 2, 16)
> > >  MODULE_FIRMWARE(ADLP_DMC_PATH);
> > > 
> > > -#define ADLS_DMC_PATHDMC_PATH(adls, 2, 01)
> > > +#define ADLS_DMC_PATHDMC_PATH(adls)
> > > +#define ADLS_DMC_LEGACY_PATH DMC_LEGACY_PATH(adls, 2, 01)
> > >  MODULE_FIRMWARE(ADLS_DMC_PATH);
> > > 
> > > -#define DG1_DMC_PATH DMC_PATH(dg1, 2, 02)
> > > +#define DG1_DMC_PATH DMC_PATH(dg1)
> > > +#define DG1_DMC_LEGACY_PATH  DMC_LEGACY_PATH(dg1, 2, 02)
> > >  MODULE_FIRMWARE(DG1_DMC_PATH);
> > > 
> > > -#define RKL_DMC_PATH DMC_PATH(rkl, 2, 03)
> > > +#define RKL_DMC_PATH DMC_PATH(rkl)
> > > +#define RKL_DMC_LEGACY_PATH  DMC_LEGACY_PATH(rkl, 2, 03)
> > >  MODULE_FIRMWARE(RKL_DMC_PATH);
> > > 
> > > -#define TGL_DMC_PATH DMC_PATH(tgl, 2, 12)
> > > +#define TGL_DMC_PATH DMC_PATH(tgl)
> > > +#define TGL_DMC_LEGACY_PATH  DMC_LEGACY_PATH(tgl, 2, 12)
> > >  MODULE_FIRMWARE(TGL_DMC_PATH);
> > > 
> > > -#define ICL_DMC_PATH DMC_PATH(icl, 1, 09)
> > > +#define ICL_DMC_PATH DMC_PATH(icl)
> > > +#define ICL_DMC_LEGACY_PATH  DMC_LEGACY_PATH(icl, 1, 09)
> > >  #define ICL_DMC_MAX_FW_SIZE  0x6000
> > >  MODULE_FIRMWARE(ICL_DMC_PATH);
> > > 
> > > -#define GLK_DMC_PATH DMC_PATH(glk, 1, 04)
> > > +#define 

Re: [Intel-gfx] [PATCH 2/2] drm/i915/dmc: Use unversioned firmware paths

2022-12-21 Thread Lucas De Marchi

On Wed, Dec 21, 2022 at 12:26:26PM +0200, Jani Nikula wrote:

On Tue, 20 Dec 2022, Gustavo Sousa  wrote:

As we do not require specific versions anymore, change the convention
for blob filenames to stop using version numbers. This simplifies code
maintenance, since we do not need to keep updating blob paths for new
DMC releases, and also makes DMC loading compatible with systems that do
not have the latest firmware release.

References: https://lore.kernel.org/r/y3081s7c5ksut...@intel.com
Signed-off-by: Gustavo Sousa 
---
 drivers/gpu/drm/i915/display/intel_dmc.c | 98 
 1 file changed, 82 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c 
b/drivers/gpu/drm/i915/display/intel_dmc.c
index 4124b3d37110..b11f0f451dd7 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -42,51 +42,70 @@
 #define DMC_VERSION_MAJOR(version) ((version) >> 16)
 #define DMC_VERSION_MINOR(version) ((version) & 0x)

-#define DMC_PATH(platform, major, minor) \
-   "i915/"\
-   __stringify(platform) "_dmc_ver" \
-   __stringify(major) "_" \
+#define DMC_PATH(platform) \
+   "i915/" __stringify(platform) "_dmc.bin"
+
+/*
+ * New DMC additions should not use this. This is used solely to remain
+ * compatible with systems that have not yet updated DMC blobs to use
+ * unversioned file names.
+ */
+#define DMC_LEGACY_PATH(platform, major, minor) \
+   "i915/"   \
+   __stringify(platform) "_dmc_ver"  \
+   __stringify(major) "_"\
__stringify(minor) ".bin"

 #define DISPLAY_VER13_DMC_MAX_FW_SIZE  0x2

 #define DISPLAY_VER12_DMC_MAX_FW_SIZE  ICL_DMC_MAX_FW_SIZE

-#define DG2_DMC_PATH   DMC_PATH(dg2, 2, 08)
+#define DG2_DMC_PATH   DMC_PATH(dg2)
+#define DG2_DMC_LEGACY_PATHDMC_LEGACY_PATH(dg2, 2, 08)
 MODULE_FIRMWARE(DG2_DMC_PATH);


We have an issue here.  Previously `modinfo --field=firmware i915`
would report i915/dg2_dmc_ver2_08.bin. Now it's going to report
i915/dg2_dmc.bin

that modinfo call is what distros use to bundle the firmware blobs in
the initrd. It may also be used for creating package dependendies.

If we do this, unless they have updated their linux-firmware
packages, the initrd will be left without the firmware.
Just checked
git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git main
and we still don't have the unversioned firmware there.



-#define ADLP_DMC_PATH  DMC_PATH(adlp, 2, 16)
+#define ADLP_DMC_PATH  DMC_PATH(adlp)
+#define ADLP_DMC_LEGACY_PATH   DMC_LEGACY_PATH(adlp, 2, 16)
 MODULE_FIRMWARE(ADLP_DMC_PATH);

-#define ADLS_DMC_PATH  DMC_PATH(adls, 2, 01)
+#define ADLS_DMC_PATH  DMC_PATH(adls)
+#define ADLS_DMC_LEGACY_PATH   DMC_LEGACY_PATH(adls, 2, 01)
 MODULE_FIRMWARE(ADLS_DMC_PATH);

-#define DG1_DMC_PATH   DMC_PATH(dg1, 2, 02)
+#define DG1_DMC_PATH   DMC_PATH(dg1)
+#define DG1_DMC_LEGACY_PATHDMC_LEGACY_PATH(dg1, 2, 02)
 MODULE_FIRMWARE(DG1_DMC_PATH);

-#define RKL_DMC_PATH   DMC_PATH(rkl, 2, 03)
+#define RKL_DMC_PATH   DMC_PATH(rkl)
+#define RKL_DMC_LEGACY_PATHDMC_LEGACY_PATH(rkl, 2, 03)
 MODULE_FIRMWARE(RKL_DMC_PATH);

-#define TGL_DMC_PATH   DMC_PATH(tgl, 2, 12)
+#define TGL_DMC_PATH   DMC_PATH(tgl)
+#define TGL_DMC_LEGACY_PATHDMC_LEGACY_PATH(tgl, 2, 12)
 MODULE_FIRMWARE(TGL_DMC_PATH);

-#define ICL_DMC_PATH   DMC_PATH(icl, 1, 09)
+#define ICL_DMC_PATH   DMC_PATH(icl)
+#define ICL_DMC_LEGACY_PATHDMC_LEGACY_PATH(icl, 1, 09)
 #define ICL_DMC_MAX_FW_SIZE0x6000
 MODULE_FIRMWARE(ICL_DMC_PATH);

-#define GLK_DMC_PATH   DMC_PATH(glk, 1, 04)
+#define GLK_DMC_PATH   DMC_PATH(glk)
+#define GLK_DMC_LEGACY_PATHDMC_LEGACY_PATH(glk, 1, 04)
 #define GLK_DMC_MAX_FW_SIZE0x4000
 MODULE_FIRMWARE(GLK_DMC_PATH);

-#define KBL_DMC_PATH   DMC_PATH(kbl, 1, 04)
+#define KBL_DMC_PATH   DMC_PATH(kbl)
+#define KBL_DMC_LEGACY_PATHDMC_LEGACY_PATH(kbl, 1, 04)
 #define KBL_DMC_MAX_FW_SIZEBXT_DMC_MAX_FW_SIZE
 MODULE_FIRMWARE(KBL_DMC_PATH);

-#define SKL_DMC_PATH   DMC_PATH(skl, 1, 27)
+#define SKL_DMC_PATH   DMC_PATH(skl)
+#define SKL_DMC_LEGACY_PATHDMC_LEGACY_PATH(skl, 1, 27)
 #define SKL_DMC_MAX_FW_SIZEBXT_DMC_MAX_FW_SIZE
 MODULE_FIRMWARE(SKL_DMC_PATH);

-#define BXT_DMC_PATH   DMC_PATH(bxt, 1, 07)
+#define BXT_DMC_PATH   DMC_PATH(bxt)
+#define BXT_DMC_LEGACY_PATHDMC_LEGACY_PATH(bxt, 1, 07)
 #define BXT_DMC_MAX_FW_SIZE0x3000
 MO

Re: [Intel-gfx] [PATCH 2/2] drm/i915/dmc: Use unversioned firmware paths

2022-12-21 Thread Gustavo Sousa
On Wed, Dec 21, 2022 at 12:26:26PM +0200, Jani Nikula wrote:
> > -   request_firmware(&fw, dev_priv->display.dmc.fw_path, dev_priv->drm.dev);
> > +   err = firmware_request_nowarn(&fw, dev_priv->display.dmc.fw_path, 
> > dev_priv->drm.dev);
> > +
> > +   if (err == -ENOENT && !dev_priv->params.dmc_firmware_path) {
> > +   legacy_path = dmc_legacy_path(dev_priv);
> > +   if (legacy_path) {
> > +   drm_dbg_kms(&dev_priv->drm,
> > +   "%s not found, falling back to %s\n",
> > +   dmc->fw_path,
> > +   legacy_path);
> > +   err = firmware_request_nowarn(&fw, legacy_path, 
> > dev_priv->drm.dev);
> > +   if (err == 0)
> > +   dev_priv->display.dmc.fw_path = legacy_path;
> > +   }
> > +   }
> > +
> 
> I'd keep the request_firmware() with warnings.

Thanks. Just sent a v2 with that.

--
Gustavo Sousa


Re: [Intel-gfx] [PATCH 2/2] drm/i915/dmc: Use unversioned firmware paths

2022-12-21 Thread Jani Nikula
On Tue, 20 Dec 2022, Gustavo Sousa  wrote:
> As we do not require specific versions anymore, change the convention
> for blob filenames to stop using version numbers. This simplifies code
> maintenance, since we do not need to keep updating blob paths for new
> DMC releases, and also makes DMC loading compatible with systems that do
> not have the latest firmware release.
>
> References: https://lore.kernel.org/r/y3081s7c5ksut...@intel.com
> Signed-off-by: Gustavo Sousa 
> ---
>  drivers/gpu/drm/i915/display/intel_dmc.c | 98 
>  1 file changed, 82 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c 
> b/drivers/gpu/drm/i915/display/intel_dmc.c
> index 4124b3d37110..b11f0f451dd7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> @@ -42,51 +42,70 @@
>  #define DMC_VERSION_MAJOR(version)   ((version) >> 16)
>  #define DMC_VERSION_MINOR(version)   ((version) & 0x)
>  
> -#define DMC_PATH(platform, major, minor) \
> - "i915/"  \
> - __stringify(platform) "_dmc_ver" \
> - __stringify(major) "_"   \
> +#define DMC_PATH(platform) \
> + "i915/" __stringify(platform) "_dmc.bin"
> +
> +/*
> + * New DMC additions should not use this. This is used solely to remain
> + * compatible with systems that have not yet updated DMC blobs to use
> + * unversioned file names.
> + */
> +#define DMC_LEGACY_PATH(platform, major, minor) \
> + "i915/" \
> + __stringify(platform) "_dmc_ver"\
> + __stringify(major) "_"  \
>   __stringify(minor) ".bin"
>  
>  #define DISPLAY_VER13_DMC_MAX_FW_SIZE0x2
>  
>  #define DISPLAY_VER12_DMC_MAX_FW_SIZEICL_DMC_MAX_FW_SIZE
>  
> -#define DG2_DMC_PATH DMC_PATH(dg2, 2, 08)
> +#define DG2_DMC_PATH DMC_PATH(dg2)
> +#define DG2_DMC_LEGACY_PATH  DMC_LEGACY_PATH(dg2, 2, 08)
>  MODULE_FIRMWARE(DG2_DMC_PATH);
>  
> -#define ADLP_DMC_PATHDMC_PATH(adlp, 2, 16)
> +#define ADLP_DMC_PATHDMC_PATH(adlp)
> +#define ADLP_DMC_LEGACY_PATH DMC_LEGACY_PATH(adlp, 2, 16)
>  MODULE_FIRMWARE(ADLP_DMC_PATH);
>  
> -#define ADLS_DMC_PATHDMC_PATH(adls, 2, 01)
> +#define ADLS_DMC_PATHDMC_PATH(adls)
> +#define ADLS_DMC_LEGACY_PATH DMC_LEGACY_PATH(adls, 2, 01)
>  MODULE_FIRMWARE(ADLS_DMC_PATH);
>  
> -#define DG1_DMC_PATH DMC_PATH(dg1, 2, 02)
> +#define DG1_DMC_PATH DMC_PATH(dg1)
> +#define DG1_DMC_LEGACY_PATH  DMC_LEGACY_PATH(dg1, 2, 02)
>  MODULE_FIRMWARE(DG1_DMC_PATH);
>  
> -#define RKL_DMC_PATH DMC_PATH(rkl, 2, 03)
> +#define RKL_DMC_PATH DMC_PATH(rkl)
> +#define RKL_DMC_LEGACY_PATH  DMC_LEGACY_PATH(rkl, 2, 03)
>  MODULE_FIRMWARE(RKL_DMC_PATH);
>  
> -#define TGL_DMC_PATH DMC_PATH(tgl, 2, 12)
> +#define TGL_DMC_PATH DMC_PATH(tgl)
> +#define TGL_DMC_LEGACY_PATH  DMC_LEGACY_PATH(tgl, 2, 12)
>  MODULE_FIRMWARE(TGL_DMC_PATH);
>  
> -#define ICL_DMC_PATH DMC_PATH(icl, 1, 09)
> +#define ICL_DMC_PATH DMC_PATH(icl)
> +#define ICL_DMC_LEGACY_PATH  DMC_LEGACY_PATH(icl, 1, 09)
>  #define ICL_DMC_MAX_FW_SIZE  0x6000
>  MODULE_FIRMWARE(ICL_DMC_PATH);
>  
> -#define GLK_DMC_PATH DMC_PATH(glk, 1, 04)
> +#define GLK_DMC_PATH DMC_PATH(glk)
> +#define GLK_DMC_LEGACY_PATH  DMC_LEGACY_PATH(glk, 1, 04)
>  #define GLK_DMC_MAX_FW_SIZE  0x4000
>  MODULE_FIRMWARE(GLK_DMC_PATH);
>  
> -#define KBL_DMC_PATH DMC_PATH(kbl, 1, 04)
> +#define KBL_DMC_PATH DMC_PATH(kbl)
> +#define KBL_DMC_LEGACY_PATH  DMC_LEGACY_PATH(kbl, 1, 04)
>  #define KBL_DMC_MAX_FW_SIZE  BXT_DMC_MAX_FW_SIZE
>  MODULE_FIRMWARE(KBL_DMC_PATH);
>  
> -#define SKL_DMC_PATH DMC_PATH(skl, 1, 27)
> +#define SKL_DMC_PATH DMC_PATH(skl)
> +#define SKL_DMC_LEGACY_PATH  DMC_LEGACY_PATH(skl, 1, 27)
>  #define SKL_DMC_MAX_FW_SIZE  BXT_DMC_MAX_FW_SIZE
>  MODULE_FIRMWARE(SKL_DMC_PATH);
>  
> -#define BXT_DMC_PATH DMC_PATH(bxt, 1, 07)
> +#define BXT_DMC_PATH DMC_PATH(bxt)
> +#define BXT_DMC_LEGACY_PATH  DMC_LEGACY_PATH(bxt, 1, 07)
>  #define BXT_DMC_MAX_FW_SIZE  0x3000
>  MODULE_FIRMWARE(BXT_DMC_PATH);
>  
> @@ -821,16 +840,63 @@ static void intel_dmc_runtime_pm_put(struct 
> drm_i915_private *dev_priv)
>   intel_display_power_put(dev_priv, POWER_DOMAIN_INIT, wakeref);
>  }
>  
> +static const char *dmc_legacy_path(struct drm_i915_private *i915)
> +{
> + if (IS_DG2(i915)) {
> + return DG2_DMC_LEGACY_PATH;
> + } else if (IS_ALDERLAKE_P(i915)) {
> + return ADLP_DMC_LEGACY_PATH;
> + } else if (IS_ALDERLAKE_