[PATCH 0/3] drm/bridge: display-connector: add external supply support

[PATCH 0/3] drm/bridge: display-connector: add external supply support

[Dmitry Baryshkov]

On some devices the 5V pin of the HDMI connector and/or the ESD
protection logic is powered on by a separate regulator. The dp-connector
for such usecases provides dp-pwr supply support. Follow this example
and make hdmi-connector support the hdmi-pwr supply.

Dmitry Baryshkov (3):
  dt-bindings: display: hdmi-connector: add hdmi-pwr supply
  drm/bridge: display-connector: rename dp_pwr to connector_pwr
  drm/bridge: display-connector: handle hdmi-pwr supply

 .../display/connector/hdmi-connector.yaml     |  3 ++
 drivers/gpu/drm/bridge/display-connector.c    | 51 +++++++++++++------
 2 files changed, 39 insertions(+), 15 deletions(-)

-- 
2.39.2

[PATCH 1/3] dt-bindings: display: hdmi-connector: add hdmi-pwr supply

[Dmitry Baryshkov]

Follow the dp-connector example and add hdmi-pwr supply to drive the 5V
pin of the HDMI connector (together with some simple glue logic possibly
attached to the connector).

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../devicetree/bindings/display/connector/hdmi-connector.yaml  | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/connector/hdmi-connector.yaml b/Documentation/devicetree/bindings/display/connector/hdmi-connector.yaml
index 83c0d008265b..94f75359c6ff 100644
--- a/Documentation/devicetree/bindings/display/connector/hdmi-connector.yaml
+++ b/Documentation/devicetree/bindings/display/connector/hdmi-connector.yaml
@@ -36,6 +36,9 @@ properties:
     description: GPIO signal to enable DDC bus
     maxItems: 1
 
+  hdmi-pwr-supply:
+    description: Power supply for the HDMI 5v pin connector
+
   port:
     $ref: /schemas/graph.yaml#/properties/port
     description: Connection to controller providing HDMI signals
-- 
2.39.2

Re: [PATCH 1/3] dt-bindings: display: hdmi-connector: add hdmi-pwr supply

[Rob Herring]

On Sun, 07 May 2023 23:12:16 +0300, Dmitry Baryshkov wrote:
> Follow the dp-connector example and add hdmi-pwr supply to drive the 5V
> pin of the HDMI connector (together with some simple glue logic possibly
> attached to the connector).
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../devicetree/bindings/display/connector/hdmi-connector.yaml  | 3 +++
>  1 file changed, 3 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.example.dtb: camera-sensor@3c: port:endpoint:data-lanes: [[1]] is too short
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/rockchip-isp1.example.dtb: camera@3c: port:endpoint:data-lanes: [[1]] is too short
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.example.dtb: pcie-ep@33800000: Unevaluated properties are not allowed ('assigned-clock-parents', 'assigned-clock-rates', 'assigned-clocks' were unexpected)
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml

doc reference errors (make refcheckdocs):
Documentation/usb/gadget_uvc.rst: Documentation/userspace-api/media/v4l/pixfmt-packed.yuv.rst
MAINTAINERS: Documentation/devicetree/bindings/pwm/pwm-apple.yaml

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230507201218.2339014-2-dmitry.baryshkov@linaro.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 1/3] dt-bindings: display: hdmi-connector: add hdmi-pwr supply

[Laurent Pinchart]

Hi Rob,

On Sun, May 07, 2023 at 04:25:44PM -0500, Rob Herring wrote:
> On Sun, 07 May 2023 23:12:16 +0300, Dmitry Baryshkov wrote:
> > Follow the dp-connector example and add hdmi-pwr supply to drive the 5V
> > pin of the HDMI connector (together with some simple glue logic possibly
> > attached to the connector).
> > 
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  .../devicetree/bindings/display/connector/hdmi-connector.yaml  | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):

The issues below don't seem to be related to Dmitry's patch, are they ?

> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.example.dtb: camera-sensor@3c: port:endpoint:data-lanes: [[1]] is too short
> 	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/rockchip-isp1.example.dtb: camera@3c: port:endpoint:data-lanes: [[1]] is too short
> 	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.example.dtb: pcie-ep@33800000: Unevaluated properties are not allowed ('assigned-clock-parents', 'assigned-clock-rates', 'assigned-clocks' were unexpected)
> 	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> 
> doc reference errors (make refcheckdocs):
> Documentation/usb/gadget_uvc.rst: Documentation/userspace-api/media/v4l/pixfmt-packed.yuv.rst
> MAINTAINERS: Documentation/devicetree/bindings/pwm/pwm-apple.yaml
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230507201218.2339014-2-dmitry.baryshkov@linaro.org
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/3] dt-bindings: display: hdmi-connector: add hdmi-pwr supply

