[PATCH v2 00/13] R-Car DU: Add support for LVDS mode selection

[PATCH v2 00/13] R-Car DU: Add support for LVDS mode selection

[Laurent Pinchart]

Hello,

This patch series adds support for LVDS mode selection in the R-Car DU driver.

Compared to v1, the LVDS panel DT bindings (02/13) have been reworked to
document common panel properties in a common file (01/13), with Mitsubishi
panels bindings now properly documented (03/13). Compared to the existing DPI
panel bindings that are currently abused by the R-Car DU driver for LVDS
panel, these new bindings specify the LVDS more explicitly.

The bindings are based on the relevant standards I have been able to find, as
well as on existing LVDS-related code and DT bindings available in the
mainline kernel. Those include

- the LVDS formats MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
  MEDIA_BUS_FMT_RGB888_1X7X4_SPWG and MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA
  (Documentation/media/uapi/v4l/subdev-formats.rst)
- iMX display DT bindings available in
  (Documentation/devicetree/bindings/display/imx/ldb.txt)
- the drivers/gpu/drm/imx/ driver

In addition to the three modes specified in the LVDS panel DT bindings, the
Renesas R-Car DU also supports the following two modes.

Slot       0       1       2       3       4       5       6
       ________________                         _________________
Clock                  \_______________________/
         ______  ______  ______  ______  ______  ______  ______
DATA0  ><_CTL0_><__R7__><__R6__><__R5__><__R4__><__R3__><__R2__><
DATA1  ><_CTL1_><__G7__><__G6__><__G5__><__G4__><__G3__><__G2__><
DATA2  ><_CTL2_><__B7__><__B6__><__B5__><__B4__><__B3__><__B2__><
DATA3  ><_CTL3_><__B1__><__B0__><__G1__><__G0__><__R1__><__R0__><
         ______  ______  ______  ______  ______  ______  ______
DATA0  ><_CTL0_><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
DATA1  ><_CTL1_><__G5__><__G4__><__G3__><__G2__><__G1__><__G0__><
DATA2  ><_CTL2_><__B5__><__B4__><__B3__><__B2__><__B1__><__B0__><
DATA3  ><_CTL3_><__B7__><__B6__><__G7__><__G6__><__R7__><__R6__><

and their mirrored version.

I haven't been able to find any standard defining those data mappings, nor any
panel using them. The control signals positions correspond to DC-balanced LVDS
(see figure 18 on page 19 of http://www.ti.com/lit/ds/symlink/ds90cf388.pdf),
but the R-Car DU doesn't support DC-balanced LVDS as far as I can tell, so
it's not a match. If anyone knows of other devices supporting these data
mappings or of standards defining them I would appreciate the information and
will update the bindings accordingly.

Patches 04/13 to 06/13 add a new DRM panel driver for LVDS panels compatible
with the DT bindings. Patches 07/13 to 09/13 update the device tree of the
R-Car H3 Salvator-X board to add panel backlight support, and patch 10/13
switches two Mitsubishi panels to use the new bindings. This series doesn't
strictly depend on them so they can be merged separately at a later point if
needed.

Patches 11/13 to 13/13 finally move the R-Car DU driver to use the DRM panel
API, and add support for LVDS mode selection.

Laurent Pinchart (13):
  devicetree/bindings: display: Document common panel properties
  devicetree/bindings: display: Add bindings for LVDS panels
  devicetree/bindings: display: Add bindings for two Mitsubishi panels
  drm: Add data mirror bus flag
  drm: panels: Constify device node argument to of_drm_find_panel()
  drm: panels: Add LVDS panel driver
  arm64: dts: r8a7795: Add PWM support
  arm64: dts: r8a7795: salvator-x: Add DU LVDS output endpoint
  arm64: dts: r8a7795: salvator-x: Add panel backlight support
  ARM: shmobile: dts: Switch to panel-lvds bindings for Mitsubishi
    panels
  drm: rcar-du: Switch to encoder .atomic_mode_set() helper function
  drm: rcar-du: Use the DRM panel API
  drm: rcar-du: Add support for LVDS mode selection

 .../display/panel/mitsubishi,aa104xd12.txt         |  47 ++++
 .../display/panel/mitsubishi,aa121td01.txt         |  47 ++++
 .../bindings/display/panel/panel-common.txt        |  91 +++++++
 .../bindings/display/panel/panel-lvds.txt          | 120 +++++++++
 arch/arm/boot/dts/r8a77xx-aa104xd12-panel.dtsi     |   3 +-
 arch/arm/boot/dts/r8a77xx-aa121td01-panel.dtsi     |   3 +-
 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts |  26 ++
 arch/arm64/boot/dts/renesas/r8a7795.dtsi           |  63 +++++
 drivers/gpu/drm/drm_panel.c                        |   2 +-
 drivers/gpu/drm/panel/Kconfig                      |  10 +
 drivers/gpu/drm/panel/Makefile                     |   1 +
 drivers/gpu/drm/panel/panel-lvds.c                 | 284 +++++++++++++++++++++
 drivers/gpu/drm/rcar-du/Kconfig                    |   1 +
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c          |  50 +++-
 drivers/gpu/drm/rcar-du/rcar_du_encoder.h          |   3 +
 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c          |   8 +-
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c          |  68 ++---
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c          |  11 +-
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h          |  13 +
 include/drm/drm_connector.h                        |   2 +
 include/drm/drm_panel.h                            |   4 +-
 21 files changed, 798 insertions(+), 59 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.txt
 create mode 100644 Documentation/devicetree/bindings/display/panel/mitsubishi,aa121td01.txt
 create mode 100644 Documentation/devicetree/bindings/display/panel/panel-common.txt
 create mode 100644 Documentation/devicetree/bindings/display/panel/panel-lvds.txt
 create mode 100644 drivers/gpu/drm/panel/panel-lvds.c

-- 
Regards,

Laurent Pinchart

[PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart]

Document properties common to several display panels in a central
location that can be referenced by the panel device tree bindings.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../bindings/display/panel/panel-common.txt        | 91 ++++++++++++++++++++++
 1 file changed, 91 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/panel-common.txt

diff --git a/Documentation/devicetree/bindings/display/panel/panel-common.txt b/Documentation/devicetree/bindings/display/panel/panel-common.txt
new file mode 100644
index 000000000000..ec52c472c845
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/panel-common.txt
@@ -0,0 +1,91 @@
+Common Properties for Display Panel
+===================================
+
+This document defines device tree properties common to several classes of
+display panels. It doesn't constitue a device tree binding specification by
+itself but is meant to be referenced by device tree bindings.
+
+When referenced from panel device tree bindings the properties defined in this
+document are defined as follows. The panel device tree bindings are
+responsible for defining whether each property is required or optional.
+
+
+Descriptive Properties
+----------------------
+
+- width-mm,
+- height-mm: The width-mm and height-mm specify the width and height of the
+  physical area where images are displayed. These properties are expressed in
+  millimeters and rounded to the closest unit.
+
+- label: The label property specifies a symbolic name for the panel as a
+  string suitable for use by humans. It typically contains a name inscribed on
+  the system (e.g. as an affixed label) or specified in the system's
+  documentation (e.g. in the user's manual).
+
+  If no such name exists, and unless the property is mandatory according to
+  device tree bindings, it shall rather be omitted than constructed of
+  non-descriptive information. For instance an LCD panel in a system that
+  contains a single panel shall not be labelled "LCD" if that name is not
+  inscribed on the system or used in a descriptive fashion in system
+  documentation.
+
+
+Display Timings
+---------------
+
+- panel-timing: Most display panels are restricted to a single resolution and
+  require specific display timings. The panel-timing subnode expresses those
+  timings as specified in the timing subnode section of the display timing
+  bindings defined in
+  Documentation/devicetree/bindings/display/display-timing.txt.
+
+
+Connectivity
+------------
+
+- ports: Panels receive video data through one or multiple connections. While
+  the nature of those connections is specific to the panel type, the
+  connectivity is expressed in a standard fashion using ports as specified in
+  the device graph bindings defined in
+  Documentation/devicetree/bindings/graph.txt.
+
+- ddc-i2c-bus: Some panels expose EDID information through an I2C-compatible
+  bus such as DDC2 or E-DDC. For such panels the ddc-i2c-bus contains a
+  phandle to the system I2C controller connected to that bus.
+
+
+Control I/Os
+------------
+
+Many display panels can be controlled through pins driven by GPIOs. The nature
+and timing of those control signals are device-specific and left for panel
+device tree bindings to specify. The following GPIO specifiers can however be
+used for panels that implement compatible control signals.
+
+- enable-gpios: Specifier for a GPIO connected to the panel enable control
+  signal. The enable signal is active high and enables operation of the panel.
+  This property can also be used for panels implementing an active low power
+  down signal, which is a negated version of the enable signal. Active low
+  enable signals (or active high power down signals) can be supported by
+  inverting the GPIO specifier polarity flag.
+
+  Note that the enable signal control panel operation only and must not be
+  confused with a backlight enable signal.
+
+- reset-gpios: Specifier for a GPIO coonnected to the panel reset control
+  signal. The reset signal is active low and resets the panel internal logic
+  while active. Active high reset signals can be supported by inverting the
+  GPIO specifier polarity flag.
+
+
+Backlight
+---------
+
+Most display panels include a backlight. Some of them also include a backlight
+controller exposed through a control bus such as I2C or DSI. Others expose
+backlight control through GPIO, PWM or other signals connected to an external
+backlight controller.
+
+- backlight: For panels whose backlight is controlled by an external backlight
+  controller, this property contains a phandle that references the controller.
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Rob Herring]

On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> Document properties common to several display panels in a central
> location that can be referenced by the panel device tree bindings.
> 

Looks good. Just one comment...

[...]

> +Connectivity
> +------------
> +
> +- ports: Panels receive video data through one or multiple connections. While
> +  the nature of those connections is specific to the panel type, the
> +  connectivity is expressed in a standard fashion using ports as specified in
> +  the device graph bindings defined in
> +  Documentation/devicetree/bindings/graph.txt.

We allow panels to either use graph binding or be a child of the display 
controller. Using the graph is preferred, but in the simple cases just a 
child node is sufficient. This should be described here or somewhere in 
this doc.

Rob

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart]

Hi Rob,

On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> > Document properties common to several display panels in a central
> > location that can be referenced by the panel device tree bindings.
> 
> Looks good. Just one comment...
> 
> [...]
> 
> > +Connectivity
> > +------------
> > +
> > +- ports: Panels receive video data through one or multiple connections.
> > While
> > +  the nature of those connections is specific to the panel type, the
> > +  connectivity is expressed in a standard fashion using ports as
> > specified in
> > +  the device graph bindings defined in
> > +  Documentation/devicetree/bindings/graph.txt.
> 
> We allow panels to either use graph binding or be a child of the display
> controller.

I knew that some display controllers use a phandle to the panel (see the 
fsl,panel and nvidia,panel properties), but I didn't know we had panels as 
children of display controller nodes. I don't think we should allow that for 
anything but DSI panels, as the DT hierarchy is based on control buses. Are 
you sure we have other panels instantiated through that mechanism ?

> Using the graph is preferred, but in the simple cases just a child node is
> sufficient. This should be described here or somewhere in this doc.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart]

Hi Rob,

On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
> > On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> >> Document properties common to several display panels in a central
> >> location that can be referenced by the panel device tree bindings.
> > 
> > Looks good. Just one comment...
> > 
> > [...]
> > 
> >> +Connectivity
> >> +------------
> >> +
> >> +- ports: Panels receive video data through one or multiple connections.
> >> While
> >> +  the nature of those connections is specific to the panel type, the
> >> +  connectivity is expressed in a standard fashion using ports as
> >> specified in
> >> +  the device graph bindings defined in
> >> +  Documentation/devicetree/bindings/graph.txt.
> > 
> > We allow panels to either use graph binding or be a child of the display
> > controller.
> 
> I knew that some display controllers use a phandle to the panel (see the
> fsl,panel and nvidia,panel properties), but I didn't know we had panels as
> children of display controller nodes. I don't think we should allow that for
> anything but DSI panels, as the DT hierarchy is based on control buses. Are
> you sure we have other panels instantiated through that mechanism ?

Ping ?

Please note that this file documents properties common to multiple panel DT 
bindings, but in no way makes it mandatory to use the OF graph bindings for 
panels. The decision is left to individual bindings.

> > Using the graph is preferred, but in the simple cases just a child node is
> > sufficient. This should be described here or somewhere in this doc.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Rob Herring]

On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
>> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
>> > On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
>> >> Document properties common to several display panels in a central
>> >> location that can be referenced by the panel device tree bindings.
>> >
>> > Looks good. Just one comment...
>> >
>> > [...]
>> >
>> >> +Connectivity
>> >> +------------
>> >> +
>> >> +- ports: Panels receive video data through one or multiple connections.
>> >> While
>> >> +  the nature of those connections is specific to the panel type, the
>> >> +  connectivity is expressed in a standard fashion using ports as
>> >> specified in
>> >> +  the device graph bindings defined in
>> >> +  Documentation/devicetree/bindings/graph.txt.
>> >
>> > We allow panels to either use graph binding or be a child of the display
>> > controller.
>>
>> I knew that some display controllers use a phandle to the panel (see the
>> fsl,panel and nvidia,panel properties), but I didn't know we had panels as
>> children of display controller nodes. I don't think we should allow that for
>> anything but DSI panels, as the DT hierarchy is based on control buses. Are
>> you sure we have other panels instantiated through that mechanism ?

Some panels have no control bus, so were do we place them? I would say
the hierarchy is based on buses with a preference for the control bus
when there are multiple buses. I'm not a fan of just sticking things
are the top level.

> Ping ?
>
> Please note that this file documents properties common to multiple panel DT
> bindings, but in no way makes it mandatory to use the OF graph bindings for
> panels. The decision is left to individual bindings.

It is mandatory in the sense that we don't want more cases of "fsl,panel".

Rob

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart]

Hi Rob,

On Tuesday 29 Nov 2016 09:14:09 Rob Herring wrote:
> On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart wrote:
> > On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
> >> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
> >>> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> >>>> Document properties common to several display panels in a central
> >>>> location that can be referenced by the panel device tree bindings.
> >>> 
> >>> Looks good. Just one comment...
> >>> 
> >>> [...]
> >>> 
> >>>> +Connectivity
> >>>> +------------
> >>>> +
> >>>> +- ports: Panels receive video data through one or multiple
> >>>> connections. While
> >>>> +  the nature of those connections is specific to the panel type, the
> >>>> +  connectivity is expressed in a standard fashion using ports as
> >>>> specified in
> >>>> +  the device graph bindings defined in
> >>>> +  Documentation/devicetree/bindings/graph.txt.
> >>> 
> >>> We allow panels to either use graph binding or be a child of the
> >>> display controller.
> >> 
> >> I knew that some display controllers use a phandle to the panel (see the
> >> fsl,panel and nvidia,panel properties), but I didn't know we had panels
> >> as children of display controller nodes. I don't think we should allow
> >> that for anything but DSI panels, as the DT hierarchy is based on control
> >> buses. Are you sure we have other panels instantiated through that
> >> mechanism ?
>
> Some panels have no control bus, so were do we place them?

I'd say under the root node, like all similar control-less devices.

> I would say the hierarchy is based on buses with a preference for the
> control bus when there are multiple buses. I'm not a fan of just sticking
> things are the top level.

OK, so much for my comment a few lines up :-)

The problem with placing non-DSI panels as children of the display controller 
and not using OF graph is that the panel bindings become dependent of the 
display controller being used. A display controller using OF graph would 
require the panel to do the same, while a display controller expecting a panel 
child node (with a specific name) would require DT properties for the panel 
node.

