[PATCH 0/6] Add LCD support for Pine64 Pinebook 1080p

[PATCH 0/6] Add LCD support for Pine64 Pinebook 1080p

[Vasily Khoruzhick]

Since ANX6345 driver has been merged we can add support for Pinebook LCD

This is a follow up on [1] which attempted to add support for all the
A64-based Pinebooks.

Since patches for 768p were dropped we don't need edp-connector binding
discussed in [1] and its earlier versions and we can use panel-simple
binding as everyone else does.

If we ever going to add support for 768p we can do it through dt-overlay
with appropriate panel node or by teaching bootloader to patch dtb with
correct panel compatible.

Similar approach was chosen in [2]

[1] https://patchwork.kernel.org/cover/10814169/
[2] https://patchwork.kernel.org/patch/11277765/

Icenowy Zheng (1):
  arm64: allwinner: a64: enable LCD-related hardware for Pinebook

Samuel Holland (1):
  drm/bridge: anx6345: Fix getting anx6345 regulators

Vasily Khoruzhick (4):
  drm/bridge: anx6345: Clean up error handling in probe()
  dt-bindings: Add Guangdong Neweast Optoelectronics CO. LTD vendor
    prefix
  dt-bindings: display: simple: Add NewEast Optoelectronics WJFH116008A
    compatible
  drm/panel: simple: Add NewEast Optoelectronics CO., LTD WJFH116008A
    panel support

 .../bindings/display/panel/panel-simple.yaml  |  2 +
 .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
 .../dts/allwinner/sun50i-a64-pinebook.dts     | 69 ++++++++++++++++++-
 .../drm/bridge/analogix/analogix-anx6345.c    | 12 ++--
 drivers/gpu/drm/panel/panel-simple.c          | 47 +++++++++++++
 5 files changed, 123 insertions(+), 9 deletions(-)

-- 
2.25.0

[PATCH 1/6] drm/bridge: anx6345: Fix getting anx6345 regulators

[Vasily Khoruzhick]

From: Samuel Holland <samuel@sholland.org>

We don't need to pass '-supply' suffix to devm_get_regulator()