[Krzysztof Kozlowski]

On 08/05/2023 04:56, Laurent Pinchart wrote:
> Hi Rob,
> 
> On Sun, May 07, 2023 at 04:25:44PM -0500, Rob Herring wrote:
>> On Sun, 07 May 2023 23:12:16 +0300, Dmitry Baryshkov wrote:
>>> Follow the dp-connector example and add hdmi-pwr supply to drive the 5V
>>> pin of the HDMI connector (together with some simple glue logic possibly
>>> attached to the connector).
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>>  .../devicetree/bindings/display/connector/hdmi-connector.yaml  | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>
>> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
>> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> The issues below don't seem to be related to Dmitry's patch, are they ?

No, can be ignored.

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: display: hdmi-connector: add hdmi-pwr supply

[Laurent Pinchart]

Hi Dmitry,

Thank you for the patch.

On Sun, May 07, 2023 at 11:12:16PM +0300, Dmitry Baryshkov wrote:
> Follow the dp-connector example and add hdmi-pwr supply to drive the 5V
> pin of the HDMI connector (together with some simple glue logic possibly
> attached to the connector).
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../devicetree/bindings/display/connector/hdmi-connector.yaml  | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/connector/hdmi-connector.yaml b/Documentation/devicetree/bindings/display/connector/hdmi-connector.yaml
> index 83c0d008265b..94f75359c6ff 100644
> --- a/Documentation/devicetree/bindings/display/connector/hdmi-connector.yaml
> +++ b/Documentation/devicetree/bindings/display/connector/hdmi-connector.yaml
> @@ -36,6 +36,9 @@ properties:
>      description: GPIO signal to enable DDC bus
>      maxItems: 1
>  
> +  hdmi-pwr-supply:
> +    description: Power supply for the HDMI 5v pin connector

I'd write

    description: Power supply for the HDMI +5V Power pin

to match the HDMI specification. With that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
>    port:
>      $ref: /schemas/graph.yaml#/properties/port
>      description: Connection to controller providing HDMI signals

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/3] dt-bindings: display: hdmi-connector: add hdmi-pwr supply

[Krzysztof Kozlowski]

On 07/05/2023 22:12, Dmitry Baryshkov wrote:
> Follow the dp-connector example and add hdmi-pwr supply to drive the 5V
> pin of the HDMI connector (together with some simple glue logic possibly
> attached to the connector).
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH 2/3] drm/bridge: display-connector: rename dp_pwr to connector_pwr

[Dmitry Baryshkov]

In preparation to adding support for the hdmi_pwr supply, rename dp_pwr
structure field to the generic connector_pwr.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/bridge/display-connector.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
index 9a12449ad7b8..0d94e6edea50 100644
--- a/drivers/gpu/drm/bridge/display-connector.c
+++ b/drivers/gpu/drm/bridge/display-connector.c
@@ -24,7 +24,7 @@ struct display_connector {
 	struct gpio_desc	*hpd_gpio;
 	int			hpd_irq;
 
-	struct regulator	*dp_pwr;
+	struct regulator	*connector_pwr;
 	struct gpio_desc	*ddc_en;
 };
 
