diff mbox series

[v2,05/11] drm/rockchip: vop2: Introduce vop hardware version

Message ID 20240904120238.3856782-6-andyshrk@163.com (mailing list archive)
State New, archived
Headers show
Series VOP Support for rk3576 | expand

Commit Message

Andy Yan Sept. 4, 2024, 12:02 p.m. UTC
From: Andy Yan <andy.yan@rock-chips.com>

There is a version number hardcoded in the VOP VERSION_INFO
register, and the version number increments sequentially based
on the production order of the SOC.

So using this version number to distinguish different VOP features
will simplify the code.

Signed-off-by: Andy Yan <andy.yan@rock-chips.com>

---

Changes in v2:
- Introduce vop hardware version

 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c |  7 ++++---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 11 +++++++++++
 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c |  3 +++
 3 files changed, 18 insertions(+), 3 deletions(-)

Comments

Sascha Hauer Sept. 5, 2024, 7:10 a.m. UTC | #1
Hi Andy,

On Wed, Sep 04, 2024 at 08:02:32PM +0800, Andy Yan wrote:
> From: Andy Yan <andy.yan@rock-chips.com>
> 
> There is a version number hardcoded in the VOP VERSION_INFO
> register, and the version number increments sequentially based
> on the production order of the SOC.
> 
> So using this version number to distinguish different VOP features
> will simplify the code.
> 
> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
> 
> ---
> 
> Changes in v2:
> - Introduce vop hardware version
> 
>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c |  7 ++++---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 11 +++++++++++
>  drivers/gpu/drm/rockchip/rockchip_vop2_reg.c |  3 +++
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
> index 9b269f6e576e..871d9bcd1d80 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
> @@ -13,6 +13,15 @@
>  #include "rockchip_drm_drv.h"
>  #include "rockchip_drm_vop.h"
>  
> +#define VOP2_VERSION(major, minor, build)	((major) << 24 | (minor) << 16 | (build))
> +
> +/* The new SOC VOP version is bigger than the old */
> +#define VOP_VERSION_RK3568	VOP2_VERSION(0x40, 0x15, 0x8023)
> +#define VOP_VERSION_RK3588	VOP2_VERSION(0x40, 0x17, 0x6786)
> +#define VOP_VERSION_RK3528	VOP2_VERSION(0x50, 0x17, 0x1263)
> +#define VOP_VERSION_RK3562	VOP2_VERSION(0x50, 0x17, 0x4350)
> +#define VOP_VERSION_RK3576	VOP2_VERSION(0x50, 0x19, 0x9765)

What about the RK3566? Does it have the same version code as the RK3568?

This new version field replaces the soc_id mechanism we had before to
99%. You keep the soc_id around just for distinguishing between RK3566
and RK3568. It would be nice to fully replace it.

I see that the VOP_VERSION_RK* numbers are the same as found in the
VOP2_SYS_VERSION_INF registers. On the other hand you never read the
value from the register which make the VOP_VERSION_RK* just arbitrary
numbers. Wouldn't it be possible to make something up for RK3566, like
VOP2_VERSION(0x40, 0x15, 0x8022) to get rid of the soc_id thingy?

Sascha
Andy Yan Sept. 5, 2024, 8:09 a.m. UTC | #2
Hi Sascha,

