* [PATCH v2 01/20] media: venus: pm_helpers: Only set rate of the core clock in core_clks_enable
2024-02-09 21:09 [PATCH v2 00/20] Venus cleanups Konrad Dybcio
@ 2024-02-09 21:09 ` Konrad Dybcio
2024-02-09 21:09 ` [PATCH v2 02/20] media: venus: pm_helpers: Rename core_clks_get to venus_clks_get Konrad Dybcio
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Konrad Dybcio @ 2024-02-09 21:09 UTC (permalink / raw)
To: Stanimir Varbanov, Vikash Garodia, Bryan O'Donoghue,
Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab,
Dikshita Agarwal, Philipp Zabel
Cc: Marijn Suijten, Stanimir Varbanov, Mauro Carvalho Chehab,
linux-media, linux-arm-msm, linux-kernel, Konrad Dybcio
Commit c22b1a29497c ("media: venus: core,pm: Vote for min clk freq
during venus boot") intended to up the rate of the Venus core clock
from the XO minimum to something more reasonable, based on the per-
SoC frequency table.
Unfortunately, it ended up calling set_rate with that same argument
on all clocks in res->clks. Fix that using the OPP API.
Fixes: c22b1a29497c ("media: venus: core,pm: Vote for min clk freq during venus boot")
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/media/platform/qcom/venus/pm_helpers.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
index 502822059498..8bd0ce4ce69d 100644
--- a/drivers/media/platform/qcom/venus/pm_helpers.c
+++ b/drivers/media/platform/qcom/venus/pm_helpers.c
@@ -41,24 +41,23 @@ static int core_clks_get(struct venus_core *core)
static int core_clks_enable(struct venus_core *core)
{
const struct venus_resources *res = core->res;
- const struct freq_tbl *freq_tbl = core->res->freq_tbl;
- unsigned int freq_tbl_size = core->res->freq_tbl_size;
- unsigned long freq;
+ struct dev_pm_opp *opp;
+ unsigned long freq = 0;
unsigned int i;
int ret;
- if (!freq_tbl)
- return -EINVAL;
+ if (core->has_opp_table) {
+ opp = dev_pm_opp_find_freq_ceil(core->dev, &freq);
+ if (IS_ERR(opp))
+ return PTR_ERR(opp);
+ dev_pm_opp_put(opp);
- freq = freq_tbl[freq_tbl_size - 1].freq;
+ ret = dev_pm_opp_set_rate(core->dev, freq);
+ if (ret)
+ return ret;
+ }
for (i = 0; i < res->clks_num; i++) {
- if (IS_V6(core)) {
- ret = clk_set_rate(core->clks[i], freq);
- if (ret)
- goto err;
- }
-
ret = clk_prepare_enable(core->clks[i]);
if (ret)
goto err;
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 02/20] media: venus: pm_helpers: Rename core_clks_get to venus_clks_get
2024-02-09 21:09 [PATCH v2 00/20] Venus cleanups Konrad Dybcio
2024-02-09 21:09 ` [PATCH v2 01/20] media: venus: pm_helpers: Only set rate of the core clock in core_clks_enable Konrad Dybcio
@ 2024-02-09 21:09 ` Konrad Dybcio
2024-02-09 21:09 ` [PATCH v2 03/20] media: venus: pm_helpers: Add kerneldoc to venus_clks_get() Konrad Dybcio
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Konrad Dybcio @ 2024-02-09 21:09 UTC (permalink / raw)
To: Stanimir Varbanov, Vikash Garodia, Bryan O'Donoghue,
Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab,
Dikshita Agarwal, Philipp Zabel
Cc: Marijn Suijten, Stanimir Varbanov, Mauro Carvalho Chehab,
linux-media, linux-arm-msm, linux-kernel, Konrad Dybcio
"core" is used in multiple contexts when talking about Venus, rename
the function to save on confusion.
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/media/platform/qcom/venus/pm_helpers.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
index 8bd0ce4ce69d..ac7c83404c6e 100644
--- a/drivers/media/platform/qcom/venus/pm_helpers.c
+++ b/drivers/media/platform/qcom/venus/pm_helpers.c
@@ -23,7 +23,7 @@
static bool legacy_binding;
-static int core_clks_get(struct venus_core *core)
+static int venus_clks_get(struct venus_core *core)
{
const struct venus_resources *res = core->res;
struct device *dev = core->dev;
@@ -294,7 +294,7 @@ static int core_get_v1(struct venus_core *core)
{
int ret;
- ret = core_clks_get(core);
+ ret = venus_clks_get(core);
if (ret)
return ret;
@@ -961,7 +961,7 @@ static int core_get_v4(struct venus_core *core)
const struct venus_resources *res = core->res;
int ret;
- ret = core_clks_get(core);
+ ret = venus_clks_get(core);
if (ret)
return ret;
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 03/20] media: venus: pm_helpers: Add kerneldoc to venus_clks_get()
2024-02-09 21:09 [PATCH v2 00/20] Venus cleanups Konrad Dybcio
2024-02-09 21:09 ` [PATCH v2 01/20] media: venus: pm_helpers: Only set rate of the core clock in core_clks_enable Konrad Dybcio
2024-02-09 21:09 ` [PATCH v2 02/20] media: venus: pm_helpers: Rename core_clks_get to venus_clks_get Konrad Dybcio
@ 2024-02-09 21:09 ` Konrad Dybcio
2024-02-09 21:09 ` [PATCH v2 04/20] media: venus: core: Set OPP clkname in a common code path Konrad Dybcio
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Konrad Dybcio @ 2024-02-09 21:09 UTC (permalink / raw)
To: Stanimir Varbanov, Vikash Garodia, Bryan O'Donoghue,
Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab,
Dikshita Agarwal, Philipp Zabel
Cc: Marijn Suijten, Stanimir Varbanov, Mauro Carvalho Chehab,
linux-media, linux-arm-msm, linux-kernel, Konrad Dybcio
To make it easier to understand the various clock requirements within
this driver, add kerneldoc to venus_clk_get() explaining the fluff.
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/media/platform/qcom/venus/pm_helpers.c | 28 ++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
index ac7c83404c6e..ea0a7d4601e2 100644
--- a/drivers/media/platform/qcom/venus/pm_helpers.c
+++ b/drivers/media/platform/qcom/venus/pm_helpers.c
@@ -23,6 +23,34 @@
static bool legacy_binding;
+/**
+ * venus_clks_get() - Get Venus clocks that are not bound to a vcodec
+ * @core: A pointer to the venus core resource
+ *
+ * The Venus block (depending on the generation) can be split into a couple
+ * of clock domains: one for "main logic" and one for each video core (0-2pcs).
+ *
+ * MSM8916 (and possibly other HFIv1 users) only feature the "main logic"
+ * domain, so this function is the only kind if clk_get necessary there.
+ *
+ * MSM8996 (and other HFIv3 users) feature two video cores, with core0 being
+ * statically proclaimed a decoder and core1 an encoder, with both having
+ * their own clock domains.
+ *
+ * SDM845 features two video cores, each one of which may or may not be
+ * subdivided into 2 enc/dec threads.
+ *
+ * Other SoCs either feature a single video core (with its own clock domain)
+ * or 1 video core and 1 CVP (Computer Vision Processor) core. In both cases
+ * we treat it the same (CVP only happens to live near-by Venus on the SoC).
+ *
+ * Due to unfortunate developments in the past, we have to support bindings
+ * (MSM8996, SDM660, SDM845) that require specifying the clocks and
+ * power-domains associated with a video core domain in a bogus subnode,
+ * which means that additional fluff is necessary..
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
static int venus_clks_get(struct venus_core *core)
{
const struct venus_resources *res = core->res;
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 04/20] media: venus: core: Set OPP clkname in a common code path
2024-02-09 21:09 [PATCH v2 00/20] Venus cleanups Konrad Dybcio
` (2 preceding siblings ...)
2024-02-09 21:09 ` [PATCH v2 03/20] media: venus: pm_helpers: Add kerneldoc to venus_clks_get() Konrad Dybcio
@ 2024-02-09 21:09 ` Konrad Dybcio
2024-02-09 21:09 ` [PATCH v2 05/20] media: venus: pm_helpers: Kill dead code Konrad Dybcio
2024-02-09 21:10 ` [PATCH v2 00/20] Venus cleanups Konrad Dybcio
5 siblings, 0 replies; 10+ messages in thread
From: Konrad Dybcio @ 2024-02-09 21:09 UTC (permalink / raw)
To: Stanimir Varbanov, Vikash Garodia, Bryan O'Donoghue,
Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab,
Dikshita Agarwal, Philipp Zabel
Cc: Marijn Suijten, Stanimir Varbanov, Mauro Carvalho Chehab,
linux-media, linux-arm-msm, linux-kernel, Konrad Dybcio
Calling devm_pm_opp_set_clkname() is repeated for all HFI versions in
pm_ops->core_power.
Move it to the common codepath.
This also lets us get rid of core_get_v1.
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/media/platform/qcom/venus/core.c | 5 +++++
drivers/media/platform/qcom/venus/pm_helpers.c | 23 ++---------------------
2 files changed, 7 insertions(+), 21 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
index ce206b709754..5ab3c414ec0f 100644
--- a/drivers/media/platform/qcom/venus/core.c
+++ b/drivers/media/platform/qcom/venus/core.c
@@ -14,6 +14,7 @@
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/pm_domain.h>
@@ -319,6 +320,10 @@ static int venus_probe(struct platform_device *pdev)
if (!core->pm_ops)
return -ENODEV;
+ ret = devm_pm_opp_set_clkname(dev, "core");
+ if (ret)
+ return ret;
+
if (core->pm_ops->core_get) {
ret = core->pm_ops->core_get(core);
if (ret)
diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
index ea0a7d4601e2..1ba65345a5e2 100644
--- a/drivers/media/platform/qcom/venus/pm_helpers.c
+++ b/drivers/media/platform/qcom/venus/pm_helpers.c
@@ -318,21 +318,6 @@ static int load_scale_v1(struct venus_inst *inst)
return ret;
}
-static int core_get_v1(struct venus_core *core)
-{
- int ret;
-
- ret = venus_clks_get(core);
- if (ret)
- return ret;
-
- ret = devm_pm_opp_set_clkname(core->dev, "core");
- if (ret)
- return ret;
-
- return 0;
-}
-
static void core_put_v1(struct venus_core *core)
{
}
@@ -350,7 +335,7 @@ static int core_power_v1(struct venus_core *core, int on)
}
static const struct venus_pm_ops pm_ops_v1 = {
- .core_get = core_get_v1,
+ .core_get = venus_clks_get,
.core_put = core_put_v1,
.core_power = core_power_v1,
.load_scale = load_scale_v1,
@@ -423,7 +408,7 @@ static int venc_power_v3(struct device *dev, int on)
}
static const struct venus_pm_ops pm_ops_v3 = {
- .core_get = core_get_v1,
+ .core_get = venus_clks_get,
.core_put = core_put_v1,
.core_power = core_power_v1,
.vdec_get = vdec_get_v3,
@@ -1013,10 +998,6 @@ static int core_get_v4(struct venus_core *core)
if (legacy_binding)
return 0;
- ret = devm_pm_opp_set_clkname(dev, "core");
- if (ret)
- return ret;
-
ret = vcodec_domains_get(core);
if (ret)
return ret;
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 05/20] media: venus: pm_helpers: Kill dead code
2024-02-09 21:09 [PATCH v2 00/20] Venus cleanups Konrad Dybcio
` (3 preceding siblings ...)
2024-02-09 21:09 ` [PATCH v2 04/20] media: venus: core: Set OPP clkname in a common code path Konrad Dybcio
@ 2024-02-09 21:09 ` Konrad Dybcio
2024-02-09 21:10 ` [PATCH v2 00/20] Venus cleanups Konrad Dybcio
5 siblings, 0 replies; 10+ messages in thread
From: Konrad Dybcio @ 2024-02-09 21:09 UTC (permalink / raw)
To: Stanimir Varbanov, Vikash Garodia, Bryan O'Donoghue,
Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab,
Dikshita Agarwal, Philipp Zabel
Cc: Marijn Suijten, Stanimir Varbanov, Mauro Carvalho Chehab,
linux-media, linux-arm-msm, linux-kernel, Konrad Dybcio
A situation like:
if (!foo)
goto bar;
for (i = 0; i < foo; i++)
...1...
bar:
...2...
is totally identical to:
for (i = 0; i < 0; i++) // === if (0)
...1...
...2...
Get rid of such boilerplate.
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/media/platform/qcom/venus/pm_helpers.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
index 1ba65345a5e2..7193075e8c04 100644
--- a/drivers/media/platform/qcom/venus/pm_helpers.c
+++ b/drivers/media/platform/qcom/venus/pm_helpers.c
@@ -878,14 +878,10 @@ static int vcodec_domains_get(struct venus_core *core)
.pd_flags = PD_FLAG_NO_DEV_LINK,
};
- if (!res->vcodec_pmdomains_num)
- goto skip_pmdomains;
-
ret = dev_pm_domain_attach_list(dev, &vcodec_data, &core->pmdomains);
if (ret < 0)
return ret;
-skip_pmdomains:
if (!core->res->opp_pmdomain)
return 0;
@@ -928,9 +924,6 @@ static int core_resets_reset(struct venus_core *core)
unsigned int i;
int ret;
- if (!res->resets_num)
- return 0;
-
for (i = 0; i < res->resets_num; i++) {
ret = reset_control_assert(core->resets[i]);
if (ret)
@@ -953,9 +946,6 @@ static int core_resets_get(struct venus_core *core)
unsigned int i;
int ret;
- if (!res->resets_num)
- return 0;
-
for (i = 0; i < res->resets_num; i++) {
core->resets[i] =
devm_reset_control_get_exclusive(dev, res->resets[i]);
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 00/20] Venus cleanups
2024-02-09 21:09 [PATCH v2 00/20] Venus cleanups Konrad Dybcio
` (4 preceding siblings ...)
2024-02-09 21:09 ` [PATCH v2 05/20] media: venus: pm_helpers: Kill dead code Konrad Dybcio
@ 2024-02-09 21:10 ` Konrad Dybcio
5 siblings, 0 replies; 10+ messages in thread
From: Konrad Dybcio @ 2024-02-09 21:10 UTC (permalink / raw)
To: Stanimir Varbanov, Vikash Garodia, Bryan O'Donoghue,
Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab,
Dikshita Agarwal, Philipp Zabel
Cc: Marijn Suijten, Stanimir Varbanov, Mauro Carvalho Chehab,
linux-media, linux-arm-msm, linux-kernel
On 9.02.2024 22:09, Konrad Dybcio wrote:
> With the driver supporting multiple generations of hardware, some mold
> has definitely grown over the code..
>
> This series attempts to amend this situation a bit by commonizing some
> code paths and fixing some bugs while at it.
>
> Only tested on SM8250.
>
> Definitely needs testing on:
>
> - SDM845 with old bindings
> - SDM845 with new bindings or 7180
> - MSM8916
> - MSM8996
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
Apologies for sending this twice. The other submission should be looked at.
Konrad
^ permalink raw reply [flat|nested] 10+ messages in thread