[PATCH v4 0/3] Support non-default LVDS data mapping for simple panel

[PATCH v4 0/3] Support non-default LVDS data mapping for simple panel

[Johannes Zink]

Some LVDS panels, such as the innolux,g101ice-l01 support multiple LVDS
data mapping modes, which can be configured by strapping a dataformat
pin on the display to a specific voltage.

This can be particularly useful for using the jeida-18 format, which
requires only 3 instead of 4 LVDS lanes.

This series moves the data-mapping property for LVDS panels in a
separate file and optionally adds it to simple-panel when matching to
the innolux,g101ice-l01 compatible. This property allows to override
the default data mapping set in the panel description in simple-panel.

The last patch in this series actually adds the driver support for
parsing the data format override device tree property and modifying the
corresponding values for bit per color and media bus format in the panel
descriptor.

Best regards
Johannes

---

Changelog:

v3 -> v4:  - driver: worked in Dan's Feedback:
             - return with proper error in case the call into
	       panel_simple_override_nondefault_lvds_datamapping()
	       failed
	     - drop the unneeded and ambiguous ret local value

- Link to v3: https://lore.kernel.org/r/20230523-simplepanel_support_nondefault_datamapping-v3-0-78ede374d3d9@pengutronix.de
v2 -> v3:  - dt bindings: Worked in Conor's and Laurent's Feedback
	     (thanks for your review): Drop the chomping indicator
	   - dt bindings: Worked in Laurent's Feedback: fix typos
	   - driver: worked in Laurent's review findings:
	     - extract function for fixing up the bus format
	     - only call this function on LVDS panels
	     - fix typo
           - Link to v2: https://lore.kernel.org/r/20230523-simplepanel_support_nondefault_datamapping-v2-0-87196f0d0b64@pengutronix.de

v1 -> v2:  - dt bindings: Worked in Rob's review findings (thanks for your
             review), refactored to use common include instead of duplication
           - driver: added missing error unwinding goto, as found by Dan
             Carpenter's test robot:
             Reported-by: kernel test robot <lkp@intel.com>
             Reported-by: Dan Carpenter <error27@gmail.com>
             Link: https://lore.kernel.org/r/202304160359.4LHmFOlU-lkp@intel.com/

To: David Airlie <airlied@gmail.com>
To: Daniel Vetter <daniel@ffwll.ch>
To: Rob Herring <robh+dt@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Thierry Reding <thierry.reding@gmail.com>
To: Neil Armstrong <neil.armstrong@linaro.org>
To: Sam Ravnborg <sam@ravnborg.org>
Cc: patchwork-jzi@pengutronix.de
Cc: kernel@pengutronix.de
Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Cc: dri-devel@lists.freedesktop.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>

---

---
Johannes Zink (3):
      dt-bindings: display: move LVDS data-mapping definition to separate file
      dt-bindings: display: simple: support non-default data-mapping
      drm/panel-simple: allow LVDS format override

 .../bindings/display/lvds-data-mapping.yaml        | 84 ++++++++++++++++++++++
 .../devicetree/bindings/display/lvds.yaml          | 77 +++-----------------
 .../bindings/display/panel/panel-simple.yaml       | 26 ++++++-
 drivers/gpu/drm/panel/panel-simple.c               | 53 ++++++++++++++
 4 files changed, 171 insertions(+), 69 deletions(-)
---
base-commit: 52920704df878050123dfeb469aa6ab8022547c1
change-id: 20230523-simplepanel_support_nondefault_datamapping-13c3f2ea28f8

Best regards,
-- 
Johannes Zink <j.zink@pengutronix.de>

[PATCH v4 1/3] dt-bindings: display: move LVDS data-mapping definition to separate file

[Johannes Zink]

As the LVDS data-mapping property is required in multiple bindings: move
it to separate file and include instead of duplicating it.

Signed-off-by: Johannes Zink <j.zink@pengutronix.de>

---

Changes:

v3 -> v4: none

v2 -> v3: worked in Conor's and Laurent's review findings (thank you
          for reviewing my work): drop +| on description

v1 -> v2: worked in Rob's review findings (thank you for reviewing my
          work): extract common properties to
          file and include it instead of duplicating it