I'm also not sure the complexity of OF graph is really that prohibitive if you 
compare it to panels as child nodes. To get the panel driver to bind to the 
panel DT node the display controller driver would need to create a platform 
device for the panel and register it. That's not very difficult, but parsing a 
single port and endpoint isn't either (and we could even provide a helper 
function for that, a version of of_drm_find_panel() that would take as an 
argument the display controller device node instead of the panel device node).

> > Ping ?
> > 
> > Please note that this file documents properties common to multiple panel
> > DT bindings, but in no way makes it mandatory to use the OF graph bindings
> > for panels. The decision is left to individual bindings.
> 
> It is mandatory in the sense that we don't want more cases of "fsl,panel".

That I agree with :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart]

Hi Rob,

On Tuesday 29 Nov 2016 20:23:41 Laurent Pinchart wrote:
> On Tuesday 29 Nov 2016 09:14:09 Rob Herring wrote:
> > On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart wrote:
> >> On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
> >>> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
> >>>> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> >>>>> Document properties common to several display panels in a central
> >>>>> location that can be referenced by the panel device tree bindings.
> >>>> 
> >>>> Looks good. Just one comment...
> >>>> 
> >>>> [...]
> >>>> 
> >>>>> +Connectivity
> >>>>> +------------
> >>>>> +
> >>>>> +- ports: Panels receive video data through one or multiple
> >>>>> connections. While
> >>>>> +  the nature of those connections is specific to the panel type, the
> >>>>> +  connectivity is expressed in a standard fashion using ports as
> >>>>> specified in
> >>>>> +  the device graph bindings defined in
> >>>>> +  Documentation/devicetree/bindings/graph.txt.
> >>>> 
> >>>> We allow panels to either use graph binding or be a child of the
> >>>> display controller.
> >>> 
> >>> I knew that some display controllers use a phandle to the panel (see
> >>> the fsl,panel and nvidia,panel properties), but I didn't know we had
> >>> panels as children of display controller nodes. I don't think we should
> >>> allow that for anything but DSI panels, as the DT hierarchy is based on
> >>> control buses. Are you sure we have other panels instantiated through
> >>> that mechanism ?
> > 
> > Some panels have no control bus, so were do we place them?
> 
> I'd say under the root node, like all similar control-less devices.
> 
> > I would say the hierarchy is based on buses with a preference for the
> > control bus when there are multiple buses. I'm not a fan of just sticking
> > things are the top level.
> 
> OK, so much for my comment a few lines up :-)
> 
> The problem with placing non-DSI panels as children of the display
> controller and not using OF graph is that the panel bindings become
> dependent of the display controller being used. A display controller using
> OF graph would require the panel to do the same, while a display controller
> expecting a panel child node (with a specific name) would require DT
> properties for the panel node.
> 
> I'm also not sure the complexity of OF graph is really that prohibitive if
> you compare it to panels as child nodes. To get the panel driver to bind to
> the panel DT node the display controller driver would need to create a
> platform device for the panel and register it. That's not very difficult,
> but parsing a single port and endpoint isn't either (and we could even
> provide a helper function for that, a version of of_drm_find_panel() that
> would take as an argument the display controller device node instead of the
> panel device node).

Ping ?

I'd like to standardize on one model for panel DT bindings, but I'm not sure 
that can be achieved given that we already have multiple competing models. In 
any case, is that blocking to merge this patch ? I only describe one 
connectivity model here as that's what my panel driver needs, but I have no 
issue adding more models later when needed. I believe this patch is a good 
step forward already.

> >> Ping ?
> >> 
> >> Please note that this file documents properties common to multiple panel
> >> DT bindings, but in no way makes it mandatory to use the OF graph
> >> bindings for panels. The decision is left to individual bindings.
> > 
> > It is mandatory in the sense that we don't want more cases of "fsl,panel".
> 
> That I agree with :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Rob Herring]

On Sun, Dec 18, 2016 at 2:54 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Tuesday 29 Nov 2016 20:23:41 Laurent Pinchart wrote:
>> On Tuesday 29 Nov 2016 09:14:09 Rob Herring wrote:
>> > On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart wrote:
>> >> On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
>> >>> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
>> >>>> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
>> >>>>> Document properties common to several display panels in a central
>> >>>>> location that can be referenced by the panel device tree bindings.
>> >>>>
>> >>>> Looks good. Just one comment...
>> >>>>
>> >>>> [...]
>> >>>>
>> >>>>> +Connectivity
>> >>>>> +------------
>> >>>>> +
>> >>>>> +- ports: Panels receive video data through one or multiple
>> >>>>> connections. While
>> >>>>> +  the nature of those connections is specific to the panel type, the
>> >>>>> +  connectivity is expressed in a standard fashion using ports as
>> >>>>> specified in
>> >>>>> +  the device graph bindings defined in
>> >>>>> +  Documentation/devicetree/bindings/graph.txt.
>> >>>>
>> >>>> We allow panels to either use graph binding or be a child of the
>> >>>> display controller.
>> >>>
>> >>> I knew that some display controllers use a phandle to the panel (see
>> >>> the fsl,panel and nvidia,panel properties), but I didn't know we had
>> >>> panels as children of display controller nodes. I don't think we should
>> >>> allow that for anything but DSI panels, as the DT hierarchy is based on
>> >>> control buses. Are you sure we have other panels instantiated through
>> >>> that mechanism ?
>> >
>> > Some panels have no control bus, so were do we place them?
>>
>> I'd say under the root node, like all similar control-less devices.
>>
>> > I would say the hierarchy is based on buses with a preference for the
>> > control bus when there are multiple buses. I'm not a fan of just sticking
>> > things are the top level.
>>
>> OK, so much for my comment a few lines up :-)
>>
>> The problem with placing non-DSI panels as children of the display
>> controller and not using OF graph is that the panel bindings become
>> dependent of the display controller being used. A display controller using
>> OF graph would require the panel to do the same, while a display controller
>> expecting a panel child node (with a specific name) would require DT
>> properties for the panel node.

Not sure I follow. They become dependent on the controller driver to
probe the panel, but the contents of the panel node would not be
controller dependent.

>> I'm also not sure the complexity of OF graph is really that prohibitive if
>> you compare it to panels as child nodes. To get the panel driver to bind to
>> the panel DT node the display controller driver would need to create a
>> platform device for the panel and register it. That's not very difficult,
>> but parsing a single port and endpoint isn't either (and we could even
>> provide a helper function for that, a version of of_drm_find_panel() that
>> would take as an argument the display controller device node instead of the
>> panel device node).
>
> Ping ?
>
> I'd like to standardize on one model for panel DT bindings, but I'm not sure
> that can be achieved given that we already have multiple competing models. In
> any case, is that blocking to merge this patch ? I only describe one
> connectivity model here as that's what my panel driver needs, but I have no
> issue adding more models later when needed. I believe this patch is a good
> step forward already.

It is an improvement which I appreciate, so yes I guess we can address
it later when needed.

Rob

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart]

Hi Rob,

On Monday 19 Dec 2016 09:38:49 Rob Herring wrote:
> On Sun, Dec 18, 2016 at 2:54 PM, Laurent Pinchart wrote:
> > On Tuesday 29 Nov 2016 20:23:41 Laurent Pinchart wrote:
> >> On Tuesday 29 Nov 2016 09:14:09 Rob Herring wrote:
> >>> On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart wrote:
> >>>> On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
> >>>>> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
> >>>>>> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> >>>>>>> Document properties common to several display panels in a central
> >>>>>>> location that can be referenced by the panel device tree bindings.
> >>>>>> 
> >>>>>> Looks good. Just one comment...
> >>>>>> 
> >>>>>> [...]
> >>>>>> 
> >>>>>>> +Connectivity
> >>>>>>> +------------
> >>>>>>> +
> >>>>>>> +- ports: Panels receive video data through one or multiple
> >>>>>>> connections. While
> >>>>>>> +  the nature of those connections is specific to the panel type,
> >>>>>>> the
> >>>>>>> +  connectivity is expressed in a standard fashion using ports as
> >>>>>>> specified in
> >>>>>>> +  the device graph bindings defined in
> >>>>>>> +  Documentation/devicetree/bindings/graph.txt.
> >>>>>> 
> >>>>>> We allow panels to either use graph binding or be a child of the
> >>>>>> display controller.
> >>>>> 
> >>>>> I knew that some display controllers use a phandle to the panel (see
> >>>>> the fsl,panel and nvidia,panel properties), but I didn't know we had
> >>>>> panels as children of display controller nodes. I don't think we
> >>>>> should allow that for anything but DSI panels, as the DT hierarchy is
> >>>>> based on control buses. Are you sure we have other panels instantiated
> >>>>> through that mechanism ?
> >>> 
> >>> Some panels have no control bus, so were do we place them?
> >> 
> >> I'd say under the root node, like all similar control-less devices.
> >> 
> >>> I would say the hierarchy is based on buses with a preference for the
> >>> control bus when there are multiple buses. I'm not a fan of just
> >>> sticking things are the top level.
> >> 
> >> OK, so much for my comment a few lines up :-)
> >> 
> >> The problem with placing non-DSI panels as children of the display
> >> controller and not using OF graph is that the panel bindings become
> >> dependent of the display controller being used. A display controller
> >> using OF graph would require the panel to do the same, while a display
> >> controller expecting a panel child node (with a specific name) would
> >> require DT properties for the panel node.
> 
> Not sure I follow.

Sorry, I meant "would not requite DT properties".

> They become dependent on the controller driver to probe the panel, but the
> contents of the panel node would not be controller dependent.

If a display controller uses OF graph then the panel DT node has to declare 
ports. If the display controller doesn't use OF graph but instead expects the 
panel to be a direct subnode, or points to the panel using a property such as 
fsl,panel, then the panel DT node will not have ports.

> >> I'm also not sure the complexity of OF graph is really that prohibitive
> >> if you compare it to panels as child nodes. To get the panel driver to
> >> bind to the panel DT node the display controller driver would need to
> >> create a platform device for the panel and register it. That's not very
> >> difficult, but parsing a single port and endpoint isn't either (and we
> >> could even provide a helper function for that, a version of
> >> of_drm_find_panel() that would take as an argument the display controller
> >> device node instead of the panel device node).
> > 
> > Ping ?
> > 
> > I'd like to standardize on one model for panel DT bindings, but I'm not
> > sure that can be achieved given that we already have multiple competing
> > models. In any case, is that blocking to merge this patch ? I only
> > describe one connectivity model here as that's what my panel driver
> > needs, but I have no issue adding more models later when needed. I
> > believe this patch is a good step forward already.
> 
> It is an improvement which I appreciate, so yes I guess we can address
> it later when needed.

Thank you. Can I get your ack then ? :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Rob Herring]

On Mon, Dec 19, 2016 at 10:54 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Monday 19 Dec 2016 09:38:49 Rob Herring wrote:
>> On Sun, Dec 18, 2016 at 2:54 PM, Laurent Pinchart wrote:
>> > On Tuesday 29 Nov 2016 20:23:41 Laurent Pinchart wrote:
>> >> On Tuesday 29 Nov 2016 09:14:09 Rob Herring wrote:
>> >>> On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart wrote:
>> >>>> On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
>> >>>>> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
>> >>>>>> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
>> >>>>>>> Document properties common to several display panels in a central
>> >>>>>>> location that can be referenced by the panel device tree bindings.
>> >>>>>>
>> >>>>>> Looks good. Just one comment...
>> >>>>>>
>> >>>>>> [...]
>> >>>>>>
>> >>>>>>> +Connectivity
>> >>>>>>> +------------
>> >>>>>>> +
>> >>>>>>> +- ports: Panels receive video data through one or multiple
>> >>>>>>> connections. While
>> >>>>>>> +  the nature of those connections is specific to the panel type,
>> >>>>>>> the
>> >>>>>>> +  connectivity is expressed in a standard fashion using ports as
>> >>>>>>> specified in
>> >>>>>>> +  the device graph bindings defined in
>> >>>>>>> +  Documentation/devicetree/bindings/graph.txt.
>> >>>>>>
>> >>>>>> We allow panels to either use graph binding or be a child of the
>> >>>>>> display controller.
>> >>>>>
>> >>>>> I knew that some display controllers use a phandle to the panel (see
>> >>>>> the fsl,panel and nvidia,panel properties), but I didn't know we had
>> >>>>> panels as children of display controller nodes. I don't think we
>> >>>>> should allow that for anything but DSI panels, as the DT hierarchy is
>> >>>>> based on control buses. Are you sure we have other panels instantiated
>> >>>>> through that mechanism ?
>> >>>
>> >>> Some panels have no control bus, so were do we place them?
>> >>
>> >> I'd say under the root node, like all similar control-less devices.
>> >>
>> >>> I would say the hierarchy is based on buses with a preference for the
>> >>> control bus when there are multiple buses. I'm not a fan of just
>> >>> sticking things are the top level.
>> >>
>> >> OK, so much for my comment a few lines up :-)
>> >>
>> >> The problem with placing non-DSI panels as children of the display
>> >> controller and not using OF graph is that the panel bindings become
>> >> dependent of the display controller being used. A display controller
>> >> using OF graph would require the panel to do the same, while a display
>> >> controller expecting a panel child node (with a specific name) would
>> >> require DT properties for the panel node.
>>
>> Not sure I follow.
>
> Sorry, I meant "would not requite DT properties".
>
>> They become dependent on the controller driver to probe the panel, but the
>> contents of the panel node would not be controller dependent.
>
> If a display controller uses OF graph then the panel DT node has to declare
> ports. If the display controller doesn't use OF graph but instead expects the
> panel to be a direct subnode, or points to the panel using a property such as
> fsl,panel, then the panel DT node will not have ports.

The controller should just ask for the panel via a common function.
That function should then look for either a child node (called
'panel') or a graph port. Of course, for more complex cases, only OF
graph may work. It really just the simple case of a controller with a
single output and single panel that I'm talking about.


>> >> I'm also not sure the complexity of OF graph is really that prohibitive
>> >> if you compare it to panels as child nodes. To get the panel driver to
>> >> bind to the panel DT node the display controller driver would need to
>> >> create a platform device for the panel and register it. That's not very
>> >> difficult, but parsing a single port and endpoint isn't either (and we
>> >> could even provide a helper function for that, a version of
>> >> of_drm_find_panel() that would take as an argument the display controller
>> >> device node instead of the panel device node).
>> >
>> > Ping ?
>> >
>> > I'd like to standardize on one model for panel DT bindings, but I'm not
>> > sure that can be achieved given that we already have multiple competing
>> > models. In any case, is that blocking to merge this patch ? I only
>> > describe one connectivity model here as that's what my panel driver
>> > needs, but I have no issue adding more models later when needed. I
>> > believe this patch is a good step forward already.
>>
>> It is an improvement which I appreciate, so yes I guess we can address
>> it later when needed.
>
> Thank you. Can I get your ack then ? :-)

Yes.

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2999 bytes --]

On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> Document properties common to several display panels in a central
> location that can be referenced by the panel device tree bindings.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../bindings/display/panel/panel-common.txt        | 91 ++++++++++++++++++++++
>  1 file changed, 91 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-common.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-common.txt b/Documentation/devicetree/bindings/display/panel/panel-common.txt
> new file mode 100644
> index 000000000000..ec52c472c845
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/panel-common.txt
> @@ -0,0 +1,91 @@
> +Common Properties for Display Panel
> +===================================
> +
> +This document defines device tree properties common to several classes of
> +display panels. It doesn't constitue a device tree binding specification by
> +itself but is meant to be referenced by device tree bindings.
> +
> +When referenced from panel device tree bindings the properties defined in this
> +document are defined as follows. The panel device tree bindings are
> +responsible for defining whether each property is required or optional.
> +
> +
> +Descriptive Properties
> +----------------------
> +
> +- width-mm,
> +- height-mm: The width-mm and height-mm specify the width and height of the
> +  physical area where images are displayed. These properties are expressed in
> +  millimeters and rounded to the closest unit.

