| Message ID | 20221229190740.45471-3-gustavo.sousa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/dmc: Make firmware loading backwards-compatible | expand |
On Thu, Dec 29, 2022 at 04:07:40PM -0300, Gustavo Sousa wrote: > New DMC releases in linux-firmware will stop using version number in > blob filenames. This new convention provides the following benefits: > > 1. It simplifies code maintenance, as new DMC releases for a platform > using the new convention will always use the same filename for the > blob. > > 2. It allows DMC to be loaded even if the target system does not have > the most recent firmware installed. > > Prepare the driver by: > > - Using the new convention for DMC_PATH() and renaming the currently > used one to make it clear it is for the legacy scheme. > > - Implementing a fallback mechanism for future transitions from > versioned to unversioned paths so that we do not cause a regression > for systems not having the most up-to-date linux-firmware files. > > v2: > - Keep using request_firmware() instead of firmware_request_nowarn(). > (Jani) > v3: > - Keep current DMC paths instead of directly using unversioned ones, > so that we do not disturb initrd generation. > (Lucas, Rodrigo) > > References: https://lore.kernel.org/r/Y3081s7c5ksutpMW@intel.com I also don't believe this link is a good reference here... Regarding the patch, I liked the approach in general. The patch is really neat, but I believe we will need to split it: 1. Add the new DMC_PATH and DMC_LEGACY_PATH only with the introduction of the mtl_dmc.bin 2. And the fallback function, add only if/when we get a real fallback. Oh, and I just realized that when doing the new _dmc.bin path we also need to make sure that we read the fw_version from the header and print as a drm_info msg somewhere. > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dmc.c | 62 ++++++++++++++++++------ > 1 file changed, 46 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..12f05b2d33a3 100644 > --- a/drivers/gpu/drm/i915/display/intel_dmc.c > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c > @@ -42,51 +42,59 @@ > #define DMC_VERSION_MAJOR(version) ((version) >> 16) > #define DMC_VERSION_MINOR(version) ((version) & 0xffff) > > -#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 0x20000 > > #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_LEGACY_PATH(dg2, 2, 08) > MODULE_FIRMWARE(DG2_DMC_PATH); > > -#define ADLP_DMC_PATH DMC_PATH(adlp, 2, 16) > +#define ADLP_DMC_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_LEGACY_PATH(adls, 2, 01) > MODULE_FIRMWARE(ADLS_DMC_PATH); > > -#define DG1_DMC_PATH DMC_PATH(dg1, 2, 02) > +#define DG1_DMC_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_LEGACY_PATH(rkl, 2, 03) > MODULE_FIRMWARE(RKL_DMC_PATH); > > -#define TGL_DMC_PATH DMC_PATH(tgl, 2, 12) > +#define TGL_DMC_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_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_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_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_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_LEGACY_PATH(bxt, 1, 07) > #define BXT_DMC_MAX_FW_SIZE 0x3000 > MODULE_FIRMWARE(BXT_DMC_PATH); > > @@ -821,16 +829,38 @@ 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_fallback_path(struct drm_i915_private *i915) > +{ > + /* No fallback paths for now. */ > + return NULL; > +} > + > static void dmc_load_work_fn(struct work_struct *work) > { > struct drm_i915_private *dev_priv; > struct intel_dmc *dmc; > const struct firmware *fw = NULL; > + const char *fallback_path; > + int err; > > dev_priv = container_of(work, typeof(*dev_priv), display.dmc.work); > dmc = &dev_priv->display.dmc; > > - request_firmware(&fw, dev_priv->display.dmc.fw_path, dev_priv->drm.dev); > + err = request_firmware(&fw, dev_priv->display.dmc.fw_path, dev_priv->drm.dev); > + > + if (err == -ENOENT && !dev_priv->params.dmc_firmware_path) { > + fallback_path = dmc_fallback_path(dev_priv); > + if (fallback_path) { > + drm_dbg_kms(&dev_priv->drm, > + "%s not found, falling back to %s\n", > + dmc->fw_path, > + fallback_path); > + err = request_firmware(&fw, fallback_path, dev_priv->drm.dev); > + if (err == 0) > + dev_priv->display.dmc.fw_path = fallback_path; > + } > + } > + > parse_dmc_fw(dev_priv, fw); > > if (intel_dmc_has_payload(dev_priv)) { > -- > 2.39.0 >
On Fri, Dec 30, 2022 at 03:47:43AM -0500, Rodrigo Vivi wrote: > On Thu, Dec 29, 2022 at 04:07:40PM -0300, Gustavo Sousa wrote: > > New DMC releases in linux-firmware will stop using version number in > > blob filenames. This new convention provides the following benefits: > > > > 1. It simplifies code maintenance, as new DMC releases for a platform > > using the new convention will always use the same filename for the > > blob. > > > > 2. It allows DMC to be loaded even if the target system does not have > > the most recent firmware installed. > > > > Prepare the driver by: > > > > - Using the new convention for DMC_PATH() and renaming the currently > > used one to make it clear it is for the legacy scheme. > > > > - Implementing a fallback mechanism for future transitions from > > versioned to unversioned paths so that we do not cause a regression > > for systems not having the most up-to-date linux-firmware files. > > > > v2: > > - Keep using request_firmware() instead of firmware_request_nowarn(). > > (Jani) > > v3: > > - Keep current DMC paths instead of directly using unversioned ones, > > so that we do not disturb initrd generation. > > (Lucas, Rodrigo) > > > > References: https://lore.kernel.org/r/Y3081s7c5ksutpMW@intel.com > > I also don't believe this link is a good reference here... Noted. > > Regarding the patch, I liked the approach in general. > > The patch is really neat, but I believe we will need to split it: > > 1. Add the new DMC_PATH and DMC_LEGACY_PATH only with the introduction > of the mtl_dmc.bin > > 2. And the fallback function, add only if/when we get a real fallback. Okay. For future reference, how should that be implemented with respect to the organization of the patches? I see two ways of doing it and have a personal preference for the first one: a) The future series would have first a patch adding the necessary functionality and a second one using it. b) The addition of the functionality would be incorporated in the same patch using it. For example, for (2), (a) would be a series two patches, the first adding the fallback mechanism and the second one changing ADLP to use unversioned paths; and (b) would be all of that in a single patch. I looks to me that approach (b) has a potential issue. For example, let's say we in the future we decide to revert that specific firmware update but we already have other platforms also using the fallback - a clean revert is not possible there and we would need to make sure that the fallback mechanism is kept. That's why I like (a) more and I think (b) would be more appropriate for cases where the functionality and it's user(s) have almost like a "1:1" relationship (not strictly speaking, read that as "having a somewhat static and restricted set of users"). > > Oh, and I just realized that when doing the new _dmc.bin path we also > need to make sure that we read the fw_version from the header and > print as a drm_info msg somewhere. I think the version is already being read by parse_dmc_fw_css() and printed at the end of dmc_load_work_fn() if the blob was loaded and parsed succesfully. > > > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> > > Cc: Jani Nikula <jani.nikula@intel.com> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_dmc.c | 62 ++++++++++++++++++------ > > 1 file changed, 46 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..12f05b2d33a3 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dmc.c > > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c > > @@ -42,51 +42,59 @@ > > #define DMC_VERSION_MAJOR(version) ((version) >> 16) > > #define DMC_VERSION_MINOR(version) ((version) & 0xffff) > > > > -#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 0x20000 > > > > #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_LEGACY_PATH(dg2, 2, 08) > > MODULE_FIRMWARE(DG2_DMC_PATH); > > > > -#define ADLP_DMC_PATH DMC_PATH(adlp, 2, 16) > > +#define ADLP_DMC_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_LEGACY_PATH(adls, 2, 01) > > MODULE_FIRMWARE(ADLS_DMC_PATH); > > > > -#define DG1_DMC_PATH DMC_PATH(dg1, 2, 02) > > +#define DG1_DMC_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_LEGACY_PATH(rkl, 2, 03) > > MODULE_FIRMWARE(RKL_DMC_PATH); > > > > -#define TGL_DMC_PATH DMC_PATH(tgl, 2, 12) > > +#define TGL_DMC_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_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_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_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_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_LEGACY_PATH(bxt, 1, 07) > > #define BXT_DMC_MAX_FW_SIZE 0x3000 > > MODULE_FIRMWARE(BXT_DMC_PATH); > > > > @@ -821,16 +829,38 @@ 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_fallback_path(struct drm_i915_private *i915) > > +{ > > + /* No fallback paths for now. */ > > + return NULL; > > +} > > + > > static void dmc_load_work_fn(struct work_struct *work) > > { > > struct drm_i915_private *dev_priv; > > struct intel_dmc *dmc; > > const struct firmware *fw = NULL; > > + const char *fallback_path; > > + int err; > > > > dev_priv = container_of(work, typeof(*dev_priv), display.dmc.work); > > dmc = &dev_priv->display.dmc; > > > > - request_firmware(&fw, dev_priv->display.dmc.fw_path, dev_priv->drm.dev); > > + err = request_firmware(&fw, dev_priv->display.dmc.fw_path, dev_priv->drm.dev); > > + > > + if (err == -ENOENT && !dev_priv->params.dmc_firmware_path) { > > + fallback_path = dmc_fallback_path(dev_priv); > > + if (fallback_path) { > > + drm_dbg_kms(&dev_priv->drm, > > + "%s not found, falling back to %s\n", > > + dmc->fw_path, > > + fallback_path); > > + err = request_firmware(&fw, fallback_path, dev_priv->drm.dev); > > + if (err == 0) > > + dev_priv->display.dmc.fw_path = fallback_path; > > + } > > + } > > + > > parse_dmc_fw(dev_priv, fw); > > > > if (intel_dmc_has_payload(dev_priv)) { > > -- > > 2.39.0 > >
On Fri, Dec 30, 2022 at 10:36:28AM -0300, Gustavo Sousa wrote: > On Fri, Dec 30, 2022 at 03:47:43AM -0500, Rodrigo Vivi wrote: > > On Thu, Dec 29, 2022 at 04:07:40PM -0300, Gustavo Sousa wrote: > > > New DMC releases in linux-firmware will stop using version number in > > > blob filenames. This new convention provides the following benefits: > > > > > > 1. It simplifies code maintenance, as new DMC releases for a platform > > > using the new convention will always use the same filename for the > > > blob. > > > > > > 2. It allows DMC to be loaded even if the target system does not have > > > the most recent firmware installed. > > > > > > Prepare the driver by: > > > > > > - Using the new convention for DMC_PATH() and renaming the currently > > > used one to make it clear it is for the legacy scheme. > > > > > > - Implementing a fallback mechanism for future transitions from > > > versioned to unversioned paths so that we do not cause a regression > > > for systems not having the most up-to-date linux-firmware files. > > > > > > v2: > > > - Keep using request_firmware() instead of firmware_request_nowarn(). > > > (Jani) > > > v3: > > > - Keep current DMC paths instead of directly using unversioned ones, > > > so that we do not disturb initrd generation. > > > (Lucas, Rodrigo) > > > > > > References: https://lore.kernel.org/r/Y3081s7c5ksutpMW@intel.com > > > > I also don't believe this link is a good reference here... > > Noted. > > > > > Regarding the patch, I liked the approach in general. > > > > The patch is really neat, but I believe we will need to split it: > > > > 1. Add the new DMC_PATH and DMC_LEGACY_PATH only with the introduction > > of the mtl_dmc.bin > > > > 2. And the fallback function, add only if/when we get a real fallback. > > Okay. For future reference, how should that be implemented with respect to the > organization of the patches? I see two ways of doing it and have a personal > preference for the first one: > > a) The future series would have first a patch adding the necessary functionality > and a second one using it. > > b) The addition of the functionality would be incorporated in the same patch > using it. > > For example, for (2), (a) would be a series two patches, the first adding the > fallback mechanism and the second one changing ADLP to use unversioned paths; > and (b) would be all of that in a single patch. > > I looks to me that approach (b) has a potential issue. For example, let's say we > in the future we decide to revert that specific firmware update but we already > have other platforms also using the fallback - a clean revert is not possible > there and we would need to make sure that the fallback mechanism is kept. > > That's why I like (a) more and I think (b) would be more appropriate for cases > where the functionality and it's user(s) have almost like a "1:1" relationship > (not strictly speaking, read that as "having a somewhat static and restricted > set of users"). yeap, it is case by case. The advantage on the (b) approach is that OSVs can backport only 1 patch. And the revert doesn't necessarily need to be a git-revert. In this case it would be only one line getting back to the older firmware. But really up to you ;) As long as they are together either in the same patch or same series. > > > > > Oh, and I just realized that when doing the new _dmc.bin path we also > > need to make sure that we read the fw_version from the header and > > print as a drm_info msg somewhere. > > I think the version is already being read by parse_dmc_fw_css() and printed at > the end of dmc_load_work_fn() if the blob was loaded and parsed succesfully. Oh, I had missed that. If it is already happening please disregard my comment. > > > > > > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> > > > Cc: Jani Nikula <jani.nikula@intel.com> > > > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_dmc.c | 62 ++++++++++++++++++------ > > > 1 file changed, 46 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..12f05b2d33a3 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dmc.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c > > > @@ -42,51 +42,59 @@ > > > #define DMC_VERSION_MAJOR(version) ((version) >> 16) > > > #define DMC_VERSION_MINOR(version) ((version) & 0xffff) > > > > > > -#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 0x20000 > > > > > > #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_LEGACY_PATH(dg2, 2, 08) > > > MODULE_FIRMWARE(DG2_DMC_PATH); > > > > > > -#define ADLP_DMC_PATH DMC_PATH(adlp, 2, 16) > > > +#define ADLP_DMC_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_LEGACY_PATH(adls, 2, 01) > > > MODULE_FIRMWARE(ADLS_DMC_PATH); > > > > > > -#define DG1_DMC_PATH DMC_PATH(dg1, 2, 02) > > > +#define DG1_DMC_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_LEGACY_PATH(rkl, 2, 03) > > > MODULE_FIRMWARE(RKL_DMC_PATH); > > > > > > -#define TGL_DMC_PATH DMC_PATH(tgl, 2, 12) > > > +#define TGL_DMC_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_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_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_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_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_LEGACY_PATH(bxt, 1, 07) > > > #define BXT_DMC_MAX_FW_SIZE 0x3000 > > > MODULE_FIRMWARE(BXT_DMC_PATH); > > > > > > @@ -821,16 +829,38 @@ 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_fallback_path(struct drm_i915_private *i915) > > > +{ > > > + /* No fallback paths for now. */ > > > + return NULL; > > > +} > > > + > > > static void dmc_load_work_fn(struct work_struct *work) > > > { > > > struct drm_i915_private *dev_priv; > > > struct intel_dmc *dmc; > > > const struct firmware *fw = NULL; > > > + const char *fallback_path; > > > + int err; > > > > > > dev_priv = container_of(work, typeof(*dev_priv), display.dmc.work); > > > dmc = &dev_priv->display.dmc; > > > > > > - request_firmware(&fw, dev_priv->display.dmc.fw_path, dev_priv->drm.dev); > > > + err = request_firmware(&fw, dev_priv->display.dmc.fw_path, dev_priv->drm.dev); > > > + > > > + if (err == -ENOENT && !dev_priv->params.dmc_firmware_path) { > > > + fallback_path = dmc_fallback_path(dev_priv); > > > + if (fallback_path) { > > > + drm_dbg_kms(&dev_priv->drm, > > > + "%s not found, falling back to %s\n", > > > + dmc->fw_path, > > > + fallback_path); > > > + err = request_firmware(&fw, fallback_path, dev_priv->drm.dev); > > > + if (err == 0) > > > + dev_priv->display.dmc.fw_path = fallback_path; > > > + } > > > + } > > > + > > > parse_dmc_fw(dev_priv, fw); > > > > > > if (intel_dmc_has_payload(dev_priv)) { > > > -- > > > 2.39.0 > > >
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c index 4124b3d37110..12f05b2d33a3 100644 --- a/drivers/gpu/drm/i915/display/intel_dmc.c +++ b/drivers/gpu/drm/i915/display/intel_dmc.c @@ -42,51 +42,59 @@ #define DMC_VERSION_MAJOR(version) ((version) >> 16) #define DMC_VERSION_MINOR(version) ((version) & 0xffff) -#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 0x20000 #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_LEGACY_PATH(dg2, 2, 08) MODULE_FIRMWARE(DG2_DMC_PATH); -#define ADLP_DMC_PATH DMC_PATH(adlp, 2, 16) +#define ADLP_DMC_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_LEGACY_PATH(adls, 2, 01) MODULE_FIRMWARE(ADLS_DMC_PATH); -#define DG1_DMC_PATH DMC_PATH(dg1, 2, 02) +#define DG1_DMC_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_LEGACY_PATH(rkl, 2, 03) MODULE_FIRMWARE(RKL_DMC_PATH); -#define TGL_DMC_PATH DMC_PATH(tgl, 2, 12) +#define TGL_DMC_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_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_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_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_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_LEGACY_PATH(bxt, 1, 07) #define BXT_DMC_MAX_FW_SIZE 0x3000 MODULE_FIRMWARE(BXT_DMC_PATH); @@ -821,16 +829,38 @@ 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_fallback_path(struct drm_i915_private *i915) +{ + /* No fallback paths for now. */ + return NULL; +} + static void dmc_load_work_fn(struct work_struct *work) { struct drm_i915_private *dev_priv; struct intel_dmc *dmc; const struct firmware *fw = NULL; + const char *fallback_path; + int err; dev_priv = container_of(work, typeof(*dev_priv), display.dmc.work); dmc = &dev_priv->display.dmc; - request_firmware(&fw, dev_priv->display.dmc.fw_path, dev_priv->drm.dev); + err = request_firmware(&fw, dev_priv->display.dmc.fw_path, dev_priv->drm.dev); + + if (err == -ENOENT && !dev_priv->params.dmc_firmware_path) { + fallback_path = dmc_fallback_path(dev_priv); + if (fallback_path) { + drm_dbg_kms(&dev_priv->drm, + "%s not found, falling back to %s\n", + dmc->fw_path, + fallback_path); + err = request_firmware(&fw, fallback_path, dev_priv->drm.dev); + if (err == 0) + dev_priv->display.dmc.fw_path = fallback_path; + } + } + parse_dmc_fw(dev_priv, fw); if (intel_dmc_has_payload(dev_priv)) {
New DMC releases in linux-firmware will stop using version number in blob filenames. This new convention provides the following benefits: 1. It simplifies code maintenance, as new DMC releases for a platform using the new convention will always use the same filename for the blob. 2. It allows DMC to be loaded even if the target system does not have the most recent firmware installed. Prepare the driver by: - Using the new convention for DMC_PATH() and renaming the currently used one to make it clear it is for the legacy scheme. - Implementing a fallback mechanism for future transitions from versioned to unversioned paths so that we do not cause a regression for systems not having the most up-to-date linux-firmware files. v2: - Keep using request_firmware() instead of firmware_request_nowarn(). (Jani) v3: - Keep current DMC paths instead of directly using unversioned ones, so that we do not disturb initrd generation. (Lucas, Rodrigo) References: https://lore.kernel.org/r/Y3081s7c5ksutpMW@intel.com Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/display/intel_dmc.c | 62 ++++++++++++++++++------ 1 file changed, 46 insertions(+), 16 deletions(-)