---
 .../bindings/display/lvds-data-mapping.yaml        | 84 ++++++++++++++++++++++
 .../devicetree/bindings/display/lvds.yaml          | 77 +++-----------------
 2 files changed, 93 insertions(+), 68 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/lvds-data-mapping.yaml b/Documentation/devicetree/bindings/display/lvds-data-mapping.yaml
new file mode 100644
index 000000000000..d68982fe2e9b
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/lvds-data-mapping.yaml
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/lvds-data-mapping.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LVDS Data Mapping
+
+maintainers:
+  - Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
+  - Thierry Reding <thierry.reding@gmail.com>
+
+description: |
+  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 devices. This bindings supports devices 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.
+
+properties:
+  data-mapping:
+    enum:
+      - jeida-18
+      - jeida-24
+      - vesa-24
+    description: |
+      The color signals mapping order.
+
+      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
+
+additionalProperties: true
+
+...
diff --git a/Documentation/devicetree/bindings/display/lvds.yaml b/Documentation/devicetree/bindings/display/lvds.yaml
index 7cd2ce7e9c33..224db4932011 100644
--- a/Documentation/devicetree/bindings/display/lvds.yaml
+++ b/Documentation/devicetree/bindings/display/lvds.yaml
@@ -6,83 +6,24 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: LVDS Display Common Properties
 
+allOf:
+  - $ref: lvds-data-mapping.yaml#
+
 maintainers:
   - Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
   - Thierry Reding <thierry.reding@gmail.com>
 
-description: |+
-  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 devices. This bindings supports devices 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.
+description:
+  This binding extends the data mapping defined in lvds-data-mapping.yaml.
+  It supports reversing the bit order on the formats defined there in order
+  to accomodate for even more specialized data formats, since a variety of
+  data formats and layouts is used to drive LVDS displays.
 
 properties:
-  data-mapping:
-    enum:
-      - jeida-18
-      - jeida-24
-      - vesa-24
-    description: |
-      The color signals mapping order.
-
-      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
-
   data-mirror:
     type: boolean
     description:
-      If set, reverse the bit order described in the data mappings below on all
+      If set, reverse the bit order described in the data mappings on all
       data lanes, transmitting bits for slots 6 to 0 instead of 0 to 6.
 
 additionalProperties: true

-- 
2.39.2

[PATCH v4 2/3] dt-bindings: display: simple: support non-default data-mapping

[Johannes Zink]

Some Displays support more than just a single default LVDS data mapping,
which can be used to run displays on only 3 LVDS lanes in the jeida-18
data-mapping mode.

Add an optional data-mapping property to allow overriding the default
data mapping. As it does not generally apply to any display and bus, use
it selectively on the innolux,g101ice-l01, which supports changing the
data mapping via a strapping pin.

Signed-off-by: Johannes Zink <j.zink@pengutronix.de>

---

Changes:

v3 -> v4: none

v2 -> v3: - worked in Laurent's review findings (thanks for reviewing
	    my work): fix typos in commit message

v1 -> v2: - worked in Rob's review findings (thanks for reviewing my
            work): use extracted common property instead of duplicating
	    the property
	  - refined commit message
	  - add an example dts for automated checking
---
 .../bindings/display/panel/panel-simple.yaml       | 26 +++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index 1d4936fc5182..e25e33f67d71 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -21,9 +21,9 @@ description: |
 
 allOf:
   - $ref: panel-common.yaml#
+  - $ref: ../lvds-data-mapping.yaml#
 
 properties:
-
   compatible:
     enum:
     # compatible must be listed in alphabetical order, ordered by compatible.
@@ -359,6 +359,17 @@ properties:
   power-supply: true
   no-hpd: true
   hpd-gpios: true
+  data-mapping: true
+
+if:
+  not:
+    properties:
+      compatible:
+        contains:
+          const: innolux,g101ice-l01
+then:
+  properties:
+    data-mapping: false
 
 additionalProperties: false
 
@@ -378,3 +389,16 @@ examples:
         };
       };
     };
+  - |
+    panel_lvds: panel-lvds {
+      compatible = "innolux,g101ice-l01";
+      power-supply = <&vcc_lcd_reg>;
+
+      data-mapping = "jeida-24";
+
+      port {
+        panel_in_lvds: endpoint {
+          remote-endpoint = <&ltdc_out_lvds>;
+        };
+      };
+    };