At 2024-09-05 15:10:56, "Sascha Hauer" <s.hauer@pengutronix.de> wrote:
>Hi Andy,
>
>On Wed, Sep 04, 2024 at 08:02:32PM +0800, Andy Yan wrote:
>> From: Andy Yan <andy.yan@rock-chips.com>
>> 
>> There is a version number hardcoded in the VOP VERSION_INFO
>> register, and the version number increments sequentially based
>> on the production order of the SOC.
>> 
>> So using this version number to distinguish different VOP features
>> will simplify the code.
>> 
>> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
>> 
>> ---
>> 
>> Changes in v2:
>> - Introduce vop hardware version
>> 
>>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c |  7 ++++---
>>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 11 +++++++++++
>>  drivers/gpu/drm/rockchip/rockchip_vop2_reg.c |  3 +++
>>  3 files changed, 18 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
>> index 9b269f6e576e..871d9bcd1d80 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
>> @@ -13,6 +13,15 @@
>>  #include "rockchip_drm_drv.h"
>>  #include "rockchip_drm_vop.h"
>>  
>> +#define VOP2_VERSION(major, minor, build)	((major) << 24 | (minor) << 16 | (build))
>> +
>> +/* The new SOC VOP version is bigger than the old */
>> +#define VOP_VERSION_RK3568	VOP2_VERSION(0x40, 0x15, 0x8023)
>> +#define VOP_VERSION_RK3588	VOP2_VERSION(0x40, 0x17, 0x6786)
>> +#define VOP_VERSION_RK3528	VOP2_VERSION(0x50, 0x17, 0x1263)
>> +#define VOP_VERSION_RK3562	VOP2_VERSION(0x50, 0x17, 0x4350)
>> +#define VOP_VERSION_RK3576	VOP2_VERSION(0x50, 0x19, 0x9765)
>
>What about the RK3566? Does it have the same version code as the RK3568?
>
>This new version field replaces the soc_id mechanism we had before to
>99%. You keep the soc_id around just for distinguishing between RK3566
>and RK3568. It would be nice to fully replace it.
>
>I see that the VOP_VERSION_RK* numbers are the same as found in the
>VOP2_SYS_VERSION_INF registers. On the other hand you never read the
>value from the register which make the VOP_VERSION_RK* just arbitrary
>numbers. Wouldn't it be possible to make something up for RK3566, like

>VOP2_VERSION(0x40, 0x15, 0x8022) to get rid of the soc_id thingy?


Yes,RK3566 and RK3568 share the same VOP IP block, so the version code at VERSION_REGISTER is
the same, the difference between rk3568 and rk33566 are introduced at soc Integration。
So i would still like to keep the soc_id to  handle situation like this。As we always have such  cause, one
same IP block, but there are some subtle differences in features across different SOCs.
I have considered reading the version register directly, but I haven't found a suitable method yet。


>
>Sascha
>
>-- 
>Pengutronix e.K.                           |                             |
>Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>
>_______________________________________________
>Linux-rockchip mailing list
>Linux-rockchip@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-rockchip
Sascha Hauer Sept. 9, 2024, 9:13 a.m. UTC | #3
On Thu, Sep 05, 2024 at 04:09:58PM +0800, Andy Yan wrote:
>    Hi Sascha,
> 
>  At 2024-09-05 15:10:56, "Sascha Hauer" <s.hauer@pengutronix.de> wrote:
>  >Hi Andy,
>  >
>  >On Wed, Sep 04, 2024 at 08:02:32PM +0800, Andy Yan wrote:
>  >> From: Andy Yan <andy.yan@rock-chips.com>
>  >>
>  >> There is a version number hardcoded in the VOP VERSION_INFO
>  >> register, and the version number increments sequentially based
>  >> on the production order of the SOC.
>  >>
>  >> So using this version number to distinguish different VOP features
>  >> will simplify the code.
>  >>
>  >> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
>  >>
>  >> ---
>  >>
>  >> Changes in v2:
>  >> - Introduce vop hardware version
>  >>
>  >>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c |  7 ++++---
>  >>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 11 +++++++++++
>  >>  drivers/gpu/drm/rockchip/rockchip_vop2_reg.c |  3 +++
>  >>  3 files changed, 18 insertions(+), 3 deletions(-)
>  >>
>  >> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
>  >> index 9b269f6e576e..871d9bcd1d80 100644
>  >> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
>  >> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
>  >> @@ -13,6 +13,15 @@
>  >>  #include "rockchip_drm_drv.h"
>  >>  #include "rockchip_drm_vop.h"
>  >>
>  >> +#define VOP2_VERSION(major, minor, build)     ((major) << 24 | (minor) << 16 | (build))
>  >> +
>  >> +/* The new SOC VOP version is bigger than the old */
>  >> +#define VOP_VERSION_RK3568    VOP2_VERSION(0x40, 0x15, 0x8023)
>  >> +#define VOP_VERSION_RK3588    VOP2_VERSION(0x40, 0x17, 0x6786)
>  >> +#define VOP_VERSION_RK3528    VOP2_VERSION(0x50, 0x17, 0x1263)
>  >> +#define VOP_VERSION_RK3562    VOP2_VERSION(0x50, 0x17, 0x4350)
>  >> +#define VOP_VERSION_RK3576    VOP2_VERSION(0x50, 0x19, 0x9765)
>  >
>  >What about the RK3566? Does it have the same version code as the RK3568?
>  >
>  >This new version field replaces the soc_id mechanism we had before to
>  >99%. You keep the soc_id around just for distinguishing between RK3566
>  >and RK3568. It would be nice to fully replace it.
>  >
>  >I see that the VOP_VERSION_RK* numbers are the same as found in the
>  >VOP2_SYS_VERSION_INF registers. On the other hand you never read the
>  >value from the register which make the VOP_VERSION_RK* just arbitrary
>  >numbers. Wouldn't it be possible to make something up for RK3566, like
>  >VOP2_VERSION(0x40, 0x15, 0x8022) to get rid of the soc_id thingy?
>  Yes,RK3566 and RK3568 share the same VOP IP block, so the version code at VERSION_REGISTER is
>  the same, the difference between rk3568 and rk33566 are introduced at soc Integration。
>  So i would still like to keep the soc_id to  handle situation like this。As we always have such  cause, one
>  same IP block, but there are some subtle differences in features across different SOCs.

