2024-01-08 12:37:49

by Konrad Dybcio

Subject: [PATCH 00/18] Qualcomm GCC/VIDEOCC reset overhaul for Venus

Some resets associated with venus require a larger delay for the hardware
on the other end to function properly. These seem to fall into three
categories:

- 150us for 8250 and earlier
- 400us for 8350 and friends
- 1000us for >=8450

Make some housecleaning changes and describe these delays in preparation
to moving this data out of the venus driver.

Signed-off-by: Konrad Dybcio <[email protected]>
---
Konrad Dybcio (18):
clk: qcom: reset: Increase max reset delay
clk: qcom: reset: Commonize the de/assert functions
clk: qcom: reset: Ensure write completion on reset de/assertion
clk: qcom: gcc-sa8775p: Set delay for Venus CLK resets
clk: qcom: gcc-sc8180x: Set delay for Venus CLK resets
clk: qcom: gcc-sc8280xp: Set delay for Venus CLK resets
clk: qcom: gcc-sm4450: Set delay for Venus CLK resets
clk: qcom: gcc-sm7150: Set delay for Venus CLK resets
clk: qcom: gcc-sm8250: Set delay for Venus CLK resets
clk: qcom: gcc-sm8350: Set delay for Venus CLK resets
clk: qcom: gcc-sm8450: Set delay for Venus CLK resets
clk: qcom: gcc-sm8550: Set delay for Venus CLK resets
clk: qcom: gcc-sm8650: Set delay for Venus CLK resets
clk: qcom: videocc-sm8150: Set delay for Venus CLK resets
clk: qcom: videocc-sm8250: Set delay for Venus CLK resets
clk: qcom: videocc-sm8350: Set delay for Venus CLK resets
clk: qcom: videocc-sm8450: Set delay for Venus CLK resets
clk: qcom: videocc-sm8550: Set delay for Venus CLK resets

drivers/clk/qcom/gcc-sa8775p.c | 4 ++--
drivers/clk/qcom/gcc-sc8180x.c | 6 +++---
drivers/clk/qcom/gcc-sc8280xp.c | 4 ++--
drivers/clk/qcom/gcc-sm4450.c | 4 ++--
drivers/clk/qcom/gcc-sm7150.c | 2 +-
drivers/clk/qcom/gcc-sm8250.c | 4 ++--
drivers/clk/qcom/gcc-sm8350.c | 4 ++--
drivers/clk/qcom/gcc-sm8450.c | 4 ++--
drivers/clk/qcom/gcc-sm8550.c | 4 ++--
drivers/clk/qcom/gcc-sm8650.c | 4 ++--
drivers/clk/qcom/reset.c | 27 ++++++++++++++-------------
drivers/clk/qcom/reset.h | 2 +-
drivers/clk/qcom/videocc-sm8150.c | 2 +-
drivers/clk/qcom/videocc-sm8250.c | 4 ++--
drivers/clk/qcom/videocc-sm8350.c | 4 ++--
drivers/clk/qcom/videocc-sm8450.c | 4 ++--
drivers/clk/qcom/videocc-sm8550.c | 4 ++--
17 files changed, 44 insertions(+), 43 deletions(-)
---
base-commit: bffdfd2e7e63175ae261131a620f809d946cf9a7
change-id: 20240105-topic-venus_reset-b5461bf1a087

Best regards,
--
Konrad Dybcio <[email protected]>

2024-01-08 12:38:01

by Konrad Dybcio

Subject: [PATCH 01/18] clk: qcom: reset: Increase max reset delay

u8 limits us to 255 microseconds of delay. Promote the delay variable to
u16 to hold bigger values.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/reset.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/reset.h b/drivers/clk/qcom/reset.h
index 9a47c838d9b1..fe0561bf53d4 100644
--- a/drivers/clk/qcom/reset.h
+++ b/drivers/clk/qcom/reset.h
@@ -11,7 +11,7 @@
struct qcom_reset_map {
unsigned int reg;
u8 bit;
- u8 udelay;
+ u16 udelay;
u32 bitmask;
};


--
2.43.0

2024-01-08 12:38:40

by Konrad Dybcio

Subject: [PATCH 03/18] clk: qcom: reset: Ensure write completion on reset de/assertion

Trying to toggle the resets in a rapid fashion can lead to the changes
not actually arriving at the clock controller block when we expect them
to. This was observed at least on SM8250.

