* [PATCH v2 00/20] Venus cleanups
@ 2024-02-09 21:09 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
` (5 more replies)
0 siblings, 6 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
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>
---
Changes in v2:
- Fix "set but unused" warning in "Drop cache properties in resource struct"
- Fix modular build with "Commonize vdec_get()"
- Rebase
- Test again on 8250, since nobody else tested other platforms since the last
submission (or at least hasn't reported that), I'm assuming nobody cares
- Needs to be tested atop [1] and similar, it's in latest -next already
- Link to v1: https://lore.kernel.org/r/20230911-topic-mars-v1-0-a7d38bf87bdb@linaro.org
[1] https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?h=for-next&id=d2cd22c9c384aa50c0b4530e842bd078427e6279
---
Konrad Dybcio (20):
media: venus: pm_helpers: Only set rate of the core clock in core_clks_enable
media: venus: pm_helpers: Rename core_clks_get to venus_clks_get
media: venus: pm_helpers: Add kerneldoc to venus_clks_get()
media: venus: core: Set OPP clkname in a common code path
media: venus: pm_helpers: Kill dead code
media: venus: pm_helpers: Move reset acquisition to common code
media: venus: core: Constify all members of the resource struct
media: venus: core: Deduplicate OPP genpd names
media: venus: core: Get rid of vcodec_num
media: venus: core: Drop cache properties in resource struct
media: venus: core: Use GENMASK for dma_mask
media: venus: core: Remove cp_start
media: venus: pm_helpers: Commonize core_power
media: venus: pm_helpers: Remove pm_ops->core_put
media: venus: core: Define a pointer to core->res
media: venus: pm_helpers: Simplify vcodec clock handling
media: venus: pm_helpers: Commonize getting clocks and GenPDs
media: venus: pm_helpers: Commonize vdec_get()
media: venus: pm_helpers: Commonize venc_get()
media: venus: pm_helpers: Use reset_bulk API
drivers/media/platform/qcom/venus/core.c | 139 ++++-------
drivers/media/platform/qcom/venus/core.h | 66 +++--
drivers/media/platform/qcom/venus/firmware.c | 3 +-
drivers/media/platform/qcom/venus/hfi_venus.c | 10 +-
drivers/media/platform/qcom/venus/pm_helpers.c | 323 +++++++++----------------
drivers/media/platform/qcom/venus/pm_helpers.h | 10 +-
drivers/media/platform/qcom/venus/vdec.c | 9 +-
drivers/media/platform/qcom/venus/venc.c | 9 +-
8 files changed, 213 insertions(+), 356 deletions(-)
---
base-commit: 445a555e0623387fa9b94e68e61681717e70200a
change-id: 20230911-topic-mars-e60bb2269411
Best regards,
--
Konrad Dybcio <konrad.dybcio@linaro.org>
^ permalink raw reply [flat|nested] 10+ messages in thread* [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
* [PATCH v2 00/20] Venus cleanups
@ 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
0 siblings, 1 reply; 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
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>
---
Changes in v2:
- Fix "set but unused" warning in "Drop cache properties in resource struct"
- Fix modular build with "Commonize vdec_get()"
- Rebase
- Test again on 8250, since nobody else tested other platforms since the last
submission (or at least hasn't reported that), I'm assuming nobody cares
- Needs to be tested atop [1] and similar, it's in latest -next already
- Link to v1: https://lore.kernel.org/r/20230911-topic-mars-v1-0-a7d38bf87bdb@linaro.org
[1] https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?h=for-next&id=d2cd22c9c384aa50c0b4530e842bd078427e6279
---
Konrad Dybcio (20):
media: venus: pm_helpers: Only set rate of the core clock in core_clks_enable
media: venus: pm_helpers: Rename core_clks_get to venus_clks_get
media: venus: pm_helpers: Add kerneldoc to venus_clks_get()
media: venus: core: Set OPP clkname in a common code path
media: venus: pm_helpers: Kill dead code
media: venus: pm_helpers: Move reset acquisition to common code
media: venus: core: Constify all members of the resource struct
media: venus: core: Deduplicate OPP genpd names
media: venus: core: Get rid of vcodec_num
media: venus: core: Drop cache properties in resource struct
media: venus: core: Use GENMASK for dma_mask
media: venus: core: Remove cp_start
media: venus: pm_helpers: Commonize core_power
media: venus: pm_helpers: Remove pm_ops->core_put
media: venus: core: Define a pointer to core->res
media: venus: pm_helpers: Simplify vcodec clock handling
media: venus: pm_helpers: Commonize getting clocks and GenPDs
media: venus: pm_helpers: Commonize vdec_get()
media: venus: pm_helpers: Commonize venc_get()
media: venus: pm_helpers: Use reset_bulk API
drivers/media/platform/qcom/venus/core.c | 139 ++++-------
drivers/media/platform/qcom/venus/core.h | 66 +++--
drivers/media/platform/qcom/venus/firmware.c | 3 +-
drivers/media/platform/qcom/venus/hfi_venus.c | 10 +-
drivers/media/platform/qcom/venus/pm_helpers.c | 323 +++++++++----------------
drivers/media/platform/qcom/venus/pm_helpers.h | 10 +-
drivers/media/platform/qcom/venus/vdec.c | 9 +-
drivers/media/platform/qcom/venus/venc.c | 9 +-
8 files changed, 213 insertions(+), 356 deletions(-)
---
base-commit: 445a555e0623387fa9b94e68e61681717e70200a
change-id: 20230911-topic-mars-e60bb2269411
Best regards,
--
Konrad Dybcio <konrad.dybcio@linaro.org>
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v2 05/20] media: venus: pm_helpers: Kill dead code 2024-02-09 21:09 Konrad Dybcio @ 2024-02-09 21:09 ` Konrad Dybcio 2024-03-04 5:40 ` Dikshita Agarwal 0 siblings, 1 reply; 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 05/20] media: venus: pm_helpers: Kill dead code 2024-02-09 21:09 ` [PATCH v2 05/20] media: venus: pm_helpers: Kill dead code Konrad Dybcio @ 2024-03-04 5:40 ` Dikshita Agarwal 2024-03-26 21:30 ` Konrad Dybcio 0 siblings, 1 reply; 10+ messages in thread From: Dikshita Agarwal @ 2024-03-04 5:40 UTC (permalink / raw) To: Konrad Dybcio, Stanimir Varbanov, Vikash Garodia, Bryan O'Donoghue, Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab, Philipp Zabel Cc: Marijn Suijten, Stanimir Varbanov, Mauro Carvalho Chehab, linux-media, linux-arm-msm, linux-kernel On 2/10/2024 2:39 AM, Konrad Dybcio wrote: > 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; > - Removing the if check and relying only on for loop is good. but I don't see the for loop here. > ret = dev_pm_domain_attach_list(dev, &vcodec_data, &core->pmdomains); > if (ret < 0) > return ret; > Also, what's the base of this change? I don't see above API in the code anywhere. > -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]); > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 05/20] media: venus: pm_helpers: Kill dead code 2024-03-04 5:40 ` Dikshita Agarwal @ 2024-03-26 21:30 ` Konrad Dybcio 0 siblings, 0 replies; 10+ messages in thread From: Konrad Dybcio @ 2024-03-26 21:30 UTC (permalink / raw) To: Dikshita Agarwal, Stanimir Varbanov, Vikash Garodia, Bryan O'Donoghue, Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab, Philipp Zabel Cc: Marijn Suijten, Stanimir Varbanov, Mauro Carvalho Chehab, linux-media, linux-arm-msm, linux-kernel On 4.03.2024 6:40 AM, Dikshita Agarwal wrote: > > > On 2/10/2024 2:39 AM, Konrad Dybcio wrote: >> 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; >> - > Removing the if check and relying only on for loop is good. > but I don't see the for loop here. >> ret = dev_pm_domain_attach_list(dev, &vcodec_data, &core->pmdomains); >> if (ret < 0) >> return ret; >> > Also, what's the base of this change? I don't see above API in the code > anywhere. It's inside the dev_pm_domain_attach_list helper.. It was there explicitly when I first submitted the patch. Konrad ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-03-26 21:30 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 ` [PATCH v2 03/20] media: venus: pm_helpers: Add kerneldoc to venus_clks_get() Konrad Dybcio 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 ` [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 -- strict thread matches above, loose matches on Subject: below -- 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-03-04 5:40 ` Dikshita Agarwal 2024-03-26 21:30 ` Konrad Dybcio
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).