Erm... this is already implied by the compatible string. Having this in
device tree is completely redundant.

> +- label: The label property specifies a symbolic name for the panel as a
> +  string suitable for use by humans. It typically contains a name inscribed on
> +  the system (e.g. as an affixed label) or specified in the system's
> +  documentation (e.g. in the user's manual).
> +
> +  If no such name exists, and unless the property is mandatory according to
> +  device tree bindings, it shall rather be omitted than constructed of
> +  non-descriptive information. For instance an LCD panel in a system that
> +  contains a single panel shall not be labelled "LCD" if that name is not
> +  inscribed on the system or used in a descriptive fashion in system
> +  documentation.
> +
> +
> +Display Timings
> +---------------
> +
> +- panel-timing: Most display panels are restricted to a single resolution and
> +  require specific display timings. The panel-timing subnode expresses those
> +  timings as specified in the timing subnode section of the display timing
> +  bindings defined in
> +  Documentation/devicetree/bindings/display/display-timing.txt.

Why? That's also implied by the compatible string. Honestly, I thought
by now we had been over this often enough...

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart]

Hi Thierry,

On Tuesday 22 Nov 2016 12:05:48 Thierry Reding wrote:
> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> > Document properties common to several display panels in a central
> > location that can be referenced by the panel device tree bindings.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  .../bindings/display/panel/panel-common.txt        | 91 +++++++++++++++++
> >  1 file changed, 91 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/display/panel/panel-common.txt> 
> > diff --git
> > a/Documentation/devicetree/bindings/display/panel/panel-common.txt
> > b/Documentation/devicetree/bindings/display/panel/panel-common.txt
> > new file mode 100644
> > index 000000000000..ec52c472c845
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/panel-common.txt
> > @@ -0,0 +1,91 @@
> > +Common Properties for Display Panel
> > +===================================
> > +
> > +This document defines device tree properties common to several classes of
> > +display panels. It doesn't constitue a device tree binding specification
> > by
> > +itself but is meant to be referenced by device tree bindings.
> > +
> > +When referenced from panel device tree bindings the properties defined in
> > this
> > +document are defined as follows. The panel device tree bindings are
> > +responsible for defining whether each property is required or optional.
> > +
> > +
> > +Descriptive Properties
> > +----------------------
> > +
> > +- width-mm,
> > +- height-mm: The width-mm and height-mm specify the width and height of
> > the
> > +  physical area where images are displayed. These properties are
> > expressed in
> > +  millimeters and rounded to the closest unit.
> 
> Erm... this is already implied by the compatible string. Having this in
> device tree is completely redundant.

Nothing new under the sun here, we already have plenty of properties that 
could be implied by compatible strings. For instance for SoC IP cores many 
vendors use both an SoC-specific compatible string and a generic compatible 
string (e.g. "renesas,gpio-r8a7795" and "renesas,gpio-rcar"). The SoC-
compatible string implies register addresses, clocks and interrupts, but we 
still describe them in DT.

At the end of the day information about devices and their integration in the 
system needs to be available, either from DT or from C code. DT bindings 
should be designed to strike a good balance there, avoiding redundant 
information in DT (and thus keeping the bindings simple) while still providing 
enough information to allow for a reasonable level of genericity in OS 
implementations.

> > +- label: The label property specifies a symbolic name for the panel as a
> > +  string suitable for use by humans. It typically contains a name
> > inscribed on
> > +  the system (e.g. as an affixed label) or specified in the system's
> > +  documentation (e.g. in the user's manual).
> > +
> > +  If no such name exists, and unless the property is mandatory according
> > to
> > +  device tree bindings, it shall rather be omitted than constructed of
> > +  non-descriptive information. For instance an LCD panel in a system that
> > +  contains a single panel shall not be labelled "LCD" if that name is not
> > +  inscribed on the system or used in a descriptive fashion in system
> > +  documentation.
> > +
> > +
> > +Display Timings
> > +---------------
> > +
> > +- panel-timing: Most display panels are restricted to a single resolution
> > and
> > +  require specific display timings. The panel-timing subnode expresses
> > those
> > +  timings as specified in the timing subnode section of the display
> > timing
> > +  bindings defined in
> > +  Documentation/devicetree/bindings/display/display-timing.txt.
> 
> Why? That's also implied by the compatible string. Honestly, I thought
> by now we had been over this often enough...

Same argument as above. I won't try to change your mind and fix the simple 
panel driver, but I still stand firm on my belief that expressing the size and 
timings in DT is the right solution in a wide variety of cases (and yes I've 
read http://sietch-tagr.blogspot.fi/2016/04/display-panels-are-not-special.html, and while I agree with the title, I still believe size and 
timings in DT are not wrong).

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Rob Herring]

On Tue, Nov 22, 2016 at 5:05 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
>> Document properties common to several display panels in a central
>> location that can be referenced by the panel device tree bindings.
>>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> ---
>>  .../bindings/display/panel/panel-common.txt        | 91 ++++++++++++++++++++++
>>  1 file changed, 91 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-common.txt
>>
>> diff --git a/Documentation/devicetree/bindings/display/panel/panel-common.txt b/Documentation/devicetree/bindings/display/panel/panel-common.txt
>> new file mode 100644
>> index 000000000000..ec52c472c845
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/panel/panel-common.txt
>> @@ -0,0 +1,91 @@
>> +Common Properties for Display Panel
>> +===================================
>> +
>> +This document defines device tree properties common to several classes of
>> +display panels. It doesn't constitue a device tree binding specification by
>> +itself but is meant to be referenced by device tree bindings.
>> +
>> +When referenced from panel device tree bindings the properties defined in this
>> +document are defined as follows. The panel device tree bindings are
>> +responsible for defining whether each property is required or optional.
>> +
>> +
>> +Descriptive Properties
>> +----------------------
>> +
>> +- width-mm,
>> +- height-mm: The width-mm and height-mm specify the width and height of the
>> +  physical area where images are displayed. These properties are expressed in
>> +  millimeters and rounded to the closest unit.
>
> Erm... this is already implied by the compatible string. Having this in
> device tree is completely redundant.
>
>> +- label: The label property specifies a symbolic name for the panel as a
>> +  string suitable for use by humans. It typically contains a name inscribed on
>> +  the system (e.g. as an affixed label) or specified in the system's
>> +  documentation (e.g. in the user's manual).
>> +
>> +  If no such name exists, and unless the property is mandatory according to
>> +  device tree bindings, it shall rather be omitted than constructed of
>> +  non-descriptive information. For instance an LCD panel in a system that
>> +  contains a single panel shall not be labelled "LCD" if that name is not
>> +  inscribed on the system or used in a descriptive fashion in system
>> +  documentation.
>> +
>> +
>> +Display Timings
>> +---------------
>> +
>> +- panel-timing: Most display panels are restricted to a single resolution and
>> +  require specific display timings. The panel-timing subnode expresses those
>> +  timings as specified in the timing subnode section of the display timing
>> +  bindings defined in
>> +  Documentation/devicetree/bindings/display/display-timing.txt.
>
> Why? That's also implied by the compatible string. Honestly, I thought
> by now we had been over this often enough...

While I completely agree we don't want *only* generic compatibles nor
generic gpio and power control, I think timing values in DT are fine.
They are just data copied out of datasheets and aren't tweaked per
platform. If the same data would make sense to put into a display
EDID, I think it also makes sense to put that data in DT.

Rob

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Emil Velikov]

Hi Laurent,

Pardon for reviving this old thread. I've noticed a couple of things
which might want some attention.

On 19 November 2016 at 03:28, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:

> +
> +- panel-timing: Most display panels are restricted to a single resolution and
> +  require specific display timings. The panel-timing subnode expresses those
> +  timings as specified in the timing subnode section of the display timing
> +  bindings defined in
> +  Documentation/devicetree/bindings/display/display-timing.txt.
Cannot find such a file in linux-next. Perhaps you meant
Documentation/devicetree/bindings/display/panel/display-timing.txt?

Documentation/devicetree/bindings/display/panel/panel.txt includes a
"rotation" property, which we might want to fold here.

Regards,
Emil

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart]

Hi Emil,

On Sunday 09 Apr 2017 12:47:01 Emil Velikov wrote:
> Hi Laurent,
> 
> Pardon for reviving this old thread. I've noticed a couple of things
> which might want some attention.
> 
> On 19 November 2016 at 03:28, Laurent Pinchart wrote:
> > +
> > +- panel-timing: Most display panels are restricted to a single resolution
> > and +  require specific display timings. The panel-timing subnode
> > expresses those +  timings as specified in the timing subnode section of
> > the display timing +  bindings defined in
> > +  Documentation/devicetree/bindings/display/display-timing.txt.
> 
> Cannot find such a file in linux-next. Perhaps you meant
> Documentation/devicetree/bindings/display/panel/display-timing.txt?

Oops. My bad, I'll fix that. Thank you for noticing it.

> Documentation/devicetree/bindings/display/panel/panel.txt includes a
> "rotation" property, which we might want to fold here.

I believe that panel.txt and panel-common.txt were added concurrently. We 
should indeed merge the two.

-- 
Regards,

Laurent Pinchart

[PATCH v2 02/13] devicetree/bindings: display: Add bindings for LVDS panels

[Laurent Pinchart]

LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
Multiple incompatible data link layers have been used over time to
transmit image data to LVDS panels. This binding supports display panels
compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
specifications.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../bindings/display/panel/panel-lvds.txt          | 120 +++++++++++++++++++++
 1 file changed, 120 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/panel-lvds.txt

diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
new file mode 100644
index 000000000000..b938269f841e
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
@@ -0,0 +1,120 @@
+LVDS Display Panel
+==================
+
+LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. Multiple
+incompatible data link layers have been used over time to transmit image data
+to LVDS panels. This bindings supports display panels compatible with the
+following specifications.
+
+[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999, February
+1999 (Version 1.0), Japan Electronic Industry Development Association (JEIDA)
+[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
+Semiconductor
+[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
+Electronics Standards Association (VESA)
+
+Device compatible with those specifications have been marketed under the
+FPD-Link and FlatLink brands.
+
+
+Required properties:
+
+- compatible: Shall contain "panel-lvds" in addition to a mandatory
+  panel-specific compatible string defined in individual panel bindings. The
+  "panel-lvds" value shall never be used on its own.
+- width-mm: See panel-common.txt.
+- height-mm: See panel-common.txt.
+- data-mapping: The color signals mapping order, "jeida-18", "jeida-24"
+  or "vesa-24".
+
+Optional properties:
+
+- label: See panel-common.txt.
+- gpios: See panel-common.txt.
+- backlight: See panel-common.txt.
+- data-mirror: If set, reverse the bit order described in the data mappings
+  below on all data lanes, transmitting bits for slots 6 to 0 instead of
+  0 to 6.
+
+Required nodes:
+
+- panel-timing: See panel-common.txt.
+- ports: See panel-common.txt. These bindings require a single port subnode
+  corresponding to the panel LVDS input.
+
+
+LVDS data mappings are defined as follows.
+
+- "jeida-18" - 18-bit data mapping compatible with the [JEIDA], [LDI] and
+  [VESA] specifications. Data are transferred as follows on 3 LVDS lanes.
+
+Slot	    0       1       2       3       4       5       6
+	________________                         _________________
+Clock	                \_______________________/
+	  ______  ______  ______  ______  ______  ______  ______
+DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
+DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
+DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><
+
+- "jeida-24" - 24-bit data mapping compatible with the [DSIM] and [LDI]
+  specifications. Data are transferred as follows on 4 LVDS lanes.
+
+Slot	    0       1       2       3       4       5       6
+	________________                         _________________
+Clock	                \_______________________/
+	  ______  ______  ______  ______  ______  ______  ______
+DATA0	><__G2__><__R7__><__R6__><__R5__><__R4__><__R3__><__R2__><
+DATA1	><__B3__><__B2__><__G7__><__G6__><__G5__><__G4__><__G3__><
+DATA2	><_CTL2_><_CTL1_><_CTL0_><__B7__><__B6__><__B5__><__B4__><
+DATA3	><_CTL3_><__B1__><__B0__><__G1__><__G0__><__R1__><__R0__><
+
+- "vesa-24" - 24-bit data mapping compatible with the [VESA] specification.
+  Data are transferred as follows on 4 LVDS lanes.
+
+Slot	    0       1       2       3       4       5       6
+	________________                         _________________
+Clock	                \_______________________/
+	  ______  ______  ______  ______  ______  ______  ______
+DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
+DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
+DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><
+DATA3	><_CTL3_><__B7__><__B6__><__G7__><__G6__><__R7__><__R6__><
+
+Control signals are mapped as follows.
+
+CTL0: HSync
+CTL1: VSync
+CTL2: Data Enable
+CTL3: 0
+
+
+Example
+-------
+
+panel {
+	compatible = "mitsubishi,aa121td01", "panel-lvds";
+
+	width-mm = <261>;
+	height-mm = <163>;
+
+	data-mapping = "jeida-24";
+
+	panel-timing {
+		/* 1280x800 @60Hz */
+		clock-frequency = <71000000>;
+		hactive = <1280>;
+		vactive = <800>;
+		hsync-len = <70>;
+		hfront-porch = <20>;
+		hback-porch = <70>;
+		vsync-len = <5>;
+		vfront-porch = <3>;
+		vback-porch = <15>;
+	};
+
+	port {
+		panel_in: endpoint {
+			remote-endpoint = <&lvds_encoder>;
+		};
+	};
+};
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 02/13] devicetree/bindings: display: Add bindings for LVDS panels

[Rob Herring]

On Sat, Nov 19, 2016 at 05:28:02AM +0200, Laurent Pinchart wrote:
> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> Multiple incompatible data link layers have been used over time to
> transmit image data to LVDS panels. This binding supports display panels
> compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> specifications.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../bindings/display/panel/panel-lvds.txt          | 120 +++++++++++++++++++++
>  1 file changed, 120 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-lvds.txt

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v2 02/13] devicetree/bindings: display: Add bindings for LVDS panels

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3191 bytes --]

On Sat, Nov 19, 2016 at 05:28:02AM +0200, Laurent Pinchart wrote:
> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> Multiple incompatible data link layers have been used over time to
> transmit image data to LVDS panels. This binding supports display panels
> compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> specifications.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../bindings/display/panel/panel-lvds.txt          | 120 +++++++++++++++++++++
>  1 file changed, 120 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> new file mode 100644
> index 000000000000..b938269f841e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> @@ -0,0 +1,120 @@
> +LVDS Display Panel
> +==================
> +
> +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. Multiple
> +incompatible data link layers have been used over time to transmit image data
> +to LVDS panels. This bindings supports display panels compatible with the
> +following specifications.
> +
> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999, February
> +1999 (Version 1.0), Japan Electronic Industry Development Association (JEIDA)
> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
> +Semiconductor
> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
> +Electronics Standards Association (VESA)
> +
> +Device compatible with those specifications have been marketed under the
> +FPD-Link and FlatLink brands.
> +
> +
> +Required properties:
> +
> +- compatible: Shall contain "panel-lvds" in addition to a mandatory
> +  panel-specific compatible string defined in individual panel bindings. The
> +  "panel-lvds" value shall never be used on its own.

What good is it if it shall never be used on its own? The above sounds
to me like the panel-specific compatible string implies the LVDS
binding, in a way that many compatible strings imply the simple binding.
Note that initially we did the very same thing with "panel-simple", only
to realize that it's completely redundant because it is never used.

> +- width-mm: See panel-common.txt.
> +- height-mm: See panel-common.txt.
> +- data-mapping: The color signals mapping order, "jeida-18", "jeida-24"
> +  or "vesa-24".
> +
> +Optional properties:
> +
> +- label: See panel-common.txt.
> +- gpios: See panel-common.txt.
> +- backlight: See panel-common.txt.
> +- data-mirror: If set, reverse the bit order described in the data mappings
> +  below on all data lanes, transmitting bits for slots 6 to 0 instead of
> +  0 to 6.
> +
> +Required nodes:
> +
> +- panel-timing: See panel-common.txt.
> +- ports: See panel-common.txt. These bindings require a single port subnode
> +  corresponding to the panel LVDS input.

