[PATCH v2 0/9] Add dual-LVDS panel support to EK874

[PATCH v2 0/9] Add dual-LVDS panel support to EK874

[Fabrizio Castro]

Dear All,

this series adds support for dual-LVDS panel IDK-2121WR
from Advantech:
https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm
Dual link support is very recent for R-Car Gen3, and I couldn't
find much on dual link panels in the kernel either, therefore
comments are very welcome to get this right.

The panel doesn't come with the EK874 kit, but it's advertised as
supported, therefore this series adds a new dts file to support
the configuration of the EK874 + IDK-2121WR.

Laurent,

it appears that Rob has been busy converting the dt-bindings relevant to this
series, and his changes are now found in linux-next. Most notably
Documentation/devicetree/bindings/display/panel/panel-lvds.txt has now become
Documentation/devicetree/bindings/display/panel/lvds.yaml

You have already taken patch:
https://patchwork.kernel.org/patch/11072749/

as such the patch I am sending you is still related to:
Documentation/devicetree/bindings/display/panel/panel-lvds.txt

Patch "dt-bindings: display: Add bindings for Advantech IDK-2121WR" is still
assuming the format is .txt, as I am not too sure about what the protocol is in
this case? Shall we take this patch and convert it later to .yaml or shall I
convert it to .yaml straight away?

Please, let me know what's the best course of action.

v1->v2:
* dt-bindings: display: renesas: lvds: Document renesas,swap-data - dropped
* drm: rcar-du: lvds: Add data swap support - dropped
* drm: rcar-du: lvds: Do not look at ports for identifying bridges - dropped
* drm: rcar-du: lvds: Add support for dual link panels - dropped
* dt-bindings: display: renesas: lvds: RZ/G2E needs renesas,companion too - taken
* drm: rcar-du: lvds: Fix bridge_to_rcar_lvds - taken
* arm64: dts: renesas: r8a774c0: Point LVDS0 to its companion LVDS1 - taken
* arm64: dts: renesas: cat874: Add definition for 12V regulator -taken
* dt-bindings: panel: lvds: Add dual-link LVDS display support - reworked according to Laurent's feedback
* dt-bindings: display: Add bindings for Advantech IDK-2121WR - reworked according to Laurent's feedback
* drm: Rename drm_bridge_timings to drm_timings - new patch
* drm/timings: Add link flags - new patch
* drm/panel: Add timings field to drm_panel - new patch
* drm: rcar-du: lvds: Fix companion's mode - reworked according to Laurent's feedback
* drm: rcar-du: lvds: Add dual-LVDS panels support - new patch
* drm/panel: lvds: Add support for the IDK-2121WR - new patch
* arm64: dts: renesas: Add EK874 board with idk-2121wr display support - Made some changes

Thanks,
Fab

Fabrizio Castro (9):
  dt-bindings: panel: lvds: Add dual-link LVDS display support
  dt-bindings: display: Add bindings for Advantech IDK-2121WR
  drm: Rename drm_bridge_timings to drm_timings
  drm/timings: Add link flags
  drm/panel: Add timings field to drm_panel
  drm: rcar-du: lvds: Fix companion's mode
  drm: rcar-du: lvds: Add dual-LVDS panels support
  drm/panel: lvds: Add support for the IDK-2121WR
  arm64: dts: renesas: Add EK874 board with idk-2121wr display support

 .../display/panel/advantech,idk-2121wr.txt         |  56 +++++++++
 .../bindings/display/panel/panel-lvds.txt          |  95 ++++++++++++----
 arch/arm64/boot/dts/renesas/Makefile               |   3 +-
 .../boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts | 116 +++++++++++++++++++
 drivers/gpu/drm/bridge/dumb-vga-dac.c              |   6 +-
 drivers/gpu/drm/bridge/sii902x.c                   |   2 +-
 drivers/gpu/drm/bridge/thc63lvd1024.c              |   2 +-
 drivers/gpu/drm/bridge/ti-tfp410.c                 |   6 +-
 drivers/gpu/drm/panel/panel-lvds.c                 |   8 ++
 drivers/gpu/drm/pl111/pl111_display.c              |   2 +-
 drivers/gpu/drm/rcar-du/rcar_lvds.c                | 126 ++++++++++++++-------
 include/drm/drm_bridge.h                           |  40 +------
 include/drm/drm_panel.h                            |   3 +
 include/drm/drm_timings.h                          |  86 ++++++++++++++
 14 files changed, 439 insertions(+), 112 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
 create mode 100644 arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
 create mode 100644 include/drm/drm_timings.h

-- 
2.7.4

[PATCH v2 1/9] dt-bindings: panel: lvds: Add dual-link LVDS display support

[Fabrizio Castro]

Dual-link LVDS displays have two ports, therefore document this
with the bindings.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

---
v1->v2:
* Reworked the description of the ports property
* lvds0_panel_in in the example has been renamed to panel_in0
* lvds1_panel_in in the example has been renamed to panel_in1

Laurent,

in linux-next they are now working with:
Documentation/devicetree/bindings/display/panel/lvds.yaml

What should I do here?

Thanks,
Fab


 .../bindings/display/panel/panel-lvds.txt          | 95 ++++++++++++++++------
 1 file changed, 71 insertions(+), 24 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
index 250850a..5231243 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
+++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
@@ -41,7 +41,12 @@ 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.
+  (for a single link panel) corresponding to the panel LVDS input, or two port
+  subnodes (for a dual link panel) corresponding to the panel LVDS inputs.
+  Dual-link LVDS panels expect even pixels (0, 2, 4, etc.) and odd pixels (1, 3,
+  5, etc.) on different input ports, it's up to the panel-specific bindings to
+  specify what port is expecting even pixels, and what port is expecting odd
+  pixels.
 
 
 LVDS data mappings are defined as follows.
@@ -92,30 +97,72 @@ 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>;
+Single port:
+	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>;
+			};
+		};
 	};
 
-	port {
-		panel_in: endpoint {
-			remote-endpoint = <&lvds_encoder>;
+Two ports:
+	panel {
+		compatible = "advantech,idk-2121wr", "panel-lvds";
+
+		width-mm = <476>;
+		height-mm = <268>;
+
+		data-mapping = "vesa-24";
+
+		panel-timing {
+			clock-frequency = <148500000>;
+			hactive = <1920>;
+			vactive = <1080>;
+			hsync-len = <44>;
+			hfront-porch = <88>;
+			hback-porch = <148>;
+			vfront-porch = <4>;
+			vback-porch = <36>;
+			vsync-len = <5>;
+		};
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+				panel_in0: endpoint {
+					remote-endpoint = <&lvds0_out>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+				panel_in1: endpoint {
+					remote-endpoint = <&lvds1_out>;
+				};
+			};
 		};
 	};
-};
-- 
2.7.4

[PATCH v2 2/9] dt-bindings: display: Add bindings for Advantech IDK-2121WR

[Fabrizio Castro]

This panel is handled through the generic lvds-panel bindings,
so only needs its additional compatible specified.

Some panel-specific documentation can be found here:
https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

---
v1->v2:
* Reworked according to Laurent's feedback
* Renamed lvds0_panel_in to panel_in0
* Renamed lvds1_panel_in to panel_in1

Laurent,

Should this be a .yaml file already?

Thanks,
Fab

 .../display/panel/advantech,idk-2121wr.txt         | 56 ++++++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt

diff --git a/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
new file mode 100644
index 0000000..6ee1d1b
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
@@ -0,0 +1,56 @@
+Advantech Co., Ltd. IDK-2121WR 21.5" LVDS panel
+===============================================
+
+Required properties:
+- compatible: should be "advantech,idk-2121wr" followed by "panel-lvds"
+
+This binding is compatible with the lvds-panel binding, which is specified
+in panel-lvds.txt in this directory.
+The panel operates in dual-link mode and thus requires two port nodes,
+the first port node expects odd pixels (1, 3, 5, etc.) and the second port
+expects even pixels (0, 2, 4, etc.).
+
+Example
+-------
+
+	panel {
+		compatible = "advantech,idk-2121wr", "panel-lvds";
+
+		width-mm = <476>;
+		height-mm = <268>;
+
+		data-mapping = "vesa-24";
+
+		panel-timing {
+			clock-frequency = <148500000>;
+			hactive = <1920>;
+			vactive = <1080>;
+			hsync-len = <44>;
+			hfront-porch = <88>;
+			hback-porch = <148>;
+			vfront-porch = <4>;
+			vback-porch = <36>;
+			vsync-len = <5>;
+		};
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				/* Odd pixels */
+				reg = <0>;
+				panel_in0: endpoint {
+					remote-endpoint = <&lvds0_out>;
+				};
+			};
+
+			port@1 {
+				/* Even pixels */
+				reg = <1>;
+				panel_in1: endpoint {
+					remote-endpoint = <&lvds1_out>;
+				};
+			};
+		};
+	};
-- 
2.7.4

[PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings

[Fabrizio Castro]

The information represented by drm_bridge_timings is also
needed by panels, therefore rename drm_bridge_timings to
drm_timings.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
Link: https://www.spinics.net/lists/linux-renesas-soc/msg43271.html

---
v1->v2:
* new patch

I have copied the license from include/drm/drm_bridge.h as that's
where the struct originally came from. What's the right SPDX license
to use in this case?

 drivers/gpu/drm/bridge/dumb-vga-dac.c |  6 ++--
 drivers/gpu/drm/bridge/sii902x.c      |  2 +-
 drivers/gpu/drm/bridge/thc63lvd1024.c |  2 +-
 drivers/gpu/drm/bridge/ti-tfp410.c    |  6 ++--
 drivers/gpu/drm/pl111/pl111_display.c |  2 +-
 include/drm/drm_bridge.h              | 40 ++---------------------
 include/drm/drm_timings.h             | 60 +++++++++++++++++++++++++++++++++++
 7 files changed, 71 insertions(+), 47 deletions(-)
 create mode 100644 include/drm/drm_timings.h

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index d32885b..bb1d928 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -228,7 +228,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
  * NOTE: the ADV7123EP seems to have other timings and need a new timings
  * set if used.
  */
-static const struct drm_bridge_timings default_dac_timings = {
+static const struct drm_timings default_dac_timings = {
 	/* Timing specifications, datasheet page 7 */
 	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
 	.setup_time_ps = 500,
@@ -239,7 +239,7 @@ static const struct drm_bridge_timings default_dac_timings = {
  * Information taken from the THS8134, THS8134A, THS8134B datasheet named
  * "SLVS205D", dated May 1990, revised March 2000.
  */
-static const struct drm_bridge_timings ti_ths8134_dac_timings = {
+static const struct drm_timings ti_ths8134_dac_timings = {
 	/* From timing diagram, datasheet page 9 */
 	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
 	/* From datasheet, page 12 */
@@ -252,7 +252,7 @@ static const struct drm_bridge_timings ti_ths8134_dac_timings = {
  * Information taken from the THS8135 datasheet named "SLAS343B", dated
  * May 2001, revised April 2013.
  */
-static const struct drm_bridge_timings ti_ths8135_dac_timings = {
+static const struct drm_timings ti_ths8135_dac_timings = {
 	/* From timing diagram, datasheet page 14 */
 	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
 	/* From datasheet, page 16 */
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index dd7aa46..0c63065 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -914,7 +914,7 @@ static int sii902x_i2c_bypass_deselect(struct i2c_mux_core *mux, u32 chan_id)
 	return 0;
 }
 
-static const struct drm_bridge_timings default_sii902x_timings = {
+static const struct drm_timings default_sii902x_timings = {
 	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE
 		 | DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE
 		 | DRM_BUS_FLAG_DE_HIGH,
diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c
index 3d74129b..9047a9e 100644
--- a/drivers/gpu/drm/bridge/thc63lvd1024.c
+++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
@@ -34,7 +34,7 @@ struct thc63_dev {
 	struct drm_bridge bridge;
 	struct drm_bridge *next;
 
-	struct drm_bridge_timings timings;
+	struct drm_timings timings;
 };
 
 static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
index dbf35c7..c086b06c 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -32,7 +32,7 @@ struct tfp410 {
 	struct delayed_work	hpd_work;
 	struct gpio_desc	*powerdown;
 
-	struct drm_bridge_timings timings;
+	struct drm_timings timings;
 
 	struct device *dev;
 };
@@ -190,7 +190,7 @@ static irqreturn_t tfp410_hpd_irq_thread(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
-static const struct drm_bridge_timings tfp410_default_timings = {
+static const struct drm_timings tfp410_default_timings = {
 	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE
 			 | DRM_BUS_FLAG_DE_HIGH,
 	.setup_time_ps = 1200,
@@ -199,7 +199,7 @@ static const struct drm_bridge_timings tfp410_default_timings = {
 
 static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c)
 {
-	struct drm_bridge_timings *timings = &dvi->timings;
+	struct drm_timings *timings = &dvi->timings;
 	struct device_node *ep;
 	u32 pclk_sample = 0;
 	u32 bus_width = 24;
diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 15d2755..c82b21f 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -188,7 +188,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
 	}
 
 	if (bridge) {
-		const struct drm_bridge_timings *btimings = bridge->timings;
+		const struct drm_timings *btimings = bridge->timings;
 
 		/*
 		 * Here is when things get really fun. Sometimes the bridge
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 7616f65..8270a38 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -27,9 +27,9 @@
 #include <linux/ctype.h>
 #include <drm/drm_mode_object.h>
 #include <drm/drm_modes.h>
+#include <drm/drm_timings.h>
 
 struct drm_bridge;
-struct drm_bridge_timings;
 struct drm_panel;
 
 /**
@@ -337,42 +337,6 @@ struct drm_bridge_funcs {
 };
 
 /**
- * struct drm_bridge_timings - timing information for the bridge
- */
-struct drm_bridge_timings {
-	/**
-	 * @input_bus_flags:
-	 *
-	 * Tells what additional settings for the pixel data on the bus
-	 * this bridge requires (like pixel signal polarity). See also
-	 * &drm_display_info->bus_flags.
-	 */
-	u32 input_bus_flags;
-	/**
-	 * @setup_time_ps:
-	 *
-	 * Defines the time in picoseconds the input data lines must be
-	 * stable before the clock edge.
-	 */
-	u32 setup_time_ps;
-	/**
-	 * @hold_time_ps:
-	 *
-	 * Defines the time in picoseconds taken for the bridge to sample the
-	 * input signal after the clock edge.
-	 */
-	u32 hold_time_ps;
-	/**
-	 * @dual_link:
-	 *
-	 * True if the bus operates in dual-link mode. The exact meaning is
-	 * dependent on the bus type. For LVDS buses, this indicates that even-
-	 * and odd-numbered pixels are received on separate links.
-	 */
-	bool dual_link;
-};
-
-/**
  * struct drm_bridge - central DRM bridge control structure
  */
 struct drm_bridge {
@@ -393,7 +357,7 @@ struct drm_bridge {
 	 *
 	 * the timing specification for the bridge, if any (may be NULL)
 	 */
-	const struct drm_bridge_timings *timings;
+	const struct drm_timings *timings;
 	/** @funcs: control functions */
 	const struct drm_bridge_funcs *funcs;
 	/** @driver_private: pointer to the bridge driver's internal context */
diff --git a/include/drm/drm_timings.h b/include/drm/drm_timings.h
new file mode 100644
index 0000000..4af8814
--- /dev/null
+++ b/include/drm/drm_timings.h
@@ -0,0 +1,60 @@
+/*
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#ifndef __DRM_TIMINGS_H__
+#define __DRM_TIMINGS_H__
+
+/**
+ * struct drm_timings - timing information
+ */
+struct drm_timings {
+	/**
+	 * @input_bus_flags:
+	 *
+	 * Tells what additional settings for the pixel data on the bus
+	 * are required (like pixel signal polarity). See also
+	 * &drm_display_info->bus_flags.
+	 */
+	u32 input_bus_flags;
+	/**
+	 * @setup_time_ps:
+	 *
+	 * Defines the time in picoseconds the input data lines must be
+	 * stable before the clock edge.
+	 */
+	u32 setup_time_ps;
+	/**
+	 * @hold_time_ps:
+	 *
+	 * Defines the time in picoseconds taken for the bridge to sample the
+	 * input signal after the clock edge.
+	 */
+	u32 hold_time_ps;
+	/**
+	 * @dual_link:
+	 *
+	 * True if the bus operates in dual-link mode. The exact meaning is
+	 * dependent on the bus type. For LVDS buses, this indicates that even-
+	 * and odd-numbered pixels are received on separate links.
+	 */
+	bool dual_link;
+};
+
+#endif /* __DRM_TIMINGS_H__ */
-- 
2.7.4

[PATCH v2 4/9] drm/timings: Add link flags

[Fabrizio Castro]

We need more information to describe dual-LVDS links, therefore
introduce link_flags.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

---
v1->v2:
* new patch

 include/drm/drm_timings.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/include/drm/drm_timings.h b/include/drm/drm_timings.h
index 4af8814..58fbf1b 100644
--- a/include/drm/drm_timings.h
+++ b/include/drm/drm_timings.h
@@ -1,4 +1,6 @@
 /*
+ * Copyright (C) 2019 Renesas Electronics Corporation
+ *
  * Permission to use, copy, modify, distribute, and sell this software and its
  * documentation for any purpose is hereby granted without fee, provided that
  * the above copyright notice appear in all copies and that both that copyright
@@ -21,6 +23,24 @@
 #ifndef __DRM_TIMINGS_H__
 #define __DRM_TIMINGS_H__
 
+#include <linux/bits.h>
+
+/**
+ * enum drm_link_flags - link_flags for &drm_timings
+ *
+ * This enum defines the details of the link.
+ *
+ * @DRM_LINK_DUAL_LVDS_ODD_EVEN:	Dual-LVDS link, with odd pixels (1,3,5,
+ *					etc.) coming through the first port, and
+ *					even pixels (0,2,4,etc.) coming through
+ *					the second port. If not specified for a
+ *					dual-LVDS panel, it is assumed that even
+ *					pixels are coming through the first port
+ */
+enum drm_link_flags {
+	DRM_LINK_DUAL_LVDS_ODD_EVEN = BIT(0),
+};
+
 /**
  * struct drm_timings - timing information
  */
@@ -55,6 +75,12 @@ struct drm_timings {
 	 * and odd-numbered pixels are received on separate links.
 	 */
 	bool dual_link;
+	/**
+	 * @link_flags
+	 *
+	 * Provides detailed information about the link.
+	 */
+	enum drm_link_flags link_flags;
 };
 
 #endif /* __DRM_TIMINGS_H__ */
-- 
2.7.4

[PATCH v2 5/9] drm/panel: Add timings field to drm_panel

[Fabrizio Castro]

We need to know if the panel supports dual-link, similarly
to bridges, therefore add a reference to drm_timings in
drm_panel.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

---
v1->v2:
* new patch

 include/drm/drm_panel.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 8c738c0..cd6ff07 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -26,6 +26,7 @@
 
 #include <linux/errno.h>
 #include <linux/list.h>
+#include <drm/drm_timings.h>
 
 struct device_node;
 struct drm_connector;
@@ -81,6 +82,7 @@ struct drm_panel_funcs {
  * struct drm_panel - DRM panel object
  * @drm: DRM device owning the panel
  * @connector: DRM connector that the panel is attached to
+ * @timings: timing information
  * @dev: parent device of the panel
  * @link: link from panel device (supplier) to DRM device (consumer)
  * @funcs: operations that can be performed on the panel
@@ -89,6 +91,7 @@ struct drm_panel_funcs {
 struct drm_panel {
 	struct drm_device *drm;
 	struct drm_connector *connector;
+	const struct drm_timings *timings;
 	struct device *dev;
 
 	const struct drm_panel_funcs *funcs;
-- 
2.7.4

[PATCH v2 8/9] drm/panel: lvds: Add support for the IDK-2121WR

[Fabrizio Castro]

The IDK-2121WR from Advantech is a dual-LVDS display.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

---
v1->v2:
* new patch

 drivers/gpu/drm/panel/panel-lvds.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-lvds.c b/drivers/gpu/drm/panel/panel-lvds.c
index 1ec57d0..2cd41757 100644
--- a/drivers/gpu/drm/panel/panel-lvds.c
+++ b/drivers/gpu/drm/panel/panel-lvds.c
@@ -22,6 +22,7 @@
 
 #include <drm/drm_crtc.h>
 #include <drm/drm_panel.h>
+#include <drm/drm_timings.h>
 
 struct panel_lvds {
 	struct drm_panel panel;
@@ -259,6 +260,7 @@ static int panel_lvds_probe(struct platform_device *pdev)
 	/* Register the panel. */
 	drm_panel_init(&lvds->panel);
 	lvds->panel.dev = lvds->dev;
+	lvds->panel.timings = of_device_get_match_data(lvds->dev);
 	lvds->panel.funcs = &panel_lvds_funcs;
 
 	ret = drm_panel_add(&lvds->panel);
@@ -287,7 +289,13 @@ static int panel_lvds_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct drm_timings advantech_idk_2121wr = {
+	.dual_link = true,
+	.link_flags = DRM_LINK_DUAL_LVDS_ODD_EVEN,
+};
+
 static const struct of_device_id panel_lvds_of_table[] = {
+	{ .compatible = "advantech,idk-2121wr", .data = &advantech_idk_2121wr},
 	{ .compatible = "panel-lvds", },
 	{ /* Sentinel */ },
 };
-- 
2.7.4

Re: [PATCH v2 1/9] dt-bindings: panel: lvds: Add dual-link LVDS display support

[Laurent Pinchart]

Hi Fabrizio,

On Thu, Aug 15, 2019 at 12:04:25PM +0100, Fabrizio Castro wrote:
> Dual-link LVDS displays have two ports, therefore document this
> with the bindings.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> 
> ---
> v1->v2:
> * Reworked the description of the ports property
> * lvds0_panel_in in the example has been renamed to panel_in0
> * lvds1_panel_in in the example has been renamed to panel_in1
> 
> Laurent,
> 
> in linux-next they are now working with:
> Documentation/devicetree/bindings/display/panel/lvds.yaml

Documentation/devicetree/bindings/display/panel/lvds.yaml is in
drm-misc-next, so I would advise rebasing on top of that.

> What should I do here?
> 
>  .../bindings/display/panel/panel-lvds.txt          | 95 ++++++++++++++++------
>  1 file changed, 71 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> index 250850a..5231243 100644
> --- a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> @@ -41,7 +41,12 @@ 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.
> +  (for a single link panel) corresponding to the panel LVDS input, or two port
> +  subnodes (for a dual link panel) corresponding to the panel LVDS inputs.
> +  Dual-link LVDS panels expect even pixels (0, 2, 4, etc.) and odd pixels (1, 3,
> +  5, etc.) on different input ports, it's up to the panel-specific bindings to
> +  specify what port is expecting even pixels, and what port is expecting odd
> +  pixels.
>  
>  
>  LVDS data mappings are defined as follows.
> @@ -92,30 +97,72 @@ 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>;
> +Single port:
> +	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>;
> +			};
> +		};
>  	};
>  
> -	port {
> -		panel_in: endpoint {
> -			remote-endpoint = <&lvds_encoder>;
> +Two ports:
> +	panel {
> +		compatible = "advantech,idk-2121wr", "panel-lvds";
> +
> +		width-mm = <476>;
> +		height-mm = <268>;
> +
> +		data-mapping = "vesa-24";
> +
> +		panel-timing {
> +			clock-frequency = <148500000>;
> +			hactive = <1920>;
> +			vactive = <1080>;
> +			hsync-len = <44>;
> +			hfront-porch = <88>;
> +			hback-porch = <148>;
> +			vfront-porch = <4>;
> +			vback-porch = <36>;
> +			vsync-len = <5>;
> +		};
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +				panel_in0: endpoint {
> +					remote-endpoint = <&lvds0_out>;
> +				};
> +			};
> +
> +			port@1 {
> +				reg = <1>;
> +				panel_in1: endpoint {
> +					remote-endpoint = <&lvds1_out>;
> +				};
> +			};
>  		};
>  	};
> -};

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/9] dt-bindings: display: Add bindings for Advantech IDK-2121WR

[Laurent Pinchart]

Hi Fabrizio,

On Thu, Aug 15, 2019 at 12:04:26PM +0100, Fabrizio Castro wrote:
> This panel is handled through the generic lvds-panel bindings,
> so only needs its additional compatible specified.
> 
> Some panel-specific documentation can be found here:
> https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> 
> ---
> v1->v2:
> * Reworked according to Laurent's feedback
> * Renamed lvds0_panel_in to panel_in0
> * Renamed lvds1_panel_in to panel_in1
> 
> Laurent,
> 
> Should this be a .yaml file already?

It's not a strict requirement, but I'm sure Rob would really appreciate.
I've converted a DT binding to yaml recently (for a panel too), and I
have to say I'm impressed by the validation yaml brings, both for DT
sources and for DT bindings. It even validates the example in the
bindings, which is great. Documentation/devicetree/writing-schema.md
should give you a few pointers to get started (in particular make sure
you run make dt_binding_check for your new bindings).

>  .../display/panel/advantech,idk-2121wr.txt         | 56 ++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
> new file mode 100644
> index 0000000..6ee1d1b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
> @@ -0,0 +1,56 @@
> +Advantech Co., Ltd. IDK-2121WR 21.5" LVDS panel
> +===============================================
> +
> +Required properties:
> +- compatible: should be "advantech,idk-2121wr" followed by "panel-lvds"
> +
> +This binding is compatible with the lvds-panel binding, which is specified
> +in panel-lvds.txt in this directory.
> +The panel operates in dual-link mode and thus requires two port nodes,
> +the first port node expects odd pixels (1, 3, 5, etc.) and the second port
> +expects even pixels (0, 2, 4, etc.).
> +
> +Example
> +-------
> +
> +	panel {
> +		compatible = "advantech,idk-2121wr", "panel-lvds";
> +
> +		width-mm = <476>;
> +		height-mm = <268>;
> +
> +		data-mapping = "vesa-24";
> +
> +		panel-timing {
> +			clock-frequency = <148500000>;
> +			hactive = <1920>;
> +			vactive = <1080>;
> +			hsync-len = <44>;
> +			hfront-porch = <88>;
> +			hback-porch = <148>;
> +			vfront-porch = <4>;
> +			vback-porch = <36>;
> +			vsync-len = <5>;
> +		};
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				/* Odd pixels */
> +				reg = <0>;
> +				panel_in0: endpoint {
> +					remote-endpoint = <&lvds0_out>;
> +				};
> +			};
> +
> +			port@1 {
> +				/* Even pixels */
> +				reg = <1>;
> +				panel_in1: endpoint {
> +					remote-endpoint = <&lvds1_out>;
> +				};
> +			};
> +		};
> +	};
> -- 
> 2.7.4
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 4/9] drm/timings: Add link flags

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Thu, Aug 15, 2019 at 12:04:28PM +0100, Fabrizio Castro wrote:
> We need more information to describe dual-LVDS links, therefore
> introduce link_flags.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> 
> ---
> v1->v2:
> * new patch
> 
>  include/drm/drm_timings.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/include/drm/drm_timings.h b/include/drm/drm_timings.h
> index 4af8814..58fbf1b 100644
> --- a/include/drm/drm_timings.h
> +++ b/include/drm/drm_timings.h
> @@ -1,4 +1,6 @@
>  /*
> + * Copyright (C) 2019 Renesas Electronics Corporation
> + *
>   * Permission to use, copy, modify, distribute, and sell this software and its
>   * documentation for any purpose is hereby granted without fee, provided that
>   * the above copyright notice appear in all copies and that both that copyright
> @@ -21,6 +23,24 @@
>  #ifndef __DRM_TIMINGS_H__
>  #define __DRM_TIMINGS_H__
>  
> +#include <linux/bits.h>
> +
> +/**
> + * enum drm_link_flags - link_flags for &drm_timings
> + *
> + * This enum defines the details of the link.
> + *
> + * @DRM_LINK_DUAL_LVDS_ODD_EVEN:	Dual-LVDS link, with odd pixels (1,3,5,
> + *					etc.) coming through the first port, and
> + *					even pixels (0,2,4,etc.) coming through
> + *					the second port. If not specified for a
> + *					dual-LVDS panel, it is assumed that even
> + *					pixels are coming through the first port
> + */
> +enum drm_link_flags {

The text will be easier to read if you inline it here.

	/**
	 * @DRM_LINK_DUAL_LVDS_ODD_EVEN: Dual-LVDS link, with odd pixels (1,3,5,
	 * etc.) coming through the first port, and  even pixels (0,2,4,etc.)
	 ...

> +	DRM_LINK_DUAL_LVDS_ODD_EVEN = BIT(0),

I would remove the dual_link field and add a DRM_LINK_DUAL_LVDS (or
alternatively two flags, dual lvds odd-even and dual lvds even-odd).

> +};
> +
>  /**
>   * struct drm_timings - timing information
>   */
> @@ -55,6 +75,12 @@ struct drm_timings {
>  	 * and odd-numbered pixels are received on separate links.
>  	 */
>  	bool dual_link;
> +	/**
> +	 * @link_flags
> +	 *
> +	 * Provides detailed information about the link.

I think this calls for a bit more information than "detailed
information". What information do you want to store in this field ?

> +	 */
> +	enum drm_link_flags link_flags;
>  };
>  
>  #endif /* __DRM_TIMINGS_H__ */
> -- 
> 2.7.4
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 5/9] drm/panel: Add timings field to drm_panel

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Thu, Aug 15, 2019 at 12:04:29PM +0100, Fabrizio Castro wrote:
> We need to know if the panel supports dual-link, similarly
> to bridges, therefore add a reference to drm_timings in
> drm_panel.

Panels may also need to report setup/hold time, so it's not about
dual-link only. I would make this explicit in the commit message.

> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> 
> ---
> v1->v2:
> * new patch
> 
>  include/drm/drm_panel.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 8c738c0..cd6ff07 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -26,6 +26,7 @@
>  
>  #include <linux/errno.h>
>  #include <linux/list.h>
> +#include <drm/drm_timings.h>

You can just add a forward-declaration of struct drm_timing.

>  
>  struct device_node;
>  struct drm_connector;
> @@ -81,6 +82,7 @@ struct drm_panel_funcs {
>   * struct drm_panel - DRM panel object
>   * @drm: DRM device owning the panel
>   * @connector: DRM connector that the panel is attached to
> + * @timings: timing information
>   * @dev: parent device of the panel
>   * @link: link from panel device (supplier) to DRM device (consumer)
>   * @funcs: operations that can be performed on the panel
> @@ -89,6 +91,7 @@ struct drm_panel_funcs {
>  struct drm_panel {
>  	struct drm_device *drm;
>  	struct drm_connector *connector;
> +	const struct drm_timings *timings;
>  	struct device *dev;
>  
>  	const struct drm_panel_funcs *funcs;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings

[Laurent Pinchart]

Hi Fabrizio,

(CC'ing Greg as the architect of the SPDX move)

On Thu, Aug 15, 2019 at 12:04:27PM +0100, Fabrizio Castro wrote:
> The information represented by drm_bridge_timings is also
> needed by panels, therefore rename drm_bridge_timings to
> drm_timings.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Link: https://www.spinics.net/lists/linux-renesas-soc/msg43271.html
> 
> ---
> v1->v2:
> * new patch
> 
> I have copied the license from include/drm/drm_bridge.h as that's
> where the struct originally came from. What's the right SPDX license
> to use in this case?

https://wiki.spdx.org/view/Legal_Team/Decisions/Dealing_with_Public_Domain_within_SPDX_Files

Greg, any idea on how we should handle this ?

>  drivers/gpu/drm/bridge/dumb-vga-dac.c |  6 ++--
>  drivers/gpu/drm/bridge/sii902x.c      |  2 +-
>  drivers/gpu/drm/bridge/thc63lvd1024.c |  2 +-
>  drivers/gpu/drm/bridge/ti-tfp410.c    |  6 ++--
>  drivers/gpu/drm/pl111/pl111_display.c |  2 +-
>  include/drm/drm_bridge.h              | 40 ++---------------------
>  include/drm/drm_timings.h             | 60 +++++++++++++++++++++++++++++++++++
>  7 files changed, 71 insertions(+), 47 deletions(-)
>  create mode 100644 include/drm/drm_timings.h
> 
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> index d32885b..bb1d928 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -228,7 +228,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
>   * NOTE: the ADV7123EP seems to have other timings and need a new timings
>   * set if used.
>   */
> -static const struct drm_bridge_timings default_dac_timings = {
> +static const struct drm_timings default_dac_timings = {
>  	/* Timing specifications, datasheet page 7 */
>  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
>  	.setup_time_ps = 500,
> @@ -239,7 +239,7 @@ static const struct drm_bridge_timings default_dac_timings = {
>   * Information taken from the THS8134, THS8134A, THS8134B datasheet named
>   * "SLVS205D", dated May 1990, revised March 2000.
>   */
> -static const struct drm_bridge_timings ti_ths8134_dac_timings = {
> +static const struct drm_timings ti_ths8134_dac_timings = {
>  	/* From timing diagram, datasheet page 9 */
>  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
>  	/* From datasheet, page 12 */
> @@ -252,7 +252,7 @@ static const struct drm_bridge_timings ti_ths8134_dac_timings = {
>   * Information taken from the THS8135 datasheet named "SLAS343B", dated
>   * May 2001, revised April 2013.
>   */
> -static const struct drm_bridge_timings ti_ths8135_dac_timings = {
> +static const struct drm_timings ti_ths8135_dac_timings = {
>  	/* From timing diagram, datasheet page 14 */
>  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
>  	/* From datasheet, page 16 */
> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> index dd7aa46..0c63065 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -914,7 +914,7 @@ static int sii902x_i2c_bypass_deselect(struct i2c_mux_core *mux, u32 chan_id)
>  	return 0;
>  }
>  
> -static const struct drm_bridge_timings default_sii902x_timings = {
> +static const struct drm_timings default_sii902x_timings = {
>  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE
>  		 | DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE
>  		 | DRM_BUS_FLAG_DE_HIGH,
> diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c
> index 3d74129b..9047a9e 100644
> --- a/drivers/gpu/drm/bridge/thc63lvd1024.c
> +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
> @@ -34,7 +34,7 @@ struct thc63_dev {
>  	struct drm_bridge bridge;
>  	struct drm_bridge *next;
>  
> -	struct drm_bridge_timings timings;
> +	struct drm_timings timings;
>  };
>  
>  static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
> diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
> index dbf35c7..c086b06c 100644
> --- a/drivers/gpu/drm/bridge/ti-tfp410.c
> +++ b/drivers/gpu/drm/bridge/ti-tfp410.c
> @@ -32,7 +32,7 @@ struct tfp410 {
>  	struct delayed_work	hpd_work;
>  	struct gpio_desc	*powerdown;
>  
> -	struct drm_bridge_timings timings;
> +	struct drm_timings timings;
>  
>  	struct device *dev;
>  };
> @@ -190,7 +190,7 @@ static irqreturn_t tfp410_hpd_irq_thread(int irq, void *arg)
>  	return IRQ_HANDLED;
>  }
>  
> -static const struct drm_bridge_timings tfp410_default_timings = {
> +static const struct drm_timings tfp410_default_timings = {
>  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE
>  			 | DRM_BUS_FLAG_DE_HIGH,
>  	.setup_time_ps = 1200,
> @@ -199,7 +199,7 @@ static const struct drm_bridge_timings tfp410_default_timings = {
>  
>  static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c)
>  {
> -	struct drm_bridge_timings *timings = &dvi->timings;
> +	struct drm_timings *timings = &dvi->timings;
>  	struct device_node *ep;
>  	u32 pclk_sample = 0;
>  	u32 bus_width = 24;
> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> index 15d2755..c82b21f 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> @@ -188,7 +188,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
>  	}
>  
>  	if (bridge) {
> -		const struct drm_bridge_timings *btimings = bridge->timings;
> +		const struct drm_timings *btimings = bridge->timings;
>  
>  		/*
>  		 * Here is when things get really fun. Sometimes the bridge
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 7616f65..8270a38 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -27,9 +27,9 @@
>  #include <linux/ctype.h>
>  #include <drm/drm_mode_object.h>
>  #include <drm/drm_modes.h>
> +#include <drm/drm_timings.h>
>  
>  struct drm_bridge;
> -struct drm_bridge_timings;
>  struct drm_panel;
>  
>  /**
> @@ -337,42 +337,6 @@ struct drm_bridge_funcs {
>  };
>  
>  /**
> - * struct drm_bridge_timings - timing information for the bridge
> - */
> -struct drm_bridge_timings {
> -	/**
> -	 * @input_bus_flags:
> -	 *
> -	 * Tells what additional settings for the pixel data on the bus
> -	 * this bridge requires (like pixel signal polarity). See also
> -	 * &drm_display_info->bus_flags.
> -	 */
> -	u32 input_bus_flags;
> -	/**
> -	 * @setup_time_ps:
> -	 *
> -	 * Defines the time in picoseconds the input data lines must be
> -	 * stable before the clock edge.
> -	 */
> -	u32 setup_time_ps;
> -	/**
> -	 * @hold_time_ps:
> -	 *
> -	 * Defines the time in picoseconds taken for the bridge to sample the
> -	 * input signal after the clock edge.
> -	 */
> -	u32 hold_time_ps;
> -	/**
> -	 * @dual_link:
> -	 *
> -	 * True if the bus operates in dual-link mode. The exact meaning is
> -	 * dependent on the bus type. For LVDS buses, this indicates that even-
> -	 * and odd-numbered pixels are received on separate links.
> -	 */
> -	bool dual_link;
> -};
> -
> -/**
>   * struct drm_bridge - central DRM bridge control structure
>   */
>  struct drm_bridge {
> @@ -393,7 +357,7 @@ struct drm_bridge {
>  	 *
>  	 * the timing specification for the bridge, if any (may be NULL)
>  	 */
> -	const struct drm_bridge_timings *timings;
> +	const struct drm_timings *timings;
>  	/** @funcs: control functions */
>  	const struct drm_bridge_funcs *funcs;
>  	/** @driver_private: pointer to the bridge driver's internal context */
> diff --git a/include/drm/drm_timings.h b/include/drm/drm_timings.h
> new file mode 100644
> index 0000000..4af8814
> --- /dev/null
> +++ b/include/drm/drm_timings.h
> @@ -0,0 +1,60 @@
> +/*
> + * Permission to use, copy, modify, distribute, and sell this software and its
> + * documentation for any purpose is hereby granted without fee, provided that
> + * the above copyright notice appear in all copies and that both that copyright
> + * notice and this permission notice appear in supporting documentation, and
> + * that the name of the copyright holders not be used in advertising or
> + * publicity pertaining to distribution of the software without specific,
> + * written prior permission.  The copyright holders make no representations
> + * about the suitability of this software for any purpose.  It is provided "as
> + * is" without express or implied warranty.
> + *
> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
> + * OF THIS SOFTWARE.
> + */
> +
> +#ifndef __DRM_TIMINGS_H__
> +#define __DRM_TIMINGS_H__
> +
> +/**
> + * struct drm_timings - timing information

This dangerously relates to video timings. I would name the structure
drm_bus_timings, or drm_bus_params (or something similar) as it contains
more than timings.

> + */
> +struct drm_timings {
> +	/**
> +	 * @input_bus_flags:
> +	 *
> +	 * Tells what additional settings for the pixel data on the bus
> +	 * are required (like pixel signal polarity). See also
> +	 * &drm_display_info->bus_flags.
> +	 */
> +	u32 input_bus_flags;
> +	/**
> +	 * @setup_time_ps:
> +	 *
> +	 * Defines the time in picoseconds the input data lines must be
> +	 * stable before the clock edge.
> +	 */
> +	u32 setup_time_ps;
> +	/**
> +	 * @hold_time_ps:
> +	 *
> +	 * Defines the time in picoseconds taken for the bridge to sample the
> +	 * input signal after the clock edge.
> +	 */
> +	u32 hold_time_ps;
> +	/**
> +	 * @dual_link:
> +	 *
> +	 * True if the bus operates in dual-link mode. The exact meaning is
> +	 * dependent on the bus type. For LVDS buses, this indicates that even-
> +	 * and odd-numbered pixels are received on separate links.
> +	 */
> +	bool dual_link;
> +};
> +
> +#endif /* __DRM_TIMINGS_H__ */

-- 
Regards,

Laurent Pinchart

RE: [PATCH v2 1/9] dt-bindings: panel: lvds: Add dual-link LVDS display support

[Fabrizio Castro]

Hi Laurent,

Thank you for your feedback!

> From: linux-kernel-owner@vger.kernel.org <linux-kernel-owner@vger.kernel.org> On Behalf Of Laurent Pinchart
> Sent: 15 August 2019 12:45
> Subject: Re: [PATCH v2 1/9] dt-bindings: panel: lvds: Add dual-link LVDS display support
> 
> Hi Fabrizio,
> 
> On Thu, Aug 15, 2019 at 12:04:25PM +0100, Fabrizio Castro wrote:
> > Dual-link LVDS displays have two ports, therefore document this
> > with the bindings.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >
> > ---
> > v1->v2:
> > * Reworked the description of the ports property
> > * lvds0_panel_in in the example has been renamed to panel_in0
> > * lvds1_panel_in in the example has been renamed to panel_in1
> >
> > Laurent,
> >
> > in linux-next they are now working with:
> > Documentation/devicetree/bindings/display/panel/lvds.yaml
> 
> Documentation/devicetree/bindings/display/panel/lvds.yaml is in
> drm-misc-next, so I would advise rebasing on top of that.

Will do.

Thanks,
Fab

> 
> > What should I do here?
> >
> >  .../bindings/display/panel/panel-lvds.txt          | 95 ++++++++++++++++------
> >  1 file changed, 71 insertions(+), 24 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> > index 250850a..5231243 100644
> > --- a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> > +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> > @@ -41,7 +41,12 @@ 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.
> > +  (for a single link panel) corresponding to the panel LVDS input, or two port
> > +  subnodes (for a dual link panel) corresponding to the panel LVDS inputs.
> > +  Dual-link LVDS panels expect even pixels (0, 2, 4, etc.) and odd pixels (1, 3,
> > +  5, etc.) on different input ports, it's up to the panel-specific bindings to
> > +  specify what port is expecting even pixels, and what port is expecting odd
> > +  pixels.
> >
> >
> >  LVDS data mappings are defined as follows.
> > @@ -92,30 +97,72 @@ 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>;
> > +Single port:
> > +	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>;
> > +			};
> > +		};
> >  	};
> >
> > -	port {
> > -		panel_in: endpoint {
> > -			remote-endpoint = <&lvds_encoder>;
> > +Two ports:
> > +	panel {
> > +		compatible = "advantech,idk-2121wr", "panel-lvds";
> > +
> > +		width-mm = <476>;
> > +		height-mm = <268>;
> > +
> > +		data-mapping = "vesa-24";
> > +
> > +		panel-timing {
> > +			clock-frequency = <148500000>;
> > +			hactive = <1920>;
> > +			vactive = <1080>;
> > +			hsync-len = <44>;
> > +			hfront-porch = <88>;
> > +			hback-porch = <148>;
> > +			vfront-porch = <4>;
> > +			vback-porch = <36>;
> > +			vsync-len = <5>;
> > +		};
> > +
> > +		ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			port@0 {
> > +				reg = <0>;
> > +				panel_in0: endpoint {
> > +					remote-endpoint = <&lvds0_out>;
> > +				};
> > +			};
> > +
> > +			port@1 {
> > +				reg = <1>;
> > +				panel_in1: endpoint {
> > +					remote-endpoint = <&lvds1_out>;
> > +				};
> > +			};
> >  		};
> >  	};
> > -};
> 
> --
> Regards,
> 
> Laurent Pinchart

RE: [PATCH v2 2/9] dt-bindings: display: Add bindings for Advantech IDK-2121WR

[Fabrizio Castro]

Hi Laurent,

Thank you for your feedback!

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 15 August 2019 12:47
> Subject: Re: [PATCH v2 2/9] dt-bindings: display: Add bindings for Advantech IDK-2121WR
> 
> Hi Fabrizio,
> 
> On Thu, Aug 15, 2019 at 12:04:26PM +0100, Fabrizio Castro wrote:
> > This panel is handled through the generic lvds-panel bindings,
> > so only needs its additional compatible specified.
> >
> > Some panel-specific documentation can be found here:
> > https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >
> > ---
> > v1->v2:
> > * Reworked according to Laurent's feedback
> > * Renamed lvds0_panel_in to panel_in0
> > * Renamed lvds1_panel_in to panel_in1
> >
> > Laurent,
> >
> > Should this be a .yaml file already?
> 
> It's not a strict requirement, but I'm sure Rob would really appreciate.
> I've converted a DT binding to yaml recently (for a panel too), and I
> have to say I'm impressed by the validation yaml brings, both for DT
> sources and for DT bindings. It even validates the example in the
> bindings, which is great. Documentation/devicetree/writing-schema.md
> should give you a few pointers to get started (in particular make sure
> you run make dt_binding_check for your new bindings).

Will give this a try.

Thanks,
Fab

> 
> >  .../display/panel/advantech,idk-2121wr.txt         | 56 ++++++++++++++++++++++
> >  1 file changed, 56 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
> b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
> > new file mode 100644
> > index 0000000..6ee1d1b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
> > @@ -0,0 +1,56 @@
> > +Advantech Co., Ltd. IDK-2121WR 21.5" LVDS panel
> > +===============================================
> > +
> > +Required properties:
> > +- compatible: should be "advantech,idk-2121wr" followed by "panel-lvds"
> > +
> > +This binding is compatible with the lvds-panel binding, which is specified
> > +in panel-lvds.txt in this directory.
> > +The panel operates in dual-link mode and thus requires two port nodes,
> > +the first port node expects odd pixels (1, 3, 5, etc.) and the second port
> > +expects even pixels (0, 2, 4, etc.).
> > +
> > +Example
> > +-------
> > +
> > +	panel {
> > +		compatible = "advantech,idk-2121wr", "panel-lvds";
> > +
> > +		width-mm = <476>;
> > +		height-mm = <268>;
> > +
> > +		data-mapping = "vesa-24";
> > +
> > +		panel-timing {
> > +			clock-frequency = <148500000>;
> > +			hactive = <1920>;
> > +			vactive = <1080>;
> > +			hsync-len = <44>;
> > +			hfront-porch = <88>;
> > +			hback-porch = <148>;
> > +			vfront-porch = <4>;
> > +			vback-porch = <36>;
> > +			vsync-len = <5>;
> > +		};
> > +
> > +		ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			port@0 {
> > +				/* Odd pixels */
> > +				reg = <0>;
> > +				panel_in0: endpoint {
> > +					remote-endpoint = <&lvds0_out>;
> > +				};
> > +			};
> > +
> > +			port@1 {
> > +				/* Even pixels */
> > +				reg = <1>;
> > +				panel_in1: endpoint {
> > +					remote-endpoint = <&lvds1_out>;
> > +				};
> > +			};
> > +		};
> > +	};
> > --
> > 2.7.4
> >
> 
> --
> Regards,
> 
> Laurent Pinchart

RE: [PATCH v2 5/9] drm/panel: Add timings field to drm_panel

[Fabrizio Castro]

Hello Laurent,

Thank you for your feedback!

> From: linux-renesas-soc-owner@vger.kernel.org <linux-renesas-soc-owner@vger.kernel.org> On Behalf Of Laurent Pinchart
> Sent: 15 August 2019 13:03
> Subject: Re: [PATCH v2 5/9] drm/panel: Add timings field to drm_panel
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> On Thu, Aug 15, 2019 at 12:04:29PM +0100, Fabrizio Castro wrote:
> > We need to know if the panel supports dual-link, similarly
> > to bridges, therefore add a reference to drm_timings in
> > drm_panel.
> 
> Panels may also need to report setup/hold time, so it's not about
> dual-link only. I would make this explicit in the commit message.

Ok

> 
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >
> > ---
> > v1->v2:
> > * new patch
> >
> >  include/drm/drm_panel.h | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> > index 8c738c0..cd6ff07 100644
> > --- a/include/drm/drm_panel.h
> > +++ b/include/drm/drm_panel.h
> > @@ -26,6 +26,7 @@
> >
> >  #include <linux/errno.h>
> >  #include <linux/list.h>
> > +#include <drm/drm_timings.h>
> 
> You can just add a forward-declaration of struct drm_timing.

Ok

Thanks,
Fab

> 
> >
> >  struct device_node;
> >  struct drm_connector;
> > @@ -81,6 +82,7 @@ struct drm_panel_funcs {
> >   * struct drm_panel - DRM panel object
> >   * @drm: DRM device owning the panel
> >   * @connector: DRM connector that the panel is attached to
> > + * @timings: timing information
> >   * @dev: parent device of the panel
> >   * @link: link from panel device (supplier) to DRM device (consumer)
> >   * @funcs: operations that can be performed on the panel
> > @@ -89,6 +91,7 @@ struct drm_panel_funcs {
> >  struct drm_panel {
> >  	struct drm_device *drm;
> >  	struct drm_connector *connector;
> > +	const struct drm_timings *timings;
> >  	struct device *dev;
> >
> >  	const struct drm_panel_funcs *funcs;
> 
> --
> Regards,
> 
> Laurent Pinchart

RE: [PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings

[Fabrizio Castro]

Hello Laurent,

Thank you for your feedback!

> From: linux-kernel-owner@vger.kernel.org <linux-kernel-owner@vger.kernel.org> On Behalf Of Laurent Pinchart
> Sent: 15 August 2019 14:19
> Subject: Re: [PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings
> 
> Hi Fabrizio,
> 
> (CC'ing Greg as the architect of the SPDX move)
> 
> On Thu, Aug 15, 2019 at 12:04:27PM +0100, Fabrizio Castro wrote:
> > The information represented by drm_bridge_timings is also
> > needed by panels, therefore rename drm_bridge_timings to
> > drm_timings.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > Link: https://www.spinics.net/lists/linux-renesas-soc/msg43271.html
> >
> > ---
> > v1->v2:
> > * new patch
> >
> > I have copied the license from include/drm/drm_bridge.h as that's
> > where the struct originally came from. What's the right SPDX license
> > to use in this case?
> 
> https://wiki.spdx.org/view/Legal_Team/Decisions/Dealing_with_Public_Domain_within_SPDX_Files
> 
> Greg, any idea on how we should handle this ?
> 
> >  drivers/gpu/drm/bridge/dumb-vga-dac.c |  6 ++--
> >  drivers/gpu/drm/bridge/sii902x.c      |  2 +-
> >  drivers/gpu/drm/bridge/thc63lvd1024.c |  2 +-
> >  drivers/gpu/drm/bridge/ti-tfp410.c    |  6 ++--
> >  drivers/gpu/drm/pl111/pl111_display.c |  2 +-
> >  include/drm/drm_bridge.h              | 40 ++---------------------
> >  include/drm/drm_timings.h             | 60 +++++++++++++++++++++++++++++++++++
> >  7 files changed, 71 insertions(+), 47 deletions(-)
> >  create mode 100644 include/drm/drm_timings.h
> >
> > diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > index d32885b..bb1d928 100644
> > --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > @@ -228,7 +228,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
> >   * NOTE: the ADV7123EP seems to have other timings and need a new timings
> >   * set if used.
> >   */
> > -static const struct drm_bridge_timings default_dac_timings = {
> > +static const struct drm_timings default_dac_timings = {
> >  	/* Timing specifications, datasheet page 7 */
> >  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
> >  	.setup_time_ps = 500,
> > @@ -239,7 +239,7 @@ static const struct drm_bridge_timings default_dac_timings = {
> >   * Information taken from the THS8134, THS8134A, THS8134B datasheet named
> >   * "SLVS205D", dated May 1990, revised March 2000.
> >   */
> > -static const struct drm_bridge_timings ti_ths8134_dac_timings = {
> > +static const struct drm_timings ti_ths8134_dac_timings = {
> >  	/* From timing diagram, datasheet page 9 */
> >  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
> >  	/* From datasheet, page 12 */
> > @@ -252,7 +252,7 @@ static const struct drm_bridge_timings ti_ths8134_dac_timings = {
> >   * Information taken from the THS8135 datasheet named "SLAS343B", dated
> >   * May 2001, revised April 2013.
> >   */
> > -static const struct drm_bridge_timings ti_ths8135_dac_timings = {
> > +static const struct drm_timings ti_ths8135_dac_timings = {
> >  	/* From timing diagram, datasheet page 14 */
> >  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
> >  	/* From datasheet, page 16 */
> > diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> > index dd7aa46..0c63065 100644
> > --- a/drivers/gpu/drm/bridge/sii902x.c
> > +++ b/drivers/gpu/drm/bridge/sii902x.c
> > @@ -914,7 +914,7 @@ static int sii902x_i2c_bypass_deselect(struct i2c_mux_core *mux, u32 chan_id)
> >  	return 0;
> >  }
> >
> > -static const struct drm_bridge_timings default_sii902x_timings = {
> > +static const struct drm_timings default_sii902x_timings = {
> >  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE
> >  		 | DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE
> >  		 | DRM_BUS_FLAG_DE_HIGH,
> > diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c
> > index 3d74129b..9047a9e 100644
> > --- a/drivers/gpu/drm/bridge/thc63lvd1024.c
> > +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
> > @@ -34,7 +34,7 @@ struct thc63_dev {
> >  	struct drm_bridge bridge;
> >  	struct drm_bridge *next;
> >
> > -	struct drm_bridge_timings timings;
> > +	struct drm_timings timings;
> >  };
> >
> >  static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
> > diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
> > index dbf35c7..c086b06c 100644
> > --- a/drivers/gpu/drm/bridge/ti-tfp410.c
> > +++ b/drivers/gpu/drm/bridge/ti-tfp410.c
> > @@ -32,7 +32,7 @@ struct tfp410 {
> >  	struct delayed_work	hpd_work;
> >  	struct gpio_desc	*powerdown;
> >
> > -	struct drm_bridge_timings timings;
> > +	struct drm_timings timings;
> >
> >  	struct device *dev;
> >  };
> > @@ -190,7 +190,7 @@ static irqreturn_t tfp410_hpd_irq_thread(int irq, void *arg)
> >  	return IRQ_HANDLED;
> >  }
> >
> > -static const struct drm_bridge_timings tfp410_default_timings = {
> > +static const struct drm_timings tfp410_default_timings = {
> >  	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE
> >  			 | DRM_BUS_FLAG_DE_HIGH,
> >  	.setup_time_ps = 1200,
> > @@ -199,7 +199,7 @@ static const struct drm_bridge_timings tfp410_default_timings = {
> >
> >  static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c)
> >  {
> > -	struct drm_bridge_timings *timings = &dvi->timings;
> > +	struct drm_timings *timings = &dvi->timings;
> >  	struct device_node *ep;
> >  	u32 pclk_sample = 0;
> >  	u32 bus_width = 24;
> > diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> > index 15d2755..c82b21f 100644
> > --- a/drivers/gpu/drm/pl111/pl111_display.c
> > +++ b/drivers/gpu/drm/pl111/pl111_display.c
> > @@ -188,7 +188,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
> >  	}
> >
> >  	if (bridge) {
> > -		const struct drm_bridge_timings *btimings = bridge->timings;
> > +		const struct drm_timings *btimings = bridge->timings;
> >
> >  		/*
> >  		 * Here is when things get really fun. Sometimes the bridge
> > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > index 7616f65..8270a38 100644
> > --- a/include/drm/drm_bridge.h
> > +++ b/include/drm/drm_bridge.h
> > @@ -27,9 +27,9 @@
> >  #include <linux/ctype.h>
> >  #include <drm/drm_mode_object.h>
> >  #include <drm/drm_modes.h>
> > +#include <drm/drm_timings.h>
> >
> >  struct drm_bridge;
> > -struct drm_bridge_timings;
> >  struct drm_panel;
> >
> >  /**
> > @@ -337,42 +337,6 @@ struct drm_bridge_funcs {
> >  };
> >
> >  /**
> > - * struct drm_bridge_timings - timing information for the bridge
> > - */
> > -struct drm_bridge_timings {
> > -	/**
> > -	 * @input_bus_flags:
> > -	 *
> > -	 * Tells what additional settings for the pixel data on the bus
> > -	 * this bridge requires (like pixel signal polarity). See also
> > -	 * &drm_display_info->bus_flags.
> > -	 */
> > -	u32 input_bus_flags;
> > -	/**
> > -	 * @setup_time_ps:
> > -	 *
> > -	 * Defines the time in picoseconds the input data lines must be
> > -	 * stable before the clock edge.
> > -	 */
> > -	u32 setup_time_ps;
> > -	/**
> > -	 * @hold_time_ps:
> > -	 *
> > -	 * Defines the time in picoseconds taken for the bridge to sample the
> > -	 * input signal after the clock edge.
> > -	 */
> > -	u32 hold_time_ps;
> > -	/**
> > -	 * @dual_link:
> > -	 *
> > -	 * True if the bus operates in dual-link mode. The exact meaning is
> > -	 * dependent on the bus type. For LVDS buses, this indicates that even-
> > -	 * and odd-numbered pixels are received on separate links.
> > -	 */
> > -	bool dual_link;
> > -};
> > -
> > -/**
> >   * struct drm_bridge - central DRM bridge control structure
> >   */
> >  struct drm_bridge {
> > @@ -393,7 +357,7 @@ struct drm_bridge {
> >  	 *
> >  	 * the timing specification for the bridge, if any (may be NULL)
> >  	 */
> > -	const struct drm_bridge_timings *timings;
> > +	const struct drm_timings *timings;
> >  	/** @funcs: control functions */
> >  	const struct drm_bridge_funcs *funcs;
> >  	/** @driver_private: pointer to the bridge driver's internal context */
> > diff --git a/include/drm/drm_timings.h b/include/drm/drm_timings.h
> > new file mode 100644
> > index 0000000..4af8814
> > --- /dev/null
> > +++ b/include/drm/drm_timings.h
> > @@ -0,0 +1,60 @@
> > +/*
> > + * Permission to use, copy, modify, distribute, and sell this software and its
> > + * documentation for any purpose is hereby granted without fee, provided that
> > + * the above copyright notice appear in all copies and that both that copyright
> > + * notice and this permission notice appear in supporting documentation, and
> > + * that the name of the copyright holders not be used in advertising or
> > + * publicity pertaining to distribution of the software without specific,
> > + * written prior permission.  The copyright holders make no representations
> > + * about the suitability of this software for any purpose.  It is provided "as
> > + * is" without express or implied warranty.
> > + *
> > + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
> > + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> > + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
> > + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
> > + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> > + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
> > + * OF THIS SOFTWARE.
> > + */
> > +
> > +#ifndef __DRM_TIMINGS_H__
> > +#define __DRM_TIMINGS_H__
> > +
> > +/**
> > + * struct drm_timings - timing information
> 
> This dangerously relates to video timings. I would name the structure
> drm_bus_timings, or drm_bus_params (or something similar) as it contains
> more than timings.

Will rename to drm_bus_timings.

Thanks,
Fab

> 
> > + */
> > +struct drm_timings {
> > +	/**
> > +	 * @input_bus_flags:
> > +	 *
> > +	 * Tells what additional settings for the pixel data on the bus
> > +	 * are required (like pixel signal polarity). See also
> > +	 * &drm_display_info->bus_flags.
> > +	 */
> > +	u32 input_bus_flags;
> > +	/**
> > +	 * @setup_time_ps:
> > +	 *
> > +	 * Defines the time in picoseconds the input data lines must be
> > +	 * stable before the clock edge.
> > +	 */
> > +	u32 setup_time_ps;
> > +	/**
> > +	 * @hold_time_ps:
> > +	 *
> > +	 * Defines the time in picoseconds taken for the bridge to sample the
> > +	 * input signal after the clock edge.
> > +	 */
> > +	u32 hold_time_ps;
> > +	/**
> > +	 * @dual_link:
> > +	 *
> > +	 * True if the bus operates in dual-link mode. The exact meaning is
> > +	 * dependent on the bus type. For LVDS buses, this indicates that even-
> > +	 * and odd-numbered pixels are received on separate links.
> > +	 */
> > +	bool dual_link;
> > +};
> > +
> > +#endif /* __DRM_TIMINGS_H__ */
> 
> --
> Regards,
> 
> Laurent Pinchart

Re: [PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings

[Greg Kroah-Hartman]

On Thu, Aug 15, 2019 at 04:18:38PM +0300, Laurent Pinchart wrote:
> Hi Fabrizio,
> 
> (CC'ing Greg as the architect of the SPDX move)

_one of_, not the one that did the most of he work, that would be Thomas :)

> On Thu, Aug 15, 2019 at 12:04:27PM +0100, Fabrizio Castro wrote:
> > The information represented by drm_bridge_timings is also
> > needed by panels, therefore rename drm_bridge_timings to
> > drm_timings.
> > 
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > Link: https://www.spinics.net/lists/linux-renesas-soc/msg43271.html
> > 
> > ---
> > v1->v2:
> > * new patch
> > 
> > I have copied the license from include/drm/drm_bridge.h as that's
> > where the struct originally came from. What's the right SPDX license
> > to use in this case?
> 
> https://wiki.spdx.org/view/Legal_Team/Decisions/Dealing_with_Public_Domain_within_SPDX_Files
> 
> Greg, any idea on how we should handle this ?

Ugh, what lunacy.  But drm_bridge.h is NOT under any "public domain"
license, so why is that an issue here?  This looks like a "normal" bsd 3
clause license to me, right?

So I would just use "BSD-3-Clause" as the SPDX license here, if I were
doing this patch...

thanks,

greg k-h

Re: [PATCH v2 5/9] drm/panel: Add timings field to drm_panel

[Sam Ravnborg]

Hi Fabrizio

On Thu, Aug 15, 2019 at 12:04:29PM +0100, Fabrizio Castro wrote:
> We need to know if the panel supports dual-link, similarly
> to bridges, therefore add a reference to drm_timings in
> drm_panel.

Why do we need to know this?
Why is it needed in drm_panel and not in some driver specific struct?

I cannot see the full series, as I was copied only on some mails.
Awaiting dri-devel moderator before I can see the rest.

	Sam

> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> 
> ---
> v1->v2:
> * new patch
> 
>  include/drm/drm_panel.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 8c738c0..cd6ff07 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -26,6 +26,7 @@
>  
>  #include <linux/errno.h>
>  #include <linux/list.h>
> +#include <drm/drm_timings.h>
>  
>  struct device_node;
>  struct drm_connector;
> @@ -81,6 +82,7 @@ struct drm_panel_funcs {
>   * struct drm_panel - DRM panel object
>   * @drm: DRM device owning the panel
>   * @connector: DRM connector that the panel is attached to
> + * @timings: timing information
>   * @dev: parent device of the panel
>   * @link: link from panel device (supplier) to DRM device (consumer)
>   * @funcs: operations that can be performed on the panel
> @@ -89,6 +91,7 @@ struct drm_panel_funcs {
>  struct drm_panel {
>  	struct drm_device *drm;
>  	struct drm_connector *connector;
> +	const struct drm_timings *timings;
>  	struct device *dev;
>  
>  	const struct drm_panel_funcs *funcs;
> -- 
> 2.7.4

Re: [PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings

[Laurent Pinchart]

Hi Greg,

On Thu, Aug 15, 2019 at 04:04:00PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Aug 15, 2019 at 04:18:38PM +0300, Laurent Pinchart wrote:
> > Hi Fabrizio,
> > 
> > (CC'ing Greg as the architect of the SPDX move)
> 
> _one of_, not the one that did the most of he work, that would be Thomas :)
> 
> > On Thu, Aug 15, 2019 at 12:04:27PM +0100, Fabrizio Castro wrote:
> > > The information represented by drm_bridge_timings is also
> > > needed by panels, therefore rename drm_bridge_timings to
> > > drm_timings.
> > > 
> > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > Link: https://www.spinics.net/lists/linux-renesas-soc/msg43271.html
> > > 
> > > ---
> > > v1->v2:
> > > * new patch
> > > 
> > > I have copied the license from include/drm/drm_bridge.h as that's
> > > where the struct originally came from. What's the right SPDX license
> > > to use in this case?
> > 
> > https://wiki.spdx.org/view/Legal_Team/Decisions/Dealing_with_Public_Domain_within_SPDX_Files
> > 
> > Greg, any idea on how we should handle this ?
> 
> Ugh, what lunacy.  But drm_bridge.h is NOT under any "public domain"
> license, so why is that an issue here?  This looks like a "normal" bsd 3
> clause license to me, right?

You're right, I overread part of the text in drm_bridge.h, it seems to
indeed be covered by a BSD 3 clause license. Sorry for the noise.

> So I would just use "BSD-3-Clause" as the SPDX license here, if I were
> doing this patch...

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 0/9] Add dual-LVDS panel support to EK874

[Sam Ravnborg]

Hi Fabrizio

> it appears that Rob has been busy converting the dt-bindings relevant to this
> series, and his changes are now found in linux-next. Most notably
> Documentation/devicetree/bindings/display/panel/panel-lvds.txt has now become
> Documentation/devicetree/bindings/display/panel/lvds.yaml
> 
> You have already taken patch:
> https://patchwork.kernel.org/patch/11072749/
> 
> as such the patch I am sending you is still related to:
> Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> 
> Patch "dt-bindings: display: Add bindings for Advantech IDK-2121WR" is still
> assuming the format is .txt, as I am not too sure about what the protocol is in
> this case? Shall we take this patch and convert it later to .yaml or shall I
> convert it to .yaml straight away?
> 
> Please, let me know what's the best course of action.

It is preferred that all new display and panel bindings uses the new
meta-schema format.
And that the writers uses the available tools to verify the bindings
submission.
This is one way to avoid simple mistakes in examples.

	Sam

RE: [PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings

[Fabrizio Castro]

Hi Greg, hi Laurent,

Thank you for your feedback!

> From: linux-kernel-owner@vger.kernel.org <linux-kernel-owner@vger.kernel.org> On Behalf Of Laurent Pinchart
> Sent: 15 August 2019 15:15
> Subject: Re: [PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings
> 
> Hi Greg,
> 
> On Thu, Aug 15, 2019 at 04:04:00PM +0200, Greg Kroah-Hartman wrote:
> > On Thu, Aug 15, 2019 at 04:18:38PM +0300, Laurent Pinchart wrote:
> > > Hi Fabrizio,
> > >
> > > (CC'ing Greg as the architect of the SPDX move)
> >
> > _one of_, not the one that did the most of he work, that would be Thomas :)
> >
> > > On Thu, Aug 15, 2019 at 12:04:27PM +0100, Fabrizio Castro wrote:
> > > > The information represented by drm_bridge_timings is also
> > > > needed by panels, therefore rename drm_bridge_timings to
> > > > drm_timings.
> > > >
> > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > > Link: https://www.spinics.net/lists/linux-renesas-soc/msg43271.html
> > > >
> > > > ---
> > > > v1->v2:
> > > > * new patch
> > > >
> > > > I have copied the license from include/drm/drm_bridge.h as that's
> > > > where the struct originally came from. What's the right SPDX license
> > > > to use in this case?
> > >
> > > https://wiki.spdx.org/view/Legal_Team/Decisions/Dealing_with_Public_Domain_within_SPDX_Files
> > >
> > > Greg, any idea on how we should handle this ?
> >
> > Ugh, what lunacy.  But drm_bridge.h is NOT under any "public domain"
> > license, so why is that an issue here?  This looks like a "normal" bsd 3
> > clause license to me, right?
> 
> You're right, I overread part of the text in drm_bridge.h, it seems to
> indeed be covered by a BSD 3 clause license. Sorry for the noise.

Mmm... This is the template for the BSD-3-Clause:

Copyright (c) <YEAR>, <OWNER>                                                    
All rights reserved.                                                             
                                                                                 
Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met:
                                                                                 
Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer.
Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution.
Neither the name of the <ORGANIZATION> nor the names of its contributors may be used to endorse or promote products derived from this software without specific prior written permission.
THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

And this is the license coming from include/drm/drm_bridge.h:

/*                                                                                                                                                                                                                                                                              
 * Copyright (c) 2016 Intel Corporation                                          
 *                                                                               
 * Permission to use, copy, modify, distribute, and sell this software and its   
 * documentation for any purpose is hereby granted without fee, provided that    
 * the above copyright notice appear in all copies and that both that copyright  
 * notice and this permission notice appear in supporting documentation, and     
 * that the name of the copyright holders not be used in advertising or          
 * publicity pertaining to distribution of the software without specific,        
 * written prior permission.  The copyright holders make no representations      
 * about the suitability of this software for any purpose.  It is provided "as   
 * is" without express or implied warranty.                                      
 *                                                                               
 * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,   
 * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO        
 * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR      
 * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,   
 * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER        
 * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE  
 * OF THIS SOFTWARE.                                                             
 */

Perhaps I am completely wrong here, and I am not a lawyer, but the wording seems different enough to me...
I am happy to use "BSD-3-Clause" though. Laurent please double check.

Thanks!
Fab

> 
> > So I would just use "BSD-3-Clause" as the SPDX license here, if I were
> > doing this patch...
> 
> --
> Regards,
> 
> Laurent Pinchart

RE: [PATCH v2 0/9] Add dual-LVDS panel support to EK874

[Fabrizio Castro]

Hi Sam,

Thank you for your feedback!

> From: Sam Ravnborg <sam@ravnborg.org>
> Sent: 15 August 2019 15:16
> Subject: Re: [PATCH v2 0/9] Add dual-LVDS panel support to EK874
> 
> Hi Fabrizio
> 
> > it appears that Rob has been busy converting the dt-bindings relevant to this
> > series, and his changes are now found in linux-next. Most notably
> > Documentation/devicetree/bindings/display/panel/panel-lvds.txt has now become
> > Documentation/devicetree/bindings/display/panel/lvds.yaml
> >
> > You have already taken patch:
> > https://patchwork.kernel.org/patch/11072749/
> >
> > as such the patch I am sending you is still related to:
> > Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >
> > Patch "dt-bindings: display: Add bindings for Advantech IDK-2121WR" is still
> > assuming the format is .txt, as I am not too sure about what the protocol is in
> > this case? Shall we take this patch and convert it later to .yaml or shall I
> > convert it to .yaml straight away?
> >
> > Please, let me know what's the best course of action.
> 
> It is preferred that all new display and panel bindings uses the new
> meta-schema format.
> And that the writers uses the available tools to verify the bindings
> submission.
> This is one way to avoid simple mistakes in examples.

Will give the meta-schema format a shot.

Thanks,
Fab

> 
> 	Sam

RE: [PATCH v2 5/9] drm/panel: Add timings field to drm_panel

[Fabrizio Castro]

Hi Sam,

Thank you for your feedback!

> From: Sam Ravnborg <sam@ravnborg.org>
> Sent: 15 August 2019 15:14
> Subject: Re: [PATCH v2 5/9] drm/panel: Add timings field to drm_panel
> 
> Hi Fabrizio
> 
> On Thu, Aug 15, 2019 at 12:04:29PM +0100, Fabrizio Castro wrote:
> > We need to know if the panel supports dual-link, similarly
> > to bridges, therefore add a reference to drm_timings in
> > drm_panel.
> 
> Why do we need to know this?

The encoders connected to a dual-LVDS panels may need to do special
arrangements for the dual-link setup, like initializing a companion
encoder, and working out which pixels (even or odd) to send to which
port (first LVDS input port or second LVDS input port). At least, this is
within the scope of the implementation of this series, which is currently
being discussed.

> Why is it needed in drm_panel and not in some driver specific struct?

The other fields are still applicable to panels, so I think it makes sense 
for this to be included in struct drm_panels. 

> 
> I cannot see the full series, as I was copied only on some mails.
> Awaiting dri-devel moderator before I can see the rest.

I am sorry about this, some people don't want to be bothered by the whole
thing, I'll make sure I add you to the recipients list for the next iterations
of this series.

Thanks,
Fab

> 
> 	Sam
> 
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >
> > ---
> > v1->v2:
> > * new patch
> >
> >  include/drm/drm_panel.h | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> > index 8c738c0..cd6ff07 100644
> > --- a/include/drm/drm_panel.h
> > +++ b/include/drm/drm_panel.h
> > @@ -26,6 +26,7 @@
> >
> >  #include <linux/errno.h>
> >  #include <linux/list.h>
> > +#include <drm/drm_timings.h>
> >
> >  struct device_node;
> >  struct drm_connector;
> > @@ -81,6 +82,7 @@ struct drm_panel_funcs {
> >   * struct drm_panel - DRM panel object
> >   * @drm: DRM device owning the panel
> >   * @connector: DRM connector that the panel is attached to
> > + * @timings: timing information
> >   * @dev: parent device of the panel
> >   * @link: link from panel device (supplier) to DRM device (consumer)
> >   * @funcs: operations that can be performed on the panel
> > @@ -89,6 +91,7 @@ struct drm_panel_funcs {
> >  struct drm_panel {
> >  	struct drm_device *drm;
> >  	struct drm_connector *connector;
> > +	const struct drm_timings *timings;
> >  	struct device *dev;
> >
> >  	const struct drm_panel_funcs *funcs;
> > --
> > 2.7.4

Re: [PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings

[Greg Kroah-Hartman]

On Thu, Aug 15, 2019 at 02:31:26PM +0000, Fabrizio Castro wrote:
> Hi Greg, hi Laurent,
> 
> Thank you for your feedback!
> 
> > From: linux-kernel-owner@vger.kernel.org <linux-kernel-owner@vger.kernel.org> On Behalf Of Laurent Pinchart
> > Sent: 15 August 2019 15:15
> > Subject: Re: [PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings
> > 
> > Hi Greg,
> > 
> > On Thu, Aug 15, 2019 at 04:04:00PM +0200, Greg Kroah-Hartman wrote:
> > > On Thu, Aug 15, 2019 at 04:18:38PM +0300, Laurent Pinchart wrote:
> > > > Hi Fabrizio,
> > > >
> > > > (CC'ing Greg as the architect of the SPDX move)
> > >
> > > _one of_, not the one that did the most of he work, that would be Thomas :)
> > >
> > > > On Thu, Aug 15, 2019 at 12:04:27PM +0100, Fabrizio Castro wrote:
> > > > > The information represented by drm_bridge_timings is also
> > > > > needed by panels, therefore rename drm_bridge_timings to
> > > > > drm_timings.
> > > > >
> > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > > > Link: https://www.spinics.net/lists/linux-renesas-soc/msg43271.html
> > > > >
> > > > > ---
> > > > > v1->v2:
> > > > > * new patch
> > > > >
> > > > > I have copied the license from include/drm/drm_bridge.h as that's
> > > > > where the struct originally came from. What's the right SPDX license
> > > > > to use in this case?
> > > >
> > > > https://wiki.spdx.org/view/Legal_Team/Decisions/Dealing_with_Public_Domain_within_SPDX_Files
> > > >
> > > > Greg, any idea on how we should handle this ?
> > >
> > > Ugh, what lunacy.  But drm_bridge.h is NOT under any "public domain"
> > > license, so why is that an issue here?  This looks like a "normal" bsd 3
> > > clause license to me, right?
> > 
> > You're right, I overread part of the text in drm_bridge.h, it seems to
> > indeed be covered by a BSD 3 clause license. Sorry for the noise.
> 
> Mmm... This is the template for the BSD-3-Clause:
> 
> Copyright (c) <YEAR>, <OWNER>                                                    
> All rights reserved.                                                             
>                                                                                  
> Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met:
>                                                                                  
> Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer.
> Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution.
> Neither the name of the <ORGANIZATION> nor the names of its contributors may be used to endorse or promote products derived from this software without specific prior written permission.
> THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> 
> And this is the license coming from include/drm/drm_bridge.h:
> 
> /*                                                                                                                                                                                                                                                                              
>  * Copyright (c) 2016 Intel Corporation                                          
>  *                                                                               
>  * Permission to use, copy, modify, distribute, and sell this software and its   
>  * documentation for any purpose is hereby granted without fee, provided that    
>  * the above copyright notice appear in all copies and that both that copyright  
>  * notice and this permission notice appear in supporting documentation, and     
>  * that the name of the copyright holders not be used in advertising or          
>  * publicity pertaining to distribution of the software without specific,        
>  * written prior permission.  The copyright holders make no representations      
>  * about the suitability of this software for any purpose.  It is provided "as   
>  * is" without express or implied warranty.                                      
>  *                                                                               
>  * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,   
>  * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO        
>  * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR      
>  * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,   
>  * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER        
>  * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE  
>  * OF THIS SOFTWARE.                                                             
>  */
> 
> Perhaps I am completely wrong here, and I am not a lawyer, but the wording seems different enough to me...
> I am happy to use "BSD-3-Clause" though. Laurent please double check.

Please talk to your lawyers about this, we are not them...

thanks,

greg k-h

RE: [PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings

[Fabrizio Castro]

Hi Greg, hi Laurent,

Thank you for your feedback!

> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Sent: 15 August 2019 15:53
> Subject: Re: [PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings
> 
> On Thu, Aug 15, 2019 at 02:31:26PM +0000, Fabrizio Castro wrote:
> > Hi Greg, hi Laurent,
> >
> > Thank you for your feedback!
> >
> > > From: linux-kernel-owner@vger.kernel.org <linux-kernel-owner@vger.kernel.org> On Behalf Of Laurent Pinchart
> > > Sent: 15 August 2019 15:15
> > > Subject: Re: [PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings
> > >
> > > Hi Greg,
> > >
> > > On Thu, Aug 15, 2019 at 04:04:00PM +0200, Greg Kroah-Hartman wrote:
> > > > On Thu, Aug 15, 2019 at 04:18:38PM +0300, Laurent Pinchart wrote:
> > > > > Hi Fabrizio,
> > > > >
> > > > > (CC'ing Greg as the architect of the SPDX move)
> > > >
> > > > _one of_, not the one that did the most of he work, that would be Thomas :)
> > > >
> > > > > On Thu, Aug 15, 2019 at 12:04:27PM +0100, Fabrizio Castro wrote:
> > > > > > The information represented by drm_bridge_timings is also
> > > > > > needed by panels, therefore rename drm_bridge_timings to
> > > > > > drm_timings.
> > > > > >
> > > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > > > > Link: https://www.spinics.net/lists/linux-renesas-soc/msg43271.html
> > > > > >
> > > > > > ---
> > > > > > v1->v2:
> > > > > > * new patch
> > > > > >
> > > > > > I have copied the license from include/drm/drm_bridge.h as that's
> > > > > > where the struct originally came from. What's the right SPDX license
> > > > > > to use in this case?
> > > > >
> > > > > https://wiki.spdx.org/view/Legal_Team/Decisions/Dealing_with_Public_Domain_within_SPDX_Files
> > > > >
> > > > > Greg, any idea on how we should handle this ?
> > > >
> > > > Ugh, what lunacy.  But drm_bridge.h is NOT under any "public domain"
> > > > license, so why is that an issue here?  This looks like a "normal" bsd 3
> > > > clause license to me, right?
> > >
> > > You're right, I overread part of the text in drm_bridge.h, it seems to
> > > indeed be covered by a BSD 3 clause license. Sorry for the noise.
> >
> > Mmm... This is the template for the BSD-3-Clause:
> >
> > Copyright (c) <YEAR>, <OWNER>
> > All rights reserved.
> >
> > Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following
> conditions are met:
> >
> > Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer.
> > Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the
> documentation and/or other materials provided with the distribution.
> > Neither the name of the <ORGANIZATION> nor the names of its contributors may be used to endorse or promote products derived
> from this software without specific prior written permission.
> > THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED
> WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY
> WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> >
> > And this is the license coming from include/drm/drm_bridge.h:
> >
> > /*
> >  * Copyright (c) 2016 Intel Corporation
> >  *
> >  * Permission to use, copy, modify, distribute, and sell this software and its
> >  * documentation for any purpose is hereby granted without fee, provided that
> >  * the above copyright notice appear in all copies and that both that copyright
> >  * notice and this permission notice appear in supporting documentation, and
> >  * that the name of the copyright holders not be used in advertising or
> >  * publicity pertaining to distribution of the software without specific,
> >  * written prior permission.  The copyright holders make no representations
> >  * about the suitability of this software for any purpose.  It is provided "as
> >  * is" without express or implied warranty.
> >  *
> >  * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
> >  * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> >  * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
> >  * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
> >  * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> >  * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
> >  * OF THIS SOFTWARE.
> >  */
> >
> > Perhaps I am completely wrong here, and I am not a lawyer, but the wording seems different enough to me...
> > I am happy to use "BSD-3-Clause" though. Laurent please double check.
> 
> Please talk to your lawyers about this, we are not them...

I am really sorry for the trouble (and the waste of time)!

I'll try and use "BSD-3-Clause" for the next version and I'll see what happens.

Thanks,
Fab

> 
> thanks,
> 
> greg k-h

RE: [PATCH v2 4/9] drm/timings: Add link flags

[Fabrizio Castro]

Hi Laurent,

Thank you for the feedback!

I think we need to come a conclusion on here:
https://patchwork.kernel.org/patch/11095547/

before taking the comments on this patch any further.

Thanks,
Fab

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 15 August 2019 13:00
> Subject: Re: [PATCH v2 4/9] drm/timings: Add link flags
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> On Thu, Aug 15, 2019 at 12:04:28PM +0100, Fabrizio Castro wrote:
> > We need more information to describe dual-LVDS links, therefore
> > introduce link_flags.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >
> > ---
> > v1->v2:
> > * new patch
> >
> >  include/drm/drm_timings.h | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >
> > diff --git a/include/drm/drm_timings.h b/include/drm/drm_timings.h
> > index 4af8814..58fbf1b 100644
> > --- a/include/drm/drm_timings.h
> > +++ b/include/drm/drm_timings.h
> > @@ -1,4 +1,6 @@
> >  /*
> > + * Copyright (C) 2019 Renesas Electronics Corporation
> > + *
> >   * Permission to use, copy, modify, distribute, and sell this software and its
> >   * documentation for any purpose is hereby granted without fee, provided that
> >   * the above copyright notice appear in all copies and that both that copyright
> > @@ -21,6 +23,24 @@
> >  #ifndef __DRM_TIMINGS_H__
> >  #define __DRM_TIMINGS_H__
> >
> > +#include <linux/bits.h>
> > +
> > +/**
> > + * enum drm_link_flags - link_flags for &drm_timings
> > + *
> > + * This enum defines the details of the link.
> > + *
> > + * @DRM_LINK_DUAL_LVDS_ODD_EVEN:	Dual-LVDS link, with odd pixels (1,3,5,
> > + *					etc.) coming through the first port, and
> > + *					even pixels (0,2,4,etc.) coming through
> > + *					the second port. If not specified for a
> > + *					dual-LVDS panel, it is assumed that even
> > + *					pixels are coming through the first port
> > + */
> > +enum drm_link_flags {
> 
> The text will be easier to read if you inline it here.
> 
> 	/**
> 	 * @DRM_LINK_DUAL_LVDS_ODD_EVEN: Dual-LVDS link, with odd pixels (1,3,5,
> 	 * etc.) coming through the first port, and  even pixels (0,2,4,etc.)
> 	 ...
> 
> > +	DRM_LINK_DUAL_LVDS_ODD_EVEN = BIT(0),
> 
> I would remove the dual_link field and add a DRM_LINK_DUAL_LVDS (or
> alternatively two flags, dual lvds odd-even and dual lvds even-odd).
> 
> > +};
> > +
> >  /**
> >   * struct drm_timings - timing information
> >   */
> > @@ -55,6 +75,12 @@ struct drm_timings {
> >  	 * and odd-numbered pixels are received on separate links.
> >  	 */
> >  	bool dual_link;
> > +	/**
> > +	 * @link_flags
> > +	 *
> > +	 * Provides detailed information about the link.
> 
> I think this calls for a bit more information than "detailed
> information". What information do you want to store in this field ?
> 
> > +	 */
> > +	enum drm_link_flags link_flags;
> >  };
> >
> >  #endif /* __DRM_TIMINGS_H__ */
> > --
> > 2.7.4
> >
> 
> --
> Regards,
> 
> Laurent Pinchart

Re: [PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings

[Laurent Pinchart]

Hi Greg,

On Thu, Aug 15, 2019 at 04:53:00PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Aug 15, 2019 at 02:31:26PM +0000, Fabrizio Castro wrote:
> > On 15 August 2019 15:15, Laurent Pinchart wrote:
> > > On Thu, Aug 15, 2019 at 04:04:00PM +0200, Greg Kroah-Hartman wrote:
> > > > On Thu, Aug 15, 2019 at 04:18:38PM +0300, Laurent Pinchart wrote:
> > > > > Hi Fabrizio,
> > > > >
> > > > > (CC'ing Greg as the architect of the SPDX move)
> > > >
> > > > _one of_, not the one that did the most of he work, that would be Thomas :)
> > > >
> > > > > On Thu, Aug 15, 2019 at 12:04:27PM +0100, Fabrizio Castro wrote:
> > > > > > The information represented by drm_bridge_timings is also
> > > > > > needed by panels, therefore rename drm_bridge_timings to
> > > > > > drm_timings.
> > > > > >
> > > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > > > > Link: https://www.spinics.net/lists/linux-renesas-soc/msg43271.html
> > > > > >
> > > > > > ---
> > > > > > v1->v2:
> > > > > > * new patch
> > > > > >
> > > > > > I have copied the license from include/drm/drm_bridge.h as that's
> > > > > > where the struct originally came from. What's the right SPDX license
> > > > > > to use in this case?
> > > > >
> > > > > https://wiki.spdx.org/view/Legal_Team/Decisions/Dealing_with_Public_Domain_within_SPDX_Files
> > > > >
> > > > > Greg, any idea on how we should handle this ?
> > > >
> > > > Ugh, what lunacy.  But drm_bridge.h is NOT under any "public domain"
> > > > license, so why is that an issue here?  This looks like a "normal" bsd 3
> > > > clause license to me, right?
> > > 
> > > You're right, I overread part of the text in drm_bridge.h, it seems to
> > > indeed be covered by a BSD 3 clause license. Sorry for the noise.
> > 
> > Mmm... This is the template for the BSD-3-Clause:
> > 
> > Copyright (c) <YEAR>, <OWNER>                                                    
> > All rights reserved.                                                             
> >                                                                                  
> > Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met:
> >                                                                                  
> > Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer.
> > Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution.
> > Neither the name of the <ORGANIZATION> nor the names of its contributors may be used to endorse or promote products derived from this software without specific prior written permission.
> > THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> > 
> > And this is the license coming from include/drm/drm_bridge.h:
> > 
> > /*                                                                                                                                                                                                                                                                              
> >  * Copyright (c) 2016 Intel Corporation                                          
> >  *                                                                               
> >  * Permission to use, copy, modify, distribute, and sell this software and its   
> >  * documentation for any purpose is hereby granted without fee, provided that    
> >  * the above copyright notice appear in all copies and that both that copyright  
> >  * notice and this permission notice appear in supporting documentation, and     
> >  * that the name of the copyright holders not be used in advertising or          
> >  * publicity pertaining to distribution of the software without specific,        
> >  * written prior permission.  The copyright holders make no representations      
> >  * about the suitability of this software for any purpose.  It is provided "as   
> >  * is" without express or implied warranty.                                      
> >  *                                                                               
> >  * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,   
> >  * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO        
> >  * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR      
> >  * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,   
> >  * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER        
> >  * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE  
> >  * OF THIS SOFTWARE.                                                             
> >  */
> > 
> > Perhaps I am completely wrong here, and I am not a lawyer, but the wording seems different enough to me...
> > I am happy to use "BSD-3-Clause" though. Laurent please double check.
> 
> Please talk to your lawyers about this, we are not them...

I don't think that's fair though. Fabrizio is reworking kernel code, and
as part of that wondered what SPDX tag to apply to a new file that
contains code moved from an existing file that has no SPDX tag, but the
above copyright notice. He's not trying to change a license, or reword
it. As SPDX is the preferred way of expressing licenses in the kernel,
he legitimately asked for help, and I think we should provide an
official answer for this (which could be not to use SPDX but copy the
license text).

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings

[Greg Kroah-Hartman]

On Thu, Aug 15, 2019 at 09:06:41PM +0300, Laurent Pinchart wrote:
> Hi Greg,
> 
> On Thu, Aug 15, 2019 at 04:53:00PM +0200, Greg Kroah-Hartman wrote:
> > On Thu, Aug 15, 2019 at 02:31:26PM +0000, Fabrizio Castro wrote:
> > > On 15 August 2019 15:15, Laurent Pinchart wrote:
> > > > On Thu, Aug 15, 2019 at 04:04:00PM +0200, Greg Kroah-Hartman wrote:
> > > > > On Thu, Aug 15, 2019 at 04:18:38PM +0300, Laurent Pinchart wrote:
> > > > > > Hi Fabrizio,
> > > > > >
> > > > > > (CC'ing Greg as the architect of the SPDX move)
> > > > >
> > > > > _one of_, not the one that did the most of he work, that would be Thomas :)
> > > > >
> > > > > > On Thu, Aug 15, 2019 at 12:04:27PM +0100, Fabrizio Castro wrote:
> > > > > > > The information represented by drm_bridge_timings is also
> > > > > > > needed by panels, therefore rename drm_bridge_timings to
> > > > > > > drm_timings.
> > > > > > >
> > > > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > > > > > Link: https://www.spinics.net/lists/linux-renesas-soc/msg43271.html
> > > > > > >
> > > > > > > ---
> > > > > > > v1->v2:
> > > > > > > * new patch
> > > > > > >
> > > > > > > I have copied the license from include/drm/drm_bridge.h as that's
> > > > > > > where the struct originally came from. What's the right SPDX license
> > > > > > > to use in this case?
> > > > > >
> > > > > > https://wiki.spdx.org/view/Legal_Team/Decisions/Dealing_with_Public_Domain_within_SPDX_Files
> > > > > >
> > > > > > Greg, any idea on how we should handle this ?
> > > > >
> > > > > Ugh, what lunacy.  But drm_bridge.h is NOT under any "public domain"
> > > > > license, so why is that an issue here?  This looks like a "normal" bsd 3
> > > > > clause license to me, right?
> > > > 
> > > > You're right, I overread part of the text in drm_bridge.h, it seems to
> > > > indeed be covered by a BSD 3 clause license. Sorry for the noise.
> > > 
> > > Mmm... This is the template for the BSD-3-Clause:
> > > 
> > > Copyright (c) <YEAR>, <OWNER>                                                    
> > > All rights reserved.                                                             
> > >                                                                                  
> > > Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met:
> > >                                                                                  
> > > Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer.
> > > Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution.
> > > Neither the name of the <ORGANIZATION> nor the names of its contributors may be used to endorse or promote products derived from this software without specific prior written permission.
> > > THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> > > 
> > > And this is the license coming from include/drm/drm_bridge.h:
> > > 
> > > /*                                                                                                                                                                                                                                                                              
> > >  * Copyright (c) 2016 Intel Corporation                                          
> > >  *                                                                               
> > >  * Permission to use, copy, modify, distribute, and sell this software and its   
> > >  * documentation for any purpose is hereby granted without fee, provided that    
> > >  * the above copyright notice appear in all copies and that both that copyright  
> > >  * notice and this permission notice appear in supporting documentation, and     
> > >  * that the name of the copyright holders not be used in advertising or          
> > >  * publicity pertaining to distribution of the software without specific,        
> > >  * written prior permission.  The copyright holders make no representations      
> > >  * about the suitability of this software for any purpose.  It is provided "as   
> > >  * is" without express or implied warranty.                                      
> > >  *                                                                               
> > >  * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,   
> > >  * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO        
> > >  * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR      
> > >  * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,   
> > >  * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER        
> > >  * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE  
> > >  * OF THIS SOFTWARE.                                                             
> > >  */
> > > 
> > > Perhaps I am completely wrong here, and I am not a lawyer, but the wording seems different enough to me...
> > > I am happy to use "BSD-3-Clause" though. Laurent please double check.
> > 
> > Please talk to your lawyers about this, we are not them...
> 
> I don't think that's fair though. Fabrizio is reworking kernel code, and
> as part of that wondered what SPDX tag to apply to a new file that
> contains code moved from an existing file that has no SPDX tag, but the
> above copyright notice. He's not trying to change a license, or reword
> it. As SPDX is the preferred way of expressing licenses in the kernel,
> he legitimately asked for help, and I think we should provide an
> official answer for this (which could be not to use SPDX but copy the
> license text).

Ah, ok, that makes more sense, didn't realize that.

Fabrizio, just copy the license text as-is to the new file if you are
copying from an existing one.  For all of these "we have to read the
text" files that are left in the kernel, we still have a ways to go to
convert them.  But, if you leave the text identical, when we match one
and fix it, the tools will catch the other identical ones as well, so
that does not create any extra work.

hope this helps,

greg k-h

RE: [PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings

[Fabrizio Castro]

Hi Greg, hi Laurent,

> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Sent: 15 August 2019 20:05
> Subject: Re: [PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings
> 
> On Thu, Aug 15, 2019 at 09:06:41PM +0300, Laurent Pinchart wrote:
> > Hi Greg,
> >
> > On Thu, Aug 15, 2019 at 04:53:00PM +0200, Greg Kroah-Hartman wrote:
> > > On Thu, Aug 15, 2019 at 02:31:26PM +0000, Fabrizio Castro wrote:
> > > > On 15 August 2019 15:15, Laurent Pinchart wrote:
> > > > > On Thu, Aug 15, 2019 at 04:04:00PM +0200, Greg Kroah-Hartman wrote:
> > > > > > On Thu, Aug 15, 2019 at 04:18:38PM +0300, Laurent Pinchart wrote:
> > > > > > > Hi Fabrizio,
> > > > > > >
> > > > > > > (CC'ing Greg as the architect of the SPDX move)
> > > > > >
> > > > > > _one of_, not the one that did the most of he work, that would be Thomas :)
> > > > > >
> > > > > > > On Thu, Aug 15, 2019 at 12:04:27PM +0100, Fabrizio Castro wrote:
> > > > > > > > The information represented by drm_bridge_timings is also
> > > > > > > > needed by panels, therefore rename drm_bridge_timings to
> > > > > > > > drm_timings.
> > > > > > > >
> > > > > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > > > > > > Link: https://www.spinics.net/lists/linux-renesas-soc/msg43271.html
> > > > > > > >
> > > > > > > > ---
> > > > > > > > v1->v2:
> > > > > > > > * new patch
> > > > > > > >
> > > > > > > > I have copied the license from include/drm/drm_bridge.h as that's
> > > > > > > > where the struct originally came from. What's the right SPDX license
> > > > > > > > to use in this case?
> > > > > > >
> > > > > > > https://wiki.spdx.org/view/Legal_Team/Decisions/Dealing_with_Public_Domain_within_SPDX_Files
> > > > > > >
> > > > > > > Greg, any idea on how we should handle this ?
> > > > > >
> > > > > > Ugh, what lunacy.  But drm_bridge.h is NOT under any "public domain"
> > > > > > license, so why is that an issue here?  This looks like a "normal" bsd 3
> > > > > > clause license to me, right?
> > > > >
> > > > > You're right, I overread part of the text in drm_bridge.h, it seems to
> > > > > indeed be covered by a BSD 3 clause license. Sorry for the noise.
> > > >
> > > > Mmm... This is the template for the BSD-3-Clause:
> > > >
> > > > Copyright (c) <YEAR>, <OWNER>
> > > > All rights reserved.
> > > >
> > > > Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following
> conditions are met:
> > > >
> > > > Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer.
> > > > Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in
> the documentation and/or other materials provided with the distribution.
> > > > Neither the name of the <ORGANIZATION> nor the names of its contributors may be used to endorse or promote products
> derived from this software without specific prior written permission.
> > > > THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED
> WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY
> WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> > > >
> > > > And this is the license coming from include/drm/drm_bridge.h:
> > > >
> > > > /*
> > > >  * Copyright (c) 2016 Intel Corporation
> > > >  *
> > > >  * Permission to use, copy, modify, distribute, and sell this software and its
> > > >  * documentation for any purpose is hereby granted without fee, provided that
> > > >  * the above copyright notice appear in all copies and that both that copyright
> > > >  * notice and this permission notice appear in supporting documentation, and
> > > >  * that the name of the copyright holders not be used in advertising or
> > > >  * publicity pertaining to distribution of the software without specific,
> > > >  * written prior permission.  The copyright holders make no representations
> > > >  * about the suitability of this software for any purpose.  It is provided "as
> > > >  * is" without express or implied warranty.
> > > >  *
> > > >  * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
> > > >  * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> > > >  * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
> > > >  * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
> > > >  * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> > > >  * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
> > > >  * OF THIS SOFTWARE.
> > > >  */
> > > >
> > > > Perhaps I am completely wrong here, and I am not a lawyer, but the wording seems different enough to me...
> > > > I am happy to use "BSD-3-Clause" though. Laurent please double check.
> > >
> > > Please talk to your lawyers about this, we are not them...
> >
> > I don't think that's fair though. Fabrizio is reworking kernel code, and
> > as part of that wondered what SPDX tag to apply to a new file that
> > contains code moved from an existing file that has no SPDX tag, but the
> > above copyright notice. He's not trying to change a license, or reword
> > it. As SPDX is the preferred way of expressing licenses in the kernel,
> > he legitimately asked for help, and I think we should provide an
> > official answer for this (which could be not to use SPDX but copy the
> > license text).
> 
> Ah, ok, that makes more sense, didn't realize that.
> 
> Fabrizio, just copy the license text as-is to the new file if you are
> copying from an existing one.  For all of these "we have to read the
> text" files that are left in the kernel, we still have a ways to go to
> convert them.  But, if you leave the text identical, when we match one
> and fix it, the tools will catch the other identical ones as well, so
> that does not create any extra work.
> 
> hope this helps,

It does! Thank you both guys!

Cheers,
Fab

> 
> greg k-h