-- 
2.39.2

[PATCH v4 3/3] drm/panel-simple: allow LVDS format override

[Johannes Zink]

Some panels support multiple LVDS data mapping formats, which can be
used e.g. run displays on jeida-18 format when only 3 LVDS lanes are
available.

Add parsing of an optional data-mapping devicetree property, which also
touches up the bits per color to match the bus format.

Signed-off-by: Johannes Zink <j.zink@pengutronix.de>

---

Changes:

  v3 -> v4: - worked in Dan's feedback (thanks for reviewing my work):
                - return with a proper error in case the call to
		  panel_simple_override_nondefault_lvds_datamapping()
		  fails
                - drop the unneeded and ambiguous ret variable

  v2 -> v3: - worked in Laurent's review findings (thanks for reviewing
              my work):
	        - extract fixing up the bus format to separate
		  function
		- only call function on LVDS panels
		- fix typos found by Laurent
		- simplified error handling

  v1 -> v2: - fix missing unwind goto found by test robot
              Reported-by: kernel test robot <lkp@intel.com>
              Reported-by: Dan Carpenter <error27@gmail.com>
              Link: https://lore.kernel.org/r/202304160359.4LHmFOlU-lkp@intel.com/
---
 drivers/gpu/drm/panel/panel-simple.c | 53 ++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 4badda6570d5..3a164931093e 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -40,6 +40,7 @@
 #include <drm/drm_edid.h>
 #include <drm/drm_mipi_dsi.h>
 #include <drm/drm_panel.h>
+#include <drm/drm_of.h>
 
 /**
  * struct panel_desc - Describes a simple panel.
@@ -549,6 +550,51 @@ static void panel_simple_parse_panel_timing_node(struct device *dev,
 		dev_err(dev, "Reject override mode: No display_timing found\n");
 }
 
+static int panel_simple_override_nondefault_lvds_datamapping(struct device *dev,
+							     struct panel_simple *panel)
+{
+	int ret, bpc;
+
+	ret = drm_of_lvds_get_data_mapping(dev->of_node);
+	if (ret < 0) {
+		if (ret == -EINVAL)
+			dev_warn(dev, "Ignore invalid data-mapping property\n");
+
+		/*
+		 * Ignore non-existing or malformatted property, fallback to
+		 * default data-mapping, and return 0.
+		 */
+		return 0;
+	}
+
+	switch (ret) {
+	default:
+		WARN_ON(1);
+		fallthrough;
+	case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
+		fallthrough;
+	case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
+		bpc = 8;
+		break;
+	case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
+		bpc = 6;
+	}
+
+	if (panel->desc->bpc != bpc || panel->desc->bus_format != ret) {
+		struct panel_desc *override_desc;
+
+		override_desc = devm_kmemdup(dev, panel->desc, sizeof(*panel->desc), GFP_KERNEL);
+		if (!override_desc)
+			return -ENOMEM;
+
+		override_desc->bus_format = ret;
+		override_desc->bpc = bpc;
+		panel->desc = override_desc;
+	}
+
+	return 0;
+}
+
 static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
 {
 	struct panel_simple *panel;
@@ -601,6 +647,13 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
 			panel_simple_parse_panel_timing_node(dev, panel, &dt);
 	}
 
