diff mbox series

[v2,2/2] pmdomain: rockchip: Add support for rk3576 SoC

Message ID 20240808163451.80750-3-detlev.casanova@collabora.com (mailing list archive)
State New
Headers show
Series Add power-controller support for rk3576 | expand

Commit Message

Detlev Casanova Aug. 8, 2024, 4:31 p.m. UTC 
From: Finley Xiao <finley.xiao@rock-chips.com>

Add configuration for RK3576 SoC and list the power domains.

Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
[rebase, reword, squash]
Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 drivers/pmdomain/rockchip/pm-domains.c | 73 ++++++++++++++++++++++++--
 1 file changed, 70 insertions(+), 3 deletions(-)

Comments

Heiko Stuebner Aug. 8, 2024, 4:41 p.m. UTC | #1 
Hi Detlev,

Am Donnerstag, 8. August 2024, 18:31:05 CEST schrieb Detlev Casanova:
> From: Finley Xiao <finley.xiao@rock-chips.com>
> 
> Add configuration for RK3576 SoC and list the power domains.
> 
> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
> [rebase, reword, squash]
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> ---
>  drivers/pmdomain/rockchip/pm-domains.c | 73 ++++++++++++++++++++++++--
>  1 file changed, 70 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index 9b76b62869d0d..863f1ad6b9e11 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c

[...]