Looks like I should go read the patch that introduces panel-common.txt
first...

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v2 02/13] devicetree/bindings: display: Add bindings for LVDS panels

[Laurent Pinchart]

Hi Thierry,

On Tuesday 22 Nov 2016 12:02:41 Thierry Reding wrote:
> On Sat, Nov 19, 2016 at 05:28:02AM +0200, Laurent Pinchart wrote:
> > LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> > Multiple incompatible data link layers have been used over time to
> > transmit image data to LVDS panels. This binding supports display panels
> > compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> > specifications.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  .../bindings/display/panel/panel-lvds.txt          | 120 ++++++++++++++++
> >  1 file changed, 120 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/display/panel/panel-lvds.txt> 
> > diff --git
> > a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> > b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt new file
> > mode 100644
> > index 000000000000..b938269f841e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> > @@ -0,0 +1,120 @@
> > +LVDS Display Panel
> > +==================
> > +
> > +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> > Multiple +incompatible data link layers have been used over time to
> > transmit image data +to LVDS panels. This bindings supports display
> > panels compatible with the +following specifications.
> > +
> > +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
> > February
> > +1999 (Version 1.0), Japan Electronic Industry Development Association
> > (JEIDA) +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95),
> > National +Semiconductor
> > +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
> > +Electronics Standards Association (VESA)
> > +
> > +Device compatible with those specifications have been marketed under the
> > +FPD-Link and FlatLink brands.
> > +
> > +
> > +Required properties:
> > +
> > +- compatible: Shall contain "panel-lvds" in addition to a mandatory
> > +  panel-specific compatible string defined in individual panel bindings.
> > The
> > +  "panel-lvds" value shall never be used on its own.
> 
> What good is it if it shall never be used on its own? The above sounds
> to me like the panel-specific compatible string implies the LVDS
> binding, in a way that many compatible strings imply the simple binding.
> Note that initially we did the very same thing with "panel-simple", only
> to realize that it's completely redundant because it is never used.

DT allows specifying multiple compatible strings in decreasing order of 
genericity to make generic OS implementations possible while retaining the 
ability to later introduce device-specific code if/when the need arises 
(mostly because of information that were overlooked, misunderstood or just not 
available at implementation time - we unfortunately can't produce 100% perfect 
solutions all the time, I very much regret that). This is exactly what the 
LVDS panel bindings mandate.

> > +- width-mm: See panel-common.txt.
> > +- height-mm: See panel-common.txt.
> > +- data-mapping: The color signals mapping order, "jeida-18", "jeida-24"
> > +  or "vesa-24".
> > +
> > +Optional properties:
> > +
> > +- label: See panel-common.txt.
> > +- gpios: See panel-common.txt.
> > +- backlight: See panel-common.txt.
> > +- data-mirror: If set, reverse the bit order described in the data
> > mappings
> > +  below on all data lanes, transmitting bits for slots 6 to 0 instead of
> > +  0 to 6.
> > +
> > +Required nodes:
> > +
> > +- panel-timing: See panel-common.txt.
> > +- ports: See panel-common.txt. These bindings require a single port
> > subnode
> > +  corresponding to the panel LVDS input.
> 
> Looks like I should go read the patch that introduces panel-common.txt
> first...

-- 
Regards,

Laurent Pinchart

[PATCH v2 03/13] devicetree/bindings: display: Add bindings for two Mitsubishi panels

[Laurent Pinchart]

The AA104XD12 and AA121TD01 are LVDS display panels. Their bindings are
modelled on the the LVS panel bindings.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../display/panel/mitsubishi,aa104xd12.txt         | 47 ++++++++++++++++++++++
 .../display/panel/mitsubishi,aa121td01.txt         | 47 ++++++++++++++++++++++
 2 files changed, 94 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.txt
 create mode 100644 Documentation/devicetree/bindings/display/panel/mitsubishi,aa121td01.txt

diff --git a/Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.txt b/Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.txt
new file mode 100644
index 000000000000..ced0121aed7d
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.txt
@@ -0,0 +1,47 @@
+Mitsubishi AA204XD12 LVDS Display Panel
+=======================================
+
+The AA104XD12 is a 10.4" XGA TFT-LCD display panel.
+
+These DT bindings follow the LVDS panel bindings defined in panel-lvds.txt
+with the following device-specific properties.
+
+
+Required properties:
+
+- compatible: Shall contain "mitsubishi,aa121td01" and "panel-lvds", in that
+  order.
+- vcc-supply: Reference to the regulator powering the panel VCC pins.
+
+
+Example
+-------
+
+panel {
+	compatible = "mitsubishi,aa104xd12", "panel-lvds";
+	vcc-supply = <&vcc_3v3>;
+
+	width-mm = <210>;
+	height-mm = <158>;
+
+	data-mapping = "jeida-24";
+
+	panel-timing {
+		/* 1024x768 @65Hz */
+		clock-frequency = <65000000>;
+		hactive = <1024>;
+		vactive = <768>;
+		hsync-len = <136>;
+		hfront-porch = <20>;
+		hback-porch = <160>;
+		vfront-porch = <3>;
+		vback-porch = <29>;
+		vsync-len = <6>;
+	};
+
+	port {
+		panel_in: endpoint {
+			remote-endpoint = <&lvds_encoder>;
+		};
+	};
+};
diff --git a/Documentation/devicetree/bindings/display/panel/mitsubishi,aa121td01.txt b/Documentation/devicetree/bindings/display/panel/mitsubishi,aa121td01.txt
new file mode 100644
index 000000000000..d6e1097504fe
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/mitsubishi,aa121td01.txt
@@ -0,0 +1,47 @@
+Mitsubishi AA121TD01 LVDS Display Panel
+=======================================
+
+The AA121TD01 is a 12.1" WXGA TFT-LCD display panel.
+
+These DT bindings follow the LVDS panel bindings defined in panel-lvds.txt
+with the following device-specific properties.
+
+
+Required properties:
+
+- compatible: Shall contain "mitsubishi,aa121td01" and "panel-lvds", in that
+  order.
+- vcc-supply: Reference to the regulator powering the panel VCC pins.
+
+
+Example
+-------
+
+panel {
+	compatible = "mitsubishi,aa121td01", "panel-lvds";
+	vcc-supply = <&vcc_3v3>;
+
+	width-mm = <261>;
+	height-mm = <163>;
+
+	data-mapping = "jeida-24";
+
+	panel-timing {
+		/* 1280x800 @60Hz */
+		clock-frequency = <71000000>;
+		hactive = <1280>;
+		vactive = <800>;
+		hsync-len = <70>;
+		hfront-porch = <20>;
+		hback-porch = <70>;
+		vsync-len = <5>;
+		vfront-porch = <3>;
+		vback-porch = <15>;
+	};
+
+	port {
+		panel_in: endpoint {
+			remote-endpoint = <&lvds_encoder>;
+		};
+	};
+};
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 03/13] devicetree/bindings: display: Add bindings for two Mitsubishi panels

[Rob Herring]

On Sat, Nov 19, 2016 at 05:28:03AM +0200, Laurent Pinchart wrote:
> The AA104XD12 and AA121TD01 are LVDS display panels. Their bindings are
> modelled on the the LVS panel bindings.

s/LVS/LVDS/

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../display/panel/mitsubishi,aa104xd12.txt         | 47 ++++++++++++++++++++++
>  .../display/panel/mitsubishi,aa121td01.txt         | 47 ++++++++++++++++++++++
>  2 files changed, 94 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.txt
>  create mode 100644 Documentation/devicetree/bindings/display/panel/mitsubishi,aa121td01.txt

With that,

Acked-by: Rob Herring <robh@kernel.org>

[PATCH v2 04/13] drm: Add data mirror bus flag

[Laurent Pinchart]

The flag indicates that data is mirrored on the bus. The exact meaning
is bus-type dependent. For LVDS buses it indicates that the seven data
bits that transmitted in a clock pulse are sent in slots 6 to 0 order.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 include/drm/drm_connector.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index ac9d7d8e0e43..5c1dda236760 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -159,6 +159,8 @@ struct drm_display_info {
 #define DRM_BUS_FLAG_PIXDATA_POSEDGE	(1<<2)
 /* drive data on neg. edge */
 #define DRM_BUS_FLAG_PIXDATA_NEGEDGE	(1<<3)
+/* data is mirrored on the bus */
+#define DRM_BUS_FLAG_DATA_MIRROR	(1<<4)
 
 	/**
 	 * @bus_flags: Additional information (like pixel signal polarity) for
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 04/13] drm: Add data mirror bus flag

[Laurent Pinchart]

Hi Stefan and Thierry,

As the author and suggester of the other bus flags, could you please review 
this patch ?

On Saturday 19 Nov 2016 05:28:04 Laurent Pinchart wrote:
> The flag indicates that data is mirrored on the bus. The exact meaning
> is bus-type dependent. For LVDS buses it indicates that the seven data
> bits that transmitted in a clock pulse are sent in slots 6 to 0 order.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  include/drm/drm_connector.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index ac9d7d8e0e43..5c1dda236760 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -159,6 +159,8 @@ struct drm_display_info {
>  #define DRM_BUS_FLAG_PIXDATA_POSEDGE	(1<<2)
>  /* drive data on neg. edge */
>  #define DRM_BUS_FLAG_PIXDATA_NEGEDGE	(1<<3)
> +/* data is mirrored on the bus */
> +#define DRM_BUS_FLAG_DATA_MIRROR	(1<<4)
> 
>  	/**
>  	 * @bus_flags: Additional information (like pixel signal polarity) for

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 04/13] drm: Add data mirror bus flag

[Stefan Agner]

Hi Laurent,

On 2016-12-18 21:31, Laurent Pinchart wrote:
> Hi Stefan and Thierry,
> 
> As the author and suggester of the other bus flags, could you please review 
> this patch ?

It looks to me like an appropriate use case for the flag. One remark
below:

> 
> On Saturday 19 Nov 2016 05:28:04 Laurent Pinchart wrote:
>> The flag indicates that data is mirrored on the bus. The exact meaning
>> is bus-type dependent. For LVDS buses it indicates that the seven data
>> bits that transmitted in a clock pulse are sent in slots 6 to 0 order.
>>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> ---
>>  include/drm/drm_connector.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index ac9d7d8e0e43..5c1dda236760 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -159,6 +159,8 @@ struct drm_display_info {
>>  #define DRM_BUS_FLAG_PIXDATA_POSEDGE	(1<<2)
>>  /* drive data on neg. edge */
>>  #define DRM_BUS_FLAG_PIXDATA_NEGEDGE	(1<<3)
>> +/* data is mirrored on the bus */
>> +#define DRM_BUS_FLAG_DATA_MIRROR	(1<<4)

Sounds like a bit endianness issue. I am wondering is if "mirror" is a
good term. Can we name the possible orderings? How about:

DRM_BUS_FLAG_DATA_MSB_TO_LSB
DRM_BUS_FLAG_DATA_LSB_TO_MSB 

--
Stefan

>>
>>  	/**
>>  	 * @bus_flags: Additional information (like pixel signal polarity) for

Re: [PATCH v2 04/13] drm: Add data mirror bus flag

[Laurent Pinchart]

Hi Stefan,

Thank you for the review.

On Tuesday 20 Dec 2016 14:01:46 Stefan Agner wrote:
> On 2016-12-18 21:31, Laurent Pinchart wrote:
> > Hi Stefan and Thierry,
> > 
> > As the author and suggester of the other bus flags, could you please
> > review this patch ?
> 
> It looks to me like an appropriate use case for the flag. One remark
> below:
>
> > On Saturday 19 Nov 2016 05:28:04 Laurent Pinchart wrote:
> >> The flag indicates that data is mirrored on the bus. The exact meaning
> >> is bus-type dependent. For LVDS buses it indicates that the seven data
> >> bits that transmitted in a clock pulse are sent in slots 6 to 0 order.
> >> 
> >> Signed-off-by: Laurent Pinchart
> >> <laurent.pinchart+renesas@ideasonboard.com>
> >> ---
> >> 
> >>  include/drm/drm_connector.h | 2 ++
> >>  1 file changed, 2 insertions(+)
> >> 
> >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> >> index ac9d7d8e0e43..5c1dda236760 100644
> >> --- a/include/drm/drm_connector.h
> >> +++ b/include/drm/drm_connector.h
> >> @@ -159,6 +159,8 @@ struct drm_display_info {
> >> 
> >>  #define DRM_BUS_FLAG_PIXDATA_POSEDGE	(1<<2)
> >>  /* drive data on neg. edge */
> >>  #define DRM_BUS_FLAG_PIXDATA_NEGEDGE	(1<<3)
> >> 
> >> +/* data is mirrored on the bus */
> >> +#define DRM_BUS_FLAG_DATA_MIRROR	(1<<4)
> 
> Sounds like a bit endianness issue. I am wondering is if "mirror" is a
> good term. Can we name the possible orderings? How about:
> 
> DRM_BUS_FLAG_DATA_MSB_TO_LSB
> DRM_BUS_FLAG_DATA_LSB_TO_MSB

LVDS display buses send pixels in RGB666 or RGB888 as follows.

- "jeida-18" - 18-bit data mapping compatible with the [JEIDA], [LDI] and
  [VESA] specifications. Data are transferred as follows on 3 LVDS lanes.

Slot	    0       1       2       3       4       5       6
	________________                         _________________
Clock	                \_______________________/
	  ______  ______  ______  ______  ______  ______  ______
DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><

- "jeida-24" - 24-bit data mapping compatible with the [DSIM] and [LDI]
  specifications. Data are transferred as follows on 4 LVDS lanes.

Slot	    0       1       2       3       4       5       6
	________________                         _________________
Clock	                \_______________________/
	  ______  ______  ______  ______  ______  ______  ______
DATA0	><__G2__><__R7__><__R6__><__R5__><__R4__><__R3__><__R2__><
DATA1	><__B3__><__B2__><__G7__><__G6__><__G5__><__G4__><__G3__><
DATA2	><_CTL2_><_CTL1_><_CTL0_><__B7__><__B6__><__B5__><__B4__><
DATA3	><_CTL3_><__B1__><__B0__><__G1__><__G0__><__R1__><__R0__><

- "vesa-24" - 24-bit data mapping compatible with the [VESA] specification.
  Data are transferred as follows on 4 LVDS lanes.

Slot	    0       1       2       3       4       5       6
	________________                         _________________
Clock	                \_______________________/
	  ______  ______  ______  ______  ______  ______  ______
DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><
DATA3	><_CTL3_><__B7__><__B6__><__G7__><__G6__><__R7__><__R6__><


Mirroring flips slots 0 to 6, resulting in a sort of LSB to MSB transmission 
(I'm not sure I'd call that endianness though). I'm fine renaming the flag as 
you propose. Do we need two flags, or should we assume MSB to LSB by default 
and add a single flag ?

> >>  	/**
> >>  	 * @bus_flags: Additional information (like pixel signal polarity) for

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 04/13] drm: Add data mirror bus flag

[Stefan Agner]

On 2016-12-20 14:21, Laurent Pinchart wrote:
> Hi Stefan,
> 
> Thank you for the review.
> 
> On Tuesday 20 Dec 2016 14:01:46 Stefan Agner wrote:
>> On 2016-12-18 21:31, Laurent Pinchart wrote:
>> > Hi Stefan and Thierry,
>> >
>> > As the author and suggester of the other bus flags, could you please
>> > review this patch ?
>>
>> It looks to me like an appropriate use case for the flag. One remark
>> below:
>>
>> > On Saturday 19 Nov 2016 05:28:04 Laurent Pinchart wrote:
>> >> The flag indicates that data is mirrored on the bus. The exact meaning
>> >> is bus-type dependent. For LVDS buses it indicates that the seven data
>> >> bits that transmitted in a clock pulse are sent in slots 6 to 0 order.
>> >>
>> >> Signed-off-by: Laurent Pinchart
>> >> <laurent.pinchart+renesas@ideasonboard.com>
>> >> ---
>> >>
>> >>  include/drm/drm_connector.h | 2 ++
>> >>  1 file changed, 2 insertions(+)
>> >>
>> >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> >> index ac9d7d8e0e43..5c1dda236760 100644
>> >> --- a/include/drm/drm_connector.h
>> >> +++ b/include/drm/drm_connector.h
>> >> @@ -159,6 +159,8 @@ struct drm_display_info {
>> >>
>> >>  #define DRM_BUS_FLAG_PIXDATA_POSEDGE	(1<<2)
>> >>  /* drive data on neg. edge */
>> >>  #define DRM_BUS_FLAG_PIXDATA_NEGEDGE	(1<<3)
>> >>
>> >> +/* data is mirrored on the bus */
>> >> +#define DRM_BUS_FLAG_DATA_MIRROR	(1<<4)
>>
>> Sounds like a bit endianness issue. I am wondering is if "mirror" is a
>> good term. Can we name the possible orderings? How about:
>>
>> DRM_BUS_FLAG_DATA_MSB_TO_LSB
>> DRM_BUS_FLAG_DATA_LSB_TO_MSB
> 
> LVDS display buses send pixels in RGB666 or RGB888 as follows.
> 
> - "jeida-18" - 18-bit data mapping compatible with the [JEIDA], [LDI] and
>   [VESA] specifications. Data are transferred as follows on 3 LVDS lanes.
> 
> Slot	    0       1       2       3       4       5       6
> 	________________                         _________________
> Clock	                \_______________________/
> 	  ______  ______  ______  ______  ______  ______  ______
> DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
> DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
> DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><
> 
> - "jeida-24" - 24-bit data mapping compatible with the [DSIM] and [LDI]
>   specifications. Data are transferred as follows on 4 LVDS lanes.
> 
> Slot	    0       1       2       3       4       5       6
> 	________________                         _________________
> Clock	                \_______________________/
> 	  ______  ______  ______  ______  ______  ______  ______
> DATA0	><__G2__><__R7__><__R6__><__R5__><__R4__><__R3__><__R2__><
> DATA1	><__B3__><__B2__><__G7__><__G6__><__G5__><__G4__><__G3__><
> DATA2	><_CTL2_><_CTL1_><_CTL0_><__B7__><__B6__><__B5__><__B4__><
> DATA3	><_CTL3_><__B1__><__B0__><__G1__><__G0__><__R1__><__R0__><
> 
> - "vesa-24" - 24-bit data mapping compatible with the [VESA] specification.
>   Data are transferred as follows on 4 LVDS lanes.
> 
> Slot	    0       1       2       3       4       5       6
> 	________________                         _________________
> Clock	                \_______________________/
> 	  ______  ______  ______  ______  ______  ______  ______
> DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
> DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
> DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><
> DATA3	><_CTL3_><__B7__><__B6__><__G7__><__G6__><__R7__><__R6__><
> 
> 
> Mirroring flips slots 0 to 6, resulting in a sort of LSB to MSB transmission 
> (I'm not sure I'd call that endianness though). I'm fine renaming the flag as 
> you propose. Do we need two flags, or should we assume MSB to LSB by default 
> and add a single flag ?

Wikipedia has a chapter about bit endianness, but I agree, I would not
call it that way; the term endianness usually refers to byte endianness.

For the other flags we usually have both variants. One variant is the
(unwritten) default for most displays (and most drivers/display
controllers configure it that way). And most displays only specify a
flag if they require the non-default setting.

I suggest to add both, just for the sake of completeness.

--
Stefan


> 
>> >>  	/**
>> >>  	 * @bus_flags: Additional information (like pixel signal polarity) for

[PATCH v2.1 04/13] drm: Add data transmission order bus flag

[Laurent Pinchart]

The flags indicate whether data is transmitted lsb to msb or msb to lsb
on the bus.

The exact meaning is bus-type dependent. For instance, for LVDS buses
the flags indicate whether the seven data bits transmitted in a clock
pulse are sent in normal order (msb to lsb, slots 0 to 6) or reverse
order (lsb to msb, slots 6 to 0).

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
Changes since v2:

- Rename the flag to DRM_BUS_FLAG_DATA_LSB_TO_MSB and add a
  corresponding DRM_BUS_FLAG_DATA_MSB_TO_LSB flag.
---
 include/drm/drm_connector.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index a9b95246e26e..712f255577ea 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -160,6 +160,10 @@ struct drm_display_info {
 #define DRM_BUS_FLAG_PIXDATA_POSEDGE	(1<<2)
 /* drive data on neg. edge */
 #define DRM_BUS_FLAG_PIXDATA_NEGEDGE	(1<<3)
+/* data is transmitted msb to lsb on the bus */
+#define DRM_BUS_FLAG_DATA_MSB_TO_LSB	(1<<4)
+/* data is transmitted lsb to msb on the bus */
+#define DRM_BUS_FLAG_DATA_LSB_TO_MSB	(1<<5)
 
 	/**
 	 * @bus_flags: Additional information (like pixel signal polarity) for
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2.1 04/13] drm: Add data transmission order bus flag

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1566 bytes --]

On Wed, Jan 04, 2017 at 02:39:26AM +0200, Laurent Pinchart wrote:
> The flags indicate whether data is transmitted lsb to msb or msb to lsb
> on the bus.
> 
> The exact meaning is bus-type dependent. For instance, for LVDS buses
> the flags indicate whether the seven data bits transmitted in a clock
> pulse are sent in normal order (msb to lsb, slots 0 to 6) or reverse
> order (lsb to msb, slots 6 to 0).
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> Changes since v2:
> 
> - Rename the flag to DRM_BUS_FLAG_DATA_LSB_TO_MSB and add a
>   corresponding DRM_BUS_FLAG_DATA_MSB_TO_LSB flag.
> ---
>  include/drm/drm_connector.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index a9b95246e26e..712f255577ea 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -160,6 +160,10 @@ struct drm_display_info {
>  #define DRM_BUS_FLAG_PIXDATA_POSEDGE	(1<<2)
>  /* drive data on neg. edge */
>  #define DRM_BUS_FLAG_PIXDATA_NEGEDGE	(1<<3)
> +/* data is transmitted msb to lsb on the bus */
> +#define DRM_BUS_FLAG_DATA_MSB_TO_LSB	(1<<4)
> +/* data is transmitted lsb to msb on the bus */
> +#define DRM_BUS_FLAG_DATA_LSB_TO_MSB	(1<<5)

Nit: "LSB" and "MSB" because they're abbreviations. If I end up applying
this I'll probably do that myself, and I leave it up to whoever else
might apply it whether or not they want to be pedantic, so:

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2.1 04/13] drm: Add data transmission order bus flag

[Laurent Pinchart]

Hi Thierry,

On Wednesday 04 Jan 2017 08:06:54 Thierry Reding wrote:
> On Wed, Jan 04, 2017 at 02:39:26AM +0200, Laurent Pinchart wrote:
> > The flags indicate whether data is transmitted lsb to msb or msb to lsb
> > on the bus.
> > 
> > The exact meaning is bus-type dependent. For instance, for LVDS buses
> > the flags indicate whether the seven data bits transmitted in a clock
> > pulse are sent in normal order (msb to lsb, slots 0 to 6) or reverse
> > order (lsb to msb, slots 6 to 0).
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > Changes since v2:
> > 
> > - Rename the flag to DRM_BUS_FLAG_DATA_LSB_TO_MSB and add a
> > 
> >   corresponding DRM_BUS_FLAG_DATA_MSB_TO_LSB flag.
> > 
> > ---
> > 
> >  include/drm/drm_connector.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index a9b95246e26e..712f255577ea 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -160,6 +160,10 @@ struct drm_display_info {
> > 
> >  #define DRM_BUS_FLAG_PIXDATA_POSEDGE	(1<<2)
> >  /* drive data on neg. edge */
> >  #define DRM_BUS_FLAG_PIXDATA_NEGEDGE	(1<<3)
> > 
> > +/* data is transmitted msb to lsb on the bus */
> > +#define DRM_BUS_FLAG_DATA_MSB_TO_LSB	(1<<4)
> > +/* data is transmitted lsb to msb on the bus */
> > +#define DRM_BUS_FLAG_DATA_LSB_TO_MSB	(1<<5)
> 
> Nit: "LSB" and "MSB" because they're abbreviations. If I end up applying
> this I'll probably do that myself, and I leave it up to whoever else
> might apply it whether or not they want to be pedantic, so:
> 
> Reviewed-by: Thierry Reding <treding@nvidia.com>

Thank you. I'll fix that in my tree too. I went for lower case because bit is 
usually abbreviated b and byte B, but I don't think it really conveys that 
meaning anyway :-)

-- 
Regards,

Laurent Pinchart

[PATCH v2 05/13] drm: panels: Constify device node argument to of_drm_find_panel()

[Laurent Pinchart]

The argument is never modified by the function, make it const.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/drm_panel.c | 2 +-
 include/drm/drm_panel.h     | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index 3dfe3c886502..308d442a531b 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -137,7 +137,7 @@ EXPORT_SYMBOL(drm_panel_detach);
  * Return: A pointer to the panel registered for the specified device tree
  * node or NULL if no panel matching the device tree node can be found.
  */
-struct drm_panel *of_drm_find_panel(struct device_node *np)
+struct drm_panel *of_drm_find_panel(const struct device_node *np)
 {
 	struct drm_panel *panel;
 
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 220d1e2b3db1..4b76cf2d5a7b 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -193,9 +193,9 @@ int drm_panel_attach(struct drm_panel *panel, struct drm_connector *connector);
 int drm_panel_detach(struct drm_panel *panel);
 
 #ifdef CONFIG_OF
-struct drm_panel *of_drm_find_panel(struct device_node *np);
+struct drm_panel *of_drm_find_panel(const struct device_node *np);
 #else
-static inline struct drm_panel *of_drm_find_panel(struct device_node *np)
+static inline struct drm_panel *of_drm_find_panel(const struct device_node *np)
 {
 	return NULL;
 }
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 05/13] drm: panels: Constify device node argument to of_drm_find_panel()

[Laurent Pinchart]

Hi Thierry,

Could you please review this patch ? Should it be merged through you ?

On Saturday 19 Nov 2016 05:28:05 Laurent Pinchart wrote:
> The argument is never modified by the function, make it const.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_panel.c | 2 +-
>  include/drm/drm_panel.h     | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index 3dfe3c886502..308d442a531b 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -137,7 +137,7 @@ EXPORT_SYMBOL(drm_panel_detach);
>   * Return: A pointer to the panel registered for the specified device tree
>   * node or NULL if no panel matching the device tree node can be found.
>   */
> -struct drm_panel *of_drm_find_panel(struct device_node *np)
> +struct drm_panel *of_drm_find_panel(const struct device_node *np)
>  {
>  	struct drm_panel *panel;
> 
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 220d1e2b3db1..4b76cf2d5a7b 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -193,9 +193,9 @@ int drm_panel_attach(struct drm_panel *panel, struct
> drm_connector *connector); int drm_panel_detach(struct drm_panel *panel);
> 
>  #ifdef CONFIG_OF
> -struct drm_panel *of_drm_find_panel(struct device_node *np);
> +struct drm_panel *of_drm_find_panel(const struct device_node *np);
>  #else
> -static inline struct drm_panel *of_drm_find_panel(struct device_node *np)
> +static inline struct drm_panel *of_drm_find_panel(const struct device_node
> *np) {
>  	return NULL;
>  }

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 05/13] drm: panels: Constify device node argument to of_drm_find_panel()

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 387 bytes --]

On Sat, Nov 19, 2016 at 05:28:05AM +0200, Laurent Pinchart wrote:
> The argument is never modified by the function, make it const.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_panel.c | 2 +-
>  include/drm/drm_panel.h     | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

Applied, thanks.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 06/13] drm: panels: Add LVDS panel driver

[Laurent Pinchart]

This driver supports LVDS panels that don't require device-specific
handling of power supplies or control signals. It implements automatic
backlight handling if the panel is attached to a backlight controller.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/panel/Kconfig      |  10 ++
 drivers/gpu/drm/panel/Makefile     |   1 +
 drivers/gpu/drm/panel/panel-lvds.c | 284 +++++++++++++++++++++++++++++++++++++
 3 files changed, 295 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-lvds.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 62aba976e744..5dc2106da2bc 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -7,6 +7,16 @@ config DRM_PANEL
 menu "Display Panels"
 	depends on DRM && DRM_PANEL
 
+config DRM_PANEL_LVDS
+	tristate "Generic LVDS panel driver"
+	depends on OF
+	depends on BACKLIGHT_CLASS_DEVICE
+	select VIDEOMODE_HELPERS
+	help
+	  This driver supports LVDS panels that don't require device-specific
+	  handling of power supplies or control signals. It implements automatic
+	  backlight handling if the panel is attached to a backlight controller.
+
 config DRM_PANEL_SIMPLE
 	tristate "support for simple panels"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index a5c7ec0236e0..20b5060d1f47 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -1,3 +1,4 @@
+obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o
 obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
 obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
 obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