+	if (desc->connector_type == DRM_MODE_CONNECTOR_LVDS) {
+		/* Optional data-mapping property for overriding bus format */
+		err = panel_simple_override_nondefault_lvds_datamapping(dev, panel);
+		if (err)
+			goto free_ddc;
+	}
+
 	connector_type = desc->connector_type;
 	/* Catch common mistakes for panels. */
 	switch (connector_type) {

-- 
2.39.2

Re: [PATCH v4 1/3] dt-bindings: display: move LVDS data-mapping definition to separate file

[Conor Dooley]

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

On Fri, Jul 28, 2023 at 04:16:55PM +0200, Johannes Zink wrote:
> As the LVDS data-mapping property is required in multiple bindings: move
> it to separate file and include instead of duplicating it.
> 
> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> 
> ---
> 
> Changes:
> 
> v3 -> v4: none
> 
> v2 -> v3: worked in Conor's and Laurent's review findings (thank you
>           for reviewing my work): drop +| on description

I think both Laurent & I provided reviewed-bys on v2. Is there a reason
you did not include them? Here's mine back again:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
and here's a link to Laurent's:
https://lore.kernel.org/all/20230602153239.GK26944@pendragon.ideasonboard.com/

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

Re: [PATCH v4 2/3] dt-bindings: display: simple: support non-default data-mapping

[Conor Dooley]

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

On Fri, Jul 28, 2023 at 04:16:56PM +0200, Johannes Zink wrote:
> Some Displays support more than just a single default LVDS data mapping,
> which can be used to run displays on only 3 LVDS lanes in the jeida-18
> data-mapping mode.
> 
> Add an optional data-mapping property to allow overriding the default
> data mapping. As it does not generally apply to any display and bus, use
> it selectively on the innolux,g101ice-l01, which supports changing the
> data mapping via a strapping pin.
> 
> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> 
> ---
> 
> Changes:
> 
> v3 -> v4: none
> 
> v2 -> v3: - worked in Laurent's review findings (thanks for reviewing
> 	    my work): fix typos in commit message

I gave you one for this patch too, no?
https://lore.kernel.org/all/20230523-jaywalker-modify-500ec1d79223@spud/
Any reason in particular you didn't pick up the tags? Here it is
against, since all that appears to have changed is some typos.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>



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

Re: [PATCH v4 2/3] dt-bindings: display: simple: support non-default data-mapping

[Johannes Zink]

Hi Conor,

On 7/29/23 12:19, Conor Dooley wrote:
> On Fri, Jul 28, 2023 at 04:16:56PM +0200, Johannes Zink wrote:
>> Some Displays support more than just a single default LVDS data mapping,
>> which can be used to run displays on only 3 LVDS lanes in the jeida-18
>> data-mapping mode.
>>
>> Add an optional data-mapping property to allow overriding the default
>> data mapping. As it does not generally apply to any display and bus, use
>> it selectively on the innolux,g101ice-l01, which supports changing the
>> data mapping via a strapping pin.
>>
>> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
>>
>> ---
>>
>> Changes:
>>
>> v3 -> v4: none
>>
>> v2 -> v3: - worked in Laurent's review findings (thanks for reviewing
>> 	    my work): fix typos in commit message
> 
> I gave you one for this patch too, no? > https://lore.kernel.org/all/20230523-jaywalker-modify-500ec1d79223@spud/
> Any reason in particular you didn't pick up the tags? Here it is
> against, since all that appears to have changed is some typos.

sorry, I forgot to add the Tags, There is no functional changes in patch 1-3 of 4.

Johannes

> 
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> 
> 

-- 
Pengutronix e.K.                | Johannes Zink                  |
Steuerwalder Str. 21            | https://www.pengutronix.de/    |
31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |

Re: [PATCH v4 3/3] drm/panel-simple: allow LVDS format override

[Johannes Zink]

Hi Dan,

do you have any input on this for me?

Best regards
Johannes

