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(-)
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);
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);
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
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
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);
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);
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
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,.*":
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
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
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,.*":
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
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
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-Pane...
"^newhaven,.*": description: Newhaven Display International "^ni,.*":
-- Regards,
Laurent Pinchart
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
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
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,
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",
}, { .compatible = "newhaven,nhd-4.3-480272ef-atxl", .data = &newhaven_nhd_43_480272ef_atxl,.data = &neweast_wjfh116008a,
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-Pane...
+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
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 = <®_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 = <®_dldo2>; + dvdd12-supply = <®_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>;
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 = <®_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 = <®_dldo2>;
dvdd12-supply = <®_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>;
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 = <®_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 = <®_dldo2>;
dvdd12-supply = <®_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
dri-devel@lists.freedesktop.org