Hi,
This is v3 of my sun6i DRM/KMS display enablement series. Most of the
patches in v2 were merged, leaving only the dumb-vga-dac patch and the
hummingbird-a31 VGA output patch.
A few things were changed, mainly the DAC enable method from a GPIO
to a regulator supply.
Changes since v2:
- Changed the enable-gpio of dumb-vga-dac to vdd-supply regulator.
This better matches the hardware that I have: the DAC has a fixed
regulator dropping the voltage from the board-wide 5V to 3.3V the
DAC uses. The regulator is controlled through a GPIO pin.
- Renamed the node of the VGA connector from "vga" to "vga-connector".
- Renamed the node of the VGA DAC from "bridge" to "vga-dac".
Regards
ChenYu
Chen-Yu Tsai (2):
drm/bridge: dumb-vga-dac: Support a VDD regulator supply
ARM: dts: sun6i: hummingbird-a31: Enable display output through VGA
bridge
.../bindings/display/bridge/dumb-vga-dac.txt | 2 +
arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 76 ++++++++++++++++++++++
drivers/gpu/drm/bridge/dumb-vga-dac.c | 35 ++++++++++
3 files changed, 113 insertions(+)
--
2.9.3
Some dumb VGA DACs are active components which require external power.
Add support for specifying a regulator as its power supply.
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
.../bindings/display/bridge/dumb-vga-dac.txt | 2 ++
drivers/gpu/drm/bridge/dumb-vga-dac.c | 35 ++++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
index 003bc246a270..164cbb15f04c 100644
--- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
+++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
@@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
- Video port 0 for RGB input
- Video port 1 for VGA output
+Optional properties:
+- vdd-supply: Power supply for DAC
Example
-------
diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index afec232185a7..59781e031220 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -12,6 +12,7 @@
#include <linux/module.h>
#include <linux/of_graph.h>
+#include <linux/regulator/consumer.h>
#include <drm/drmP.h>
#include <drm/drm_atomic_helper.h>
@@ -23,6 +24,7 @@ struct dumb_vga {
struct drm_connector connector;
struct i2c_adapter *ddc;
+ struct regulator *vdd;
};
static inline struct dumb_vga *
@@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
return 0;
}
+static void dumb_vga_enable(struct drm_bridge *bridge)
+{
+ struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
+ int ret;
+
+ if (!IS_ERR(vga->vdd)) {
+ ret = regulator_enable(vga->vdd);
+
+ if (ret) {
+ DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
+ return;
+ }
+ }
+}
+
+static void dumb_vga_disable(struct drm_bridge *bridge)
+{
+ struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
+
+ if (!IS_ERR(vga->vdd))
+ regulator_disable(vga->vdd);
+}
+
static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
.attach = dumb_vga_attach,
+ .enable = dumb_vga_enable,
+ .disable = dumb_vga_disable,
};
static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
@@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
return -ENOMEM;
platform_set_drvdata(pdev, vga);
+ vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
+ if (IS_ERR(vga->vdd)) {
+ ret = PTR_ERR(vga->vdd);
+ if (ret == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
+ }
+
vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
if (IS_ERR(vga->ddc)) {
if (PTR_ERR(vga->ddc) == -ENODEV) {
--
2.9.3
The Hummingbird A31 board has a VGA DAC which converts RGB output
from the LCD interface to VGA analog signals.
Add nodes for the VGA DAC, its power supply, and enable this part
of the display pipeline.
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 76 +++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
index 9a74637f677f..1ab28b2108fe 100644
--- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
+++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
@@ -63,6 +63,62 @@
stdout-path = "serial0:115200n8";
};
+ vga-connector {
+ compatible = "vga-connector";
+
+ port {
+ vga_con_in: endpoint {
+ remote-endpoint = <&vga_dac_out>;
+ };
+ };
+ };
+
+ vga-dac {
+ compatible = "dumb-vga-dac";
+ vdd-supply = <®_vga_3v3>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ vga_dac_in: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&tcon0_out_vga>;
+ };
+ };
+
+ port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+
+ vga_dac_out: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&vga_con_in>;
+ };
+ };
+ };
+ };
+
+ reg_vga_3v3: vga_3v3_regulator {
+ compatible = "regulator-fixed";
+ pinctrl-names = "default";
+ pinctrl-0 = <&vga_3v3_enable_pin_hummingbird>;
+ regulator-name = "vga-3v3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-boot-on;
+ enable-active-high;
+ gpio = <&pio 7 25 GPIO_ACTIVE_HIGH>; /* PH25 */
+ };
+
wifi_pwrseq: wifi_pwrseq {
compatible = "mmc-pwrseq-simple";
reset-gpios = <&pio 6 10 GPIO_ACTIVE_LOW>; /* PG10 */
@@ -166,6 +222,13 @@
allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
};
+ vga_3v3_enable_pin_hummingbird: vga_3v3_enable_pin {
+ allwinner,pins = "PH25";
+ allwinner,function = "gpio_out";
+ allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+ allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+ };
+
wifi_reset_pin_hummingbird: wifi_reset_pin@0 {
allwinner,pins = "PG10";
allwinner,function = "gpio_out";
@@ -245,6 +308,19 @@
status = "okay";
};
+&tcon0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&lcd0_rgb888_pins>;
+ status = "okay";
+};
+
+&tcon0_out {
+ tcon0_out_vga: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&vga_dac_in>;
+ };
+};
+
&uart0 {
pinctrl-names = "default";
pinctrl-0 = <&uart0_pins_a>;
--
2.9.3
On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
> Some dumb VGA DACs are active components which require external power.
> Add support for specifying a regulator as its power supply.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
> ---
> .../bindings/display/bridge/dumb-vga-dac.txt | 2 ++
For the binding,
Acked-by: Rob Herring <[email protected]>
One code comment below...
> drivers/gpu/drm/bridge/dumb-vga-dac.c | 35 ++++++++++++++++++++++
> 2 files changed, 37 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> index 003bc246a270..164cbb15f04c 100644
> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> - Video port 0 for RGB input
> - Video port 1 for VGA output
>
> +Optional properties:
> +- vdd-supply: Power supply for DAC
>
> Example
> -------
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> index afec232185a7..59781e031220 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -12,6 +12,7 @@
>
> #include <linux/module.h>
> #include <linux/of_graph.h>
> +#include <linux/regulator/consumer.h>
>
> #include <drm/drmP.h>
> #include <drm/drm_atomic_helper.h>
> @@ -23,6 +24,7 @@ struct dumb_vga {
> struct drm_connector connector;
>
> struct i2c_adapter *ddc;
> + struct regulator *vdd;
> };
>
> static inline struct dumb_vga *
> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
> return 0;
> }
>
> +static void dumb_vga_enable(struct drm_bridge *bridge)
> +{
> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> + int ret;
> +
> + if (!IS_ERR(vga->vdd)) {
if (IS_ERR())
return;
...will save some intentation.
> + ret = regulator_enable(vga->vdd);
> +
> + if (ret) {
> + DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
> + return;
> + }
> + }
> +}
> +
> +static void dumb_vga_disable(struct drm_bridge *bridge)
> +{
> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +
> + if (!IS_ERR(vga->vdd))
> + regulator_disable(vga->vdd);
> +}
> +
> static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
> .attach = dumb_vga_attach,
> + .enable = dumb_vga_enable,
> + .disable = dumb_vga_disable,
> };
>
> static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
> return -ENOMEM;
> platform_set_drvdata(pdev, vga);
>
> + vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
> + if (IS_ERR(vga->vdd)) {
> + ret = PTR_ERR(vga->vdd);
> + if (ret == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
> + }
> +
> vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
> if (IS_ERR(vga->ddc)) {
> if (PTR_ERR(vga->ddc) == -ENODEV) {
> --
> 2.9.3
>
On Mon, Oct 31, 2016 at 2:28 PM, Rob Herring <[email protected]> wrote:
> On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
>> Some dumb VGA DACs are active components which require external power.
>> Add support for specifying a regulator as its power supply.
>>
>> Signed-off-by: Chen-Yu Tsai <[email protected]>
>> ---
>> .../bindings/display/bridge/dumb-vga-dac.txt | 2 ++
>
> For the binding,
>
> Acked-by: Rob Herring <[email protected]>
>
> One code comment below...
>
>> drivers/gpu/drm/bridge/dumb-vga-dac.c | 35 ++++++++++++++++++++++
>> 2 files changed, 37 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> index 003bc246a270..164cbb15f04c 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>> - Video port 0 for RGB input
>> - Video port 1 for VGA output
>>
>> +Optional properties:
>> +- vdd-supply: Power supply for DAC
>>
>> Example
>> -------
>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> index afec232185a7..59781e031220 100644
>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> @@ -12,6 +12,7 @@
>>
>> #include <linux/module.h>
>> #include <linux/of_graph.h>
>> +#include <linux/regulator/consumer.h>
>>
>> #include <drm/drmP.h>
>> #include <drm/drm_atomic_helper.h>
>> @@ -23,6 +24,7 @@ struct dumb_vga {
>> struct drm_connector connector;
>>
>> struct i2c_adapter *ddc;
>> + struct regulator *vdd;
>> };
>>
>> static inline struct dumb_vga *
>> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>> return 0;
>> }
>>
>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>> +{
>> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>> + int ret;
>> +
>> + if (!IS_ERR(vga->vdd)) {
>
> if (IS_ERR())
> return;
>
> ...will save some intentation.
I thought about that, though if someone were to add more stuff to
this, such as an enable GPIO, they might have to rework it. A standalone
block of code would be easier to work with. I'm OK either way though.
ChenYu
>
>> + ret = regulator_enable(vga->vdd);
>> +
>> + if (ret) {
>> + DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
>> + return;
>> + }
>> + }
>> +}
>> +
>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>> +{
>> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>> +
>> + if (!IS_ERR(vga->vdd))
>> + regulator_disable(vga->vdd);
>> +}
>> +
>> static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>> .attach = dumb_vga_attach,
>> + .enable = dumb_vga_enable,
>> + .disable = dumb_vga_disable,
>> };
>>
>> static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
>> return -ENOMEM;
>> platform_set_drvdata(pdev, vga);
>>
>> + vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>> + if (IS_ERR(vga->vdd)) {
>> + ret = PTR_ERR(vga->vdd);
>> + if (ret == -EPROBE_DEFER)
>> + return -EPROBE_DEFER;
>> + dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
>> + }
>> +
>> vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>> if (IS_ERR(vga->ddc)) {
>> if (PTR_ERR(vga->ddc) == -ENODEV) {
>> --
>> 2.9.3
>>
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
Hi,
On Wed, Nov 2, 2016 at 9:33 AM, Chen-Yu Tsai <[email protected]> wrote:
> On Mon, Oct 31, 2016 at 2:28 PM, Rob Herring <[email protected]> wrote:
>> On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
>>> Some dumb VGA DACs are active components which require external power.
>>> Add support for specifying a regulator as its power supply.
>>>
>>> Signed-off-by: Chen-Yu Tsai <[email protected]>
>>> ---
>>> .../bindings/display/bridge/dumb-vga-dac.txt | 2 ++
>>
>> For the binding,
>>
>> Acked-by: Rob Herring <[email protected]>
Any comments on this patch from the DRM people?
ChenYu
>>
>> One code comment below...
>>
>>> drivers/gpu/drm/bridge/dumb-vga-dac.c | 35 ++++++++++++++++++++++
>>> 2 files changed, 37 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> index 003bc246a270..164cbb15f04c 100644
>>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>>> - Video port 0 for RGB input
>>> - Video port 1 for VGA output
>>>
>>> +Optional properties:
>>> +- vdd-supply: Power supply for DAC
>>>
>>> Example
>>> -------
>>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> index afec232185a7..59781e031220 100644
>>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> @@ -12,6 +12,7 @@
>>>
>>> #include <linux/module.h>
>>> #include <linux/of_graph.h>
>>> +#include <linux/regulator/consumer.h>
>>>
>>> #include <drm/drmP.h>
>>> #include <drm/drm_atomic_helper.h>
>>> @@ -23,6 +24,7 @@ struct dumb_vga {
>>> struct drm_connector connector;
>>>
>>> struct i2c_adapter *ddc;
>>> + struct regulator *vdd;
>>> };
>>>
>>> static inline struct dumb_vga *
>>> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>>> return 0;
>>> }
>>>
>>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>>> +{
>>> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>> + int ret;
>>> +
>>> + if (!IS_ERR(vga->vdd)) {
>>
>> if (IS_ERR())
>> return;
>>
>> ...will save some intentation.
>
> I thought about that, though if someone were to add more stuff to
> this, such as an enable GPIO, they might have to rework it. A standalone
> block of code would be easier to work with. I'm OK either way though.
>
> ChenYu
>
>>
>>> + ret = regulator_enable(vga->vdd);
>>> +
>>> + if (ret) {
>>> + DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
>>> + return;
>>> + }
>>> + }
>>> +}
>>> +
>>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>>> +{
>>> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>> +
>>> + if (!IS_ERR(vga->vdd))
>>> + regulator_disable(vga->vdd);
>>> +}
>>> +
>>> static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>>> .attach = dumb_vga_attach,
>>> + .enable = dumb_vga_enable,
>>> + .disable = dumb_vga_disable,
>>> };
>>>
>>> static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>>> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
>>> return -ENOMEM;
>>> platform_set_drvdata(pdev, vga);
>>>
>>> + vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>>> + if (IS_ERR(vga->vdd)) {
>>> + ret = PTR_ERR(vga->vdd);
>>> + if (ret == -EPROBE_DEFER)
>>> + return -EPROBE_DEFER;
>>> + dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
>>> + }
>>> +
>>> vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>>> if (IS_ERR(vga->ddc)) {
>>> if (PTR_ERR(vga->ddc) == -ENODEV) {
>>> --
>>> 2.9.3
>>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
>> For more options, visit https://groups.google.com/d/optout.
Hi,
On 11/15/2016 08:29 AM, Chen-Yu Tsai wrote:
> Hi,
>
> On Wed, Nov 2, 2016 at 9:33 AM, Chen-Yu Tsai <[email protected]> wrote:
>> On Mon, Oct 31, 2016 at 2:28 PM, Rob Herring <[email protected]> wrote:
>>> On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
>>>> Some dumb VGA DACs are active components which require external power.
>>>> Add support for specifying a regulator as its power supply.
>>>>
>>>> Signed-off-by: Chen-Yu Tsai <[email protected]>
>>>> ---
>>>> .../bindings/display/bridge/dumb-vga-dac.txt | 2 ++
>>>
>>> For the binding,
>>>
>>> Acked-by: Rob Herring <[email protected]>
>
> Any comments on this patch from the DRM people?
A comment below.
>
> ChenYu
>
>>>
>>> One code comment below...
>>>
>>>> drivers/gpu/drm/bridge/dumb-vga-dac.c | 35 ++++++++++++++++++++++
>>>> 2 files changed, 37 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>>> index 003bc246a270..164cbb15f04c 100644
>>>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>>> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>>>> - Video port 0 for RGB input
>>>> - Video port 1 for VGA output
>>>>
>>>> +Optional properties:
>>>> +- vdd-supply: Power supply for DAC
>>>>
>>>> Example
>>>> -------
>>>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>>> index afec232185a7..59781e031220 100644
>>>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>>> @@ -12,6 +12,7 @@
>>>>
>>>> #include <linux/module.h>
>>>> #include <linux/of_graph.h>
>>>> +#include <linux/regulator/consumer.h>
>>>>
>>>> #include <drm/drmP.h>
>>>> #include <drm/drm_atomic_helper.h>
>>>> @@ -23,6 +24,7 @@ struct dumb_vga {
>>>> struct drm_connector connector;
>>>>
>>>> struct i2c_adapter *ddc;
>>>> + struct regulator *vdd;
>>>> };
>>>>
>>>> static inline struct dumb_vga *
>>>> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>>>> return 0;
>>>> }
>>>>
>>>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>>>> +{
>>>> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>>> + int ret;
>>>> +
>>>> + if (!IS_ERR(vga->vdd)) {
>>>
>>> if (IS_ERR())
>>> return;
>>>
>>> ...will save some intentation.
>>
>> I thought about that, though if someone were to add more stuff to
>> this, such as an enable GPIO, they might have to rework it. A standalone
>> block of code would be easier to work with. I'm OK either way though.
>>
>> ChenYu
>>
>>>
>>>> + ret = regulator_enable(vga->vdd);
>>>> +
>>>> + if (ret) {
>>>> + DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
>>>> + return;
>>>> + }
>>>> + }
>>>> +}
>>>> +
>>>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>>>> +{
>>>> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>>> +
>>>> + if (!IS_ERR(vga->vdd))
>>>> + regulator_disable(vga->vdd);
>>>> +}
>>>> +
>>>> static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>>>> .attach = dumb_vga_attach,
>>>> + .enable = dumb_vga_enable,
>>>> + .disable = dumb_vga_disable,
>>>> };
>>>>
>>>> static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>>>> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
>>>> return -ENOMEM;
>>>> platform_set_drvdata(pdev, vga);
>>>>
>>>> + vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>>>> + if (IS_ERR(vga->vdd)) {
>>>> + ret = PTR_ERR(vga->vdd);
>>>> + if (ret == -EPROBE_DEFER)
>>>> + return -EPROBE_DEFER;
>>>> + dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
A cleaner way would be to set vga->vdd to NULL here, and do:
int ret = 0;
if (vga->vdd)
ret = regulator_enable(vga->vdd);
if (ret) {
...
return;
}
/* if a gpio is added later */
if (vga->gpio)
ret = gpiod_set_value_cansleep(vga->gpio, 1);
if (ret) {
...
return;
}
...
We don't really care what error code is contained in vga->vdd once we have
identified whether we got a regulator or not.
The same thing can be done for ddc too, but that can done left for later.
If you're okay with the suggestion, can you re-spin a patch?
Thanks,
Archit
>>>> + }
>>>> +
>>>> vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>>>> if (IS_ERR(vga->ddc)) {
>>>> if (PTR_ERR(vga->ddc) == -ENODEV) {
>>>> --
>>>> 2.9.3
>>>>
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
>>> For more options, visit https://groups.google.com/d/optout.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project