Fine with me. You could leave a comment in the code or commit
message that explains why we need both.

>  I have considered reading the version register directly, but I haven't found a suitable method yet.

You could check the expected version from the driver data against
the register value, but that would only be an additional sanity check.
Not sure if it's worth it.

Sascha
Diederik de Haas Sept. 9, 2024, 9:36 a.m. UTC | #4
On Mon Sep 9, 2024 at 11:13 AM CEST, Sascha Hauer wrote:
> On Thu, Sep 05, 2024 at 04:09:58PM +0800, Andy Yan wrote:
> >  At 2024-09-05 15:10:56, "Sascha Hauer" <s.hauer@pengutronix.de> wrote:
> >  >On Wed, Sep 04, 2024 at 08:02:32PM +0800, Andy Yan wrote:
> >  >> From: Andy Yan <andy.yan@rock-chips.com>
> >  >>
> >  >> There is a version number hardcoded in the VOP VERSION_INFO
> >  >> register, and the version number increments sequentially based
> >  >> on the production order of the SOC.
> >  >>
> >  >> So using this version number to distinguish different VOP features
> >  >> will simplify the code.
> >  >>
> >  >> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
> >  >>
> >  >> ---
> >  >>
> >  >> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
> >  >> index 9b269f6e576e..871d9bcd1d80 100644
> >  >> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
> >  >> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
> >  >> @@ -13,6 +13,15 @@
> >  >>  #include "rockchip_drm_drv.h"
> >  >>  #include "rockchip_drm_vop.h"
> >  >>
> >  >> +#define VOP2_VERSION(major, minor, build)     ((major) << 24 | (minor) << 16 | (build))
> >  >> +
> >  >> +/* The new SOC VOP version is bigger than the old */
> >  >> +#define VOP_VERSION_RK3568    VOP2_VERSION(0x40, 0x15, 0x8023)
> >  >> +#define VOP_VERSION_RK3588    VOP2_VERSION(0x40, 0x17, 0x6786)
> >  >> +#define VOP_VERSION_RK3528    VOP2_VERSION(0x50, 0x17, 0x1263)
> >  >> +#define VOP_VERSION_RK3562    VOP2_VERSION(0x50, 0x17, 0x4350)
> >  >> +#define VOP_VERSION_RK3576    VOP2_VERSION(0x50, 0x19, 0x9765)
> >  >
> >  >What about the RK3566? Does it have the same version code as the RK3568?
> >  >
> >  >This new version field replaces the soc_id mechanism we had before to
> >  >99%. You keep the soc_id around just for distinguishing between RK3566
> >  >and RK3568. It would be nice to fully replace it.
> >  >
> >  >I see that the VOP_VERSION_RK* numbers are the same as found in the
> >  >VOP2_SYS_VERSION_INF registers. On the other hand you never read the
> >  >value from the register which make the VOP_VERSION_RK* just arbitrary
> >  >numbers. Wouldn't it be possible to make something up for RK3566, like
> >  >VOP2_VERSION(0x40, 0x15, 0x8022) to get rid of the soc_id thingy?
> >  Yes,RK3566 and RK3568 share the same VOP IP block, so the version code at VERSION_REGISTER is
> >  the same, the difference between rk3568 and rk33566 are introduced at soc Integration。
> >  So i would still like to keep the soc_id to  handle situation like this。As we always have such  cause, one
> >  same IP block, but there are some subtle differences in features across different SOCs.
>
> Fine with me. You could leave a comment in the code or commit
> message that explains why we need both.