diff --git a/drivers/gpu/drm/panel/panel-lvds.c b/drivers/gpu/drm/panel/panel-lvds.c
new file mode 100644
index 000000000000..0a563a287abe
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-lvds.c
@@ -0,0 +1,284 @@
+/*
+ * rcar_du_crtc.c  --  R-Car Display Unit CRTCs
+ *
+ * Copyright (C) 2016 Laurent Pinchart
+ * Copyright (C) 2016 Renesas Electronics Corporation
+ *
+ * Contact: Laurent Pinchart (laurent.pinchart@ideasonboard.com)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/backlight.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include <drm/drmP.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_panel.h>
+
+#include <video/display_timing.h>
+#include <video/of_display_timing.h>
+#include <video/videomode.h>
+
+struct panel_lvds {
+	struct drm_panel panel;
+	struct device *dev;
+
+	const char *label;
+	unsigned int width;
+	unsigned int height;
+	struct videomode video_mode;
+	unsigned int bus_format;
+	bool data_mirror;
+
+	struct backlight_device *backlight;
+
+	struct gpio_desc *enable_gpio;
+	struct gpio_desc *reset_gpio;
+};
+
+static inline struct panel_lvds *to_panel_lvds(struct drm_panel *panel)
+{
+	return container_of(panel, struct panel_lvds, panel);
+}
+
+static int panel_lvds_disable(struct drm_panel *panel)
+{
+	struct panel_lvds *lvds = to_panel_lvds(panel);
+
+	if (lvds->backlight) {
+		lvds->backlight->props.power = FB_BLANK_POWERDOWN;
+		lvds->backlight->props.state |= BL_CORE_FBBLANK;
+		backlight_update_status(lvds->backlight);
+	}
+
+	return 0;
+}
+
+static int panel_lvds_unprepare(struct drm_panel *panel)
+{
+	struct panel_lvds *lvds = to_panel_lvds(panel);
+
+	if (lvds->enable_gpio)
+		gpiod_set_value_cansleep(lvds->enable_gpio, 0);
+
+	return 0;
+}
+
+static int panel_lvds_prepare(struct drm_panel *panel)
+{
+	struct panel_lvds *lvds = to_panel_lvds(panel);
+
+	if (lvds->enable_gpio)
+		gpiod_set_value_cansleep(lvds->enable_gpio, 1);
+
+	return 0;
+}
+
+static int panel_lvds_enable(struct drm_panel *panel)
+{
+	struct panel_lvds *lvds = to_panel_lvds(panel);
+
+	if (lvds->backlight) {
+		lvds->backlight->props.state &= ~BL_CORE_FBBLANK;
+		lvds->backlight->props.power = FB_BLANK_UNBLANK;
+		backlight_update_status(lvds->backlight);
+	}
+
+	return 0;
+}
+
+static int panel_lvds_get_modes(struct drm_panel *panel)
+{
+	struct panel_lvds *lvds = to_panel_lvds(panel);
+	struct drm_connector *connector = lvds->panel.connector;
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_create(lvds->panel.drm);
+	if (!mode)
+		return 0;
+
+	drm_display_mode_from_videomode(&lvds->video_mode, mode);
+	mode->type |= DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	drm_mode_probed_add(connector, mode);
+
+	connector->display_info.width_mm = lvds->width;
+	connector->display_info.height_mm = lvds->height;
+	drm_display_info_set_bus_formats(&connector->display_info,
+					 &lvds->bus_format, 1);
+	connector->display_info.bus_flags = lvds->data_mirror
+					  ? DRM_BUS_FLAG_DATA_MIRROR : 0;
+
+	return 1;
+}
+
+static const struct drm_panel_funcs panel_lvds_funcs = {
+	.disable = panel_lvds_disable,
+	.unprepare = panel_lvds_unprepare,
+	.prepare = panel_lvds_prepare,
+	.enable = panel_lvds_enable,
+	.get_modes = panel_lvds_get_modes,
+};
+
+static int panel_lvds_parse_dt(struct panel_lvds *lvds)
+{
+	struct device_node *np = lvds->dev->of_node;
+	struct display_timing timing;
+	const char *mapping;
+	int ret;
+
+	ret = of_get_display_timing(np, "panel-timing", &timing);
+	if (ret < 0)
+		return ret;
+
+	videomode_from_timing(&timing, &lvds->video_mode);
+
+	ret = of_property_read_u32(np, "width-mm", &lvds->width);
+	if (ret < 0) {
+		dev_err(lvds->dev, "%s: invalid or missing %s DT property\n",
+			of_node_full_name(np), "width-mm");
+		return -ENODEV;
+	}
+	ret = of_property_read_u32(np, "height-mm", &lvds->height);
+	if (ret < 0) {
+		dev_err(lvds->dev, "%s: invalid or missing %s DT property\n",
+			of_node_full_name(np), "height-mm");
+		return -ENODEV;
+	}
+
+	of_property_read_string(np, "label", &lvds->label);
+
+	ret = of_property_read_string(np, "data-mapping", &mapping);
+	if (ret < 0) {
+		dev_err(lvds->dev, "%s: invalid or missing %s DT property\n",
+			of_node_full_name(np), "data-mapping");
+		return -ENODEV;
+	}
+
+	if (!strcmp(mapping, "jeida-18")) {
+		lvds->bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG;
+	} else if (!strcmp(mapping, "jeida-24")) {
+		lvds->bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA;
+	} else if (!strcmp(mapping, "vesa-24")) {
+		lvds->bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG;
+	} else {
+		dev_err(lvds->dev, "%s: invalid or missing %s DT property\n",
+			of_node_full_name(np), "data-mapping");
+		return -EINVAL;
+	}
+
+	lvds->data_mirror = of_property_read_bool(np, "data-mirror");
+
+	return 0;
+}
+
+static int panel_lvds_probe(struct platform_device *pdev)
+{
+	struct panel_lvds *lvds;
+	struct device_node *np;
+	int ret;
+
+	lvds = devm_kzalloc(&pdev->dev, sizeof(*lvds), GFP_KERNEL);
+	if (!lvds)
+		return -ENOMEM;
+
+	lvds->dev = &pdev->dev;
+
+	ret = panel_lvds_parse_dt(lvds);
+	if (ret < 0)
+		return ret;
+
+	/* Get GPIOs and backlight controller. */
+	lvds->enable_gpio = devm_gpiod_get_optional(lvds->dev, "enable",
+						     GPIOD_OUT_LOW);
+	if (IS_ERR(lvds->enable_gpio)) {
+		ret = PTR_ERR(lvds->enable_gpio);
+		dev_err(lvds->dev, "failed to request %s GPIO: %d\n",
+			"enable", ret);
+		return ret;
+	}
+
+	lvds->reset_gpio = devm_gpiod_get_optional(lvds->dev, "reset",
+						     GPIOD_OUT_HIGH);
+	if (IS_ERR(lvds->reset_gpio)) {
+		ret = PTR_ERR(lvds->reset_gpio);
+		dev_err(lvds->dev, "failed to request %s GPIO: %d\n",
+			"reset", ret);
+		return ret;
+	}
+
+	np = of_parse_phandle(lvds->dev->of_node, "backlight", 0);
+	if (np) {
+		lvds->backlight = of_find_backlight_by_node(np);
+		of_node_put(np);
+
+		if (!lvds->backlight)
+			return -EPROBE_DEFER;
+	}
+
+	/*
+	 * TODO: Handle all power supplies specified in the DT node in a generic
+	 * way for panels that don't care about power supply ordering. LVDS
+	 * panels that require a specific power sequence will need a dedicated
+	 * driver.
+	 */
+
+	/* Register the panel. */
+	drm_panel_init(&lvds->panel);
+	lvds->panel.dev = lvds->dev;
+	lvds->panel.funcs = &panel_lvds_funcs;
+
+	ret = drm_panel_add(&lvds->panel);
+	if (ret < 0)
+		goto error;
+
+	dev_set_drvdata(lvds->dev, lvds);
+	return 0;
+
+error:
+	put_device(&lvds->backlight->dev);
+	return ret;
+}
+
+static int panel_lvds_remove(struct platform_device *pdev)
+{
+	struct panel_lvds *lvds = dev_get_drvdata(&pdev->dev);
+
+	drm_panel_detach(&lvds->panel);
+	drm_panel_remove(&lvds->panel);
+
+	panel_lvds_disable(&lvds->panel);
+
+	if (lvds->backlight)
+		put_device(&lvds->backlight->dev);
+
+	return 0;
+}
+
+static const struct of_device_id panel_lvds_of_table[] = {
+	{ .compatible = "panel-lvds", },
+};
+
+MODULE_DEVICE_TABLE(of, rcar_du_of_table);
+
+static struct platform_driver panel_lvds_driver = {
+	.probe		= panel_lvds_probe,
+	.remove		= panel_lvds_remove,
+	.driver		= {
+		.name	= "panel-lvds",
+		.of_match_table = panel_lvds_of_table,
+	},
+};
+
+module_platform_driver(panel_lvds_driver);
+
+MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>");
+MODULE_DESCRIPTION("LVDS Panel Driver");
+MODULE_LICENSE("GPL");
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 06/13] drm: panels: Add LVDS panel driver

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 799 bytes --]

On Sat, Nov 19, 2016 at 05:28:06AM +0200, Laurent Pinchart wrote:
> This driver supports LVDS panels that don't require device-specific
> handling of power supplies or control signals. It implements automatic
> backlight handling if the panel is attached to a backlight controller.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/panel/Kconfig      |  10 ++
>  drivers/gpu/drm/panel/Makefile     |   1 +
>  drivers/gpu/drm/panel/panel-lvds.c | 284 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 295 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-lvds.c

The bulk of this is a duplicate of panel-simple.c and it adds all the
things on top that I've been repeatedly rejecting in the past.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v2 06/13] drm: panels: Add LVDS panel driver

[Laurent Pinchart]

Hi Thierry,

On Tuesday 22 Nov 2016 12:14:57 Thierry Reding wrote:
> On Sat, Nov 19, 2016 at 05:28:06AM +0200, Laurent Pinchart wrote:
> > This driver supports LVDS panels that don't require device-specific
> > handling of power supplies or control signals. It implements automatic
> > backlight handling if the panel is attached to a backlight controller.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  drivers/gpu/drm/panel/Kconfig      |  10 ++
> >  drivers/gpu/drm/panel/Makefile     |   1 +
> >  drivers/gpu/drm/panel/panel-lvds.c | 284 ++++++++++++++++++++++++++++++++
> >  3 files changed, 295 insertions(+)
> >  create mode 100644 drivers/gpu/drm/panel/panel-lvds.c
> 
> The bulk of this is a duplicate of panel-simple.c

Implementing the panel API should obviously be expected to produce some 
quantity of similar code.

> and it adds all the things on top that I've been repeatedly rejecting in the
> past.

It's a good thing I haven't tried to add them to panel-simple.c then :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 06/13] drm: panels: Add LVDS panel driver

[Laurent Pinchart]

Hi Thierry,

Ping ?

On Tuesday 22 Nov 2016 15:17:09 Laurent Pinchart wrote:
> On Tuesday 22 Nov 2016 12:14:57 Thierry Reding wrote:
> > On Sat, Nov 19, 2016 at 05:28:06AM +0200, Laurent Pinchart wrote:
> >> This driver supports LVDS panels that don't require device-specific
> >> handling of power supplies or control signals. It implements automatic
> >> backlight handling if the panel is attached to a backlight controller.
> >> 
> >> Signed-off-by: Laurent Pinchart
> >> <laurent.pinchart+renesas@ideasonboard.com>
> >> ---
> >> 
> >>  drivers/gpu/drm/panel/Kconfig      |  10 ++
> >>  drivers/gpu/drm/panel/Makefile     |   1 +
> >>  drivers/gpu/drm/panel/panel-lvds.c | 284 ++++++++++++++++++++++++++++++
> >>  3 files changed, 295 insertions(+)
> >>  create mode 100644 drivers/gpu/drm/panel/panel-lvds.c
> > 
> > The bulk of this is a duplicate of panel-simple.c
> 
> Implementing the panel API should obviously be expected to produce some
> quantity of similar code.
> 
> > and it adds all the things on top that I've been repeatedly rejecting in
> > the past.
> 
> It's a good thing I haven't tried to add them to panel-simple.c then :-)

-- 
Regards,

Laurent Pinchart

[PATCH v2 07/13] arm64: dts: r8a7795: Add PWM support

[Laurent Pinchart]

Add the 7 PWM channels to the r8a7795 device tree, in the disabled
state.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a7795.dtsi | 63 ++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index 8c15040f2540..9b717dee1f7b 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -877,6 +877,69 @@
 			status = "disabled";
 		};
 