Fixes: 6aa192698089 ("drm/bridge: Add Analogix anx6345 support")
Signed-off-by: Samuel Holland <samuel@sholland.org>
Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
index 56f55c53abfd..0d8d083b0207 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
@@ -712,14 +712,14 @@ static int anx6345_i2c_probe(struct i2c_client *client,
 		DRM_DEBUG("No panel found\n");
 
 	/* 1.2V digital core power regulator  */
-	anx6345->dvdd12 = devm_regulator_get(dev, "dvdd12-supply");
+	anx6345->dvdd12 = devm_regulator_get(dev, "dvdd12");
 	if (IS_ERR(anx6345->dvdd12)) {
 		DRM_ERROR("dvdd12-supply not found\n");
 		return PTR_ERR(anx6345->dvdd12);
 	}
 
 	/* 2.5V digital core power regulator  */
-	anx6345->dvdd25 = devm_regulator_get(dev, "dvdd25-supply");
+	anx6345->dvdd25 = devm_regulator_get(dev, "dvdd25");
 	if (IS_ERR(anx6345->dvdd25)) {
 		DRM_ERROR("dvdd25-supply not found\n");
 		return PTR_ERR(anx6345->dvdd25);
-- 
2.25.0

[PATCH 2/6] drm/bridge: anx6345: Clean up error handling in probe()

[Vasily Khoruzhick]

devm_regulator_get() returns either a dummy regulator or -EPROBE_DEFER,
we don't need to print scary message in either case.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
index 0d8d083b0207..0204bbe4f0a0 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
@@ -713,17 +713,13 @@ static int anx6345_i2c_probe(struct i2c_client *client,
 
 	/* 1.2V digital core power regulator  */
 	anx6345->dvdd12 = devm_regulator_get(dev, "dvdd12");
-	if (IS_ERR(anx6345->dvdd12)) {
-		DRM_ERROR("dvdd12-supply not found\n");
+	if (IS_ERR(anx6345->dvdd12))
 		return PTR_ERR(anx6345->dvdd12);
-	}
 
 	/* 2.5V digital core power regulator  */
 	anx6345->dvdd25 = devm_regulator_get(dev, "dvdd25");
-	if (IS_ERR(anx6345->dvdd25)) {
-		DRM_ERROR("dvdd25-supply not found\n");
+	if (IS_ERR(anx6345->dvdd25))
 		return PTR_ERR(anx6345->dvdd25);
-	}
 
 	/* GPIO for chip reset */
 	anx6345->gpiod_reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
-- 
2.25.0

[PATCH 3/6] dt-bindings: Add Guangdong Neweast Optoelectronics CO. LTD vendor prefix

[Vasily Khoruzhick]

Add vendor prefix for Guangdong Neweast Optoelectronics CO. LTD

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 6456a6dfd83d..a390a793422b 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -665,6 +665,8 @@ patternProperties:
     description: Nexbox
   "^nextthing,.*":
     description: Next Thing Co.
+  "^neweast,.*":
+    description: Guangdong Neweast Optoelectronics CO., LT
   "^newhaven,.*":
     description: Newhaven Display International
   "^ni,.*":
-- 
2.25.0

[PATCH 4/6] dt-bindings: display: simple: Add NewEast Optoelectronics WJFH116008A compatible

[Vasily Khoruzhick]

This commit adds compatible for NewEast Optoelectronics WJFH116008A panel
to panel-simple binding

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 .../devicetree/bindings/display/panel/panel-simple.yaml         | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index 8fe60ee2531c..721de94cc80a 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -43,6 +43,8 @@ properties:
       - satoz,sat050at40h12r2
         # Sharp LS020B1DD01D 2.0" HQVGA TFT LCD panel
       - sharp,ls020b1dd01d
+        # NewEast Optoelectronics CO., LTD WJFH116008A eDP TFT LCD panel
+      - neweast,wjfh116008a
 
   backlight: true
   enable-gpios: true
-- 
2.25.0

[PATCH 5/6] drm/panel: simple: Add NewEast Optoelectronics CO., LTD WJFH116008A panel support

[Vasily Khoruzhick]

This commit adds support for the NewEast Optoelectronics CO., LTD
WJFH116008A 11.6" 1920x1080 TFT LCD panel.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 drivers/gpu/drm/panel/panel-simple.c | 47 ++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index e14c14ac62b5..aa04afaf3d26 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2224,6 +2224,50 @@ static const struct panel_desc netron_dy_e231732 = {
 	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
 };
 
+static const struct drm_display_mode neweast_wjfh116008a_modes[] = {
+{
+	.clock = 138500,
+	.hdisplay = 1920,
+	.hsync_start = 1920 + 48,
+	.hsync_end = 1920 + 48 + 32,
+	.htotal = 1920 + 48 + 32 + 80,
+	.vdisplay = 1080,
+	.vsync_start = 1080 + 3,
+	.vsync_end = 1080 + 3 + 5,
+	.vtotal = 1080 + 3 + 5 + 23,
+	.vrefresh = 60,
+	.flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+}, {
+	.clock = 110920,
+	.hdisplay = 1920,
+	.hsync_start = 1920 + 48,
+	.hsync_end = 1920 + 48 + 32,
+	.htotal = 1920 + 48 + 32 + 80,
+	.vdisplay = 1080,
+	.vsync_start = 1080 + 3,
+	.vsync_end = 1080 + 3 + 5,
+	.vtotal = 1080 + 3 + 5 + 23,
+	.vrefresh = 48,
+	.flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+} };
+
+static const struct panel_desc neweast_wjfh116008a = {
+	.modes = neweast_wjfh116008a_modes,
+	.num_modes = 2,
+	.bpc = 6,
+	.size = {
+		.width = 260,
+		.height = 150,
+	},
+	.delay = {
+		.prepare = 110,
+		.enable = 20,
+		.unprepare = 500,
+	},
+	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
+	.connector_type = DRM_MODE_CONNECTOR_eDP,
+};
+
 static const struct drm_display_mode newhaven_nhd_43_480272ef_atxl_mode = {
 	.clock = 9000,
 	.hdisplay = 480,
@@ -3399,6 +3443,9 @@ static const struct of_device_id platform_of_match[] = {
 	}, {
 		.compatible = "netron-dy,e231732",
 		.data = &netron_dy_e231732,
+	}, {
+		.compatible = "neweast,wjfh116008a",
+		.data = &neweast_wjfh116008a,
 	}, {
 		.compatible = "newhaven,nhd-4.3-480272ef-atxl",
 		.data = &newhaven_nhd_43_480272ef_atxl,
-- 
2.25.0

[PATCH 6/6] arm64: allwinner: a64: enable LCD-related hardware for Pinebook

[Vasily Khoruzhick]

From: Icenowy Zheng <icenowy@aosc.io>

Pinebook has an ANX6345 bridge connected to the RGB666 LCD output and
eDP panel input. The bridge is controlled via I2C that's connected to
R_I2C bus.

Enable all this hardware in device tree.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 .../dts/allwinner/sun50i-a64-pinebook.dts     | 69 ++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
index c06c540e6c08..f5633f550d8a 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
@@ -48,6 +48,18 @@ lid_switch {
 		};
 	};
 
+	panel_edp: panel-edp {
+		compatible = "neweast,wjfh116008a";
+		backlight = <&backlight>;
+		power-supply = <&reg_dc1sw>;
+
+		port {
+			panel_edp_in: endpoint {
+				remote-endpoint = <&anx6345_out_edp>;
+			};
+		};
+	};
+
 	reg_vbklt: vbklt {
 		compatible = "regulator-fixed";
 		regulator-name = "vbklt";
@@ -109,6 +121,10 @@ &dai {
 	status = "okay";
 };
 
+&de {
+	status = "okay";
+};
+
 &ehci0 {
 	phys = <&usbphy 0>;
 	phy-names = "usb";
@@ -119,6 +135,10 @@ &ehci1 {
 	status = "okay";
 };
 
+&mixer0 {
+	status = "okay";
+};
+
 &mmc0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc0_pins>;
@@ -177,12 +197,45 @@ &pwm {
 	status = "okay";
 };
 
-/* The ANX6345 eDP-bridge is on r_i2c */
 &r_i2c {
 	clock-frequency = <100000>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&r_i2c_pl89_pins>;
 	status = "okay";
+
+	anx6345: anx6345@38 {
+		compatible = "analogix,anx6345";
+		reg = <0x38>;
+		reset-gpios = <&pio 3 24 GPIO_ACTIVE_LOW>; /* PD24 */
+		dvdd25-supply = <&reg_dldo2>;
+		dvdd12-supply = <&reg_fldo1>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			anx6345_in: port@0 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0>;
+				anx6345_in_tcon0: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&tcon0_out_anx6345>;
+				};
+			};
+
+			anx6345_out: port@1 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <1>;
+
+				anx6345_out_edp: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&panel_edp_in>;
+				};
+			};
+		};
+	};
 };
 
 &r_pio {
@@ -357,6 +410,20 @@ &sound {
 			"MIC2", "Internal Microphone Right";
 };
 
+&tcon0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&lcd_rgb666_pins>;
+
+	status = "okay";
+};
+
+&tcon0_out {
+	tcon0_out_anx6345: endpoint@0 {
+		reg = <0>;
+		remote-endpoint = <&anx6345_in_tcon0>;
+	};
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pb_pins>;
-- 
2.25.0

Re: [PATCH 3/6] dt-bindings: Add Guangdong Neweast Optoelectronics CO. LTD vendor prefix

[Sam Ravnborg]

Hi Vasily

On Thu, Feb 20, 2020 at 12:35:05AM -0800, Vasily Khoruzhick wrote:
> Add vendor prefix for Guangdong Neweast Optoelectronics CO. LTD
> 
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 6456a6dfd83d..a390a793422b 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -665,6 +665,8 @@ patternProperties:
>      description: Nexbox
>    "^nextthing,.*":
>      description: Next Thing Co.
> +  "^neweast,.*":
> +    description: Guangdong Neweast Optoelectronics CO., LT

Alphabetical order.
"new" comes before "nex".

	Sam

Re: [PATCH 1/6] drm/bridge: anx6345: Fix getting anx6345 regulators

[Laurent Pinchart]

Hi Vasily,

Thank you for the patch.

On Thu, Feb 20, 2020 at 12:35:03AM -0800, Vasily Khoruzhick wrote:
> From: Samuel Holland <samuel@sholland.org>
> 
> We don't need to pass '-supply' suffix to devm_get_regulator()
> 
> Fixes: 6aa192698089 ("drm/bridge: Add Analogix anx6345 support")
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>

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

> ---
>  drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> index 56f55c53abfd..0d8d083b0207 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> @@ -712,14 +712,14 @@ static int anx6345_i2c_probe(struct i2c_client *client,
>  		DRM_DEBUG("No panel found\n");
>  
>  	/* 1.2V digital core power regulator  */
> -	anx6345->dvdd12 = devm_regulator_get(dev, "dvdd12-supply");
> +	anx6345->dvdd12 = devm_regulator_get(dev, "dvdd12");
>  	if (IS_ERR(anx6345->dvdd12)) {
>  		DRM_ERROR("dvdd12-supply not found\n");
>  		return PTR_ERR(anx6345->dvdd12);
>  	}
>  
>  	/* 2.5V digital core power regulator  */
> -	anx6345->dvdd25 = devm_regulator_get(dev, "dvdd25-supply");
> +	anx6345->dvdd25 = devm_regulator_get(dev, "dvdd25");
>  	if (IS_ERR(anx6345->dvdd25)) {
>  		DRM_ERROR("dvdd25-supply not found\n");
>  		return PTR_ERR(anx6345->dvdd25);

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/6] drm/bridge: anx6345: Clean up error handling in probe()

[Laurent Pinchart]

Hi Vasily,

Thank you for the patch.

On Thu, Feb 20, 2020 at 12:35:04AM -0800, Vasily Khoruzhick wrote:
> devm_regulator_get() returns either a dummy regulator or -EPROBE_DEFER,
> we don't need to print scary message in either case.
> 
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> index 0d8d083b0207..0204bbe4f0a0 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> @@ -713,17 +713,13 @@ static int anx6345_i2c_probe(struct i2c_client *client,
>  
>  	/* 1.2V digital core power regulator  */
>  	anx6345->dvdd12 = devm_regulator_get(dev, "dvdd12");
> -	if (IS_ERR(anx6345->dvdd12)) {
> -		DRM_ERROR("dvdd12-supply not found\n");
> +	if (IS_ERR(anx6345->dvdd12))
>  		return PTR_ERR(anx6345->dvdd12);
> -	}

There could be other errors such as -EBUSY or -EPERM. The following
would ensure a message gets printed in those cases, while avoiding
spamming the kernel log in the EPROBE_DEFER case.

	if (IS_ERR(anx6345->dvdd12)) {
		if (PTR_ERR(anx6345->dvdd12) != -EPROBE_DEFER)
			DRM_ERROR("Failed to get dvdd12 supply (%d)\n",
				  PTR_ERR(anx6345->dvdd12));
		return PTR_ERR(anx6345->dvdd12);
	}

But maybe it's overkill ? With or without that change (for the second
regulator below too),

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

>  	/* 2.5V digital core power regulator  */
>  	anx6345->dvdd25 = devm_regulator_get(dev, "dvdd25");
> -	if (IS_ERR(anx6345->dvdd25)) {
> -		DRM_ERROR("dvdd25-supply not found\n");
> +	if (IS_ERR(anx6345->dvdd25))
>  		return PTR_ERR(anx6345->dvdd25);
> -	}
>  
>  	/* GPIO for chip reset */
>  	anx6345->gpiod_reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/6] dt-bindings: display: simple: Add NewEast Optoelectronics WJFH116008A compatible

[Laurent Pinchart]

On Thu, Feb 20, 2020 at 12:35:06AM -0800, Vasily Khoruzhick wrote:
> This commit adds compatible for NewEast Optoelectronics WJFH116008A panel
> to panel-simple binding
> 
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  .../devicetree/bindings/display/panel/panel-simple.yaml         | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
> index 8fe60ee2531c..721de94cc80a 100644
> --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
> @@ -43,6 +43,8 @@ properties:
>        - satoz,sat050at40h12r2
>          # Sharp LS020B1DD01D 2.0" HQVGA TFT LCD panel
>        - sharp,ls020b1dd01d
> +        # NewEast Optoelectronics CO., LTD WJFH116008A eDP TFT LCD panel
> +      - neweast,wjfh116008a

Please keep the entries alphabetically sorted. With this fixed,

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

>  
>    backlight: true
>    enable-gpios: true

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/6] dt-bindings: Add Guangdong Neweast Optoelectronics CO. LTD vendor prefix

