[v2,17/20] media: venus: pm_helpers: Commonize getting clocks and GenPDs
Commit Message
As has been the story with the past few commits, much of the resource
acquisition logic is totally identical between different generations
and there's no good reason to invent a new function for each one.
Commonize core_get() and rename it to venus_get_resources() to be more
meaningful.
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/media/platform/qcom/venus/core.c | 8 +++-----
drivers/media/platform/qcom/venus/pm_helpers.c | 5 +----
drivers/media/platform/qcom/venus/pm_helpers.h | 3 +--
3 files changed, 5 insertions(+), 11 deletions(-)
Comments
On 2/10/2024 2:40 AM, Konrad Dybcio wrote:
> As has been the story with the past few commits, much of the resource
> acquisition logic is totally identical between different generations
> and there's no good reason to invent a new function for each one.
>
> Commonize core_get() and rename it to venus_get_resources() to be more
> meaningful.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> drivers/media/platform/qcom/venus/core.c | 8 +++-----
> drivers/media/platform/qcom/venus/pm_helpers.c | 5 +----
> drivers/media/platform/qcom/venus/pm_helpers.h | 3 +--
> 3 files changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> index 680674dd0d68..873affe17537 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -334,11 +334,9 @@ static int venus_probe(struct platform_device *pdev)
> return PTR_ERR(core->resets[i]);
> }
>
> - if (core->pm_ops->core_get) {
> - ret = core->pm_ops->core_get(core);
> - if (ret)
> - return ret;
> - }
> + ret = venus_get_resources(core);
> + if (ret)
> + return ret;
>
> ret = dma_set_mask_and_coherent(dev, res->dma_mask);
> if (ret)
> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
> index a292c788ffba..1cbcffbc29af 100644
> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
> @@ -326,7 +326,6 @@ static int load_scale_v1(struct venus_inst *inst)
> }
>
> static const struct venus_pm_ops pm_ops_v1 = {
> - .core_get = venus_clks_get,
core_get is initialized with venus_clks_get in patch 4 and then being
removed here. It would be better to combine both patches.
> .load_scale = load_scale_v1,
> };
>
> @@ -395,7 +394,6 @@ static int venc_power_v3(struct device *dev, int on)
> }
>
> static const struct venus_pm_ops pm_ops_v3 = {
> - .core_get = venus_clks_get,
> .vdec_get = vdec_get_v3,
> .vdec_power = vdec_power_v3,
> .venc_get = venc_get_v3,
> @@ -920,7 +918,7 @@ static int core_resets_reset(struct venus_core *core)
> return ret;
> }
>
> -static int core_get_v4(struct venus_core *core)
> +int venus_get_resources(struct venus_core *core)
> {
> struct device *dev = core->dev;
> const struct venus_resources *res = core->res;
> @@ -1109,7 +1107,6 @@ static int load_scale_v4(struct venus_inst *inst)
> }
>
> static const struct venus_pm_ops pm_ops_v4 = {
> - .core_get = core_get_v4,
> .vdec_get = vdec_get_v4,
> .vdec_put = vdec_put_v4,
> .vdec_power = vdec_power_v4,
> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.h b/drivers/media/platform/qcom/venus/pm_helpers.h
> index 3014b39aa6e3..7a55a55029f3 100644
> --- a/drivers/media/platform/qcom/venus/pm_helpers.h
> +++ b/drivers/media/platform/qcom/venus/pm_helpers.h
> @@ -10,8 +10,6 @@ struct venus_core;
> #define POWER_OFF 0
>
> struct venus_pm_ops {
> - int (*core_get)(struct venus_core *core);
> -
> int (*vdec_get)(struct device *dev);
> void (*vdec_put)(struct device *dev);
> int (*vdec_power)(struct device *dev, int on);
> @@ -28,6 +26,7 @@ struct venus_pm_ops {
> const struct venus_pm_ops *venus_pm_get(enum hfi_version version);
> int venus_core_power(struct venus_core *core, int on);
> void vcodec_domains_put(struct venus_core *core);
> +int venus_get_resources(struct venus_core *core);
>
> static inline int venus_pm_load_scale(struct venus_inst *inst)
> {
>
On 4.03.2024 8:13 AM, Dikshita Agarwal wrote:
>
>
> On 2/10/2024 2:40 AM, Konrad Dybcio wrote:
>> As has been the story with the past few commits, much of the resource
>> acquisition logic is totally identical between different generations
>> and there's no good reason to invent a new function for each one.
>>
>> Commonize core_get() and rename it to venus_get_resources() to be more
>> meaningful.
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>> drivers/media/platform/qcom/venus/core.c | 8 +++-----
>> drivers/media/platform/qcom/venus/pm_helpers.c | 5 +----
>> drivers/media/platform/qcom/venus/pm_helpers.h | 3 +--
>> 3 files changed, 5 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
>> index 680674dd0d68..873affe17537 100644
>> --- a/drivers/media/platform/qcom/venus/core.c
>> +++ b/drivers/media/platform/qcom/venus/core.c
>> @@ -334,11 +334,9 @@ static int venus_probe(struct platform_device *pdev)
>> return PTR_ERR(core->resets[i]);
>> }
>>
>> - if (core->pm_ops->core_get) {
>> - ret = core->pm_ops->core_get(core);
>> - if (ret)
>> - return ret;
>> - }
>> + ret = venus_get_resources(core);
>> + if (ret)
>> + return ret;
>>
>> ret = dma_set_mask_and_coherent(dev, res->dma_mask);
>> if (ret)
>> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
>> index a292c788ffba..1cbcffbc29af 100644
>> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
>> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
>> @@ -326,7 +326,6 @@ static int load_scale_v1(struct venus_inst *inst)
>> }
>>
>> static const struct venus_pm_ops pm_ops_v1 = {
>> - .core_get = venus_clks_get,
> core_get is initialized with venus_clks_get in patch 4 and then being
> removed here. It would be better to combine both patches.
Generally I'd agree, but this series is rather big and both patch 4 and
this one are logically separated well enough, so I'd rather not waste time
moving things around and fighting merge conflicts for no functional change
Konrad
@@ -334,11 +334,9 @@ static int venus_probe(struct platform_device *pdev)
return PTR_ERR(core->resets[i]);
}
- if (core->pm_ops->core_get) {
- ret = core->pm_ops->core_get(core);
- if (ret)
- return ret;
- }
+ ret = venus_get_resources(core);
+ if (ret)
+ return ret;
ret = dma_set_mask_and_coherent(dev, res->dma_mask);
if (ret)
@@ -326,7 +326,6 @@ static int load_scale_v1(struct venus_inst *inst)
}
static const struct venus_pm_ops pm_ops_v1 = {
- .core_get = venus_clks_get,
.load_scale = load_scale_v1,
};
@@ -395,7 +394,6 @@ static int venc_power_v3(struct device *dev, int on)
}
static const struct venus_pm_ops pm_ops_v3 = {
- .core_get = venus_clks_get,
.vdec_get = vdec_get_v3,
.vdec_power = vdec_power_v3,
.venc_get = venc_get_v3,
@@ -920,7 +918,7 @@ static int core_resets_reset(struct venus_core *core)
return ret;
}
-static int core_get_v4(struct venus_core *core)
+int venus_get_resources(struct venus_core *core)
{
struct device *dev = core->dev;
const struct venus_resources *res = core->res;
@@ -1109,7 +1107,6 @@ static int load_scale_v4(struct venus_inst *inst)
}
static const struct venus_pm_ops pm_ops_v4 = {
- .core_get = core_get_v4,
.vdec_get = vdec_get_v4,
.vdec_put = vdec_put_v4,
.vdec_power = vdec_power_v4,
@@ -10,8 +10,6 @@ struct venus_core;
#define POWER_OFF 0
struct venus_pm_ops {
- int (*core_get)(struct venus_core *core);
-
int (*vdec_get)(struct device *dev);
void (*vdec_put)(struct device *dev);
int (*vdec_power)(struct device *dev, int on);
@@ -28,6 +26,7 @@ struct venus_pm_ops {
const struct venus_pm_ops *venus_pm_get(enum hfi_version version);
int venus_core_power(struct venus_core *core, int on);
void vcodec_domains_put(struct venus_core *core);
+int venus_get_resources(struct venus_core *core);
static inline int venus_pm_load_scale(struct venus_inst *inst)
{