@@ -319,14 +319,14 @@ static int display_connector_probe(struct platform_device *pdev)
 	if (type == DRM_MODE_CONNECTOR_DisplayPort) {
 		int ret;
 
-		conn->dp_pwr = devm_regulator_get_optional(&pdev->dev, "dp-pwr");
+		conn->connector_pwr = devm_regulator_get_optional(&pdev->dev, "dp-pwr");
 
-		if (IS_ERR(conn->dp_pwr)) {
-			ret = PTR_ERR(conn->dp_pwr);
+		if (IS_ERR(conn->connector_pwr)) {
+			ret = PTR_ERR(conn->connector_pwr);
 
 			switch (ret) {
 			case -ENODEV:
-				conn->dp_pwr = NULL;
+				conn->connector_pwr = NULL;
 				break;
 
 			case -EPROBE_DEFER:
@@ -338,8 +338,8 @@ static int display_connector_probe(struct platform_device *pdev)
 			}
 		}
 
-		if (conn->dp_pwr) {
-			ret = regulator_enable(conn->dp_pwr);
+		if (conn->connector_pwr) {
+			ret = regulator_enable(conn->connector_pwr);
 			if (ret) {
 				dev_err(&pdev->dev, "failed to enable DP PWR regulator: %d\n", ret);
 				return ret;
@@ -389,8 +389,8 @@ static int display_connector_remove(struct platform_device *pdev)
 	if (conn->ddc_en)
 		gpiod_set_value(conn->ddc_en, 0);
 
-	if (conn->dp_pwr)
-		regulator_disable(conn->dp_pwr);
+	if (conn->connector_pwr)
+		regulator_disable(conn->connector_pwr);
 
 	drm_bridge_remove(&conn->bridge);
 
-- 
2.39.2

Re: [PATCH 2/3] drm/bridge: display-connector: rename dp_pwr to connector_pwr

[Laurent Pinchart]

Hi Dmitry,

Thank you for the patch.

On Sun, May 07, 2023 at 11:12:17PM +0300, Dmitry Baryshkov wrote:
> In preparation to adding support for the hdmi_pwr supply, rename dp_pwr
> structure field to the generic connector_pwr.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/gpu/drm/bridge/display-connector.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
> index 9a12449ad7b8..0d94e6edea50 100644
> --- a/drivers/gpu/drm/bridge/display-connector.c
> +++ b/drivers/gpu/drm/bridge/display-connector.c
> @@ -24,7 +24,7 @@ struct display_connector {
>  	struct gpio_desc	*hpd_gpio;
>  	int			hpd_irq;
>  
> -	struct regulator	*dp_pwr;
> +	struct regulator	*connector_pwr;

This makes sense, but I would shorten the name to just "pwr", "power" or
"supply". The field is part of the display_connector structure, so it
implicitly refers to the connector.

With or without that change,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	struct gpio_desc	*ddc_en;
>  };
>  
> @@ -319,14 +319,14 @@ static int display_connector_probe(struct platform_device *pdev)
>  	if (type == DRM_MODE_CONNECTOR_DisplayPort) {
>  		int ret;
>  
> -		conn->dp_pwr = devm_regulator_get_optional(&pdev->dev, "dp-pwr");
> +		conn->connector_pwr = devm_regulator_get_optional(&pdev->dev, "dp-pwr");
>  
> -		if (IS_ERR(conn->dp_pwr)) {
> -			ret = PTR_ERR(conn->dp_pwr);
> +		if (IS_ERR(conn->connector_pwr)) {
> +			ret = PTR_ERR(conn->connector_pwr);
>  
>  			switch (ret) {
>  			case -ENODEV:
> -				conn->dp_pwr = NULL;
> +				conn->connector_pwr = NULL;
>  				break;
>  
>  			case -EPROBE_DEFER:
> @@ -338,8 +338,8 @@ static int display_connector_probe(struct platform_device *pdev)
>  			}
>  		}
>  
> -		if (conn->dp_pwr) {
> -			ret = regulator_enable(conn->dp_pwr);
> +		if (conn->connector_pwr) {
> +			ret = regulator_enable(conn->connector_pwr);
>  			if (ret) {
>  				dev_err(&pdev->dev, "failed to enable DP PWR regulator: %d\n", ret);
>  				return ret;
> @@ -389,8 +389,8 @@ static int display_connector_remove(struct platform_device *pdev)
>  	if (conn->ddc_en)
>  		gpiod_set_value(conn->ddc_en, 0);
>  
> -	if (conn->dp_pwr)
> -		regulator_disable(conn->dp_pwr);
> +	if (conn->connector_pwr)
> +		regulator_disable(conn->connector_pwr);
>  
>  	drm_bridge_remove(&conn->bridge);
>  

-- 
Regards,

Laurent Pinchart

[PATCH 3/3] drm/bridge: display-connector: handle hdmi-pwr supply

[Dmitry Baryshkov]

On some devices the 5V pin of the HDMI connector and/or the ESD
protection logic is powered on by a separate regulator. Instead of
declaring this regulator as always-on, make hdmi-connector support the
additional hdmi-pwr supply.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/bridge/display-connector.c | 37 +++++++++++++++++-----
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
index 0d94e6edea50..037ba2eb5a2f 100644
--- a/drivers/gpu/drm/bridge/display-connector.c
+++ b/drivers/gpu/drm/bridge/display-connector.c
@@ -337,18 +337,12 @@ static int display_connector_probe(struct platform_device *pdev)
 				return ret;
 			}
 		}
-
-		if (conn->connector_pwr) {
-			ret = regulator_enable(conn->connector_pwr);
-			if (ret) {
-				dev_err(&pdev->dev, "failed to enable DP PWR regulator: %d\n", ret);
-				return ret;
-			}
-		}
 	}
 
 	/* enable DDC */
 	if (type == DRM_MODE_CONNECTOR_HDMIA) {
+		int ret;
+
 		conn->ddc_en = devm_gpiod_get_optional(&pdev->dev, "ddc-en",
 						       GPIOD_OUT_HIGH);
 
@@ -356,6 +350,33 @@ static int display_connector_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev, "Couldn't get ddc-en gpio\n");
 			return PTR_ERR(conn->ddc_en);
 		}
+
+		conn->connector_pwr = devm_regulator_get_optional(&pdev->dev, "hdmi-pwr");
+
+		if (IS_ERR(conn->connector_pwr)) {
+			ret = PTR_ERR(conn->connector_pwr);
+
+			switch (ret) {
+			case -ENODEV:
+				conn->connector_pwr = NULL;
+				break;
+
+			case -EPROBE_DEFER:
+				return -EPROBE_DEFER;
+
+			default:
+				dev_err(&pdev->dev, "failed to get HDMI PWR regulator: %d\n", ret);
+				return ret;
+			}
+		}
+	}
+
+	if (conn->connector_pwr) {
+		ret = regulator_enable(conn->connector_pwr);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to enable DP PWR regulator: %d\n", ret);
+			return ret;
+		}
 	}
 
 	conn->bridge.funcs = &display_connector_bridge_funcs;
-- 
2.39.2

Re: [PATCH 3/3] drm/bridge: display-connector: handle hdmi-pwr supply

[Laurent Pinchart]

Hi Dmitry,

Thank you for the patch.

On Sun, May 07, 2023 at 11:12:18PM +0300, Dmitry Baryshkov wrote:
> On some devices the 5V pin of the HDMI connector and/or the ESD
> protection logic is powered on by a separate regulator. Instead of
> declaring this regulator as always-on, make hdmi-connector support the
> additional hdmi-pwr supply.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/gpu/drm/bridge/display-connector.c | 37 +++++++++++++++++-----
>  1 file changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
> index 0d94e6edea50..037ba2eb5a2f 100644
> --- a/drivers/gpu/drm/bridge/display-connector.c
> +++ b/drivers/gpu/drm/bridge/display-connector.c
> @@ -337,18 +337,12 @@ static int display_connector_probe(struct platform_device *pdev)
>  				return ret;
>  			}
>  		}
> -
> -		if (conn->connector_pwr) {
> -			ret = regulator_enable(conn->connector_pwr);
> -			if (ret) {
> -				dev_err(&pdev->dev, "failed to enable DP PWR regulator: %d\n", ret);
> -				return ret;
> -			}
> -		}
>  	}
>  
>  	/* enable DDC */
>  	if (type == DRM_MODE_CONNECTOR_HDMIA) {
> +		int ret;
> +
>  		conn->ddc_en = devm_gpiod_get_optional(&pdev->dev, "ddc-en",
>  						       GPIOD_OUT_HIGH);
>  
> @@ -356,6 +350,33 @@ static int display_connector_probe(struct platform_device *pdev)
>  			dev_err(&pdev->dev, "Couldn't get ddc-en gpio\n");
>  			return PTR_ERR(conn->ddc_en);
>  		}
> +
> +		conn->connector_pwr = devm_regulator_get_optional(&pdev->dev, "hdmi-pwr");
> +
> +		if (IS_ERR(conn->connector_pwr)) {
> +			ret = PTR_ERR(conn->connector_pwr);
> +
> +			switch (ret) {
> +			case -ENODEV:
> +				conn->connector_pwr = NULL;
> +				break;
> +
> +			case -EPROBE_DEFER:
> +				return -EPROBE_DEFER;
> +
> +			default:
> +				dev_err(&pdev->dev, "failed to get HDMI PWR regulator: %d\n", ret);
> +				return ret;
> +			}
> +		}
> +	}

Please share this logic with the DP code. You can move it to a separate
function for instance, that would take the regulator name as a
parameter.

> +
> +	if (conn->connector_pwr) {
> +		ret = regulator_enable(conn->connector_pwr);
> +		if (ret) {
> +			dev_err(&pdev->dev, "failed to enable DP PWR regulator: %d\n", ret);
> +			return ret;
> +		}
>  	}
>  
>  	conn->bridge.funcs = &display_connector_bridge_funcs;

-- 
Regards,

Laurent Pinchart