Also (or especially?) add that to the commit message of patch 6 of this
series. Patch 6's commit message talks about RK3576 while it changes
code related to RK3566 and I (too?) thought that not using VOP_VERSION
was an oversight, while it turns out to be deliberate.

Cheers,
  Diederik
Andy Yan Sept. 12, 2024, 6:41 a.m. UTC | #5
Hi Sascha,
At 2024-09-09 17:13:55, "Sascha Hauer" <s.hauer@pengutronix.de> wrote:
>On Thu, Sep 05, 2024 at 04:09:58PM +0800, Andy Yan wrote:
>>    Hi Sascha,
>> 
>>  At 2024-09-05 15:10:56, "Sascha Hauer" <s.hauer@pengutronix.de> wrote:
>>  >Hi Andy,
>>  >
>>  >On Wed, Sep 04, 2024 at 08:02:32PM +0800, Andy Yan wrote:
>>  >> From: Andy Yan <andy.yan@rock-chips.com>
>>  >>
>>  >> There is a version number hardcoded in the VOP VERSION_INFO
>>  >> register, and the version number increments sequentially based
>>  >> on the production order of the SOC.
>>  >>
>>  >> So using this version number to distinguish different VOP features
>>  >> will simplify the code.
>>  >>
>>  >> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
>>  >>
>>  >> ---
>>  >>
>>  >> Changes in v2:
>>  >> - Introduce vop hardware version
>>  >>
>>  >>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c |  7 ++++---
>>  >>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 11 +++++++++++
>>  >>  drivers/gpu/drm/rockchip/rockchip_vop2_reg.c |  3 +++
>>  >>  3 files changed, 18 insertions(+), 3 deletions(-)
>>  >>
>>  >> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
>>  >> index 9b269f6e576e..871d9bcd1d80 100644
>>  >> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
>>  >> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
>>  >> @@ -13,6 +13,15 @@
>>  >>  #include "rockchip_drm_drv.h"
>>  >>  #include "rockchip_drm_vop.h"
>>  >>
>>  >> +#define VOP2_VERSION(major, minor, build)     ((major) << 24 | (minor) << 16 | (build))
>>  >> +
>>  >> +/* The new SOC VOP version is bigger than the old */
>>  >> +#define VOP_VERSION_RK3568    VOP2_VERSION(0x40, 0x15, 0x8023)
>>  >> +#define VOP_VERSION_RK3588    VOP2_VERSION(0x40, 0x17, 0x6786)
>>  >> +#define VOP_VERSION_RK3528    VOP2_VERSION(0x50, 0x17, 0x1263)
>>  >> +#define VOP_VERSION_RK3562    VOP2_VERSION(0x50, 0x17, 0x4350)
>>  >> +#define VOP_VERSION_RK3576    VOP2_VERSION(0x50, 0x19, 0x9765)
>>  >
>>  >What about the RK3566? Does it have the same version code as the RK3568?
>>  >
>>  >This new version field replaces the soc_id mechanism we had before to
>>  >99%. You keep the soc_id around just for distinguishing between RK3566
>>  >and RK3568. It would be nice to fully replace it.
>>  >
>>  >I see that the VOP_VERSION_RK* numbers are the same as found in the
>>  >VOP2_SYS_VERSION_INF registers. On the other hand you never read the
>>  >value from the register which make the VOP_VERSION_RK* just arbitrary
>>  >numbers. Wouldn't it be possible to make something up for RK3566, like
>>  >VOP2_VERSION(0x40, 0x15, 0x8022) to get rid of the soc_id thingy?
>>  Yes,RK3566 and RK3568 share the same VOP IP block, so the version code at VERSION_REGISTER is
>>  the same, the difference between rk3568 and rk33566 are introduced at soc Integration。
>>  So i would still like to keep the soc_id to  handle situation like this。As we always have such  cause, one
>>  same IP block, but there are some subtle differences in features across different SOCs.
>
>Fine with me. You could leave a comment in the code or commit

>message that explains why we need both.


Ok, will add in V3.>
>>  I have considered reading the version register directly, but I haven't found a suitable method yet.
>
>You could check the expected version from the driver data against
>the register value, but that would only be an additional sanity check.

>Not sure if it's worth it.


I think we can add a check like that to make sure the version code matchs the
real register value,rather than being an arbitrarily created value.
>Sascha
>
>-- 
>Pengutronix e.K.                           |                             |
>Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>
>_______________________________________________
>Linux-rockchip mailing list
>Linux-rockchip@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-rockchip
Andy Yan Sept. 12, 2024, 6:44 a.m. UTC | #6
Hi ,
At 2024-09-09 17:36:14, "Diederik de Haas" <didi.debian@cknow.org> wrote:
>On Mon Sep 9, 2024 at 11:13 AM CEST, Sascha Hauer wrote:
>> On Thu, Sep 05, 2024 at 04:09:58PM +0800, Andy Yan wrote:
>> >  At 2024-09-05 15:10:56, "Sascha Hauer" <s.hauer@pengutronix.de> wrote:
>> >  >On Wed, Sep 04, 2024 at 08:02:32PM +0800, Andy Yan wrote:
>> >  >> From: Andy Yan <andy.yan@rock-chips.com>
>> >  >>
>> >  >> There is a version number hardcoded in the VOP VERSION_INFO
>> >  >> register, and the version number increments sequentially based
>> >  >> on the production order of the SOC.
>> >  >>
>> >  >> So using this version number to distinguish different VOP features
>> >  >> will simplify the code.
>> >  >>
>> >  >> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
>> >  >>
>> >  >> ---
>> >  >>
>> >  >> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
>> >  >> index 9b269f6e576e..871d9bcd1d80 100644
>> >  >> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
>> >  >> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
>> >  >> @@ -13,6 +13,15 @@
>> >  >>  #include "rockchip_drm_drv.h"
>> >  >>  #include "rockchip_drm_vop.h"
>> >  >>
>> >  >> +#define VOP2_VERSION(major, minor, build)     ((major) << 24 | (minor) << 16 | (build))
>> >  >> +
>> >  >> +/* The new SOC VOP version is bigger than the old */
>> >  >> +#define VOP_VERSION_RK3568    VOP2_VERSION(0x40, 0x15, 0x8023)
>> >  >> +#define VOP_VERSION_RK3588    VOP2_VERSION(0x40, 0x17, 0x6786)
>> >  >> +#define VOP_VERSION_RK3528    VOP2_VERSION(0x50, 0x17, 0x1263)
>> >  >> +#define VOP_VERSION_RK3562    VOP2_VERSION(0x50, 0x17, 0x4350)
>> >  >> +#define VOP_VERSION_RK3576    VOP2_VERSION(0x50, 0x19, 0x9765)
>> >  >
>> >  >What about the RK3566? Does it have the same version code as the RK3568?
>> >  >
>> >  >This new version field replaces the soc_id mechanism we had before to
>> >  >99%. You keep the soc_id around just for distinguishing between RK3566
>> >  >and RK3568. It would be nice to fully replace it.
>> >  >
>> >  >I see that the VOP_VERSION_RK* numbers are the same as found in the
>> >  >VOP2_SYS_VERSION_INF registers. On the other hand you never read the
>> >  >value from the register which make the VOP_VERSION_RK* just arbitrary
>> >  >numbers. Wouldn't it be possible to make something up for RK3566, like
>> >  >VOP2_VERSION(0x40, 0x15, 0x8022) to get rid of the soc_id thingy?
>> >  Yes,RK3566 and RK3568 share the same VOP IP block, so the version code at VERSION_REGISTER is
>> >  the same, the difference between rk3568 and rk33566 are introduced at soc Integration。
>> >  So i would still like to keep the soc_id to  handle situation like this。As we always have such  cause, one
>> >  same IP block, but there are some subtle differences in features across different SOCs.
>>
>> Fine with me. You could leave a comment in the code or commit
>> message that explains why we need both.
>
>Also (or especially?) add that to the commit message of patch 6 of this
>series. Patch 6's commit message talks about RK3576 while it changes
>code related to RK3566 and I (too?) thought that not using VOP_VERSION

>was an oversight, while it turns out to be deliberate.


OK, will do in v3.>
>Cheers,
>  Diederik
diff mbox series

Patch

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 6a7982ad3550..f32cfb25063f 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -354,7 +354,7 @@  static bool vop2_output_uv_swap(u32 bus_format, u32 output_mode)
 
 static bool vop2_output_rg_swap(struct vop2 *vop2, u32 bus_format)
 {
-	if (vop2->data->soc_id == 3588) {
+	if (vop2->version == VOP_VERSION_RK3588) {
 		if (bus_format == MEDIA_BUS_FMT_YUV8_1X24 ||
 		    bus_format == MEDIA_BUS_FMT_YUV10_1X30)
 			return true;
@@ -820,7 +820,7 @@  static void vop2_enable(struct vop2 *vop2)
 	if (vop2->data->soc_id == 3566)
 		vop2_writel(vop2, RK3568_OTP_WIN_EN, 1);
 
-	if (vop2->data->soc_id == 3588)
+	if (vop2->version == VOP_VERSION_RK3588)
 		rk3588_vop2_power_domain_enable_all(vop2);
 
 	vop2_writel(vop2, RK3568_REG_CFG_DONE, RK3568_REG_CFG_DONE__GLB_CFG_DONE_EN);
@@ -1231,7 +1231,7 @@  static void vop2_plane_atomic_update(struct drm_plane *plane,
 		 * this bit is gating disable, we should write 1 to
 		 * disable gating when enable afbc.
 		 */
-		if (vop2->data->soc_id == 3566 || vop2->data->soc_id == 3568)
+		if (vop2->version == VOP_VERSION_RK3568)
 			vop2_win_write(win, VOP2_WIN_AFBC_AUTO_GATING_EN, 0);
 		else
 			vop2_win_write(win, VOP2_WIN_AFBC_AUTO_GATING_EN, 1);
@@ -2320,6 +2320,7 @@  static int vop2_bind(struct device *dev, struct device *master, void *data)
 	vop2->dev = dev;
 	vop2->data = vop2_data;
 	vop2->ops = vop2_data->ops;
+	vop2->version = vop2_data->version;
 	vop2->drm = drm;
 
 	dev_set_drvdata(dev, vop2);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
index 9b269f6e576e..871d9bcd1d80 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
@@ -13,6 +13,15 @@ 
 #include "rockchip_drm_drv.h"
 #include "rockchip_drm_vop.h"
 
+#define VOP2_VERSION(major, minor, build)	((major) << 24 | (minor) << 16 | (build))
+
+/* The new SOC VOP version is bigger than the old */
+#define VOP_VERSION_RK3568	VOP2_VERSION(0x40, 0x15, 0x8023)
+#define VOP_VERSION_RK3588	VOP2_VERSION(0x40, 0x17, 0x6786)
+#define VOP_VERSION_RK3528	VOP2_VERSION(0x50, 0x17, 0x1263)
+#define VOP_VERSION_RK3562	VOP2_VERSION(0x50, 0x17, 0x4350)
+#define VOP_VERSION_RK3576	VOP2_VERSION(0x50, 0x19, 0x9765)
+
 #define VOP2_VP_FEATURE_OUTPUT_10BIT        BIT(0)
 
 #define VOP2_FEATURE_HAS_SYS_GRF	BIT(0)
@@ -235,6 +244,7 @@  struct vop2_ops {
 struct vop2_data {
 	u8 nr_vps;
 	u64 feature;
+	u32 version;
 	const struct vop2_ops *ops;
 	const struct vop2_win_data *win;
 	const struct vop2_video_port_data *vp;
@@ -252,6 +262,7 @@  struct vop2_data {
 };
 
 struct vop2 {
+	u32 version;
 	struct device *dev;
 	struct drm_device *drm;
 	struct vop2_video_port vps[ROCKCHIP_MAX_CRTC];
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
index b44fff0d4cb7..a8a129892977 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
@@ -1550,6 +1550,7 @@  static const struct vop2_ops rk3588_vop_ops = {
 };
 
 static const struct vop2_data rk3566_vop = {
+	.version = VOP_VERSION_RK3568,
 	.feature = VOP2_FEATURE_HAS_SYS_GRF,
 	.nr_vps = 3,
 	.max_input = { 4096, 2304 },
@@ -1568,6 +1569,7 @@  static const struct vop2_data rk3566_vop = {
 };
 
 static const struct vop2_data rk3568_vop = {
+	.version = VOP_VERSION_RK3568,
 	.feature = VOP2_FEATURE_HAS_SYS_GRF,
 	.nr_vps = 3,
 	.max_input = { 4096, 2304 },
@@ -1586,6 +1588,7 @@  static const struct vop2_data rk3568_vop = {
 };
 
 static const struct vop2_data rk3588_vop = {
+	.version = VOP_VERSION_RK3588,
 	.feature = VOP2_FEATURE_HAS_SYS_GRF | VOP2_FEATURE_HAS_VO1_GRF |
 		   VOP2_FEATURE_HAS_VOP_GRF | VOP2_FEATURE_HAS_SYS_PMU,
 	.nr_vps = 4,