[Laurent Pinchart]

Hi Vasily,

Thank you for the patch.

On Thu, Feb 20, 2020 at 12:35:05AM -0800, Vasily Khoruzhick wrote:
> Add vendor prefix for Guangdong Neweast Optoelectronics CO. LTD
> 
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 6456a6dfd83d..a390a793422b 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -665,6 +665,8 @@ patternProperties:
>      description: Nexbox
>    "^nextthing,.*":
>      description: Next Thing Co.
> +  "^neweast,.*":
> +    description: Guangdong Neweast Optoelectronics CO., LT

Google only returns two hits for this name, beside the ones related to
this patch series. Are you sure this is the correct company name ?

>    "^newhaven,.*":
>      description: Newhaven Display International
>    "^ni,.*":

-- 
Regards,

Laurent Pinchart

Re: [PATCH 5/6] drm/panel: simple: Add NewEast Optoelectronics CO., LTD WJFH116008A panel support

[Laurent Pinchart]

Hi Vasily,

Thank you for the patch.

On Thu, Feb 20, 2020 at 12:35:07AM -0800, Vasily Khoruzhick wrote:
> This commit adds support for the NewEast Optoelectronics CO., LTD
> WJFH116008A 11.6" 1920x1080 TFT LCD panel.
> 
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  drivers/gpu/drm/panel/panel-simple.c | 47 ++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index e14c14ac62b5..aa04afaf3d26 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -2224,6 +2224,50 @@ static const struct panel_desc netron_dy_e231732 = {
>  	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
>  };
>  
> +static const struct drm_display_mode neweast_wjfh116008a_modes[] = {
> +{
> +	.clock = 138500,
> +	.hdisplay = 1920,
> +	.hsync_start = 1920 + 48,
> +	.hsync_end = 1920 + 48 + 32,
> +	.htotal = 1920 + 48 + 32 + 80,
> +	.vdisplay = 1080,
> +	.vsync_start = 1080 + 3,
> +	.vsync_end = 1080 + 3 + 5,
> +	.vtotal = 1080 + 3 + 5 + 23,
> +	.vrefresh = 60,
> +	.flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
> +}, {
> +	.clock = 110920,
> +	.hdisplay = 1920,
> +	.hsync_start = 1920 + 48,
> +	.hsync_end = 1920 + 48 + 32,
> +	.htotal = 1920 + 48 + 32 + 80,
> +	.vdisplay = 1080,
> +	.vsync_start = 1080 + 3,
> +	.vsync_end = 1080 + 3 + 5,
> +	.vtotal = 1080 + 3 + 5 + 23,
> +	.vrefresh = 48,
> +	.flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
> +} };

