Hi Maxime,
On Wed, Jul 23, 2025 at 09:00:28AM +0200, Maxime Ripard wrote:
> Hi,
>
> On Tue, Jul 22, 2025 at 04:41:12PM +0200, Otto Pflüger wrote:
> > Enable the DPU and DSI clocks specified in the device tree.
> > Disable the DSI clock when it is not needed.
> >
> > Signed-off-by: Otto Pflüger <otto.pflue...@abscue.de>
> > ---
> > drivers/gpu/drm/sprd/sprd_dpu.c | 7 +++++++
> > drivers/gpu/drm/sprd/sprd_dpu.h | 1 +
> > drivers/gpu/drm/sprd/sprd_dsi.c | 9 +++++++++
> > drivers/gpu/drm/sprd/sprd_dsi.h | 4 +++-
> > 4 files changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/sprd/sprd_dpu.c
> > b/drivers/gpu/drm/sprd/sprd_dpu.c
> > index
> > 0d9eb778794d92418b39f8535d94abde3566de43..9d274600e6a80bdfc435f6c6eff77c9dd71cb38c
> > 100644
> > --- a/drivers/gpu/drm/sprd/sprd_dpu.c
> > +++ b/drivers/gpu/drm/sprd/sprd_dpu.c
> > @@ -3,6 +3,7 @@
> > * Copyright (C) 2020 Unisoc Inc.
> > */
> >
> > +#include <linux/clk.h>
> > #include <linux/component.h>
> > #include <linux/delay.h>
> > #include <linux/dma-buf.h>
> > @@ -794,6 +795,12 @@ static int sprd_dpu_context_init(struct sprd_dpu *dpu,
> > if (ctx->irq < 0)
> > return ctx->irq;
> >
> > + ctx->clk = devm_clk_get_optional_enabled(dev, "core");
> > + if (IS_ERR(ctx->clk)) {
> > + dev_err(dev, "failed to get DPU core clock\n");
> > + return PTR_ERR(ctx->clk);
> > + }
> > +
> > /* disable and clear interrupts before register dpu IRQ. */
> > writel(0x00, ctx->base + REG_DPU_INT_EN);
> > writel(0xff, ctx->base + REG_DPU_INT_CLR);
> > diff --git a/drivers/gpu/drm/sprd/sprd_dpu.h
> > b/drivers/gpu/drm/sprd/sprd_dpu.h
> > index
> > 157a78f24dc18b071602552ea9d005af66525263..d48b922de580a8a4bf07c4610c431d3321f7b810
> > 100644
> > --- a/drivers/gpu/drm/sprd/sprd_dpu.h
> > +++ b/drivers/gpu/drm/sprd/sprd_dpu.h
> > @@ -44,6 +44,7 @@ enum {
> > */
> > struct dpu_context {
> > void __iomem *base;
> > + struct clk *clk;
> > int irq;
> > u8 if_type;
> > struct videomode vm;
> > diff --git a/drivers/gpu/drm/sprd/sprd_dsi.c
> > b/drivers/gpu/drm/sprd/sprd_dsi.c
> > index
> > e01d1d28fe579644ec2e0c83ec9170269932adfe..2af4273a6c73185084290c9d14b8ac18914d514b
> > 100644
> > --- a/drivers/gpu/drm/sprd/sprd_dsi.c
> > +++ b/drivers/gpu/drm/sprd/sprd_dsi.c
> > @@ -828,6 +828,8 @@ static void sprd_dsi_bridge_pre_enable(struct
> > drm_bridge *bridge)
> > struct sprd_dsi *dsi = bridge_to_dsi(bridge);
> > struct dsi_context *ctx = &dsi->ctx;
> >
> > + clk_prepare_enable(ctx->clk);
> > +
> > if (ctx->enabled) {
> > drm_warn(dsi->drm, "dsi is initialized\n");
> > return;
> > @@ -875,6 +877,8 @@ static void sprd_dsi_bridge_post_disable(struct
> > drm_bridge *bridge)
> > sprd_dphy_fini(ctx);
> > sprd_dsi_fini(ctx);
> >
> > + clk_disable_unprepare(ctx->clk);
> > +
> > ctx->enabled = false;
> > }
>
> I'm a bit confused. Why do you need to enable / disable that clock in
> pre_enable / post_disable, if you already enabled it at probe?
These are two different clocks. DPU uses devm_clk_get_optional_enabled,
while DSI uses devm_clk_get_optional and enables/disables it when
needed. Ideally both clocks should be disabled when not needed, but this
will be implemented later.
Best regards,
Otto Pflüger
