Re: [PATCH v12 11/14] drm/mediatek: dpi: Add tvd_clk enable/disable flow

[Rex-BC Chen]

On Tue, 2022-06-21 at 12:08 +0800, CK Hu wrote:
> Hi, Rex:
> 
> On Tue, 2022-06-21 at 11:50 +0800, Rex-BC Chen wrote:
> > On Tue, 2022-06-21 at 11:45 +0800, CK Hu wrote:
> > > On Tue, 2022-06-21 at 11:11 +0800, Rex-BC Chen wrote:
> > > > On Tue, 2022-06-21 at 10:55 +0800, CK Hu wrote:
> > > > > Hi, Bo-Chen:
> > > > > 
> > > > > On Mon, 2022-06-20 at 20:10 +0800, Bo-Chen Chen wrote:
> > > > > > We should enable/disable tvd_clk when power_on/power_off,
> > > > > > so
> > > > > > add
> > > > > > this
> > > > > > patch to do this.
> > > > > 
> > > > > Without this patch, what would happen?
> > > > > It seems this patch is redundant for these SoCs:
> > > > > 
> > > > > static const struct of_device_id mtk_dpi_of_ids[] = {
> > > > > 	{ .compatible = "mediatek,mt2701-dpi",
> > > > > 	  .data = &mt2701_conf,
> > > > > 	},
> > > > > 	{ .compatible = "mediatek,mt8173-dpi",
> > > > > 	  .data = &mt8173_conf,
> > > > > 	},
> > > > > 	{ .compatible = "mediatek,mt8183-dpi",
> > > > > 	  .data = &mt8183_conf,
> > > > > 	},
> > > > > 	{ .compatible = "mediatek,mt8192-dpi",
> > > > > 	  .data = &mt8192_conf,
> > > > > 	},
> > > > > 	{ },
> > > > > };
> > > > > 
> > > > > Regards,
> > > > > CK
> > > > > 
> > > > 
> > > > Hello CK,
> > > > 
> > > > IMO, this is a bug fix patch. From the usage of clock, if we
> > > > want
> > > > to
> > > > use it, we should enable it . Therefore, I think we should add
> > > > this
> > > > and
> > > > I will add a fix tag for this patch.
> > > 
> > > I think mt8173 chromebook use this driver for HDMI output. So
> > > mt8173
> > > chromebook HDMI could not work normally?
> > > 
> > > Regards,
> > > CK
> > > 
> > 
> > Hmm..
> > I am not sure about this. But without this patch, dpi is also
> > working
> > in mt8183/mt8192. It may be related to the ccf driver. But anyway,
> > I
> > think we should do this whether ccf driver helps us to enable this
> > clock.
> 
> OK. So, could you help to fix the bug in ccf? If HDMI is disabled but
> ccf still turn on this clock, the power would be wasted.
> 
> Regards,
> CK
> 

I am also testing if we don't have this patch and it also "works"
(dpintf is working fine).
do you think we need this patch or just drop this?

For the ccf driver, I am not familiar to ccf and also not a expert of
ccf.
It just a guest for this. I am not sure whether it's a "bug" or just a.
And I think it's not the purpose of this series. If there is any issue,
I think we will fix it in the future.

BRs,
Bo-Chen

> > 
> > BRs,
> > Bo-Chen
> > 
> > > > 
> > > > BRs,
> > > > Bo-Chen
> > > > > 
> > > > > > 
> > > > > > Signed-off-by: Bo-Chen Chen <rex-bc.chen@xxxxxxxxxxxx>
> > > > > > ---
> > > > > >  drivers/gpu/drm/mediatek/mtk_dpi.c | 11 ++++++++++-
> > > > > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > > > > b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > > > > index 2717b1741b7a..f83ecb154457 100644
> > > > > > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > > > > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > > > > @@ -455,6 +455,7 @@ static void mtk_dpi_power_off(struct
> > > > > > mtk_dpi
> > > > > > *dpi)
> > > > > >  	mtk_dpi_disable(dpi);
> > > > > >  	clk_disable_unprepare(dpi->pixel_clk);
> > > > > >  	clk_disable_unprepare(dpi->engine_clk);
> > > > > > +	clk_disable_unprepare(dpi->tvd_clk);
> > > > > >  }
> > > > > >  
> > > > > >  static int mtk_dpi_power_on(struct mtk_dpi *dpi)
> > > > > > @@ -464,10 +465,16 @@ static int mtk_dpi_power_on(struct
> > > > > > mtk_dpi
> > > > > > *dpi)
> > > > > >  	if (++dpi->refcount != 1)
> > > > > >  		return 0;
> > > > > >  
> > > > > > +	ret = clk_prepare_enable(dpi->tvd_clk);
> > > > > > +	if (ret) {
> > > > > > +		dev_err(dpi->dev, "Failed to enable tvd pll:
> > > > > > %d\n",
> > > > > > ret);
> > > > > > +		goto err_refcount;
> > > > > > +	}
> > > > > > +
> > > > > >  	ret = clk_prepare_enable(dpi->engine_clk);
> > > > > >  	if (ret) {
> > > > > >  		dev_err(dpi->dev, "Failed to enable engine
> > > > > > clock:
> > > > > > %d\n", ret);
> > > > > > -		goto err_refcount;
> > > > > > +		goto err_engine;
> > > > > >  	}
> > > > > >  
> > > > > >  	ret = clk_prepare_enable(dpi->pixel_clk);
> > > > > > @@ -484,6 +491,8 @@ static int mtk_dpi_power_on(struct
> > > > > > mtk_dpi
> > > > > > *dpi)
> > > > > >  
> > > > > >  err_pixel:
> > > > > >  	clk_disable_unprepare(dpi->engine_clk);
> > > > > > +err_engine:
> > > > > > +	clk_disable_unprepare(dpi->tvd_clk);
> > > > > >  err_refcount:
> > > > > >  	dpi->refcount--;
> > > > > >  	return ret;
> > > > > 
> > > > > 
> > > > 
> > > > 
> > > 
> > > 
> > 
> > 
> 
>