From: Ondrej Jirman <[email protected]>
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 <[email protected]>
---
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);
+
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 *
--
2.21.0
Hi,
On Tue, May 21, 2019 at 01:50:08AM +0200, [email protected] wrote:
> From: Ondrej Jirman <[email protected]>
>
> 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 <[email protected]>
> ---
> 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?
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
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, [email protected] wrote:
> > From: Ondrej Jirman <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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, [email protected] wrote:
> > > From: Ondrej Jirman <[email protected]>
> > >
> > > 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 <[email protected]>
> > > ---
> > > 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
