On Tue, Feb 25, 2025 at 07:10:00PM +0100, neil.armstrong@linaro.org wrote:

On 25/02/2025 19:06, Dmitry Baryshkov wrote:

quoted

On Tue, Feb 25, 2025 at 06:55:58PM +0100, neil.armstrong@linaro.org wrote:

quoted

On 25/02/2025 11:41, Dmitry Baryshkov wrote:

quoted

On Tue, 25 Feb 2025 at 12:04, Neil Armstrong [off-list ref] wrote:

quoted

On 25/02/2025 11:02, Dmitry Baryshkov wrote:

quoted

On Tue, Feb 25, 2025 at 10:05:10AM +0100, Neil Armstrong wrote:

quoted

In order to support the SM8650 iris33 hardware, we need to provide specific
reset and constoller power off sequences via the vpu_ops callbacks.

Add those callbacks, and use the current helpers for currently supported
platforms.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
    drivers/media/platform/qcom/iris/iris_vpu2.c       |  2 ++
    drivers/media/platform/qcom/iris/iris_vpu3.c       |  2 ++
    drivers/media/platform/qcom/iris/iris_vpu_common.c | 14 ++++++++++----
    drivers/media/platform/qcom/iris/iris_vpu_common.h |  4 ++++
    4 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_vpu2.c b/drivers/media/platform/qcom/iris/iris_vpu2.c
index 8f502aed43ce2fa6a272a2ce14ff1ca54d3e63a2..093e2068ec35e902f6c7bb3a487a679f9eada39a 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu2.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu2.c

@@ -33,6 +33,8 @@ static u64 iris_vpu2_calc_freq(struct iris_inst *inst, size_t data_size)
    }

    const struct vpu_ops iris_vpu2_ops = {
+    .reset_controller = iris_vpu_reset_controller,
       .power_off_hw = iris_vpu_power_off_hw,
+    .power_off_controller = iris_vpu_power_off_controller,
       .calc_freq = iris_vpu2_calc_freq,
    };

diff --git a/drivers/media/platform/qcom/iris/iris_vpu3.c b/drivers/media/platform/qcom/iris/iris_vpu3.c
index b484638e6105a69319232f667ee7ae95e3853698..95f362633c95b101ecfda6480c4c0b73416bd00c 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu3.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu3.c

@@ -117,6 +117,8 @@ static u64 iris_vpu3_calculate_frequency(struct iris_inst *inst, size_t data_siz
    }

    const struct vpu_ops iris_vpu3_ops = {
+    .reset_controller = iris_vpu_reset_controller,
       .power_off_hw = iris_vpu3_power_off_hardware,
+    .power_off_controller = iris_vpu_power_off_controller,
       .calc_freq = iris_vpu3_calculate_frequency,
    };

diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c
index fe9896d66848cdcd8c67bd45bbf3b6ce4a01ab10..ec8b10d836d0993bcd722a2bafbb577b85f41fc9 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c

@@ -211,7 +211,7 @@ int iris_vpu_prepare_pc(struct iris_core *core)
       return -EAGAIN;
    }

-static int iris_vpu_power_off_controller(struct iris_core *core)
+int iris_vpu_power_off_controller(struct iris_core *core)
    {
       u32 val = 0;
       int ret;

@@ -264,23 +264,29 @@ void iris_vpu_power_off(struct iris_core *core)
    {
       dev_pm_opp_set_rate(core->dev, 0);
       core->iris_platform_data->vpu_ops->power_off_hw(core);
-    iris_vpu_power_off_controller(core);
+    core->iris_platform_data->vpu_ops->power_off_controller(core);
       iris_unset_icc_bw(core);

       if (!iris_vpu_watchdog(core, core->intr_status))
               disable_irq_nosync(core->irq);
    }

-static int iris_vpu_power_on_controller(struct iris_core *core)
+int iris_vpu_reset_controller(struct iris_core *core)

If these functions are platform-specific, please rename them
accordingly, like iris_vpu2_3_foo() or just iris_vpu2_foo().

They are not, this is the whole point.

I think they are, you are adding them to the platform-specific ops. In
the end, they are not applicable to 3.3.

Vpu 3.3 is added on the next patch, with specific callbacks
for 3.3, this very patch has no functional change, it still uses
the same "common" reset and controller power off for vpu2 and vpu3.

This very patch is a preparation for vpu33, using common helpers
in vpu_ops is already done in the vpu2 support, I simply extend
the same logic here.

I'd really expect that iris_vpu_foo() functions apply to every platform.
These functions are now being used for VPU2 and VPU3 only. Thus I assume
that they are platform specific and should have platform-specific
prefix.

Thanks for your advice, but I followed the code and naming style of
the current merged driver, perhaps Dikshita will give some suggestions
on how the naming should be done in this case.

Agreed.


-- 
With best wishes
Dmitry