> @@ -175,6 +195,9 @@ struct rockchip_pmu {
>  #define DOMAIN_RK3568(name, pwr, req, wakeup)		\
>  	DOMAIN_M(name, pwr, pwr, req, req, req, wakeup)
>  
> +#define DOMAIN_RK3576(name, p_offset, pwr, status, r_status, r_offset, req, idle, g_mask, delay, wakeup)	\
> +	DOMAIN_M_O_R_G(name, p_offset, pwr, status, r_status, r_offset, req, idle, idle, g_mask, delay, wakeup)
> +
>  /*
>   * Dynamic Memory Controller may need to coordinate with us -- see
>   * rockchip_pmu_block().
> @@ -552,7 +575,10 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
>  			/* if powering up, leave idle mode */
>  			rockchip_pmu_set_idle_request(pd, false);
>  
> -			rockchip_pmu_restore_qos(pd);
> +			if (pd->info->delay_us)
> +				udelay(pd->info->delay_us);
> +			else
> +				rockchip_pmu_restore_qos(pd);

I still want this behaviour change in a separate patch with adequate
commit message please.

Going from always handling qos to allowing to just wait a specific time
needs explanation and is not part of "just" adding rk3576 support.


Heiko
Detlev Casanova Aug. 8, 2024, 5:53 p.m. UTC | #2 
Hi Heiko,

On Thursday, 8 August 2024 12:41:05 EDT Heiko Stübner wrote:
> Hi Detlev,
> 
> > 
> > @@ -552,7 +575,10 @@ static int rockchip_pd_power(struct
> > rockchip_pm_domain *pd, bool power_on)> 
> >  			/* if powering up, leave idle mode */
> >  			rockchip_pmu_set_idle_request(pd, false);
> > 
> > -			rockchip_pmu_restore_qos(pd);
> > +			if (pd->info->delay_us)
> > +				udelay(pd->info->delay_us);
> > +			else
> > +				rockchip_pmu_restore_qos(pd);
> 
> I still want this behaviour change in a separate patch with adequate
> commit message please.
> 
> Going from always handling qos to allowing to just wait a specific time
> needs explanation and is not part of "just" adding rk3576 support.

You are right, I didn't takle this issue.
This is actually a bug, the else is not supposed to be there, it should only 
be an added delay for some PDs.

Unfortunately, I'm not sure why that delay is needed exactly, so I'm willing 
to remove it for now (only used by nputop and vop, both unsupported) and come 
back to it if needed when VOP/NPU support is added.

Would that work for this upstream ?

Detlev.
Heiko Stuebner Aug. 8, 2024, 5:59 p.m. UTC | #3 
Hi Detlev,

Am Donnerstag, 8. August 2024, 19:53:20 CEST schrieb Detlev Casanova:
> On Thursday, 8 August 2024 12:41:05 EDT Heiko Stübner wrote:
> > > @@ -552,7 +575,10 @@ static int rockchip_pd_power(struct
> > > rockchip_pm_domain *pd, bool power_on)> 
> > >  			/* if powering up, leave idle mode */
> > >  			rockchip_pmu_set_idle_request(pd, false);
> > > 
> > > -			rockchip_pmu_restore_qos(pd);
> > > +			if (pd->info->delay_us)
> > > +				udelay(pd->info->delay_us);
> > > +			else
> > > +				rockchip_pmu_restore_qos(pd);
> > 
> > I still want this behaviour change in a separate patch with adequate
> > commit message please.
> > 
> > Going from always handling qos to allowing to just wait a specific time
> > needs explanation and is not part of "just" adding rk3576 support.
> 
> You are right, I didn't takle this issue.
> This is actually a bug, the else is not supposed to be there, it should only 
> be an added delay for some PDs.
> 
> Unfortunately, I'm not sure why that delay is needed exactly, so I'm willing 
> to remove it for now (only used by nputop and vop, both unsupported) and come 
> back to it if needed when VOP/NPU support is added.
> 
> Would that work for this upstream ?

that would work. The whole delay thing is not part of the dt-binding
which would be more critical to get right in the first round.

So as long as your "add rk3576 support" patch really only adds the
rk3576-specific data, but does not change how the shared code
behaves, we should be fine and can find out about that delay later.


Heiko
diff mbox series

Patch

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 9b76b62869d0d..863f1ad6b9e11 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -33,6 +33,7 @@ 
 #include <dt-bindings/power/rk3368-power.h>
 #include <dt-bindings/power/rk3399-power.h>
 #include <dt-bindings/power/rk3568-power.h>
+#include <dt-bindings/power/rockchip,rk3576-power.h>
 #include <dt-bindings/power/rk3588-power.h>
 
 struct rockchip_domain_info {
@@ -50,6 +51,7 @@  struct rockchip_domain_info {
 	u32 pwr_offset;
 	u32 mem_offset;
 	u32 req_offset;
+	u32 delay_us;
 };
 
 struct rockchip_pmu_info {
@@ -144,9 +146,27 @@  struct rockchip_pmu {
 	.active_wakeup = wakeup,			\
 }
 
-#define DOMAIN_RK3036(_name, req, ack, idle, wakeup)		\
+#define DOMAIN_M_O_R_G(_name, p_offset, pwr, status, r_status, r_offset, req, idle, ack, g_mask, delay, wakeup)	\
 {							\
-	.name = _name,				\
+	.name = _name,					\
+	.pwr_offset = p_offset,				\
+	.pwr_w_mask = (pwr) << 16,			\
+	.pwr_mask = (pwr),				\
+	.status_mask = (status),			\
+	.mem_status_mask = (r_status),			\
+	.repair_status_mask = (r_status),		\
+	.req_offset = r_offset,				\
+	.req_w_mask = (req) << 16,			\
+	.req_mask = (req),				\
+	.idle_mask = (idle),				\
+	.ack_mask = (ack),				\
+	.delay_us = delay,				\
+	.active_wakeup = wakeup,			\
+}
+
+#define DOMAIN_RK3036(_name, req, ack, idle, wakeup)	\
+{							\
+	.name = _name,					\
 	.req_mask = (req),				\
 	.req_w_mask = (req) << 16,			\
 	.ack_mask = (ack),				\
@@ -175,6 +195,9 @@  struct rockchip_pmu {
 #define DOMAIN_RK3568(name, pwr, req, wakeup)		\
 	DOMAIN_M(name, pwr, pwr, req, req, req, wakeup)
 
+#define DOMAIN_RK3576(name, p_offset, pwr, status, r_status, r_offset, req, idle, g_mask, delay, wakeup)	\
+	DOMAIN_M_O_R_G(name, p_offset, pwr, status, r_status, r_offset, req, idle, idle, g_mask, delay, wakeup)
+
 /*
  * Dynamic Memory Controller may need to coordinate with us -- see
  * rockchip_pmu_block().
@@ -552,7 +575,10 @@  static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
 			/* if powering up, leave idle mode */
 			rockchip_pmu_set_idle_request(pd, false);
 
-			rockchip_pmu_restore_qos(pd);
+			if (pd->info->delay_us)
+				udelay(pd->info->delay_us);
+			else
+				rockchip_pmu_restore_qos(pd);
 		}
 
 		clk_bulk_disable(pd->num_clks, pd->clks);
@@ -1106,6 +1132,28 @@  static const struct rockchip_domain_info rk3568_pm_domains[] = {
 	[RK3568_PD_PIPE]	= DOMAIN_RK3568("pipe", BIT(8), BIT(11), false),
 };
 
+static const struct rockchip_domain_info rk3576_pm_domains[] = {
+	[RK3576_PD_NPU]		= DOMAIN_RK3576("npu",    0x0, BIT(0),  BIT(0), 0,       0x0, 0,       0,       0,       0,    false),
+	[RK3576_PD_NVM]		= DOMAIN_RK3576("nvm",    0x0, BIT(6),  0,      BIT(6),  0x4, BIT(2),  BIT(18), BIT(2),  0,    false),
+	[RK3576_PD_SDGMAC]	= DOMAIN_RK3576("sdgmac", 0x0, BIT(7),  0,      BIT(7),  0x4, BIT(1),  BIT(17), 0x6,     0,    false),
+	[RK3576_PD_AUDIO]	= DOMAIN_RK3576("audio",  0x0, BIT(8),  0,      BIT(8),  0x4, BIT(0),  BIT(16), BIT(0),  0,    false),
+	[RK3576_PD_PHP]		= DOMAIN_RK3576("php",    0x0, BIT(9),  0,      BIT(9),  0x0, BIT(15), BIT(15), BIT(15), 0,    false),
+	[RK3576_PD_SUBPHP]	= DOMAIN_RK3576("subphp", 0x0, BIT(10), 0,      BIT(10), 0x0, 0,       0,       0,       0,    false),
+	[RK3576_PD_VOP]		= DOMAIN_RK3576("vop",    0x0, BIT(11), 0,      BIT(11), 0x0, 0x6000,  0x6000,  0x6000,  15,   false),
+	[RK3576_PD_VO1]		= DOMAIN_RK3576("vo1",    0x0, BIT(14), 0,      BIT(14), 0x0, BIT(12), BIT(12), 0x7000,  0,    false),
+	[RK3576_PD_VO0]		= DOMAIN_RK3576("vo0",    0x0, BIT(15), 0,      BIT(15), 0x0, BIT(11), BIT(11), 0x6800,  0,    false),
+	[RK3576_PD_USB]		= DOMAIN_RK3576("usb",    0x4, BIT(0),  0,      BIT(16), 0x0, BIT(10), BIT(10), 0x6400,  0,    true),
+	[RK3576_PD_VI]		= DOMAIN_RK3576("vi",     0x4, BIT(1),  0,      BIT(17), 0x0, BIT(9),  BIT(9),  BIT(9),  0,    false),
+	[RK3576_PD_VEPU0]	= DOMAIN_RK3576("vepu0",  0x4, BIT(2),  0,      BIT(18), 0x0, BIT(7),  BIT(7),  0x280,   0,    false),
+	[RK3576_PD_VEPU1]	= DOMAIN_RK3576("vepu1",  0x4, BIT(3),  0,      BIT(19), 0x0, BIT(8),  BIT(8),  BIT(8),  0,    false),
+	[RK3576_PD_VDEC]	= DOMAIN_RK3576("vdec",   0x4, BIT(4),  0,      BIT(20), 0x0, BIT(6),  BIT(6),  BIT(6),  0,    false),
+	[RK3576_PD_VPU]		= DOMAIN_RK3576("vpu",    0x4, BIT(5),  0,      BIT(21), 0x0, BIT(5),  BIT(5),  BIT(5),  0,    false),
+	[RK3576_PD_NPUTOP]	= DOMAIN_RK3576("nputop", 0x4, BIT(6),  0,      BIT(22), 0x0, 0x18,    0x18,    0x18,    15,   false),
+	[RK3576_PD_NPU0]	= DOMAIN_RK3576("npu0",   0x4, BIT(7),  0,      BIT(23), 0x0, BIT(1),  BIT(1),  0x1a,    0,    false),
+	[RK3576_PD_NPU1]	= DOMAIN_RK3576("npu1",   0x4, BIT(8),  0,      BIT(24), 0x0, BIT(2),  BIT(2),  0x1c,    0,    false),
+	[RK3576_PD_GPU]		= DOMAIN_RK3576("gpu",    0x4, BIT(9),  0,      BIT(25), 0x0, BIT(0),  BIT(0),  BIT(0),  0,    false),
+};
+
 static const struct rockchip_domain_info rk3588_pm_domains[] = {
 	[RK3588_PD_GPU]		= DOMAIN_RK3588("gpu",     0x0, BIT(0),  0,       0x0, 0,       BIT(1),  0x0, BIT(0),  BIT(0),  false),
 	[RK3588_PD_NPU]		= DOMAIN_RK3588("npu",     0x0, BIT(1),  BIT(1),  0x0, 0,       0,       0x0, 0,       0,       false),
@@ -1284,6 +1332,21 @@  static const struct rockchip_pmu_info rk3568_pmu = {
 	.domain_info = rk3568_pm_domains,
 };
 
+static const struct rockchip_pmu_info rk3576_pmu = {
+	.pwr_offset = 0x210,
+	.status_offset = 0x230,
+	.chain_status_offset = 0x248,
+	.mem_status_offset = 0x250,
+	.mem_pwr_offset = 0x300,
+	.req_offset = 0x110,
+	.idle_offset = 0x128,
+	.ack_offset = 0x120,
+	.repair_status_offset = 0x570,
+
+	.num_domains = ARRAY_SIZE(rk3576_pm_domains),
+	.domain_info = rk3576_pm_domains,
+};
+
 static const struct rockchip_pmu_info rk3588_pmu = {
 	.pwr_offset = 0x14c,
 	.status_offset = 0x180,
@@ -1359,6 +1422,10 @@  static const struct of_device_id rockchip_pm_domain_dt_match[] = {
 		.compatible = "rockchip,rk3568-power-controller",
 		.data = (void *)&rk3568_pmu,
 	},
+	{
+		.compatible = "rockchip,rk3576-power-controller",
+		.data = (void *)&rk3576_pmu,
+	},
 	{
 		.compatible = "rockchip,rk3588-power-controller",
 		.data = (void *)&rk3588_pmu,