| Message ID | 20200422040410.6251-2-samuel@sholland.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] media: cedrus: Program output format during each run | expand |
Dne sreda, 22. april 2020 ob 06:04:10 CEST je Samuel Holland napisal(a): > This allows the VE clocks and PLL_VE to be disabled most of the time. > A runtime PM reference is held while streaming. > > Originally-by: Jernej Škrabec <jernej.skrabec@gmail.com> I guess this could be SOB? Either way I'm fine. > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > > v2: moved PM reference to cedrus_{start,stop}_streaming, based on an > earlier patch by Jernej Skrabec. Removes the need for autosuspend. > I tested this with running 2x v4l2-request-test in parallel. > I tested previous and this patch with LibreELEC and I didn't noticed any regressions: Tested-by: Jernej Skrabec <jernej.skrabec@siol.net> Best regards, Jernej
On Wed, 22 Apr 2020 at 01:00, Samuel Holland <samuel@sholland.org> wrote: > > This allows the VE clocks and PLL_VE to be disabled most of the time. > A runtime PM reference is held while streaming. > > Originally-by: Jernej Škrabec <jernej.skrabec@gmail.com> Originally-by is not documented, perhaps just go with Signed-off-by, as Jernej suggested. > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > > v2: moved PM reference to cedrus_{start,stop}_streaming, based on an > earlier patch by Jernej Skrabec. Removes the need for autosuspend. > I tested this with running 2x v4l2-request-test in parallel. > > --- > drivers/staging/media/sunxi/cedrus/cedrus.c | 7 ++ > .../staging/media/sunxi/cedrus/cedrus_hw.c | 106 ++++++++++++------ > .../staging/media/sunxi/cedrus/cedrus_hw.h | 3 + > .../staging/media/sunxi/cedrus/cedrus_video.c | 33 ++++-- > 4 files changed, 104 insertions(+), 45 deletions(-) > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c > index 05a85517ff60..bc27f9430eeb 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus.c > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c > @@ -16,6 +16,7 @@ > #include <linux/platform_device.h> > #include <linux/module.h> > #include <linux/of.h> > +#include <linux/pm.h> > > #include <media/v4l2-device.h> > #include <media/v4l2-ioctl.h> > @@ -551,12 +552,18 @@ static const struct of_device_id cedrus_dt_match[] = { > }; > MODULE_DEVICE_TABLE(of, cedrus_dt_match); > > +static const struct dev_pm_ops cedrus_dev_pm_ops = { > + SET_RUNTIME_PM_OPS(cedrus_hw_suspend, > + cedrus_hw_resume, NULL) > +}; > + > static struct platform_driver cedrus_driver = { > .probe = cedrus_probe, > .remove = cedrus_remove, > .driver = { > .name = CEDRUS_NAME, > .of_match_table = of_match_ptr(cedrus_dt_match), > + .pm = &cedrus_dev_pm_ops, > }, > }; > module_platform_driver(cedrus_driver); > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > index daf5f244f93b..1744e6fcc999 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > @@ -19,6 +19,7 @@ > #include <linux/dma-mapping.h> > #include <linux/interrupt.h> > #include <linux/clk.h> > +#include <linux/pm_runtime.h> > #include <linux/regmap.h> > #include <linux/reset.h> > #include <linux/soc/sunxi/sunxi_sram.h> > @@ -140,6 +141,64 @@ static irqreturn_t cedrus_irq(int irq, void *data) > return IRQ_HANDLED; > } > > +int cedrus_hw_suspend(struct device *device) > +{ > + struct cedrus_dev *dev = dev_get_drvdata(device); > + > + reset_control_assert(dev->rstc); > + > + clk_disable_unprepare(dev->ram_clk); > + clk_disable_unprepare(dev->mod_clk); > + clk_disable_unprepare(dev->ahb_clk); > + You can use clk_bulk API here. > + return 0; > +} > + > +int cedrus_hw_resume(struct device *device) > +{ > + struct cedrus_dev *dev = dev_get_drvdata(device); > + int ret; > + > + ret = clk_prepare_enable(dev->ahb_clk); > + if (ret) { > + dev_err(dev->dev, "Failed to enable AHB clock\n"); > + > + return ret; > + } > + > + ret = clk_prepare_enable(dev->mod_clk); > + if (ret) { > + dev_err(dev->dev, "Failed to enable MOD clock\n"); > + > + goto err_ahb_clk; > + } > + > + ret = clk_prepare_enable(dev->ram_clk); > + if (ret) { > + dev_err(dev->dev, "Failed to enable RAM clock\n"); > + > + goto err_mod_clk; > + } > + > + ret = reset_control_reset(dev->rstc); > + if (ret) { > + dev_err(dev->dev, "Failed to apply reset\n"); > + > + goto err_ram_clk; > + } > + > + return 0; > + > +err_ram_clk: > + clk_disable_unprepare(dev->ram_clk); > +err_mod_clk: > + clk_disable_unprepare(dev->mod_clk); > +err_ahb_clk: > + clk_disable_unprepare(dev->ahb_clk); > + > + return ret; > +} > + > int cedrus_hw_probe(struct cedrus_dev *dev) > { > const struct cedrus_variant *variant; > @@ -236,42 +295,17 @@ int cedrus_hw_probe(struct cedrus_dev *dev) > goto err_sram; > } > > - ret = clk_prepare_enable(dev->ahb_clk); > - if (ret) { > - dev_err(dev->dev, "Failed to enable AHB clock\n"); > - > - goto err_sram; > - } > - > - ret = clk_prepare_enable(dev->mod_clk); > - if (ret) { > - dev_err(dev->dev, "Failed to enable MOD clock\n"); > - > - goto err_ahb_clk; > - } > - > - ret = clk_prepare_enable(dev->ram_clk); > - if (ret) { > - dev_err(dev->dev, "Failed to enable RAM clock\n"); > - > - goto err_mod_clk; > - } > - > - ret = reset_control_reset(dev->rstc); > - if (ret) { > - dev_err(dev->dev, "Failed to apply reset\n"); > - > - goto err_ram_clk; > + pm_runtime_enable(dev->dev); > + if (!pm_runtime_enabled(dev->dev)) { > + ret = cedrus_hw_resume(dev->dev); > + if (ret) > + goto err_pm; > } > > return 0; > > -err_ram_clk: > - clk_disable_unprepare(dev->ram_clk); > -err_mod_clk: > - clk_disable_unprepare(dev->mod_clk); > -err_ahb_clk: > - clk_disable_unprepare(dev->ahb_clk); > +err_pm: > + pm_runtime_disable(dev->dev); > err_sram: > sunxi_sram_release(dev->dev); > err_mem: > @@ -282,11 +316,9 @@ int cedrus_hw_probe(struct cedrus_dev *dev) > > void cedrus_hw_remove(struct cedrus_dev *dev) > { > - reset_control_assert(dev->rstc); > - > - clk_disable_unprepare(dev->ram_clk); > - clk_disable_unprepare(dev->mod_clk); > - clk_disable_unprepare(dev->ahb_clk); > + pm_runtime_disable(dev->dev); > + if (!pm_runtime_status_suspended(dev->dev)) > + cedrus_hw_suspend(dev->dev); > > sunxi_sram_release(dev->dev); > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h > index 604ff932fbf5..45f641f0bfa2 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h > @@ -22,6 +22,9 @@ void cedrus_engine_disable(struct cedrus_dev *dev); > void cedrus_dst_format_set(struct cedrus_dev *dev, > struct v4l2_pix_format *fmt); > > +int cedrus_hw_suspend(struct device *device); > +int cedrus_hw_resume(struct device *device); > + > int cedrus_hw_probe(struct cedrus_dev *dev); > void cedrus_hw_remove(struct cedrus_dev *dev); > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c > index ed3f511f066f..16d82309e7b6 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c > @@ -13,6 +13,8 @@ > * Marek Szyprowski, <m.szyprowski@samsung.com> > */ > > +#include <linux/pm_runtime.h> > + > #include <media/videobuf2-dma-contig.h> > #include <media/v4l2-device.h> > #include <media/v4l2-ioctl.h> > @@ -450,12 +452,24 @@ static int cedrus_start_streaming(struct vb2_queue *vq, unsigned int count) > return -EINVAL; > } > > - if (V4L2_TYPE_IS_OUTPUT(vq->type) && > - dev->dec_ops[ctx->current_codec]->start) > - ret = dev->dec_ops[ctx->current_codec]->start(ctx); > + if (V4L2_TYPE_IS_OUTPUT(vq->type)) { > + ret = pm_runtime_get_sync(dev->dev); It's entirely up to you, but you could do get_sync/put between .device_run, as everything should happen in the context of v4l2_m2m_ops.device_run. (In that case, I believe you'd want to enable autosuspend.) Not sure there's much to gain from an power consumption pov, though. With the Originally-by changed: Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com> Thanks, Ezequiel
On 5/5/20 7:53 AM, Ezequiel Garcia wrote: > On Wed, 22 Apr 2020 at 01:00, Samuel Holland <samuel@sholland.org> wrote: >> >> This allows the VE clocks and PLL_VE to be disabled most of the time. >> A runtime PM reference is held while streaming. >> >> Originally-by: Jernej Škrabec <jernej.skrabec@gmail.com> > > Originally-by is not documented, perhaps just go with Signed-off-by, > as Jernej suggested. > >> Signed-off-by: Samuel Holland <samuel@sholland.org> >> --- >> >> v2: moved PM reference to cedrus_{start,stop}_streaming, based on an >> earlier patch by Jernej Skrabec. Removes the need for autosuspend. >> I tested this with running 2x v4l2-request-test in parallel. >> >> --- >> drivers/staging/media/sunxi/cedrus/cedrus.c | 7 ++ >> .../staging/media/sunxi/cedrus/cedrus_hw.c | 106 ++++++++++++------ >> .../staging/media/sunxi/cedrus/cedrus_hw.h | 3 + >> .../staging/media/sunxi/cedrus/cedrus_video.c | 33 ++++-- >> 4 files changed, 104 insertions(+), 45 deletions(-) >> >> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c >> index 05a85517ff60..bc27f9430eeb 100644 >> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c >> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c >> @@ -16,6 +16,7 @@ >> #include <linux/platform_device.h> >> #include <linux/module.h> >> #include <linux/of.h> >> +#include <linux/pm.h> >> >> #include <media/v4l2-device.h> >> #include <media/v4l2-ioctl.h> >> @@ -551,12 +552,18 @@ static const struct of_device_id cedrus_dt_match[] = { >> }; >> MODULE_DEVICE_TABLE(of, cedrus_dt_match); >> >> +static const struct dev_pm_ops cedrus_dev_pm_ops = { >> + SET_RUNTIME_PM_OPS(cedrus_hw_suspend, >> + cedrus_hw_resume, NULL) >> +}; >> + >> static struct platform_driver cedrus_driver = { >> .probe = cedrus_probe, >> .remove = cedrus_remove, >> .driver = { >> .name = CEDRUS_NAME, >> .of_match_table = of_match_ptr(cedrus_dt_match), >> + .pm = &cedrus_dev_pm_ops, >> }, >> }; >> module_platform_driver(cedrus_driver); >> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c >> index daf5f244f93b..1744e6fcc999 100644 >> --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c >> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c >> @@ -19,6 +19,7 @@ >> #include <linux/dma-mapping.h> >> #include <linux/interrupt.h> >> #include <linux/clk.h> >> +#include <linux/pm_runtime.h> >> #include <linux/regmap.h> >> #include <linux/reset.h> >> #include <linux/soc/sunxi/sunxi_sram.h> >> @@ -140,6 +141,64 @@ static irqreturn_t cedrus_irq(int irq, void *data) >> return IRQ_HANDLED; >> } >> >> +int cedrus_hw_suspend(struct device *device) >> +{ >> + struct cedrus_dev *dev = dev_get_drvdata(device); >> + >> + reset_control_assert(dev->rstc); >> + >> + clk_disable_unprepare(dev->ram_clk); >> + clk_disable_unprepare(dev->mod_clk); >> + clk_disable_unprepare(dev->ahb_clk); >> + > > You can use clk_bulk API here. Since this change is already tested, I'd prefer to do that as a separate patch. >> + return 0; >> +} >> + >> +int cedrus_hw_resume(struct device *device) >> +{ >> + struct cedrus_dev *dev = dev_get_drvdata(device); >> + int ret; >> + >> + ret = clk_prepare_enable(dev->ahb_clk); >> + if (ret) { >> + dev_err(dev->dev, "Failed to enable AHB clock\n"); >> + >> + return ret; >> + } >> + >> + ret = clk_prepare_enable(dev->mod_clk); >> + if (ret) { >> + dev_err(dev->dev, "Failed to enable MOD clock\n"); >> + >> + goto err_ahb_clk; >> + } >> + >> + ret = clk_prepare_enable(dev->ram_clk); >> + if (ret) { >> + dev_err(dev->dev, "Failed to enable RAM clock\n"); >> + >> + goto err_mod_clk; >> + } >> + >> + ret = reset_control_reset(dev->rstc); >> + if (ret) { >> + dev_err(dev->dev, "Failed to apply reset\n"); >> + >> + goto err_ram_clk; >> + } >> + >> + return 0; >> + >> +err_ram_clk: >> + clk_disable_unprepare(dev->ram_clk); >> +err_mod_clk: >> + clk_disable_unprepare(dev->mod_clk); >> +err_ahb_clk: >> + clk_disable_unprepare(dev->ahb_clk); >> + >> + return ret; >> +} >> + >> int cedrus_hw_probe(struct cedrus_dev *dev) >> { >> const struct cedrus_variant *variant; >> @@ -236,42 +295,17 @@ int cedrus_hw_probe(struct cedrus_dev *dev) >> goto err_sram; >> } >> >> - ret = clk_prepare_enable(dev->ahb_clk); >> - if (ret) { >> - dev_err(dev->dev, "Failed to enable AHB clock\n"); >> - >> - goto err_sram; >> - } >> - >> - ret = clk_prepare_enable(dev->mod_clk); >> - if (ret) { >> - dev_err(dev->dev, "Failed to enable MOD clock\n"); >> - >> - goto err_ahb_clk; >> - } >> - >> - ret = clk_prepare_enable(dev->ram_clk); >> - if (ret) { >> - dev_err(dev->dev, "Failed to enable RAM clock\n"); >> - >> - goto err_mod_clk; >> - } >> - >> - ret = reset_control_reset(dev->rstc); >> - if (ret) { >> - dev_err(dev->dev, "Failed to apply reset\n"); >> - >> - goto err_ram_clk; >> + pm_runtime_enable(dev->dev); >> + if (!pm_runtime_enabled(dev->dev)) { >> + ret = cedrus_hw_resume(dev->dev); >> + if (ret) >> + goto err_pm; >> } >> >> return 0; >> >> -err_ram_clk: >> - clk_disable_unprepare(dev->ram_clk); >> -err_mod_clk: >> - clk_disable_unprepare(dev->mod_clk); >> -err_ahb_clk: >> - clk_disable_unprepare(dev->ahb_clk); >> +err_pm: >> + pm_runtime_disable(dev->dev); >> err_sram: >> sunxi_sram_release(dev->dev); >> err_mem: >> @@ -282,11 +316,9 @@ int cedrus_hw_probe(struct cedrus_dev *dev) >> >> void cedrus_hw_remove(struct cedrus_dev *dev) >> { >> - reset_control_assert(dev->rstc); >> - >> - clk_disable_unprepare(dev->ram_clk); >> - clk_disable_unprepare(dev->mod_clk); >> - clk_disable_unprepare(dev->ahb_clk); >> + pm_runtime_disable(dev->dev); >> + if (!pm_runtime_status_suspended(dev->dev)) >> + cedrus_hw_suspend(dev->dev); >> >> sunxi_sram_release(dev->dev); >> >> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h >> index 604ff932fbf5..45f641f0bfa2 100644 >> --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h >> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h >> @@ -22,6 +22,9 @@ void cedrus_engine_disable(struct cedrus_dev *dev); >> void cedrus_dst_format_set(struct cedrus_dev *dev, >> struct v4l2_pix_format *fmt); >> >> +int cedrus_hw_suspend(struct device *device); >> +int cedrus_hw_resume(struct device *device); >> + >> int cedrus_hw_probe(struct cedrus_dev *dev); >> void cedrus_hw_remove(struct cedrus_dev *dev); >> >> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c >> index ed3f511f066f..16d82309e7b6 100644 >> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c >> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c >> @@ -13,6 +13,8 @@ >> * Marek Szyprowski, <m.szyprowski@samsung.com> >> */ >> >> +#include <linux/pm_runtime.h> >> + >> #include <media/videobuf2-dma-contig.h> >> #include <media/v4l2-device.h> >> #include <media/v4l2-ioctl.h> >> @@ -450,12 +452,24 @@ static int cedrus_start_streaming(struct vb2_queue *vq, unsigned int count) >> return -EINVAL; >> } >> >> - if (V4L2_TYPE_IS_OUTPUT(vq->type) && >> - dev->dec_ops[ctx->current_codec]->start) >> - ret = dev->dec_ops[ctx->current_codec]->start(ctx); >> + if (V4L2_TYPE_IS_OUTPUT(vq->type)) { >> + ret = pm_runtime_get_sync(dev->dev); > > It's entirely up to you, but you could do get_sync/put > between .device_run, as everything should happen > in the context of v4l2_m2m_ops.device_run. > (In that case, I believe you'd want to enable autosuspend.) > > Not sure there's much to gain from an power consumption > pov, though. Agreed. Either you have a short autosuspend delay, and get per-frame latency; or a longer delay, and the hardware is on for the entire stream anyway. So I don't think it's worth the overhead. > With the Originally-by changed: > > Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com> > > Thanks, > Ezequiel > Thanks, Samuel
On Sat, May 09, 2020 at 03:03:55PM -0500, Samuel Holland wrote: > On 5/5/20 7:53 AM, Ezequiel Garcia wrote: > > On Wed, 22 Apr 2020 at 01:00, Samuel Holland <samuel@sholland.org> wrote: > >> > >> This allows the VE clocks and PLL_VE to be disabled most of the time. > >> A runtime PM reference is held while streaming. > >> > >> Originally-by: Jernej Škrabec <jernej.skrabec@gmail.com> > > > > Originally-by is not documented, perhaps just go with Signed-off-by, > > as Jernej suggested. > > > >> Signed-off-by: Samuel Holland <samuel@sholland.org> > >> --- > >> > >> v2: moved PM reference to cedrus_{start,stop}_streaming, based on an > >> earlier patch by Jernej Skrabec. Removes the need for autosuspend. > >> I tested this with running 2x v4l2-request-test in parallel. > >> > >> --- > >> drivers/staging/media/sunxi/cedrus/cedrus.c | 7 ++ > >> .../staging/media/sunxi/cedrus/cedrus_hw.c | 106 ++++++++++++------ > >> .../staging/media/sunxi/cedrus/cedrus_hw.h | 3 + > >> .../staging/media/sunxi/cedrus/cedrus_video.c | 33 ++++-- > >> 4 files changed, 104 insertions(+), 45 deletions(-) > >> > >> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c > >> index 05a85517ff60..bc27f9430eeb 100644 > >> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c > >> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c > >> @@ -16,6 +16,7 @@ > >> #include <linux/platform_device.h> > >> #include <linux/module.h> > >> #include <linux/of.h> > >> +#include <linux/pm.h> > >> > >> #include <media/v4l2-device.h> > >> #include <media/v4l2-ioctl.h> > >> @@ -551,12 +552,18 @@ static const struct of_device_id cedrus_dt_match[] = { > >> }; > >> MODULE_DEVICE_TABLE(of, cedrus_dt_match); > >> > >> +static const struct dev_pm_ops cedrus_dev_pm_ops = { > >> + SET_RUNTIME_PM_OPS(cedrus_hw_suspend, > >> + cedrus_hw_resume, NULL) > >> +}; > >> + > >> static struct platform_driver cedrus_driver = { > >> .probe = cedrus_probe, > >> .remove = cedrus_remove, > >> .driver = { > >> .name = CEDRUS_NAME, > >> .of_match_table = of_match_ptr(cedrus_dt_match), > >> + .pm = &cedrus_dev_pm_ops, > >> }, > >> }; > >> module_platform_driver(cedrus_driver); > >> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > >> index daf5f244f93b..1744e6fcc999 100644 > >> --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > >> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > >> @@ -19,6 +19,7 @@ > >> #include <linux/dma-mapping.h> > >> #include <linux/interrupt.h> > >> #include <linux/clk.h> > >> +#include <linux/pm_runtime.h> > >> #include <linux/regmap.h> > >> #include <linux/reset.h> > >> #include <linux/soc/sunxi/sunxi_sram.h> > >> @@ -140,6 +141,64 @@ static irqreturn_t cedrus_irq(int irq, void *data) > >> return IRQ_HANDLED; > >> } > >> > >> +int cedrus_hw_suspend(struct device *device) > >> +{ > >> + struct cedrus_dev *dev = dev_get_drvdata(device); > >> + > >> + reset_control_assert(dev->rstc); > >> + > >> + clk_disable_unprepare(dev->ram_clk); > >> + clk_disable_unprepare(dev->mod_clk); > >> + clk_disable_unprepare(dev->ahb_clk); > >> + > > > > You can use clk_bulk API here. > > Since this change is already tested, I'd prefer to do that as a separate > patch. Given that those three clocks are also pretty different from a semantic point of view, I'm not sure it's wise to switch to the bulk API anyway. Maxime
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c index 05a85517ff60..bc27f9430eeb 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c @@ -16,6 +16,7 @@ #include <linux/platform_device.h> #include <linux/module.h> #include <linux/of.h> +#include <linux/pm.h> #include <media/v4l2-device.h> #include <media/v4l2-ioctl.h> @@ -551,12 +552,18 @@ static const struct of_device_id cedrus_dt_match[] = { }; MODULE_DEVICE_TABLE(of, cedrus_dt_match); +static const struct dev_pm_ops cedrus_dev_pm_ops = { + SET_RUNTIME_PM_OPS(cedrus_hw_suspend, + cedrus_hw_resume, NULL) +}; + static struct platform_driver cedrus_driver = { .probe = cedrus_probe, .remove = cedrus_remove, .driver = { .name = CEDRUS_NAME, .of_match_table = of_match_ptr(cedrus_dt_match), + .pm = &cedrus_dev_pm_ops, }, }; module_platform_driver(cedrus_driver); diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c index daf5f244f93b..1744e6fcc999 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c @@ -19,6 +19,7 @@ #include <linux/dma-mapping.h> #include <linux/interrupt.h> #include <linux/clk.h> +#include <linux/pm_runtime.h> #include <linux/regmap.h> #include <linux/reset.h> #include <linux/soc/sunxi/sunxi_sram.h> @@ -140,6 +141,64 @@ static irqreturn_t cedrus_irq(int irq, void *data) return IRQ_HANDLED; } +int cedrus_hw_suspend(struct device *device) +{ + struct cedrus_dev *dev = dev_get_drvdata(device); + + reset_control_assert(dev->rstc); + + clk_disable_unprepare(dev->ram_clk); + clk_disable_unprepare(dev->mod_clk); + clk_disable_unprepare(dev->ahb_clk); + + return 0; +} + +int cedrus_hw_resume(struct device *device) +{ + struct cedrus_dev *dev = dev_get_drvdata(device); + int ret; + + ret = clk_prepare_enable(dev->ahb_clk); + if (ret) { + dev_err(dev->dev, "Failed to enable AHB clock\n"); + + return ret; + } + + ret = clk_prepare_enable(dev->mod_clk); + if (ret) { + dev_err(dev->dev, "Failed to enable MOD clock\n"); + + goto err_ahb_clk; + } + + ret = clk_prepare_enable(dev->ram_clk); + if (ret) { + dev_err(dev->dev, "Failed to enable RAM clock\n"); + + goto err_mod_clk; + } + + ret = reset_control_reset(dev->rstc); + if (ret) { + dev_err(dev->dev, "Failed to apply reset\n"); + + goto err_ram_clk; + } + + return 0; + +err_ram_clk: + clk_disable_unprepare(dev->ram_clk); +err_mod_clk: + clk_disable_unprepare(dev->mod_clk); +err_ahb_clk: + clk_disable_unprepare(dev->ahb_clk); + + return ret; +} + int cedrus_hw_probe(struct cedrus_dev *dev) { const struct cedrus_variant *variant; @@ -236,42 +295,17 @@ int cedrus_hw_probe(struct cedrus_dev *dev) goto err_sram; } - ret = clk_prepare_enable(dev->ahb_clk); - if (ret) { - dev_err(dev->dev, "Failed to enable AHB clock\n"); - - goto err_sram; - } - - ret = clk_prepare_enable(dev->mod_clk); - if (ret) { - dev_err(dev->dev, "Failed to enable MOD clock\n"); - - goto err_ahb_clk; - } - - ret = clk_prepare_enable(dev->ram_clk); - if (ret) { - dev_err(dev->dev, "Failed to enable RAM clock\n"); - - goto err_mod_clk; - } - - ret = reset_control_reset(dev->rstc); - if (ret) { - dev_err(dev->dev, "Failed to apply reset\n"); - - goto err_ram_clk; + pm_runtime_enable(dev->dev); + if (!pm_runtime_enabled(dev->dev)) { + ret = cedrus_hw_resume(dev->dev); + if (ret) + goto err_pm; } return 0; -err_ram_clk: - clk_disable_unprepare(dev->ram_clk); -err_mod_clk: - clk_disable_unprepare(dev->mod_clk); -err_ahb_clk: - clk_disable_unprepare(dev->ahb_clk); +err_pm: + pm_runtime_disable(dev->dev); err_sram: sunxi_sram_release(dev->dev); err_mem: @@ -282,11 +316,9 @@ int cedrus_hw_probe(struct cedrus_dev *dev) void cedrus_hw_remove(struct cedrus_dev *dev) { - reset_control_assert(dev->rstc); - - clk_disable_unprepare(dev->ram_clk); - clk_disable_unprepare(dev->mod_clk); - clk_disable_unprepare(dev->ahb_clk); + pm_runtime_disable(dev->dev); + if (!pm_runtime_status_suspended(dev->dev)) + cedrus_hw_suspend(dev->dev); sunxi_sram_release(dev->dev); diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h index 604ff932fbf5..45f641f0bfa2 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h @@ -22,6 +22,9 @@ void cedrus_engine_disable(struct cedrus_dev *dev); void cedrus_dst_format_set(struct cedrus_dev *dev, struct v4l2_pix_format *fmt); +int cedrus_hw_suspend(struct device *device); +int cedrus_hw_resume(struct device *device); + int cedrus_hw_probe(struct cedrus_dev *dev); void cedrus_hw_remove(struct cedrus_dev *dev); diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c index ed3f511f066f..16d82309e7b6 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c @@ -13,6 +13,8 @@ * Marek Szyprowski, <m.szyprowski@samsung.com> */ +#include <linux/pm_runtime.h> + #include <media/videobuf2-dma-contig.h> #include <media/v4l2-device.h> #include <media/v4l2-ioctl.h> @@ -450,12 +452,24 @@ static int cedrus_start_streaming(struct vb2_queue *vq, unsigned int count) return -EINVAL; } - if (V4L2_TYPE_IS_OUTPUT(vq->type) && - dev->dec_ops[ctx->current_codec]->start) - ret = dev->dec_ops[ctx->current_codec]->start(ctx); + if (V4L2_TYPE_IS_OUTPUT(vq->type)) { + ret = pm_runtime_get_sync(dev->dev); + if (ret < 0) + goto err_cleanup; - if (ret) - cedrus_queue_cleanup(vq, VB2_BUF_STATE_QUEUED); + if (dev->dec_ops[ctx->current_codec]->start) { + ret = dev->dec_ops[ctx->current_codec]->start(ctx); + if (ret) + goto err_pm; + } + } + + return 0; + +err_pm: + pm_runtime_put(dev->dev); +err_cleanup: + cedrus_queue_cleanup(vq, VB2_BUF_STATE_QUEUED); return ret; } @@ -465,9 +479,12 @@ static void cedrus_stop_streaming(struct vb2_queue *vq) struct cedrus_ctx *ctx = vb2_get_drv_priv(vq); struct cedrus_dev *dev = ctx->dev; - if (V4L2_TYPE_IS_OUTPUT(vq->type) && - dev->dec_ops[ctx->current_codec]->stop) - dev->dec_ops[ctx->current_codec]->stop(ctx); + if (V4L2_TYPE_IS_OUTPUT(vq->type)) { + if (dev->dec_ops[ctx->current_codec]->stop) + dev->dec_ops[ctx->current_codec]->stop(ctx); + + pm_runtime_put(dev->dev); + } cedrus_queue_cleanup(vq, VB2_BUF_STATE_ERROR); }
This allows the VE clocks and PLL_VE to be disabled most of the time. A runtime PM reference is held while streaming. Originally-by: Jernej Škrabec <jernej.skrabec@gmail.com> Signed-off-by: Samuel Holland <samuel@sholland.org> --- v2: moved PM reference to cedrus_{start,stop}_streaming, based on an earlier patch by Jernej Skrabec. Removes the need for autosuspend. I tested this with running 2x v4l2-request-test in parallel. --- drivers/staging/media/sunxi/cedrus/cedrus.c | 7 ++ .../staging/media/sunxi/cedrus/cedrus_hw.c | 106 ++++++++++++------ .../staging/media/sunxi/cedrus/cedrus_hw.h | 3 + .../staging/media/sunxi/cedrus/cedrus_video.c | 33 ++++-- 4 files changed, 104 insertions(+), 45 deletions(-)