diff mbox series

drm/i915/dmc: Let's abstract the dmc path.

Message ID 20210421094406.2017733-1-rodrigo.vivi@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/dmc: Let's abstract the dmc path. | expand

Commit Message

Rodrigo Vivi April 21, 2021, 9:44 a.m. UTC
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.

v2: define DMC_PATH inside .c (Lucas)

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>
Reviewed-by: José Roberto de Souza <jose.souza@intel.com> #v1
---
 drivers/gpu/drm/i915/display/intel_csr.c | 26 +++++++++++++++---------
 1 file changed, 16 insertions(+), 10 deletions(-)

Comments

Jani Nikula April 21, 2021, 12:18 p.m. UTC | #1
On Wed, 21 Apr 2021, 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.
>
> v2: define DMC_PATH inside .c (Lucas)
>
> 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>
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com> #v1

Nice,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

On a related note, should we finally consider a s/csr/dmc/g rename all
over the place? It's just confusing to keep using two names.


> ---
>  drivers/gpu/drm/i915/display/intel_csr.c | 26 +++++++++++++++---------
>  1 file 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..26a922d34263 100644
> --- a/drivers/gpu/drm/i915/display/intel_csr.c
> +++ b/drivers/gpu/drm/i915/display/intel_csr.c
> @@ -38,50 +38,56 @@
>   * low-power state and comes back to normal.
>   */
>  
> +#define DMC_PATH(platform, major, minor) \
> +	"i915/"				 \
> +	__stringify(platform) "_dmc_ver" \
> +	__stringify(major) "_"		 \
> +	__stringify(minor) ".bin"
> +
>  #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);
Lucas De Marchi April 21, 2021, 2:57 p.m. UTC | #2
On Wed, Apr 21, 2021 at 05:44:06AM -0400, Rodrigo Vivi 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.
>
>v2: define DMC_PATH inside .c (Lucas)
>
>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>
>Reviewed-by: José Roberto de Souza <jose.souza@intel.com> #v1


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>


Lucas De Marchi

>---
> drivers/gpu/drm/i915/display/intel_csr.c | 26 +++++++++++++++---------
> 1 file 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..26a922d34263 100644
>--- a/drivers/gpu/drm/i915/display/intel_csr.c
>+++ b/drivers/gpu/drm/i915/display/intel_csr.c
>@@ -38,50 +38,56 @@
>  * low-power state and comes back to normal.
>  */
>
>+#define DMC_PATH(platform, major, minor) \
>+	"i915/"				 \
>+	__stringify(platform) "_dmc_ver" \
>+	__stringify(major) "_"		 \
>+	__stringify(minor) ".bin"
>+
> #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);
>-- 
>2.30.2
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Lucas De Marchi April 21, 2021, 2:58 p.m. UTC | #3
On Wed, Apr 21, 2021 at 03:18:17PM +0300, Jani Nikula wrote:
>On Wed, 21 Apr 2021, 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.
>>
>> v2: define DMC_PATH inside .c (Lucas)
>>
>> 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>
>> Reviewed-by: José Roberto de Souza <jose.souza@intel.com> #v1
>
>Nice,
>
>Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
>On a related note, should we finally consider a s/csr/dmc/g rename all
>over the place? It's just confusing to keep using two names.

+1 

But maybe we need a comment about dmc vs csr use, because spec uses
both.

Lucas De Marchi


>
>
>> ---
>>  drivers/gpu/drm/i915/display/intel_csr.c | 26 +++++++++++++++---------
>>  1 file 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..26a922d34263 100644
>> --- a/drivers/gpu/drm/i915/display/intel_csr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_csr.c
>> @@ -38,50 +38,56 @@
>>   * low-power state and comes back to normal.
>>   */
>>
>> +#define DMC_PATH(platform, major, minor) \
>> +	"i915/"				 \
>> +	__stringify(platform) "_dmc_ver" \
>> +	__stringify(major) "_"		 \
>> +	__stringify(minor) ".bin"
>> +
>>  #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);
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_csr.c b/drivers/gpu/drm/i915/display/intel_csr.c
index 26a3c6787e9e..26a922d34263 100644
--- a/drivers/gpu/drm/i915/display/intel_csr.c
+++ b/drivers/gpu/drm/i915/display/intel_csr.c
@@ -38,50 +38,56 @@ 
  * low-power state and comes back to normal.
  */
 
+#define DMC_PATH(platform, major, minor) \
+	"i915/"				 \
+	__stringify(platform) "_dmc_ver" \
+	__stringify(major) "_"		 \
+	__stringify(minor) ".bin"
+
 #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);