This should be indented one step to the right, see boe_nv101wxmn51_modes
for instance.

The only different between the two modes is the clock, leading to
different refresh rates. Are only those two clock frequencies supported,
or does the panel support anything in-between as well ? In the latter
case, would it make sense to use display_timing instead of
drm_display_mode ? See dlc_dlc0700yzg_1_timing for an example.

> +
> +static const struct panel_desc neweast_wjfh116008a = {
> +	.modes = neweast_wjfh116008a_modes,
> +	.num_modes = 2,
> +	.bpc = 6,
> +	.size = {
> +		.width = 260,
> +		.height = 150,
> +	},
> +	.delay = {
> +		.prepare = 110,
> +		.enable = 20,
> +		.unprepare = 500,
> +	},
> +	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> +	.connector_type = DRM_MODE_CONNECTOR_eDP,
> +};
> +
>  static const struct drm_display_mode newhaven_nhd_43_480272ef_atxl_mode = {
>  	.clock = 9000,
>  	.hdisplay = 480,
> @@ -3399,6 +3443,9 @@ static const struct of_device_id platform_of_match[] = {
>  	}, {
>  		.compatible = "netron-dy,e231732",
>  		.data = &netron_dy_e231732,
> +	}, {
> +		.compatible = "neweast,wjfh116008a",
> +		.data = &neweast_wjfh116008a,
>  	}, {
>  		.compatible = "newhaven,nhd-4.3-480272ef-atxl",
>  		.data = &newhaven_nhd_43_480272ef_atxl,

-- 
Regards,

Laurent Pinchart

Re: [PATCH 6/6] arm64: allwinner: a64: enable LCD-related hardware for Pinebook

[Laurent Pinchart]

Hi Vasily,

Thank you for the patch.

On Thu, Feb 20, 2020 at 12:35:08AM -0800, Vasily Khoruzhick wrote:
> From: Icenowy Zheng <icenowy@aosc.io>
> 
> Pinebook has an ANX6345 bridge connected to the RGB666 LCD output and
> eDP panel input. The bridge is controlled via I2C that's connected to
> R_I2C bus.
> 
> Enable all this hardware in device tree.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  .../dts/allwinner/sun50i-a64-pinebook.dts     | 69 ++++++++++++++++++-
>  1 file changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> index c06c540e6c08..f5633f550d8a 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> @@ -48,6 +48,18 @@ lid_switch {
>  		};
>  	};
>  
> +	panel_edp: panel-edp {
> +		compatible = "neweast,wjfh116008a";
> +		backlight = <&backlight>;
> +		power-supply = <&reg_dc1sw>;
> +
> +		port {
> +			panel_edp_in: endpoint {
> +				remote-endpoint = <&anx6345_out_edp>;
> +			};
> +		};
> +	};
> +
>  	reg_vbklt: vbklt {
>  		compatible = "regulator-fixed";
>  		regulator-name = "vbklt";
> @@ -109,6 +121,10 @@ &dai {
>  	status = "okay";
>  };
>  
> +&de {
> +	status = "okay";
> +};
> +
>  &ehci0 {
>  	phys = <&usbphy 0>;
>  	phy-names = "usb";
> @@ -119,6 +135,10 @@ &ehci1 {
>  	status = "okay";
>  };
>  
> +&mixer0 {
> +	status = "okay";
> +};
> +
>  &mmc0 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&mmc0_pins>;
> @@ -177,12 +197,45 @@ &pwm {
>  	status = "okay";
>  };
>  
> -/* The ANX6345 eDP-bridge is on r_i2c */
>  &r_i2c {
>  	clock-frequency = <100000>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&r_i2c_pl89_pins>;
>  	status = "okay";
> +
> +	anx6345: anx6345@38 {
> +		compatible = "analogix,anx6345";
> +		reg = <0x38>;
> +		reset-gpios = <&pio 3 24 GPIO_ACTIVE_LOW>; /* PD24 */
> +		dvdd25-supply = <&reg_dldo2>;
> +		dvdd12-supply = <&reg_fldo1>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			anx6345_in: port@0 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				reg = <0>;
> +				anx6345_in_tcon0: endpoint@0 {
> +					reg = <0>;
> +					remote-endpoint = <&tcon0_out_anx6345>;
> +				};

As there's a single endpoint, you can drop the reg property, the @0
suffix, and the #address-cells and #size-cells property in the port@0
node (but not in the ports node).

> +			};
> +
> +			anx6345_out: port@1 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				reg = <1>;
> +
> +				anx6345_out_edp: endpoint@0 {
> +					reg = <0>;
> +					remote-endpoint = <&panel_edp_in>;
> +				};

Same here.

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

> +			};
> +		};
> +	};
>  };
>  
>  &r_pio {
> @@ -357,6 +410,20 @@ &sound {
>  			"MIC2", "Internal Microphone Right";
>  };
>  
> +&tcon0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&lcd_rgb666_pins>;
> +
> +	status = "okay";
> +};
> +
> +&tcon0_out {
> +	tcon0_out_anx6345: endpoint@0 {
> +		reg = <0>;
> +		remote-endpoint = <&anx6345_in_tcon0>;
> +	};
> +};
> +
>  &uart0 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&uart0_pb_pins>;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/6] dt-bindings: Add Guangdong Neweast Optoelectronics CO. LTD vendor prefix

