Re: [PATCH v2 2/4] drm/verisilicon: add model ID constants and DCU Lite chip identity

[Joey Lu <a0987203069@gmail.com>]

On 5/19/2026 3:37 PM, Icenowy Zheng wrote:
> 在 2026-05-19二的 13:51 +0800，Joey Lu写道：
>> Introduce symbolic constants VSDC_MODEL_DC8200 and
>> VSDC_MODEL_DCU_LITE
>> to replace magic numbers in the hardware database and probe path.
>>
>> Register the DCU Lite chip identity (model 0x0, revision 0x5560,
>> customer_id 0x305) in vs_chip_identities[], making the existing
>> vs_fill_chip_identity() path able to recognise Nuvoton MA35D1
>> hardware
>> purely through register reads.
> The HWDB change should be added in the end of the series, making it a
> gate to the newly added changes that is finally opened when
> everything's ready.
>
>> Also add three register-level macros for forthcoming DCU Lite
>> support:
>> - VSDC_DISP_IRQ_VSYNC(n) in vs_crtc_regs.h, for per-output VSYNC IRQ
>>    bits used by the DCU Lite IRQ enable/status registers.
>> - VSDC_FB_CONFIG_ENABLE, VSDC_FB_CONFIG_VALID and
>> VSDC_FB_CONFIG_RESET
>>    in vs_primary_plane_regs.h, for the framebuffer enable and
>>    commit-cycle bits used by the DCU Lite plane update path.
> Maybe you can split the register change
Understood. I will split the register macro additions into a separate 
patch: one for the new vs_crtc_regs.h IRQ macro and one for the 
vs_primary_plane_regs.h FB_CONFIG bits, keeping them independent of the 
HWDB identity change.
>> No behaviour change for existing DC8200 platforms.
>>
>> Signed-off-by: Joey Lu <a0987203069@gmail.com>
>> ---
>>   drivers/gpu/drm/verisilicon/vs_crtc_regs.h       |  1 +
>>   drivers/gpu/drm/verisilicon/vs_hwdb.c            | 16 ++++++++++++--
>> --
>>   drivers/gpu/drm/verisilicon/vs_hwdb.h            |  3 +++
>>   .../gpu/drm/verisilicon/vs_primary_plane_regs.h  |  3 +++
>>   4 files changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/verisilicon/vs_crtc_regs.h
>> b/drivers/gpu/drm/verisilicon/vs_crtc_regs.h
>> index c7930e817635..d4da22b08cd5 100644
>> --- a/drivers/gpu/drm/verisilicon/vs_crtc_regs.h
>> +++ b/drivers/gpu/drm/verisilicon/vs_crtc_regs.h
>> @@ -54,6 +54,7 @@
>>   #define VSDC_DISP_GAMMA_DATA(n)			(0x1460 +
>> 0x4 * (n))
>>   
>>   #define VSDC_DISP_IRQ_STA			0x147C
>> +#define VSDC_DISP_IRQ_VSYNC(n)			BIT(n)
>>   
>>   #define VSDC_DISP_IRQ_EN			0x1480
>>   
>> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c
>> b/drivers/gpu/drm/verisilicon/vs_hwdb.c
>> index 09336af0900a..a25c4b16181d 100644
>> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.c
>> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c
>> @@ -90,7 +90,7 @@ static const struct vs_formats
>> vs_formats_with_yuv444 = {
>>   
>>   static struct vs_chip_identity vs_chip_identities[] = {
>>   	{
>> -		.model = 0x8200,
>> +		.model = VSDC_MODEL_DC8200,
> I don't think such a macro is needed.
Understood. I will remove `VSDC_MODEL_DC8200` and use the literal 
`0x8200` directly in vs_hwdb.c with a comment.
>>   		.revision = 0x5720,
>>   		.customer_id = ~0U,
>>   
>> @@ -98,7 +98,7 @@ static struct vs_chip_identity vs_chip_identities[]
>> = {
>>   		.formats = &vs_formats_no_yuv444,
>>   	},
>>   	{
>> -		.model = 0x8200,
>> +		.model = VSDC_MODEL_DC8200,
>>   		.revision = 0x5721,
>>   		.customer_id = 0x30B,
>>   
>> @@ -106,7 +106,7 @@ static struct vs_chip_identity
>> vs_chip_identities[] = {
>>   		.formats = &vs_formats_no_yuv444,
>>   	},
>>   	{
>> -		.model = 0x8200,
>> +		.model = VSDC_MODEL_DC8200,
>>   		.revision = 0x5720,
>>   		.customer_id = 0x310,
>>   
>> @@ -114,13 +114,21 @@ static struct vs_chip_identity
>> vs_chip_identities[] = {
>>   		.formats = &vs_formats_with_yuv444,
>>   	},
>>   	{
>> -		.model = 0x8200,
>> +		.model = VSDC_MODEL_DC8200,
>>   		.revision = 0x5720,
>>   		.customer_id = 0x311,
>>   
>>   		.display_count = 2,
>>   		.formats = &vs_formats_no_yuv444,
>>   	},
>> +	{
>> +		.model = VSDC_MODEL_DCU_LITE,
> The number is 0x0 and the whole public name of this IP is
> "DCUltraLite", w/o any numbers.
>
> I suggest leave it at 0x0 and add a comment saying this is DCUltraLite
> -- Verisilicon people are abusing suffix for their IP names now.
Understood. I will remove the `VSDC_MODEL_DCU_LITE` macro and use `0x0` 
directly with a `/* DCUltraLite */` comment in vs_hwdb.c.
>> +		.revision = 0x5560,
>> +		.customer_id = 0x305,
>> +
>> +		.display_count = 1,
>> +		.formats = &vs_formats_no_yuv444,
>> +	},
>>   };
>>   
>>   int vs_fill_chip_identity(struct regmap *regs,
>> diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.h
>> b/drivers/gpu/drm/verisilicon/vs_hwdb.h
>> index 92192e4fa086..cca126bd2da5 100644
>> --- a/drivers/gpu/drm/verisilicon/vs_hwdb.h
>> +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.h
>> @@ -9,6 +9,9 @@
>>   #include <linux/regmap.h>
>>   #include <linux/types.h>
>>   
>> +#define VSDC_MODEL_DC8200 0x8200
>> +#define VSDC_MODEL_DCU_LITE 0x0
>> +
>>   struct vs_formats {
>>   	const u32 *array;
>>   	unsigned int num;
>> diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane_regs.h
>> b/drivers/gpu/drm/verisilicon/vs_primary_plane_regs.h
>> index cbb125c46b39..67d4b00f294e 100644
>> --- a/drivers/gpu/drm/verisilicon/vs_primary_plane_regs.h
>> +++ b/drivers/gpu/drm/verisilicon/vs_primary_plane_regs.h
>> @@ -16,6 +16,9 @@
>>   #define VSDC_FB_STRIDE(n)			(0x1408 + 0x4 * (n))
>>   
>>   #define VSDC_FB_CONFIG(n)			(0x1518 + 0x4 * (n))
>> +#define VSDC_FB_CONFIG_ENABLE			BIT(0)
>> +#define VSDC_FB_CONFIG_VALID			BIT(3)
>> +#define VSDC_FB_CONFIG_RESET			BIT(4)
> Should the new IRQ register to be added here too?
>
> Thanks,
> Icenowy
`VSDC_DISP_IRQ_VSYNC(n)` is a bit-mask for the IRQ status/enable 
registers (`VSDC_DISP_IRQ_STA` / `VSDC_DISP_IRQ_EN`) which already live 
in vs_crtc_regs.h. Keeping it there alongside the register addresses it 
operates on is cleaner than splitting the IRQ definitions across two 
headers.
>>   #define VSDC_FB_CONFIG_CLEAR_EN			BIT(8)
>>   #define VSDC_FB_CONFIG_ROT_MASK			GENMASK(13,
>> 11)
>>   #define VSDC_FB_CONFIG_ROT(v)			((v) << 11)