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
