diff mbox series

[v2,20/20] media: venus: pm_helpers: Use reset_bulk API

Message ID 20230911-topic-mars-v2-20-3dac84b88c4b@linaro.org
State Superseded
Headers show
Series Venus cleanups | expand

Commit Message

Konrad Dybcio Feb. 9, 2024, 9:10 p.m. UTC
All of the resets are toggled together. Use the bulk api to save on some
code complexity.

The delay between resets is now correctly determined by the reset
framework.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/media/platform/qcom/venus/core.c       | 15 ++++++++++-----
 drivers/media/platform/qcom/venus/core.h       |  4 ++--
 drivers/media/platform/qcom/venus/pm_helpers.c | 15 +++------------
 3 files changed, 15 insertions(+), 19 deletions(-)

Comments

Philipp Zabel Feb. 21, 2024, 1:34 p.m. UTC | #1
On Mi, 2024-02-14 at 22:20 +0100, Konrad Dybcio wrote:
> On 14.02.2024 14:31, Philipp Zabel wrote:
> > Hi Konrad,
> > 
> > On Fr, 2024-02-09 at 22:10 +0100, Konrad Dybcio wrote:
> > > All of the resets are toggled together. Use the bulk api to save on some
> > > code complexity.
> > > 
> > > The delay between resets is now correctly determined by the reset
> > > framework.
> > 
> > If this is a recent change, could you reference the commit?
> 
> It's a series that recently landed in -next [1]

Missing link?

> [...]
> 
> > 
> > Since VIDC_RESETS_NUM_MAX is only 2, I don't think a separate
> > allocation is worth it.
> 
> It's 2 today, anyway. I wanted to keep it flexible

If this is expected to grow, fine.

> [...]
> 
> > > +	ret = reset_control_bulk_reset(res->resets_num, core->resets);
> > > +	if (ret)
> > > +		dev_err(core->dev, "Failed to toggle resets: %d\n", ret);
> > >  
> > > -err:
> > >  	return ret;
> > 
> > Could be simplified to:
> > 
> > 	return reset_control_bulk_reset(res->resets_num, core-
> > > resets);
> 
> I intentionally kept the if (ret) to print a specific error message
> in case the call fails, this driver doesn't go a good job of telling
> the user/developer what went wrong.

Oh, ok.

regards
Philipp
Konrad Dybcio Feb. 21, 2024, 1:37 p.m. UTC | #2
On 21.02.2024 14:34, Philipp Zabel wrote:
> On Mi, 2024-02-14 at 22:20 +0100, Konrad Dybcio wrote:
>> On 14.02.2024 14:31, Philipp Zabel wrote:
>>> Hi Konrad,
>>>
>>> On Fr, 2024-02-09 at 22:10 +0100, Konrad Dybcio wrote:
>>>> All of the resets are toggled together. Use the bulk api to save on some
>>>> code complexity.
>>>>
>>>> The delay between resets is now correctly determined by the reset
>>>> framework.
>>>
>>> If this is a recent change, could you reference the commit?
>>
>> It's a series that recently landed in -next [1]
> 
> Missing link?

Yes, sorry!

Konrad

[1] https://lore.kernel.org/linux-arm-msm/20240105-topic-venus_reset-v2-0-c37eba13b5ce@linaro.org/
Philipp Zabel Feb. 21, 2024, 1:45 p.m. UTC | #3
On Mi, 2024-02-21 at 14:37 +0100, Konrad Dybcio wrote:
> On 21.02.2024 14:34, Philipp Zabel wrote:
> > On Mi, 2024-02-14 at 22:20 +0100, Konrad Dybcio wrote:
> > > On 14.02.2024 14:31, Philipp Zabel wrote:
> > > > Hi Konrad,
> > > > 
> > > > On Fr, 2024-02-09 at 22:10 +0100, Konrad Dybcio wrote:
> > > > > All of the resets are toggled together. Use the bulk api to save on some
> > > > > code complexity.
> > > > > 
> > > > > The delay between resets is now correctly determined by the reset
> > > > > framework.
> > > > 
> > > > If this is a recent change, could you reference the commit?
> > > 
> > > It's a series that recently landed in -next [1]
> > 
> > Missing link?
> 
> Yes, sorry!
> 
> Konrad
> 
> [1] https://lore.kernel.org/linux-arm-msm/20240105-topic-venus_reset-v2-0-c37eba13b5ce@linaro.org/

Thank you. With that,

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp
diff mbox series

Patch

diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
index 873affe17537..ff5601a5ce77 100644
--- a/drivers/media/platform/qcom/venus/core.c
+++ b/drivers/media/platform/qcom/venus/core.c
@@ -328,11 +328,16 @@  static int venus_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	for (i = 0; i < res->resets_num; i++) {
-		core->resets[i] = devm_reset_control_get_exclusive(dev, res->resets[i]);
-		if (IS_ERR(core->resets[i]))
-			return PTR_ERR(core->resets[i]);
-	}
+	core->resets = devm_kcalloc(dev, res->resets_num, sizeof(*core->resets), GFP_KERNEL);
+	if (res->resets_num && !core->resets)
+		return -ENOMEM;
+
+	for (i = 0; i < res->resets_num; i++)
+		core->resets[i].id = res->resets[i];
+
+	ret = devm_reset_control_bulk_get_exclusive(dev, res->resets_num, core->resets);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get resets\n");
 
 	ret = venus_get_resources(core);
 	if (ret)
diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 6ecaa3e38cac..2376b9cbdf2c 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -130,7 +130,7 @@  struct venus_format {
  * @pmdomains:	a pointer to a list of pmdomains
  * @opp_dl_venus: an device-link for device OPP
  * @opp_pmdomain: an OPP power-domain
- * @resets: an array of reset signals
+ * @resets: a reset_control_bulk_data array of hardware reset signals
  * @vdev_dec:	a reference to video device structure for decoder instances
  * @vdev_enc:	a reference to video device structure for encoder instances
  * @v4l2_dev:	a holder for v4l2 device structure
@@ -183,7 +183,7 @@  struct venus_core {
 	struct dev_pm_domain_list *pmdomains;
 	struct device_link *opp_dl_venus;
 	struct device *opp_pmdomain;
-	struct reset_control *resets[VIDC_RESETS_NUM_MAX];
+	struct reset_control_bulk_data *resets;
 	struct video_device *vdev_dec;
 	struct video_device *vdev_enc;
 	struct v4l2_device v4l2_dev;
diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
index 9df8f2292c17..170fb131cb1e 100644
--- a/drivers/media/platform/qcom/venus/pm_helpers.c
+++ b/drivers/media/platform/qcom/venus/pm_helpers.c
@@ -865,21 +865,12 @@  void vcodec_domains_put(struct venus_core *core)
 static int core_resets_reset(struct venus_core *core)
 {
 	const struct venus_resources *res = core->res;
-	unsigned int i;
 	int ret;
 
-	for (i = 0; i < res->resets_num; i++) {
-		ret = reset_control_assert(core->resets[i]);
-		if (ret)
-			goto err;
-
-		usleep_range(150, 250);
-		ret = reset_control_deassert(core->resets[i]);
-		if (ret)
-			goto err;
-	}
+	ret = reset_control_bulk_reset(res->resets_num, core->resets);
+	if (ret)
+		dev_err(core->dev, "Failed to toggle resets: %d\n", ret);
 
-err:
 	return ret;
 }