[PATCH 0/2] Add Sharp panel option for Lenovo Miix 630

[PATCH 0/2] Add Sharp panel option for Lenovo Miix 630

[Jeffrey Hugo]

The Lenovo Miix 630 laptop can be found with one of two panels - a BOE
or Sharp option.  This likely provides options during manufacturing.

These panels connect via eDP, however they sit behind a DSI to eDP
bridge on the laptop, so they can easily be handled by the existing
simple panel code.

This series adds support for the Sharp option.

Jeffrey Hugo (2):
  dt-bindings: panel: Add Sharp LD-D5116Z01B
  drm/panel: simple: Add support for Sharp LD-D5116Z01B panel

 .../display/panel/sharp,ld-d5116z01b.txt      | 27 +++++++++++++++++++
 drivers/gpu/drm/panel/panel-simple.c          | 26 ++++++++++++++++++
 2 files changed, 53 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/sharp,ld-d5116z01b.txt

-- 
2.17.1

[PATCH 1/2] dt-bindings: panel: Add Sharp LD-D5116Z01B

[Jeffrey Hugo]

The Sharp LD-D5116Z01B is a 12.3" eDP panel with a 1920X1280 resolution.

Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
---
 .../display/panel/sharp,ld-d5116z01b.txt      | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/sharp,ld-d5116z01b.txt