On 7/28/23 16:16, Johannes Zink wrote:
> Some panels support multiple LVDS data mapping formats, which can be
> used e.g. run displays on jeida-18 format when only 3 LVDS lanes are
> available.
> 
> Add parsing of an optional data-mapping devicetree property, which also
> touches up the bits per color to match the bus format.
> 
> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> 
> ---
> 
> Changes:
> 
>    v3 -> v4: - worked in Dan's feedback (thanks for reviewing my work):
>                  - return with a proper error in case the call to
> 		  panel_simple_override_nondefault_lvds_datamapping()
> 		  fails
>                  - drop the unneeded and ambiguous ret variable
> 
>    v2 -> v3: - worked in Laurent's review findings (thanks for reviewing
>                my work):
> 	        - extract fixing up the bus format to separate
> 		  function
> 		- only call function on LVDS panels
> 		- fix typos found by Laurent
> 		- simplified error handling
> 
>    v1 -> v2: - fix missing unwind goto found by test robot
>                Reported-by: kernel test robot <lkp@intel.com>
>                Reported-by: Dan Carpenter <error27@gmail.com>
>                Link: https://lore.kernel.org/r/202304160359.4LHmFOlU-lkp@intel.com/
> ---
>   drivers/gpu/drm/panel/panel-simple.c | 53 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 53 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index 4badda6570d5..3a164931093e 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -40,6 +40,7 @@
>   #include <drm/drm_edid.h>
>   #include <drm/drm_mipi_dsi.h>
>   #include <drm/drm_panel.h>
> +#include <drm/drm_of.h>
>   
>   /**
>    * struct panel_desc - Describes a simple panel.
> @@ -549,6 +550,51 @@ static void panel_simple_parse_panel_timing_node(struct device *dev,
>   		dev_err(dev, "Reject override mode: No display_timing found\n");
>   }
>   
> +static int panel_simple_override_nondefault_lvds_datamapping(struct device *dev,
> +							     struct panel_simple *panel)
> +{
> +	int ret, bpc;
> +
> +	ret = drm_of_lvds_get_data_mapping(dev->of_node);
> +	if (ret < 0) {
> +		if (ret == -EINVAL)
> +			dev_warn(dev, "Ignore invalid data-mapping property\n");
> +
> +		/*
> +		 * Ignore non-existing or malformatted property, fallback to
> +		 * default data-mapping, and return 0.
> +		 */
> +		return 0;
> +	}
> +
> +	switch (ret) {
> +	default:
> +		WARN_ON(1);
> +		fallthrough;
> +	case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
> +		fallthrough;
> +	case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
> +		bpc = 8;
> +		break;
> +	case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
> +		bpc = 6;
> +	}
> +
> +	if (panel->desc->bpc != bpc || panel->desc->bus_format != ret) {
> +		struct panel_desc *override_desc;
> +
> +		override_desc = devm_kmemdup(dev, panel->desc, sizeof(*panel->desc), GFP_KERNEL);
> +		if (!override_desc)
> +			return -ENOMEM;
> +
> +		override_desc->bus_format = ret;
> +		override_desc->bpc = bpc;
> +		panel->desc = override_desc;
> +	}
> +
> +	return 0;
> +}
> +
>   static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
>   {
>   	struct panel_simple *panel;
> @@ -601,6 +647,13 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
>   			panel_simple_parse_panel_timing_node(dev, panel, &dt);
>   	}
>   
> +	if (desc->connector_type == DRM_MODE_CONNECTOR_LVDS) {
> +		/* Optional data-mapping property for overriding bus format */
> +		err = panel_simple_override_nondefault_lvds_datamapping(dev, panel);
> +		if (err)
> +			goto free_ddc;
> +	}
> +
>   	connector_type = desc->connector_type;
>   	/* Catch common mistakes for panels. */
>   	switch (connector_type) {
> 

-- 
Pengutronix e.K.                | Johannes Zink                  |
Steuerwalder Str. 21            | https://www.pengutronix.de/    |
31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |

Re: [PATCH v4 0/3] Support non-default LVDS data mapping for simple panel

[Johannes Zink]

Hi everyone,

gentle ping - is there any feedback you can give me for moving this series forward?

Best regards
Johannes

On 7/28/23 16:16, Johannes Zink wrote:
> Some LVDS panels, such as the innolux,g101ice-l01 support multiple LVDS
> data mapping modes, which can be configured by strapping a dataformat
> pin on the display to a specific voltage.
> 
> This can be particularly useful for using the jeida-18 format, which
> requires only 3 instead of 4 LVDS lanes.
> 
> This series moves the data-mapping property for LVDS panels in a
> separate file and optionally adds it to simple-panel when matching to
> the innolux,g101ice-l01 compatible. This property allows to override
> the default data mapping set in the panel description in simple-panel.
> 
> The last patch in this series actually adds the driver support for
> parsing the data format override device tree property and modifying the
> corresponding values for bit per color and media bus format in the panel
> descriptor.
> 
> Best regards
> Johannes
> 
> ---
> 
> Changelog:
> 
> v3 -> v4:  - driver: worked in Dan's Feedback:
>               - return with proper error in case the call into
> 	       panel_simple_override_nondefault_lvds_datamapping()
> 	       failed
> 	     - drop the unneeded and ambiguous ret local value
> 
> - Link to v3: https://lore.kernel.org/r/20230523-simplepanel_support_nondefault_datamapping-v3-0-78ede374d3d9@pengutronix.de
> v2 -> v3:  - dt bindings: Worked in Conor's and Laurent's Feedback
> 	     (thanks for your review): Drop the chomping indicator
> 	   - dt bindings: Worked in Laurent's Feedback: fix typos
> 	   - driver: worked in Laurent's review findings:
> 	     - extract function for fixing up the bus format
> 	     - only call this function on LVDS panels
> 	     - fix typo
>             - Link to v2: https://lore.kernel.org/r/20230523-simplepanel_support_nondefault_datamapping-v2-0-87196f0d0b64@pengutronix.de
> 
> v1 -> v2:  - dt bindings: Worked in Rob's review findings (thanks for your
>               review), refactored to use common include instead of duplication
>             - driver: added missing error unwinding goto, as found by Dan
>               Carpenter's test robot:
>               Reported-by: kernel test robot <lkp@intel.com>
>               Reported-by: Dan Carpenter <error27@gmail.com>
>               Link: https://lore.kernel.org/r/202304160359.4LHmFOlU-lkp@intel.com/
> 
> To: David Airlie <airlied@gmail.com>
> To: Daniel Vetter <daniel@ffwll.ch>
> To: Rob Herring <robh+dt@kernel.org>
> To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> To: Conor Dooley <conor+dt@kernel.org>
> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> To: Thierry Reding <thierry.reding@gmail.com>
> To: Neil Armstrong <neil.armstrong@linaro.org>
> To: Sam Ravnborg <sam@ravnborg.org>
> Cc: patchwork-jzi@pengutronix.de
> Cc: kernel@pengutronix.de
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> 
> ---
> 
> ---
> Johannes Zink (3):
>        dt-bindings: display: move LVDS data-mapping definition to separate file
>        dt-bindings: display: simple: support non-default data-mapping
>        drm/panel-simple: allow LVDS format override
> 
>   .../bindings/display/lvds-data-mapping.yaml        | 84 ++++++++++++++++++++++
>   .../devicetree/bindings/display/lvds.yaml          | 77 +++-----------------
>   .../bindings/display/panel/panel-simple.yaml       | 26 ++++++-
>   drivers/gpu/drm/panel/panel-simple.c               | 53 ++++++++++++++
>   4 files changed, 171 insertions(+), 69 deletions(-)
> ---
> base-commit: 52920704df878050123dfeb469aa6ab8022547c1
> change-id: 20230523-simplepanel_support_nondefault_datamapping-13c3f2ea28f8
> 
> Best regards,

