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@xxxxxxxxxxxxxx> wrote: > >Hi Andy, > > > >On Wed, Sep 04, 2024 at 08:02:32PM +0800, Andy Yan wrote: > >> From: Andy Yan <andy.yan@xxxxxxxxxxxxxx> > >> > >> 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@xxxxxxxxxxxxxx> > >> > >> --- > >> > >> 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 -- 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 |