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
This commit adds support for the NewEast Optoelectronics CO., LTD
WJFH116008A 11.6" 1920x1080 TFT LCD panel.
Signed-off-by: Vasily Khoruzhick <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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 <[email protected]>
> /* 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
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 <[email protected]>
> ---
> 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
On Thu, Feb 20, 2020 at 5:53 AM Laurent Pinchart
<[email protected]> 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 <[email protected]>
> > ---
> > 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 <[email protected]>
>
> > /* 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
On Thu, Feb 20, 2020 at 5:59 AM Laurent Pinchart
<[email protected]> 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 <[email protected]>
> > ---
> > 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