lkml.org 
[lkml]   [2018]   [May]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2 11/29] venus: venc,vdec: adds clocks needed for venus 4xx
    On Mon, May 28, 2018 at 5:47 PM Stanimir Varbanov
    <stanimir.varbanov@linaro.org> wrote:
    >
    > Hi Tomasz,
    >
    > On 05/24/2018 09:11 AM, Tomasz Figa wrote:
    > > Hi Stanimir,
    > >
    > > On Tue, May 15, 2018 at 5:10 PM Stanimir Varbanov <
    > > stanimir.varbanov@linaro.org> wrote:
    > >
    > >> This extends the clocks number to support suspend and resume
    > >> on Venus version 4xx.
    > >
    > >> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
    > >> ---
    > >> drivers/media/platform/qcom/venus/core.h | 4 +--
    > >> drivers/media/platform/qcom/venus/vdec.c | 42
    > > ++++++++++++++++++++++++++------
    > >> drivers/media/platform/qcom/venus/venc.c | 42
    > > ++++++++++++++++++++++++++------
    > >> 3 files changed, 72 insertions(+), 16 deletions(-)
    > >
    > >> diff --git a/drivers/media/platform/qcom/venus/core.h
    > > b/drivers/media/platform/qcom/venus/core.h
    > >> index 8d3e150800c9..b5b9a84e9155 100644
    > >> --- a/drivers/media/platform/qcom/venus/core.h
    > >> +++ b/drivers/media/platform/qcom/venus/core.h
    > >> @@ -92,8 +92,8 @@ struct venus_core {
    > >> void __iomem *base;
    > >> int irq;
    > >> struct clk *clks[VIDC_CLKS_NUM_MAX];
    > >> - struct clk *core0_clk;
    > >> - struct clk *core1_clk;
    > >> + struct clk *core0_clk, *core0_bus_clk;
    > >> + struct clk *core1_clk, *core1_bus_clk;
    > >> struct video_device *vdev_dec;
    > >> struct video_device *vdev_enc;
    > >> struct v4l2_device v4l2_dev;
    > >> diff --git a/drivers/media/platform/qcom/venus/vdec.c
    > > b/drivers/media/platform/qcom/venus/vdec.c
    > >> index 261a51adeef2..c45452634e7e 100644
    > >> --- a/drivers/media/platform/qcom/venus/vdec.c
    > >> +++ b/drivers/media/platform/qcom/venus/vdec.c
    > >> @@ -1081,12 +1081,18 @@ static int vdec_probe(struct platform_device
    > > *pdev)
    > >> if (!core)
    > >> return -EPROBE_DEFER;
    > >
    > >> - if (core->res->hfi_version == HFI_VERSION_3XX) {
    > >> + if (IS_V3(core) || IS_V4(core)) {
    > >> core->core0_clk = devm_clk_get(dev, "core");
    > >> if (IS_ERR(core->core0_clk))
    > >> return PTR_ERR(core->core0_clk);
    > >> }
    > >
    > >> + if (IS_V4(core)) {
    > >> + core->core0_bus_clk = devm_clk_get(dev, "bus");
    > >> + if (IS_ERR(core->core0_bus_clk))
    > >> + return PTR_ERR(core->core0_bus_clk);
    > >> + }
    > >> +
    > >
    > > Rather than doing this conditional dance, wouldn't it make more sense to
    > > just list all the clocks in variant data struct and use clk_bulk_get()?
    >
    > Do you mean the same as it is done for venus/core.c ?

    I mean clk_bulk_get() as in drivers/clk/clk-bulk.c. I guess
    venus/core.c would also benefit from switching to these helpers.

    >
    > >
    > >> platform_set_drvdata(pdev, core);
    > >
    > >> vdev = video_device_alloc();
    > >> @@ -1132,12 +1138,23 @@ static __maybe_unused int
    > > vdec_runtime_suspend(struct device *dev)
    > >> {
    > >> struct venus_core *core = dev_get_drvdata(dev);
    > >
    > >> - if (core->res->hfi_version == HFI_VERSION_1XX)
    > >> + if (IS_V1(core))
    > >> return 0;
    > >
    > >> - writel(0, core->base + WRAPPER_VDEC_VCODEC_POWER_CONTROL);
    > >> + if (IS_V3(core))
    > >> + writel(0, core->base + WRAPPER_VDEC_VCODEC_POWER_CONTROL);
    > >> + else if (IS_V4(core))
    > >> + writel(0, core->base +
    > > WRAPPER_VCODEC0_MMCC_POWER_CONTROL);
    > >> +
    > >> + if (IS_V4(core))
    > >> + clk_disable_unprepare(core->core0_bus_clk);
    > >> +
    > >> clk_disable_unprepare(core->core0_clk);
    > >> - writel(1, core->base + WRAPPER_VDEC_VCODEC_POWER_CONTROL);
    > >> +
    > >> + if (IS_V3(core))
    > >> + writel(1, core->base + WRAPPER_VDEC_VCODEC_POWER_CONTROL);
    > >> + else if (IS_V4(core))
    > >> + writel(1, core->base +
    > > WRAPPER_VCODEC0_MMCC_POWER_CONTROL);
    > >
    > > Almost every step here differs between version. I'd suggest splitting this
    > > into separate functions for both versions.
    >
    > I think it will be better to squash this patch with 13/29.

    I see. Let me review patch 13 first then.

    Best regards,
    Tomasz

    \
     
     \ /
      Last update: 2018-05-31 08:51    [W:2.488 / U:0.396 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site