Read back the value after regmap_update_bits to ensure write completion.

Fixes: db1029814f1f ("clk: qcom: reset: Ensure write completion on reset de/assertion")
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/reset.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/reset.c b/drivers/clk/qcom/reset.c
index c4ac4d18829b..57024d1a0524 100644
--- a/drivers/clk/qcom/reset.c
+++ b/drivers/clk/qcom/reset.c
@@ -33,7 +33,12 @@ static int qcom_reset_set_assert(struct reset_controller_dev *rcdev, unsigned lo
map = &rst->reset_map[id];
mask = map->bitmask ? map->bitmask : BIT(map->bit);

- return regmap_update_bits(rst->regmap, map->reg, mask, assert ? mask : 0);
+ regmap_update_bits(rst->regmap, map->reg, mask, assert ? mask : 0);
+
+ /* Read back the register to ensure write completion, ignore the value */
+ regmap_read(rst->regmap, map->reg, &mask);
+
+ return 0;
}

static int qcom_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)

--
2.43.0

2024-01-08 12:38:57

by Konrad Dybcio

Subject: [PATCH 04/18] clk: qcom: gcc-sa8775p: Set delay for Venus CLK resets

Some Venus resets may require more time when toggling. Describe that.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gcc-sa8775p.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sa8775p.c b/drivers/clk/qcom/gcc-sa8775p.c
index 8171d23c96e6..c2b403cb6301 100644
--- a/drivers/clk/qcom/gcc-sa8775p.c
+++ b/drivers/clk/qcom/gcc-sa8775p.c
@@ -4662,8 +4662,8 @@ static const struct qcom_reset_map gcc_sa8775p_resets[] = {
[GCC_USB3UNIPHY_PHY_MP0_BCR] = { 0x5c020 },
[GCC_USB3UNIPHY_PHY_MP1_BCR] = { 0x5c024 },
[GCC_USB_PHY_CFG_AHB2PHY_BCR] = { 0x76000 },
- [GCC_VIDEO_AXI0_CLK_ARES] = { 0x34014, 2 },
- [GCC_VIDEO_AXI1_CLK_ARES] = { 0x3401c, 2 },
+ [GCC_VIDEO_AXI0_CLK_ARES] = { .reg = 0x34014, .bit = 2, .udelay = 400 },
+ [GCC_VIDEO_AXI1_CLK_ARES] = { .reg = 0x3401c, .bit = 2, .udelay = 400 },
[GCC_VIDEO_BCR] = { 0x34000 },
};


--
2.43.0

2024-01-08 12:39:15

by Konrad Dybcio

Subject: [PATCH 05/18] clk: qcom: gcc-sc8180x: Set delay for Venus CLK resets

Some Venus resets may require more time when toggling. Describe that.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gcc-sc8180x.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sc8180x.c b/drivers/clk/qcom/gcc-sc8180x.c
index ae2147381559..1351c52bcacb 100644
--- a/drivers/clk/qcom/gcc-sc8180x.c
+++ b/drivers/clk/qcom/gcc-sc8180x.c
@@ -4528,9 +4528,9 @@ static const struct qcom_reset_map gcc_sc8180x_resets[] = {
[GCC_USB30_PRIM_BCR] = { 0xf000 },
[GCC_USB30_SEC_BCR] = { 0x10000 },
[GCC_USB_PHY_CFG_AHB2PHY_BCR] = { 0x6a000 },
- [GCC_VIDEO_AXIC_CLK_BCR] = { 0xb02c, 2 },
- [GCC_VIDEO_AXI0_CLK_BCR] = { 0xb024, 2 },
- [GCC_VIDEO_AXI1_CLK_BCR] = { 0xb028, 2 },
+ [GCC_VIDEO_AXIC_CLK_BCR] = { .reg = 0xb02c, .bit = 2, .udelay = 150 },
+ [GCC_VIDEO_AXI0_CLK_BCR] = { .reg = 0xb024, .bit = 2, .udelay = 150 },
+ [GCC_VIDEO_AXI1_CLK_BCR] = { .reg = 0xb028, .bit = 2, .udelay = 150 },
};