[Sam Ravnborg]

Hi Laurent.

> > +  "^neweast,.*":
> > +    description: Guangdong Neweast Optoelectronics CO., LT
> 
> Google only returns two hits for this name, beside the ones related to
> this patch series. Are you sure this is the correct company name ?

Seems legit:
http://www.eastbl.com/

But maybe their chinese name was better a basis for vendor prefix?

Guangdong New Oriental Optoelectronics

	Sam

Re: [PATCH 3/6] dt-bindings: Add Guangdong Neweast Optoelectronics CO. LTD vendor prefix

[Vasily Khoruzhick]

On Thu, Feb 20, 2020 at 1:21 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Laurent.
>
> > > +  "^neweast,.*":
> > > +    description: Guangdong Neweast Optoelectronics CO., LT
> >
> > Google only returns two hits for this name, beside the ones related to
> > this patch series. Are you sure this is the correct company name ?
>
> Seems legit:
> http://www.eastbl.com/
>
> But maybe their chinese name was better a basis for vendor prefix?
>
> Guangdong New Oriental Optoelectronics

They call themselves "Guangdong NewEast Optoelectronics" in English,
so I think it's better to keep it as is.

>
>         Sam

Re: [PATCH 2/6] drm/bridge: anx6345: Clean up error handling in probe()

[Vasily Khoruzhick]

On Thu, Feb 20, 2020 at 5:53 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Vasily,

Hi Laurent,