diff --git a/Documentation/devicetree/bindings/display/panel/sharp,ld-d5116z01b.txt b/Documentation/devicetree/bindings/display/panel/sharp,ld-d5116z01b.txt
new file mode 100644
index 000000000000..3938c2847fe5
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/sharp,ld-d5116z01b.txt
@@ -0,0 +1,27 @@
+Sharp LD-D5116Z01B 12.3" WUXGA+ eDP panel
+
+Required properties:
+- compatible: should be "sharp,ld-d5116z01b"
+- power-supply: regulator to provide the VCC supply voltage (3.3 volts)
+
+This binding is compatible with the simple-panel binding.
+
+The device node can contain one 'port' child node with one child
+'endpoint' node, according to the bindings defined in [1]. This
+node should describe panel's video bus.
+
+[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
+
+Example:
+
+	panel: panel {
+		compatible = "sharp,ld-d5116z01b";
+		power-supply = <&vlcd_3v3>;
+		no-hpd
+
+		port {
+			panel_ep: endpoint {
+				remote-endpoint = <&bridge_out_ep>;
+			};
+		};
+	};
-- 
2.17.1

Re: [PATCH 1/2] dt-bindings: panel: Add Sharp LD-D5116Z01B

[Sam Ravnborg]

Hi Jeffrey.

On Fri, Jul 05, 2019 at 09:56:55AM -0700, Jeffrey Hugo wrote:
> The Sharp LD-D5116Z01B is a 12.3" eDP panel with a 1920X1280 resolution.
> 
> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> ---
>  .../display/panel/sharp,ld-d5116z01b.txt      | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/sharp,ld-d5116z01b.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/sharp,ld-d5116z01b.txt b/Documentation/devicetree/bindings/display/panel/sharp,ld-d5116z01b.txt
> new file mode 100644
> index 000000000000..3938c2847fe5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/sharp,ld-d5116z01b.txt
> @@ -0,0 +1,27 @@
> +Sharp LD-D5116Z01B 12.3" WUXGA+ eDP panel
> +
> +Required properties:
> +- compatible: should be "sharp,ld-d5116z01b"
> +- power-supply: regulator to provide the VCC supply voltage (3.3 volts)
> +
> +This binding is compatible with the simple-panel binding.
> +
> +The device node can contain one 'port' child node with one child
> +'endpoint' node, according to the bindings defined in [1]. This
> +node should describe panel's video bus.
> +
> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Example:
> +
> +	panel: panel {
> +		compatible = "sharp,ld-d5116z01b";
> +		power-supply = <&vlcd_3v3>;
> +		no-hpd
The binding do not mention no-hpd - but it is part of panel-simple
binding. Is it included in the example for any special reason?

Also there is a syntax error, ";" is missing.


	Sam

> +
> +		port {
> +			panel_ep: endpoint {
> +				remote-endpoint = <&bridge_out_ep>;
> +			};
> +		};
> +	};
> -- 
> 2.17.1

Re: [PATCH 1/2] dt-bindings: panel: Add Sharp LD-D5116Z01B

[Jeffrey Hugo]

On Fri, Jul 5, 2019 at 11:23 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Jeffrey.
>
> On Fri, Jul 05, 2019 at 09:56:55AM -0700, Jeffrey Hugo wrote:
> > +     panel: panel {
> > +             compatible = "sharp,ld-d5116z01b";
> > +             power-supply = <&vlcd_3v3>;
> > +             no-hpd
> The binding do not mention no-hpd - but it is part of panel-simple
> binding. Is it included in the example for any special reason?

I just copied (poorly apparently since a ";" is missing) from my
platform's dt.  There is no particular reason the example lists
no-hpd.  I'll drop it in the next rev.

[PATCH 2/2] drm/panel: simple: Add support for Sharp LD-D5116Z01B panel

[Jeffrey Hugo]

The Sharp LD-D5116Z01B is a 12.3" eDP panel with a 1920X1280 resolution.

Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
---
 drivers/gpu/drm/panel/panel-simple.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 5a93c4edf1e4..e6f578667324 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2354,6 +2354,29 @@ static const struct panel_desc samsung_ltn140at29_301 = {
 	},
 };
 
+static const struct drm_display_mode sharp_ld_d5116z01b_mode = {
+	.clock = 168480,
+	.hdisplay = 1920,
+	.hsync_start = 1920 + 48,
+	.hsync_end = 1920 + 48 + 32,
+	.htotal = 1920 + 48 + 32 + 80,
+	.vdisplay = 1280,
+	.vsync_start = 1280 + 3,
+	.vsync_end = 1280 + 3 + 10,
+	.vtotal = 1280 + 3 + 10 + 57,
+	.vrefresh = 60,
+};
+
+static const struct panel_desc sharp_ld_d5116z01b = {
+	.modes = &sharp_ld_d5116z01b_mode,
+	.num_modes = 1,
+	.bpc = 8,
+	.size = {
+		.width = 260,
+		.height = 120,
+	},
+};
+
 static const struct drm_display_mode sharp_lq035q7db03_mode = {
 	.clock = 5500,
 	.hdisplay = 240,
@@ -3002,6 +3025,9 @@ static const struct of_device_id platform_of_match[] = {
 	}, {
 		.compatible = "samsung,ltn140at29-301",
 		.data = &samsung_ltn140at29_301,
+	}, {
+		.compatible = "sharp,ld-d5116z01b",
+		.data = &sharp_ld_d5116z01b,
 	}, {
 		.compatible = "sharp,lq035q7db03",
 		.data = &sharp_lq035q7db03,
-- 
2.17.1

Re: [PATCH 2/2] drm/panel: simple: Add support for Sharp LD-D5116Z01B panel

[Sam Ravnborg]

Hi Jeffrey.

Patch looks good, but there is a few fields that are not initialized.
Did you forget them, or are they not needed?

On Fri, Jul 05, 2019 at 09:57:55AM -0700, Jeffrey Hugo wrote:
> The Sharp LD-D5116Z01B is a 12.3" eDP panel with a 1920X1280 resolution.
> 
> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> ---
>  drivers/gpu/drm/panel/panel-simple.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index 5a93c4edf1e4..e6f578667324 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -2354,6 +2354,29 @@ static const struct panel_desc samsung_ltn140at29_301 = {
>  	},
>  };
>  
> +static const struct drm_display_mode sharp_ld_d5116z01b_mode = {
> +	.clock = 168480,
> +	.hdisplay = 1920,
> +	.hsync_start = 1920 + 48,
> +	.hsync_end = 1920 + 48 + 32,
> +	.htotal = 1920 + 48 + 32 + 80,
> +	.vdisplay = 1280,
> +	.vsync_start = 1280 + 3,
> +	.vsync_end = 1280 + 3 + 10,
> +	.vtotal = 1280 + 3 + 10 + 57,
> +	.vrefresh = 60,
> +};
No .flags? Is it not needed for an eDP panel?

> +
> +static const struct panel_desc sharp_ld_d5116z01b = {
> +	.modes = &sharp_ld_d5116z01b_mode,
> +	.num_modes = 1,
> +	.bpc = 8,
> +	.size = {
> +		.width = 260,
> +		.height = 120,
> +	},
> +};
No .bus_format?
No .bus_flags?

	Sam

Re: [PATCH 2/2] drm/panel: simple: Add support for Sharp LD-D5116Z01B panel

[Jeffrey Hugo]

On Fri, Jul 5, 2019 at 11:21 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Jeffrey.
>
> Patch looks good, but there is a few fields that are not initialized.
> Did you forget them, or are they not needed?

Thanks for the review.  Overlooked some of them.

>
> On Fri, Jul 05, 2019 at 09:57:55AM -0700, Jeffrey Hugo wrote:
> > The Sharp LD-D5116Z01B is a 12.3" eDP panel with a 1920X1280 resolution.
> >
> > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> > ---
> >  drivers/gpu/drm/panel/panel-simple.c | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > index 5a93c4edf1e4..e6f578667324 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -2354,6 +2354,29 @@ static const struct panel_desc samsung_ltn140at29_301 = {
> >       },
> >  };
> >
> > +static const struct drm_display_mode sharp_ld_d5116z01b_mode = {
> > +     .clock = 168480,
> > +     .hdisplay = 1920,
> > +     .hsync_start = 1920 + 48,
> > +     .hsync_end = 1920 + 48 + 32,
> > +     .htotal = 1920 + 48 + 32 + 80,
> > +     .vdisplay = 1280,
> > +     .vsync_start = 1280 + 3,
> > +     .vsync_end = 1280 + 3 + 10,
> > +     .vtotal = 1280 + 3 + 10 + 57,
> > +     .vrefresh = 60,
> > +};
> No .flags? Is it not needed for an eDP panel?

The flags don't appear to make sense per my understanding of eDP.
Therefore I intended .flags to be 0, which it implicitly is because
this is a static struct.  Would you prefer I explicitly list .flags =
0?

>
> > +
> > +static const struct panel_desc sharp_ld_d5116z01b = {
> > +     .modes = &sharp_ld_d5116z01b_mode,
> > +     .num_modes = 1,
> > +     .bpc = 8,
> > +     .size = {
> > +             .width = 260,
> > +             .height = 120,
> > +     },
> > +};
> No .bus_format?

Ah, yes.  Looks like it should be MEDIA_BUS_FMT_RGB888_1X24
Will fix.

> No .bus_flags?

eDP is differential signaling, so what I see generally doesn't apply,
but DRM_BUS_FLAG_DATA_MSB_TO_LSB does apply, so I'll add that.

Re: [PATCH 2/2] drm/panel: simple: Add support for Sharp LD-D5116Z01B panel

[Sam Ravnborg]

Hi Jeffrey.

On Fri, Jul 05, 2019 at 12:35:30PM -0600, Jeffrey Hugo wrote:
> On Fri, Jul 5, 2019 at 11:21 AM Sam Ravnborg <sam@ravnborg.org> wrote:
> >
> > Hi Jeffrey.
> >
> > Patch looks good, but there is a few fields that are not initialized.
> > Did you forget them, or are they not needed?
> 
> Thanks for the review.  Overlooked some of them.
> 
> >
> > On Fri, Jul 05, 2019 at 09:57:55AM -0700, Jeffrey Hugo wrote:
> > > The Sharp LD-D5116Z01B is a 12.3" eDP panel with a 1920X1280 resolution.
> > >
> > > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> > > ---
> > >  drivers/gpu/drm/panel/panel-simple.c | 26 ++++++++++++++++++++++++++
> > >  1 file changed, 26 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > > index 5a93c4edf1e4..e6f578667324 100644
> > > --- a/drivers/gpu/drm/panel/panel-simple.c
> > > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > > @@ -2354,6 +2354,29 @@ static const struct panel_desc samsung_ltn140at29_301 = {
> > >       },
> > >  };
> > >
> > > +static const struct drm_display_mode sharp_ld_d5116z01b_mode = {
> > > +     .clock = 168480,
> > > +     .hdisplay = 1920,
> > > +     .hsync_start = 1920 + 48,
> > > +     .hsync_end = 1920 + 48 + 32,
> > > +     .htotal = 1920 + 48 + 32 + 80,
> > > +     .vdisplay = 1280,
> > > +     .vsync_start = 1280 + 3,
> > > +     .vsync_end = 1280 + 3 + 10,
> > > +     .vtotal = 1280 + 3 + 10 + 57,
> > > +     .vrefresh = 60,
> > > +};
> > No .flags? Is it not needed for an eDP panel?
> 
> The flags don't appear to make sense per my understanding of eDP.
> Therefore I intended .flags to be 0, which it implicitly is because
> this is a static struct.  Would you prefer I explicitly list .flags =
> 0?
I was not sure hence my Q. And do not set it to 0, as then someone will
come and say this is default and delete it.
Maybe mention it in the changelog.


> 
> >
> > > +
> > > +static const struct panel_desc sharp_ld_d5116z01b = {
> > > +     .modes = &sharp_ld_d5116z01b_mode,
> > > +     .num_modes = 1,
> > > +     .bpc = 8,
> > > +     .size = {
> > > +             .width = 260,
> > > +             .height = 120,
> > > +     },
> > > +};
> > No .bus_format?
> 
> Ah, yes.  Looks like it should be MEDIA_BUS_FMT_RGB888_1X24
> Will fix.
> 
> > No .bus_flags?
> 
> eDP is differential signaling, so what I see generally doesn't apply,
> but DRM_BUS_FLAG_DATA_MSB_TO_LSB does apply, so I'll add that.

With your changes you can add my:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

On both patches.

	Sam