-- 
Pengutronix e.K.                | Johannes Zink                  |
Steuerwalder Str. 21            | https://www.pengutronix.de/    |
31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |

Re: [PATCH v4 3/3] drm/panel-simple: allow LVDS format override

[Dan Carpenter]

On Fri, Aug 18, 2023 at 09:04:34AM +0200, Johannes Zink wrote:
> Hi Dan,
> 
> do you have any input on this for me?
> 

Sorry, I was out of office and the truth is that I'm never going to
catch up on all the email I missed.  :/

Looks okay to me.  I can't remember what I said about this code in v3
but it looks good now.  I'm not a DRM dev so I'm not sure my review
counts for much.  You should always just assume that if I'm quiet
then I'm happy.  :)

regards,
dan carpenter

Re: [PATCH v4 3/3] drm/panel-simple: allow LVDS format override

[Johannes Zink]

Hi Dan,

On 9/13/23 13:14, Dan Carpenter wrote:
> On Fri, Aug 18, 2023 at 09:04:34AM +0200, Johannes Zink wrote:
>> Hi Dan,
>>
>> do you have any input on this for me?
>>
> 
> Sorry, I was out of office and the truth is that I'm never going to
> catch up on all the email I missed.  :/
> 

nevermind, that's why I sent ping...

> Looks okay to me.  I can't remember what I said about this code in v3
> but it looks good now.  I'm not a DRM dev so I'm not sure my review
> counts for much.  

IIRC it was a mistake I made with a return value that I have fixed in v4.

You should always just assume that if I'm quiet
> then I'm happy.  :)

That's good to know ;-) Thanks for your review!

Johannes

> 
> regards,
> dan carpenter
> 
> 

-- 
Pengutronix e.K.                | Johannes Zink                  |
Steuerwalder Str. 21            | https://www.pengutronix.de/    |
31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |