| Message ID | 20230911-topic-mars-v2-17-3dac84b88c4b@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Venus cleanups | expand |
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
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, .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) {
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(-)