| Message ID | 20250305-topic-sm8x50-iris-v10-v2-2-bd65a3fc099e@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: qcom: iris: add support for SM8650 | expand |
On 05/03/2025 19:05, Neil Armstrong wrote: > In order to support vpu33, the iris_vpu_power_off_controller needs to be > reused and extended, but the AON_WRAPPER_MVP_NOC_LPI_CONTROL cannot be > set from the power_off_controller sequence like vpu2 and vpu3 so > split the power_off_controller into 3 steps: > - iris_vpu_power_off_controller_begin > - iris_vpu_power_off_controller_end > - iris_vpu_power_off_controller_disable > > And use them in a common iris_vpu_power_off_controller() for > vpu2 and vpu3 based platforms. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > drivers/media/platform/qcom/iris/iris_vpu_common.c | 46 ++++++++++++++++------ > 1 file changed, 33 insertions(+), 13 deletions(-) > > diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c > index fe9896d66848cdcd8c67bd45bbf3b6ce4a01ab10..d6ce92f3c7544e44dccca26bf6a4c95a720f9229 100644 > --- a/drivers/media/platform/qcom/iris/iris_vpu_common.c > +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c > @@ -211,33 +211,29 @@ int iris_vpu_prepare_pc(struct iris_core *core) > return -EAGAIN; > } > > -static int iris_vpu_power_off_controller(struct iris_core *core) > +static void iris_vpu_power_off_controller_begin(struct iris_core *core) > { > - u32 val = 0; > - int ret; > - > writel(MSK_SIGNAL_FROM_TENSILICA | MSK_CORE_POWER_ON, core->reg_base + CPU_CS_X2RPMH); > +} > > - writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL); > - > - ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS, > - val, val & BIT(0), 200, 2000); > - if (ret) > - goto disable_power; > +static int iris_vpu_power_off_controller_end(struct iris_core *core) > +{ > + u32 val = 0; > + int ret; > > writel(REQ_POWER_DOWN_PREP, core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_CONTROL); > > ret = readl_poll_timeout(core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_STATUS, > val, val & BIT(0), 200, 2000); > if (ret) > - goto disable_power; > + return ret; > > writel(0x0, core->reg_base + WRAPPER_DEBUG_BRIDGE_LPI_CONTROL); > > ret = readl_poll_timeout(core->reg_base + WRAPPER_DEBUG_BRIDGE_LPI_STATUS, > val, val == 0, 200, 2000); > if (ret) > - goto disable_power; > + return ret; > > writel(CTL_AXI_CLK_HALT | CTL_CLK_HALT, > core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG); > @@ -245,10 +241,34 @@ static int iris_vpu_power_off_controller(struct iris_core *core) > writel(0x0, core->reg_base + WRAPPER_TZ_QNS4PDXFIFO_RESET); > writel(0x0, core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG); > > -disable_power: > + return 0; > +} > + > +static void iris_vpu_power_off_controller_disable(struct iris_core *core) > +{ > iris_disable_unprepare_clock(core, IRIS_CTRL_CLK); > iris_disable_unprepare_clock(core, IRIS_AXI_CLK); > iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]); > +} > + > +static int iris_vpu_power_off_controller(struct iris_core *core) > +{ > + u32 val = 0; > + int ret; > + > + iris_vpu_power_off_controller_begin(core); > + > + writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL); > + > + ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS, > + val, val & BIT(0), 200, 2000); > + if (ret) > + goto disable_power; > + > + iris_vpu_power_off_controller_end(core); > + > +disable_power: > + iris_vpu_power_off_controller_disable(core); > > return 0; > } > > -- > 2.34.1 > > Have you checked that rb5/sm8250 still works after this change ? --- bod
On 3/6/2025 12:35 AM, Neil Armstrong wrote: > In order to support vpu33, the iris_vpu_power_off_controller needs to be > reused and extended, but the AON_WRAPPER_MVP_NOC_LPI_CONTROL cannot be > set from the power_off_controller sequence like vpu2 and vpu3 so > split the power_off_controller into 3 steps: > - iris_vpu_power_off_controller_begin > - iris_vpu_power_off_controller_end > - iris_vpu_power_off_controller_disable > NAK. I don't think splitting the API into these small functions is beneficial in this case, The extracted parts are too trivial to justify separate functions, and this actually makes the flow harder to follow rather than improving re-usability or clarity. If there is no clear or significant re-use case, I'd prefer keeping the logic consolidated in single API to maintain readability. Thanks, Dikshita > And use them in a common iris_vpu_power_off_controller() for > vpu2 and vpu3 based platforms. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > drivers/media/platform/qcom/iris/iris_vpu_common.c | 46 ++++++++++++++++------ > 1 file changed, 33 insertions(+), 13 deletions(-) > > diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c > index fe9896d66848cdcd8c67bd45bbf3b6ce4a01ab10..d6ce92f3c7544e44dccca26bf6a4c95a720f9229 100644 > --- a/drivers/media/platform/qcom/iris/iris_vpu_common.c > +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c > @@ -211,33 +211,29 @@ int iris_vpu_prepare_pc(struct iris_core *core) > return -EAGAIN; > } > > -static int iris_vpu_power_off_controller(struct iris_core *core) > +static void iris_vpu_power_off_controller_begin(struct iris_core *core) > { > - u32 val = 0; > - int ret; > - > writel(MSK_SIGNAL_FROM_TENSILICA | MSK_CORE_POWER_ON, core->reg_base + CPU_CS_X2RPMH); > +} > > - writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL); > - > - ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS, > - val, val & BIT(0), 200, 2000); > - if (ret) > - goto disable_power; > +static int iris_vpu_power_off_controller_end(struct iris_core *core) > +{ > + u32 val = 0; > + int ret; > > writel(REQ_POWER_DOWN_PREP, core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_CONTROL); > > ret = readl_poll_timeout(core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_STATUS, > val, val & BIT(0), 200, 2000); > if (ret) > - goto disable_power; > + return ret; > > writel(0x0, core->reg_base + WRAPPER_DEBUG_BRIDGE_LPI_CONTROL); > > ret = readl_poll_timeout(core->reg_base + WRAPPER_DEBUG_BRIDGE_LPI_STATUS, > val, val == 0, 200, 2000); > if (ret) > - goto disable_power; > + return ret; > > writel(CTL_AXI_CLK_HALT | CTL_CLK_HALT, > core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG); > @@ -245,10 +241,34 @@ static int iris_vpu_power_off_controller(struct iris_core *core) > writel(0x0, core->reg_base + WRAPPER_TZ_QNS4PDXFIFO_RESET); > writel(0x0, core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG); > > -disable_power: > + return 0; > +} > + > +static void iris_vpu_power_off_controller_disable(struct iris_core *core) > +{ > iris_disable_unprepare_clock(core, IRIS_CTRL_CLK); > iris_disable_unprepare_clock(core, IRIS_AXI_CLK); > iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]); > +} > + > +static int iris_vpu_power_off_controller(struct iris_core *core) > +{ > + u32 val = 0; > + int ret; > + > + iris_vpu_power_off_controller_begin(core); > + > + writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL); > + > + ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS, > + val, val & BIT(0), 200, 2000); > + if (ret) > + goto disable_power; > + > + iris_vpu_power_off_controller_end(core); > + > +disable_power: > + iris_vpu_power_off_controller_disable(core); > > return 0; > } >
On 06/03/2025 13:34, Bryan O'Donoghue wrote: > On 05/03/2025 19:05, Neil Armstrong wrote: >> In order to support vpu33, the iris_vpu_power_off_controller needs to be >> reused and extended, but the AON_WRAPPER_MVP_NOC_LPI_CONTROL cannot be >> set from the power_off_controller sequence like vpu2 and vpu3 so >> split the power_off_controller into 3 steps: >> - iris_vpu_power_off_controller_begin >> - iris_vpu_power_off_controller_end >> - iris_vpu_power_off_controller_disable >> >> And use them in a common iris_vpu_power_off_controller() for >> vpu2 and vpu3 based platforms. >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> drivers/media/platform/qcom/iris/iris_vpu_common.c | 46 ++++++++++++++++------ >> 1 file changed, 33 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c >> index fe9896d66848cdcd8c67bd45bbf3b6ce4a01ab10..d6ce92f3c7544e44dccca26bf6a4c95a720f9229 100644 >> --- a/drivers/media/platform/qcom/iris/iris_vpu_common.c >> +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c >> @@ -211,33 +211,29 @@ int iris_vpu_prepare_pc(struct iris_core *core) >> return -EAGAIN; >> } >> >> -static int iris_vpu_power_off_controller(struct iris_core *core) >> +static void iris_vpu_power_off_controller_begin(struct iris_core *core) >> { >> - u32 val = 0; >> - int ret; >> - >> writel(MSK_SIGNAL_FROM_TENSILICA | MSK_CORE_POWER_ON, core->reg_base + CPU_CS_X2RPMH); >> +} >> >> - writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL); >> - >> - ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS, >> - val, val & BIT(0), 200, 2000); >> - if (ret) >> - goto disable_power; >> +static int iris_vpu_power_off_controller_end(struct iris_core *core) >> +{ >> + u32 val = 0; >> + int ret; >> >> writel(REQ_POWER_DOWN_PREP, core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_CONTROL); >> >> ret = readl_poll_timeout(core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_STATUS, >> val, val & BIT(0), 200, 2000); >> if (ret) >> - goto disable_power; >> + return ret; >> >> writel(0x0, core->reg_base + WRAPPER_DEBUG_BRIDGE_LPI_CONTROL); >> >> ret = readl_poll_timeout(core->reg_base + WRAPPER_DEBUG_BRIDGE_LPI_STATUS, >> val, val == 0, 200, 2000); >> if (ret) >> - goto disable_power; >> + return ret; >> >> writel(CTL_AXI_CLK_HALT | CTL_CLK_HALT, >> core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG); >> @@ -245,10 +241,34 @@ static int iris_vpu_power_off_controller(struct iris_core *core) >> writel(0x0, core->reg_base + WRAPPER_TZ_QNS4PDXFIFO_RESET); >> writel(0x0, core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG); >> >> -disable_power: >> + return 0; >> +} >> + >> +static void iris_vpu_power_off_controller_disable(struct iris_core *core) >> +{ >> iris_disable_unprepare_clock(core, IRIS_CTRL_CLK); >> iris_disable_unprepare_clock(core, IRIS_AXI_CLK); >> iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]); >> +} >> + >> +static int iris_vpu_power_off_controller(struct iris_core *core) >> +{ >> + u32 val = 0; >> + int ret; >> + >> + iris_vpu_power_off_controller_begin(core); >> + >> + writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL); >> + >> + ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS, >> + val, val & BIT(0), 200, 2000); >> + if (ret) >> + goto disable_power; >> + >> + iris_vpu_power_off_controller_end(core); >> + >> +disable_power: >> + iris_vpu_power_off_controller_disable(core); >> >> return 0; >> } >> >> -- >> 2.34.1 >> >> > > Have you checked that rb5/sm8250 still works after this change ? I'm on it, but it doesn't add any functional changes. Neil > > --- > bod
On 06/03/2025 14:06, Dikshita Agarwal wrote: > > > On 3/6/2025 12:35 AM, Neil Armstrong wrote: >> In order to support vpu33, the iris_vpu_power_off_controller needs to be >> reused and extended, but the AON_WRAPPER_MVP_NOC_LPI_CONTROL cannot be >> set from the power_off_controller sequence like vpu2 and vpu3 so >> split the power_off_controller into 3 steps: >> - iris_vpu_power_off_controller_begin >> - iris_vpu_power_off_controller_end >> - iris_vpu_power_off_controller_disable >> > NAK. > I don't think splitting the API into these small functions is beneficial in > this case, The extracted parts are too trivial to justify separate > functions, and this actually makes the flow harder to follow rather than > improving re-usability or clarity. > If there is no clear or significant re-use case, I'd prefer keeping the > logic consolidated in single API to maintain readability. Right I agree, I tried to do my best and reuse code, and this is the result. So what would be the next step ? I can: - move iris_vpu_power_off_controller into vpu2, and add it into the vpu3 ops - re-implement it for vpu33 as v1 Neil > > Thanks, > Dikshita >> And use them in a common iris_vpu_power_off_controller() for >> vpu2 and vpu3 based platforms. >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> drivers/media/platform/qcom/iris/iris_vpu_common.c | 46 ++++++++++++++++------ >> 1 file changed, 33 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c >> index fe9896d66848cdcd8c67bd45bbf3b6ce4a01ab10..d6ce92f3c7544e44dccca26bf6a4c95a720f9229 100644 >> --- a/drivers/media/platform/qcom/iris/iris_vpu_common.c >> +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c >> @@ -211,33 +211,29 @@ int iris_vpu_prepare_pc(struct iris_core *core) >> return -EAGAIN; >> } >> >> -static int iris_vpu_power_off_controller(struct iris_core *core) >> +static void iris_vpu_power_off_controller_begin(struct iris_core *core) >> { >> - u32 val = 0; >> - int ret; >> - >> writel(MSK_SIGNAL_FROM_TENSILICA | MSK_CORE_POWER_ON, core->reg_base + CPU_CS_X2RPMH); >> +} >> >> - writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL); >> - >> - ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS, >> - val, val & BIT(0), 200, 2000); >> - if (ret) >> - goto disable_power; >> +static int iris_vpu_power_off_controller_end(struct iris_core *core) >> +{ >> + u32 val = 0; >> + int ret; >> >> writel(REQ_POWER_DOWN_PREP, core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_CONTROL); >> >> ret = readl_poll_timeout(core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_STATUS, >> val, val & BIT(0), 200, 2000); >> if (ret) >> - goto disable_power; >> + return ret; >> >> writel(0x0, core->reg_base + WRAPPER_DEBUG_BRIDGE_LPI_CONTROL); >> >> ret = readl_poll_timeout(core->reg_base + WRAPPER_DEBUG_BRIDGE_LPI_STATUS, >> val, val == 0, 200, 2000); >> if (ret) >> - goto disable_power; >> + return ret; >> >> writel(CTL_AXI_CLK_HALT | CTL_CLK_HALT, >> core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG); >> @@ -245,10 +241,34 @@ static int iris_vpu_power_off_controller(struct iris_core *core) >> writel(0x0, core->reg_base + WRAPPER_TZ_QNS4PDXFIFO_RESET); >> writel(0x0, core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG); >> >> -disable_power: >> + return 0; >> +} >> + >> +static void iris_vpu_power_off_controller_disable(struct iris_core *core) >> +{ >> iris_disable_unprepare_clock(core, IRIS_CTRL_CLK); >> iris_disable_unprepare_clock(core, IRIS_AXI_CLK); >> iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]); >> +} >> + >> +static int iris_vpu_power_off_controller(struct iris_core *core) >> +{ >> + u32 val = 0; >> + int ret; >> + >> + iris_vpu_power_off_controller_begin(core); >> + >> + writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL); >> + >> + ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS, >> + val, val & BIT(0), 200, 2000); >> + if (ret) >> + goto disable_power; >> + >> + iris_vpu_power_off_controller_end(core); >> + >> +disable_power: >> + iris_vpu_power_off_controller_disable(core); >> >> return 0; >> } >>
On 3/6/2025 6:39 PM, Neil Armstrong wrote: > On 06/03/2025 14:06, Dikshita Agarwal wrote: >> >> >> On 3/6/2025 12:35 AM, Neil Armstrong wrote: >>> In order to support vpu33, the iris_vpu_power_off_controller needs to be >>> reused and extended, but the AON_WRAPPER_MVP_NOC_LPI_CONTROL cannot be >>> set from the power_off_controller sequence like vpu2 and vpu3 so >>> split the power_off_controller into 3 steps: >>> - iris_vpu_power_off_controller_begin >>> - iris_vpu_power_off_controller_end >>> - iris_vpu_power_off_controller_disable >>> >> NAK. >> I don't think splitting the API into these small functions is beneficial in >> this case, The extracted parts are too trivial to justify separate >> functions, and this actually makes the flow harder to follow rather than >> improving re-usability or clarity. >> If there is no clear or significant re-use case, I'd prefer keeping the >> logic consolidated in single API to maintain readability. > > Right I agree, I tried to do my best and reuse code, and this is the result. > > So what would be the next step ? I can: > - move iris_vpu_power_off_controller into vpu2, and add it into the vpu3 ops you can keep it in common, as it will still be used for both vpu2 and vpu3 > - re-implement it for vpu33 as v1 right, would prefer going back to v1 for vpu33, if there is no significant re-use. Thanks, Dikshita > > Neil > >> >> Thanks, >> Dikshita >>> And use them in a common iris_vpu_power_off_controller() for >>> vpu2 and vpu3 based platforms. >>> >>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>> --- >>> drivers/media/platform/qcom/iris/iris_vpu_common.c | 46 >>> ++++++++++++++++------ >>> 1 file changed, 33 insertions(+), 13 deletions(-) >>> >>> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c >>> b/drivers/media/platform/qcom/iris/iris_vpu_common.c >>> index >>> fe9896d66848cdcd8c67bd45bbf3b6ce4a01ab10..d6ce92f3c7544e44dccca26bf6a4c95a720f9229 100644 >>> --- a/drivers/media/platform/qcom/iris/iris_vpu_common.c >>> +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c >>> @@ -211,33 +211,29 @@ int iris_vpu_prepare_pc(struct iris_core *core) >>> return -EAGAIN; >>> } >>> -static int iris_vpu_power_off_controller(struct iris_core *core) >>> +static void iris_vpu_power_off_controller_begin(struct iris_core *core) >>> { >>> - u32 val = 0; >>> - int ret; >>> - >>> writel(MSK_SIGNAL_FROM_TENSILICA | MSK_CORE_POWER_ON, >>> core->reg_base + CPU_CS_X2RPMH); >>> +} >>> - writel(REQ_POWER_DOWN_PREP, core->reg_base + >>> AON_WRAPPER_MVP_NOC_LPI_CONTROL); >>> - >>> - ret = readl_poll_timeout(core->reg_base + >>> AON_WRAPPER_MVP_NOC_LPI_STATUS, >>> - val, val & BIT(0), 200, 2000); >>> - if (ret) >>> - goto disable_power; >>> +static int iris_vpu_power_off_controller_end(struct iris_core *core) >>> +{ >>> + u32 val = 0; >>> + int ret; >>> writel(REQ_POWER_DOWN_PREP, core->reg_base + >>> WRAPPER_IRIS_CPU_NOC_LPI_CONTROL); >>> ret = readl_poll_timeout(core->reg_base + >>> WRAPPER_IRIS_CPU_NOC_LPI_STATUS, >>> val, val & BIT(0), 200, 2000); >>> if (ret) >>> - goto disable_power; >>> + return ret; >>> writel(0x0, core->reg_base + WRAPPER_DEBUG_BRIDGE_LPI_CONTROL); >>> ret = readl_poll_timeout(core->reg_base + >>> WRAPPER_DEBUG_BRIDGE_LPI_STATUS, >>> val, val == 0, 200, 2000); >>> if (ret) >>> - goto disable_power; >>> + return ret; >>> writel(CTL_AXI_CLK_HALT | CTL_CLK_HALT, >>> core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG); >>> @@ -245,10 +241,34 @@ static int iris_vpu_power_off_controller(struct >>> iris_core *core) >>> writel(0x0, core->reg_base + WRAPPER_TZ_QNS4PDXFIFO_RESET); >>> writel(0x0, core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG); >>> -disable_power: >>> + return 0; >>> +} >>> + >>> +static void iris_vpu_power_off_controller_disable(struct iris_core *core) >>> +{ >>> iris_disable_unprepare_clock(core, IRIS_CTRL_CLK); >>> iris_disable_unprepare_clock(core, IRIS_AXI_CLK); >>> iris_disable_power_domains(core, >>> core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]); >>> +} >>> + >>> +static int iris_vpu_power_off_controller(struct iris_core *core) >>> +{ >>> + u32 val = 0; >>> + int ret; >>> + >>> + iris_vpu_power_off_controller_begin(core); >>> + >>> + writel(REQ_POWER_DOWN_PREP, core->reg_base + >>> AON_WRAPPER_MVP_NOC_LPI_CONTROL); >>> + >>> + ret = readl_poll_timeout(core->reg_base + >>> AON_WRAPPER_MVP_NOC_LPI_STATUS, >>> + val, val & BIT(0), 200, 2000); >>> + if (ret) >>> + goto disable_power; >>> + >>> + iris_vpu_power_off_controller_end(core); >>> + >>> +disable_power: >>> + iris_vpu_power_off_controller_disable(core); >>> return 0; >>> } >>> >
diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c index fe9896d66848cdcd8c67bd45bbf3b6ce4a01ab10..d6ce92f3c7544e44dccca26bf6a4c95a720f9229 100644 --- a/drivers/media/platform/qcom/iris/iris_vpu_common.c +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c @@ -211,33 +211,29 @@ int iris_vpu_prepare_pc(struct iris_core *core) return -EAGAIN; } -static int iris_vpu_power_off_controller(struct iris_core *core) +static void iris_vpu_power_off_controller_begin(struct iris_core *core) { - u32 val = 0; - int ret; - writel(MSK_SIGNAL_FROM_TENSILICA | MSK_CORE_POWER_ON, core->reg_base + CPU_CS_X2RPMH); +} - writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL); - - ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS, - val, val & BIT(0), 200, 2000); - if (ret) - goto disable_power; +static int iris_vpu_power_off_controller_end(struct iris_core *core) +{ + u32 val = 0; + int ret; writel(REQ_POWER_DOWN_PREP, core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_CONTROL); ret = readl_poll_timeout(core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_STATUS, val, val & BIT(0), 200, 2000); if (ret) - goto disable_power; + return ret; writel(0x0, core->reg_base + WRAPPER_DEBUG_BRIDGE_LPI_CONTROL); ret = readl_poll_timeout(core->reg_base + WRAPPER_DEBUG_BRIDGE_LPI_STATUS, val, val == 0, 200, 2000); if (ret) - goto disable_power; + return ret; writel(CTL_AXI_CLK_HALT | CTL_CLK_HALT, core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG); @@ -245,10 +241,34 @@ static int iris_vpu_power_off_controller(struct iris_core *core) writel(0x0, core->reg_base + WRAPPER_TZ_QNS4PDXFIFO_RESET); writel(0x0, core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG); -disable_power: + return 0; +} + +static void iris_vpu_power_off_controller_disable(struct iris_core *core) +{ iris_disable_unprepare_clock(core, IRIS_CTRL_CLK); iris_disable_unprepare_clock(core, IRIS_AXI_CLK); iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]); +} + +static int iris_vpu_power_off_controller(struct iris_core *core) +{ + u32 val = 0; + int ret; + + iris_vpu_power_off_controller_begin(core); + + writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL); + + ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS, + val, val & BIT(0), 200, 2000); + if (ret) + goto disable_power; + + iris_vpu_power_off_controller_end(core); + +disable_power: + iris_vpu_power_off_controller_disable(core); return 0; }
In order to support vpu33, the iris_vpu_power_off_controller needs to be reused and extended, but the AON_WRAPPER_MVP_NOC_LPI_CONTROL cannot be set from the power_off_controller sequence like vpu2 and vpu3 so split the power_off_controller into 3 steps: - iris_vpu_power_off_controller_begin - iris_vpu_power_off_controller_end - iris_vpu_power_off_controller_disable And use them in a common iris_vpu_power_off_controller() for vpu2 and vpu3 based platforms. Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- drivers/media/platform/qcom/iris/iris_vpu_common.c | 46 ++++++++++++++++------ 1 file changed, 33 insertions(+), 13 deletions(-)