| Message ID | 20190520235009.16734-6-megous@megous.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for Orange Pi 3 | expand |
Hi Maxime, On Tue, May 21, 2019 at 01:46:11PM +0200, Maxime Ripard wrote: > Hi, > > On Tue, May 21, 2019 at 01:50:08AM +0200, megous@megous.com wrote: > > From: Ondrej Jirman <megous@megous.com> > > > > Orange Pi 3 board requires enabling a voltage shifting circuit via GPIO > > for the DDC bus to be usable. > > > > Add support for hdmi-connector node's optional ddc-en-gpios property to > > support this use case. > > > > Signed-off-by: Ondrej Jirman <megous@megous.com> > > --- > > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 55 +++++++++++++++++++++++++-- > > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 3 ++ > > 2 files changed, 55 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > > index 39d8509d96a0..59b81ba02d96 100644 > > --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > > +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > > @@ -98,6 +98,30 @@ static u32 sun8i_dw_hdmi_find_possible_crtcs(struct drm_device *drm, > > return crtcs; > > } > > > > +static int sun8i_dw_hdmi_find_connector_pdev(struct device *dev, > > + struct platform_device **pdev_out) > > +{ > > + struct platform_device *pdev; > > + struct device_node *remote; > > + > > + remote = of_graph_get_remote_node(dev->of_node, 1, -1); > > + if (!remote) > > + return -ENODEV; > > + > > + if (!of_device_is_compatible(remote, "hdmi-connector")) { > > + of_node_put(remote); > > + return -ENODEV; > > + } > > + > > + pdev = of_find_device_by_node(remote); > > + of_node_put(remote); > > + if (!pdev) > > + return -ENODEV; > > + > > + *pdev_out = pdev; > > + return 0; > > +} > > + > > static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master, > > void *data) > > { > > @@ -151,16 +175,29 @@ static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master, > > return PTR_ERR(hdmi->regulator); > > } > > > > + ret = sun8i_dw_hdmi_find_connector_pdev(dev, &hdmi->connector_pdev); > > + if (!ret) { > > + hdmi->ddc_en = gpiod_get_optional(&hdmi->connector_pdev->dev, > > + "ddc-en", GPIOD_OUT_HIGH); > > + if (IS_ERR(hdmi->ddc_en)) { > > + platform_device_put(hdmi->connector_pdev); > > + dev_err(dev, "Couldn't get ddc-en gpio\n"); > > + return PTR_ERR(hdmi->ddc_en); > > + } > > + } > > + > > ret = regulator_enable(hdmi->regulator); > > if (ret) { > > dev_err(dev, "Failed to enable regulator\n"); > > - return ret; > > + goto err_unref_ddc_en; > > } > > > > + gpiod_set_value(hdmi->ddc_en, 1); > > + > > Do you really need this to be done all the time? I'm guessing you > would only need this when running .get_modes, right? I don't think it hurts anything. Enabled voltage shifting circuit doesn't draw any current, unless DDC is actually transmitting data. On most boards I'd imagine this circuit is always on anyway (Orange Pi 3 schematic even has an option to tie this signal to VCC-IO instead of GPIO). Schematic: https://megous.com/dl/tmp/bfcdd32d655aaa76.png thank you and regards, o. > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
On Tue, May 21, 2019 at 02:15:19PM +0200, Ondřej Jirman wrote: > Hi Maxime, > > On Tue, May 21, 2019 at 01:46:11PM +0200, Maxime Ripard wrote: > > Hi, > > > > On Tue, May 21, 2019 at 01:50:08AM +0200, megous@megous.com wrote: > > > From: Ondrej Jirman <megous@megous.com> > > > > > > Orange Pi 3 board requires enabling a voltage shifting circuit via GPIO > > > for the DDC bus to be usable. > > > > > > Add support for hdmi-connector node's optional ddc-en-gpios property to > > > support this use case. > > > > > > Signed-off-by: Ondrej Jirman <megous@megous.com> > > > --- > > > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 55 +++++++++++++++++++++++++-- > > > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 3 ++ > > > 2 files changed, 55 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > > > index 39d8509d96a0..59b81ba02d96 100644 > > > --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > > > +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > > > @@ -98,6 +98,30 @@ static u32 sun8i_dw_hdmi_find_possible_crtcs(struct drm_device *drm, > > > return crtcs; > > > } > > > > > > +static int sun8i_dw_hdmi_find_connector_pdev(struct device *dev, > > > + struct platform_device **pdev_out) > > > +{ > > > + struct platform_device *pdev; > > > + struct device_node *remote; > > > + > > > + remote = of_graph_get_remote_node(dev->of_node, 1, -1); > > > + if (!remote) > > > + return -ENODEV; > > > + > > > + if (!of_device_is_compatible(remote, "hdmi-connector")) { > > > + of_node_put(remote); > > > + return -ENODEV; > > > + } > > > + > > > + pdev = of_find_device_by_node(remote); > > > + of_node_put(remote); > > > + if (!pdev) > > > + return -ENODEV; > > > + > > > + *pdev_out = pdev; > > > + return 0; > > > +} > > > + > > > static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master, > > > void *data) > > > { > > > @@ -151,16 +175,29 @@ static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master, > > > return PTR_ERR(hdmi->regulator); > > > } > > > > > > + ret = sun8i_dw_hdmi_find_connector_pdev(dev, &hdmi->connector_pdev); > > > + if (!ret) { > > > + hdmi->ddc_en = gpiod_get_optional(&hdmi->connector_pdev->dev, > > > + "ddc-en", GPIOD_OUT_HIGH); > > > + if (IS_ERR(hdmi->ddc_en)) { > > > + platform_device_put(hdmi->connector_pdev); > > > + dev_err(dev, "Couldn't get ddc-en gpio\n"); > > > + return PTR_ERR(hdmi->ddc_en); > > > + } > > > + } > > > + > > > ret = regulator_enable(hdmi->regulator); > > > if (ret) { > > > dev_err(dev, "Failed to enable regulator\n"); > > > - return ret; > > > + goto err_unref_ddc_en; > > > } > > > > > > + gpiod_set_value(hdmi->ddc_en, 1); > > > + > > > > Do you really need this to be done all the time? I'm guessing you > > would only need this when running .get_modes, right? > > I don't think it hurts anything. Enabled voltage shifting circuit doesn't > draw any current, unless DDC is actually transmitting data. On most boards > I'd imagine this circuit is always on anyway (Orange Pi 3 schematic even has > an option to tie this signal to VCC-IO instead of GPIO). Ok, it works for me then Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c index 39d8509d96a0..59b81ba02d96 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c @@ -98,6 +98,30 @@ static u32 sun8i_dw_hdmi_find_possible_crtcs(struct drm_device *drm, return crtcs; } +static int sun8i_dw_hdmi_find_connector_pdev(struct device *dev, + struct platform_device **pdev_out) +{ + struct platform_device *pdev; + struct device_node *remote; + + remote = of_graph_get_remote_node(dev->of_node, 1, -1); + if (!remote) + return -ENODEV; + + if (!of_device_is_compatible(remote, "hdmi-connector")) { + of_node_put(remote); + return -ENODEV; + } + + pdev = of_find_device_by_node(remote); + of_node_put(remote); + if (!pdev) + return -ENODEV; + + *pdev_out = pdev; + return 0; +} + static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master, void *data) { @@ -151,16 +175,29 @@ static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master, return PTR_ERR(hdmi->regulator); } + ret = sun8i_dw_hdmi_find_connector_pdev(dev, &hdmi->connector_pdev); + if (!ret) { + hdmi->ddc_en = gpiod_get_optional(&hdmi->connector_pdev->dev, + "ddc-en", GPIOD_OUT_HIGH); + if (IS_ERR(hdmi->ddc_en)) { + platform_device_put(hdmi->connector_pdev); + dev_err(dev, "Couldn't get ddc-en gpio\n"); + return PTR_ERR(hdmi->ddc_en); + } + } + ret = regulator_enable(hdmi->regulator); if (ret) { dev_err(dev, "Failed to enable regulator\n"); - return ret; + goto err_unref_ddc_en; } + gpiod_set_value(hdmi->ddc_en, 1); + ret = reset_control_deassert(hdmi->rst_ctrl); if (ret) { dev_err(dev, "Could not deassert ctrl reset control\n"); - goto err_disable_regulator; + goto err_disable_ddc_en; } ret = clk_prepare_enable(hdmi->clk_tmds); @@ -213,8 +250,14 @@ static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master, clk_disable_unprepare(hdmi->clk_tmds); err_assert_ctrl_reset: reset_control_assert(hdmi->rst_ctrl); -err_disable_regulator: +err_disable_ddc_en: + gpiod_set_value(hdmi->ddc_en, 0); regulator_disable(hdmi->regulator); +err_unref_ddc_en: + if (hdmi->ddc_en) + gpiod_put(hdmi->ddc_en); + + platform_device_put(hdmi->connector_pdev); return ret; } @@ -228,7 +271,13 @@ static void sun8i_dw_hdmi_unbind(struct device *dev, struct device *master, sun8i_hdmi_phy_remove(hdmi); clk_disable_unprepare(hdmi->clk_tmds); reset_control_assert(hdmi->rst_ctrl); + gpiod_set_value(hdmi->ddc_en, 0); regulator_disable(hdmi->regulator); + + if (hdmi->ddc_en) + gpiod_put(hdmi->ddc_en); + + platform_device_put(hdmi->connector_pdev); } static const struct component_ops sun8i_dw_hdmi_ops = { diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 720c5aa8adc1..dad66b8301c2 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h @@ -9,6 +9,7 @@ #include <drm/bridge/dw_hdmi.h> #include <drm/drm_encoder.h> #include <linux/clk.h> +#include <linux/gpio/consumer.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> #include <linux/reset.h> @@ -190,6 +191,8 @@ struct sun8i_dw_hdmi { struct regulator *regulator; const struct sun8i_dw_hdmi_quirks *quirks; struct reset_control *rst_ctrl; + struct platform_device *connector_pdev; + struct gpio_desc *ddc_en; }; static inline struct sun8i_dw_hdmi *