| Message ID | 20171013225337.5196-2-phh@phh.me (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Pierre-Hugues, On Fri, Oct 13, 2017 at 7:53 PM, Pierre-Hugues Husson <phh@phh.me> 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().
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 <phh@phh.me> Acked-by: Hans Verkuil <hans.verkuil@cisco.com> 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(&hdmi->i2c->adap); >
Hi Fabio, 2017-10-14 2:18 GMT+02:00 Fabio Estevam <festevam@gmail.com>: > Hi Pierre-Hugues, > > On Fri, Oct 13, 2017 at 7:53 PM, Pierre-Hugues Husson <phh@phh.me> 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
Hi Pierre, On Sat, Oct 14, 2017 at 10:21 AM, Pierre-Hugues Husson <phh@phh.me> wrote: > 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
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 <phh@phh.me> > > Acked-by: Hans Verkuil <hans.verkuil@cisco.com> Tested-by: Hans Verkuil <hans.verkuil@cisco.com> > > 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(&hdmi->i2c->adap); >> >
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?
2017-10-20 10:12 GMT+02:00 Russell King - ARM Linux <linux@armlinux.org.uk>: > 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
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(&hdmi->i2c->adap);
The documentation already mentions "cec" optional clock, but currently the driver doesn't enable it. Signed-off-by: Pierre-Hugues Husson <phh@phh.me> --- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)