> Thank you for the patch.
>
> On Thu, Feb 20, 2020 at 12:35:04AM -0800, Vasily Khoruzhick wrote:
> > devm_regulator_get() returns either a dummy regulator or -EPROBE_DEFER,
> > we don't need to print scary message in either case.
> >
> > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> > ---
> >  drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> > index 0d8d083b0207..0204bbe4f0a0 100644
> > --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> > +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> > @@ -713,17 +713,13 @@ static int anx6345_i2c_probe(struct i2c_client *client,
> >
> >       /* 1.2V digital core power regulator  */
> >       anx6345->dvdd12 = devm_regulator_get(dev, "dvdd12");
> > -     if (IS_ERR(anx6345->dvdd12)) {
> > -             DRM_ERROR("dvdd12-supply not found\n");
> > +     if (IS_ERR(anx6345->dvdd12))
> >               return PTR_ERR(anx6345->dvdd12);
> > -     }
>
> There could be other errors such as -EBUSY or -EPERM. The following
> would ensure a message gets printed in those cases, while avoiding
> spamming the kernel log in the EPROBE_DEFER case.
>
>         if (IS_ERR(anx6345->dvdd12)) {
>                 if (PTR_ERR(anx6345->dvdd12) != -EPROBE_DEFER)
>                         DRM_ERROR("Failed to get dvdd12 supply (%d)\n",
>                                   PTR_ERR(anx6345->dvdd12));
>                 return PTR_ERR(anx6345->dvdd12);
>         }
>
> But maybe it's overkill ? With or without that change (for the second
> regulator below too),

Thanks, I'll do as you suggested.



> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> >       /* 2.5V digital core power regulator  */
> >       anx6345->dvdd25 = devm_regulator_get(dev, "dvdd25");
> > -     if (IS_ERR(anx6345->dvdd25)) {
> > -             DRM_ERROR("dvdd25-supply not found\n");
> > +     if (IS_ERR(anx6345->dvdd25))
> >               return PTR_ERR(anx6345->dvdd25);
> > -     }
> >
> >       /* GPIO for chip reset */
> >       anx6345->gpiod_reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH 3/6] dt-bindings: Add Guangdong Neweast Optoelectronics CO. LTD vendor prefix

[Vasily Khoruzhick]

On Thu, Feb 20, 2020 at 1:35 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Vasily
>
> On Thu, Feb 20, 2020 at 12:35:05AM -0800, Vasily Khoruzhick wrote:
> > Add vendor prefix for Guangdong Neweast Optoelectronics CO. LTD
> >
> > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > index 6456a6dfd83d..a390a793422b 100644
> > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > @@ -665,6 +665,8 @@ patternProperties:
> >      description: Nexbox
> >    "^nextthing,.*":
> >      description: Next Thing Co.
> > +  "^neweast,.*":
> > +    description: Guangdong Neweast Optoelectronics CO., LT
>
> Alphabetical order.
> "new" comes before "nex".

Will fix in v2

>
>         Sam

Re: [PATCH 3/6] dt-bindings: Add Guangdong Neweast Optoelectronics CO. LTD vendor prefix

[Vasily Khoruzhick]

On Thu, Feb 20, 2020 at 5:56 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Vasily,

Hi Laurent,

> Thank you for the patch.
>
> On Thu, Feb 20, 2020 at 12:35:05AM -0800, Vasily Khoruzhick wrote:
> > Add vendor prefix for Guangdong Neweast Optoelectronics CO. LTD
> >
> > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > index 6456a6dfd83d..a390a793422b 100644
> > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > @@ -665,6 +665,8 @@ patternProperties:
> >      description: Nexbox
> >    "^nextthing,.*":
> >      description: Next Thing Co.
> > +  "^neweast,.*":
> > +    description: Guangdong Neweast Optoelectronics CO., LT
>
> Google only returns two hits for this name, beside the ones related to
> this patch series. Are you sure this is the correct company name ?

That is what datasheet says:

http://files.pine64.org/doc/datasheet/pinebook/11.6inches-1080P-IPS-LCD-Panel-spec-WJFH116008A.pdf



> >    "^newhaven,.*":
> >      description: Newhaven Display International
> >    "^ni,.*":
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH 5/6] drm/panel: simple: Add NewEast Optoelectronics CO., LTD WJFH116008A panel support

[Vasily Khoruzhick]

On Thu, Feb 20, 2020 at 5:59 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Vasily,

Hi Laurent,

>
> Thank you for the patch.
>
> On Thu, Feb 20, 2020 at 12:35:07AM -0800, Vasily Khoruzhick wrote:
> > This commit adds support for the NewEast Optoelectronics CO., LTD
> > WJFH116008A 11.6" 1920x1080 TFT LCD panel.
> >
> > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> > ---
> >  drivers/gpu/drm/panel/panel-simple.c | 47 ++++++++++++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > index e14c14ac62b5..aa04afaf3d26 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -2224,6 +2224,50 @@ static const struct panel_desc netron_dy_e231732 = {
> >       .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> >  };
> >
> > +static const struct drm_display_mode neweast_wjfh116008a_modes[] = {
> > +{
> > +     .clock = 138500,
> > +     .hdisplay = 1920,
> > +     .hsync_start = 1920 + 48,
> > +     .hsync_end = 1920 + 48 + 32,
> > +     .htotal = 1920 + 48 + 32 + 80,
> > +     .vdisplay = 1080,
> > +     .vsync_start = 1080 + 3,
> > +     .vsync_end = 1080 + 3 + 5,
> > +     .vtotal = 1080 + 3 + 5 + 23,
> > +     .vrefresh = 60,
> > +     .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
> > +}, {
> > +     .clock = 110920,
> > +     .hdisplay = 1920,
> > +     .hsync_start = 1920 + 48,
> > +     .hsync_end = 1920 + 48 + 32,
> > +     .htotal = 1920 + 48 + 32 + 80,
> > +     .vdisplay = 1080,
> > +     .vsync_start = 1080 + 3,
> > +     .vsync_end = 1080 + 3 + 5,
> > +     .vtotal = 1080 + 3 + 5 + 23,
> > +     .vrefresh = 48,
> > +     .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
> > +} };
>
> This should be indented one step to the right, see boe_nv101wxmn51_modes
> for instance.

