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
[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;
