diff mbox

drm/panel/simple: add optronics B101XTN01.0

Message ID 1407357090-3801-1-git-send-email-robdclark@gmail.com (mailing list archive)
State Superseded
Headers show

Commit Message

Rob Clark Aug. 6, 2014, 8:31 p.m. UTC
LVDS panel, make/model described as:

AU Optronics Corporation - B101XTN01.0 (H/W:0A)

See:
http://www.encore-electronic.com/media/B101XTN01.0.pdf

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/panel/panel-simple.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

Rob Clark Aug. 6, 2014, 8:40 p.m. UTC | #1
On Wed, Aug 6, 2014 at 4:31 PM, Rob Clark <robdclark@gmail.com> wrote:
> LVDS panel, make/model described as:
>
> AU Optronics Corporation - B101XTN01.0 (H/W:0A)
>
> See:
> http://www.encore-electronic.com/media/B101XTN01.0.pdf
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  drivers/gpu/drm/panel/panel-simple.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index a251361..16119ba 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -447,6 +447,29 @@ static const struct panel_desc samsung_ltn101nt05 = {
>         },
>  };
>
> +static const struct drm_display_mode optronics_lvds_mode = {
> +       .clock = 72000,
> +       .hdisplay = 1366,
> +       .hsync_start = 1366 + 20,
> +       .hsync_end = 1366 + 20 + 70,
> +       .htotal = 1366 + 20 + 70,
> +       .vdisplay = 768,
> +       .vsync_start = 768 + 14,
> +       .vsync_end = 768 + 14 + 42,
> +       .vtotal = 768 + 14 + 42,
> +       .vrefresh = 60,
> +       .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
> +};
> +
> +static const struct panel_desc optronics_lvds = {
> +       .modes = &optronics_lvds_mode,
> +       .num_modes = 1,
> +       .size = {
> +               .width = 1366,
> +               .height = 768,
> +       },
> +};
> +
>  static const struct of_device_id platform_of_match[] = {
>         {
>                 .compatible = "auo,b101aw03",
> @@ -476,6 +499,10 @@ static const struct of_device_id platform_of_match[] = {
>                 .compatible = "samsung,ltn101nt05",
>                 .data = &samsung_ltn101nt05,
>         }, {
> +               .compatible = "optronics,b101xtn01",
> +               .data = &optronics_lvds,
> +       }, {
> +       }, {

erm, ignore the duplicate '}, {'.. opps..

BR,
-R

>                 .compatible = "simple-panel",
>         }, {
>                 /* sentinel */
> --
> 1.9.3
>
Thierry Reding Aug. 7, 2014, 7:33 a.m. UTC | #2
On Wed, Aug 06, 2014 at 04:31:30PM -0400, Rob Clark wrote:
> LVDS panel, make/model described as:
> 
> AU Optronics Corporation - B101XTN01.0 (H/W:0A)
> 
> See:
> http://www.encore-electronic.com/media/B101XTN01.0.pdf

I've made it a custom to mention which board a panel is used on in the
commit message and I've requested others to do the same. The idea was
that it would give some kind of indication where to find the hardware if
somebody wanted to test or look at for reference.

> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  drivers/gpu/drm/panel/panel-simple.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)

This is missing device tree binding documentation.

> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index a251361..16119ba 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -447,6 +447,29 @@ static const struct panel_desc samsung_ltn101nt05 = {
>  	},
>  };
>  
> +static const struct drm_display_mode optronics_lvds_mode = {

Can you please name this according to the model for consistency with
other panels?

> +	.clock = 72000,
> +	.hdisplay = 1366,
> +	.hsync_start = 1366 + 20,
> +	.hsync_end = 1366 + 20 + 70,
> +	.htotal = 1366 + 20 + 70,
> +	.vdisplay = 768,
> +	.vsync_start = 768 + 14,
> +	.vsync_end = 768 + 14 + 42,
> +	.vtotal = 768 + 14 + 42,
> +	.vrefresh = 60,
> +	.flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
> +};
> +
> +static const struct panel_desc optronics_lvds = {
> +	.modes = &optronics_lvds_mode,
> +	.num_modes = 1,
> +	.size = {
> +		.width = 1366,
> +		.height = 768,

This needs to be the physical dimension in mm.

> +	},
> +};

There's now also a new field called .bpc which contains the number of
bits per color (usually 6 or 8). Can you provide that as well? According
to the datasheet you linked to it should be 6.

> +
>  static const struct of_device_id platform_of_match[] = {
>  	{
>  		.compatible = "auo,b101aw03",
> @@ -476,6 +499,10 @@ static const struct of_device_id platform_of_match[] = {
>  		.compatible = "samsung,ltn101nt05",
>  		.data = &samsung_ltn101nt05,
>  	}, {
> +		.compatible = "optronics,b101xtn01",

From the commit message this panel is from AU Optronics, so it should
use the auo vendor prefix.

Thierry
Rob Clark Aug. 7, 2014, 12:50 p.m. UTC | #3
On Thu, Aug 7, 2014 at 3:33 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Wed, Aug 06, 2014 at 04:31:30PM -0400, Rob Clark wrote:
>> LVDS panel, make/model described as:
>>
>> AU Optronics Corporation - B101XTN01.0 (H/W:0A)
>>
>> See:
>> http://www.encore-electronic.com/media/B101XTN01.0.pdf
>
> I've made it a custom to mention which board a panel is used on in the
> commit message and I've requested others to do the same. The idea was
> that it would give some kind of indication where to find the hardware if
> somebody wanted to test or look at for reference.

It's just a free-standing panel, which (in this case) can be plugged
to an ifc6410 (and I think various other inforce boards..) looks like
a standard(ish) 41 pin connector.

>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>  drivers/gpu/drm/panel/panel-simple.c | 27 +++++++++++++++++++++++++++
>>  1 file changed, 27 insertions(+)
>
> This is missing device tree binding documentation.

there is nothing new, so I assume you just mean adding the compatible
string to a list?

>>
>> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
>> index a251361..16119ba 100644
>> --- a/drivers/gpu/drm/panel/panel-simple.c
>> +++ b/drivers/gpu/drm/panel/panel-simple.c
>> @@ -447,6 +447,29 @@ static const struct panel_desc samsung_ltn101nt05 = {
>>       },
>>  };
>>
>> +static const struct drm_display_mode optronics_lvds_mode = {
>
> Can you please name this according to the model for consistency with
> other panels?
>
>> +     .clock = 72000,
>> +     .hdisplay = 1366,
>> +     .hsync_start = 1366 + 20,
>> +     .hsync_end = 1366 + 20 + 70,
>> +     .htotal = 1366 + 20 + 70,
>> +     .vdisplay = 768,
>> +     .vsync_start = 768 + 14,
>> +     .vsync_end = 768 + 14 + 42,
>> +     .vtotal = 768 + 14 + 42,
>> +     .vrefresh = 60,
>> +     .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
>> +};
>> +
>> +static const struct panel_desc optronics_lvds = {
>> +     .modes = &optronics_lvds_mode,
>> +     .num_modes = 1,
>> +     .size = {
>> +             .width = 1366,
>> +             .height = 768,
>
> This needs to be the physical dimension in mm.
>
>> +     },
>> +};
>
> There's now also a new field called .bpc which contains the number of
> bits per color (usually 6 or 8). Can you provide that as well? According
> to the datasheet you linked to it should be 6.

well, that was a follow-on question I had as soon as I find a
different 8bpc panel.. but ok, nice that this was added.

>> +
>>  static const struct of_device_id platform_of_match[] = {
>>       {
>>               .compatible = "auo,b101aw03",
>> @@ -476,6 +499,10 @@ static const struct of_device_id platform_of_match[] = {
>>               .compatible = "samsung,ltn101nt05",
>>               .data = &samsung_ltn101nt05,
>>       }, {
>> +             .compatible = "optronics,b101xtn01",
>
> From the commit message this panel is from AU Optronics, so it should
> use the auo vendor prefix.

so, "auo,b101xtn01" works for everyone?

BR,
-R

> Thierry
Thierry Reding Aug. 7, 2014, 3:08 p.m. UTC | #4
On Thu, Aug 07, 2014 at 08:50:22AM -0400, Rob Clark wrote:
> On Thu, Aug 7, 2014 at 3:33 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> > On Wed, Aug 06, 2014 at 04:31:30PM -0400, Rob Clark wrote:
> >> LVDS panel, make/model described as:
> >>
> >> AU Optronics Corporation - B101XTN01.0 (H/W:0A)
> >>
> >> See:
> >> http://www.encore-electronic.com/media/B101XTN01.0.pdf
> >
> > I've made it a custom to mention which board a panel is used on in the
> > commit message and I've requested others to do the same. The idea was
> > that it would give some kind of indication where to find the hardware if
> > somebody wanted to test or look at for reference.
> 
> It's just a free-standing panel, which (in this case) can be plugged
> to an ifc6410 (and I think various other inforce boards..) looks like
> a standard(ish) 41 pin connector.

Well, it certainly doesn't have to be exclusively used on a board. I
guess if it isn't shipped with the IFC6410 as standard, maybe you could
simply mention that it's been tested on an IFC6410.

What I like to avoid is that people submit panel patches that are not
tested on real hardware.

> >> Signed-off-by: Rob Clark <robdclark@gmail.com>
> >> ---
> >>  drivers/gpu/drm/panel/panel-simple.c | 27 +++++++++++++++++++++++++++
> >>  1 file changed, 27 insertions(+)
> >
> > This is missing device tree binding documentation.
> 
> there is nothing new, so I assume you just mean adding the compatible
> string to a list?

There's no such list. You should add a new file, named after the
compatible string and stick it along with all the others into
Documentation/devicetree/bindings/panel. You can simply use one of the
other bindings that are derived from simple-panel as template and adjust
for the panel-specifics. There are a couple from AUO in that directory
already, so it should be easy.

> >>  static const struct of_device_id platform_of_match[] = {
> >>       {
> >>               .compatible = "auo,b101aw03",
> >> @@ -476,6 +499,10 @@ static const struct of_device_id platform_of_match[] = {
> >>               .compatible = "samsung,ltn101nt05",
> >>               .data = &samsung_ltn101nt05,
> >>       }, {
> >> +             .compatible = "optronics,b101xtn01",
> >
> > From the commit message this panel is from AU Optronics, so it should
> > use the auo vendor prefix.
> 
> so, "auo,b101xtn01" works for everyone?

Yeah, that's consistent with the other ones from AUO, so looking good to
me.

Thierry
diff mbox

Patch

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index a251361..16119ba 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -447,6 +447,29 @@  static const struct panel_desc samsung_ltn101nt05 = {
 	},
 };
 
+static const struct drm_display_mode optronics_lvds_mode = {
+	.clock = 72000,
+	.hdisplay = 1366,
+	.hsync_start = 1366 + 20,
+	.hsync_end = 1366 + 20 + 70,
+	.htotal = 1366 + 20 + 70,
+	.vdisplay = 768,
+	.vsync_start = 768 + 14,
+	.vsync_end = 768 + 14 + 42,
+	.vtotal = 768 + 14 + 42,
+	.vrefresh = 60,
+	.flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+};
+
+static const struct panel_desc optronics_lvds = {
+	.modes = &optronics_lvds_mode,
+	.num_modes = 1,
+	.size = {
+		.width = 1366,
+		.height = 768,
+	},
+};
+
 static const struct of_device_id platform_of_match[] = {
 	{
 		.compatible = "auo,b101aw03",
@@ -476,6 +499,10 @@  static const struct of_device_id platform_of_match[] = {
 		.compatible = "samsung,ltn101nt05",
 		.data = &samsung_ltn101nt05,
 	}, {
+		.compatible = "optronics,b101xtn01",
+		.data = &optronics_lvds,
+	}, {
+	}, {
 		.compatible = "simple-panel",
 	}, {
 		/* sentinel */