Re: [PATCH 1/3] drm/v3d: fix up register addresses for V3D 7.x
On 2023-09-28 15:03, Maira Canal wrote: > Hi Iago, > > On 9/28/23 08:45, Iago Toral Quiroga wrote: > > Please, add a commit message and your s-o-b to the patch. Here is a reference > on how to format your patches [1]. > > Also, please, run checkpatch on this patch and address the warnings. > > [1] https://docs.kernel.org/process/submitting-patches.html > >> --- >> drivers/gpu/drm/v3d/v3d_debugfs.c | 173 +- >> drivers/gpu/drm/v3d/v3d_gem.c | 3 + >> drivers/gpu/drm/v3d/v3d_irq.c | 47 >> drivers/gpu/drm/v3d/v3d_regs.h| 51 - >> drivers/gpu/drm/v3d/v3d_sched.c | 41 --- >> 5 files changed, 200 insertions(+), 115 deletions(-) >> >> diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c >> b/drivers/gpu/drm/v3d/v3d_debugfs.c >> index 330669f51fa7..90b2b5b2710c 100644 >> --- a/drivers/gpu/drm/v3d/v3d_debugfs.c >> +++ b/drivers/gpu/drm/v3d/v3d_debugfs.c >> @@ -12,69 +12,83 @@ >> #include "v3d_drv.h" >> #include "v3d_regs.h" >> > > [...] > >> diff --git a/drivers/gpu/drm/v3d/v3d_regs.h b/drivers/gpu/drm/v3d/v3d_regs.h >> index 3663e0d6bf76..9fbcbfedaae1 100644 >> --- a/drivers/gpu/drm/v3d/v3d_regs.h >> +++ b/drivers/gpu/drm/v3d/v3d_regs.h >> @@ -57,6 +57,7 @@ >> #define V3D_HUB_INT_MSK_STS0x0005c >> #define V3D_HUB_INT_MSK_SET0x00060 >> #define V3D_HUB_INT_MSK_CLR0x00064 >> +# define V3D_V7_HUB_INT_GMPV BIT(6) >> # define V3D_HUB_INT_MMU_WRV BIT(5) >> # define V3D_HUB_INT_MMU_PTI BIT(4) >> # define V3D_HUB_INT_MMU_CAP BIT(3) >> @@ -64,6 +65,7 @@ >> # define V3D_HUB_INT_TFUC BIT(1) >> # define V3D_HUB_INT_TFUF BIT(0) >> +/* GCA registers only exist in V3D < 41 */ >> #define V3D_GCA_CACHE_CTRL 0xc >> # define V3D_GCA_CACHE_CTRL_FLUSH BIT(0) >> @@ -87,6 +89,7 @@ >> # define V3D_TOP_GR_BRIDGE_SW_INIT_1_V3D_CLK_108_SW_INIT BIT(0) >> #define V3D_TFU_CS 0x00400 >> +#define V3D_V7_TFU_CS 0x00700 > > What do you think about something like > > #define V3D_TFU_CS(ver) (ver >= 71) ? 0x00700 : 0x00400 > > I believe that the code would get much cleaner and would avoid a bunch > of the if statements that you used. > > I would apply this format to all the new registers. Sure, that sounds good. Thanks! Iago > Best Regards, > - Maíra > >> /* Stops current job, empties input fifo. */ >> # define V3D_TFU_CS_TFURST BIT(31) >> # define V3D_TFU_CS_CVTCT_MASK V3D_MASK(23, 16) >> @@ -96,6 +99,7 @@ >> # define V3D_TFU_CS_BUSY BIT(0) >> #define V3D_TFU_SU 0x00404 >> +#define V3D_V7_TFU_SU 0x00704 >> /* Interrupt when FINTTHR input slots are free (0 = disabled) */ >> # define V3D_TFU_SU_FINTTHR_MASK V3D_MASK(13, 8) >> # define V3D_TFU_SU_FINTTHR_SHIFT 8 >> @@ -107,38 +111,53 @@ >> # define V3D_TFU_SU_THROTTLE_SHIFT 0 >> #define V3D_TFU_ICFG 0x00408 >> +#define V3D_V7_TFU_ICFG0x00708 >> /* Interrupt when the conversion is complete. */ >> # define V3D_TFU_ICFG_IOC BIT(0) >> /* Input Image Address */ >> #define V3D_TFU_IIA0x0040c >> +#define V3D_V7_TFU_IIA 0x0070c >> /* Input Chroma Address */ >> #define V3D_TFU_ICA0x00410 >> +#define V3D_V7_TFU_ICA 0x00710 >> /* Input Image Stride */ >> #define V3D_TFU_IIS0x00414 >> +#define V3D_V7_TFU_IIS 0x00714 >> /* Input Image U-Plane Address */ >> #define V3D_TFU_IUA0x00418 >> +#define V3D_V7_TFU_IUA 0x00718 >> +/* Image output config (VD 7.x only) */ >> +#define V3D_V7_TFU_IOC 0x0071c >> /* Output Image Address */ >> #define V3D_TFU_IOA0x0041c >> +#define V3D_V7_TFU_IOA 0x00720 >> /* Image Output Size */ >> #define V3D_TFU_IOS0x00420 >> +#define V3D_V7_TFU_IOS 0x00724 >> /* TFU YUV Coefficient 0 */ >> #define V3D_TFU_COEF0 0x00424 >> -/* Use these regs instead of the defaults. */ >> +#define V3D_V7_TFU_COEF0 0x00728 >> +/* Use these regs instead of the defaults
Re: [PATCH 1/3] drm/v3d: fix up register addresses for V3D 7.x
Hi Iago, On 9/28/23 08:45, Iago Toral Quiroga wrote: Please, add a commit message and your s-o-b to the patch. Here is a reference on how to format your patches [1]. Also, please, run checkpatch on this patch and address the warnings. [1] https://docs.kernel.org/process/submitting-patches.html --- drivers/gpu/drm/v3d/v3d_debugfs.c | 173 +- drivers/gpu/drm/v3d/v3d_gem.c | 3 + drivers/gpu/drm/v3d/v3d_irq.c | 47 drivers/gpu/drm/v3d/v3d_regs.h| 51 - drivers/gpu/drm/v3d/v3d_sched.c | 41 --- 5 files changed, 200 insertions(+), 115 deletions(-) diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c b/drivers/gpu/drm/v3d/v3d_debugfs.c index 330669f51fa7..90b2b5b2710c 100644 --- a/drivers/gpu/drm/v3d/v3d_debugfs.c +++ b/drivers/gpu/drm/v3d/v3d_debugfs.c @@ -12,69 +12,83 @@ #include "v3d_drv.h" #include "v3d_regs.h" [...] diff --git a/drivers/gpu/drm/v3d/v3d_regs.h b/drivers/gpu/drm/v3d/v3d_regs.h index 3663e0d6bf76..9fbcbfedaae1 100644 --- a/drivers/gpu/drm/v3d/v3d_regs.h +++ b/drivers/gpu/drm/v3d/v3d_regs.h @@ -57,6 +57,7 @@ #define V3D_HUB_INT_MSK_STS0x0005c #define V3D_HUB_INT_MSK_SET0x00060 #define V3D_HUB_INT_MSK_CLR0x00064 +# define V3D_V7_HUB_INT_GMPV BIT(6) # define V3D_HUB_INT_MMU_WRV BIT(5) # define V3D_HUB_INT_MMU_PTI BIT(4) # define V3D_HUB_INT_MMU_CAP BIT(3) @@ -64,6 +65,7 @@ # define V3D_HUB_INT_TFUC BIT(1) # define V3D_HUB_INT_TFUF BIT(0) +/* GCA registers only exist in V3D < 41 */ #define V3D_GCA_CACHE_CTRL 0xc # define V3D_GCA_CACHE_CTRL_FLUSH BIT(0) @@ -87,6 +89,7 @@ # define V3D_TOP_GR_BRIDGE_SW_INIT_1_V3D_CLK_108_SW_INIT BIT(0) #define V3D_TFU_CS 0x00400 +#define V3D_V7_TFU_CS 0x00700 What do you think about something like #define V3D_TFU_CS(ver) (ver >= 71) ? 0x00700 : 0x00400 I believe that the code would get much cleaner and would avoid a bunch of the if statements that you used. I would apply this format to all the new registers. Best Regards, - Maíra /* Stops current job, empties input fifo. */ # define V3D_TFU_CS_TFURST BIT(31) # define V3D_TFU_CS_CVTCT_MASK V3D_MASK(23, 16) @@ -96,6 +99,7 @@ # define V3D_TFU_CS_BUSY BIT(0) #define V3D_TFU_SU 0x00404 +#define V3D_V7_TFU_SU 0x00704 /* Interrupt when FINTTHR input slots are free (0 = disabled) */ # define V3D_TFU_SU_FINTTHR_MASK V3D_MASK(13, 8) # define V3D_TFU_SU_FINTTHR_SHIFT 8 @@ -107,38 +111,53 @@ # define V3D_TFU_SU_THROTTLE_SHIFT 0 #define V3D_TFU_ICFG 0x00408 +#define V3D_V7_TFU_ICFG0x00708 /* Interrupt when the conversion is complete. */ # define V3D_TFU_ICFG_IOC BIT(0) /* Input Image Address */ #define V3D_TFU_IIA0x0040c +#define V3D_V7_TFU_IIA 0x0070c /* Input Chroma Address */ #define V3D_TFU_ICA0x00410 +#define V3D_V7_TFU_ICA 0x00710 /* Input Image Stride */ #define V3D_TFU_IIS0x00414 +#define V3D_V7_TFU_IIS 0x00714 /* Input Image U-Plane Address */ #define V3D_TFU_IUA0x00418 +#define V3D_V7_TFU_IUA 0x00718 +/* Image output config (VD 7.x only) */ +#define V3D_V7_TFU_IOC 0x0071c /* Output Image Address */ #define V3D_TFU_IOA0x0041c +#define V3D_V7_TFU_IOA 0x00720 /* Image Output Size */ #define V3D_TFU_IOS0x00420 +#define V3D_V7_TFU_IOS 0x00724 /* TFU YUV Coefficient 0 */ #define V3D_TFU_COEF0 0x00424 -/* Use these regs instead of the defaults. */ +#define V3D_V7_TFU_COEF0 0x00728 +/* Use these regs instead of the defaults (V3D 4.x only) */ # define V3D_TFU_COEF0_USECOEF BIT(31) /* TFU YUV Coefficient 1 */ #define V3D_TFU_COEF1 0x00428 +#define V3D_V7_TFU_COEF1 0x0072c /* TFU YUV Coefficient 2 */ #define V3D_TFU_COEF2 0x0042c +#define V3D_V7_TFU_C
[PATCH 1/3] drm/v3d: fix up register addresses for V3D 7.x
--- drivers/gpu/drm/v3d/v3d_debugfs.c | 173 +- drivers/gpu/drm/v3d/v3d_gem.c | 3 + drivers/gpu/drm/v3d/v3d_irq.c | 47 drivers/gpu/drm/v3d/v3d_regs.h| 51 - drivers/gpu/drm/v3d/v3d_sched.c | 41 --- 5 files changed, 200 insertions(+), 115 deletions(-) diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c b/drivers/gpu/drm/v3d/v3d_debugfs.c index 330669f51fa7..90b2b5b2710c 100644 --- a/drivers/gpu/drm/v3d/v3d_debugfs.c +++ b/drivers/gpu/drm/v3d/v3d_debugfs.c @@ -12,69 +12,83 @@ #include "v3d_drv.h" #include "v3d_regs.h" -#define REGDEF(reg) { reg, #reg } +#define REGDEF(min_ver, max_ver, reg) { min_ver, max_ver, reg, #reg } struct v3d_reg_def { + u32 min_ver; + u32 max_ver; u32 reg; const char *name; }; static const struct v3d_reg_def v3d_hub_reg_defs[] = { - REGDEF(V3D_HUB_AXICFG), - REGDEF(V3D_HUB_UIFCFG), - REGDEF(V3D_HUB_IDENT0), - REGDEF(V3D_HUB_IDENT1), - REGDEF(V3D_HUB_IDENT2), - REGDEF(V3D_HUB_IDENT3), - REGDEF(V3D_HUB_INT_STS), - REGDEF(V3D_HUB_INT_MSK_STS), - - REGDEF(V3D_MMU_CTL), - REGDEF(V3D_MMU_VIO_ADDR), - REGDEF(V3D_MMU_VIO_ID), - REGDEF(V3D_MMU_DEBUG_INFO), + REGDEF(33, 42, V3D_HUB_AXICFG), + REGDEF(33, 71, V3D_HUB_UIFCFG), + REGDEF(33, 71, V3D_HUB_IDENT0), + REGDEF(33, 71, V3D_HUB_IDENT1), + REGDEF(33, 71, V3D_HUB_IDENT2), + REGDEF(33, 71, V3D_HUB_IDENT3), + REGDEF(33, 71, V3D_HUB_INT_STS), + REGDEF(33, 71, V3D_HUB_INT_MSK_STS), + + REGDEF(33, 71, V3D_MMU_CTL), + REGDEF(33, 71, V3D_MMU_VIO_ADDR), + REGDEF(33, 71, V3D_MMU_VIO_ID), + REGDEF(33, 71, V3D_MMU_DEBUG_INFO), + + REGDEF(71, 71, V3D_V7_GMP_STATUS), + REGDEF(71, 71, V3D_V7_GMP_CFG), + REGDEF(71, 71, V3D_V7_GMP_VIO_ADDR), }; static const struct v3d_reg_def v3d_gca_reg_defs[] = { - REGDEF(V3D_GCA_SAFE_SHUTDOWN), - REGDEF(V3D_GCA_SAFE_SHUTDOWN_ACK), + REGDEF(33, 33, V3D_GCA_SAFE_SHUTDOWN), + REGDEF(33, 33, V3D_GCA_SAFE_SHUTDOWN_ACK), }; static const struct v3d_reg_def v3d_core_reg_defs[] = { - REGDEF(V3D_CTL_IDENT0), - REGDEF(V3D_CTL_IDENT1), - REGDEF(V3D_CTL_IDENT2), - REGDEF(V3D_CTL_MISCCFG), - REGDEF(V3D_CTL_INT_STS), - REGDEF(V3D_CTL_INT_MSK_STS), - REGDEF(V3D_CLE_CT0CS), - REGDEF(V3D_CLE_CT0CA), - REGDEF(V3D_CLE_CT0EA), - REGDEF(V3D_CLE_CT1CS), - REGDEF(V3D_CLE_CT1CA), - REGDEF(V3D_CLE_CT1EA), - - REGDEF(V3D_PTB_BPCA), - REGDEF(V3D_PTB_BPCS), - - REGDEF(V3D_GMP_STATUS), - REGDEF(V3D_GMP_CFG), - REGDEF(V3D_GMP_VIO_ADDR), - - REGDEF(V3D_ERR_FDBGO), - REGDEF(V3D_ERR_FDBGB), - REGDEF(V3D_ERR_FDBGS), - REGDEF(V3D_ERR_STAT), + REGDEF(33, 71, V3D_CTL_IDENT0), + REGDEF(33, 71, V3D_CTL_IDENT1), + REGDEF(33, 71, V3D_CTL_IDENT2), + REGDEF(33, 71, V3D_CTL_MISCCFG), + REGDEF(33, 71, V3D_CTL_INT_STS), + REGDEF(33, 71, V3D_CTL_INT_MSK_STS), + REGDEF(33, 71, V3D_CLE_CT0CS), + REGDEF(33, 71, V3D_CLE_CT0CA), + REGDEF(33, 71, V3D_CLE_CT0EA), + REGDEF(33, 71, V3D_CLE_CT1CS), + REGDEF(33, 71, V3D_CLE_CT1CA), + REGDEF(33, 71, V3D_CLE_CT1EA), + + REGDEF(33, 71, V3D_PTB_BPCA), + REGDEF(33, 71, V3D_PTB_BPCS), + + REGDEF(33, 41, V3D_GMP_STATUS), + REGDEF(33, 41, V3D_GMP_CFG), + REGDEF(33, 41, V3D_GMP_VIO_ADDR), + + REGDEF(33, 71, V3D_ERR_FDBGO), + REGDEF(33, 71, V3D_ERR_FDBGB), + REGDEF(33, 71, V3D_ERR_FDBGS), + REGDEF(33, 71, V3D_ERR_STAT), }; static const struct v3d_reg_def v3d_csd_reg_defs[] = { - REGDEF(V3D_CSD_STATUS), - REGDEF(V3D_CSD_CURRENT_CFG0), - REGDEF(V3D_CSD_CURRENT_CFG1), - REGDEF(V3D_CSD_CURRENT_CFG2), - REGDEF(V3D_CSD_CURRENT_CFG3), - REGDEF(V3D_CSD_CURRENT_CFG4), - REGDEF(V3D_CSD_CURRENT_CFG5), - REGDEF(V3D_CSD_CURRENT_CFG6), + REGDEF(41, 71, V3D_CSD_STATUS), + REGDEF(41, 41, V3D_CSD_CURRENT_CFG0), + REGDEF(41, 41, V3D_CSD_CURRENT_CFG1), + REGDEF(41, 41, V3D_CSD_CURRENT_CFG2), + REGDEF(41, 41, V3D_CSD_CURRENT_CFG3), + REGDEF(41, 41, V3D_CSD_CURRENT_CFG4), + REGDEF(41, 41, V3D_CSD_CURRENT_CFG5), + REGDEF(41, 41, V3D_CSD_CURRENT_CFG6), + REGDEF(71, 71, V3D_V7_CSD_CURRENT_CFG0), + REGDEF(71, 71, V3D_V7_CSD_CURRENT_CFG1), + REGDEF(71, 71, V3D_V7_CSD_CURRENT_CFG2), + REGDEF(71, 71, V3D_V7_CSD_CURRENT_CFG3), + REGDEF(71, 71, V3D_V7_CSD_CURRENT_CFG4), + REGDEF(71, 71, V3D_V7_CSD_CURRENT_CFG5), + REGDEF(71, 71, V3D_V7_CSD_CURRENT_CFG6), + REGDEF(71, 71, V3D_V7_CSD_CURRENT_CFG7), }; static int v3d_v3d_debugfs_regs(struct seq_file *m, void *unused) @@ -85,38 +99,37 @@ static int v3d_v3d_debugfs_regs(struct seq_file *m, void *unused) int i, core;