static struct gdsc *gcc_sc8180x_gdscs[] = {

--
2.43.0

2024-01-08 12:39:33

by Konrad Dybcio

Subject: [PATCH 06/18] clk: qcom: gcc-sc8280xp: Set delay for Venus CLK resets

Some Venus resets may require more time when toggling. Describe that.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gcc-sc8280xp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sc8280xp.c b/drivers/clk/qcom/gcc-sc8280xp.c
index bfb77931e868..9f4db815688c 100644
--- a/drivers/clk/qcom/gcc-sc8280xp.c
+++ b/drivers/clk/qcom/gcc-sc8280xp.c
@@ -7448,8 +7448,8 @@ static const struct qcom_reset_map gcc_sc8280xp_resets[] = {
[GCC_USB4PHY_PHY_PRIM_BCR] = { 0x4a004 },
[GCC_USB_PHY_CFG_AHB2PHY_BCR] = { 0x6a000 },
[GCC_VIDEO_BCR] = { 0x28000 },
- [GCC_VIDEO_AXI0_CLK_ARES] = { 0x28010, 2 },
- [GCC_VIDEO_AXI1_CLK_ARES] = { 0x28018, 2 },
+ [GCC_VIDEO_AXI0_CLK_ARES] = { .reg = 0x28010, .bit = 2, .udelay = 400 },
+ [GCC_VIDEO_AXI1_CLK_ARES] = { .reg = 0x28018, .bit = 2, .udelay = 400 },
};

static struct gdsc *gcc_sc8280xp_gdscs[] = {

--
2.43.0

2024-01-08 12:39:49

by Konrad Dybcio

Subject: [PATCH 07/18] clk: qcom: gcc-sm4450: Set delay for Venus CLK resets

Some Venus resets may require more time when toggling. Describe that.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gcc-sm4450.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sm4450.c b/drivers/clk/qcom/gcc-sm4450.c
index 31abe2775fc8..ab8fb77d15a2 100644
--- a/drivers/clk/qcom/gcc-sm4450.c
+++ b/drivers/clk/qcom/gcc-sm4450.c
@@ -2791,8 +2791,8 @@ static const struct qcom_reset_map gcc_sm4450_resets[] = {
[GCC_VENUS_BCR] = { 0xb601c },
[GCC_VIDEO_BCR] = { 0x42000 },
[GCC_VIDEO_VENUS_BCR] = { 0xb6000 },
- [GCC_VENUS_CTL_AXI_CLK_ARES] = { 0x4201c, 2 },
- [GCC_VIDEO_VENUS_CTL_CLK_ARES] = { 0xb6038, 2 },
+ [GCC_VENUS_CTL_AXI_CLK_ARES] = { .reg = 0x4201c, .bit = 2, .udelay = 400 },
+ [GCC_VIDEO_VENUS_CTL_CLK_ARES] = { .reg = 0xb6038, .bit = 2, .udelay = 400 },
};

static const struct clk_rcg_dfs_data gcc_dfs_clocks[] = {

--
2.43.0

2024-01-08 12:40:08

by Konrad Dybcio

Subject: [PATCH 08/18] clk: qcom: gcc-sm7150: Set delay for Venus CLK resets

Some Venus resets may require more time when toggling. Describe that.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gcc-sm7150.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/gcc-sm7150.c b/drivers/clk/qcom/gcc-sm7150.c
index d9983bb27475..7c5596331c30 100644
--- a/drivers/clk/qcom/gcc-sm7150.c
+++ b/drivers/clk/qcom/gcc-sm7150.c
@@ -2918,7 +2918,7 @@ static const struct qcom_reset_map gcc_sm7150_resets[] = {
[GCC_USB3_PHY_PRIM_BCR] = { 0x50000 },
[GCC_USB3_PHY_SEC_BCR] = { 0x5000c },
[GCC_QUSB2PHY_PRIM_BCR] = { 0x26000 },
- [GCC_VIDEO_AXI_CLK_BCR] = { 0xb01c, 2 },
+ [GCC_VIDEO_AXI_CLK_BCR] = { .reg = 0xb01c, .bit = 2, .udelay = 150 },
};

static const struct clk_rcg_dfs_data gcc_sm7150_dfs_desc[] = {

--
2.43.0

2024-01-08 12:40:23

by Konrad Dybcio

Subject: [PATCH 09/18] clk: qcom: gcc-sm8250: Set delay for Venus CLK resets

Some Venus resets may require more time when toggling. Describe that.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gcc-sm8250.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sm8250.c b/drivers/clk/qcom/gcc-sm8250.c
index c6c5261264f1..61d01d4c379b 100644
--- a/drivers/clk/qcom/gcc-sm8250.c
+++ b/drivers/clk/qcom/gcc-sm8250.c
@@ -3576,8 +3576,8 @@ static const struct qcom_reset_map gcc_sm8250_resets[] = {
[GCC_USB3PHY_PHY_PRIM_BCR] = { 0x50004 },
[GCC_USB3PHY_PHY_SEC_BCR] = { 0x50010 },
[GCC_USB_PHY_CFG_AHB2PHY_BCR] = { 0x6a000 },
- [GCC_VIDEO_AXI0_CLK_ARES] = { 0xb024, 2 },
- [GCC_VIDEO_AXI1_CLK_ARES] = { 0xb028, 2 },
+ [GCC_VIDEO_AXI0_CLK_ARES] = { 0xb024, .bit = 2, .udelay = 150 },
+ [GCC_VIDEO_AXI1_CLK_ARES] = { 0xb028, .bit = 2, .udelay = 150 },
};

static const struct clk_rcg_dfs_data gcc_dfs_clocks[] = {

--
2.43.0

2024-01-08 12:40:39

by Konrad Dybcio

Subject: [PATCH 10/18] clk: qcom: gcc-sm8350: Set delay for Venus CLK resets

Some Venus resets may require more time when toggling. Describe that.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gcc-sm8350.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sm8350.c b/drivers/clk/qcom/gcc-sm8350.c
index 1385a98eb3bb..df4842588a24 100644
--- a/drivers/clk/qcom/gcc-sm8350.c
+++ b/drivers/clk/qcom/gcc-sm8350.c
@@ -3743,8 +3743,8 @@ static const struct qcom_reset_map gcc_sm8350_resets[] = {
[GCC_USB3PHY_PHY_PRIM_BCR] = { 0x50004 },
[GCC_USB3PHY_PHY_SEC_BCR] = { 0x50010 },
[GCC_USB_PHY_CFG_AHB2PHY_BCR] = { 0x6a000 },
- [GCC_VIDEO_AXI0_CLK_ARES] = { 0x28010, 2 },
- [GCC_VIDEO_AXI1_CLK_ARES] = { 0x28018, 2 },
+ [GCC_VIDEO_AXI0_CLK_ARES] = { .reg = 0x28010, .bit = 2, .udelay = 400 },
+ [GCC_VIDEO_AXI1_CLK_ARES] = { .reg = 0x28018, .bit = 2, .udelay = 400 },
[GCC_VIDEO_BCR] = { 0x28000 },
};


--
2.43.0

2024-01-08 12:41:00

by Konrad Dybcio

Subject: [PATCH 11/18] clk: qcom: gcc-sm8450: Set delay for Venus CLK resets

Some Venus resets may require more time when toggling. Describe that.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gcc-sm8450.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sm8450.c b/drivers/clk/qcom/gcc-sm8450.c
index 563542982551..1825b3456dd0 100644
--- a/drivers/clk/qcom/gcc-sm8450.c
+++ b/drivers/clk/qcom/gcc-sm8450.c
@@ -3202,8 +3202,8 @@ static const struct qcom_reset_map gcc_sm8450_resets[] = {
[GCC_USB3PHY_PHY_PRIM_BCR] = { 0x60004 },
[GCC_USB3PHY_PHY_SEC_BCR] = { 0x60010 },
[GCC_USB_PHY_CFG_AHB2PHY_BCR] = { 0x7a000 },
- [GCC_VIDEO_AXI0_CLK_ARES] = { 0x42018, 2 },
- [GCC_VIDEO_AXI1_CLK_ARES] = { 0x42020, 2 },
+ [GCC_VIDEO_AXI0_CLK_ARES] = { .reg = 0x42018, .bit = 2, .udelay = 1000 },
+ [GCC_VIDEO_AXI1_CLK_ARES] = { .reg = 0x42020, .bit = 2, .udelay = 1000 },
[GCC_VIDEO_BCR] = { 0x42000 },
};


--
2.43.0

2024-01-08 12:41:32

by Konrad Dybcio

Subject: [PATCH 13/18] clk: qcom: gcc-sm8650: Set delay for Venus CLK resets

Some Venus resets may require more time when toggling. Describe that.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gcc-sm8650.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sm8650.c b/drivers/clk/qcom/gcc-sm8650.c
index 9174dd82308c..63becb03cd90 100644
--- a/drivers/clk/qcom/gcc-sm8650.c
+++ b/drivers/clk/qcom/gcc-sm8650.c
@@ -3734,8 +3734,8 @@ static const struct qcom_reset_map gcc_sm8650_resets[] = {
[GCC_USB3_PHY_SEC_BCR] = { 0x5000c },
[GCC_USB3PHY_PHY_PRIM_BCR] = { 0x50004 },
[GCC_USB3PHY_PHY_SEC_BCR] = { 0x50010 },
- [GCC_VIDEO_AXI0_CLK_ARES] = { 0x32018, 2 },
- [GCC_VIDEO_AXI1_CLK_ARES] = { 0x32024, 2 },
+ [GCC_VIDEO_AXI0_CLK_ARES] = { .reg = 0x32018, .bit = 2, .udelay = 1000 },
+ [GCC_VIDEO_AXI1_CLK_ARES] = { .reg = 0x32024, .bit = 2, .udelay = 1000 },
[GCC_VIDEO_BCR] = { 0x32000 },
};


--
2.43.0

2024-01-08 12:41:50

by Konrad Dybcio

Subject: [PATCH 14/18] clk: qcom: videocc-sm8150: Set delay for Venus CLK resets

Some Venus resets may require more time when toggling. Describe that.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/videocc-sm8150.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/videocc-sm8150.c b/drivers/clk/qcom/videocc-sm8150.c
index f1456eaa87c4..bead5186a5d6 100644
--- a/drivers/clk/qcom/videocc-sm8150.c
+++ b/drivers/clk/qcom/videocc-sm8150.c
@@ -215,7 +215,7 @@ static const struct regmap_config video_cc_sm8150_regmap_config = {
};

static const struct qcom_reset_map video_cc_sm8150_resets[] = {
- [VIDEO_CC_MVSC_CORE_CLK_BCR] = { 0x850, 2 },
+ [VIDEO_CC_MVSC_CORE_CLK_BCR] = { .reg = 0x850, .bit = 2, .udelay = 150 },
[VIDEO_CC_INTERFACE_BCR] = { 0x8f0 },
[VIDEO_CC_MVS0_BCR] = { 0x870 },
[VIDEO_CC_MVS1_BCR] = { 0x8b0 },

--
2.43.0

2024-01-08 12:41:56

by Konrad Dybcio

Subject: [PATCH 02/18] clk: qcom: reset: Commonize the de/assert functions

They do the same thing, except the last argument of the last function
call differs. Commonize them.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/reset.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/qcom/reset.c b/drivers/clk/qcom/reset.c
index e45e32804d2c..c4ac4d18829b 100644
--- a/drivers/clk/qcom/reset.c
+++ b/drivers/clk/qcom/reset.c
@@ -22,8 +22,8 @@ static int qcom_reset(struct reset_controller_dev *rcdev, unsigned long id)
return 0;
}

-static int
-qcom_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
+static int qcom_reset_set_assert(struct reset_controller_dev *rcdev, unsigned long id,
+ bool assert)
{
struct qcom_reset_controller *rst;
const struct qcom_reset_map *map;
@@ -33,21 +33,17 @@ qcom_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
map = &rst->reset_map[id];
mask = map->bitmask ? map->bitmask : BIT(map->bit);

- return regmap_update_bits(rst->regmap, map->reg, mask, mask);
+ return regmap_update_bits(rst->regmap, map->reg, mask, assert ? mask : 0);
}

-static int
-qcom_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id)
+static int qcom_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
{
- struct qcom_reset_controller *rst;
- const struct qcom_reset_map *map;
- u32 mask;
-
- rst = to_qcom_reset_controller(rcdev);
- map = &rst->reset_map[id];
- mask = map->bitmask ? map->bitmask : BIT(map->bit);
+ return qcom_reset_set_assert(rcdev, id, true);
+}

- return regmap_update_bits(rst->regmap, map->reg, mask, 0);
+static int qcom_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+ return qcom_reset_set_assert(rcdev, id, false);
}

const struct reset_control_ops qcom_reset_ops = {

--
2.43.0

2024-01-08 12:42:06

by Konrad Dybcio

Subject: [PATCH 15/18] clk: qcom: videocc-sm8250: Set delay for Venus CLK resets

Some Venus resets may require more time when toggling. Describe that.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/videocc-sm8250.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/videocc-sm8250.c b/drivers/clk/qcom/videocc-sm8250.c
index ad46c4014a40..51b9816ec458 100644
--- a/drivers/clk/qcom/videocc-sm8250.c
+++ b/drivers/clk/qcom/videocc-sm8250.c
@@ -323,10 +323,10 @@ static struct clk_regmap *video_cc_sm8250_clocks[] = {
static const struct qcom_reset_map video_cc_sm8250_resets[] = {
[VIDEO_CC_CVP_INTERFACE_BCR] = { 0xe54 },
[VIDEO_CC_CVP_MVS0_BCR] = { 0xd14 },
- [VIDEO_CC_MVS0C_CLK_ARES] = { 0xc34, 2 },
+ [VIDEO_CC_MVS0C_CLK_ARES] = { 0xc34, .bit = 2, .udelay = 150 },
[VIDEO_CC_CVP_MVS0C_BCR] = { 0xbf4 },
[VIDEO_CC_CVP_MVS1_BCR] = { 0xd94 },
- [VIDEO_CC_MVS1C_CLK_ARES] = { 0xcd4, 2 },
+ [VIDEO_CC_MVS1C_CLK_ARES] = { 0xcd4, .bit = 2, .udelay = 150 },
[VIDEO_CC_CVP_MVS1C_BCR] = { 0xc94 },
};


--
2.43.0

2024-01-08 12:42:20

by Konrad Dybcio

Subject: [PATCH 16/18] clk: qcom: videocc-sm8350: Set delay for Venus CLK resets

Some Venus resets may require more time when toggling. Describe that.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/videocc-sm8350.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/videocc-sm8350.c b/drivers/clk/qcom/videocc-sm8350.c
index 7246f3c99492..8db2bb995558 100644
--- a/drivers/clk/qcom/videocc-sm8350.c
+++ b/drivers/clk/qcom/videocc-sm8350.c
@@ -488,10 +488,10 @@ static struct clk_regmap *video_cc_sm8350_clocks[] = {
static const struct qcom_reset_map video_cc_sm8350_resets[] = {
[VIDEO_CC_CVP_INTERFACE_BCR] = { 0xe54 },
[VIDEO_CC_CVP_MVS0_BCR] = { 0xd14 },
- [VIDEO_CC_MVS0C_CLK_ARES] = { 0xc34, 2 },
+ [VIDEO_CC_MVS0C_CLK_ARES] = { .reg = 0xc34, .bit = 2, .udelay = 400 },
[VIDEO_CC_CVP_MVS0C_BCR] = { 0xbf4 },
[VIDEO_CC_CVP_MVS1_BCR] = { 0xd94 },
- [VIDEO_CC_MVS1C_CLK_ARES] = { 0xcd4, 2 },
+ [VIDEO_CC_MVS1C_CLK_ARES] = { .reg = 0xcd4, .bit = 2, .udelay = 400 },
[VIDEO_CC_CVP_MVS1C_BCR] = { 0xc94 },
};


--
2.43.0

2024-01-08 12:42:40

by Konrad Dybcio

Subject: [PATCH 17/18] clk: qcom: videocc-sm8450: Set delay for Venus CLK resets

Some Venus resets may require more time when toggling. Describe that.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/videocc-sm8450.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/videocc-sm8450.c b/drivers/clk/qcom/videocc-sm8450.c
index 16a61146e619..67ca302a0737 100644
--- a/drivers/clk/qcom/videocc-sm8450.c
+++ b/drivers/clk/qcom/videocc-sm8450.c
@@ -373,8 +373,8 @@ static const struct qcom_reset_map video_cc_sm8450_resets[] = {
[CVP_VIDEO_CC_MVS0C_BCR] = { 0x8048 },
[CVP_VIDEO_CC_MVS1_BCR] = { 0x80bc },
[CVP_VIDEO_CC_MVS1C_BCR] = { 0x8070 },
- [VIDEO_CC_MVS0C_CLK_ARES] = { 0x8064, 2 },
- [VIDEO_CC_MVS1C_CLK_ARES] = { 0x808c, 2 },
+ [VIDEO_CC_MVS0C_CLK_ARES] = { .reg = 0x8064, .bit = 2, .udelay = 1000 },
+ [VIDEO_CC_MVS1C_CLK_ARES] = { .reg = 0x808c, .bit = 2, .udelay = 1000 },
};

static const struct regmap_config video_cc_sm8450_regmap_config = {

--
2.43.0

2024-01-08 12:42:55

by Konrad Dybcio

Subject: [PATCH 18/18] clk: qcom: videocc-sm8550: Set delay for Venus CLK resets

Some Venus resets may require more time when toggling. Describe that.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/videocc-sm8550.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/videocc-sm8550.c b/drivers/clk/qcom/videocc-sm8550.c
index f3c9dfaee968..e3f146347da7 100644
--- a/drivers/clk/qcom/videocc-sm8550.c
+++ b/drivers/clk/qcom/videocc-sm8550.c
@@ -378,8 +378,8 @@ static const struct qcom_reset_map video_cc_sm8550_resets[] = {
[CVP_VIDEO_CC_MVS0C_BCR] = { 0x8048 },
[CVP_VIDEO_CC_MVS1_BCR] = { 0x80c8 },
[CVP_VIDEO_CC_MVS1C_BCR] = { 0x8074 },
- [VIDEO_CC_MVS0C_CLK_ARES] = { 0x8064, 2 },
- [VIDEO_CC_MVS1C_CLK_ARES] = { 0x8090, 2 },
+ [VIDEO_CC_MVS0C_CLK_ARES] = { .reg = 0x8064, .bit = 2, .udelay = 1000 },
+ [VIDEO_CC_MVS1C_CLK_ARES] = { .reg = 0x8090, .bit = 2, .udelay = 1000 },
};

static const struct regmap_config video_cc_sm8550_regmap_config = {

--
2.43.0

2024-01-08 12:44:53

by Konrad Dybcio

Subject: [PATCH 12/18] clk: qcom: gcc-sm8550: Set delay for Venus CLK resets

Some Venus resets may require more time when toggling. Describe that.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gcc-sm8550.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sm8550.c b/drivers/clk/qcom/gcc-sm8550.c
index b883dffe5f7a..4cbc728f5c72 100644
--- a/drivers/clk/qcom/gcc-sm8550.c
+++ b/drivers/clk/qcom/gcc-sm8550.c
@@ -3276,8 +3276,8 @@ static const struct qcom_reset_map gcc_sm8550_resets[] = {
[GCC_USB3PHY_PHY_PRIM_BCR] = { 0x50004 },
[GCC_USB3PHY_PHY_SEC_BCR] = { 0x50010 },
[GCC_USB_PHY_CFG_AHB2PHY_BCR] = { 0x6a000 },
- [GCC_VIDEO_AXI0_CLK_ARES] = { 0x32018, 2 },
- [GCC_VIDEO_AXI1_CLK_ARES] = { 0x32024, 2 },
+ [GCC_VIDEO_AXI0_CLK_ARES] = { .reg = 0x32018, .bit = 2, .udelay = 1000 },
+ [GCC_VIDEO_AXI1_CLK_ARES] = { .reg = 0x32024, .bit = 2, .udelay = 1000 },
[GCC_VIDEO_BCR] = { 0x32000 },
};


--
2.43.0

2024-01-08 23:45:23

by Bryan O'Donoghue

Subject: Re: [PATCH 00/18] Qualcomm GCC/VIDEOCC reset overhaul for Venus

On 08/01/2024 12:32, Konrad Dybcio wrote:
> Some resets associated with venus require a larger delay for the hardware
> on the other end to function properly. These seem to fall into three
> categories:
>
> - 150us for 8250 and earlier
> - 400us for 8350 and friends
> - 1000us for >=8450
>

What's your reference for these delay values ?

---
bod

2024-01-09 00:30:22

by Bryan O'Donoghue

Subject: Re: [PATCH 02/18] clk: qcom: reset: Commonize the de/assert functions

On 08/01/2024 12:32, Konrad Dybcio wrote:
> -static int
> -qcom_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
> +static int qcom_reset_set_assert(struct reset_controller_dev *rcdev, unsigned long id,
> + bool assert)

Personally I'd not elongate the function declaration.

> {
> struct qcom_reset_controller *rst;
> const struct qcom_reset_map *map;
> @@ -33,21 +33,17 @@ qcom_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
> map = &rst->reset_map[id];
> mask = map->bitmask ? map->bitmask : BIT(map->bit);
>
> - return regmap_update_bits(rst->regmap, map->reg, mask, mask);
> + return regmap_update_bits(rst->regmap, map->reg, mask, assert ? mask : 0);

and I'd probably do

u32 bits = 0;

if (assert)
bits = mask;

regmap_update_bits(rst->regmap, map->reg, mask, bits);

because I prefer for aesthetic reasons not to do ternary inputs like that.

However its up to you to change or not.

Reviewed-by: Bryan O'Donoghue <[email protected]>

---
bod

2024-01-09 00:35:02

by Bryan O'Donoghue

Subject: Re: [PATCH 09/18] clk: qcom: gcc-sm8250: Set delay for Venus CLK resets

On 08/01/2024 12:32, Konrad Dybcio wrote:
> Some Venus resets may require more time when toggling. Describe that.

May or does ?

I'd prefer a strong declaration of where this value came from and why
its being added.

May is ambiguous.

"Downstream has a 150 us delay for this. My own testing shows this to be
necessary in upstream"

Later commits want to add a 1000 us delay. Have all of these delays been
tested ?

If not please describe where the values come.

> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/clk/qcom/gcc-sm8250.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/qcom/gcc-sm8250.c b/drivers/clk/qcom/gcc-sm8250.c
> index c6c5261264f1..61d01d4c379b 100644
> --- a/drivers/clk/qcom/gcc-sm8250.c
> +++ b/drivers/clk/qcom/gcc-sm8250.c
> @@ -3576,8 +3576,8 @@ static const struct qcom_reset_map gcc_sm8250_resets[] = {
> [GCC_USB3PHY_PHY_PRIM_BCR] = { 0x50004 },
> [GCC_USB3PHY_PHY_SEC_BCR] = { 0x50010 },
> [GCC_USB_PHY_CFG_AHB2PHY_BCR] = { 0x6a000 },
> - [GCC_VIDEO_AXI0_CLK_ARES] = { 0xb024, 2 },
> - [GCC_VIDEO_AXI1_CLK_ARES] = { 0xb028, 2 },
> + [GCC_VIDEO_AXI0_CLK_ARES] = { 0xb024, .bit = 2, .udelay = 150 },
> + [GCC_VIDEO_AXI1_CLK_ARES] = { 0xb028, .bit = 2, .udelay = 150 },
> };
>
> static const struct clk_rcg_dfs_data gcc_dfs_clocks[] = {
>

2024-01-09 09:33:53

by Konrad Dybcio

Subject: Re: [PATCH 09/18] clk: qcom: gcc-sm8250: Set delay for Venus CLK resets

On 1/9/24 01:34, Bryan O'Donoghue wrote:
> On 08/01/2024 12:32, Konrad Dybcio wrote:
>> Some Venus resets may require more time when toggling. Describe that.
>
> May or does ?
>
> I'd prefer a strong declaration of where this value came from and why its being added.
>
> May is ambiguous.
>
> "Downstream has a 150 us delay for this. My own testing shows this to be necessary in upstream"

Alright

>
> Later commits want to add a 1000 us delay. Have all of these delays been tested ?

No, we don't support Venus on many of the newer SoCs..


>
> If not please describe where the values come.

They come from the downstream Venus driver as you mentioned.
I checked a couple different downstream SoC kernel trees and
tried to assign the values based on what I found in a kernel
for that platform. Some are fairly educated guesses.

Konrad

2024-01-27 23:05:59

by Bjorn Andersson

Subject: Re: Re: [PATCH 09/18] clk: qcom: gcc-sm8250: Set delay for Venus CLK resets

On Tue, Jan 09, 2024 at 10:33:39AM +0100, Konrad Dybcio wrote:
>
>
> On 1/9/24 01:34, Bryan O'Donoghue wrote:
> > On 08/01/2024 12:32, Konrad Dybcio wrote:
> > > Some Venus resets may require more time when toggling. Describe that.
> >
> > May or does ?
> >
> > I'd prefer a strong declaration of where this value came from and why its being added.
> >
> > May is ambiguous.
> >
> > "Downstream has a 150 us delay for this. My own testing shows this to be necessary in upstream"
>
> Alright
>
> >
> > Later commits want to add a 1000 us delay. Have all of these delays been tested ?
>
> No, we don't support Venus on many of the newer SoCs..
>
>
> >
> > If not please describe where the values come.
>
> They come from the downstream Venus driver as you mentioned.
> I checked a couple different downstream SoC kernel trees and
> tried to assign the values based on what I found in a kernel
> for that platform. Some are fairly educated guesses.
>

It would be nice to have documented for which cases you guessed (and in
which downstream kernel you found other values?), so that if anyone is
coming to the tree later with conflicting information they have a better
chance to reason about the discrepancy.

Thanks,
Bjorn