Re: [Intel-gfx] [PATCH] drm/i915/dmc: Let's abstract the dmc path.

From: Lucas De Marchi <lucas.de.marchi@gmail.com>
To: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Intel Graphics <intel-gfx@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH] drm/i915/dmc: Let's abstract the dmc path.
Date: Tue, 20 Apr 2021 13:40:08 -0700	[thread overview]
Message-ID: <CAKi4VA+Rn+XNJNooWGrve+9+07fo7_gDKZD4j0gz3K_LdvMZ-g@mail.gmail.com> (raw)
In-Reply-To: <20210420191226.1930570-1-rodrigo.vivi@intel.com>

On Tue, Apr 20, 2021 at 12:12 PM Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>
> Although this abstraction removes the convenience of grepping
> for the file name, it:
> - makes addition easier.
> - makes it easier to tweak global path when experiments are needed.
> - get in sync with guc/huc, without getting overly abstracted.
> - allows future junction with CSR_VERSION for simplicity.
> - Enforces dmc file will never change this standard.
>
> Cc: Fei Yang <fei.yang@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_csr.c | 20 ++++++++++----------
>  drivers/gpu/drm/i915/display/intel_csr.h |  6 ++++++
>  2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_csr.c b/drivers/gpu/drm/i915/display/intel_csr.c
> index 26a3c6787e9e..ad68bcc611c4 100644
> --- a/drivers/gpu/drm/i915/display/intel_csr.c
> +++ b/drivers/gpu/drm/i915/display/intel_csr.c
> @@ -40,48 +40,48 @@
>
>  #define GEN12_CSR_MAX_FW_SIZE          ICL_CSR_MAX_FW_SIZE
>
> -#define ADLS_CSR_PATH                  "i915/adls_dmc_ver2_01.bin"
> +#define ADLS_CSR_PATH                  DMC_PATH(adls, 2, 01)
>  #define ADLS_CSR_VERSION_REQUIRED      CSR_VERSION(2, 1)
>  MODULE_FIRMWARE(ADLS_CSR_PATH);
>
> -#define DG1_CSR_PATH                   "i915/dg1_dmc_ver2_02.bin"
> +#define DG1_CSR_PATH                   DMC_PATH(dg1, 2, 02)
>  #define DG1_CSR_VERSION_REQUIRED       CSR_VERSION(2, 2)
>  MODULE_FIRMWARE(DG1_CSR_PATH);
>
> -#define RKL_CSR_PATH                   "i915/rkl_dmc_ver2_02.bin"
> +#define RKL_CSR_PATH                   DMC_PATH(rkl, 2, 02)
>  #define RKL_CSR_VERSION_REQUIRED       CSR_VERSION(2, 2)
>  MODULE_FIRMWARE(RKL_CSR_PATH);
>
> -#define TGL_CSR_PATH                   "i915/tgl_dmc_ver2_08.bin"
> +#define TGL_CSR_PATH                   DMC_PATH(tgl, 2, 08)
>  #define TGL_CSR_VERSION_REQUIRED       CSR_VERSION(2, 8)
>  MODULE_FIRMWARE(TGL_CSR_PATH);
>
> -#define ICL_CSR_PATH                   "i915/icl_dmc_ver1_09.bin"
> +#define ICL_CSR_PATH                   DMC_PATH(icl, 1, 09)
>  #define ICL_CSR_VERSION_REQUIRED       CSR_VERSION(1, 9)
>  #define ICL_CSR_MAX_FW_SIZE            0x6000
>  MODULE_FIRMWARE(ICL_CSR_PATH);
>
> -#define CNL_CSR_PATH                   "i915/cnl_dmc_ver1_07.bin"
> +#define CNL_CSR_PATH                   DMC_PATH(cnl, 1, 07)
>  #define CNL_CSR_VERSION_REQUIRED       CSR_VERSION(1, 7)
>  #define CNL_CSR_MAX_FW_SIZE            GLK_CSR_MAX_FW_SIZE
>  MODULE_FIRMWARE(CNL_CSR_PATH);
>
> -#define GLK_CSR_PATH                   "i915/glk_dmc_ver1_04.bin"
> +#define GLK_CSR_PATH                   DMC_PATH(glk, 1, 04)
>  #define GLK_CSR_VERSION_REQUIRED       CSR_VERSION(1, 4)
>  #define GLK_CSR_MAX_FW_SIZE            0x4000
>  MODULE_FIRMWARE(GLK_CSR_PATH);
>
> -#define KBL_CSR_PATH                   "i915/kbl_dmc_ver1_04.bin"
> +#define KBL_CSR_PATH                   DMC_PATH(kbl, 1, 04)
>  #define KBL_CSR_VERSION_REQUIRED       CSR_VERSION(1, 4)
>  #define KBL_CSR_MAX_FW_SIZE            BXT_CSR_MAX_FW_SIZE
>  MODULE_FIRMWARE(KBL_CSR_PATH);
>
> -#define SKL_CSR_PATH                   "i915/skl_dmc_ver1_27.bin"
> +#define SKL_CSR_PATH                   DMC_PATH(skl, 1, 27)
>  #define SKL_CSR_VERSION_REQUIRED       CSR_VERSION(1, 27)
>  #define SKL_CSR_MAX_FW_SIZE            BXT_CSR_MAX_FW_SIZE
>  MODULE_FIRMWARE(SKL_CSR_PATH);
>
> -#define BXT_CSR_PATH                   "i915/bxt_dmc_ver1_07.bin"
> +#define BXT_CSR_PATH                   DMC_PATH(bxt, 1, 07)
>  #define BXT_CSR_VERSION_REQUIRED       CSR_VERSION(1, 7)
>  #define BXT_CSR_MAX_FW_SIZE            0x3000
>  MODULE_FIRMWARE(BXT_CSR_PATH);
> diff --git a/drivers/gpu/drm/i915/display/intel_csr.h b/drivers/gpu/drm/i915/display/intel_csr.h
> index 03c64f8af7ab..fbfdaa3df07e 100644
> --- a/drivers/gpu/drm/i915/display/intel_csr.h
> +++ b/drivers/gpu/drm/i915/display/intel_csr.h
> @@ -8,6 +8,12 @@
>
>  struct drm_i915_private;
>
> +#define DMC_PATH(platform, major, minor) \
> +       "i915/"                          \
> +       __stringify(platform) "_dmc_ver" \
> +       __stringify(major) "_"           \
> +       __stringify(minor) ".bin"

contrary to the macros below that are used in other places, I don't
expect DMC_PATH to be ever used outside intel_csr.c.
Why are you adding it to the header?

thanks
Lucas De Marchi

> +
>  #define CSR_VERSION(major, minor)      ((major) << 16 | (minor))
>  #define CSR_VERSION_MAJOR(version)     ((version) >> 16)
>  #define CSR_VERSION_MINOR(version)     ((version) & 0xffff)
> --
> 2.30.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx