diff mbox

[v2,1/2] drm/bridge: dw-hdmi: support optional supply regulators

Message ID 2265807.RXHBN9kUi4@phil (mailing list archive)
State Superseded
Headers show

Commit Message

Heiko Stübner March 10, 2015, 9:45 p.m. UTC
At least the Rockchip variant of the dw_hdmi can have controllable power supplies
providing 1.0 and 1.8V. Therefore add the possibility for the generic bridge
driver to enable supplies provided by the hw-specific drivers.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
changes since v1:
- follow suggestion from Russell King to keep regulator handling local
  to the rockchip implementation for the time being and only generalize
  when a real second implementation needs regulator handling

 .../devicetree/bindings/drm/bridge/dw_hdmi.txt     |  5 ++++
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 32 +++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

Comments

Philipp Zabel March 11, 2015, 9:28 a.m. UTC | #1
Hi Heiko,

Am Dienstag, den 10.03.2015, 22:45 +0100 schrieb Heiko Stuebner:
> At least the Rockchip variant of the dw_hdmi can have controllable power supplies
> providing 1.0 and 1.8V. Therefore add the possibility for the generic bridge
> driver to enable supplies provided by the hw-specific drivers.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
> changes since v1:
> - follow suggestion from Russell King to keep regulator handling local
>   to the rockchip implementation for the time being and only generalize
>   when a real second implementation needs regulator handling
> 
>  .../devicetree/bindings/drm/bridge/dw_hdmi.txt     |  5 ++++
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 32 +++++++++++++++++++++-
>  2 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt
> index a905c14..bb74640 100644
> --- a/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt
> +++ b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt
> @@ -22,6 +22,11 @@ Optional properties
>  - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
>  - clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec"
>  
> +Optional supplies:
> +rockchip,rk3288-dw-hdmi handles two optional power supplies:
> +- avdd1v0-supply: 1.0V power supply
> +- avdd1v8-supply: 1.8V power supply

Are these the names used in the Rockchip documentation?

Since the older implementation on i.MX6 uses 1.1V (HDMI_VP) and 2.5V
(HDMI_VPH), I wonder whether each SoC should use their own name or
whether there should be common names that don't include the voltage.
I don't have the Synopsys HDMI TX docs, but I've seen avddhv and avddlv
used of other cores' analog supplies.

regards
Philipp
Heiko Stübner March 11, 2015, 9:50 a.m. UTC | #2
Hi Philipp,

Am Mittwoch, 11. März 2015, 10:28:29 schrieb Philipp Zabel:
> Am Dienstag, den 10.03.2015, 22:45 +0100 schrieb Heiko Stuebner:
> > At least the Rockchip variant of the dw_hdmi can have controllable power
> > supplies providing 1.0 and 1.8V. Therefore add the possibility for the
> > generic bridge driver to enable supplies provided by the hw-specific
> > drivers.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> > changes since v1:
> > - follow suggestion from Russell King to keep regulator handling local
> > 
> >   to the rockchip implementation for the time being and only generalize
> >   when a real second implementation needs regulator handling
> >  
> >  .../devicetree/bindings/drm/bridge/dw_hdmi.txt     |  5 ++++
> >  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c        | 32
> >  +++++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt
> > b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt index
> > a905c14..bb74640 100644
> > --- a/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt
> > +++ b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt
> > @@ -22,6 +22,11 @@ Optional properties
> > 
> >  - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
> >  - clocks, clock-names: phandle to the HDMI CEC clock, name should be
> >  "cec"
> > 
> > +Optional supplies:
> > +rockchip,rk3288-dw-hdmi handles two optional power supplies:
> > +- avdd1v0-supply: 1.0V power supply
> > +- avdd1v8-supply: 1.8V power supply
> 
> Are these the names used in the Rockchip documentation?
> 
> Since the older implementation on i.MX6 uses 1.1V (HDMI_VP) and 2.5V
> (HDMI_VPH), I wonder whether each SoC should use their own name or
> whether there should be common names that don't include the voltage.
> I don't have the Synopsys HDMI TX docs, but I've seen avddhv and avddlv
> used of other cores' analog supplies.

The pins of the soc connected to the regulators are named:
	HDMI_AVDD_1V0
	HDMI_AVDD_1V8
The datasheet only calls both "DC supply voltage for Analog part of HDMI" and 
never mentions them again.

The databook of the IP block itself seems to really call this VPH and VP as 
phy supplies, but documentation is a bit sparse about them.

I guess as we already use the databook clock-names I should also switch to the 
databook supply names (avdd1v0 -> vp and avdd1v8 -> vph).


Heiko
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt
index a905c14..bb74640 100644
--- a/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt
+++ b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt
@@ -22,6 +22,11 @@  Optional properties
 - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
 - clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec"
 
+Optional supplies:
+rockchip,rk3288-dw-hdmi handles two optional power supplies:
+- avdd1v0-supply: 1.0V power supply
+- avdd1v8-supply: 1.8V power supply
+
 Example:
 	hdmi: hdmi@0120000 {
 		compatible = "fsl,imx6q-hdmi";
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index d236faa..647a240 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -11,6 +11,7 @@ 
 #include <linux/platform_device.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <drm/drm_of.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
@@ -28,6 +29,9 @@  struct rockchip_hdmi {
 	struct device *dev;
 	struct regmap *regmap;
 	struct drm_encoder encoder;
+	struct regulator_bulk_data supplies[2];
+	int nsupplies;
+	bool supplies_enabled;
 };
 
 #define to_rockchip_hdmi(x)	container_of(x, struct rockchip_hdmi, x)
@@ -179,6 +183,12 @@  static struct drm_encoder_funcs dw_hdmi_rockchip_encoder_funcs = {
 
 static void dw_hdmi_rockchip_encoder_disable(struct drm_encoder *encoder)
 {
+	struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder);
+
+	if (hdmi->nsupplies > 0 && hdmi->supplies_enabled) {
+		regulator_bulk_disable(hdmi->nsupplies, hdmi->supplies);
+		hdmi->supplies_enabled = false;
+	}
 }
 
 static bool
@@ -199,7 +209,16 @@  static void dw_hdmi_rockchip_encoder_commit(struct drm_encoder *encoder)
 {
 	struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder);
 	u32 val;
-	int mux;
+	int mux, ret;
+
+	if (hdmi->nsupplies > 0 && !hdmi->supplies_enabled) {
+		ret = regulator_bulk_enable(hdmi->nsupplies, hdmi->supplies);
+		if (ret) {
+			dev_err(hdmi->dev, "could not enable hdmi analog supplies\n");
+			return;
+		}
+		hdmi->supplies_enabled = true;
+	}
 
 	mux = rockchip_drm_encoder_get_mux_id(hdmi->dev->of_node, encoder);
 	if (mux)
@@ -275,6 +294,17 @@  static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
 	if (!iores)
 		return -ENXIO;
 
+	hdmi->supplies[0].supply = "avdd1v0";
+	hdmi->supplies[1].supply = "avdd1v8";
+	hdmi->nsupplies = 2;
+
+	ret = devm_regulator_bulk_get(hdmi->dev,
+				      hdmi->nsupplies, hdmi->supplies);
+	if (ret == -EPROBE_DEFER)
+		return ret;
+	if (ret)
+		hdmi->nsupplies = 0;
+
 	platform_set_drvdata(pdev, hdmi);
 
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);