+		pwm0: pwm@e6e30000 {
+			compatible = "renesas,pwm-r8a7795", "renesas,pwm-rcar";
+			reg = <0 0xe6e30000 0 0x8>;
+			clocks = <&cpg CPG_MOD 523>;
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			#pwm-cells = <2>;
+			status = "disabled";
+		};
+
+		pwm1: pwm@e6e31000 {
+			compatible = "renesas,pwm-r8a7795", "renesas,pwm-rcar";
+			reg = <0 0xe6e31000 0 0x8>;
+			clocks = <&cpg CPG_MOD 523>;
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			#pwm-cells = <2>;
+			status = "disabled";
+		};
+
+		pwm2: pwm@e6e32000 {
+			compatible = "renesas,pwm-r8a7795", "renesas,pwm-rcar";
+			reg = <0 0xe6e32000 0 0x8>;
+			clocks = <&cpg CPG_MOD 523>;
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			#pwm-cells = <2>;
+			status = "disabled";
+		};
+
+		pwm3: pwm@e6e33000 {
+			compatible = "renesas,pwm-r8a7795", "renesas,pwm-rcar";
+			reg = <0 0xe6e33000 0 0x8>;
+			clocks = <&cpg CPG_MOD 523>;
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			#pwm-cells = <2>;
+			status = "disabled";
+		};
+
+		pwm4: pwm@e6e34000 {
+			compatible = "renesas,pwm-r8a7795", "renesas,pwm-rcar";
+			reg = <0 0xe6e34000 0 0x8>;
+			clocks = <&cpg CPG_MOD 523>;
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			#pwm-cells = <2>;
+			status = "disabled";
+		};
+
+		pwm5: pwm@e6e35000 {
+			compatible = "renesas,pwm-r8a7795", "renesas,pwm-rcar";
+			reg = <0 0xe6e35000 0 0x8>;
+			clocks = <&cpg CPG_MOD 523>;
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			#pwm-cells = <2>;
+			status = "disabled";
+		};
+
+		pwm6: pwm@e6e36000 {
+			compatible = "renesas,pwm-r8a7795", "renesas,pwm-rcar";
+			reg = <0 0xe6e36000 0 0x8>;
+			clocks = <&cpg CPG_MOD 523>;
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			#pwm-cells = <2>;
+			status = "disabled";
+		};
+
 		rcar_sound: sound@ec500000 {
 			/*
 			 * #sound-dai-cells is required
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 07/13] arm64: dts: r8a7795: Add PWM support

[Geert Uytterhoeven]

On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> Add the 7 PWM channels to the r8a7795 device tree, in the disabled
> state.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 07/13] arm64: dts: r8a7795: Add PWM support

[Laurent Pinchart]

Hi Simon,

On Monday 21 Nov 2016 09:27:38 Geert Uytterhoeven wrote:
> On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart wrote:
> > Add the 7 PWM channels to the r8a7795 device tree, in the disabled
> > state.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

This patch is independent from the rest of the series, could you please pick 
it up ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 07/13] arm64: dts: r8a7795: Add PWM support

[Laurent Pinchart]

Hi Simon,

On Monday 21 Nov 2016 11:18:59 Laurent Pinchart wrote:
> On Monday 21 Nov 2016 09:27:38 Geert Uytterhoeven wrote:
> > On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart wrote:
> >> Add the 7 PWM channels to the r8a7795 device tree, in the disabled
> >> state.
> >> 
> >> Signed-off-by: Laurent Pinchart
> >> <laurent.pinchart+renesas@ideasonboard.com>
> > 
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> This patch is independent from the rest of the series, could you please pick
> it up ?

Ping ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 07/13] arm64: dts: r8a7795: Add PWM support

[Simon Horman]

On Wed, Jan 04, 2017 at 03:06:24AM +0200, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Monday 21 Nov 2016 11:18:59 Laurent Pinchart wrote:
> > On Monday 21 Nov 2016 09:27:38 Geert Uytterhoeven wrote:
> > > On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart wrote:
> > >> Add the 7 PWM channels to the r8a7795 device tree, in the disabled
> > >> state.
> > >> 
> > >> Signed-off-by: Laurent Pinchart
> > >> <laurent.pinchart+renesas@ideasonboard.com>
> > > 
> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > 
> > This patch is independent from the rest of the series, could you please pick
> > it up ?
> 
> Ping ?

Sorry for letting this slip through, I have queued it up.

[PATCH v2 08/13] arm64: dts: r8a7795: salvator-x: Add DU LVDS output endpoint

[Laurent Pinchart]

Declaring the endpoint makes LVDS enablement easier by just including
the corresponding panel's dtsi file.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
index b1eab6876f8c..2c30813d0f86 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
@@ -191,6 +191,10 @@
 				remote-endpoint = <&adv7123_in>;
 			};
 		};
+		port@3 {
+			lvds_connector: endpoint {
+			};
+		};
 	};
 };
 
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 08/13] arm64: dts: r8a7795: salvator-x: Add DU LVDS output endpoint

[Laurent Pinchart]

Hi Simon,

Could you please pick this patch up ?

On Saturday 19 Nov 2016 05:28:08 Laurent Pinchart wrote:
> Declaring the endpoint makes LVDS enablement easier by just including
> the corresponding panel's dtsi file.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
>  arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts index
> b1eab6876f8c..2c30813d0f86 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> @@ -191,6 +191,10 @@
>  				remote-endpoint = <&adv7123_in>;
>  			};
>  		};
> +		port@3 {
> +			lvds_connector: endpoint {
> +			};
> +		};
>  	};
>  };

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 08/13] arm64: dts: r8a7795: salvator-x: Add DU LVDS output endpoint

[Simon Horman]

On Wed, Jan 04, 2017 at 03:07:14AM +0200, Laurent Pinchart wrote:
> Hi Simon,
> 
> Could you please pick this patch up ?

Hi Laurent,

I believe this is present in v4.10-rc1 as ea3c17b03b9b ("arm64: dts:
r8a7795: salvator-x: Add DU LVDS output endpoint").

[PATCH v2 09/13] arm64: dts: r8a7795: salvator-x: Add panel backlight support

[Laurent Pinchart]

The panel backlight is controlled through a GPIO and a PWM channel.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
index 2c30813d0f86..a905318a347c 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
@@ -178,6 +178,16 @@
 			};
 		};
 	};
+
+	backlight: backlight {
+		compatible = "pwm-backlight";
+		pwms = <&pwm1 0 50000>;
+
+		brightness-levels = <256 128 64 16 8 4 0>;
+		default-brightness-level = <6>;
+
+		enable-gpios = <&gpio6 7 GPIO_ACTIVE_HIGH>;
+	};
 };
 
 &du {
@@ -238,6 +248,11 @@
 		function = "du";
 	};
 
+	pwm1_pins: pwm {
+		groups = "pwm1_a";
+		function = "pwm1";
+	};
+
 	sdhi0_pins: sd0 {
 		groups = "sdhi0_data4", "sdhi0_ctrl";
 		function = "sdhi0";
@@ -275,6 +290,13 @@
 	};
 };
 
+&pwm1 {
+	pinctrl-0 = <&pwm1_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
+
 &scif1 {
 	pinctrl-0 = <&scif1_pins>;
 	pinctrl-names = "default";
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 09/13] arm64: dts: r8a7795: salvator-x: Add panel backlight support

[Geert Uytterhoeven]

Hi Laurent,

On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> The panel backlight is controlled through a GPIO and a PWM channel.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> @@ -178,6 +178,16 @@
>                         };
>                 };
>         };
> +
> +       backlight: backlight {
> +               compatible = "pwm-backlight";
> +               pwms = <&pwm1 0 50000>;
> +
> +               brightness-levels = <256 128 64 16 8 4 0>;

Would it make sense to define more and/or linear levels?

> +               default-brightness-level = <6>;

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 09/13] arm64: dts: r8a7795: salvator-x: Add panel backlight support

[Laurent Pinchart]

Hi Geert,

On Monday 21 Nov 2016 09:36:22 Geert Uytterhoeven wrote:
> On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart wrote:
> > The panel backlight is controlled through a GPIO and a PWM channel.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> > +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> > @@ -178,6 +178,16 @@
> >                         };
> >                 };
> >         };
> > +
> > +       backlight: backlight {
> > +               compatible = "pwm-backlight";
> > +               pwms = <&pwm1 0 50000>;
> > +
> > +               brightness-levels = <256 128 64 16 8 4 0>;
> 
> Would it make sense to define more and/or linear levels?

Possibly, this is pretty arbitrary. Linear levels might not be the best option 
given that the human eye doesn't have a linear response to light power, but we 
could certainly have more levels. In that case I'd prefer modifying the pwm-
backlight DT bindings though, and specifying the PWM resolution instead of 
discrete levels.

Note that the LVDS panel backlight PWM control signal is multiplexed with the 
external memory A21 signal on the Salvator-X board, with SW5 selecting which 
how to route the signal. When using backlight control we can't access the 
whole NOR flash anymore, so I'm not sure this patch should be merged.

> > +               default-brightness-level = <6>;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 09/13] arm64: dts: r8a7795: salvator-x: Add panel backlight support

[Geert Uytterhoeven]

Hi Laurent,

On Mon, Nov 21, 2016 at 10:19 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday 21 Nov 2016 09:36:22 Geert Uytterhoeven wrote:
>> On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart wrote:
>> > The panel backlight is controlled through a GPIO and a PWM channel.

>> > --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
>> > +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
>> > @@ -178,6 +178,16 @@
>> >                         };
>> >                 };
>> >         };
>> > +
>> > +       backlight: backlight {
>> > +               compatible = "pwm-backlight";
>> > +               pwms = <&pwm1 0 50000>;
>> > +
>> > +               brightness-levels = <256 128 64 16 8 4 0>;
>>
>> Would it make sense to define more and/or linear levels?
>
> Possibly, this is pretty arbitrary. Linear levels might not be the best option
> given that the human eye doesn't have a linear response to light power, but we

It not only depends on the human eye, but also on the backlight hardware
(is the conversion from voltage (L_VBRT) to light linear?).

> could certainly have more levels. In that case I'd prefer modifying the pwm-
> backlight DT bindings though, and specifying the PWM resolution instead of
> discrete levels.
>
> Note that the LVDS panel backlight PWM control signal is multiplexed with the
> external memory A21 signal on the Salvator-X board, with SW5 selecting which
> how to route the signal. When using backlight control we can't access the
> whole NOR flash anymore, so I'm not sure this patch should be merged.

That NOR flash is also optional, right?
My Ex Memory Connector is not populated.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 09/13] arm64: dts: r8a7795: salvator-x: Add panel backlight support

[Laurent Pinchart]

Hi Geert,

On Monday 21 Nov 2016 10:23:46 Geert Uytterhoeven wrote:
> On Mon, Nov 21, 2016 at 10:19 AM, Laurent Pinchart wrote:
> > On Monday 21 Nov 2016 09:36:22 Geert Uytterhoeven wrote:
> >> On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart wrote:
> >>> The panel backlight is controlled through a GPIO and a PWM channel.
> >>> 
> >>> --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> >>> +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> >>> @@ -178,6 +178,16 @@
> >>>                         };
> >>>                 };
> >>>         };
> >>> +
> >>> +       backlight: backlight {
> >>> +               compatible = "pwm-backlight";
> >>> +               pwms = <&pwm1 0 50000>;
> >>> +
> >>> +               brightness-levels = <256 128 64 16 8 4 0>;
> >> 
> >> Would it make sense to define more and/or linear levels?
> > 
> > Possibly, this is pretty arbitrary. Linear levels might not be the best
> > option given that the human eye doesn't have a linear response to light
> > power, but we
>
> It not only depends on the human eye, but also on the backlight hardware
> (is the conversion from voltage (L_VBRT) to light linear?).

So we need to specify transfer functions in DT ;-)

> > could certainly have more levels. In that case I'd prefer modifying the
> > pwm- backlight DT bindings though, and specifying the PWM resolution
> > instead of discrete levels.
> > 
> > Note that the LVDS panel backlight PWM control signal is multiplexed with
> > the external memory A21 signal on the Salvator-X board, with SW5
> > selecting which how to route the signal. When using backlight control we
> > can't access the whole NOR flash anymore, so I'm not sure this patch
> > should be merged.
>
> That NOR flash is also optional, right?
> My Ex Memory Connector is not populated.

That's correct. The Salvator-X DT file in mainline is just an example anyway, 
and we should pick the most useful peripherals for that purpose.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 09/13] arm64: dts: r8a7795: salvator-x: Add panel backlight support

[Laurent Pinchart]

Hi Geert,

On Monday 21 Nov 2016 11:59:47 Laurent Pinchart wrote:
> On Monday 21 Nov 2016 10:23:46 Geert Uytterhoeven wrote:
> > On Mon, Nov 21, 2016 at 10:19 AM, Laurent Pinchart wrote:
> >> On Monday 21 Nov 2016 09:36:22 Geert Uytterhoeven wrote:
> >>> On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart wrote:
> >>>> The panel backlight is controlled through a GPIO and a PWM channel.
> >>>> 
> >>>> --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> >>>> +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> >>>> @@ -178,6 +178,16 @@
> >>>>                         };
> >>>>                 };
> >>>>         };
> >>>> +
> >>>> +       backlight: backlight {
> >>>> +               compatible = "pwm-backlight";
> >>>> +               pwms = <&pwm1 0 50000>;
> >>>> +
> >>>> +               brightness-levels = <256 128 64 16 8 4 0>;
> >>> 
> >>> Would it make sense to define more and/or linear levels?
> >> 
> >> Possibly, this is pretty arbitrary. Linear levels might not be the best
> >> option given that the human eye doesn't have a linear response to light
> >> power, but we
> > 
> > It not only depends on the human eye, but also on the backlight hardware
> > (is the conversion from voltage (L_VBRT) to light linear?).
> 
> So we need to specify transfer functions in DT ;-)
> 
> >> could certainly have more levels. In that case I'd prefer modifying the
> >> pwm- backlight DT bindings though, and specifying the PWM resolution
> >> instead of discrete levels.
> >> 
> >> Note that the LVDS panel backlight PWM control signal is multiplexed
> >> with the external memory A21 signal on the Salvator-X board, with SW5
> >> selecting which how to route the signal. When using backlight control we
> >> can't access the whole NOR flash anymore, so I'm not sure this patch
> >> should be merged.
> > 
> > That NOR flash is also optional, right?
> > My Ex Memory Connector is not populated.
> 
> That's correct. The Salvator-X DT file in mainline is just an example
> anyway, and we should pick the most useful peripherals for that purpose.

Would you like me to include this in my next Salvator-X DT patch series for 
upstream merge ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 09/13] arm64: dts: r8a7795: salvator-x: Add panel backlight support

[Geert Uytterhoeven]

Hi Laurent,

On Wed, Apr 5, 2017 at 10:45 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday 21 Nov 2016 11:59:47 Laurent Pinchart wrote:
>> On Monday 21 Nov 2016 10:23:46 Geert Uytterhoeven wrote:
>> > On Mon, Nov 21, 2016 at 10:19 AM, Laurent Pinchart wrote:
>> >> On Monday 21 Nov 2016 09:36:22 Geert Uytterhoeven wrote:
>> >>> On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart wrote:
>> >>>> The panel backlight is controlled through a GPIO and a PWM channel.
>> >>>>
>> >>>> --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
>> >>>> +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
>> >>>> @@ -178,6 +178,16 @@
>> >>>>                         };
>> >>>>                 };
>> >>>>         };
>> >>>> +
>> >>>> +       backlight: backlight {
>> >>>> +               compatible = "pwm-backlight";
>> >>>> +               pwms = <&pwm1 0 50000>;
>> >>>> +
>> >>>> +               brightness-levels = <256 128 64 16 8 4 0>;
>> >>>
>> >>> Would it make sense to define more and/or linear levels?
>> >>
>> >> Possibly, this is pretty arbitrary. Linear levels might not be the best
>> >> option given that the human eye doesn't have a linear response to light
>> >> power, but we
>> >
>> > It not only depends on the human eye, but also on the backlight hardware
>> > (is the conversion from voltage (L_VBRT) to light linear?).
>>
>> So we need to specify transfer functions in DT ;-)
>>
>> >> could certainly have more levels. In that case I'd prefer modifying the
>> >> pwm- backlight DT bindings though, and specifying the PWM resolution
>> >> instead of discrete levels.
>> >>
>> >> Note that the LVDS panel backlight PWM control signal is multiplexed
>> >> with the external memory A21 signal on the Salvator-X board, with SW5
>> >> selecting which how to route the signal. When using backlight control we
>> >> can't access the whole NOR flash anymore, so I'm not sure this patch
>> >> should be merged.
>> >
>> > That NOR flash is also optional, right?
>> > My Ex Memory Connector is not populated.
>>
>> That's correct. The Salvator-X DT file in mainline is just an example
>> anyway, and we should pick the most useful peripherals for that purpose.
>
> Would you like me to include this in my next Salvator-X DT patch series for
> upstream merge ?

That's fine for me (CC Simon). Thx!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 09/13] arm64: dts: r8a7795: salvator-x: Add panel backlight support

[Simon Horman]

On Wed, Apr 05, 2017 at 10:55:32AM +0200, Geert Uytterhoeven wrote:
> Hi Laurent,
> 
> On Wed, Apr 5, 2017 at 10:45 AM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > On Monday 21 Nov 2016 11:59:47 Laurent Pinchart wrote:
> >> On Monday 21 Nov 2016 10:23:46 Geert Uytterhoeven wrote:
> >> > On Mon, Nov 21, 2016 at 10:19 AM, Laurent Pinchart wrote:
> >> >> On Monday 21 Nov 2016 09:36:22 Geert Uytterhoeven wrote:
> >> >>> On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart wrote:
> >> >>>> The panel backlight is controlled through a GPIO and a PWM channel.
> >> >>>>
> >> >>>> --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> >> >>>> +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> >> >>>> @@ -178,6 +178,16 @@
> >> >>>>                         };
> >> >>>>                 };
> >> >>>>         };
> >> >>>> +
> >> >>>> +       backlight: backlight {
> >> >>>> +               compatible = "pwm-backlight";
> >> >>>> +               pwms = <&pwm1 0 50000>;
> >> >>>> +
> >> >>>> +               brightness-levels = <256 128 64 16 8 4 0>;
> >> >>>
> >> >>> Would it make sense to define more and/or linear levels?
> >> >>
> >> >> Possibly, this is pretty arbitrary. Linear levels might not be the best
> >> >> option given that the human eye doesn't have a linear response to light
> >> >> power, but we
> >> >
> >> > It not only depends on the human eye, but also on the backlight hardware
> >> > (is the conversion from voltage (L_VBRT) to light linear?).
> >>
> >> So we need to specify transfer functions in DT ;-)
> >>
> >> >> could certainly have more levels. In that case I'd prefer modifying the
> >> >> pwm- backlight DT bindings though, and specifying the PWM resolution
> >> >> instead of discrete levels.
> >> >>
> >> >> Note that the LVDS panel backlight PWM control signal is multiplexed
> >> >> with the external memory A21 signal on the Salvator-X board, with SW5
> >> >> selecting which how to route the signal. When using backlight control we
> >> >> can't access the whole NOR flash anymore, so I'm not sure this patch
> >> >> should be merged.
> >> >
> >> > That NOR flash is also optional, right?
> >> > My Ex Memory Connector is not populated.
> >>
> >> That's correct. The Salvator-X DT file in mainline is just an example
> >> anyway, and we should pick the most useful peripherals for that purpose.
> >
> > Would you like me to include this in my next Salvator-X DT patch series for
> > upstream merge ?
> 
> That's fine for me (CC Simon). Thx!

Sounds good to me.

[PATCH v2 10/13] ARM: shmobile: dts: Switch to panel-lvds bindings for Mitsubishi panels

[Laurent Pinchart]

The aa104xd12 and aa121td01 panels are LVDS panels, not DPI panels.
Use the correct DT bindings.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm/boot/dts/r8a77xx-aa104xd12-panel.dtsi | 3 ++-
 arch/arm/boot/dts/r8a77xx-aa121td01-panel.dtsi | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/r8a77xx-aa104xd12-panel.dtsi b/arch/arm/boot/dts/r8a77xx-aa104xd12-panel.dtsi
index 65cb50f0c29f..238d14bb0ebe 100644
--- a/arch/arm/boot/dts/r8a77xx-aa104xd12-panel.dtsi
+++ b/arch/arm/boot/dts/r8a77xx-aa104xd12-panel.dtsi
@@ -10,10 +10,11 @@
 
 / {
 	panel {
-		compatible = "mitsubishi,aa104xd12", "panel-dpi";
+		compatible = "mitsubishi,aa104xd12", "panel-lvds";
 
 		width-mm = <210>;
 		height-mm = <158>;
+		data-mapping = "jeida-18";
 
 		panel-timing {
 			/* 1024x768 @65Hz */
diff --git a/arch/arm/boot/dts/r8a77xx-aa121td01-panel.dtsi b/arch/arm/boot/dts/r8a77xx-aa121td01-panel.dtsi
index a07ebf8f6938..04aafd479775 100644
--- a/arch/arm/boot/dts/r8a77xx-aa121td01-panel.dtsi
+++ b/arch/arm/boot/dts/r8a77xx-aa121td01-panel.dtsi
@@ -10,10 +10,11 @@
 
 / {
 	panel {
-		compatible = "mitsubishi,aa121td01", "panel-dpi";
+		compatible = "mitsubishi,aa121td01", "panel-lvds";
 
 		width-mm = <261>;
 		height-mm = <163>;
+		data-mapping = "jeida-18";
 
 		panel-timing {
 			/* 1280x800 @60Hz */
-- 
Regards,

Laurent Pinchart

[PATCH v2 11/13] drm: rcar-du: Switch to encoder .atomic_mode_set() helper function

[Laurent Pinchart]

The native encoder mode set helper function for atomic drivers is
.atomic_mode_set(). Replace the legacy .mode_set() implementation.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 8 ++++----
 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
index ab8645c57e2d..3974d9495f37 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
@@ -83,16 +83,16 @@ static int rcar_du_encoder_atomic_check(struct drm_encoder *encoder,
 }
 
 static void rcar_du_encoder_mode_set(struct drm_encoder *encoder,
-				     struct drm_display_mode *mode,
-				     struct drm_display_mode *adjusted_mode)
+				     struct drm_crtc_state *crtc_state,
+				     struct drm_connector_state *conn_state)
 {
 	struct rcar_du_encoder *renc = to_rcar_encoder(encoder);
 
-	rcar_du_crtc_route_output(encoder->crtc, renc->output);
+	rcar_du_crtc_route_output(crtc_state->crtc, renc->output);
 }
 
 static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
-	.mode_set = rcar_du_encoder_mode_set,
+	.atomic_mode_set = rcar_du_encoder_mode_set,
 	.disable = rcar_du_encoder_disable,
 	.enable = rcar_du_encoder_enable,
 	.atomic_check = rcar_du_encoder_atomic_check,
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c b/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
index f9515f53cc5b..a1a2c5e7822c 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
@@ -67,16 +67,16 @@ static int rcar_du_hdmienc_atomic_check(struct drm_encoder *encoder,
 
 
 static void rcar_du_hdmienc_mode_set(struct drm_encoder *encoder,
-				     struct drm_display_mode *mode,
-				     struct drm_display_mode *adjusted_mode)
+				     struct drm_crtc_state *crtc_state,
+				     struct drm_connector_state *conn_state)
 {
 	struct rcar_du_hdmienc *hdmienc = to_rcar_hdmienc(encoder);
 
-	rcar_du_crtc_route_output(encoder->crtc, hdmienc->renc->output);
+	rcar_du_crtc_route_output(crtc_state->crtc, hdmienc->renc->output);
 }
 
 static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
-	.mode_set = rcar_du_hdmienc_mode_set,
+	.atomic_mode_set = rcar_du_hdmienc_mode_set,
 	.disable = rcar_du_hdmienc_disable,
 	.enable = rcar_du_hdmienc_enable,
 	.atomic_check = rcar_du_hdmienc_atomic_check,
-- 
Regards,

Laurent Pinchart

[PATCH v2 12/13] drm: rcar-du: Use the DRM panel API

[Laurent Pinchart]

Instead of parsing the panel device tree node manually, use the panel
API to delegate panel handling to a panel driver.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/Kconfig           |  1 +
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 12 ++++++
 drivers/gpu/drm/rcar-du/rcar_du_encoder.h |  3 ++
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 68 +++++++++++--------------------
 4 files changed, 40 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index 4c2fd056dd6d..2bab449add76 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -20,6 +20,7 @@ config DRM_RCAR_HDMI
 config DRM_RCAR_LVDS
 	bool "R-Car DU LVDS Encoder Support"
 	depends on DRM_RCAR_DU
+	select DRM_PANEL
 	help
 	  Enable support for the R-Car Display Unit embedded LVDS encoders.
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
index 3974d9495f37..2408216dc1fb 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
@@ -16,6 +16,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_panel.h>
 
 #include "rcar_du_drv.h"
 #include "rcar_du_encoder.h"
@@ -33,6 +34,11 @@ static void rcar_du_encoder_disable(struct drm_encoder *encoder)
 {
 	struct rcar_du_encoder *renc = to_rcar_encoder(encoder);
 
+	if (renc->connector->panel) {
+		drm_panel_disable(renc->connector->panel);
+		drm_panel_unprepare(renc->connector->panel);
+	}
+
 	if (renc->lvds)
 		rcar_du_lvdsenc_enable(renc->lvds, encoder->crtc, false);
 }
@@ -43,6 +49,11 @@ static void rcar_du_encoder_enable(struct drm_encoder *encoder)
 
 	if (renc->lvds)
 		rcar_du_lvdsenc_enable(renc->lvds, encoder->crtc, true);
+
+	if (renc->connector->panel) {
+		drm_panel_prepare(renc->connector->panel);
+		drm_panel_enable(renc->connector->panel);
+	}
 }
 
 static int rcar_du_encoder_atomic_check(struct drm_encoder *encoder,
@@ -89,6 +100,7 @@ static void rcar_du_encoder_mode_set(struct drm_encoder *encoder,
 	struct rcar_du_encoder *renc = to_rcar_encoder(encoder);
 
 	rcar_du_crtc_route_output(crtc_state->crtc, renc->output);
+	renc->connector = to_rcar_connector(conn_state->connector);
 }
 
 static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.h b/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
index 7fc10a9c34c3..269fbab15907 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
@@ -16,6 +16,7 @@
 
 #include <drm/drm_crtc.h>
 
+struct drm_panel;
 struct rcar_du_device;
 struct rcar_du_hdmienc;
 struct rcar_du_lvdsenc;
@@ -31,6 +32,7 @@ enum rcar_du_encoder_type {
 struct rcar_du_encoder {
 	struct drm_encoder base;
 	enum rcar_du_output output;
+	struct rcar_du_connector *connector;
 	struct rcar_du_hdmienc *hdmi;
 	struct rcar_du_lvdsenc *lvds;
 };
@@ -43,6 +45,7 @@ struct rcar_du_encoder {
 struct rcar_du_connector {
 	struct drm_connector connector;
 	struct rcar_du_encoder *encoder;
+	struct drm_panel *panel;
 };
 
 #define to_rcar_connector(c) \
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
index 64e9f0b86e58..ee87494f9218 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
@@ -15,6 +15,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_panel.h>
 
 #include <video/display_timing.h>
 #include <video/of_display_timing.h>
@@ -25,36 +26,11 @@
 #include "rcar_du_kms.h"
 #include "rcar_du_lvdscon.h"
 
-struct rcar_du_lvds_connector {
-	struct rcar_du_connector connector;
-
-	struct {
-		unsigned int width_mm;		/* Panel width in mm */
-		unsigned int height_mm;		/* Panel height in mm */
-		struct videomode mode;
-	} panel;
-};
-
-#define to_rcar_lvds_connector(c) \
-	container_of(c, struct rcar_du_lvds_connector, connector.connector)
-
 static int rcar_du_lvds_connector_get_modes(struct drm_connector *connector)
 {
-	struct rcar_du_lvds_connector *lvdscon =
-		to_rcar_lvds_connector(connector);
-	struct drm_display_mode *mode;
-
-	mode = drm_mode_create(connector->dev);
-	if (mode == NULL)
-		return 0;
+	struct rcar_du_connector *rcon = to_rcar_connector(connector);
 
-	mode->type = DRM_MODE_TYPE_PREFERRED | DRM_MODE_TYPE_DRIVER;
-
-	drm_display_mode_from_videomode(&lvdscon->panel.mode, mode);
-
-	drm_mode_probed_add(connector, mode);
-
-	return 1;
+	return drm_panel_get_modes(rcon->panel);
 }
 
 static const struct drm_connector_helper_funcs connector_helper_funcs = {
@@ -67,12 +43,20 @@ rcar_du_lvds_connector_detect(struct drm_connector *connector, bool force)
 	return connector_status_connected;
 }
 
+static void rcar_du_lvds_connector_destroy(struct drm_connector *connector)
+{
+	struct rcar_du_connector *rcon = to_rcar_connector(connector);
+
+	drm_panel_detach(rcon->panel);
+	drm_connector_cleanup(connector);
+}
+
 static const struct drm_connector_funcs connector_funcs = {
 	.dpms = drm_atomic_helper_connector_dpms,
 	.reset = drm_atomic_helper_connector_reset,
 	.detect = rcar_du_lvds_connector_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = drm_connector_cleanup,
+	.destroy = rcar_du_lvds_connector_destroy,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
@@ -82,27 +66,19 @@ int rcar_du_lvds_connector_init(struct rcar_du_device *rcdu,
 				const struct device_node *np)
 {
 	struct drm_encoder *encoder = rcar_encoder_to_drm_encoder(renc);
-	struct rcar_du_lvds_connector *lvdscon;
+	struct rcar_du_connector *rcon;
 	struct drm_connector *connector;
-	struct display_timing timing;
 	int ret;
 
-	lvdscon = devm_kzalloc(rcdu->dev, sizeof(*lvdscon), GFP_KERNEL);
-	if (lvdscon == NULL)
+	rcon = devm_kzalloc(rcdu->dev, sizeof(*rcon), GFP_KERNEL);
+	if (rcon == NULL)
 		return -ENOMEM;
 
-	ret = of_get_display_timing(np, "panel-timing", &timing);
-	if (ret < 0)
-		return ret;
-
-	videomode_from_timing(&timing, &lvdscon->panel.mode);
+	connector = &rcon->connector;
 
-	of_property_read_u32(np, "width-mm", &lvdscon->panel.width_mm);
-	of_property_read_u32(np, "height-mm", &lvdscon->panel.height_mm);
-
-	connector = &lvdscon->connector.connector;
-	connector->display_info.width_mm = lvdscon->panel.width_mm;
-	connector->display_info.height_mm = lvdscon->panel.height_mm;
+	rcon->panel = of_drm_find_panel(np);
+	if (!rcon->panel)
+		return -EPROBE_DEFER;
 
 	ret = drm_connector_init(rcdu->ddev, connector, &connector_funcs,
 				 DRM_MODE_CONNECTOR_LVDS);
@@ -119,7 +95,11 @@ int rcar_du_lvds_connector_init(struct rcar_du_device *rcdu,
 	if (ret < 0)
 		return ret;
 
-	lvdscon->connector.encoder = renc;
+	ret = drm_panel_attach(rcon->panel, connector);
+	if (ret < 0)
+		return ret;
+
+	rcon->encoder = renc;
 
 	return 0;
 }
-- 
Regards,

Laurent Pinchart

[PATCH v2 13/13] drm: rcar-du: Add support for LVDS mode selection

[Laurent Pinchart]

Retrieve the LVDS mode from the panel and configure the LVDS encoder
accordingly. LVDS mode selection is static as LVDS panels can't be
hot-plugged on any of the device supported by the driver. Support for
dynamic mode selection can be implemented in the future when needed.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 30 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 11 +++++++++--
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h | 13 +++++++++++++
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
index 2408216dc1fb..4134c16becda 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
@@ -98,9 +98,39 @@ static void rcar_du_encoder_mode_set(struct drm_encoder *encoder,
 				     struct drm_connector_state *conn_state)
 {
 	struct rcar_du_encoder *renc = to_rcar_encoder(encoder);
+	struct drm_display_info *info = &conn_state->connector->display_info;
+	enum rcar_lvds_mode mode;
 
 	rcar_du_crtc_route_output(crtc_state->crtc, renc->output);
 	renc->connector = to_rcar_connector(conn_state->connector);
+
+	if (!renc->lvds)
+		return;
+
+	if (!info->num_bus_formats || !info->bus_formats) {
+		dev_err(encoder->dev->dev, "no LVDS bus format reported\n");
+		return;
+	}
+
+	switch (info->bus_formats[0]) {
+	case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
+	case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
+		mode = RCAR_LVDS_MODE_JEIDA;
+		break;
+	case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
+		mode = RCAR_LVDS_MODE_VESA;
+		break;
+	default:
+		dev_err(encoder->dev->dev,
+			"unsupported LVDS bus format 0x%04x\n",
+			info->bus_formats[0]);
+		return;
+	}
+
+	if (info->bus_flags & DRM_BUS_FLAG_DATA_MIRROR)
+		mode |= RCAR_LVDS_MODE_MIRROR;
+
+	rcar_du_lvdsenc_set_mode(renc->lvds, mode);
 }
 
 static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
index e3a4985f6f3f..1661f6201210 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
@@ -31,6 +31,7 @@ struct rcar_du_lvdsenc {
 	bool enabled;
 
 	enum rcar_lvds_input input;
+	enum rcar_lvds_mode mode;
 };
 
 static void rcar_lvds_write(struct rcar_du_lvdsenc *lvds, u32 reg, u32 data)
@@ -61,7 +62,7 @@ static void rcar_du_lvdsenc_start_gen2(struct rcar_du_lvdsenc *lvds,
 	/* Select the input, hardcode mode 0, enable LVDS operation and turn
 	 * bias circuitry on.
 	 */
-	lvdcr0 = LVDCR0_BEN | LVDCR0_LVEN;
+	lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_BEN | LVDCR0_LVEN;
 	if (rcrtc->index == 2)
 		lvdcr0 |= LVDCR0_DUSEL;
 	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
@@ -114,7 +115,7 @@ static void rcar_du_lvdsenc_start_gen3(struct rcar_du_lvdsenc *lvds,
 	 * Turn the PLL on, set it to LVDS normal mode, wait for the startup
 	 * delay and turn the output on.
 	 */
-	lvdcr0 = LVDCR0_PLLON;
+	lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_PLLON;
 	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
 
 	lvdcr0 |= LVDCR0_PWD;
@@ -211,6 +212,12 @@ void rcar_du_lvdsenc_atomic_check(struct rcar_du_lvdsenc *lvds,
 		mode->clock = clamp(mode->clock, 25175, 148500);
 }
 
+void rcar_du_lvdsenc_set_mode(struct rcar_du_lvdsenc *lvds,
+			      enum rcar_lvds_mode mode)
+{
+	lvds->mode = mode;
+}
+
 static int rcar_du_lvdsenc_get_resources(struct rcar_du_lvdsenc *lvds,
 					 struct platform_device *pdev)
 {
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h
index dfdba746edf4..7218ac89333e 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h
@@ -26,8 +26,17 @@ enum rcar_lvds_input {
 	RCAR_LVDS_INPUT_DU2,
 };
 
+/* Keep in sync with the LVDCR0.LVMD hardware register values. */
+enum rcar_lvds_mode {
+	RCAR_LVDS_MODE_JEIDA = 0,
+	RCAR_LVDS_MODE_MIRROR = 1,
+	RCAR_LVDS_MODE_VESA = 4,
+};
+
 #if IS_ENABLED(CONFIG_DRM_RCAR_LVDS)
 int rcar_du_lvdsenc_init(struct rcar_du_device *rcdu);
+void rcar_du_lvdsenc_set_mode(struct rcar_du_lvdsenc *lvds,
+			      enum rcar_lvds_mode mode);
 int rcar_du_lvdsenc_enable(struct rcar_du_lvdsenc *lvds,
 			   struct drm_crtc *crtc, bool enable);
 void rcar_du_lvdsenc_atomic_check(struct rcar_du_lvdsenc *lvds,
@@ -37,6 +46,10 @@ static inline int rcar_du_lvdsenc_init(struct rcar_du_device *rcdu)
 {
 	return 0;
 }
+static inline void rcar_du_lvdsenc_set_mode(struct rcar_du_lvdsenc *lvds,
+					    enum rcar_lvds_mode mode)
+{
+}
 static inline int rcar_du_lvdsenc_enable(struct rcar_du_lvdsenc *lvds,
 					 struct drm_crtc *crtc, bool enable)
 {
-- 
Regards,

Laurent Pinchart