Re: [PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock
2017-10-20 10:12 GMT+02:00 Russell King - ARM Linux: > On Sat, Oct 14, 2017 at 12:53:35AM +0200, Pierre-Hugues Husson wrote: >> @@ -2382,6 +2383,18 @@ __dw_hdmi_probe(struct platform_device *pdev, >> goto err_isfr; >> } >> >> + hdmi->cec_clk = devm_clk_get(hdmi->dev, "cec"); >> + if (IS_ERR(hdmi->cec_clk)) { >> + hdmi->cec_clk = NULL; > > What if devm_clk_get() returns EPROBE_DEFER? Does that really mean the > clock does not exist? Should be fixed in v2. Thanks
Re: [PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock
On Sat, Oct 14, 2017 at 12:53:35AM +0200, Pierre-Hugues Husson wrote: > @@ -2382,6 +2383,18 @@ __dw_hdmi_probe(struct platform_device *pdev, > goto err_isfr; > } > > + hdmi->cec_clk = devm_clk_get(hdmi->dev, "cec"); > + if (IS_ERR(hdmi->cec_clk)) { > + hdmi->cec_clk = NULL; What if devm_clk_get() returns EPROBE_DEFER? Does that really mean the clock does not exist? -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up
Re: [PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock
On 14/10/17 10:16, Hans Verkuil wrote: > On 10/14/2017 12:53 AM, Pierre-Hugues Husson wrote: >> The documentation already mentions "cec" optional clock, but >> currently the driver doesn't enable it. >> >> Signed-off-by: Pierre-Hugues Husson> > Acked-by: Hans Verkuil Tested-by: Hans Verkuil > > Thanks! > > Hans > >> --- >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 + >> 1 file changed, 17 insertions(+) >> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> index bf14214fa464..5007cdf43131 100644 >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> @@ -138,6 +138,7 @@ struct dw_hdmi { >> struct device *dev; >> struct clk *isfr_clk; >> struct clk *iahb_clk; >> +struct clk *cec_clk; >> struct dw_hdmi_i2c *i2c; >> >> struct hdmi_data_info hdmi_data; >> @@ -2382,6 +2383,18 @@ __dw_hdmi_probe(struct platform_device *pdev, >> goto err_isfr; >> } >> >> +hdmi->cec_clk = devm_clk_get(hdmi->dev, "cec"); >> +if (IS_ERR(hdmi->cec_clk)) { >> +hdmi->cec_clk = NULL; >> +} else { >> +ret = clk_prepare_enable(hdmi->cec_clk); >> +if (ret) { >> +dev_err(hdmi->dev, "Cannot enable HDMI cec clock: %d\n", >> +ret); >> +goto err_res; >> +} >> +} >> + >> /* Product and revision IDs */ >> hdmi->version = (hdmi_readb(hdmi, HDMI_DESIGN_ID) << 8) >>| (hdmi_readb(hdmi, HDMI_REVISION_ID) << 0); >> @@ -2518,6 +2531,8 @@ __dw_hdmi_probe(struct platform_device *pdev, >> cec_notifier_put(hdmi->cec_notifier); >> >> clk_disable_unprepare(hdmi->iahb_clk); >> +if (hdmi->cec_clk) >> +clk_disable_unprepare(hdmi->cec_clk); >> err_isfr: >> clk_disable_unprepare(hdmi->isfr_clk); >> err_res: >> @@ -2541,6 +2556,8 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi) >> >> clk_disable_unprepare(hdmi->iahb_clk); >> clk_disable_unprepare(hdmi->isfr_clk); >> +if (hdmi->cec_clk) >> +clk_disable_unprepare(hdmi->cec_clk); >> >> if (hdmi->i2c) >> i2c_del_adapter(>i2c->adap); >> >
Re: [PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock
Hi Pierre, On Sat, Oct 14, 2017 at 10:21 AM, Pierre-Hugues Hussonwrote: > As far as I understand, dw_hdmi_cec_enable only gates > the CEC clock inside the Synopsis IP, > but the SOC still has to provide a specific CEC clock to it. > To enable such an external CEC clock, the binding documentation [1] > mentions a "cec" optional clock, and I'm not seeing any code in dw-hdmi > to enable it. Ok, understood. In the case of i.MX6 we control the CEC clock via HDMI_MC_CLKDIS_CECCLK_DISABLE bit inside the Synopsis IP. Looks like Rockchip needs the external CEC clock then. Regards, Fabio Estevam
Re: [PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock
Hi Fabio, 2017-10-14 2:18 GMT+02:00 Fabio Estevam: > Hi Pierre-Hugues, > > On Fri, Oct 13, 2017 at 7:53 PM, Pierre-Hugues Husson wrote: >> The documentation already mentions "cec" optional clock, but >> currently the driver doesn't enable it. > > The cec clock is enabled at dw_hdmi_cec_enable(). As far as I understand, dw_hdmi_cec_enable only gates the CEC clock inside the Synopsis IP, but the SOC still has to provide a specific CEC clock to it. To enable such an external CEC clock, the binding documentation [1] mentions a "cec" optional clock, and I'm not seeing any code in dw-hdmi to enable it. Regards, [1] On next-20170929 Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt line 28
Re: [PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock
On 10/14/2017 12:53 AM, Pierre-Hugues Husson wrote: > The documentation already mentions "cec" optional clock, but > currently the driver doesn't enable it. > > Signed-off-by: Pierre-Hugues HussonAcked-by: Hans Verkuil Thanks! Hans > --- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 + > 1 file changed, 17 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index bf14214fa464..5007cdf43131 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -138,6 +138,7 @@ struct dw_hdmi { > struct device *dev; > struct clk *isfr_clk; > struct clk *iahb_clk; > + struct clk *cec_clk; > struct dw_hdmi_i2c *i2c; > > struct hdmi_data_info hdmi_data; > @@ -2382,6 +2383,18 @@ __dw_hdmi_probe(struct platform_device *pdev, > goto err_isfr; > } > > + hdmi->cec_clk = devm_clk_get(hdmi->dev, "cec"); > + if (IS_ERR(hdmi->cec_clk)) { > + hdmi->cec_clk = NULL; > + } else { > + ret = clk_prepare_enable(hdmi->cec_clk); > + if (ret) { > + dev_err(hdmi->dev, "Cannot enable HDMI cec clock: %d\n", > + ret); > + goto err_res; > + } > + } > + > /* Product and revision IDs */ > hdmi->version = (hdmi_readb(hdmi, HDMI_DESIGN_ID) << 8) > | (hdmi_readb(hdmi, HDMI_REVISION_ID) << 0); > @@ -2518,6 +2531,8 @@ __dw_hdmi_probe(struct platform_device *pdev, > cec_notifier_put(hdmi->cec_notifier); > > clk_disable_unprepare(hdmi->iahb_clk); > + if (hdmi->cec_clk) > + clk_disable_unprepare(hdmi->cec_clk); > err_isfr: > clk_disable_unprepare(hdmi->isfr_clk); > err_res: > @@ -2541,6 +2556,8 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi) > > clk_disable_unprepare(hdmi->iahb_clk); > clk_disable_unprepare(hdmi->isfr_clk); > + if (hdmi->cec_clk) > + clk_disable_unprepare(hdmi->cec_clk); > > if (hdmi->i2c) > i2c_del_adapter(>i2c->adap); >
Re: [PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock
Hi Pierre-Hugues, On Fri, Oct 13, 2017 at 7:53 PM, Pierre-Hugues Hussonwrote: > The documentation already mentions "cec" optional clock, but > currently the driver doesn't enable it. The cec clock is enabled at dw_hdmi_cec_enable().