Will do.

> The only different between the two modes is the clock, leading to
> different refresh rates. Are only those two clock frequencies supported,
> or does the panel support anything in-between as well ? In the latter
> case, would it make sense to use display_timing instead of
> drm_display_mode ? See dlc_dlc0700yzg_1_timing for an example.

These are coming from EDID. The datasheet [1] says typical frequency
is 138.5MHz and min/max are not specified, so I'm not sure whether it
supports anything in between. I did check that both modes work though.

[1] http://files.pine64.org/doc/datasheet/pinebook/11.6inches-1080P-IPS-LCD-Panel-spec-WJFH116008A.pdf



> > +
> > +static const struct panel_desc neweast_wjfh116008a = {
> > +     .modes = neweast_wjfh116008a_modes,
> > +     .num_modes = 2,
> > +     .bpc = 6,
> > +     .size = {
> > +             .width = 260,
> > +             .height = 150,
> > +     },
> > +     .delay = {
> > +             .prepare = 110,
> > +             .enable = 20,
> > +             .unprepare = 500,
> > +     },
> > +     .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> > +     .connector_type = DRM_MODE_CONNECTOR_eDP,
> > +};
> > +
> >  static const struct drm_display_mode newhaven_nhd_43_480272ef_atxl_mode = {
> >       .clock = 9000,
> >       .hdisplay = 480,
> > @@ -3399,6 +3443,9 @@ static const struct of_device_id platform_of_match[] = {
> >       }, {
> >               .compatible = "netron-dy,e231732",
> >               .data = &netron_dy_e231732,
> > +     }, {
> > +             .compatible = "neweast,wjfh116008a",
> > +             .data = &neweast_wjfh116008a,
> >       }, {
> >               .compatible = "newhaven,nhd-4.3-480272ef-atxl",
> >               .data = &newhaven_nhd_43_480272ef_atxl,
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH 6/6] arm64: allwinner: a64: enable LCD-related hardware for Pinebook

[Vasily Khoruzhick]

On Thu, Feb 20, 2020 at 6:17 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Vasily,

Hi Laurent,

> Thank you for the patch.
>
> On Thu, Feb 20, 2020 at 12:35:08AM -0800, Vasily Khoruzhick wrote:
> > From: Icenowy Zheng <icenowy@aosc.io>
> >
> > Pinebook has an ANX6345 bridge connected to the RGB666 LCD output and
> > eDP panel input. The bridge is controlled via I2C that's connected to
> > R_I2C bus.
> >
> > Enable all this hardware in device tree.
> >
> > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> > ---
> >  .../dts/allwinner/sun50i-a64-pinebook.dts     | 69 ++++++++++++++++++-
> >  1 file changed, 68 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> > index c06c540e6c08..f5633f550d8a 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> > @@ -48,6 +48,18 @@ lid_switch {
> >               };
> >       };
> >
> > +     panel_edp: panel-edp {
> > +             compatible = "neweast,wjfh116008a";
> > +             backlight = <&backlight>;
> > +             power-supply = <&reg_dc1sw>;
> > +
> > +             port {
> > +                     panel_edp_in: endpoint {
> > +                             remote-endpoint = <&anx6345_out_edp>;
> > +                     };
> > +             };
> > +     };
> > +
> >       reg_vbklt: vbklt {
> >               compatible = "regulator-fixed";
> >               regulator-name = "vbklt";
> > @@ -109,6 +121,10 @@ &dai {
> >       status = "okay";
> >  };
> >
> > +&de {
> > +     status = "okay";
> > +};
> > +
> >  &ehci0 {
> >       phys = <&usbphy 0>;
> >       phy-names = "usb";
> > @@ -119,6 +135,10 @@ &ehci1 {
> >       status = "okay";
> >  };
> >
> > +&mixer0 {
> > +     status = "okay";
> > +};
> > +
> >  &mmc0 {
> >       pinctrl-names = "default";
> >       pinctrl-0 = <&mmc0_pins>;
> > @@ -177,12 +197,45 @@ &pwm {
> >       status = "okay";
> >  };
> >
> > -/* The ANX6345 eDP-bridge is on r_i2c */
> >  &r_i2c {
> >       clock-frequency = <100000>;
> >       pinctrl-names = "default";
> >       pinctrl-0 = <&r_i2c_pl89_pins>;
> >       status = "okay";
> > +
> > +     anx6345: anx6345@38 {
> > +             compatible = "analogix,anx6345";
> > +             reg = <0x38>;
> > +             reset-gpios = <&pio 3 24 GPIO_ACTIVE_LOW>; /* PD24 */
> > +             dvdd25-supply = <&reg_dldo2>;
> > +             dvdd12-supply = <&reg_fldo1>;
> > +
> > +             ports {
> > +                     #address-cells = <1>;
> > +                     #size-cells = <0>;
> > +
> > +                     anx6345_in: port@0 {
> > +                             #address-cells = <1>;
> > +                             #size-cells = <0>;
> > +                             reg = <0>;
> > +                             anx6345_in_tcon0: endpoint@0 {
> > +                                     reg = <0>;
> > +                                     remote-endpoint = <&tcon0_out_anx6345>;
> > +                             };
>
> As there's a single endpoint, you can drop the reg property, the @0
> suffix, and the #address-cells and #size-cells property in the port@0
> node (but not in the ports node).

Will do

> > +                     };
> > +
> > +                     anx6345_out: port@1 {
> > +                             #address-cells = <1>;
> > +                             #size-cells = <0>;
> > +                             reg = <1>;
> > +
> > +                             anx6345_out_edp: endpoint@0 {
> > +                                     reg = <0>;
> > +                                     remote-endpoint = <&panel_edp_in>;
> > +                             };
>
> Same here.

Will do

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

Thanks for reviewing the series!


>
> > +                     };
> > +             };
> > +     };
> >  };
> >
> >  &r_pio {
> > @@ -357,6 +410,20 @@ &sound {
> >                       "MIC2", "Internal Microphone Right";
> >  };
> >
> > +&tcon0 {
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&lcd_rgb666_pins>;
> > +
> > +     status = "okay";
> > +};
> > +
> > +&tcon0_out {
> > +     tcon0_out_anx6345: endpoint@0 {
> > +             reg = <0>;
> > +             remote-endpoint = <&anx6345_in_tcon0>;
> > +     };
> > +};
> > +
> >  &uart0 {
> >       pinctrl-names = "default";
> >       pinctrl-0 = <&uart0_pb_pins>;
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH 1/6] drm/bridge: anx6345: Fix getting anx6345 regulators

[Neil Armstrong]

On 20/02/2020 09:35, Vasily Khoruzhick wrote:
> From: Samuel Holland <samuel@sholland.org>
> 
> We don't need to pass '-supply' suffix to devm_get_regulator()
> 
> Fixes: 6aa192698089 ("drm/bridge: Add Analogix anx6345 support")
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> index 56f55c53abfd..0d8d083b0207 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> @@ -712,14 +712,14 @@ static int anx6345_i2c_probe(struct i2c_client *client,
>  		DRM_DEBUG("No panel found\n");
>  
>  	/* 1.2V digital core power regulator  */
> -	anx6345->dvdd12 = devm_regulator_get(dev, "dvdd12-supply");
> +	anx6345->dvdd12 = devm_regulator_get(dev, "dvdd12");
>  	if (IS_ERR(anx6345->dvdd12)) {
>  		DRM_ERROR("dvdd12-supply not found\n");
>  		return PTR_ERR(anx6345->dvdd12);
>  	}
>  
>  	/* 2.5V digital core power regulator  */
> -	anx6345->dvdd25 = devm_regulator_get(dev, "dvdd25-supply");
> +	anx6345->dvdd25 = devm_regulator_get(dev, "dvdd25");
>  	if (IS_ERR(anx6345->dvdd25)) {
>  		DRM_ERROR("dvdd25-supply not found\n");
>  		return PTR_ERR(anx6345->dvdd25);
> 

This is a duplicate of "drm/bridge: analogix-anx6345: Avoid duplicate -supply suffix" (20200218155440.BEFB968C65@verein.lst.de)

But this one has fixes and review from laurent, so I'll push this one when the serie is ready

Neil

Re: [PATCH 1/6] drm/bridge: anx6345: Fix getting anx6345 regulators

[Torsten Duwe]

On Fri, Feb 21, 2020 at 09:32:01AM +0100, Neil Armstrong wrote:
> On 20/02/2020 09:35, Vasily Khoruzhick wrote:
> > From: Samuel Holland <samuel@sholland.org>
> > 
> > We don't need to pass '-supply' suffix to devm_get_regulator()
> > 
> > Fixes: 6aa192698089 ("drm/bridge: Add Analogix anx6345 support")
> > Signed-off-by: Samuel Holland <samuel@sholland.org>
> > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> > ---
> >  drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> > index 56f55c53abfd..0d8d083b0207 100644
> > --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> > +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> > @@ -712,14 +712,14 @@ static int anx6345_i2c_probe(struct i2c_client *client,
> >  		DRM_DEBUG("No panel found\n");
> >  
> >  	/* 1.2V digital core power regulator  */
> > -	anx6345->dvdd12 = devm_regulator_get(dev, "dvdd12-supply");
> > +	anx6345->dvdd12 = devm_regulator_get(dev, "dvdd12");
> >  	if (IS_ERR(anx6345->dvdd12)) {
> >  		DRM_ERROR("dvdd12-supply not found\n");
> >  		return PTR_ERR(anx6345->dvdd12);
> >  	}
> >  
> >  	/* 2.5V digital core power regulator  */
> > -	anx6345->dvdd25 = devm_regulator_get(dev, "dvdd25-supply");
> > +	anx6345->dvdd25 = devm_regulator_get(dev, "dvdd25");
> >  	if (IS_ERR(anx6345->dvdd25)) {
> >  		DRM_ERROR("dvdd25-supply not found\n");
> >  		return PTR_ERR(anx6345->dvdd25);
> > 
> 
> This is a duplicate of "drm/bridge: analogix-anx6345: Avoid duplicate -supply suffix" (20200218155440.BEFB968C65@verein.lst.de)
> 
> But this one has fixes and review from laurent, so I'll push this one when the serie is ready

I really don't mind, as long as it gets fixed.
The change is pretty obvious when you look at commit 69511a452e6dc.

Signed-off-by: Torsten Duwe <duwe@suse.de>
Reviewed-by: Mark Brown <broonie@kernel.org>
(broonie had replied to my submission back in November)

There's one other fix required for the anx6345 and, while at it,
I had also fixed the code I copied in that hurry as well. 
What about these? All 3 fixes can go in independently, so I wouldn't
tie them to this series.

	Torsten