[PATCH/RFC 00/15] R-Car DU: LVDS dual-link mode support

[PATCH/RFC 00/15] R-Car DU: LVDS dual-link mode support

[Laurent Pinchart]

Hello everybody,

This patch series implements support for LVDS dual-link mode in the
R-Car DU and R-Car LVDS encoder drivers, and well as in the thc63lvd1024
LVDS decoder driver. I decided to tag it with RFC as it includes an
extension to the drm_bridge API.

LVDS dual-link is a mode of operation where two individual LVDS links
are operated together to carry even- and odd-numbered pixels separately.
This doubles the possible bandwidth of the video transmission. Both the
transmitter and the receiver need to support this mode of operation.

The R-Car D3 and E3 SoCs include two independent LVDS encoders that can
be grouped together to operate in dual-link mode. When used separately,
the LVDS encoders are connected to two different CRTCs and transmit
independent video streams. When used in dual-link mode, the first LVDS
encoder is connected to the first CRTC, and split even- and odd-numbered
pixels. It transmits half of the pixels on its LVDS output, and sends
the other half to the second LVDS encoder for transmittion over the
second LVDS link. The second LVDS encoder thus operates under control of
the first one, and isn't connected directly to a CRTC.

On the receiving side, the THC63LVD1024 LVDS-to-parallel bridge has two
LVDS inputs and two parallel outputs. It can operate in four different
modes:

- Single-in, single-out: The first LVDS input receives the video stream,
  and the bridge outputs it on the first parallel output. The second
  LVDS input and the second parallel output are not used.

- Single-in, dual-out: The first LVDS input receives the video stream,
  and the bridge splits even- and odd-numbered pixels and outputs them
  on the first and second parallel outputs. The second LVDS input is not
  used.

- Dual-in, single-out: The two LVDS inputs are used in dual-link mode,
  and the bridge combines the even- and odd-numbered pixels and outputs
  them on the first parallel output. The second parallel output is not
  used.

- Dual-in, dual-out: The two LVDS inputs are used in dual-link mode,
  and the bridge outputs the even- and odd-numbered pixels on the first
  parallel output.

The operating mode is selected by two input pins of the bridge, which
are connected to DIP switches on the development boards I use. The mode
is thus fixed from a Linux point of view.

The first three patches are borrowed from the omapdrm pending rework.
They clarify the DRM bus flags and extend the drm_bridge_timings to
report input bus flags instead of just the input sampling edge. They
have been extensively reviewed, there's no need to comment on them here.

Patch 04/15 adds a new dual_link boolen field to the drm_bridge_timings
structure to let bridges report their LVDS mode of operation. Patch
05/15 clarifies the THC63LVD1024 DT bindings to document dual-link
operation, and patch 06/15 implements dual-link support in the
thc64lvd1024 bridge driver by setting the drm_bridge_timings dual_link
field according to the mode selected through DT.

Patch 07/15 extends the R-Car LVDS DT bindings to specify the companion
LVDS encoder for dual-link operation. Patches 08/15 to 10/15 then clean
up the LVDS encoder driver and fix a couple of issues related to the D3
and E3 SoCs. Patch 11/15 implements dual-link support in the LVDS
encoder driver, which involves retrieving the operation mode from the
LVDS receiver, locating the companion LVDS encoder, and configuring both
encoders when dual-link operation is desired. The API towards the DU
driver is also extended to report the mode of operation.

Patch 12/15 implements dual-link mode support in the DU driver. There is
no specific configuration to be performed there, as dual-link is fully
implemented in the LVDS encoder driver, but the DU driver has to skip
creation of the DRM encoder and connector related to the second LVDS
encoder when dual-link is used, as the second LVDS encoder operates as a
slave of the first one, transparently from a CRTC (and thus userspace)
perspective.

Patch 13/15 specifies the companion LVDS encoder in the D3 and E3 DT
bindings. This by itself doesn't enable dual-link mode, the LVDS0
encoder is still connected to the HDMI output through LVDS receiver, and
the LVDS1 encoder is not used. Patches 14/15 and 15/15, not intended to
be merged, enable dual-link operation for the D3 and E3 boards for
testing and require flipping DIP switches on the boards.

The patches are based on top of my drm/du/next branch, and are available
for convenience at

	git://linuxtv.org/pinchartl/media.git drm/du/lvds/dual-link

They have been tested successfully on the D3 Draak board. I expect them
to work on E3 as well, but I don't have access to an Ebisu board to test
this.

Laurent Pinchart (14):
  drm: Clarify definition of the DRM_BUS_FLAG_(PIXDATA|SYNC)_* macros
  drm: Use new DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags
  drm: bridge: Add dual_link field to the drm_bridge_timings structure
  dt-bindings: display: bridge: thc63lvd1024: Document dual-link
    operation
  drm: bridge: thc63: Report input bus mode through bridge timings
  dt-bindings: display: renesas: lvds: Add renesas,companion property
  drm: rcar-du: lvds: Remove LVDS double-enable checks
  drm: rcar-du: lvds: Adjust operating frequency for D3 and E3
  drm: rcar-du: lvds: Set LVEN and LVRES bits together on D3
  drm: rcar-du: lvds: Add support for dual-link mode
  drm: rcar-du: Skip LVDS1 output on Gen3 when using dual-link LVDS mode
  arm64: dts: renesas: r8a7799[05]: Point LVDS0 to its companion LVDS1
  [HACK] arm64: dts: renesas: draak: Enable LVDS dual-link operation
  [HACK] arm64: dts: renesas: ebisu: Enable LVDS dual-link operation

Stefan Agner (1):
  drm/bridge: use bus flags in bridge timings

 .../bindings/display/bridge/renesas,lvds.txt  |   6 +
 .../display/bridge/thine,thc63lvd1024.txt     |   7 +
 .../arm64/boot/dts/renesas/r8a77990-ebisu.dts |  21 ++-
 arch/arm64/boot/dts/renesas/r8a77990.dtsi     |   2 +
 .../arm64/boot/dts/renesas/r8a77995-draak.dts |  21 ++-
 arch/arm64/boot/dts/renesas/r8a77995.dtsi     |   2 +
 drivers/gpu/drm/bridge/dumb-vga-dac.c         |   6 +-
 drivers/gpu/drm/bridge/tc358767.c             |   4 +-
 drivers/gpu/drm/bridge/thc63lvd1024.c         |  46 ++++--
 drivers/gpu/drm/drm_modes.c                   |  12 +-
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c    |   2 +-
 drivers/gpu/drm/imx/ipuv3-crtc.c              |   2 +-
 drivers/gpu/drm/mxsfb/mxsfb_crtc.c            |   6 +-
 .../gpu/drm/omapdrm/displays/encoder-tfp410.c |   5 +-
 .../displays/panel-lgphilips-lb035q02.c       |   5 +-
 .../omapdrm/displays/panel-nec-nl8048hl11.c   |   5 +-
 .../displays/panel-sharp-ls037v7dw01.c        |   5 +-
 .../omapdrm/displays/panel-sony-acx565akm.c   |   5 +-
 .../omapdrm/displays/panel-tpo-td028ttec1.c   |   5 +-
 .../omapdrm/displays/panel-tpo-td043mtea1.c   |   5 +-
 drivers/gpu/drm/omapdrm/dss/dsi.c             |   4 +-
 drivers/gpu/drm/omapdrm/dss/sdi.c             |   4 +-
 drivers/gpu/drm/omapdrm/omap_encoder.c        |   8 +-
 drivers/gpu/drm/panel/panel-arm-versatile.c   |   4 +-
 drivers/gpu/drm/panel/panel-ilitek-ili9322.c  |   4 +-
 drivers/gpu/drm/panel/panel-seiko-43wvf1g.c   |   2 +-
 drivers/gpu/drm/panel/panel-simple.c          |  24 +--
 drivers/gpu/drm/panel/panel-tpo-tpg110.c      |  10 +-
 drivers/gpu/drm/pl111/pl111_display.c         |   2 +-
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c     |  12 ++
 drivers/gpu/drm/rcar-du/rcar_du_kms.c         |   2 +-
 drivers/gpu/drm/rcar-du/rcar_lvds.c           | 140 +++++++++++++-----
 drivers/gpu/drm/rcar-du/rcar_lvds.h           |   5 +
 drivers/gpu/drm/sun4i/sun4i_tcon.c            |   4 +-
 drivers/gpu/drm/tve200/tve200_display.c       |   3 +-
 include/drm/drm_bridge.h                      |  19 ++-
 include/drm/drm_connector.h                   |  36 ++++-
 37 files changed, 329 insertions(+), 126 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH/RFC 01/15] drm: Clarify definition of the DRM_BUS_FLAG_(PIXDATA|SYNC)_* macros

[Laurent Pinchart]

The DRM_BUS_FLAG_PIXDATA_POSEDGE and DRM_BUS_FLAG_PIXDATA_NEGEDGE macros
and their DRM_BUS_FLAG_SYNC_* counterparts define on which pixel clock
edge data and sync signals are driven. They are however used in some
drivers to define on which pixel clock edge data and sync signals are
sampled, which should usually (but not always) be the opposite edge of
the driving edge. This creates confusion.

Create four new macros for both PIXDATA and SYNC that explicitly state
the driving and sampling edge in their name to remove the confusion. The
driving macros are defined as the opposite of the sampling macros to
made code simpler based on the assumption that the driving and sampling
edges are opposite.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 include/drm/drm_connector.h | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 8fe22abb1e10..411c0eb4c00e 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -330,19 +330,47 @@ struct drm_display_info {
 
 #define DRM_BUS_FLAG_DE_LOW		(1<<0)
 #define DRM_BUS_FLAG_DE_HIGH		(1<<1)
-/* drive data on pos. edge */
+
+/*
+ * Don't use those two flags directly, use the DRM_BUS_FLAG_PIXDATA_DRIVE_*
+ * and DRM_BUS_FLAG_PIXDATA_SAMPLE_* variants to qualify the flags explicitly.
+ * The DRM_BUS_FLAG_PIXDATA_SAMPLE_* flags are defined as the opposite of the
+ * DRM_BUS_FLAG_PIXDATA_DRIVE_* flags to make code simpler, as signals are
+ * usually to be sampled on the opposite edge of the driving edge.
+ */
 #define DRM_BUS_FLAG_PIXDATA_POSEDGE	(1<<2)
-/* drive data on neg. edge */
 #define DRM_BUS_FLAG_PIXDATA_NEGEDGE	(1<<3)
+
+/* Drive data on rising edge */
+#define DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE	DRM_BUS_FLAG_PIXDATA_POSEDGE
+/* Drive data on falling edge */
+#define DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE	DRM_BUS_FLAG_PIXDATA_NEGEDGE
+/* Sample data on rising edge */
+#define DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE	DRM_BUS_FLAG_PIXDATA_NEGEDGE
+/* Sample data on falling edge */
+#define DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE	DRM_BUS_FLAG_PIXDATA_POSEDGE
+
 /* data is transmitted MSB to LSB on the bus */
 #define DRM_BUS_FLAG_DATA_MSB_TO_LSB	(1<<4)
 /* data is transmitted LSB to MSB on the bus */
 #define DRM_BUS_FLAG_DATA_LSB_TO_MSB	(1<<5)
-/* drive sync on pos. edge */
+
+/*
+ * Similarly to the DRM_BUS_FLAG_PIXDATA_* flags, don't use these two flags
+ * directly, use one of the DRM_BUS_FLAG_SYNC_(DRIVE|SAMPLE)_* instead.
+ */
 #define DRM_BUS_FLAG_SYNC_POSEDGE	(1<<6)
-/* drive sync on neg. edge */
 #define DRM_BUS_FLAG_SYNC_NEGEDGE	(1<<7)
 
+/* Drive sync on rising edge */
+#define DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE		DRM_BUS_FLAG_SYNC_POSEDGE
+/* Drive sync on falling edge */
+#define DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE		DRM_BUS_FLAG_SYNC_NEGEDGE
+/* Sample sync on rising edge */
+#define DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE	DRM_BUS_FLAG_SYNC_NEGEDGE
+/* Sample sync on falling edge */
+#define DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE	DRM_BUS_FLAG_SYNC_POSEDGE
+
 	/**
 	 * @bus_flags: Additional information (like pixel signal polarity) for
 	 * the pixel data on the bus, using DRM_BUS_FLAGS\_ defines.
-- 
Regards,

Laurent Pinchart

[PATCH/RFC 02/15] drm: Use new DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags

[Laurent Pinchart]

The DRM_BUS_FLAG_PIXDATA_(POS|NEG)EDGE and
DRM_BUS_FLAG_SYNC_(POS|NEG)EDGE flags are deprecated in favour of the
new DRM_BUS_FLAG_PIXDATA_(DRIVE|SAMPLE)_(POS|NEG)EDGE and
new DRM_BUS_FLAG_SYNC_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags. Replace them
through the code.

This effectively changes the value of the .sampling_edge bridge timings
field in the dumb-vga-dac driver. This is safe to do as no driver
consumes these values yet.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/bridge/dumb-vga-dac.c         |  6 ++---
 drivers/gpu/drm/bridge/tc358767.c             |  4 ++--
 drivers/gpu/drm/drm_modes.c                   | 12 +++++-----
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c    |  2 +-
 drivers/gpu/drm/imx/ipuv3-crtc.c              |  2 +-
 drivers/gpu/drm/mxsfb/mxsfb_crtc.c            |  6 ++---
 .../gpu/drm/omapdrm/displays/encoder-tfp410.c |  5 ++--
 .../displays/panel-lgphilips-lb035q02.c       |  5 ++--
 .../omapdrm/displays/panel-nec-nl8048hl11.c   |  5 ++--
 .../displays/panel-sharp-ls037v7dw01.c        |  5 ++--
 .../omapdrm/displays/panel-sony-acx565akm.c   |  5 ++--
 .../omapdrm/displays/panel-tpo-td028ttec1.c   |  5 ++--
 .../omapdrm/displays/panel-tpo-td043mtea1.c   |  5 ++--
 drivers/gpu/drm/omapdrm/dss/dsi.c             |  4 ++--
 drivers/gpu/drm/omapdrm/dss/sdi.c             |  4 ++--
 drivers/gpu/drm/omapdrm/omap_encoder.c        |  8 +++----
 drivers/gpu/drm/panel/panel-arm-versatile.c   |  4 ++--
 drivers/gpu/drm/panel/panel-ilitek-ili9322.c  |  4 ++--
 drivers/gpu/drm/panel/panel-seiko-43wvf1g.c   |  2 +-
 drivers/gpu/drm/panel/panel-simple.c          | 24 +++++++++----------
 drivers/gpu/drm/panel/panel-tpo-tpg110.c      | 10 ++++----
 drivers/gpu/drm/pl111/pl111_display.c         |  2 +-
 drivers/gpu/drm/sun4i/sun4i_tcon.c            |  4 ++--
 drivers/gpu/drm/tve200/tve200_display.c       |  3 ++-
 include/drm/drm_bridge.h                      |  9 +++----
 25 files changed, 77 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index 0805801f4e94..94ed450e308d 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
  */
 static const struct drm_bridge_timings default_dac_timings = {
 	/* Timing specifications, datasheet page 7 */
-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
 	.setup_time_ps = 500,
 	.hold_time_ps = 1500,
 };
@@ -245,7 +245,7 @@ static const struct drm_bridge_timings default_dac_timings = {
  */
 static const struct drm_bridge_timings ti_ths8134_dac_timings = {
 	/* From timing diagram, datasheet page 9 */
-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
 	/* From datasheet, page 12 */
 	.setup_time_ps = 3000,
 	/* I guess this means latched input */
@@ -258,7 +258,7 @@ static const struct drm_bridge_timings ti_ths8134_dac_timings = {
  */
 static const struct drm_bridge_timings ti_ths8135_dac_timings = {
 	/* From timing diagram, datasheet page 14 */
-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
 	/* From datasheet, page 16 */
 	.setup_time_ps = 2000,
 	.hold_time_ps = 500,
diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 888980d4bc74..e570c9dee180 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1222,8 +1222,8 @@ static int tc_bridge_attach(struct drm_bridge *bridge)
 					 &bus_format, 1);
 	tc->connector.display_info.bus_flags =
 		DRM_BUS_FLAG_DE_HIGH |
-		DRM_BUS_FLAG_PIXDATA_NEGEDGE |
-		DRM_BUS_FLAG_SYNC_NEGEDGE;
+		DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE |
+		DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE;
 	drm_connector_attach_encoder(&tc->connector, tc->bridge.encoder);
 
 	return 0;
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 869ac6f4671e..56f92a0bba62 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -655,22 +655,22 @@ EXPORT_SYMBOL_GPL(drm_display_mode_to_videomode);
  * @bus_flags: information about pixelclk, sync and DE polarity will be stored
  * here
  *
- * Sets DRM_BUS_FLAG_DE_(LOW|HIGH),  DRM_BUS_FLAG_PIXDATA_(POS|NEG)EDGE and
- * DISPLAY_FLAGS_SYNC_(POS|NEG)EDGE in @bus_flags according to DISPLAY_FLAGS
+ * Sets DRM_BUS_FLAG_DE_(LOW|HIGH),  DRM_BUS_FLAG_PIXDATA_DRIVE_(POS|NEG)EDGE
+ * and DISPLAY_FLAGS_SYNC_(POS|NEG)EDGE in @bus_flags according to DISPLAY_FLAGS
  * found in @vm
  */
 void drm_bus_flags_from_videomode(const struct videomode *vm, u32 *bus_flags)
 {
 	*bus_flags = 0;
 	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
-		*bus_flags |= DRM_BUS_FLAG_PIXDATA_POSEDGE;
+		*bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
 	if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
-		*bus_flags |= DRM_BUS_FLAG_PIXDATA_NEGEDGE;
+		*bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE;
 
 	if (vm->flags & DISPLAY_FLAGS_SYNC_POSEDGE)
-		*bus_flags |= DRM_BUS_FLAG_SYNC_POSEDGE;
+		*bus_flags |= DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE;
 	if (vm->flags & DISPLAY_FLAGS_SYNC_NEGEDGE)
-		*bus_flags |= DRM_BUS_FLAG_SYNC_NEGEDGE;
+		*bus_flags |= DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE;
 
 	if (vm->flags & DISPLAY_FLAGS_DE_LOW)
 		*bus_flags |= DRM_BUS_FLAG_DE_LOW;
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index bf256971063d..83c841b50272 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -94,7 +94,7 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	drm_display_mode_to_videomode(mode, &vm);
 
 	/* INV_PXCK as default (most display sample data on rising edge) */
-	if (!(con->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE))
+	if (!(con->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE))
 		pol |= DCU_SYN_POL_INV_PXCK;
 
 	if (vm.flags & DISPLAY_FLAGS_HSYNC_LOW)
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index ec3602ebbc1c..311a20c942eb 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -295,7 +295,7 @@ static void ipu_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	sig_cfg.enable_pol = !(imx_crtc_state->bus_flags & DRM_BUS_FLAG_DE_LOW);
 	/* Default to driving pixel data on negative clock edges */
 	sig_cfg.clk_pol = !!(imx_crtc_state->bus_flags &
-			     DRM_BUS_FLAG_PIXDATA_POSEDGE);
+			     DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE);
 	sig_cfg.bus_format = imx_crtc_state->bus_format;
 	sig_cfg.v_to_h_sync = 0;
 	sig_cfg.hsync_pin = imx_crtc_state->di_hsync_pin;
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index 0ee1ca8a316a..98e9bda91e80 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -253,12 +253,12 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb)
 	if (!(bus_flags & DRM_BUS_FLAG_DE_LOW))
 		vdctrl0 |= VDCTRL0_ENABLE_ACT_HIGH;
 	/*
-	 * DRM_BUS_FLAG_PIXDATA_ defines are controller centric,
+	 * DRM_BUS_FLAG_PIXDATA_DRIVE_ defines are controller centric,
 	 * controllers VDCTRL0_DOTCLK is display centric.
 	 * Drive on positive edge       -> display samples on falling edge
-	 * DRM_BUS_FLAG_PIXDATA_POSEDGE -> VDCTRL0_DOTCLK_ACT_FALLING
+	 * DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE -> VDCTRL0_DOTCLK_ACT_FALLING
 	 */
-	if (bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE)
+	if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
 		vdctrl0 |= VDCTRL0_DOTCLK_ACT_FALLING;
 
 	writel(vdctrl0, mxsfb->base + LCDC_VDCTRL0);
diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c b/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c
index f1a748353279..a98d8759016d 100644
--- a/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c
+++ b/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c
@@ -111,8 +111,9 @@ static int tfp410_probe(struct platform_device *pdev)
 	dssdev->output_type = OMAP_DISPLAY_TYPE_DVI;
 	dssdev->owner = THIS_MODULE;
 	dssdev->of_ports = BIT(1) | BIT(0);
-	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_POSEDGE
-			  | DRM_BUS_FLAG_PIXDATA_POSEDGE;
+	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
+			  | DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE
+			  | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
 
 	dssdev->next = omapdss_of_find_connected_device(pdev->dev.of_node, 1);
 	if (IS_ERR(dssdev->next)) {
diff --git a/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c b/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
index f6ef8ff964dd..e36570b2bd15 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
@@ -229,8 +229,9 @@ static int lb035q02_panel_spi_probe(struct spi_device *spi)
 	 * DE is active LOW
 	 * DATA needs to be driven on the FALLING edge
 	 */
-	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_NEGEDGE
-			  | DRM_BUS_FLAG_PIXDATA_POSEDGE;
+	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
+			  | DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE
+			  | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
 
 	omapdss_display_init(dssdev);
 	omapdss_device_register(dssdev);
diff --git a/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c b/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c
index f445de6369f7..6d3d49b90214 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c
@@ -218,8 +218,9 @@ static int nec_8048_probe(struct spi_device *spi)
 	dssdev->type = OMAP_DISPLAY_TYPE_DPI;
 	dssdev->owner = THIS_MODULE;
 	dssdev->of_ports = BIT(0);
-	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_POSEDGE
-			  | DRM_BUS_FLAG_PIXDATA_POSEDGE;
+	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
+			  | DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE
+			  | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
 
 	omapdss_display_init(dssdev);
 	omapdss_device_register(dssdev);
diff --git a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
index 64b1369cb274..9532166e71df 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
@@ -227,8 +227,9 @@ static int sharp_ls_probe(struct platform_device *pdev)
 	 * Note: According to the panel documentation:
 	 * DATA needs to be driven on the FALLING edge
 	 */
-	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_NEGEDGE
-			  | DRM_BUS_FLAG_PIXDATA_POSEDGE;
+	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
+			  | DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE
+			  | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
 
 	omapdss_display_init(dssdev);
 	omapdss_device_register(dssdev);
diff --git a/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c b/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c
index e04663856b31..2a13dbaf35ce 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c
@@ -741,8 +741,9 @@ static int acx565akm_probe(struct spi_device *spi)
 	dssdev->type = OMAP_DISPLAY_TYPE_SDI;
 	dssdev->owner = THIS_MODULE;
 	dssdev->of_ports = BIT(0);
-	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_NEGEDGE
-			  | DRM_BUS_FLAG_PIXDATA_POSEDGE;
+	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
+			  | DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE
+			  | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
 
 	omapdss_display_init(dssdev);
 	omapdss_device_register(dssdev);
diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
index 7ddc8c574a61..52ebd5027ded 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
@@ -351,8 +351,9 @@ static int td028ttec1_panel_probe(struct spi_device *spi)
 	 * Note: According to the panel documentation:
 	 * SYNC needs to be driven on the FALLING edge
 	 */
-	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_POSEDGE
-			  | DRM_BUS_FLAG_PIXDATA_NEGEDGE;
+	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
+			  | DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE
+			  | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE;
 
 	omapdss_display_init(dssdev);
 	omapdss_device_register(dssdev);
diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c
index 8440fcb744d9..9c56e5d2292c 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c
@@ -449,8 +449,9 @@ static int tpo_td043_probe(struct spi_device *spi)
 	 * Note: According to the panel documentation:
 	 * SYNC needs to be driven on the FALLING edge
 	 */
-	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_POSEDGE
-			  | DRM_BUS_FLAG_PIXDATA_NEGEDGE;
+	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
+			  | DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE
+			  | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE;
 
 	omapdss_display_init(dssdev);
 	omapdss_device_register(dssdev);
diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
index 64fb788b6647..0f06872e81f7 100644
--- a/drivers/gpu/drm/omapdrm/dss/dsi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
@@ -5144,9 +5144,9 @@ static int dsi_init_output(struct dsi_data *dsi)
 	out->ops = &dsi_ops;
 	out->owner = THIS_MODULE;
 	out->of_ports = BIT(0);
-	out->bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE
+	out->bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE
 		       | DRM_BUS_FLAG_DE_HIGH
-		       | DRM_BUS_FLAG_SYNC_NEGEDGE;
+		       | DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE;
 
 	out->next = omapdss_of_find_connected_device(out->dev->of_node, 0);
 	if (IS_ERR(out->next)) {
diff --git a/drivers/gpu/drm/omapdrm/dss/sdi.c b/drivers/gpu/drm/omapdrm/dss/sdi.c
index b2fe2387037a..fcf10bba4f48 100644
--- a/drivers/gpu/drm/omapdrm/dss/sdi.c
+++ b/drivers/gpu/drm/omapdrm/dss/sdi.c
@@ -294,8 +294,8 @@ static int sdi_init_output(struct sdi_device *sdi)
 	out->of_ports = BIT(1);
 	out->ops = &sdi_ops;
 	out->owner = THIS_MODULE;
-	out->bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE	/* 15.5.9.1.2 */
-		       | DRM_BUS_FLAG_SYNC_POSEDGE;
+	out->bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE	/* 15.5.9.1.2 */
+		       | DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE;
 
 	out->next = omapdss_of_find_connected_device(out->dev->of_node, 1);
 	if (IS_ERR(out->next)) {
diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index 0d85b3a35767..5527b6f70fe5 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -115,17 +115,17 @@ static void omap_encoder_mode_set(struct drm_encoder *encoder,
 
 		if (!(vm.flags & (DISPLAY_FLAGS_PIXDATA_POSEDGE |
 				  DISPLAY_FLAGS_PIXDATA_NEGEDGE))) {
-			if (bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE)
+			if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
 				vm.flags |= DISPLAY_FLAGS_PIXDATA_POSEDGE;
-			else if (bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
+			else if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
 				vm.flags |= DISPLAY_FLAGS_PIXDATA_NEGEDGE;
 		}
 
 		if (!(vm.flags & (DISPLAY_FLAGS_SYNC_POSEDGE |
 				  DISPLAY_FLAGS_SYNC_NEGEDGE))) {
-			if (bus_flags & DRM_BUS_FLAG_SYNC_POSEDGE)
+			if (bus_flags & DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE)
 				vm.flags |= DISPLAY_FLAGS_SYNC_POSEDGE;
-			else if (bus_flags & DRM_BUS_FLAG_SYNC_NEGEDGE)
+			else if (bus_flags & DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE)
 				vm.flags |= DISPLAY_FLAGS_SYNC_NEGEDGE;
 		}
 	}
diff --git a/drivers/gpu/drm/panel/panel-arm-versatile.c b/drivers/gpu/drm/panel/panel-arm-versatile.c
index b428c4678106..078fa2c0eef8 100644
--- a/drivers/gpu/drm/panel/panel-arm-versatile.c
+++ b/drivers/gpu/drm/panel/panel-arm-versatile.c
@@ -191,7 +191,7 @@ static const struct versatile_panel_type versatile_panels[] = {
 			.vrefresh = 390,
 			.flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
 		},
-		.bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
+		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
 	},
 	/*
 	 * Sanyo ALR252RGT 240x320 portrait display found on the
@@ -215,7 +215,7 @@ static const struct versatile_panel_type versatile_panels[] = {
 			.vrefresh = 116,
 			.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
 		},
-		.bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
+		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
 		.ib2 = true,
 	},
 };
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9322.c b/drivers/gpu/drm/panel/panel-ilitek-ili9322.c
index bd38bf4f1ba6..35497ff08391 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9322.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9322.c
@@ -412,11 +412,11 @@ static int ili9322_init(struct drm_panel *panel, struct ili9322 *ili)
 	if (ili->conf->dclk_active_high) {
 		reg = ILI9322_POL_DCLK;
 		connector->display_info.bus_flags |=
-			DRM_BUS_FLAG_PIXDATA_POSEDGE;
+			DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
 	} else {
 		reg = 0;
 		connector->display_info.bus_flags |=
-			DRM_BUS_FLAG_PIXDATA_NEGEDGE;
+			DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE;
 	}
 	if (ili->conf->de_active_high) {
 		reg |= ILI9322_POL_DE;
diff --git a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
index 2d99e28ff117..bdcc5d80823d 100644
--- a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
+++ b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
@@ -328,7 +328,7 @@ static const struct seiko_panel_desc seiko_43wvf1g = {
 		.height = 57,
 	},
 	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
-	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_NEGEDGE,
+	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
 };
 
 static const struct of_device_id platform_of_match[] = {
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 9e8218f6a3f2..64516f23a957 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -914,7 +914,7 @@ static const struct panel_desc cdtech_s043wq26h_ct7 = {
 		.width = 95,
 		.height = 54,
 	},
-	.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
 };
 
 static const struct drm_display_mode cdtech_s070wv95_ct16_mode = {
@@ -1034,7 +1034,7 @@ static const struct panel_desc dataimage_scf0700c48ggu18 = {
 		.height = 91,
 	},
 	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
-	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
 };
 
 static const struct display_timing dlc_dlc0700yzg_1_timing = {
@@ -1119,7 +1119,7 @@ static const struct panel_desc edt_et057090dhu = {
 		.height = 86,
 	},
 	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
-	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_NEGEDGE,
+	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
 };
 
 static const struct drm_display_mode edt_etm0700g0dh6_mode = {
@@ -1145,7 +1145,7 @@ static const struct panel_desc edt_etm0700g0dh6 = {
 		.height = 91,
 	},
 	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
-	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_NEGEDGE,
+	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
 };
 
 static const struct panel_desc edt_etm0700g0bdh6 = {
@@ -1157,7 +1157,7 @@ static const struct panel_desc edt_etm0700g0bdh6 = {
 		.height = 91,
 	},
 	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
-	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
 };
 
 static const struct drm_display_mode foxlink_fl500wvr00_a0t_mode = {
@@ -1311,7 +1311,7 @@ static const struct panel_desc innolux_at043tn24 = {
 		.height = 54,
 	},
 	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
-	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
 };
 
 static const struct drm_display_mode innolux_at070tn92_mode = {
@@ -1818,7 +1818,7 @@ static const struct panel_desc nec_nl4827hc19_05b = {
 		.height = 54,
 	},
 	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
-	.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
 };
 
 static const struct drm_display_mode netron_dy_e231732_mode = {
@@ -1867,8 +1867,8 @@ static const struct panel_desc newhaven_nhd_43_480272ef_atxl = {
 		.height = 54,
 	},
 	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
-	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE |
-		     DRM_BUS_FLAG_SYNC_POSEDGE,
+	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
+		     DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
 };
 
 static const struct display_timing nlt_nl192108ac18_02d_timing = {
@@ -2029,7 +2029,7 @@ static const struct panel_desc ortustech_com43h4m85ulc = {
 		.height = 93,
 	},
 	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
-	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
 };
 
 static const struct drm_display_mode pda_91_00156_a0_mode = {
@@ -2398,7 +2398,7 @@ static const struct panel_desc toshiba_lt089ac29000 = {
 		.height = 116,
 	},
 	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
-	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
 };
 
 static const struct drm_display_mode tpk_f07a_0102_mode = {
@@ -2421,7 +2421,7 @@ static const struct panel_desc tpk_f07a_0102 = {
 		.width = 152,
 		.height = 91,
 	},
-	.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
 };
 
 static const struct drm_display_mode tpk_f10a_0102_mode = {
diff --git a/drivers/gpu/drm/panel/panel-tpo-tpg110.c b/drivers/gpu/drm/panel/panel-tpo-tpg110.c
index 5a9f8f4d5d24..25f00cfc1af4 100644
--- a/drivers/gpu/drm/panel/panel-tpo-tpg110.c
+++ b/drivers/gpu/drm/panel/panel-tpo-tpg110.c
@@ -118,7 +118,7 @@ static const struct tpg110_panel_mode tpg110_modes[] = {
 			.vtotal = 480 + 10 + 1 + 35,
 			.vrefresh = 60,
 		},
-		.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
 	},
 	{
 		.name = "640x480 RGB",
@@ -135,7 +135,7 @@ static const struct tpg110_panel_mode tpg110_modes[] = {
 			.vtotal = 480 + 18 + 1 + 27,
 			.vrefresh = 60,
 		},
-		.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
 	},
 	{
 		.name = "480x272 RGB",
@@ -152,7 +152,7 @@ static const struct tpg110_panel_mode tpg110_modes[] = {
 			.vtotal = 272 + 2 + 1 + 12,
 			.vrefresh = 60,
 		},
-		.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
 	},
 	{
 		.name = "480x640 RGB",
@@ -169,7 +169,7 @@ static const struct tpg110_panel_mode tpg110_modes[] = {
 			.vtotal = 640 + 4 + 1 + 8,
 			.vrefresh = 60,
 		},
-		.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
 	},
 	{
 		.name = "400x240 RGB",
@@ -186,7 +186,7 @@ static const struct tpg110_panel_mode tpg110_modes[] = {
 			.vtotal = 240 + 2 + 1 + 20,
 			.vrefresh = 60,
 		},
-		.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
 	},
 };
 
diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 754f6b25f265..0c5d391f0a8f 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,
 			tim2 |= TIM2_IOE;
 
 		if (connector->display_info.bus_flags &
-		    DRM_BUS_FLAG_PIXDATA_NEGEDGE)
+		    DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
 			tim2 |= TIM2_IPC;
 	}
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 7136fc91c603..ca713d200280 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -561,10 +561,10 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
 	 * Following code is a way to avoid quirks all around TCON
 	 * and DOTCLOCK drivers.
 	 */
-	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE)
+	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
 		clk_set_phase(tcon->dclk, 240);
 
-	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
+	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
 		clk_set_phase(tcon->dclk, 0);
 
 	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
diff --git a/drivers/gpu/drm/tve200/tve200_display.c b/drivers/gpu/drm/tve200/tve200_display.c
index e8723a2412a6..d775d10dbe6a 100644
--- a/drivers/gpu/drm/tve200/tve200_display.c
+++ b/drivers/gpu/drm/tve200/tve200_display.c
@@ -149,7 +149,8 @@ static void tve200_display_enable(struct drm_simple_display_pipe *pipe,
 	/* Vsync IRQ at start of Vsync at first */
 	ctrl1 |= TVE200_VSTSTYPE_VSYNC;
 
-	if (connector->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
+	if (connector->display_info.bus_flags &
+	    DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
 		ctrl1 |= TVE200_CTRL_TVCLKP;
 
 	if ((mode->hdisplay == 352 && mode->vdisplay == 240) || /* SIF(525) */
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 9da8c93f7976..5e5129206f40 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -246,10 +246,11 @@ struct drm_bridge_timings {
 	/**
 	 * @sampling_edge:
 	 *
-	 * Tells whether the bridge samples the digital input signal
-	 * from the display engine on the positive or negative edge of the
-	 * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE
-	 * bitwise flags from the DRM connector (bit 2 and 3 valid).
+	 * Tells whether the bridge samples the digital input signals from the
+	 * display engine on the positive or negative edge of the clock. This
+	 * should use the DRM_BUS_FLAG_PIXDATA_SAMPLE_[POS|NEG]EDGE and
+	 * DRM_BUS_FLAG_SYNC_SAMPLE_[POS|NEG]EDGE bitwise flags from the DRM
+	 * connector (bit 2, 3, 6 and 7 valid).
 	 */
 	u32 sampling_edge;
 	/**
-- 
Regards,

Laurent Pinchart

[PATCH/RFC 03/15] drm/bridge: use bus flags in bridge timings

[Laurent Pinchart]

From: Stefan Agner <stefan@agner.ch>

The DRM bus flags convey additional information on pixel data on
the bus. All current available bus flags might be of interest for
a bridge. Remove the sampling_edge field and use bus_flags.

In the case at hand a dumb VGA bridge needs a specific data enable
polarity (DRM_BUS_FLAG_DE_LOW).

Signed-off-by: Stefan Agner <stefan@agner.ch>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/bridge/dumb-vga-dac.c |  6 +++---
 include/drm/drm_bridge.h              | 12 +++++-------
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index 94ed450e308d..e64736c39a9f 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
  */
 static const struct drm_bridge_timings default_dac_timings = {
 	/* Timing specifications, datasheet page 7 */
-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
 	.setup_time_ps = 500,
 	.hold_time_ps = 1500,
 };
@@ -245,7 +245,7 @@ static const struct drm_bridge_timings default_dac_timings = {
  */
 static const struct drm_bridge_timings ti_ths8134_dac_timings = {
 	/* From timing diagram, datasheet page 9 */
-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
 	/* From datasheet, page 12 */
 	.setup_time_ps = 3000,
 	/* I guess this means latched input */
@@ -258,7 +258,7 @@ static const struct drm_bridge_timings ti_ths8134_dac_timings = {
  */
 static const struct drm_bridge_timings ti_ths8135_dac_timings = {
 	/* From timing diagram, datasheet page 14 */
-	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
 	/* From datasheet, page 16 */
 	.setup_time_ps = 2000,
 	.hold_time_ps = 500,
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 5e5129206f40..d4428913a4e1 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -244,15 +244,13 @@ struct drm_bridge_funcs {
  */
 struct drm_bridge_timings {
 	/**
-	 * @sampling_edge:
+	 * @input_bus_flags:
 	 *
-	 * Tells whether the bridge samples the digital input signals from the
-	 * display engine on the positive or negative edge of the clock. This
-	 * should use the DRM_BUS_FLAG_PIXDATA_SAMPLE_[POS|NEG]EDGE and
-	 * DRM_BUS_FLAG_SYNC_SAMPLE_[POS|NEG]EDGE bitwise flags from the DRM
-	 * connector (bit 2, 3, 6 and 7 valid).
+	 * 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 sampling_edge;
+	u32 input_bus_flags;
 	/**
 	 * @setup_time_ps:
 	 *
-- 
Regards,

Laurent Pinchart

[PATCH/RFC 04/15] drm: bridge: Add dual_link field to the drm_bridge_timings structure

[Laurent Pinchart]

Extend the drm_bridge_timings structure with a new dual_link field to
indicate that the bridge's input bus carries data on two separate
physical links. The first use case is LVDS dual-link mode where even-
and odd-numbered pixels are transferred on separate LVDS links.

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

diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index d4428913a4e1..aea1fcfd92a7 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -265,6 +265,14 @@ struct drm_bridge_timings {
 	 * 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;
 };
 
 /**
-- 
Regards,

Laurent Pinchart

[PATCH/RFC 05/15] dt-bindings: display: bridge: thc63lvd1024: Document dual-link operation

[Laurent Pinchart]

The THC63LVD1024 LVDS decoder can operate in two modes, single-link or
dual-link. In dual-link mode both input ports are used to carry even-
and odd-numbered pixels separately. Document this in the DT bindings,
along with the related rules governing port and usage.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../bindings/display/bridge/thine,thc63lvd1024.txt         | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
index 37f0c04d5a28..4ff6eb9bbc19 100644
--- a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
+++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
@@ -28,6 +28,13 @@ Optional video port nodes:
 - port@1: Second LVDS input port
 - port@3: Second digital CMOS/TTL parallel output
 
+The device can operate in single-link mode or dual-link mode. In single-link
+mode, all pixels are received on port@0, and port@1 shall not contain any
+endpoint. In dual-link mode, even-numbered pixels are received on port@0 and
+odd-numbered pixels on port@1, and both port@0 and port@1 shall contain
+endpoints.
+
+
 Example:
 --------
 
-- 
Regards,

Laurent Pinchart

[PATCH/RFC 06/15] drm: bridge: thc63: Report input bus mode through bridge timings

[Laurent Pinchart]

Set a drm_bridge_timings in the drm_bridge, and use it to report the
input bus mode (single-link or dual-link). The other fields of the
timings structure are kept to 0 as they do not apply to LVDS buses.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/bridge/thc63lvd1024.c | 46 ++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c
index b083a740565c..206b0af5e154 100644
--- a/drivers/gpu/drm/bridge/thc63lvd1024.c
+++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
@@ -31,6 +31,8 @@ struct thc63_dev {
 
 	struct drm_bridge bridge;
 	struct drm_bridge *next;
+
+	struct drm_bridge_timings timings;
 };
 
 static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
@@ -48,15 +50,28 @@ static int thc63_attach(struct drm_bridge *bridge)
 static enum drm_mode_status thc63_mode_valid(struct drm_bridge *bridge,
 					const struct drm_display_mode *mode)
 {
+	struct thc63_dev *thc63 = to_thc63(bridge);
+	unsigned int min_freq;
+	unsigned int max_freq;
+
 	/*
-	 * The THC63LVD1024 clock frequency range is 8 to 135 MHz in single-in
-	 * mode. Note that the limits are different in dual-in, single-out mode,
-	 * and will need to be adjusted accordingly.
+	 * The THC63LVD1024 pixel rate range is 8 to 135 MHz in all modes but
+	 * dual-in, single-out where it is 40 to 150 MHz. As dual-in, dual-out
+	 * isn't supported by the driver yet, simply derive the limits from the
+	 * input mode.
 	 */
-	if (mode->clock < 8000)
+	if (thc63->timings.dual_link) {
+		min_freq = 40000;
+		max_freq = 150000;
+	} else {
+		min_freq = 8000;
+		max_freq = 135000;
+	}
+
+	if (mode->clock < min_freq)
 		return MODE_CLOCK_LOW;
 
-	if (mode->clock > 135000)
+	if (mode->clock > max_freq)
 		return MODE_CLOCK_HIGH;
 
 	return MODE_OK;
@@ -101,19 +116,19 @@ static const struct drm_bridge_funcs thc63_bridge_func = {
 
 static int thc63_parse_dt(struct thc63_dev *thc63)
 {
-	struct device_node *thc63_out;
+	struct device_node *endpoint;
 	struct device_node *remote;
 
-	thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
-						  THC63_RGB_OUT0, -1);
-	if (!thc63_out) {
+	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
+						 THC63_RGB_OUT0, -1);
+	if (!endpoint) {
 		dev_err(thc63->dev, "Missing endpoint in port@%u\n",
 			THC63_RGB_OUT0);
 		return -ENODEV;
 	}
 
-	remote = of_graph_get_remote_port_parent(thc63_out);
-	of_node_put(thc63_out);
+	remote = of_graph_get_remote_port_parent(endpoint);
+	of_node_put(endpoint);
 	if (!remote) {
 		dev_err(thc63->dev, "Endpoint in port@%u unconnected\n",
 			THC63_RGB_OUT0);
@@ -132,6 +147,14 @@ static int thc63_parse_dt(struct thc63_dev *thc63)
 	if (!thc63->next)
 		return -EPROBE_DEFER;
 
+	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
+						 THC63_LVDS_IN1, -1);
+	of_node_put(endpoint);
+
+	thc63->timings.dual_link = endpoint != NULL;
+	dev_dbg(thc63->dev, "operating in %s-link mode\n",
+		thc63->timings.dual_link ? "dual" : "single");
+
 	return 0;
 }
 
@@ -188,6 +211,7 @@ static int thc63_probe(struct platform_device *pdev)
 	thc63->bridge.driver_private = thc63;
 	thc63->bridge.of_node = pdev->dev.of_node;
 	thc63->bridge.funcs = &thc63_bridge_func;
+	thc63->bridge.timings = &thc63->timings;
 
 	drm_bridge_add(&thc63->bridge);
 
-- 
Regards,

Laurent Pinchart

[PATCH/RFC 07/15] dt-bindings: display: renesas: lvds: Add renesas,companion property

[Laurent Pinchart]

Add a new optional renesas,companion property to point to the companion
LVDS encoder. This is used to support dual-link operation where the main
LVDS encoder splits even-numbered and odd-numbered pixels between the
two LVDS encoders.

The new property doesn't control the mode of operation, it only
describes the relationship between the master and companion LVDS
encoders.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../devicetree/bindings/display/bridge/renesas,lvds.txt     | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
index 900a884ad9f5..a720dbb5de69 100644
--- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
@@ -45,6 +45,12 @@ OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
 
 Each port shall have a single endpoint.
 
+Optional properties:
+
+- renesas,companion : phandle to the companion LVDS encoder. This property is
+  valid for the first LVDS encoder D3 and E3 SoCs only, and points to the
+  second encoder to be used as a companion in dual-link mode.
+
 
 Example:
 
-- 
Regards,

Laurent Pinchart

[PATCH/RFC 08/15] drm: rcar-du: lvds: Remove LVDS double-enable checks

[Laurent Pinchart]

The DRM core and DU driver guarantee that the LVDS bridge will not be
double-enabled or double-disabled. Remove the corresponding unnecessary
checks.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 7ef97b2a6eda..cd202157264a 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -63,7 +63,6 @@ struct rcar_lvds {
 		struct clk *extal;		/* External clock */
 		struct clk *dotclkin[2];	/* External DU clocks */
 	} clocks;
-	bool enabled;
 
 	struct drm_display_mode display_mode;
 	enum rcar_lvds_mode mode;
@@ -368,15 +367,12 @@ int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq)
 
 	dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
 
-	WARN_ON(lvds->enabled);
-
 	ret = clk_prepare_enable(lvds->clocks.mod);
 	if (ret < 0)
 		return ret;
 
 	__rcar_lvds_pll_setup_d3_e3(lvds, freq, true);
 
-	lvds->enabled = true;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(rcar_lvds_clk_enable);
@@ -390,13 +386,9 @@ void rcar_lvds_clk_disable(struct drm_bridge *bridge)
 
 	dev_dbg(lvds->dev, "disabling LVDS PLL\n");
 
-	WARN_ON(!lvds->enabled);
-
 	rcar_lvds_write(lvds, LVDPLLCR, 0);
 
 	clk_disable_unprepare(lvds->clocks.mod);
-
-	lvds->enabled = false;
 }
 EXPORT_SYMBOL_GPL(rcar_lvds_clk_disable);
 
@@ -417,8 +409,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
 	u32 lvdcr0;
 	int ret;
 
-	WARN_ON(lvds->enabled);
-
 	ret = clk_prepare_enable(lvds->clocks.mod);
 	if (ret < 0)
 		return;
@@ -503,16 +493,12 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
 		drm_panel_prepare(lvds->panel);
 		drm_panel_enable(lvds->panel);
 	}
-
-	lvds->enabled = true;
 }
 
 static void rcar_lvds_disable(struct drm_bridge *bridge)
 {
 	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
 
-	WARN_ON(!lvds->enabled);
-
 	if (lvds->panel) {
 		drm_panel_disable(lvds->panel);
 		drm_panel_unprepare(lvds->panel);
@@ -523,8 +509,6 @@ static void rcar_lvds_disable(struct drm_bridge *bridge)
 	rcar_lvds_write(lvds, LVDPLLCR, 0);
 
 	clk_disable_unprepare(lvds->clocks.mod);
-
-	lvds->enabled = false;
 }
 
 static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
@@ -583,8 +567,6 @@ static void rcar_lvds_mode_set(struct drm_bridge *bridge,
 {
 	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
 
-	WARN_ON(lvds->enabled);
-
 	lvds->display_mode = *adjusted_mode;
 
 	rcar_lvds_get_lvds_mode(lvds);
@@ -784,7 +766,6 @@ static int rcar_lvds_probe(struct platform_device *pdev)
 
 	lvds->dev = &pdev->dev;
 	lvds->info = of_device_get_match_data(&pdev->dev);
-	lvds->enabled = false;
 
 	ret = rcar_lvds_parse_dt(lvds);
 	if (ret < 0)
-- 
Regards,

Laurent Pinchart

[PATCH/RFC 09/15] drm: rcar-du: lvds: Adjust operating frequency for D3 and E3

[Laurent Pinchart]

The D3 and E3 SoCs have different pixel clock frequency limits for the
LVDS encoder than the other SoCs in the Gen3 family. Adjust the mode
fixup implementation accordingly.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index cd202157264a..b1abe737dc05 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -515,11 +515,16 @@ static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
 				 const struct drm_display_mode *mode,
 				 struct drm_display_mode *adjusted_mode)
 {
+	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
+	int min_freq;
+
 	/*
 	 * The internal LVDS encoder has a restricted clock frequency operating
-	 * range (31MHz to 148.5MHz). Clamp the clock accordingly.
+	 * range, from 5MHz to 148.5MHz on D3 and E3, and from 31MHz to
+	 * 148.5MHz on all other platforms. Clamp the clock accordingly.
 	 */
-	adjusted_mode->clock = clamp(adjusted_mode->clock, 31000, 148500);
+	min_freq = lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL ? 5000 : 31000;
+	adjusted_mode->clock = clamp(adjusted_mode->clock, min_freq, 148500);
 
 	return true;
 }
-- 
Regards,

Laurent Pinchart

[PATCH/RFC 10/15] drm: rcar-du: lvds: Set LVEN and LVRES bits together on D3

[Laurent Pinchart]

On the D3 SoC the LVDS PHY must be enabled in the same register write
that enables the LVDS output. Skip writing the LVEN bit independently
on that platform, it will be set by the write that sets LVRES.

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

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index b1abe737dc05..5ac92ee15be0 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -475,9 +475,13 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
 	}
 
 	if (lvds->info->quirks & RCAR_LVDS_QUIRK_GEN3_LVEN) {
-		/* Turn on the LVDS PHY. */
+		/*
+		 * Turn on the LVDS PHY. On D3, the LVEN and LVRES bit must be
+		 * set at the same time, so don't write the register yet.
+		 */
 		lvdcr0 |= LVDCR0_LVEN;
-		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+		if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_PWD))
+			rcar_lvds_write(lvds, LVDCR0, lvdcr0);
 	}
 
 	if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL)) {
-- 
Regards,

Laurent Pinchart

[PATCH/RFC 11/15] drm: rcar-du: lvds: Add support for dual-link mode

[Laurent Pinchart]

In dual-link mode the LVDS0 encoder transmits even-numbered pixels, and
sends odd-numbered pixels to the LVDS1 encoder for transmission on a
separate link.

To implement support for this mode of operation, determine if the LVDS
connection operates in dual-link mode by querying the next device in the
pipeline, locate the companion encoder, and control it directly through
its bridge operations.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 104 ++++++++++++++++++++++++----
 drivers/gpu/drm/rcar-du/rcar_lvds.h |   5 ++
 2 files changed, 96 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 5ac92ee15be0..90d33514bb3e 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -66,6 +66,9 @@ struct rcar_lvds {
 
 	struct drm_display_mode display_mode;
 	enum rcar_lvds_mode mode;
+
+	struct drm_bridge *companion;
+	bool dual_link;
 };
 
 #define bridge_to_rcar_lvds(bridge) \
@@ -400,11 +403,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
 {
 	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
 	const struct drm_display_mode *mode = &lvds->display_mode;
-	/*
-	 * FIXME: We should really retrieve the CRTC through the state, but how
-	 * do we get a state pointer?
-	 */
-	struct drm_crtc *crtc = lvds->bridge.encoder->crtc;
 	u32 lvdhcr;
 	u32 lvdcr0;
 	int ret;
@@ -413,6 +411,10 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
 	if (ret < 0)
 		return;
 
+	/* Enable the companion LVDS encoder in dual-link mode. */
+	if (lvds->dual_link && lvds->companion)
+		lvds->companion->funcs->enable(lvds->companion);
+
 	/*
 	 * Hardcode the channels and control signals routing for now.
 	 *
@@ -435,17 +437,33 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
 	rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
 
 	if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
-		/* Disable dual-link mode. */
-		rcar_lvds_write(lvds, LVDSTRIPE, 0);
+		/*
+		 * Configure vertical stripe based on the mode of operation of
+		 * the connected device.
+		 */
+		rcar_lvds_write(lvds, LVDSTRIPE,
+				lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
 	}
 
-	/* PLL clock configuration. */
-	lvds->info->pll_setup(lvds, mode->clock * 1000);
+	/*
+	 * PLL clock configuration on all instances but the companion in
+	 * dual-link mode.
+	 */
+	if (!lvds->dual_link || lvds->companion)
+		lvds->info->pll_setup(lvds, mode->clock * 1000);
 
 	/* Set the LVDS mode and select the input. */
 	lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
-	if (drm_crtc_index(crtc) == 2)
-		lvdcr0 |= LVDCR0_DUSEL;
+
+	if (lvds->bridge.encoder) {
+		/*
+		 * FIXME: We should really retrieve the CRTC through the state,
+		 * but how do we get a state pointer?
+		 */
+		if (drm_crtc_index(lvds->bridge.encoder->crtc) == 2)
+			lvdcr0 |= LVDCR0_DUSEL;
+	}
+
 	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
 
 	/* Turn all the channels on. */
@@ -512,6 +530,10 @@ static void rcar_lvds_disable(struct drm_bridge *bridge)
 	rcar_lvds_write(lvds, LVDCR1, 0);
 	rcar_lvds_write(lvds, LVDPLLCR, 0);
 
+	/* Disable the companion LVDS encoder in dual-link mode. */
+	if (lvds->dual_link && lvds->companion)
+		lvds->companion->funcs->disable(lvds->companion);
+
 	clk_disable_unprepare(lvds->clocks.mod);
 }
 
@@ -628,10 +650,54 @@ static const struct drm_bridge_funcs rcar_lvds_bridge_ops = {
 	.mode_set = rcar_lvds_mode_set,
 };
 
+bool rcar_lvds_dual_link(struct drm_bridge *bridge)
+{
+	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
+
+	return lvds->dual_link;
+}
+EXPORT_SYMBOL_GPL(rcar_lvds_dual_link);
+
 /* -----------------------------------------------------------------------------
  * Probe & Remove
  */
 
+static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds)
+{
+	const struct of_device_id *match;
+	struct device_node *companion;
+	struct device *dev = lvds->dev;
+	int ret = 0;
+
+	/* Locate the companion LVDS encoder for dual-link operation, if any. */
+	companion = of_parse_phandle(dev->of_node, "renesas,companion", 0);
+	if (!companion)
+		return -ENODEV;
+
+	/*
+	 * Sanity check: the companion encoder must have the same compatible
+	 * string.
+	 */
+	match = of_match_device(dev->driver->of_match_table, dev);
+	if (!of_device_is_compatible(companion, match->compatible)) {
+		ret = -ENODEV;
+		goto done;
+	}
+
+	lvds->companion = of_drm_find_bridge(companion);
+	if (!lvds->companion) {
+		ret = -EPROBE_DEFER;
+		goto done;
+	}
+
+	dev_dbg(dev, "Found companion encoder %pOF\n", companion);
+
+done:
+	of_node_put(companion);
+
+	return ret;
+}
+
 static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
 {
 	struct device_node *local_output = NULL;
@@ -682,14 +748,26 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
 
 	if (is_bridge) {
 		lvds->next_bridge = of_drm_find_bridge(remote);
-		if (!lvds->next_bridge)
+		if (!lvds->next_bridge) {
 			ret = -EPROBE_DEFER;
+			goto done;
+		}
+
+		if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
+			lvds->dual_link = lvds->next_bridge->timings
+					? lvds->next_bridge->timings->dual_link
+					: false;
 	} else {
 		lvds->panel = of_drm_find_panel(remote);
-		if (IS_ERR(lvds->panel))
+		if (IS_ERR(lvds->panel)) {
 			ret = PTR_ERR(lvds->panel);
+			goto done;
+		}
 	}
 
+	if (lvds->dual_link)
+		ret = rcar_lvds_parse_dt_companion(lvds);
+
 done:
 	of_node_put(local_output);
 	of_node_put(remote_input);
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.h b/drivers/gpu/drm/rcar-du/rcar_lvds.h
index a709cae1bc32..222ec0e60785 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.h
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.h
@@ -15,6 +15,7 @@ struct drm_bridge;
 #if IS_ENABLED(CONFIG_DRM_RCAR_LVDS)
 int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq);
 void rcar_lvds_clk_disable(struct drm_bridge *bridge);
+bool rcar_lvds_dual_link(struct drm_bridge *bridge);
 #else
 static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge,
 				       unsigned long freq)
@@ -22,6 +23,10 @@ static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge,
 	return -ENOSYS;
 }
 static inline void rcar_lvds_clk_disable(struct drm_bridge *bridge) { }
+static inline bool rcar_lvds_dual_link(struct drm_bridge *bridge)
+{
+	return false;
+}
 #endif /* CONFIG_DRM_RCAR_LVDS */
 
 #endif /* __RCAR_LVDS_H__ */
-- 
Regards,

Laurent Pinchart

[PATCH/RFC 12/15] drm: rcar-du: Skip LVDS1 output on Gen3 when using dual-link LVDS mode

[Laurent Pinchart]

In dual-link LVDS mode, the LVDS1 encoder is used as a companion for
LVDS0, and both encoders transmit data from DU0. The LVDS1 output of DU1
can't be used in that case, don't create an encoder and connector for
it.

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

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
index 6c91753af7bc..fe046d194944 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
@@ -16,6 +16,7 @@
 #include "rcar_du_drv.h"
 #include "rcar_du_encoder.h"
 #include "rcar_du_kms.h"
+#include "rcar_lvds.h"
 
 /* -----------------------------------------------------------------------------
  * Encoder
@@ -97,6 +98,17 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
 		}
 	}
 
+	/*
+	 * On Gen3 skip the LVDS1 output if the LVDS1 encoder is used as a
+	 * companion for LVDS0 in dual-link mode.
+	 */
+	if (rcdu->info->gen >= 3 && output == RCAR_DU_OUTPUT_LVDS1) {
+		if (bridge && rcar_lvds_dual_link(bridge)) {
+			ret = -ENOLINK;
+			goto done;
+		}
+	}
+
 	ret = drm_encoder_init(rcdu->ddev, encoder, &encoder_funcs,
 			       DRM_MODE_ENCODER_NONE, NULL);
 	if (ret < 0)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index 3b7d50a8fb9b..7f9d43a1ee42 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -352,7 +352,7 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu,
 	}
 
 	ret = rcar_du_encoder_init(rcdu, output, entity);
-	if (ret && ret != -EPROBE_DEFER)
+	if (ret && ret != -EPROBE_DEFER && ret != -ENOLINK)
 		dev_warn(rcdu->dev,
 			 "failed to initialize encoder %pOF on output %u (%d), skipping\n",
 			 entity, output, ret);
-- 
Regards,

Laurent Pinchart

[PATCH/RFC 13/15] arm64: dts: renesas: r8a7799[05]: Point LVDS0 to its companion LVDS1

[Laurent Pinchart]

Add the new renesas,companion property to the LVDS0 node to point to the
companion LVDS encoder LVDS1.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a77990.dtsi | 2 ++
 arch/arm64/boot/dts/renesas/r8a77995.dtsi | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
index b2f606e286ce..90c5ce8ab7e1 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
@@ -1678,6 +1678,8 @@
 			resets = <&cpg 727>;
 			status = "disabled";
 
+			renesas,companion = <&lvds1>;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
index 8530d9fc1371..81762884a2dc 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
@@ -1038,6 +1038,8 @@
 			resets = <&cpg 727>;
 			status = "disabled";
 
+			renesas,companion = <&lvds1>;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
-- 
Regards,

Laurent Pinchart

[PATCH/RFC 14/15] [HACK] arm64: dts: renesas: draak: Enable LVDS dual-link operation

[Laurent Pinchart]

Enable and connect the second LVDS encoder to the second LVDS input of
the THC63LVD1024 for dual-link LVDS operation. This requires changing
the default settings of SW45 and SW47 to OFF and ON respectively.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../arm64/boot/dts/renesas/r8a77995-draak.dts | 21 +++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index 89df9bc844c0..f48dbe35c4b1 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -77,11 +77,18 @@
 
 			port@0 {
 				reg = <0>;
-				thc63lvd1024_in: endpoint {
+				thc63lvd1024_in0: endpoint {
 					remote-endpoint = <&lvds0_out>;
 				};
 			};
 
+			port@1 {
+				reg = <1>;
+				thc63lvd1024_in1: endpoint {
+					remote-endpoint = <&lvds1_out>;
+				};
+			};
+
 			port@2 {
 				reg = <2>;
 				thc63lvd1024_out: endpoint {
@@ -349,17 +356,27 @@
 	ports {
 		port@1 {
 			lvds0_out: endpoint {
-				remote-endpoint = <&thc63lvd1024_in>;
+				remote-endpoint = <&thc63lvd1024_in0>;
 			};
 		};
 	};
 };
 
 &lvds1 {
+	status = "okay";
+
 	clocks = <&cpg CPG_MOD 727>,
 		 <&x12_clk>,
 		 <&extal_clk>;
 	clock-names = "fck", "dclkin.0", "extal";
+
+	ports {
+		port@1 {
+			lvds1_out: endpoint {
+				remote-endpoint = <&thc63lvd1024_in1>;
+			};
+		};
+	};
 };
 
 &ohci0 {
-- 
Regards,

Laurent Pinchart

[PATCH/RFC 15/15] [HACK] arm64: dts: renesas: ebisu: Enable LVDS dual-link operation

[Laurent Pinchart]

Enable and connect the second LVDS encoder to the second LVDS input of
the THC63LVD1024 for dual-link LVDS operation. This requires changing
the default settings of SW45 and SW47 to OFF and ON respectively.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../arm64/boot/dts/renesas/r8a77990-ebisu.dts | 21 +++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
index 62bdddcbbae7..4b9f3305b666 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
@@ -83,11 +83,18 @@
 
 			port@0 {
 				reg = <0>;
-				thc63lvd1024_in: endpoint {
+				thc63lvd1024_in0: endpoint {
 					remote-endpoint = <&lvds0_out>;
 				};
 			};
 
+			port@1 {
+				reg = <1>;
+				thc63lvd1024_in1: endpoint {
+					remote-endpoint = <&lvds1_out>;
+				};
+			};
+
 			port@2 {
 				reg = <2>;
 				thc63lvd1024_out: endpoint {
@@ -436,17 +443,27 @@
 	ports {
 		port@1 {
 			lvds0_out: endpoint {
-				remote-endpoint = <&thc63lvd1024_in>;
+				remote-endpoint = <&thc63lvd1024_in0>;
 			};
 		};
 	};
 };
 
 &lvds1 {
+	status = "okay";
+
 	clocks = <&cpg CPG_MOD 727>,
 		 <&x13_clk>,
 		 <&extal_clk>;
 	clock-names = "fck", "dclkin.0", "extal";
+
+	ports {
+		port@1 {
+			lvds1_out: endpoint {
+				remote-endpoint = <&thc63lvd1024_in1>;
+			};
+		};
+	};
 };
 
 &ohci0 {
-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 10/15] drm: rcar-du: lvds: Set LVEN and LVRES bits together on D3

[Jacopo Mondi]

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

Hi Laurent,

On Thu, Mar 07, 2019 at 01:23:40AM +0200, Laurent Pinchart wrote:
> On the D3 SoC the LVDS PHY must be enabled in the same register write
> that enables the LVDS output. Skip writing the LVEN bit independently
> on that platform, it will be set by the write that sets LVRES.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index b1abe737dc05..5ac92ee15be0 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -475,9 +475,13 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
>  	}
>
>  	if (lvds->info->quirks & RCAR_LVDS_QUIRK_GEN3_LVEN) {
> -		/* Turn on the LVDS PHY. */
> +		/*
> +		 * Turn on the LVDS PHY. On D3, the LVEN and LVRES bit must be
> +		 * set at the same time, so don't write the register yet.
> +		 */
>  		lvdcr0 |= LVDCR0_LVEN;
> -		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> +		if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_PWD))

This is quite obscure, and works because D3 is the only SoC that has
(quirks & RCAR_LVDS_QUIRK_GEN3_LVEN) and (!(quirks & RCAR_LVDS_QUIRK_PWD)).

I guess there are not many ways around this.

> +			rcar_lvds_write(lvds, LVDCR0, lvdcr0);

I have verified this against the latest v1.50 datasheet, and it
matches what's reported in section 37.3.7 so please add my:
Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

I would like just to add that the same section prescribes a precise
order in which LVDS0 and LVDS1 have to be configured when working with
vertical stripe output. Is that enforced in this series?

Thanks
   j

>  	}
>
>  	if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL)) {
> --
> Regards,
>
> Laurent Pinchart
>

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

Re: [PATCH/RFC 09/15] drm: rcar-du: lvds: Adjust operating frequency for D3 and E3

[Jacopo Mondi]

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

Hi Laurent,
   thanks, this matches section 37.1.1 of v1.50 datasheet

Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Thanks
  j

On Thu, Mar 07, 2019 at 01:23:39AM +0200, Laurent Pinchart wrote:
> The D3 and E3 SoCs have different pixel clock frequency limits for the
> LVDS encoder than the other SoCs in the Gen3 family. Adjust the mode
> fixup implementation accordingly.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index cd202157264a..b1abe737dc05 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -515,11 +515,16 @@ static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
>  				 const struct drm_display_mode *mode,
>  				 struct drm_display_mode *adjusted_mode)
>  {
> +	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> +	int min_freq;
> +
>  	/*
>  	 * The internal LVDS encoder has a restricted clock frequency operating
> -	 * range (31MHz to 148.5MHz). Clamp the clock accordingly.
> +	 * range, from 5MHz to 148.5MHz on D3 and E3, and from 31MHz to
> +	 * 148.5MHz on all other platforms. Clamp the clock accordingly.
>  	 */
> -	adjusted_mode->clock = clamp(adjusted_mode->clock, 31000, 148500);
> +	min_freq = lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL ? 5000 : 31000;
> +	adjusted_mode->clock = clamp(adjusted_mode->clock, min_freq, 148500);
>
>  	return true;
>  }
> --
> Regards,
>
> Laurent Pinchart
>

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

Re: [PATCH/RFC 05/15] dt-bindings: display: bridge: thc63lvd1024: Document dual-link operation

[Jacopo Mondi]

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

Hi Laurent,

On Thu, Mar 07, 2019 at 01:23:35AM +0200, Laurent Pinchart wrote:
> The THC63LVD1024 LVDS decoder can operate in two modes, single-link or
> dual-link. In dual-link mode both input ports are used to carry even-
> and odd-numbered pixels separately. Document this in the DT bindings,
> along with the related rules governing port and usage.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../bindings/display/bridge/thine,thc63lvd1024.txt         | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> index 37f0c04d5a28..4ff6eb9bbc19 100644
> --- a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> @@ -28,6 +28,13 @@ Optional video port nodes:
>  - port@1: Second LVDS input port
>  - port@3: Second digital CMOS/TTL parallel output
>
> +The device can operate in single-link mode or dual-link mode. In single-link
> +mode, all pixels are received on port@0, and port@1 shall not contain any
> +endpoint. In dual-link mode, even-numbered pixels are received on port@0 and
> +odd-numbered pixels on port@1, and both port@0 and port@1 shall contain
> +endpoints.

You know, I'm not sure this is helpful, as if we have to go and
describe what the chip supports, a paragraph for dual ouput mode would
be required as well. The bindings already document that the chip
supports single/dual input/output modes, maybe you can just add rules
that prescribes how to populate the endpoints, for both input and
output modes?

> +
> +

You can drop this empty line.

Thanks
   j

>  Example:
>  --------
>
> --
> Regards,
>
> Laurent Pinchart
>

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

Re: [PATCH/RFC 11/15] drm: rcar-du: lvds: Add support for dual-link mode

[Jacopo Mondi]

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

Hi Laurent,

On Thu, Mar 07, 2019 at 01:23:41AM +0200, Laurent Pinchart wrote:
> In dual-link mode the LVDS0 encoder transmits even-numbered pixels, and
> sends odd-numbered pixels to the LVDS1 encoder for transmission on a
> separate link.
>
> To implement support for this mode of operation, determine if the LVDS
> connection operates in dual-link mode by querying the next device in the
> pipeline, locate the companion encoder, and control it directly through
> its bridge operations.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 104 ++++++++++++++++++++++++----
>  drivers/gpu/drm/rcar-du/rcar_lvds.h |   5 ++
>  2 files changed, 96 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 5ac92ee15be0..90d33514bb3e 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -66,6 +66,9 @@ struct rcar_lvds {
>
>  	struct drm_display_mode display_mode;
>  	enum rcar_lvds_mode mode;
> +
> +	struct drm_bridge *companion;
> +	bool dual_link;
>  };
>
>  #define bridge_to_rcar_lvds(bridge) \
> @@ -400,11 +403,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
>  {
>  	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
>  	const struct drm_display_mode *mode = &lvds->display_mode;
> -	/*
> -	 * FIXME: We should really retrieve the CRTC through the state, but how
> -	 * do we get a state pointer?
> -	 */
> -	struct drm_crtc *crtc = lvds->bridge.encoder->crtc;
>  	u32 lvdhcr;
>  	u32 lvdcr0;
>  	int ret;
> @@ -413,6 +411,10 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
>  	if (ret < 0)
>  		return;
>
> +	/* Enable the companion LVDS encoder in dual-link mode. */
> +	if (lvds->dual_link && lvds->companion)
> +		lvds->companion->funcs->enable(lvds->companion);
> +
>  	/*
>  	 * Hardcode the channels and control signals routing for now.
>  	 *
> @@ -435,17 +437,33 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
>  	rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
>
>  	if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
> -		/* Disable dual-link mode. */
> -		rcar_lvds_write(lvds, LVDSTRIPE, 0);
> +		/*
> +		 * Configure vertical stripe based on the mode of operation of
> +		 * the connected device.
> +		 */
> +		rcar_lvds_write(lvds, LVDSTRIPE,
> +				lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
>  	}
>
> -	/* PLL clock configuration. */
> -	lvds->info->pll_setup(lvds, mode->clock * 1000);
> +	/*
> +	 * PLL clock configuration on all instances but the companion in
> +	 * dual-link mode.
> +	 */
> +	if (!lvds->dual_link || lvds->companion)
> +		lvds->info->pll_setup(lvds, mode->clock * 1000);
>
>  	/* Set the LVDS mode and select the input. */
>  	lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
> -	if (drm_crtc_index(crtc) == 2)
> -		lvdcr0 |= LVDCR0_DUSEL;
> +
> +	if (lvds->bridge.encoder) {
> +		/*
> +		 * FIXME: We should really retrieve the CRTC through the state,
> +		 * but how do we get a state pointer?
> +		 */
> +		if (drm_crtc_index(lvds->bridge.encoder->crtc) == 2)
> +			lvdcr0 |= LVDCR0_DUSEL;
> +	}
> +
>  	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
>
>  	/* Turn all the channels on. */
> @@ -512,6 +530,10 @@ static void rcar_lvds_disable(struct drm_bridge *bridge)
>  	rcar_lvds_write(lvds, LVDCR1, 0);
>  	rcar_lvds_write(lvds, LVDPLLCR, 0);
>
> +	/* Disable the companion LVDS encoder in dual-link mode. */
> +	if (lvds->dual_link && lvds->companion)
> +		lvds->companion->funcs->disable(lvds->companion);
> +
>  	clk_disable_unprepare(lvds->clocks.mod);
>  }
>
> @@ -628,10 +650,54 @@ static const struct drm_bridge_funcs rcar_lvds_bridge_ops = {
>  	.mode_set = rcar_lvds_mode_set,
>  };
>
> +bool rcar_lvds_dual_link(struct drm_bridge *bridge)
> +{
> +	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> +
> +	return lvds->dual_link;
> +}
> +EXPORT_SYMBOL_GPL(rcar_lvds_dual_link);
> +
>  /* -----------------------------------------------------------------------------
>   * Probe & Remove
>   */
>
> +static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds)
> +{
> +	const struct of_device_id *match;
> +	struct device_node *companion;
> +	struct device *dev = lvds->dev;
> +	int ret = 0;
> +
> +	/* Locate the companion LVDS encoder for dual-link operation, if any. */
> +	companion = of_parse_phandle(dev->of_node, "renesas,companion", 0);
> +	if (!companion)
> +		return -ENODEV;
> +
> +	/*
> +	 * Sanity check: the companion encoder must have the same compatible
> +	 * string.
> +	 */
> +	match = of_match_device(dev->driver->of_match_table, dev);
> +	if (!of_device_is_compatible(companion, match->compatible)) {
> +		ret = -ENODEV;
> +		goto done;
> +	}
> +
> +	lvds->companion = of_drm_find_bridge(companion);
> +	if (!lvds->companion) {
> +		ret = -EPROBE_DEFER;
> +		goto done;
> +	}
> +
> +	dev_dbg(dev, "Found companion encoder %pOF\n", companion);
> +
> +done:
> +	of_node_put(companion);
> +
> +	return ret;
> +}
> +
>  static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>  {
>  	struct device_node *local_output = NULL;
> @@ -682,14 +748,26 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>
>  	if (is_bridge) {
>  		lvds->next_bridge = of_drm_find_bridge(remote);
> -		if (!lvds->next_bridge)
> +		if (!lvds->next_bridge) {
>  			ret = -EPROBE_DEFER;
> +			goto done;
> +		}
> +
> +		if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
> +			lvds->dual_link = lvds->next_bridge->timings
> +					? lvds->next_bridge->timings->dual_link
> +					: false;

I wonder if, in all this patch, you could not use "lvds->companion" in
place of "lvds->dual_link", and thus drop that from timings. This mean
"renesas,companion" would only be used when operating in dual link
mode, which might be less nice (and could actually called differently
in that case).

Both the THC631024 and the rcar_du-lvds driver would decide
independently if they operate in dual link mode or not (one counting
its endpoints, the other inspecting the "renesas,companion" property).

The only place where this might be tricky is the here above

 +	/*
 +	 * PLL clock configuration on all instances but the companion in
 +	 * dual-link mode.
 +	 */
 +	if (!lvds->dual_link || lvds->companion)
 +		lvds->info->pll_setup(lvds, mode->clock * 1000);

but that would remove the need for the thc63 bridge to report its
operating mode in timings...

>  	} else {
>  		lvds->panel = of_drm_find_panel(remote);
> -		if (IS_ERR(lvds->panel))
> +		if (IS_ERR(lvds->panel)) {
>  			ret = PTR_ERR(lvds->panel);
> +			goto done;
> +		}
>  	}
>
> +	if (lvds->dual_link)

Note: if (!is_bridge) you would never set lvds->dual_link, so this
should be moved inside the here above "if (is_bridge)"

> +		ret = rcar_lvds_parse_dt_companion(lvds);
> +
>  done:
>  	of_node_put(local_output);
>  	of_node_put(remote_input);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.h b/drivers/gpu/drm/rcar-du/rcar_lvds.h
> index a709cae1bc32..222ec0e60785 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.h
> @@ -15,6 +15,7 @@ struct drm_bridge;
>  #if IS_ENABLED(CONFIG_DRM_RCAR_LVDS)
>  int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq);
>  void rcar_lvds_clk_disable(struct drm_bridge *bridge);
> +bool rcar_lvds_dual_link(struct drm_bridge *bridge);
>  #else
>  static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge,
>  				       unsigned long freq)
> @@ -22,6 +23,10 @@ static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge,
>  	return -ENOSYS;
>  }
>  static inline void rcar_lvds_clk_disable(struct drm_bridge *bridge) { }
> +static inline bool rcar_lvds_dual_link(struct drm_bridge *bridge)
> +{
> +	return false;
> +}
>  #endif /* CONFIG_DRM_RCAR_LVDS */
>
>  #endif /* __RCAR_LVDS_H__ */
> --
> Regards,
>
> Laurent Pinchart
>

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

Re: [PATCH/RFC 06/15] drm: bridge: thc63: Report input bus mode through bridge timings

[Jacopo Mondi]

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

Hi Laurent,

On Thu, Mar 07, 2019 at 01:23:36AM +0200, Laurent Pinchart wrote:
> Set a drm_bridge_timings in the drm_bridge, and use it to report the
> input bus mode (single-link or dual-link). The other fields of the
> timings structure are kept to 0 as they do not apply to LVDS buses.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/bridge/thc63lvd1024.c | 46 ++++++++++++++++++++-------
>  1 file changed, 35 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c
> index b083a740565c..206b0af5e154 100644
> --- a/drivers/gpu/drm/bridge/thc63lvd1024.c
> +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
> @@ -31,6 +31,8 @@ struct thc63_dev {
>
>  	struct drm_bridge bridge;
>  	struct drm_bridge *next;
> +
> +	struct drm_bridge_timings timings;
>  };
>
>  static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
> @@ -48,15 +50,28 @@ static int thc63_attach(struct drm_bridge *bridge)
>  static enum drm_mode_status thc63_mode_valid(struct drm_bridge *bridge,
>  					const struct drm_display_mode *mode)
>  {
> +	struct thc63_dev *thc63 = to_thc63(bridge);
> +	unsigned int min_freq;
> +	unsigned int max_freq;
> +
>  	/*
> -	 * The THC63LVD1024 clock frequency range is 8 to 135 MHz in single-in
> -	 * mode. Note that the limits are different in dual-in, single-out mode,
> -	 * and will need to be adjusted accordingly.
> +	 * The THC63LVD1024 pixel rate range is 8 to 135 MHz in all modes but
> +	 * dual-in, single-out where it is 40 to 150 MHz. As dual-in, dual-out
> +	 * isn't supported by the driver yet, simply derive the limits from the
> +	 * input mode.
>  	 */
> -	if (mode->clock < 8000)
> +	if (thc63->timings.dual_link) {
> +		min_freq = 40000;
> +		max_freq = 150000;
> +	} else {
> +		min_freq = 8000;
> +		max_freq = 135000;
> +	}
> +
> +	if (mode->clock < min_freq)
>  		return MODE_CLOCK_LOW;
>
> -	if (mode->clock > 135000)
> +	if (mode->clock > max_freq)
>  		return MODE_CLOCK_HIGH;
>
>  	return MODE_OK;
> @@ -101,19 +116,19 @@ static const struct drm_bridge_funcs thc63_bridge_func = {
>
>  static int thc63_parse_dt(struct thc63_dev *thc63)
>  {
> -	struct device_node *thc63_out;
> +	struct device_node *endpoint;
>  	struct device_node *remote;
>
> -	thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> -						  THC63_RGB_OUT0, -1);
> -	if (!thc63_out) {
> +	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> +						 THC63_RGB_OUT0, -1);
> +	if (!endpoint) {
>  		dev_err(thc63->dev, "Missing endpoint in port@%u\n",
>  			THC63_RGB_OUT0);
>  		return -ENODEV;
>  	}
>
> -	remote = of_graph_get_remote_port_parent(thc63_out);
> -	of_node_put(thc63_out);
> +	remote = of_graph_get_remote_port_parent(endpoint);
> +	of_node_put(endpoint);
>  	if (!remote) {
>  		dev_err(thc63->dev, "Endpoint in port@%u unconnected\n",
>  			THC63_RGB_OUT0);
> @@ -132,6 +147,14 @@ static int thc63_parse_dt(struct thc63_dev *thc63)
>  	if (!thc63->next)
>  		return -EPROBE_DEFER;
>
> +	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> +						 THC63_LVDS_IN1, -1);
> +	of_node_put(endpoint);
> +

Should you check if endpoint is enabled?

By the way, this seems to me works properly, as in [12/15] if skip
creation of the LVDS1 encoder if it is operating in dual link mode.
Without that, we would hit again the issue of matching on device
nodes, as the same bridge would be attached twice, am I wrong?

I feel we've been there already :)
https://lkml.org/lkml/2018/3/20/341

and it is not just an issue on matching on endpoints, it's that the
drm bridge itself that would need to handle multiple attach/detaches..

> +	thc63->timings.dual_link = endpoint != NULL;
> +	dev_dbg(thc63->dev, "operating in %s-link mode\n",
> +		thc63->timings.dual_link ? "dual" : "single");
> +
>  	return 0;
>  }
>
> @@ -188,6 +211,7 @@ static int thc63_probe(struct platform_device *pdev)
>  	thc63->bridge.driver_private = thc63;
>  	thc63->bridge.of_node = pdev->dev.of_node;
>  	thc63->bridge.funcs = &thc63_bridge_func;
> +	thc63->bridge.timings = &thc63->timings;
>
>  	drm_bridge_add(&thc63->bridge);
>
> --
> Regards,
>
> Laurent Pinchart
>

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

Re: [PATCH/RFC 05/15] dt-bindings: display: bridge: thc63lvd1024: Document dual-link operation

[Laurent Pinchart]

Hi Jacopo,

On Fri, Mar 08, 2019 at 05:49:25PM +0100, Jacopo Mondi wrote:
> On Thu, Mar 07, 2019 at 01:23:35AM +0200, Laurent Pinchart wrote:
> > The THC63LVD1024 LVDS decoder can operate in two modes, single-link or
> > dual-link. In dual-link mode both input ports are used to carry even-
> > and odd-numbered pixels separately. Document this in the DT bindings,
> > along with the related rules governing port and usage.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  .../bindings/display/bridge/thine,thc63lvd1024.txt         | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > index 37f0c04d5a28..4ff6eb9bbc19 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > @@ -28,6 +28,13 @@ Optional video port nodes:
> >  - port@1: Second LVDS input port
> >  - port@3: Second digital CMOS/TTL parallel output
> >
> > +The device can operate in single-link mode or dual-link mode. In single-link
> > +mode, all pixels are received on port@0, and port@1 shall not contain any
> > +endpoint. In dual-link mode, even-numbered pixels are received on port@0 and
> > +odd-numbered pixels on port@1, and both port@0 and port@1 shall contain
> > +endpoints.
> 
> You know, I'm not sure this is helpful, as if we have to go and
> describe what the chip supports, a paragraph for dual ouput mode would
> be required as well. The bindings already document that the chip
> supports single/dual input/output modes, maybe you can just add rules
> that prescribes how to populate the endpoints, for both input and
> output modes?

That's what I was trying to do :-) How else would you like to see it
described ?

> > +
> > +
> 
> You can drop this empty line.
> 
> >  Example:
> >  --------
> >

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 06/15] drm: bridge: thc63: Report input bus mode through bridge timings

[Laurent Pinchart]

Hi Jacopo,

On Fri, Mar 08, 2019 at 06:32:59PM +0100, Jacopo Mondi wrote:
> On Thu, Mar 07, 2019 at 01:23:36AM +0200, Laurent Pinchart wrote:
> > Set a drm_bridge_timings in the drm_bridge, and use it to report the
> > input bus mode (single-link or dual-link). The other fields of the
> > timings structure are kept to 0 as they do not apply to LVDS buses.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  drivers/gpu/drm/bridge/thc63lvd1024.c | 46 ++++++++++++++++++++-------
> >  1 file changed, 35 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c
> > index b083a740565c..206b0af5e154 100644
> > --- a/drivers/gpu/drm/bridge/thc63lvd1024.c
> > +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
> > @@ -31,6 +31,8 @@ struct thc63_dev {
> >
> >  	struct drm_bridge bridge;
> >  	struct drm_bridge *next;
> > +
> > +	struct drm_bridge_timings timings;
> >  };
> >
> >  static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
> > @@ -48,15 +50,28 @@ static int thc63_attach(struct drm_bridge *bridge)
> >  static enum drm_mode_status thc63_mode_valid(struct drm_bridge *bridge,
> >  					const struct drm_display_mode *mode)
> >  {
> > +	struct thc63_dev *thc63 = to_thc63(bridge);
> > +	unsigned int min_freq;
> > +	unsigned int max_freq;
> > +
> >  	/*
> > -	 * The THC63LVD1024 clock frequency range is 8 to 135 MHz in single-in
> > -	 * mode. Note that the limits are different in dual-in, single-out mode,
> > -	 * and will need to be adjusted accordingly.
> > +	 * The THC63LVD1024 pixel rate range is 8 to 135 MHz in all modes but
> > +	 * dual-in, single-out where it is 40 to 150 MHz. As dual-in, dual-out
> > +	 * isn't supported by the driver yet, simply derive the limits from the
> > +	 * input mode.
> >  	 */
> > -	if (mode->clock < 8000)
> > +	if (thc63->timings.dual_link) {
> > +		min_freq = 40000;
> > +		max_freq = 150000;
> > +	} else {
> > +		min_freq = 8000;
> > +		max_freq = 135000;
> > +	}
> > +
> > +	if (mode->clock < min_freq)
> >  		return MODE_CLOCK_LOW;
> >
> > -	if (mode->clock > 135000)
> > +	if (mode->clock > max_freq)
> >  		return MODE_CLOCK_HIGH;
> >
> >  	return MODE_OK;
> > @@ -101,19 +116,19 @@ static const struct drm_bridge_funcs thc63_bridge_func = {
> >
> >  static int thc63_parse_dt(struct thc63_dev *thc63)
> >  {
> > -	struct device_node *thc63_out;
> > +	struct device_node *endpoint;
> >  	struct device_node *remote;
> >
> > -	thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> > -						  THC63_RGB_OUT0, -1);
> > -	if (!thc63_out) {
> > +	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> > +						 THC63_RGB_OUT0, -1);
> > +	if (!endpoint) {
> >  		dev_err(thc63->dev, "Missing endpoint in port@%u\n",
> >  			THC63_RGB_OUT0);
> >  		return -ENODEV;
> >  	}
> >
> > -	remote = of_graph_get_remote_port_parent(thc63_out);
> > -	of_node_put(thc63_out);
> > +	remote = of_graph_get_remote_port_parent(endpoint);
> > +	of_node_put(endpoint);
> >  	if (!remote) {
> >  		dev_err(thc63->dev, "Endpoint in port@%u unconnected\n",
> >  			THC63_RGB_OUT0);
> > @@ -132,6 +147,14 @@ static int thc63_parse_dt(struct thc63_dev *thc63)
> >  	if (!thc63->next)
> >  		return -EPROBE_DEFER;
> >
> > +	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> > +						 THC63_LVDS_IN1, -1);
> > +	of_node_put(endpoint);
> > +
> 
> Should you check if endpoint is enabled?

Can endpoints be disabled ?

> By the way, this seems to me works properly, as in [12/15] if skip
> creation of the LVDS1 encoder if it is operating in dual link mode.
> Without that, we would hit again the issue of matching on device
> nodes, as the same bridge would be attached twice, am I wrong?
> 
> I feel we've been there already :)
> https://lkml.org/lkml/2018/3/20/341
> 
> and it is not just an issue on matching on endpoints, it's that the
> drm bridge itself that would need to handle multiple attach/detaches..

Correct. Thanks to 12/15 we don't need this yet :-)

> > +	thc63->timings.dual_link = endpoint != NULL;
> > +	dev_dbg(thc63->dev, "operating in %s-link mode\n",
> > +		thc63->timings.dual_link ? "dual" : "single");
> > +
> >  	return 0;
> >  }
> >
> > @@ -188,6 +211,7 @@ static int thc63_probe(struct platform_device *pdev)
> >  	thc63->bridge.driver_private = thc63;
> >  	thc63->bridge.of_node = pdev->dev.of_node;
> >  	thc63->bridge.funcs = &thc63_bridge_func;
> > +	thc63->bridge.timings = &thc63->timings;
> >
> >  	drm_bridge_add(&thc63->bridge);
> >

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 10/15] drm: rcar-du: lvds: Set LVEN and LVRES bits together on D3

[Laurent Pinchart]

Hi Jacopo,

On Fri, Mar 08, 2019 at 05:25:12PM +0100, Jacopo Mondi wrote:
> On Thu, Mar 07, 2019 at 01:23:40AM +0200, Laurent Pinchart wrote:
> > On the D3 SoC the LVDS PHY must be enabled in the same register write
> > that enables the LVDS output. Skip writing the LVEN bit independently
> > on that platform, it will be set by the write that sets LVRES.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > index b1abe737dc05..5ac92ee15be0 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -475,9 +475,13 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> >  	}
> >
> >  	if (lvds->info->quirks & RCAR_LVDS_QUIRK_GEN3_LVEN) {
> > -		/* Turn on the LVDS PHY. */
> > +		/*
> > +		 * Turn on the LVDS PHY. On D3, the LVEN and LVRES bit must be
> > +		 * set at the same time, so don't write the register yet.
> > +		 */
> >  		lvdcr0 |= LVDCR0_LVEN;
> > -		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> > +		if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_PWD))
> 
> This is quite obscure, and works because D3 is the only SoC that has
> (quirks & RCAR_LVDS_QUIRK_GEN3_LVEN) and (!(quirks & RCAR_LVDS_QUIRK_PWD)).
> 
> I guess there are not many ways around this.

We could add a model field to the info structure, or another quirk, but
I'd rather not add yet another if it's not needed for now. I agree it's
not very nice though, it bothered me too when I wrote the code.

> > +			rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> 
> I have verified this against the latest v1.50 datasheet, and it
> matches what's reported in section 37.3.7 so please add my:
> Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> 
> I would like just to add that the same section prescribes a precise
> order in which LVDS0 and LVDS1 have to be configured when working with
> vertical stripe output. Is that enforced in this series?

It is, the companion is enabled before and disabled after the master for
this reason. My code initially violated the constraint, and the HDMI
output remained blank.

Note that figure 37.9 describes a sequence where register writes are
interleaved. As it's titled "The sample setting of the vertical stripe
output", I've considered it as a sample only.

> >  	}
> >
> >  	if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL)) {

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 11/15] drm: rcar-du: lvds: Add support for dual-link mode

[Laurent Pinchart]

Hi Jacopo,

On Fri, Mar 08, 2019 at 06:20:23PM +0100, Jacopo Mondi wrote:
> On Thu, Mar 07, 2019 at 01:23:41AM +0200, Laurent Pinchart wrote:
> > In dual-link mode the LVDS0 encoder transmits even-numbered pixels, and
> > sends odd-numbered pixels to the LVDS1 encoder for transmission on a
> > separate link.
> >
> > To implement support for this mode of operation, determine if the LVDS
> > connection operates in dual-link mode by querying the next device in the
> > pipeline, locate the companion encoder, and control it directly through
> > its bridge operations.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 104 ++++++++++++++++++++++++----
> >  drivers/gpu/drm/rcar-du/rcar_lvds.h |   5 ++
> >  2 files changed, 96 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > index 5ac92ee15be0..90d33514bb3e 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -66,6 +66,9 @@ struct rcar_lvds {
> >
> >  	struct drm_display_mode display_mode;
> >  	enum rcar_lvds_mode mode;
> > +
> > +	struct drm_bridge *companion;
> > +	bool dual_link;
> >  };
> >
> >  #define bridge_to_rcar_lvds(bridge) \
> > @@ -400,11 +403,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> >  {
> >  	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> >  	const struct drm_display_mode *mode = &lvds->display_mode;
> > -	/*
> > -	 * FIXME: We should really retrieve the CRTC through the state, but how
> > -	 * do we get a state pointer?
> > -	 */
> > -	struct drm_crtc *crtc = lvds->bridge.encoder->crtc;
> >  	u32 lvdhcr;
> >  	u32 lvdcr0;
> >  	int ret;
> > @@ -413,6 +411,10 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> >  	if (ret < 0)
> >  		return;
> >
> > +	/* Enable the companion LVDS encoder in dual-link mode. */
> > +	if (lvds->dual_link && lvds->companion)
> > +		lvds->companion->funcs->enable(lvds->companion);
> > +
> >  	/*
> >  	 * Hardcode the channels and control signals routing for now.
> >  	 *
> > @@ -435,17 +437,33 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> >  	rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
> >
> >  	if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
> > -		/* Disable dual-link mode. */
> > -		rcar_lvds_write(lvds, LVDSTRIPE, 0);
> > +		/*
> > +		 * Configure vertical stripe based on the mode of operation of
> > +		 * the connected device.
> > +		 */
> > +		rcar_lvds_write(lvds, LVDSTRIPE,
> > +				lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
> >  	}
> >
> > -	/* PLL clock configuration. */
> > -	lvds->info->pll_setup(lvds, mode->clock * 1000);
> > +	/*
> > +	 * PLL clock configuration on all instances but the companion in
> > +	 * dual-link mode.
> > +	 */
> > +	if (!lvds->dual_link || lvds->companion)
> > +		lvds->info->pll_setup(lvds, mode->clock * 1000);
> >
> >  	/* Set the LVDS mode and select the input. */
> >  	lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
> > -	if (drm_crtc_index(crtc) == 2)
> > -		lvdcr0 |= LVDCR0_DUSEL;
> > +
> > +	if (lvds->bridge.encoder) {
> > +		/*
> > +		 * FIXME: We should really retrieve the CRTC through the state,
> > +		 * but how do we get a state pointer?
> > +		 */
> > +		if (drm_crtc_index(lvds->bridge.encoder->crtc) == 2)
> > +			lvdcr0 |= LVDCR0_DUSEL;
> > +	}
> > +
> >  	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> >
> >  	/* Turn all the channels on. */
> > @@ -512,6 +530,10 @@ static void rcar_lvds_disable(struct drm_bridge *bridge)
> >  	rcar_lvds_write(lvds, LVDCR1, 0);
> >  	rcar_lvds_write(lvds, LVDPLLCR, 0);
> >
> > +	/* Disable the companion LVDS encoder in dual-link mode. */
> > +	if (lvds->dual_link && lvds->companion)
> > +		lvds->companion->funcs->disable(lvds->companion);
> > +
> >  	clk_disable_unprepare(lvds->clocks.mod);
> >  }
> >
> > @@ -628,10 +650,54 @@ static const struct drm_bridge_funcs rcar_lvds_bridge_ops = {
> >  	.mode_set = rcar_lvds_mode_set,
> >  };
> >
> > +bool rcar_lvds_dual_link(struct drm_bridge *bridge)
> > +{
> > +	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> > +
> > +	return lvds->dual_link;
> > +}
> > +EXPORT_SYMBOL_GPL(rcar_lvds_dual_link);
> > +
> >  /* -----------------------------------------------------------------------------
> >   * Probe & Remove
> >   */
> >
> > +static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds)
> > +{
> > +	const struct of_device_id *match;
> > +	struct device_node *companion;
> > +	struct device *dev = lvds->dev;
> > +	int ret = 0;
> > +
> > +	/* Locate the companion LVDS encoder for dual-link operation, if any. */
> > +	companion = of_parse_phandle(dev->of_node, "renesas,companion", 0);
> > +	if (!companion)
> > +		return -ENODEV;
> > +
> > +	/*
> > +	 * Sanity check: the companion encoder must have the same compatible
> > +	 * string.
> > +	 */
> > +	match = of_match_device(dev->driver->of_match_table, dev);
> > +	if (!of_device_is_compatible(companion, match->compatible)) {
> > +		ret = -ENODEV;
> > +		goto done;
> > +	}
> > +
> > +	lvds->companion = of_drm_find_bridge(companion);
> > +	if (!lvds->companion) {
> > +		ret = -EPROBE_DEFER;
> > +		goto done;
> > +	}
> > +
> > +	dev_dbg(dev, "Found companion encoder %pOF\n", companion);
> > +
> > +done:
> > +	of_node_put(companion);
> > +
> > +	return ret;
> > +}
> > +
> >  static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> >  {
> >  	struct device_node *local_output = NULL;
> > @@ -682,14 +748,26 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> >
> >  	if (is_bridge) {
> >  		lvds->next_bridge = of_drm_find_bridge(remote);
> > -		if (!lvds->next_bridge)
> > +		if (!lvds->next_bridge) {
> >  			ret = -EPROBE_DEFER;
> > +			goto done;
> > +		}
> > +
> > +		if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
> > +			lvds->dual_link = lvds->next_bridge->timings
> > +					? lvds->next_bridge->timings->dual_link
> > +					: false;
> 
> I wonder if, in all this patch, you could not use "lvds->companion" in
> place of "lvds->dual_link", and thus drop that from timings. This mean
> "renesas,companion" would only be used when operating in dual link
> mode, which might be less nice (and could actually called differently
> in that case).
> 
> Both the THC631024 and the rcar_du-lvds driver would decide
> independently if they operate in dual link mode or not (one counting
> its endpoints, the other inspecting the "renesas,companion" property).

I decided to specify the companion in DT regardless of which operating
mode is used, as it's a property of the LVDS encoder, not of the
operating mode. Furthermore, I can foresee setups where the mode would
be selected dynamically at runtime. Our development boards don't allow
that as they hardcode the mode using DIP switches, but nothing would
prevent the mode selection signals to be connected to GPIOs. I thus
think lvds->companion should point to the companion unconditionally, and
lvds->dual_link should select the operating mode. The dual_link field is
currently set at probe time as I have no need for dynamic configuration
of the mode and no mean of testing it, so I decided not to implement
dynamic switching yet.

> The only place where this might be tricky is the here above
> 
>  +	/*
>  +	 * PLL clock configuration on all instances but the companion in
>  +	 * dual-link mode.
>  +	 */
>  +	if (!lvds->dual_link || lvds->companion)
>  +		lvds->info->pll_setup(lvds, mode->clock * 1000);
> 
> but that would remove the need for the thc63 bridge to report its
> operating mode in timings...
> 
> >  	} else {
> >  		lvds->panel = of_drm_find_panel(remote);
> > -		if (IS_ERR(lvds->panel))
> > +		if (IS_ERR(lvds->panel)) {
> >  			ret = PTR_ERR(lvds->panel);
> > +			goto done;
> > +		}
> >  	}
> >
> > +	if (lvds->dual_link)
> 
> Note: if (!is_bridge) you would never set lvds->dual_link, so this
> should be moved inside the here above "if (is_bridge)"

I don't yet, but nothing prevents a panel from operating in dual mode,
even if not implemented yet. That's why I've move this check out of the
bridge/panel conditional code.

> > +		ret = rcar_lvds_parse_dt_companion(lvds);
> > +
> >  done:
> >  	of_node_put(local_output);
> >  	of_node_put(remote_input);
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.h b/drivers/gpu/drm/rcar-du/rcar_lvds.h
> > index a709cae1bc32..222ec0e60785 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.h
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.h
> > @@ -15,6 +15,7 @@ struct drm_bridge;
> >  #if IS_ENABLED(CONFIG_DRM_RCAR_LVDS)
> >  int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq);
> >  void rcar_lvds_clk_disable(struct drm_bridge *bridge);
> > +bool rcar_lvds_dual_link(struct drm_bridge *bridge);
> >  #else
> >  static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge,
> >  				       unsigned long freq)
> > @@ -22,6 +23,10 @@ static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge,
> >  	return -ENOSYS;
> >  }
> >  static inline void rcar_lvds_clk_disable(struct drm_bridge *bridge) { }
> > +static inline bool rcar_lvds_dual_link(struct drm_bridge *bridge)
> > +{
> > +	return false;
> > +}
> >  #endif /* CONFIG_DRM_RCAR_LVDS */
> >
> >  #endif /* __RCAR_LVDS_H__ */

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 11/15] drm: rcar-du: lvds: Add support for dual-link mode

[Jacopo Mondi]

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

Hi Laurent,

On Fri, Mar 08, 2019 at 08:12:39PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Fri, Mar 08, 2019 at 06:20:23PM +0100, Jacopo Mondi wrote:
> > On Thu, Mar 07, 2019 at 01:23:41AM +0200, Laurent Pinchart wrote:
> > > In dual-link mode the LVDS0 encoder transmits even-numbered pixels, and
> > > sends odd-numbered pixels to the LVDS1 encoder for transmission on a
> > > separate link.
> > >
> > > To implement support for this mode of operation, determine if the LVDS
> > > connection operates in dual-link mode by querying the next device in the
> > > pipeline, locate the companion encoder, and control it directly through
> > > its bridge operations.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 104 ++++++++++++++++++++++++----
> > >  drivers/gpu/drm/rcar-du/rcar_lvds.h |   5 ++
> > >  2 files changed, 96 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > index 5ac92ee15be0..90d33514bb3e 100644
> > > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > @@ -66,6 +66,9 @@ struct rcar_lvds {
> > >
> > >  	struct drm_display_mode display_mode;
> > >  	enum rcar_lvds_mode mode;
> > > +
> > > +	struct drm_bridge *companion;
> > > +	bool dual_link;
> > >  };
> > >
> > >  #define bridge_to_rcar_lvds(bridge) \
> > > @@ -400,11 +403,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> > >  {
> > >  	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> > >  	const struct drm_display_mode *mode = &lvds->display_mode;
> > > -	/*
> > > -	 * FIXME: We should really retrieve the CRTC through the state, but how
> > > -	 * do we get a state pointer?
> > > -	 */
> > > -	struct drm_crtc *crtc = lvds->bridge.encoder->crtc;
> > >  	u32 lvdhcr;
> > >  	u32 lvdcr0;
> > >  	int ret;
> > > @@ -413,6 +411,10 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> > >  	if (ret < 0)
> > >  		return;
> > >
> > > +	/* Enable the companion LVDS encoder in dual-link mode. */
> > > +	if (lvds->dual_link && lvds->companion)
> > > +		lvds->companion->funcs->enable(lvds->companion);
> > > +
> > >  	/*
> > >  	 * Hardcode the channels and control signals routing for now.
> > >  	 *
> > > @@ -435,17 +437,33 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> > >  	rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
> > >
> > >  	if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
> > > -		/* Disable dual-link mode. */
> > > -		rcar_lvds_write(lvds, LVDSTRIPE, 0);
> > > +		/*
> > > +		 * Configure vertical stripe based on the mode of operation of
> > > +		 * the connected device.
> > > +		 */
> > > +		rcar_lvds_write(lvds, LVDSTRIPE,
> > > +				lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
> > >  	}
> > >
> > > -	/* PLL clock configuration. */
> > > -	lvds->info->pll_setup(lvds, mode->clock * 1000);
> > > +	/*
> > > +	 * PLL clock configuration on all instances but the companion in
> > > +	 * dual-link mode.
> > > +	 */
> > > +	if (!lvds->dual_link || lvds->companion)
> > > +		lvds->info->pll_setup(lvds, mode->clock * 1000);
> > >
> > >  	/* Set the LVDS mode and select the input. */
> > >  	lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
> > > -	if (drm_crtc_index(crtc) == 2)
> > > -		lvdcr0 |= LVDCR0_DUSEL;
> > > +
> > > +	if (lvds->bridge.encoder) {
> > > +		/*
> > > +		 * FIXME: We should really retrieve the CRTC through the state,
> > > +		 * but how do we get a state pointer?
> > > +		 */
> > > +		if (drm_crtc_index(lvds->bridge.encoder->crtc) == 2)
> > > +			lvdcr0 |= LVDCR0_DUSEL;
> > > +	}
> > > +
> > >  	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> > >
> > >  	/* Turn all the channels on. */
> > > @@ -512,6 +530,10 @@ static void rcar_lvds_disable(struct drm_bridge *bridge)
> > >  	rcar_lvds_write(lvds, LVDCR1, 0);
> > >  	rcar_lvds_write(lvds, LVDPLLCR, 0);
> > >
> > > +	/* Disable the companion LVDS encoder in dual-link mode. */
> > > +	if (lvds->dual_link && lvds->companion)
> > > +		lvds->companion->funcs->disable(lvds->companion);
> > > +
> > >  	clk_disable_unprepare(lvds->clocks.mod);
> > >  }
> > >
> > > @@ -628,10 +650,54 @@ static const struct drm_bridge_funcs rcar_lvds_bridge_ops = {
> > >  	.mode_set = rcar_lvds_mode_set,
> > >  };
> > >
> > > +bool rcar_lvds_dual_link(struct drm_bridge *bridge)
> > > +{
> > > +	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> > > +
> > > +	return lvds->dual_link;
> > > +}
> > > +EXPORT_SYMBOL_GPL(rcar_lvds_dual_link);
> > > +
> > >  /* -----------------------------------------------------------------------------
> > >   * Probe & Remove
> > >   */
> > >
> > > +static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds)
> > > +{
> > > +	const struct of_device_id *match;
> > > +	struct device_node *companion;
> > > +	struct device *dev = lvds->dev;
> > > +	int ret = 0;
> > > +
> > > +	/* Locate the companion LVDS encoder for dual-link operation, if any. */
> > > +	companion = of_parse_phandle(dev->of_node, "renesas,companion", 0);
> > > +	if (!companion)
> > > +		return -ENODEV;
> > > +
> > > +	/*
> > > +	 * Sanity check: the companion encoder must have the same compatible
> > > +	 * string.
> > > +	 */
> > > +	match = of_match_device(dev->driver->of_match_table, dev);
> > > +	if (!of_device_is_compatible(companion, match->compatible)) {
> > > +		ret = -ENODEV;
> > > +		goto done;
> > > +	}
> > > +
> > > +	lvds->companion = of_drm_find_bridge(companion);
> > > +	if (!lvds->companion) {
> > > +		ret = -EPROBE_DEFER;
> > > +		goto done;
> > > +	}
> > > +
> > > +	dev_dbg(dev, "Found companion encoder %pOF\n", companion);
> > > +
> > > +done:
> > > +	of_node_put(companion);
> > > +
> > > +	return ret;
> > > +}
> > > +
> > >  static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> > >  {
> > >  	struct device_node *local_output = NULL;
> > > @@ -682,14 +748,26 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> > >
> > >  	if (is_bridge) {
> > >  		lvds->next_bridge = of_drm_find_bridge(remote);
> > > -		if (!lvds->next_bridge)
> > > +		if (!lvds->next_bridge) {
> > >  			ret = -EPROBE_DEFER;
> > > +			goto done;
> > > +		}
> > > +
> > > +		if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
> > > +			lvds->dual_link = lvds->next_bridge->timings
> > > +					? lvds->next_bridge->timings->dual_link
> > > +					: false;
> >
> > I wonder if, in all this patch, you could not use "lvds->companion" in
> > place of "lvds->dual_link", and thus drop that from timings. This mean
> > "renesas,companion" would only be used when operating in dual link
> > mode, which might be less nice (and could actually called differently
> > in that case).
> >
> > Both the THC631024 and the rcar_du-lvds driver would decide
> > independently if they operate in dual link mode or not (one counting
> > its endpoints, the other inspecting the "renesas,companion" property).
>
> I decided to specify the companion in DT regardless of which operating
> mode is used, as it's a property of the LVDS encoder, not of the
> operating mode. Furthermore, I can foresee setups where the mode would
> be selected dynamically at runtime. Our development boards don't allow
> that as they hardcode the mode using DIP switches, but nothing would
> prevent the mode selection signals to be connected to GPIOs. I thus
> think lvds->companion should point to the companion unconditionally, and
> lvds->dual_link should select the operating mode. The dual_link field is
> currently set at probe time as I have no need for dynamic configuration
> of the mode and no mean of testing it, so I decided not to implement
> dynamic switching yet.
>

Fine, I tried thinking up a bit if the "renesas,companion" property
could have been made an endpoint property, to be specified in both
the rcar-lvds endpoints and in the DU endpoints, so that the
rcar_du_encoder does not need to pick into the lvds-encoder to know
if it is operating in dual link mode or not and skip creation of the
encoder associated to LVDS1 in such a case.

Furthermore, we could make a "vstripe-even" "vstripe-odd" endpoint
properties, that would allow you to control the ST_SWAP field of
LVDSTRIPE register, and create another endpoint property that contains
the phandle to the companion, like "dual-link-companion" or
"dual-link-slave". Those properties would need to be specified in both
DU and LVDS endpoints though, which might be clunky, but that would
possibly save a few cross-driver calls.

Anyway, just putting a few more options on the table, if you have
already considered those feel free to stick to this implementation...

> > The only place where this might be tricky is the here above
> >
> >  +	/*
> >  +	 * PLL clock configuration on all instances but the companion in
> >  +	 * dual-link mode.
> >  +	 */
> >  +	if (!lvds->dual_link || lvds->companion)
> >  +		lvds->info->pll_setup(lvds, mode->clock * 1000);
> >
> > but that would remove the need for the thc63 bridge to report its
> > operating mode in timings...
> >
> > >  	} else {
> > >  		lvds->panel = of_drm_find_panel(remote);
> > > -		if (IS_ERR(lvds->panel))
> > > +		if (IS_ERR(lvds->panel)) {
> > >  			ret = PTR_ERR(lvds->panel);
> > > +			goto done;
> > > +		}
> > >  	}
> > >
> > > +	if (lvds->dual_link)
> >
> > Note: if (!is_bridge) you would never set lvds->dual_link, so this
> > should be moved inside the here above "if (is_bridge)"
>
> I don't yet, but nothing prevents a panel from operating in dual mode,
> even if not implemented yet. That's why I've move this check out of the
> bridge/panel conditional code.

I see, still right now is of no use. It's fine though, really minor
stuff.

Thanks
   j

>
> > > +		ret = rcar_lvds_parse_dt_companion(lvds);
> > > +
> > >  done:
> > >  	of_node_put(local_output);
> > >  	of_node_put(remote_input);
> > > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.h b/drivers/gpu/drm/rcar-du/rcar_lvds.h
> > > index a709cae1bc32..222ec0e60785 100644
> > > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.h
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.h
> > > @@ -15,6 +15,7 @@ struct drm_bridge;
> > >  #if IS_ENABLED(CONFIG_DRM_RCAR_LVDS)
> > >  int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq);
> > >  void rcar_lvds_clk_disable(struct drm_bridge *bridge);
> > > +bool rcar_lvds_dual_link(struct drm_bridge *bridge);
> > >  #else
> > >  static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge,
> > >  				       unsigned long freq)
> > > @@ -22,6 +23,10 @@ static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge,
> > >  	return -ENOSYS;
> > >  }
> > >  static inline void rcar_lvds_clk_disable(struct drm_bridge *bridge) { }
> > > +static inline bool rcar_lvds_dual_link(struct drm_bridge *bridge)
> > > +{
> > > +	return false;
> > > +}
> > >  #endif /* CONFIG_DRM_RCAR_LVDS */
> > >
> > >  #endif /* __RCAR_LVDS_H__ */
>
> --
> Regards,
>
> Laurent Pinchart

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

Re: [PATCH/RFC 05/15] dt-bindings: display: bridge: thc63lvd1024: Document dual-link operation

[Jacopo Mondi]

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

HI Laurent,

On Fri, Mar 08, 2019 at 07:57:39PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Fri, Mar 08, 2019 at 05:49:25PM +0100, Jacopo Mondi wrote:
> > On Thu, Mar 07, 2019 at 01:23:35AM +0200, Laurent Pinchart wrote:
> > > The THC63LVD1024 LVDS decoder can operate in two modes, single-link or
> > > dual-link. In dual-link mode both input ports are used to carry even-
> > > and odd-numbered pixels separately. Document this in the DT bindings,
> > > along with the related rules governing port and usage.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > >  .../bindings/display/bridge/thine,thc63lvd1024.txt         | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > > index 37f0c04d5a28..4ff6eb9bbc19 100644
> > > --- a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > > +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > > @@ -28,6 +28,13 @@ Optional video port nodes:
> > >  - port@1: Second LVDS input port
> > >  - port@3: Second digital CMOS/TTL parallel output
> > >
> > > +The device can operate in single-link mode or dual-link mode. In single-link
> > > +mode, all pixels are received on port@0, and port@1 shall not contain any
> > > +endpoint. In dual-link mode, even-numbered pixels are received on port@0 and
> > > +odd-numbered pixels on port@1, and both port@0 and port@1 shall contain
> > > +endpoints.
> >
> > You know, I'm not sure this is helpful, as if we have to go and
> > describe what the chip supports, a paragraph for dual ouput mode would
> > be required as well. The bindings already document that the chip
> > supports single/dual input/output modes, maybe you can just add rules
> > that prescribes how to populate the endpoints, for both input and
> > output modes?
>
> That's what I was trying to do :-) How else would you like to see it
> described ?
>

I wonder if it won't be enough to expand the Optional endpoint
properties description as in:

 - port@1: Second LVDS input port, to be used for dual-input mode
 - port@3: Second digital CMOS/TTL parallel output, to be used for
   dual-output mode.

If you prefer providing more context, it's fine what you had, just
keep in mind there's also dual-output mode and not just the dual-input
one.

Up to you, feel free to add my
Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
if relevant.

Thanks
  j

> > > +
> > > +
> >
> > You can drop this empty line.
> >
> > >  Example:
> > >  --------
> > >
>
> --
> Regards,
>
> Laurent Pinchart

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

Re: [PATCH/RFC 06/15] drm: bridge: thc63: Report input bus mode through bridge timings

[Jacopo Mondi]

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

Hi Laurent

On Fri, Mar 08, 2019 at 08:00:28PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Fri, Mar 08, 2019 at 06:32:59PM +0100, Jacopo Mondi wrote:
> > On Thu, Mar 07, 2019 at 01:23:36AM +0200, Laurent Pinchart wrote:
> > > Set a drm_bridge_timings in the drm_bridge, and use it to report the
> > > input bus mode (single-link or dual-link). The other fields of the
> > > timings structure are kept to 0 as they do not apply to LVDS buses.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > >  drivers/gpu/drm/bridge/thc63lvd1024.c | 46 ++++++++++++++++++++-------
> > >  1 file changed, 35 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c
> > > index b083a740565c..206b0af5e154 100644
> > > --- a/drivers/gpu/drm/bridge/thc63lvd1024.c
> > > +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
> > > @@ -31,6 +31,8 @@ struct thc63_dev {
> > >
> > >  	struct drm_bridge bridge;
> > >  	struct drm_bridge *next;
> > > +
> > > +	struct drm_bridge_timings timings;
> > >  };
> > >
> > >  static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
> > > @@ -48,15 +50,28 @@ static int thc63_attach(struct drm_bridge *bridge)
> > >  static enum drm_mode_status thc63_mode_valid(struct drm_bridge *bridge,
> > >  					const struct drm_display_mode *mode)
> > >  {
> > > +	struct thc63_dev *thc63 = to_thc63(bridge);
> > > +	unsigned int min_freq;
> > > +	unsigned int max_freq;
> > > +
> > >  	/*
> > > -	 * The THC63LVD1024 clock frequency range is 8 to 135 MHz in single-in
> > > -	 * mode. Note that the limits are different in dual-in, single-out mode,
> > > -	 * and will need to be adjusted accordingly.
> > > +	 * The THC63LVD1024 pixel rate range is 8 to 135 MHz in all modes but
> > > +	 * dual-in, single-out where it is 40 to 150 MHz. As dual-in, dual-out
> > > +	 * isn't supported by the driver yet, simply derive the limits from the
> > > +	 * input mode.
> > >  	 */
> > > -	if (mode->clock < 8000)
> > > +	if (thc63->timings.dual_link) {
> > > +		min_freq = 40000;
> > > +		max_freq = 150000;
> > > +	} else {
> > > +		min_freq = 8000;
> > > +		max_freq = 135000;
> > > +	}
> > > +
> > > +	if (mode->clock < min_freq)
> > >  		return MODE_CLOCK_LOW;
> > >
> > > -	if (mode->clock > 135000)
> > > +	if (mode->clock > max_freq)
> > >  		return MODE_CLOCK_HIGH;
> > >
> > >  	return MODE_OK;
> > > @@ -101,19 +116,19 @@ static const struct drm_bridge_funcs thc63_bridge_func = {
> > >
> > >  static int thc63_parse_dt(struct thc63_dev *thc63)
> > >  {
> > > -	struct device_node *thc63_out;
> > > +	struct device_node *endpoint;
> > >  	struct device_node *remote;
> > >
> > > -	thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> > > -						  THC63_RGB_OUT0, -1);
> > > -	if (!thc63_out) {
> > > +	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> > > +						 THC63_RGB_OUT0, -1);
> > > +	if (!endpoint) {
> > >  		dev_err(thc63->dev, "Missing endpoint in port@%u\n",
> > >  			THC63_RGB_OUT0);
> > >  		return -ENODEV;
> > >  	}
> > >
> > > -	remote = of_graph_get_remote_port_parent(thc63_out);
> > > -	of_node_put(thc63_out);
> > > +	remote = of_graph_get_remote_port_parent(endpoint);
> > > +	of_node_put(endpoint);
> > >  	if (!remote) {
> > >  		dev_err(thc63->dev, "Endpoint in port@%u unconnected\n",
> > >  			THC63_RGB_OUT0);
> > > @@ -132,6 +147,14 @@ static int thc63_parse_dt(struct thc63_dev *thc63)
> > >  	if (!thc63->next)
> > >  		return -EPROBE_DEFER;
> > >
> > > +	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> > > +						 THC63_LVDS_IN1, -1);
> > > +	of_node_put(endpoint);
> > > +
> >
> > Should you check if endpoint is enabled?
>
> Can endpoints be disabled ?

I meant the remote port parent, sorry about that.

Thanks
  j

>
> > By the way, this seems to me works properly, as in [12/15] if skip
> > creation of the LVDS1 encoder if it is operating in dual link mode.
> > Without that, we would hit again the issue of matching on device
> > nodes, as the same bridge would be attached twice, am I wrong?
> >
> > I feel we've been there already :)
> > https://lkml.org/lkml/2018/3/20/341
> >
> > and it is not just an issue on matching on endpoints, it's that the
> > drm bridge itself that would need to handle multiple attach/detaches..
>
> Correct. Thanks to 12/15 we don't need this yet :-)
>
> > > +	thc63->timings.dual_link = endpoint != NULL;
> > > +	dev_dbg(thc63->dev, "operating in %s-link mode\n",
> > > +		thc63->timings.dual_link ? "dual" : "single");
> > > +
> > >  	return 0;
> > >  }
> > >
> > > @@ -188,6 +211,7 @@ static int thc63_probe(struct platform_device *pdev)
> > >  	thc63->bridge.driver_private = thc63;
> > >  	thc63->bridge.of_node = pdev->dev.of_node;
> > >  	thc63->bridge.funcs = &thc63_bridge_func;
> > > +	thc63->bridge.timings = &thc63->timings;
> > >
> > >  	drm_bridge_add(&thc63->bridge);
> > >
>
> --
> Regards,
>
> Laurent Pinchart

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

Re: [PATCH/RFC 11/15] drm: rcar-du: lvds: Add support for dual-link mode

[Laurent Pinchart]

Hi Jacopo,

On Sat, Mar 09, 2019 at 12:11:32PM +0100, Jacopo Mondi wrote:
> On Fri, Mar 08, 2019 at 08:12:39PM +0200, Laurent Pinchart wrote:
> > On Fri, Mar 08, 2019 at 06:20:23PM +0100, Jacopo Mondi wrote:
> >> On Thu, Mar 07, 2019 at 01:23:41AM +0200, Laurent Pinchart wrote:
> >>> In dual-link mode the LVDS0 encoder transmits even-numbered pixels, and
> >>> sends odd-numbered pixels to the LVDS1 encoder for transmission on a
> >>> separate link.
> >>>
> >>> To implement support for this mode of operation, determine if the LVDS
> >>> connection operates in dual-link mode by querying the next device in the
> >>> pipeline, locate the companion encoder, and control it directly through
> >>> its bridge operations.
> >>>
> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >>> ---
> >>>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 104 ++++++++++++++++++++++++----
> >>>  drivers/gpu/drm/rcar-du/rcar_lvds.h |   5 ++
> >>>  2 files changed, 96 insertions(+), 13 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> >>> index 5ac92ee15be0..90d33514bb3e 100644
> >>> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> >>> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> >>> @@ -66,6 +66,9 @@ struct rcar_lvds {
> >>>
> >>>  	struct drm_display_mode display_mode;
> >>>  	enum rcar_lvds_mode mode;
> >>> +
> >>> +	struct drm_bridge *companion;
> >>> +	bool dual_link;
> >>>  };
> >>>
> >>>  #define bridge_to_rcar_lvds(bridge) \
> >>> @@ -400,11 +403,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> >>>  {
> >>>  	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> >>>  	const struct drm_display_mode *mode = &lvds->display_mode;
> >>> -	/*
> >>> -	 * FIXME: We should really retrieve the CRTC through the state, but how
> >>> -	 * do we get a state pointer?
> >>> -	 */
> >>> -	struct drm_crtc *crtc = lvds->bridge.encoder->crtc;
> >>>  	u32 lvdhcr;
> >>>  	u32 lvdcr0;
> >>>  	int ret;
> >>> @@ -413,6 +411,10 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> >>>  	if (ret < 0)
> >>>  		return;
> >>>
> >>> +	/* Enable the companion LVDS encoder in dual-link mode. */
> >>> +	if (lvds->dual_link && lvds->companion)
> >>> +		lvds->companion->funcs->enable(lvds->companion);
> >>> +
> >>>  	/*
> >>>  	 * Hardcode the channels and control signals routing for now.
> >>>  	 *
> >>> @@ -435,17 +437,33 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> >>>  	rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
> >>>
> >>>  	if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
> >>> -		/* Disable dual-link mode. */
> >>> -		rcar_lvds_write(lvds, LVDSTRIPE, 0);
> >>> +		/*
> >>> +		 * Configure vertical stripe based on the mode of operation of
> >>> +		 * the connected device.
> >>> +		 */
> >>> +		rcar_lvds_write(lvds, LVDSTRIPE,
> >>> +				lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
> >>>  	}
> >>>
> >>> -	/* PLL clock configuration. */
> >>> -	lvds->info->pll_setup(lvds, mode->clock * 1000);
> >>> +	/*
> >>> +	 * PLL clock configuration on all instances but the companion in
> >>> +	 * dual-link mode.
> >>> +	 */
> >>> +	if (!lvds->dual_link || lvds->companion)
> >>> +		lvds->info->pll_setup(lvds, mode->clock * 1000);
> >>>
> >>>  	/* Set the LVDS mode and select the input. */
> >>>  	lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
> >>> -	if (drm_crtc_index(crtc) == 2)
> >>> -		lvdcr0 |= LVDCR0_DUSEL;
> >>> +
> >>> +	if (lvds->bridge.encoder) {
> >>> +		/*
> >>> +		 * FIXME: We should really retrieve the CRTC through the state,
> >>> +		 * but how do we get a state pointer?
> >>> +		 */
> >>> +		if (drm_crtc_index(lvds->bridge.encoder->crtc) == 2)
> >>> +			lvdcr0 |= LVDCR0_DUSEL;
> >>> +	}
> >>> +
> >>>  	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> >>>
> >>>  	/* Turn all the channels on. */
> >>> @@ -512,6 +530,10 @@ static void rcar_lvds_disable(struct drm_bridge *bridge)
> >>>  	rcar_lvds_write(lvds, LVDCR1, 0);
> >>>  	rcar_lvds_write(lvds, LVDPLLCR, 0);
> >>>
> >>> +	/* Disable the companion LVDS encoder in dual-link mode. */
> >>> +	if (lvds->dual_link && lvds->companion)
> >>> +		lvds->companion->funcs->disable(lvds->companion);
> >>> +
> >>>  	clk_disable_unprepare(lvds->clocks.mod);
> >>>  }
> >>>
> >>> @@ -628,10 +650,54 @@ static const struct drm_bridge_funcs rcar_lvds_bridge_ops = {
> >>>  	.mode_set = rcar_lvds_mode_set,
> >>>  };
> >>>
> >>> +bool rcar_lvds_dual_link(struct drm_bridge *bridge)
> >>> +{
> >>> +	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> >>> +
> >>> +	return lvds->dual_link;
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(rcar_lvds_dual_link);
> >>> +
> >>>  /* -----------------------------------------------------------------------------
> >>>   * Probe & Remove
> >>>   */
> >>>
> >>> +static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds)
> >>> +{
> >>> +	const struct of_device_id *match;
> >>> +	struct device_node *companion;
> >>> +	struct device *dev = lvds->dev;
> >>> +	int ret = 0;
> >>> +
> >>> +	/* Locate the companion LVDS encoder for dual-link operation, if any. */
> >>> +	companion = of_parse_phandle(dev->of_node, "renesas,companion", 0);
> >>> +	if (!companion)
> >>> +		return -ENODEV;
> >>> +
> >>> +	/*
> >>> +	 * Sanity check: the companion encoder must have the same compatible
> >>> +	 * string.
> >>> +	 */
> >>> +	match = of_match_device(dev->driver->of_match_table, dev);
> >>> +	if (!of_device_is_compatible(companion, match->compatible)) {
> >>> +		ret = -ENODEV;
> >>> +		goto done;
> >>> +	}
> >>> +
> >>> +	lvds->companion = of_drm_find_bridge(companion);
> >>> +	if (!lvds->companion) {
> >>> +		ret = -EPROBE_DEFER;
> >>> +		goto done;
> >>> +	}
> >>> +
> >>> +	dev_dbg(dev, "Found companion encoder %pOF\n", companion);
> >>> +
> >>> +done:
> >>> +	of_node_put(companion);
> >>> +
> >>> +	return ret;
> >>> +}
> >>> +
> >>>  static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> >>>  {
> >>>  	struct device_node *local_output = NULL;
> >>> @@ -682,14 +748,26 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> >>>
> >>>  	if (is_bridge) {
> >>>  		lvds->next_bridge = of_drm_find_bridge(remote);
> >>> -		if (!lvds->next_bridge)
> >>> +		if (!lvds->next_bridge) {
> >>>  			ret = -EPROBE_DEFER;
> >>> +			goto done;
> >>> +		}
> >>> +
> >>> +		if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
> >>> +			lvds->dual_link = lvds->next_bridge->timings
> >>> +					? lvds->next_bridge->timings->dual_link
> >>> +					: false;
> >>
> >> I wonder if, in all this patch, you could not use "lvds->companion" in
> >> place of "lvds->dual_link", and thus drop that from timings. This mean
> >> "renesas,companion" would only be used when operating in dual link
> >> mode, which might be less nice (and could actually called differently
> >> in that case).
> >>
> >> Both the THC631024 and the rcar_du-lvds driver would decide
> >> independently if they operate in dual link mode or not (one counting
> >> its endpoints, the other inspecting the "renesas,companion" property).
> >
> > I decided to specify the companion in DT regardless of which operating
> > mode is used, as it's a property of the LVDS encoder, not of the
> > operating mode. Furthermore, I can foresee setups where the mode would
> > be selected dynamically at runtime. Our development boards don't allow
> > that as they hardcode the mode using DIP switches, but nothing would
> > prevent the mode selection signals to be connected to GPIOs. I thus
> > think lvds->companion should point to the companion unconditionally, and
> > lvds->dual_link should select the operating mode. The dual_link field is
> > currently set at probe time as I have no need for dynamic configuration
> > of the mode and no mean of testing it, so I decided not to implement
> > dynamic switching yet.
> 
> Fine, I tried thinking up a bit if the "renesas,companion" property
> could have been made an endpoint property, to be specified in both
> the rcar-lvds endpoints and in the DU endpoints, so that the
> rcar_du_encoder does not need to pick into the lvds-encoder to know
> if it is operating in dual link mode or not and skip creation of the
> encoder associated to LVDS1 in such a case.
> 
> Furthermore, we could make a "vstripe-even" "vstripe-odd" endpoint
> properties, that would allow you to control the ST_SWAP field of
> LVDSTRIPE register, and create another endpoint property that contains
> the phandle to the companion, like "dual-link-companion" or
> "dual-link-slave". Those properties would need to be specified in both
> DU and LVDS endpoints though, which might be clunky, but that would
> possibly save a few cross-driver calls.

Regarding the stripe odd/even control, I think this should be handled
dynamically without involving DT. We will need an API for bridges to
report what order(s) they support, and allow the remote component to be
configured accordingly. A DT property isn't needed for this.

Regarding the companion, it's not a generic concept. It happens that
Renesas decided to implement dual-link support by configuring one of the
two LVDS encoders as a master and the other one as a slave, but there
could be other options. That's why I made this a Renesas-specific
property. I believe it is placed correctly in the LVDS encoder DT node,
and not in the port or endpoint, as it's a property of the LVDS encoder
itself.

Other models are possible though, I could have added a third port to the
LVDS encoders to model this connection, but as ports don't indicate a
direction, I would have needed an extra property anyway to differentiate
between the master and the slave.

> Anyway, just putting a few more options on the table, if you have
> already considered those feel free to stick to this implementation...
> 
> >> The only place where this might be tricky is the here above
> >>
> >>  +	/*
> >>  +	 * PLL clock configuration on all instances but the companion in
> >>  +	 * dual-link mode.
> >>  +	 */
> >>  +	if (!lvds->dual_link || lvds->companion)
> >>  +		lvds->info->pll_setup(lvds, mode->clock * 1000);
> >>
> >> but that would remove the need for the thc63 bridge to report its
> >> operating mode in timings...
> >>
> >>>  	} else {
> >>>  		lvds->panel = of_drm_find_panel(remote);
> >>> -		if (IS_ERR(lvds->panel))
> >>> +		if (IS_ERR(lvds->panel)) {
> >>>  			ret = PTR_ERR(lvds->panel);
> >>> +			goto done;
> >>> +		}
> >>>  	}
> >>>
> >>> +	if (lvds->dual_link)
> >>
> >> Note: if (!is_bridge) you would never set lvds->dual_link, so this
> >> should be moved inside the here above "if (is_bridge)"
> >
> > I don't yet, but nothing prevents a panel from operating in dual mode,
> > even if not implemented yet. That's why I've move this check out of the
> > bridge/panel conditional code.
> 
> I see, still right now is of no use. It's fine though, really minor
> stuff.
> 
> >>> +		ret = rcar_lvds_parse_dt_companion(lvds);
> >>> +
> >>>  done:
> >>>  	of_node_put(local_output);
> >>>  	of_node_put(remote_input);
> >>> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.h b/drivers/gpu/drm/rcar-du/rcar_lvds.h
> >>> index a709cae1bc32..222ec0e60785 100644
> >>> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.h
> >>> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.h
> >>> @@ -15,6 +15,7 @@ struct drm_bridge;
> >>>  #if IS_ENABLED(CONFIG_DRM_RCAR_LVDS)
> >>>  int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq);
> >>>  void rcar_lvds_clk_disable(struct drm_bridge *bridge);
> >>> +bool rcar_lvds_dual_link(struct drm_bridge *bridge);
> >>>  #else
> >>>  static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge,
> >>>  				       unsigned long freq)
> >>> @@ -22,6 +23,10 @@ static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge,
> >>>  	return -ENOSYS;
> >>>  }
> >>>  static inline void rcar_lvds_clk_disable(struct drm_bridge *bridge) { }
> >>> +static inline bool rcar_lvds_dual_link(struct drm_bridge *bridge)
> >>> +{
> >>> +	return false;
> >>> +}
> >>>  #endif /* CONFIG_DRM_RCAR_LVDS */
> >>>
> >>>  #endif /* __RCAR_LVDS_H__ */

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 06/15] drm: bridge: thc63: Report input bus mode through bridge timings

[Laurent Pinchart]

Hi Jacopo,

On Sat, Mar 09, 2019 at 12:24:08PM +0100, Jacopo Mondi wrote:
> On Fri, Mar 08, 2019 at 08:00:28PM +0200, Laurent Pinchart wrote:
> > On Fri, Mar 08, 2019 at 06:32:59PM +0100, Jacopo Mondi wrote:
> >> On Thu, Mar 07, 2019 at 01:23:36AM +0200, Laurent Pinchart wrote:
> >>> Set a drm_bridge_timings in the drm_bridge, and use it to report the
> >>> input bus mode (single-link or dual-link). The other fields of the
> >>> timings structure are kept to 0 as they do not apply to LVDS buses.
> >>>
> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >>> ---
> >>>  drivers/gpu/drm/bridge/thc63lvd1024.c | 46 ++++++++++++++++++++-------
> >>>  1 file changed, 35 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c
> >>> index b083a740565c..206b0af5e154 100644
> >>> --- a/drivers/gpu/drm/bridge/thc63lvd1024.c
> >>> +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
> >>> @@ -31,6 +31,8 @@ struct thc63_dev {
> >>>
> >>>  	struct drm_bridge bridge;
> >>>  	struct drm_bridge *next;
> >>> +
> >>> +	struct drm_bridge_timings timings;
> >>>  };
> >>>
> >>>  static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
> >>> @@ -48,15 +50,28 @@ static int thc63_attach(struct drm_bridge *bridge)
> >>>  static enum drm_mode_status thc63_mode_valid(struct drm_bridge *bridge,
> >>>  					const struct drm_display_mode *mode)
> >>>  {
> >>> +	struct thc63_dev *thc63 = to_thc63(bridge);
> >>> +	unsigned int min_freq;
> >>> +	unsigned int max_freq;
> >>> +
> >>>  	/*
> >>> -	 * The THC63LVD1024 clock frequency range is 8 to 135 MHz in single-in
> >>> -	 * mode. Note that the limits are different in dual-in, single-out mode,
> >>> -	 * and will need to be adjusted accordingly.
> >>> +	 * The THC63LVD1024 pixel rate range is 8 to 135 MHz in all modes but
> >>> +	 * dual-in, single-out where it is 40 to 150 MHz. As dual-in, dual-out
> >>> +	 * isn't supported by the driver yet, simply derive the limits from the
> >>> +	 * input mode.
> >>>  	 */
> >>> -	if (mode->clock < 8000)
> >>> +	if (thc63->timings.dual_link) {
> >>> +		min_freq = 40000;
> >>> +		max_freq = 150000;
> >>> +	} else {
> >>> +		min_freq = 8000;
> >>> +		max_freq = 135000;
> >>> +	}
> >>> +
> >>> +	if (mode->clock < min_freq)
> >>>  		return MODE_CLOCK_LOW;
> >>>
> >>> -	if (mode->clock > 135000)
> >>> +	if (mode->clock > max_freq)
> >>>  		return MODE_CLOCK_HIGH;
> >>>
> >>>  	return MODE_OK;
> >>> @@ -101,19 +116,19 @@ static const struct drm_bridge_funcs thc63_bridge_func = {
> >>>
> >>>  static int thc63_parse_dt(struct thc63_dev *thc63)
> >>>  {
> >>> -	struct device_node *thc63_out;
> >>> +	struct device_node *endpoint;
> >>>  	struct device_node *remote;
> >>>
> >>> -	thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> >>> -						  THC63_RGB_OUT0, -1);
> >>> -	if (!thc63_out) {
> >>> +	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> >>> +						 THC63_RGB_OUT0, -1);
> >>> +	if (!endpoint) {
> >>>  		dev_err(thc63->dev, "Missing endpoint in port@%u\n",
> >>>  			THC63_RGB_OUT0);
> >>>  		return -ENODEV;
> >>>  	}
> >>>
> >>> -	remote = of_graph_get_remote_port_parent(thc63_out);
> >>> -	of_node_put(thc63_out);
> >>> +	remote = of_graph_get_remote_port_parent(endpoint);
> >>> +	of_node_put(endpoint);
> >>>  	if (!remote) {
> >>>  		dev_err(thc63->dev, "Endpoint in port@%u unconnected\n",
> >>>  			THC63_RGB_OUT0);
> >>> @@ -132,6 +147,14 @@ static int thc63_parse_dt(struct thc63_dev *thc63)
> >>>  	if (!thc63->next)
> >>>  		return -EPROBE_DEFER;
> >>>
> >>> +	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> >>> +						 THC63_LVDS_IN1, -1);
> >>> +	of_node_put(endpoint);
> >>> +
> >>
> >> Should you check if endpoint is enabled?
> >
> > Can endpoints be disabled ?
> 
> I meant the remote port parent, sorry about that.

Strictly speaking, you're right. I don't know if there's a practical use
for this, but I'll fix that in v2.

> >> By the way, this seems to me works properly, as in [12/15] if skip
> >> creation of the LVDS1 encoder if it is operating in dual link mode.
> >> Without that, we would hit again the issue of matching on device
> >> nodes, as the same bridge would be attached twice, am I wrong?
> >>
> >> I feel we've been there already :)
> >> https://lkml.org/lkml/2018/3/20/341
> >>
> >> and it is not just an issue on matching on endpoints, it's that the
> >> drm bridge itself that would need to handle multiple attach/detaches..
> >
> > Correct. Thanks to 12/15 we don't need this yet :-)
> >
> >>> +	thc63->timings.dual_link = endpoint != NULL;
> >>> +	dev_dbg(thc63->dev, "operating in %s-link mode\n",
> >>> +		thc63->timings.dual_link ? "dual" : "single");
> >>> +
> >>>  	return 0;
> >>>  }
> >>>
> >>> @@ -188,6 +211,7 @@ static int thc63_probe(struct platform_device *pdev)
> >>>  	thc63->bridge.driver_private = thc63;
> >>>  	thc63->bridge.of_node = pdev->dev.of_node;
> >>>  	thc63->bridge.funcs = &thc63_bridge_func;
> >>> +	thc63->bridge.timings = &thc63->timings;
> >>>
> >>>  	drm_bridge_add(&thc63->bridge);
> >>>

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 05/15] dt-bindings: display: bridge: thc63lvd1024: Document dual-link operation

[Laurent Pinchart]

Hi Jacopo,

On Sat, Mar 09, 2019 at 12:23:08PM +0100, Jacopo Mondi wrote:
> On Fri, Mar 08, 2019 at 07:57:39PM +0200, Laurent Pinchart wrote:
> > On Fri, Mar 08, 2019 at 05:49:25PM +0100, Jacopo Mondi wrote:
> >> On Thu, Mar 07, 2019 at 01:23:35AM +0200, Laurent Pinchart wrote:
> >>> The THC63LVD1024 LVDS decoder can operate in two modes, single-link or
> >>> dual-link. In dual-link mode both input ports are used to carry even-
> >>> and odd-numbered pixels separately. Document this in the DT bindings,
> >>> along with the related rules governing port and usage.
> >>>
> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >>> ---
> >>>  .../bindings/display/bridge/thine,thc63lvd1024.txt         | 7 +++++++
> >>>  1 file changed, 7 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> >>> index 37f0c04d5a28..4ff6eb9bbc19 100644
> >>> --- a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> >>> +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> >>> @@ -28,6 +28,13 @@ Optional video port nodes:
> >>>  - port@1: Second LVDS input port
> >>>  - port@3: Second digital CMOS/TTL parallel output
> >>>
> >>> +The device can operate in single-link mode or dual-link mode. In single-link
> >>> +mode, all pixels are received on port@0, and port@1 shall not contain any
> >>> +endpoint. In dual-link mode, even-numbered pixels are received on port@0 and
> >>> +odd-numbered pixels on port@1, and both port@0 and port@1 shall contain
> >>> +endpoints.
> >>
> >> You know, I'm not sure this is helpful, as if we have to go and
> >> describe what the chip supports, a paragraph for dual ouput mode would
> >> be required as well. The bindings already document that the chip
> >> supports single/dual input/output modes, maybe you can just add rules
> >> that prescribes how to populate the endpoints, for both input and
> >> output modes?
> >
> > That's what I was trying to do :-) How else would you like to see it
> > described ?
> 
> I wonder if it won't be enough to expand the Optional endpoint
> properties description as in:
> 
>  - port@1: Second LVDS input port, to be used for dual-input mode
>  - port@3: Second digital CMOS/TTL parallel output, to be used for
>    dual-output mode.

I think it's useful to explain the constraints regarding presence (or
absence) of endpoints for ports 0 and 1 depending on which mode the
device operates under, not just what the ports are used for.

> If you prefer providing more context, it's fine what you had, just
> keep in mind there's also dual-output mode and not just the dual-input
> one.

Absolutely. The reason why I haven't expanded the bindings to document
dual-output mode is that I have no way to test it, and I don't think
untested bindings (meaning without a testable end-to-end implementation)
are a good idea.

> Up to you, feel free to add my
> Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> if relevant.
> 
> >>> +
> >>> +
> >>
> >> You can drop this empty line.
> >>
> >>>  Example:
> >>>  --------
> >>>

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 07/15] dt-bindings: display: renesas: lvds: Add renesas,companion property

[Geert Uytterhoeven]

Hi Laurent,

On Thu, Mar 7, 2019 at 12:24 AM Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> Add a new optional renesas,companion property to point to the companion
> LVDS encoder. This is used to support dual-link operation where the main
> LVDS encoder splits even-numbered and odd-numbered pixels between the
> two LVDS encoders.

Note that Documentation/devicetree/bindings/usb/generic.txt already
describes a "companion" property without vendor prefix.

> The new property doesn't control the mode of operation, it only
> describes the relationship between the master and companion LVDS
> encoders.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../devicetree/bindings/display/bridge/renesas,lvds.txt     | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> index 900a884ad9f5..a720dbb5de69 100644
> --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> @@ -45,6 +45,12 @@ OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>
>  Each port shall have a single endpoint.
>
> +Optional properties:
> +
> +- renesas,companion : phandle to the companion LVDS encoder. This property is
> +  valid for the first LVDS encoder D3 and E3 SoCs only, and points to the

... on D3 ...

> +  second encoder to be used as a companion in dual-link mode.

Why restrict this to the first LVDS decoder, and not have a backlink in the
other node, like for USB?

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH/RFC 07/15] dt-bindings: display: renesas: lvds: Add renesas,companion property

[Laurent Pinchart]

Hi Geert,

On Mon, Mar 18, 2019 at 11:21:15AM +0100, Geert Uytterhoeven wrote:
> On Thu, Mar 7, 2019 at 12:24 AM Laurent Pinchart wrote:
> > Add a new optional renesas,companion property to point to the companion
> > LVDS encoder. This is used to support dual-link operation where the main
> > LVDS encoder splits even-numbered and odd-numbered pixels between the
> > two LVDS encoders.
> 
> Note that Documentation/devicetree/bindings/usb/generic.txt already
> describes a "companion" property without vendor prefix.
> 
> > The new property doesn't control the mode of operation, it only
> > describes the relationship between the master and companion LVDS
> > encoders.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  .../devicetree/bindings/display/bridge/renesas,lvds.txt     | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > index 900a884ad9f5..a720dbb5de69 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > @@ -45,6 +45,12 @@ OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> >
> >  Each port shall have a single endpoint.
> >
> > +Optional properties:
> > +
> > +- renesas,companion : phandle to the companion LVDS encoder. This property is
> > +  valid for the first LVDS encoder D3 and E3 SoCs only, and points to the
> 
> ... on D3 ...
> 
> > +  second encoder to be used as a companion in dual-link mode.
> 
> Why restrict this to the first LVDS decoder, and not have a backlink in the
> other node,

Because there's no need to ? :-) Furthermore there's a need for the
master to know its a master, so have a link in a single direction is
pretty convenient. Otherwise I'd need an extra property to identify the
master.

> like for USB?

I'm not sure what USB does in that regard.

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 04/15] drm: bridge: Add dual_link field to the drm_bridge_timings structure

[Kieran Bingham]

Hi Laurent,

On 06/03/2019 23:23, Laurent Pinchart wrote:
> Extend the drm_bridge_timings structure with a new dual_link field to
> indicate that the bridge's input bus carries data on two separate
> physical links. The first use case is LVDS dual-link mode where even-
> and odd-numbered pixels are transferred on separate LVDS links.

Do you foresee this becoming a bitfield in the future if there are more
options?

I don't think that affects this right now though, and it's fine as a bool.


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

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>


> ---
>  include/drm/drm_bridge.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index d4428913a4e1..aea1fcfd92a7 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -265,6 +265,14 @@ struct drm_bridge_timings {
>  	 * 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;
>  };
>  
>  /**
> 

-- 
Regards
--
Kieran

Re: [PATCH/RFC 03/15] drm/bridge: use bus flags in bridge timings

[Kieran Bingham]

Hi Laurent,

On 06/03/2019 23:23, Laurent Pinchart wrote:
> From: Stefan Agner <stefan@agner.ch>
> 
> The DRM bus flags convey additional information on pixel data on
> the bus. All current available bus flags might be of interest for
> a bridge. Remove the sampling_edge field and use bus_flags.
> 
> In the case at hand a dumb VGA bridge needs a specific data enable
> polarity (DRM_BUS_FLAG_DE_LOW).
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Trivial but, should you have a +renesas            ^ there?

> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>


> ---
>  drivers/gpu/drm/bridge/dumb-vga-dac.c |  6 +++---
>  include/drm/drm_bridge.h              | 12 +++++-------
>  2 files changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> index 94ed450e308d..e64736c39a9f 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
>   */
>  static const struct drm_bridge_timings default_dac_timings = {
>  	/* Timing specifications, datasheet page 7 */
> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
>  	.setup_time_ps = 500,
>  	.hold_time_ps = 1500,
>  };
> @@ -245,7 +245,7 @@ static const struct drm_bridge_timings default_dac_timings = {
>   */
>  static const struct drm_bridge_timings ti_ths8134_dac_timings = {
>  	/* From timing diagram, datasheet page 9 */
> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
>  	/* From datasheet, page 12 */
>  	.setup_time_ps = 3000,
>  	/* I guess this means latched input */
> @@ -258,7 +258,7 @@ static const struct drm_bridge_timings ti_ths8134_dac_timings = {
>   */
>  static const struct drm_bridge_timings ti_ths8135_dac_timings = {
>  	/* From timing diagram, datasheet page 14 */
> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
> +	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
>  	/* From datasheet, page 16 */
>  	.setup_time_ps = 2000,
>  	.hold_time_ps = 500,
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 5e5129206f40..d4428913a4e1 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -244,15 +244,13 @@ struct drm_bridge_funcs {
>   */
>  struct drm_bridge_timings {
>  	/**
> -	 * @sampling_edge:
> +	 * @input_bus_flags:
>  	 *
> -	 * Tells whether the bridge samples the digital input signals from the
> -	 * display engine on the positive or negative edge of the clock. This
> -	 * should use the DRM_BUS_FLAG_PIXDATA_SAMPLE_[POS|NEG]EDGE and
> -	 * DRM_BUS_FLAG_SYNC_SAMPLE_[POS|NEG]EDGE bitwise flags from the DRM
> -	 * connector (bit 2, 3, 6 and 7 valid).
> +	 * 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 sampling_edge;
> +	u32 input_bus_flags;
>  	/**
>  	 * @setup_time_ps:
>  	 *
> 

-- 
Regards
--
Kieran

Re: [PATCH/RFC 02/15] drm: Use new DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags

[Kieran Bingham]

Hi Laurent,

On 06/03/2019 23:23, Laurent Pinchart wrote:
> The DRM_BUS_FLAG_PIXDATA_(POS|NEG)EDGE and
> DRM_BUS_FLAG_SYNC_(POS|NEG)EDGE flags are deprecated in favour of the
> new DRM_BUS_FLAG_PIXDATA_(DRIVE|SAMPLE)_(POS|NEG)EDGE and
> new DRM_BUS_FLAG_SYNC_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags. Replace them
> through the code.
> 
> This effectively changes the value of the .sampling_edge bridge timings
> field in the dumb-vga-dac driver. This is safe to do as no driver
> consumes these values yet.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/gpu/drm/bridge/dumb-vga-dac.c         |  6 ++---
>  drivers/gpu/drm/bridge/tc358767.c             |  4 ++--
>  drivers/gpu/drm/drm_modes.c                   | 12 +++++-----
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c    |  2 +-
>  drivers/gpu/drm/imx/ipuv3-crtc.c              |  2 +-
>  drivers/gpu/drm/mxsfb/mxsfb_crtc.c            |  6 ++---
>  .../gpu/drm/omapdrm/displays/encoder-tfp410.c |  5 ++--
>  .../displays/panel-lgphilips-lb035q02.c       |  5 ++--
>  .../omapdrm/displays/panel-nec-nl8048hl11.c   |  5 ++--
>  .../displays/panel-sharp-ls037v7dw01.c        |  5 ++--
>  .../omapdrm/displays/panel-sony-acx565akm.c   |  5 ++--
>  .../omapdrm/displays/panel-tpo-td028ttec1.c   |  5 ++--
>  .../omapdrm/displays/panel-tpo-td043mtea1.c   |  5 ++--
>  drivers/gpu/drm/omapdrm/dss/dsi.c             |  4 ++--
>  drivers/gpu/drm/omapdrm/dss/sdi.c             |  4 ++--
>  drivers/gpu/drm/omapdrm/omap_encoder.c        |  8 +++----
>  drivers/gpu/drm/panel/panel-arm-versatile.c   |  4 ++--
>  drivers/gpu/drm/panel/panel-ilitek-ili9322.c  |  4 ++--
>  drivers/gpu/drm/panel/panel-seiko-43wvf1g.c   |  2 +-
>  drivers/gpu/drm/panel/panel-simple.c          | 24 +++++++++----------
>  drivers/gpu/drm/panel/panel-tpo-tpg110.c      | 10 ++++----
>  drivers/gpu/drm/pl111/pl111_display.c         |  2 +-
>  drivers/gpu/drm/sun4i/sun4i_tcon.c            |  4 ++--
>  drivers/gpu/drm/tve200/tve200_display.c       |  3 ++-
>  include/drm/drm_bridge.h                      |  9 +++----

This seems to cover all the uses in my tree.


Your changes appear to be consistent and cover all uses of the existing,
now deprecated flags.

I have a few queries on some of the actual flags used below based on the
surrounding comments, but even if those comments prompt action - I don't
think those changes should necessarily be part of this patch.

Therefore:

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>



>  25 files changed, 77 insertions(+), 68 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> index 0805801f4e94..94ed450e308d 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
>   */
>  static const struct drm_bridge_timings default_dac_timings = {
>  	/* Timing specifications, datasheet page 7 */
> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
>  	.setup_time_ps = 500,
>  	.hold_time_ps = 1500,
>  };
> @@ -245,7 +245,7 @@ static const struct drm_bridge_timings default_dac_timings = {
>   */
>  static const struct drm_bridge_timings ti_ths8134_dac_timings = {
>  	/* From timing diagram, datasheet page 9 */
> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
>  	/* From datasheet, page 12 */
>  	.setup_time_ps = 3000,
>  	/* I guess this means latched input */
> @@ -258,7 +258,7 @@ static const struct drm_bridge_timings ti_ths8134_dac_timings = {
>   */
>  static const struct drm_bridge_timings ti_ths8135_dac_timings = {
>  	/* From timing diagram, datasheet page 14 */
> -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
>  	/* From datasheet, page 16 */
>  	.setup_time_ps = 2000,
>  	.hold_time_ps = 500,
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index 888980d4bc74..e570c9dee180 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -1222,8 +1222,8 @@ static int tc_bridge_attach(struct drm_bridge *bridge)
>  					 &bus_format, 1);
>  	tc->connector.display_info.bus_flags =
>  		DRM_BUS_FLAG_DE_HIGH |
> -		DRM_BUS_FLAG_PIXDATA_NEGEDGE |
> -		DRM_BUS_FLAG_SYNC_NEGEDGE;
> +		DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE |
> +		DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE;
>  	drm_connector_attach_encoder(&tc->connector, tc->bridge.encoder);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 869ac6f4671e..56f92a0bba62 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -655,22 +655,22 @@ EXPORT_SYMBOL_GPL(drm_display_mode_to_videomode);
>   * @bus_flags: information about pixelclk, sync and DE polarity will be stored
>   * here
>   *
> - * Sets DRM_BUS_FLAG_DE_(LOW|HIGH),  DRM_BUS_FLAG_PIXDATA_(POS|NEG)EDGE and
> - * DISPLAY_FLAGS_SYNC_(POS|NEG)EDGE in @bus_flags according to DISPLAY_FLAGS
> + * Sets DRM_BUS_FLAG_DE_(LOW|HIGH),  DRM_BUS_FLAG_PIXDATA_DRIVE_(POS|NEG)EDGE
> + * and DISPLAY_FLAGS_SYNC_(POS|NEG)EDGE in @bus_flags according to DISPLAY_FLAGS
>   * found in @vm
>   */
>  void drm_bus_flags_from_videomode(const struct videomode *vm, u32 *bus_flags)
>  {
>  	*bus_flags = 0;
>  	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> -		*bus_flags |= DRM_BUS_FLAG_PIXDATA_POSEDGE;
> +		*bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
>  	if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> -		*bus_flags |= DRM_BUS_FLAG_PIXDATA_NEGEDGE;
> +		*bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE;
>  
>  	if (vm->flags & DISPLAY_FLAGS_SYNC_POSEDGE)
> -		*bus_flags |= DRM_BUS_FLAG_SYNC_POSEDGE;
> +		*bus_flags |= DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE;
>  	if (vm->flags & DISPLAY_FLAGS_SYNC_NEGEDGE)
> -		*bus_flags |= DRM_BUS_FLAG_SYNC_NEGEDGE;
> +		*bus_flags |= DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE;
>  
>  	if (vm->flags & DISPLAY_FLAGS_DE_LOW)
>  		*bus_flags |= DRM_BUS_FLAG_DE_LOW;
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> index bf256971063d..83c841b50272 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> @@ -94,7 +94,7 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
>  	drm_display_mode_to_videomode(mode, &vm);
>  
>  	/* INV_PXCK as default (most display sample data on rising edge) */

I know it's not directly related to your code change but this comment
confuses me.

This doesn't look like it's setting INV_PXCK by default, as it's only
doing it if the PIXDATA_DRIVE_POSEDGE flag is not set in the bus_flags?

Aha - it's because it's talking about sampling ... So DRIVE_POSEDGE ==
SAMPLE_NEGEDGE ...

So given that - perhaps the below conditional should be:
>>> + if (!(con->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE))
?

(In fact, I believe that was the purpose of these new macros right?)


> -	if (!(con->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE))
> +	if (!(con->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE))
>  		pol |= DCU_SYN_POL_INV_PXCK;
>  
>  	if (vm.flags & DISPLAY_FLAGS_HSYNC_LOW)
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index ec3602ebbc1c..311a20c942eb 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -295,7 +295,7 @@ static void ipu_crtc_mode_set_nofb(struct drm_crtc *crtc)
>  	sig_cfg.enable_pol = !(imx_crtc_state->bus_flags & DRM_BUS_FLAG_DE_LOW);
>  	/* Default to driving pixel data on negative clock edges */
>  	sig_cfg.clk_pol = !!(imx_crtc_state->bus_flags &
> -			     DRM_BUS_FLAG_PIXDATA_POSEDGE);
> +			     DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE);


Hrm ... and here again the comment and code don't seem to match ...
unless I'm mis-interpreting...

(perhaps clk_pol is the inverse? - though it's defined as "true =
rising-edge" in include/video/imx-ipu-v3.h).

If this is wrong, I'd consider it a separate change to this patch though.


>  	sig_cfg.bus_format = imx_crtc_state->bus_format;
>  	sig_cfg.v_to_h_sync = 0;
>  	sig_cfg.hsync_pin = imx_crtc_state->di_hsync_pin;
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> index 0ee1ca8a316a..98e9bda91e80 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> @@ -253,12 +253,12 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb)
>  	if (!(bus_flags & DRM_BUS_FLAG_DE_LOW))
>  		vdctrl0 |= VDCTRL0_ENABLE_ACT_HIGH;
>  	/*
> -	 * DRM_BUS_FLAG_PIXDATA_ defines are controller centric,
> +	 * DRM_BUS_FLAG_PIXDATA_DRIVE_ defines are controller centric,
>  	 * controllers VDCTRL0_DOTCLK is display centric.
>  	 * Drive on positive edge       -> display samples on falling edge
> -	 * DRM_BUS_FLAG_PIXDATA_POSEDGE -> VDCTRL0_DOTCLK_ACT_FALLING
> +	 * DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE -> VDCTRL0_DOTCLK_ACT_FALLING
>  	 */
> -	if (bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE)
> +	if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
>  		vdctrl0 |= VDCTRL0_DOTCLK_ACT_FALLING;
>  
>  	writel(vdctrl0, mxsfb->base + LCDC_VDCTRL0);
> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c b/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c
> index f1a748353279..a98d8759016d 100644
> --- a/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c
> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c
> @@ -111,8 +111,9 @@ static int tfp410_probe(struct platform_device *pdev)
>  	dssdev->output_type = OMAP_DISPLAY_TYPE_DVI;
>  	dssdev->owner = THIS_MODULE;
>  	dssdev->of_ports = BIT(1) | BIT(0);
> -	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_POSEDGE
> -			  | DRM_BUS_FLAG_PIXDATA_POSEDGE;
> +	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
> +			  | DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE
> +			  | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
>  
>  	dssdev->next = omapdss_of_find_connected_device(pdev->dev.of_node, 1);
>  	if (IS_ERR(dssdev->next)) {
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c b/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
> index f6ef8ff964dd..e36570b2bd15 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
> @@ -229,8 +229,9 @@ static int lb035q02_panel_spi_probe(struct spi_device *spi)


<extra context added by me>

>  	/*
>  	 * Note: According to the panel documentation:
>  	 * DE is active LOW
>  	 * DATA needs to be driven on the FALLING edge
>  	 */
> -	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_NEGEDGE
> -			  | DRM_BUS_FLAG_PIXDATA_POSEDGE;
> +	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
> +			  | DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE
> +			  | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;

Wow - I don't think I should look at comments vs code any more ...

Ok - so this is documented an identified inversion in commit:
d7b56e6f4e34b6d182b3c4e03c1bd88a3acfb210, so I'll just move on here :)

I see there is a similar pattern below in other displays where the
documentation states an inverted polarity so this must be a common thing.


>  
>  	omapdss_display_init(dssdev);
>  	omapdss_device_register(dssdev);
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c b/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c
> index f445de6369f7..6d3d49b90214 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c
> @@ -218,8 +218,9 @@ static int nec_8048_probe(struct spi_device *spi)
>  	dssdev->type = OMAP_DISPLAY_TYPE_DPI;
>  	dssdev->owner = THIS_MODULE;
>  	dssdev->of_ports = BIT(0);
> -	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_POSEDGE
> -			  | DRM_BUS_FLAG_PIXDATA_POSEDGE;
> +	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
> +			  | DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE
> +			  | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
>  
>  	omapdss_display_init(dssdev);
>  	omapdss_device_register(dssdev);
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> index 64b1369cb274..9532166e71df 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> @@ -227,8 +227,9 @@ static int sharp_ls_probe(struct platform_device *pdev)
>  	 * Note: According to the panel documentation:
>  	 * DATA needs to be driven on the FALLING edge
>  	 */
> -	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_NEGEDGE
> -			  | DRM_BUS_FLAG_PIXDATA_POSEDGE;
> +	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
> +			  | DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE
> +			  | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
>  
>  	omapdss_display_init(dssdev);
>  	omapdss_device_register(dssdev);
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c b/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c
> index e04663856b31..2a13dbaf35ce 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c
> @@ -741,8 +741,9 @@ static int acx565akm_probe(struct spi_device *spi)
>  	dssdev->type = OMAP_DISPLAY_TYPE_SDI;
>  	dssdev->owner = THIS_MODULE;
>  	dssdev->of_ports = BIT(0);
> -	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_NEGEDGE
> -			  | DRM_BUS_FLAG_PIXDATA_POSEDGE;
> +	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
> +			  | DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE
> +			  | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
>  
>  	omapdss_display_init(dssdev);
>  	omapdss_device_register(dssdev);
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> index 7ddc8c574a61..52ebd5027ded 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> @@ -351,8 +351,9 @@ static int td028ttec1_panel_probe(struct spi_device *spi)
>  	 * Note: According to the panel documentation:
>  	 * SYNC needs to be driven on the FALLING edge
>  	 */
> -	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_POSEDGE
> -			  | DRM_BUS_FLAG_PIXDATA_NEGEDGE;
> +	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
> +			  | DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE
> +			  | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE;
>  
>  	omapdss_display_init(dssdev);
>  	omapdss_device_register(dssdev);
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c
> index 8440fcb744d9..9c56e5d2292c 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c
> @@ -449,8 +449,9 @@ static int tpo_td043_probe(struct spi_device *spi)
>  	 * Note: According to the panel documentation:
>  	 * SYNC needs to be driven on the FALLING edge
>  	 */
> -	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_POSEDGE
> -			  | DRM_BUS_FLAG_PIXDATA_NEGEDGE;
> +	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
> +			  | DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE
> +			  | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE;
>  
>  	omapdss_display_init(dssdev);
>  	omapdss_device_register(dssdev);
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
> index 64fb788b6647..0f06872e81f7 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -5144,9 +5144,9 @@ static int dsi_init_output(struct dsi_data *dsi)
>  	out->ops = &dsi_ops;
>  	out->owner = THIS_MODULE;
>  	out->of_ports = BIT(0);
> -	out->bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE
> +	out->bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE
>  		       | DRM_BUS_FLAG_DE_HIGH
> -		       | DRM_BUS_FLAG_SYNC_NEGEDGE;
> +		       | DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE;
>  
>  	out->next = omapdss_of_find_connected_device(out->dev->of_node, 0);
>  	if (IS_ERR(out->next)) {
> diff --git a/drivers/gpu/drm/omapdrm/dss/sdi.c b/drivers/gpu/drm/omapdrm/dss/sdi.c
> index b2fe2387037a..fcf10bba4f48 100644
> --- a/drivers/gpu/drm/omapdrm/dss/sdi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/sdi.c
> @@ -294,8 +294,8 @@ static int sdi_init_output(struct sdi_device *sdi)
>  	out->of_ports = BIT(1);
>  	out->ops = &sdi_ops;
>  	out->owner = THIS_MODULE;
> -	out->bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE	/* 15.5.9.1.2 */
> -		       | DRM_BUS_FLAG_SYNC_POSEDGE;
> +	out->bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE	/* 15.5.9.1.2 */
> +		       | DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE;
>  
>  	out->next = omapdss_of_find_connected_device(out->dev->of_node, 1);
>  	if (IS_ERR(out->next)) {
> diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
> index 0d85b3a35767..5527b6f70fe5 100644
> --- a/drivers/gpu/drm/omapdrm/omap_encoder.c
> +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
> @@ -115,17 +115,17 @@ static void omap_encoder_mode_set(struct drm_encoder *encoder,
>  
>  		if (!(vm.flags & (DISPLAY_FLAGS_PIXDATA_POSEDGE |
>  				  DISPLAY_FLAGS_PIXDATA_NEGEDGE))) {
> -			if (bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE)
> +			if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
>  				vm.flags |= DISPLAY_FLAGS_PIXDATA_POSEDGE;
> -			else if (bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
> +			else if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
>  				vm.flags |= DISPLAY_FLAGS_PIXDATA_NEGEDGE;
>  		}
>  
>  		if (!(vm.flags & (DISPLAY_FLAGS_SYNC_POSEDGE |
>  				  DISPLAY_FLAGS_SYNC_NEGEDGE))) {
> -			if (bus_flags & DRM_BUS_FLAG_SYNC_POSEDGE)
> +			if (bus_flags & DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE)
>  				vm.flags |= DISPLAY_FLAGS_SYNC_POSEDGE;
> -			else if (bus_flags & DRM_BUS_FLAG_SYNC_NEGEDGE)
> +			else if (bus_flags & DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE)
>  				vm.flags |= DISPLAY_FLAGS_SYNC_NEGEDGE;
>  		}
>  	}
> diff --git a/drivers/gpu/drm/panel/panel-arm-versatile.c b/drivers/gpu/drm/panel/panel-arm-versatile.c
> index b428c4678106..078fa2c0eef8 100644
> --- a/drivers/gpu/drm/panel/panel-arm-versatile.c
> +++ b/drivers/gpu/drm/panel/panel-arm-versatile.c
> @@ -191,7 +191,7 @@ static const struct versatile_panel_type versatile_panels[] = {
>  			.vrefresh = 390,
>  			.flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
>  		},
> -		.bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> +		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
>  	},
>  	/*
>  	 * Sanyo ALR252RGT 240x320 portrait display found on the
> @@ -215,7 +215,7 @@ static const struct versatile_panel_type versatile_panels[] = {
>  			.vrefresh = 116,
>  			.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
>  		},
> -		.bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> +		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
>  		.ib2 = true,
>  	},
>  };
> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9322.c b/drivers/gpu/drm/panel/panel-ilitek-ili9322.c
> index bd38bf4f1ba6..35497ff08391 100644
> --- a/drivers/gpu/drm/panel/panel-ilitek-ili9322.c
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9322.c
> @@ -412,11 +412,11 @@ static int ili9322_init(struct drm_panel *panel, struct ili9322 *ili)
>  	if (ili->conf->dclk_active_high) {
>  		reg = ILI9322_POL_DCLK;
>  		connector->display_info.bus_flags |=
> -			DRM_BUS_FLAG_PIXDATA_POSEDGE;
> +			DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
>  	} else {
>  		reg = 0;
>  		connector->display_info.bus_flags |=
> -			DRM_BUS_FLAG_PIXDATA_NEGEDGE;
> +			DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE;
>  	}
>  	if (ili->conf->de_active_high) {
>  		reg |= ILI9322_POL_DE;
> diff --git a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
> index 2d99e28ff117..bdcc5d80823d 100644
> --- a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
> +++ b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
> @@ -328,7 +328,7 @@ static const struct seiko_panel_desc seiko_43wvf1g = {
>  		.height = 57,
>  	},
>  	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
>  };
>  
>  static const struct of_device_id platform_of_match[] = {
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index 9e8218f6a3f2..64516f23a957 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -914,7 +914,7 @@ static const struct panel_desc cdtech_s043wq26h_ct7 = {
>  		.width = 95,
>  		.height = 54,
>  	},
> -	.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
>  };
>  
>  static const struct drm_display_mode cdtech_s070wv95_ct16_mode = {
> @@ -1034,7 +1034,7 @@ static const struct panel_desc dataimage_scf0700c48ggu18 = {
>  		.height = 91,
>  	},
>  	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
>  };
>  
>  static const struct display_timing dlc_dlc0700yzg_1_timing = {
> @@ -1119,7 +1119,7 @@ static const struct panel_desc edt_et057090dhu = {
>  		.height = 86,
>  	},
>  	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
>  };
>  
>  static const struct drm_display_mode edt_etm0700g0dh6_mode = {
> @@ -1145,7 +1145,7 @@ static const struct panel_desc edt_etm0700g0dh6 = {
>  		.height = 91,
>  	},
>  	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
>  };
>  
>  static const struct panel_desc edt_etm0700g0bdh6 = {
> @@ -1157,7 +1157,7 @@ static const struct panel_desc edt_etm0700g0bdh6 = {
>  		.height = 91,
>  	},
>  	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
>  };
>  
>  static const struct drm_display_mode foxlink_fl500wvr00_a0t_mode = {
> @@ -1311,7 +1311,7 @@ static const struct panel_desc innolux_at043tn24 = {
>  		.height = 54,
>  	},
>  	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
>  };
>  
>  static const struct drm_display_mode innolux_at070tn92_mode = {
> @@ -1818,7 +1818,7 @@ static const struct panel_desc nec_nl4827hc19_05b = {
>  		.height = 54,
>  	},
>  	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> -	.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
>  };
>  
>  static const struct drm_display_mode netron_dy_e231732_mode = {
> @@ -1867,8 +1867,8 @@ static const struct panel_desc newhaven_nhd_43_480272ef_atxl = {
>  		.height = 54,
>  	},
>  	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE |
> -		     DRM_BUS_FLAG_SYNC_POSEDGE,
> +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
> +		     DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
>  };
>  
>  static const struct display_timing nlt_nl192108ac18_02d_timing = {
> @@ -2029,7 +2029,7 @@ static const struct panel_desc ortustech_com43h4m85ulc = {
>  		.height = 93,
>  	},
>  	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
>  };
>  
>  static const struct drm_display_mode pda_91_00156_a0_mode = {
> @@ -2398,7 +2398,7 @@ static const struct panel_desc toshiba_lt089ac29000 = {
>  		.height = 116,
>  	},
>  	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
>  };
>  
>  static const struct drm_display_mode tpk_f07a_0102_mode = {
> @@ -2421,7 +2421,7 @@ static const struct panel_desc tpk_f07a_0102 = {
>  		.width = 152,
>  		.height = 91,
>  	},
> -	.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +	.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
>  };
>  
>  static const struct drm_display_mode tpk_f10a_0102_mode = {
> diff --git a/drivers/gpu/drm/panel/panel-tpo-tpg110.c b/drivers/gpu/drm/panel/panel-tpo-tpg110.c
> index 5a9f8f4d5d24..25f00cfc1af4 100644
> --- a/drivers/gpu/drm/panel/panel-tpo-tpg110.c
> +++ b/drivers/gpu/drm/panel/panel-tpo-tpg110.c
> @@ -118,7 +118,7 @@ static const struct tpg110_panel_mode tpg110_modes[] = {
>  			.vtotal = 480 + 10 + 1 + 35,
>  			.vrefresh = 60,
>  		},
> -		.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
>  	},
>  	{
>  		.name = "640x480 RGB",
> @@ -135,7 +135,7 @@ static const struct tpg110_panel_mode tpg110_modes[] = {
>  			.vtotal = 480 + 18 + 1 + 27,
>  			.vrefresh = 60,
>  		},
> -		.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
>  	},
>  	{
>  		.name = "480x272 RGB",
> @@ -152,7 +152,7 @@ static const struct tpg110_panel_mode tpg110_modes[] = {
>  			.vtotal = 272 + 2 + 1 + 12,
>  			.vrefresh = 60,
>  		},
> -		.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
>  	},
>  	{
>  		.name = "480x640 RGB",
> @@ -169,7 +169,7 @@ static const struct tpg110_panel_mode tpg110_modes[] = {
>  			.vtotal = 640 + 4 + 1 + 8,
>  			.vrefresh = 60,
>  		},
> -		.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
>  	},
>  	{
>  		.name = "400x240 RGB",
> @@ -186,7 +186,7 @@ static const struct tpg110_panel_mode tpg110_modes[] = {
>  			.vtotal = 240 + 2 + 1 + 20,
>  			.vrefresh = 60,
>  		},
> -		.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
>  	},
>  };
>  
> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> index 754f6b25f265..0c5d391f0a8f 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,
>  			tim2 |= TIM2_IOE;
>  
>  		if (connector->display_info.bus_flags &
> -		    DRM_BUS_FLAG_PIXDATA_NEGEDGE)
> +		    DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
>  			tim2 |= TIM2_IPC;
>  	}
>  
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index 7136fc91c603..ca713d200280 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -561,10 +561,10 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
>  	 * Following code is a way to avoid quirks all around TCON
>  	 * and DOTCLOCK drivers.
>  	 */
> -	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE)
> +	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
>  		clk_set_phase(tcon->dclk, 240);
>  
> -	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
> +	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
>  		clk_set_phase(tcon->dclk, 0);
>  
>  	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
> diff --git a/drivers/gpu/drm/tve200/tve200_display.c b/drivers/gpu/drm/tve200/tve200_display.c
> index e8723a2412a6..d775d10dbe6a 100644
> --- a/drivers/gpu/drm/tve200/tve200_display.c
> +++ b/drivers/gpu/drm/tve200/tve200_display.c
> @@ -149,7 +149,8 @@ static void tve200_display_enable(struct drm_simple_display_pipe *pipe,
>  	/* Vsync IRQ at start of Vsync at first */
>  	ctrl1 |= TVE200_VSTSTYPE_VSYNC;
>  
> -	if (connector->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
> +	if (connector->display_info.bus_flags &
> +	    DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
>  		ctrl1 |= TVE200_CTRL_TVCLKP;
>  
>  	if ((mode->hdisplay == 352 && mode->vdisplay == 240) || /* SIF(525) */
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 9da8c93f7976..5e5129206f40 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -246,10 +246,11 @@ struct drm_bridge_timings {
>  	/**
>  	 * @sampling_edge:
>  	 *
> -	 * Tells whether the bridge samples the digital input signal
> -	 * from the display engine on the positive or negative edge of the
> -	 * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE
> -	 * bitwise flags from the DRM connector (bit 2 and 3 valid).
> +	 * Tells whether the bridge samples the digital input signals from the> +	 * display engine on the positive or negative edge of the clock. This
> +	 * should use the DRM_BUS_FLAG_PIXDATA_SAMPLE_[POS|NEG]EDGE and
> +	 * DRM_BUS_FLAG_SYNC_SAMPLE_[POS|NEG]EDGE bitwise flags from the DRM
> +	 * connector (bit 2, 3, 6 and 7 valid).
>  	 */
>  	u32 sampling_edge;
>  	/**
> 

-- 
Regards
--
Kieran

Re: [PATCH/RFC 01/15] drm: Clarify definition of the DRM_BUS_FLAG_(PIXDATA|SYNC)_* macros

[Kieran Bingham]

Hi Laurent,

Thank you for the patch,

On 06/03/2019 23:23, Laurent Pinchart wrote:
> The DRM_BUS_FLAG_PIXDATA_POSEDGE and DRM_BUS_FLAG_PIXDATA_NEGEDGE macros
> and their DRM_BUS_FLAG_SYNC_* counterparts define on which pixel clock
> edge data and sync signals are driven. They are however used in some
> drivers to define on which pixel clock edge data and sync signals are
> sampled, which should usually (but not always) be the opposite edge of
> the driving edge. This creates confusion.
> 
> Create four new macros for both PIXDATA and SYNC that explicitly state
> the driving and sampling edge in their name to remove the confusion. The
> driving macros are defined as the opposite of the sampling macros to
> made code simpler based on the assumption that the driving and sampling

s/made/make/

> edges are opposite.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>



> ---
>  include/drm/drm_connector.h | 36 ++++++++++++++++++++++++++++++++----
>  1 file changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 8fe22abb1e10..411c0eb4c00e 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -330,19 +330,47 @@ struct drm_display_info {
>  
>  #define DRM_BUS_FLAG_DE_LOW		(1<<0)
>  #define DRM_BUS_FLAG_DE_HIGH		(1<<1)
> -/* drive data on pos. edge */
> +
> +/*
> + * Don't use those two flags directly, use the DRM_BUS_FLAG_PIXDATA_DRIVE_*
> + * and DRM_BUS_FLAG_PIXDATA_SAMPLE_* variants to qualify the flags explicitly.
> + * The DRM_BUS_FLAG_PIXDATA_SAMPLE_* flags are defined as the opposite of the
> + * DRM_BUS_FLAG_PIXDATA_DRIVE_* flags to make code simpler, as signals are
> + * usually to be sampled on the opposite edge of the driving edge.
> + */
>  #define DRM_BUS_FLAG_PIXDATA_POSEDGE	(1<<2)
> -/* drive data on neg. edge */
>  #define DRM_BUS_FLAG_PIXDATA_NEGEDGE	(1<<3)
> +
> +/* Drive data on rising edge */
> +#define DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE	DRM_BUS_FLAG_PIXDATA_POSEDGE
> +/* Drive data on falling edge */
> +#define DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE	DRM_BUS_FLAG_PIXDATA_NEGEDGE
> +/* Sample data on rising edge */
> +#define DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE	DRM_BUS_FLAG_PIXDATA_NEGEDGE
> +/* Sample data on falling edge */
> +#define DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE	DRM_BUS_FLAG_PIXDATA_POSEDGE
> +
>  /* data is transmitted MSB to LSB on the bus */
>  #define DRM_BUS_FLAG_DATA_MSB_TO_LSB	(1<<4)
>  /* data is transmitted LSB to MSB on the bus */
>  #define DRM_BUS_FLAG_DATA_LSB_TO_MSB	(1<<5)
> -/* drive sync on pos. edge */
> +
> +/*
> + * Similarly to the DRM_BUS_FLAG_PIXDATA_* flags, don't use these two flags
> + * directly, use one of the DRM_BUS_FLAG_SYNC_(DRIVE|SAMPLE)_* instead.
> + */
>  #define DRM_BUS_FLAG_SYNC_POSEDGE	(1<<6)
> -/* drive sync on neg. edge */
>  #define DRM_BUS_FLAG_SYNC_NEGEDGE	(1<<7)
>  
> +/* Drive sync on rising edge */
> +#define DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE		DRM_BUS_FLAG_SYNC_POSEDGE
> +/* Drive sync on falling edge */
> +#define DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE		DRM_BUS_FLAG_SYNC_NEGEDGE
> +/* Sample sync on rising edge */
> +#define DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE	DRM_BUS_FLAG_SYNC_NEGEDGE
> +/* Sample sync on falling edge */
> +#define DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE	DRM_BUS_FLAG_SYNC_POSEDGE
> +
>  	/**
>  	 * @bus_flags: Additional information (like pixel signal polarity) for
>  	 * the pixel data on the bus, using DRM_BUS_FLAGS\_ defines.
> 

-- 
Regards
--
Kieran

Re: [PATCH/RFC 04/15] drm: bridge: Add dual_link field to the drm_bridge_timings structure

[Laurent Pinchart]

Hi Kieran,

On Wed, Apr 24, 2019 at 09:12:48AM +0100, Kieran Bingham wrote:
> On 06/03/2019 23:23, Laurent Pinchart wrote:
> > Extend the drm_bridge_timings structure with a new dual_link field to
> > indicate that the bridge's input bus carries data on two separate
> > physical links. The first use case is LVDS dual-link mode where even-
> > and odd-numbered pixels are transferred on separate LVDS links.
> 
> Do you foresee this becoming a bitfield in the future if there are more
> options?
> 
> I don't think that affects this right now though, and it's fine as a bool.

I don't know yet. Maybe we'll combine this with other flags, maybe not.
It's a bit of a ad-hoc parameter, if/when other link types require a
similar feature, we'll likely refactor this.

> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> > ---
> >  include/drm/drm_bridge.h | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > index d4428913a4e1..aea1fcfd92a7 100644
> > --- a/include/drm/drm_bridge.h
> > +++ b/include/drm/drm_bridge.h
> > @@ -265,6 +265,14 @@ struct drm_bridge_timings {
> >  	 * 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;
> >  };
> >  
> >  /**

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 02/15] drm: Use new DRM_BUS_FLAG_*_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags

[Laurent Pinchart]

Hi Kieran,

On Wed, Apr 24, 2019 at 09:15:22AM +0100, Kieran Bingham wrote:
> On 06/03/2019 23:23, Laurent Pinchart wrote:
> > The DRM_BUS_FLAG_PIXDATA_(POS|NEG)EDGE and
> > DRM_BUS_FLAG_SYNC_(POS|NEG)EDGE flags are deprecated in favour of the
> > new DRM_BUS_FLAG_PIXDATA_(DRIVE|SAMPLE)_(POS|NEG)EDGE and
> > new DRM_BUS_FLAG_SYNC_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags. Replace them
> > through the code.
> > 
> > This effectively changes the value of the .sampling_edge bridge timings
> > field in the dumb-vga-dac driver. This is safe to do as no driver
> > consumes these values yet.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> >  drivers/gpu/drm/bridge/dumb-vga-dac.c         |  6 ++---
> >  drivers/gpu/drm/bridge/tc358767.c             |  4 ++--
> >  drivers/gpu/drm/drm_modes.c                   | 12 +++++-----
> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c    |  2 +-
> >  drivers/gpu/drm/imx/ipuv3-crtc.c              |  2 +-
> >  drivers/gpu/drm/mxsfb/mxsfb_crtc.c            |  6 ++---
> >  .../gpu/drm/omapdrm/displays/encoder-tfp410.c |  5 ++--
> >  .../displays/panel-lgphilips-lb035q02.c       |  5 ++--
> >  .../omapdrm/displays/panel-nec-nl8048hl11.c   |  5 ++--
> >  .../displays/panel-sharp-ls037v7dw01.c        |  5 ++--
> >  .../omapdrm/displays/panel-sony-acx565akm.c   |  5 ++--
> >  .../omapdrm/displays/panel-tpo-td028ttec1.c   |  5 ++--
> >  .../omapdrm/displays/panel-tpo-td043mtea1.c   |  5 ++--
> >  drivers/gpu/drm/omapdrm/dss/dsi.c             |  4 ++--
> >  drivers/gpu/drm/omapdrm/dss/sdi.c             |  4 ++--
> >  drivers/gpu/drm/omapdrm/omap_encoder.c        |  8 +++----
> >  drivers/gpu/drm/panel/panel-arm-versatile.c   |  4 ++--
> >  drivers/gpu/drm/panel/panel-ilitek-ili9322.c  |  4 ++--
> >  drivers/gpu/drm/panel/panel-seiko-43wvf1g.c   |  2 +-
> >  drivers/gpu/drm/panel/panel-simple.c          | 24 +++++++++----------
> >  drivers/gpu/drm/panel/panel-tpo-tpg110.c      | 10 ++++----
> >  drivers/gpu/drm/pl111/pl111_display.c         |  2 +-
> >  drivers/gpu/drm/sun4i/sun4i_tcon.c            |  4 ++--
> >  drivers/gpu/drm/tve200/tve200_display.c       |  3 ++-
> >  include/drm/drm_bridge.h                      |  9 +++----
> 
> This seems to cover all the uses in my tree.
> 
> 
> Your changes appear to be consistent and cover all uses of the existing,
> now deprecated flags.
> 
> I have a few queries on some of the actual flags used below based on the
> surrounding comments, but even if those comments prompt action - I don't
> think those changes should necessarily be part of this patch.
> 
> Therefore:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> >  25 files changed, 77 insertions(+), 68 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > index 0805801f4e94..94ed450e308d 100644
> > --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
> >   */
> >  static const struct drm_bridge_timings default_dac_timings = {
> >  	/* Timing specifications, datasheet page 7 */
> > -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > +	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
> >  	.setup_time_ps = 500,
> >  	.hold_time_ps = 1500,
> >  };
> > @@ -245,7 +245,7 @@ static const struct drm_bridge_timings default_dac_timings = {
> >   */
> >  static const struct drm_bridge_timings ti_ths8134_dac_timings = {
> >  	/* From timing diagram, datasheet page 9 */
> > -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > +	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
> >  	/* From datasheet, page 12 */
> >  	.setup_time_ps = 3000,
> >  	/* I guess this means latched input */
> > @@ -258,7 +258,7 @@ static const struct drm_bridge_timings ti_ths8134_dac_timings = {
> >   */
> >  static const struct drm_bridge_timings ti_ths8135_dac_timings = {
> >  	/* From timing diagram, datasheet page 14 */
> > -	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > +	.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
> >  	/* From datasheet, page 16 */
> >  	.setup_time_ps = 2000,
> >  	.hold_time_ps = 500,
> > diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> > index 888980d4bc74..e570c9dee180 100644
> > --- a/drivers/gpu/drm/bridge/tc358767.c
> > +++ b/drivers/gpu/drm/bridge/tc358767.c
> > @@ -1222,8 +1222,8 @@ static int tc_bridge_attach(struct drm_bridge *bridge)
> >  					 &bus_format, 1);
> >  	tc->connector.display_info.bus_flags =
> >  		DRM_BUS_FLAG_DE_HIGH |
> > -		DRM_BUS_FLAG_PIXDATA_NEGEDGE |
> > -		DRM_BUS_FLAG_SYNC_NEGEDGE;
> > +		DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE |
> > +		DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE;
> >  	drm_connector_attach_encoder(&tc->connector, tc->bridge.encoder);
> >  
> >  	return 0;
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index 869ac6f4671e..56f92a0bba62 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -655,22 +655,22 @@ EXPORT_SYMBOL_GPL(drm_display_mode_to_videomode);
> >   * @bus_flags: information about pixelclk, sync and DE polarity will be stored
> >   * here
> >   *
> > - * Sets DRM_BUS_FLAG_DE_(LOW|HIGH),  DRM_BUS_FLAG_PIXDATA_(POS|NEG)EDGE and
> > - * DISPLAY_FLAGS_SYNC_(POS|NEG)EDGE in @bus_flags according to DISPLAY_FLAGS
> > + * Sets DRM_BUS_FLAG_DE_(LOW|HIGH),  DRM_BUS_FLAG_PIXDATA_DRIVE_(POS|NEG)EDGE
> > + * and DISPLAY_FLAGS_SYNC_(POS|NEG)EDGE in @bus_flags according to DISPLAY_FLAGS
> >   * found in @vm
> >   */
> >  void drm_bus_flags_from_videomode(const struct videomode *vm, u32 *bus_flags)
> >  {
> >  	*bus_flags = 0;
> >  	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> > -		*bus_flags |= DRM_BUS_FLAG_PIXDATA_POSEDGE;
> > +		*bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
> >  	if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> > -		*bus_flags |= DRM_BUS_FLAG_PIXDATA_NEGEDGE;
> > +		*bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE;
> >  
> >  	if (vm->flags & DISPLAY_FLAGS_SYNC_POSEDGE)
> > -		*bus_flags |= DRM_BUS_FLAG_SYNC_POSEDGE;
> > +		*bus_flags |= DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE;
> >  	if (vm->flags & DISPLAY_FLAGS_SYNC_NEGEDGE)
> > -		*bus_flags |= DRM_BUS_FLAG_SYNC_NEGEDGE;
> > +		*bus_flags |= DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE;
> >  
> >  	if (vm->flags & DISPLAY_FLAGS_DE_LOW)
> >  		*bus_flags |= DRM_BUS_FLAG_DE_LOW;
> > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> > index bf256971063d..83c841b50272 100644
> > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> > @@ -94,7 +94,7 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
> >  	drm_display_mode_to_videomode(mode, &vm);
> >  
> >  	/* INV_PXCK as default (most display sample data on rising edge) */
> 
> I know it's not directly related to your code change but this comment
> confuses me.
> 
> This doesn't look like it's setting INV_PXCK by default, as it's only
> doing it if the PIXDATA_DRIVE_POSEDGE flag is not set in the bus_flags?
> 
> Aha - it's because it's talking about sampling ... So DRIVE_POSEDGE ==
> SAMPLE_NEGEDGE ...
> 
> So given that - perhaps the below conditional should be:
> >>> + if (!(con->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE))
> ?
> 
> (In fact, I believe that was the purpose of these new macros right?)

I agree with you, but I think this requires going through individual
drivers and checking them one by one. I wanted to keep the
POSEDGE/NEGEDGE names unchanged in this series to ease review.

> > -	if (!(con->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE))
> > +	if (!(con->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE))
> >  		pol |= DCU_SYN_POL_INV_PXCK;
> >  
> >  	if (vm.flags & DISPLAY_FLAGS_HSYNC_LOW)
> > diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> > index ec3602ebbc1c..311a20c942eb 100644
> > --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> > +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> > @@ -295,7 +295,7 @@ static void ipu_crtc_mode_set_nofb(struct drm_crtc *crtc)
> >  	sig_cfg.enable_pol = !(imx_crtc_state->bus_flags & DRM_BUS_FLAG_DE_LOW);
> >  	/* Default to driving pixel data on negative clock edges */
> >  	sig_cfg.clk_pol = !!(imx_crtc_state->bus_flags &
> > -			     DRM_BUS_FLAG_PIXDATA_POSEDGE);
> > +			     DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE);
> 
> 
> Hrm ... and here again the comment and code don't seem to match ...
> unless I'm mis-interpreting...
> 
> (perhaps clk_pol is the inverse? - though it's defined as "true =
> rising-edge" in include/video/imx-ipu-v3.h).
> 
> If this is wrong, I'd consider it a separate change to this patch though.
> 
> 
> >  	sig_cfg.bus_format = imx_crtc_state->bus_format;
> >  	sig_cfg.v_to_h_sync = 0;
> >  	sig_cfg.hsync_pin = imx_crtc_state->di_hsync_pin;
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > index 0ee1ca8a316a..98e9bda91e80 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > @@ -253,12 +253,12 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb)
> >  	if (!(bus_flags & DRM_BUS_FLAG_DE_LOW))
> >  		vdctrl0 |= VDCTRL0_ENABLE_ACT_HIGH;
> >  	/*
> > -	 * DRM_BUS_FLAG_PIXDATA_ defines are controller centric,
> > +	 * DRM_BUS_FLAG_PIXDATA_DRIVE_ defines are controller centric,
> >  	 * controllers VDCTRL0_DOTCLK is display centric.
> >  	 * Drive on positive edge       -> display samples on falling edge
> > -	 * DRM_BUS_FLAG_PIXDATA_POSEDGE -> VDCTRL0_DOTCLK_ACT_FALLING
> > +	 * DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE -> VDCTRL0_DOTCLK_ACT_FALLING
> >  	 */
> > -	if (bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE)
> > +	if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
> >  		vdctrl0 |= VDCTRL0_DOTCLK_ACT_FALLING;
> >  
> >  	writel(vdctrl0, mxsfb->base + LCDC_VDCTRL0);
> > diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c b/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c
> > index f1a748353279..a98d8759016d 100644
> > --- a/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c
> > +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c
> > @@ -111,8 +111,9 @@ static int tfp410_probe(struct platform_device *pdev)
> >  	dssdev->output_type = OMAP_DISPLAY_TYPE_DVI;
> >  	dssdev->owner = THIS_MODULE;
> >  	dssdev->of_ports = BIT(1) | BIT(0);
> > -	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_POSEDGE
> > -			  | DRM_BUS_FLAG_PIXDATA_POSEDGE;
> > +	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
> > +			  | DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE
> > +			  | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
> >  
> >  	dssdev->next = omapdss_of_find_connected_device(pdev->dev.of_node, 1);
> >  	if (IS_ERR(dssdev->next)) {
> > diff --git a/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c b/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
> > index f6ef8ff964dd..e36570b2bd15 100644
> > --- a/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
> > +++ b/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
> > @@ -229,8 +229,9 @@ static int lb035q02_panel_spi_probe(struct spi_device *spi)
> 
> 
> <extra context added by me>
> 
> >  	/*
> >  	 * Note: According to the panel documentation:
> >  	 * DE is active LOW
> >  	 * DATA needs to be driven on the FALLING edge
> >  	 */
> > -	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_NEGEDGE
> > -			  | DRM_BUS_FLAG_PIXDATA_POSEDGE;
> > +	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
> > +			  | DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE
> > +			  | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
> 
> Wow - I don't think I should look at comments vs code any more ...
> 
> Ok - so this is documented an identified inversion in commit:
> d7b56e6f4e34b6d182b3c4e03c1bd88a3acfb210, so I'll just move on here :)
> 
> I see there is a similar pattern below in other displays where the
> documentation states an inverted polarity so this must be a common thing.

I believe this is caused by delays on the PCB and/or in the display
controller that are not taken into account. We've discussed this in the
past for the adv7123, where the setup time, hold time, pixel clock
frequency and delays need all to be taken into account to pick the best
edge. I unfortunately lack the necessary hardware to test this driver.

> >  
> >  	omapdss_display_init(dssdev);
> >  	omapdss_device_register(dssdev);
> > diff --git a/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c b/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c
> > index f445de6369f7..6d3d49b90214 100644
> > --- a/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c
> > +++ b/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c
> > @@ -218,8 +218,9 @@ static int nec_8048_probe(struct spi_device *spi)
> >  	dssdev->type = OMAP_DISPLAY_TYPE_DPI;
> >  	dssdev->owner = THIS_MODULE;
> >  	dssdev->of_ports = BIT(0);
> > -	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_POSEDGE
> > -			  | DRM_BUS_FLAG_PIXDATA_POSEDGE;
> > +	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
> > +			  | DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE
> > +			  | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
> >  
> >  	omapdss_display_init(dssdev);
> >  	omapdss_device_register(dssdev);
> > diff --git a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> > index 64b1369cb274..9532166e71df 100644
> > --- a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> > +++ b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> > @@ -227,8 +227,9 @@ static int sharp_ls_probe(struct platform_device *pdev)
> >  	 * Note: According to the panel documentation:
> >  	 * DATA needs to be driven on the FALLING edge
> >  	 */
> > -	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_NEGEDGE
> > -			  | DRM_BUS_FLAG_PIXDATA_POSEDGE;
> > +	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
> > +			  | DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE
> > +			  | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
> >  
> >  	omapdss_display_init(dssdev);
> >  	omapdss_device_register(dssdev);
> > diff --git a/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c b/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c
> > index e04663856b31..2a13dbaf35ce 100644
> > --- a/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c
> > +++ b/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c
> > @@ -741,8 +741,9 @@ static int acx565akm_probe(struct spi_device *spi)
> >  	dssdev->type = OMAP_DISPLAY_TYPE_SDI;
> >  	dssdev->owner = THIS_MODULE;
> >  	dssdev->of_ports = BIT(0);
> > -	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_NEGEDGE
> > -			  | DRM_BUS_FLAG_PIXDATA_POSEDGE;
> > +	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
> > +			  | DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE
> > +			  | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
> >  
> >  	omapdss_display_init(dssdev);
> >  	omapdss_device_register(dssdev);
> > diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> > index 7ddc8c574a61..52ebd5027ded 100644
> > --- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> > +++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> > @@ -351,8 +351,9 @@ static int td028ttec1_panel_probe(struct spi_device *spi)
> >  	 * Note: According to the panel documentation:
> >  	 * SYNC needs to be driven on the FALLING edge
> >  	 */
> > -	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_POSEDGE
> > -			  | DRM_BUS_FLAG_PIXDATA_NEGEDGE;
> > +	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
> > +			  | DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE
> > +			  | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE;
> >  
> >  	omapdss_display_init(dssdev);
> >  	omapdss_device_register(dssdev);
> > diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c
> > index 8440fcb744d9..9c56e5d2292c 100644
> > --- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c
> > +++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c
> > @@ -449,8 +449,9 @@ static int tpo_td043_probe(struct spi_device *spi)
> >  	 * Note: According to the panel documentation:
> >  	 * SYNC needs to be driven on the FALLING edge
> >  	 */
> > -	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_SYNC_POSEDGE
> > -			  | DRM_BUS_FLAG_PIXDATA_NEGEDGE;
> > +	dssdev->bus_flags = DRM_BUS_FLAG_DE_HIGH
> > +			  | DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE
> > +			  | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE;
> >  
> >  	omapdss_display_init(dssdev);
> >  	omapdss_device_register(dssdev);
> > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
> > index 64fb788b6647..0f06872e81f7 100644
> > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> > @@ -5144,9 +5144,9 @@ static int dsi_init_output(struct dsi_data *dsi)
> >  	out->ops = &dsi_ops;
> >  	out->owner = THIS_MODULE;
> >  	out->of_ports = BIT(0);
> > -	out->bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE
> > +	out->bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE
> >  		       | DRM_BUS_FLAG_DE_HIGH
> > -		       | DRM_BUS_FLAG_SYNC_NEGEDGE;
> > +		       | DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE;
> >  
> >  	out->next = omapdss_of_find_connected_device(out->dev->of_node, 0);
> >  	if (IS_ERR(out->next)) {
> > diff --git a/drivers/gpu/drm/omapdrm/dss/sdi.c b/drivers/gpu/drm/omapdrm/dss/sdi.c
> > index b2fe2387037a..fcf10bba4f48 100644
> > --- a/drivers/gpu/drm/omapdrm/dss/sdi.c
> > +++ b/drivers/gpu/drm/omapdrm/dss/sdi.c
> > @@ -294,8 +294,8 @@ static int sdi_init_output(struct sdi_device *sdi)
> >  	out->of_ports = BIT(1);
> >  	out->ops = &sdi_ops;
> >  	out->owner = THIS_MODULE;
> > -	out->bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE	/* 15.5.9.1.2 */
> > -		       | DRM_BUS_FLAG_SYNC_POSEDGE;
> > +	out->bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE	/* 15.5.9.1.2 */
> > +		       | DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE;
> >  
> >  	out->next = omapdss_of_find_connected_device(out->dev->of_node, 1);
> >  	if (IS_ERR(out->next)) {
> > diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
> > index 0d85b3a35767..5527b6f70fe5 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_encoder.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
> > @@ -115,17 +115,17 @@ static void omap_encoder_mode_set(struct drm_encoder *encoder,
> >  
> >  		if (!(vm.flags & (DISPLAY_FLAGS_PIXDATA_POSEDGE |
> >  				  DISPLAY_FLAGS_PIXDATA_NEGEDGE))) {
> > -			if (bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE)
> > +			if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
> >  				vm.flags |= DISPLAY_FLAGS_PIXDATA_POSEDGE;
> > -			else if (bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
> > +			else if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
> >  				vm.flags |= DISPLAY_FLAGS_PIXDATA_NEGEDGE;
> >  		}
> >  
> >  		if (!(vm.flags & (DISPLAY_FLAGS_SYNC_POSEDGE |
> >  				  DISPLAY_FLAGS_SYNC_NEGEDGE))) {
> > -			if (bus_flags & DRM_BUS_FLAG_SYNC_POSEDGE)
> > +			if (bus_flags & DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE)
> >  				vm.flags |= DISPLAY_FLAGS_SYNC_POSEDGE;
> > -			else if (bus_flags & DRM_BUS_FLAG_SYNC_NEGEDGE)
> > +			else if (bus_flags & DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE)
> >  				vm.flags |= DISPLAY_FLAGS_SYNC_NEGEDGE;
> >  		}
> >  	}
> > diff --git a/drivers/gpu/drm/panel/panel-arm-versatile.c b/drivers/gpu/drm/panel/panel-arm-versatile.c
> > index b428c4678106..078fa2c0eef8 100644
> > --- a/drivers/gpu/drm/panel/panel-arm-versatile.c
> > +++ b/drivers/gpu/drm/panel/panel-arm-versatile.c
> > @@ -191,7 +191,7 @@ static const struct versatile_panel_type versatile_panels[] = {
> >  			.vrefresh = 390,
> >  			.flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
> >  		},
> > -		.bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> > +		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> >  	},
> >  	/*
> >  	 * Sanyo ALR252RGT 240x320 portrait display found on the
> > @@ -215,7 +215,7 @@ static const struct versatile_panel_type versatile_panels[] = {
> >  			.vrefresh = 116,
> >  			.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> >  		},
> > -		.bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> > +		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> >  		.ib2 = true,
> >  	},
> >  };
> > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9322.c b/drivers/gpu/drm/panel/panel-ilitek-ili9322.c
> > index bd38bf4f1ba6..35497ff08391 100644
> > --- a/drivers/gpu/drm/panel/panel-ilitek-ili9322.c
> > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9322.c
> > @@ -412,11 +412,11 @@ static int ili9322_init(struct drm_panel *panel, struct ili9322 *ili)
> >  	if (ili->conf->dclk_active_high) {
> >  		reg = ILI9322_POL_DCLK;
> >  		connector->display_info.bus_flags |=
> > -			DRM_BUS_FLAG_PIXDATA_POSEDGE;
> > +			DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
> >  	} else {
> >  		reg = 0;
> >  		connector->display_info.bus_flags |=
> > -			DRM_BUS_FLAG_PIXDATA_NEGEDGE;
> > +			DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE;
> >  	}
> >  	if (ili->conf->de_active_high) {
> >  		reg |= ILI9322_POL_DE;
> > diff --git a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
> > index 2d99e28ff117..bdcc5d80823d 100644
> > --- a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
> > +++ b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
> > @@ -328,7 +328,7 @@ static const struct seiko_panel_desc seiko_43wvf1g = {
> >  		.height = 57,
> >  	},
> >  	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> >  };
> >  
> >  static const struct of_device_id platform_of_match[] = {
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > index 9e8218f6a3f2..64516f23a957 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -914,7 +914,7 @@ static const struct panel_desc cdtech_s043wq26h_ct7 = {
> >  		.width = 95,
> >  		.height = 54,
> >  	},
> > -	.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > +	.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> >  };
> >  
> >  static const struct drm_display_mode cdtech_s070wv95_ct16_mode = {
> > @@ -1034,7 +1034,7 @@ static const struct panel_desc dataimage_scf0700c48ggu18 = {
> >  		.height = 91,
> >  	},
> >  	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> >  };
> >  
> >  static const struct display_timing dlc_dlc0700yzg_1_timing = {
> > @@ -1119,7 +1119,7 @@ static const struct panel_desc edt_et057090dhu = {
> >  		.height = 86,
> >  	},
> >  	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> >  };
> >  
> >  static const struct drm_display_mode edt_etm0700g0dh6_mode = {
> > @@ -1145,7 +1145,7 @@ static const struct panel_desc edt_etm0700g0dh6 = {
> >  		.height = 91,
> >  	},
> >  	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> >  };
> >  
> >  static const struct panel_desc edt_etm0700g0bdh6 = {
> > @@ -1157,7 +1157,7 @@ static const struct panel_desc edt_etm0700g0bdh6 = {
> >  		.height = 91,
> >  	},
> >  	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> >  };
> >  
> >  static const struct drm_display_mode foxlink_fl500wvr00_a0t_mode = {
> > @@ -1311,7 +1311,7 @@ static const struct panel_desc innolux_at043tn24 = {
> >  		.height = 54,
> >  	},
> >  	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> >  };
> >  
> >  static const struct drm_display_mode innolux_at070tn92_mode = {
> > @@ -1818,7 +1818,7 @@ static const struct panel_desc nec_nl4827hc19_05b = {
> >  		.height = 54,
> >  	},
> >  	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> > -	.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > +	.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> >  };
> >  
> >  static const struct drm_display_mode netron_dy_e231732_mode = {
> > @@ -1867,8 +1867,8 @@ static const struct panel_desc newhaven_nhd_43_480272ef_atxl = {
> >  		.height = 54,
> >  	},
> >  	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE |
> > -		     DRM_BUS_FLAG_SYNC_POSEDGE,
> > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
> > +		     DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
> >  };
> >  
> >  static const struct display_timing nlt_nl192108ac18_02d_timing = {
> > @@ -2029,7 +2029,7 @@ static const struct panel_desc ortustech_com43h4m85ulc = {
> >  		.height = 93,
> >  	},
> >  	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> >  };
> >  
> >  static const struct drm_display_mode pda_91_00156_a0_mode = {
> > @@ -2398,7 +2398,7 @@ static const struct panel_desc toshiba_lt089ac29000 = {
> >  		.height = 116,
> >  	},
> >  	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> >  };
> >  
> >  static const struct drm_display_mode tpk_f07a_0102_mode = {
> > @@ -2421,7 +2421,7 @@ static const struct panel_desc tpk_f07a_0102 = {
> >  		.width = 152,
> >  		.height = 91,
> >  	},
> > -	.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > +	.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> >  };
> >  
> >  static const struct drm_display_mode tpk_f10a_0102_mode = {
> > diff --git a/drivers/gpu/drm/panel/panel-tpo-tpg110.c b/drivers/gpu/drm/panel/panel-tpo-tpg110.c
> > index 5a9f8f4d5d24..25f00cfc1af4 100644
> > --- a/drivers/gpu/drm/panel/panel-tpo-tpg110.c
> > +++ b/drivers/gpu/drm/panel/panel-tpo-tpg110.c
> > @@ -118,7 +118,7 @@ static const struct tpg110_panel_mode tpg110_modes[] = {
> >  			.vtotal = 480 + 10 + 1 + 35,
> >  			.vrefresh = 60,
> >  		},
> > -		.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > +		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> >  	},
> >  	{
> >  		.name = "640x480 RGB",
> > @@ -135,7 +135,7 @@ static const struct tpg110_panel_mode tpg110_modes[] = {
> >  			.vtotal = 480 + 18 + 1 + 27,
> >  			.vrefresh = 60,
> >  		},
> > -		.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > +		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> >  	},
> >  	{
> >  		.name = "480x272 RGB",
> > @@ -152,7 +152,7 @@ static const struct tpg110_panel_mode tpg110_modes[] = {
> >  			.vtotal = 272 + 2 + 1 + 12,
> >  			.vrefresh = 60,
> >  		},
> > -		.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > +		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> >  	},
> >  	{
> >  		.name = "480x640 RGB",
> > @@ -169,7 +169,7 @@ static const struct tpg110_panel_mode tpg110_modes[] = {
> >  			.vtotal = 640 + 4 + 1 + 8,
> >  			.vrefresh = 60,
> >  		},
> > -		.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > +		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> >  	},
> >  	{
> >  		.name = "400x240 RGB",
> > @@ -186,7 +186,7 @@ static const struct tpg110_panel_mode tpg110_modes[] = {
> >  			.vtotal = 240 + 2 + 1 + 20,
> >  			.vrefresh = 60,
> >  		},
> > -		.bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > +		.bus_flags = DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> >  	},
> >  };
> >  
> > diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> > index 754f6b25f265..0c5d391f0a8f 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,
> >  			tim2 |= TIM2_IOE;
> >  
> >  		if (connector->display_info.bus_flags &
> > -		    DRM_BUS_FLAG_PIXDATA_NEGEDGE)
> > +		    DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
> >  			tim2 |= TIM2_IPC;
> >  	}
> >  
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > index 7136fc91c603..ca713d200280 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > @@ -561,10 +561,10 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
> >  	 * Following code is a way to avoid quirks all around TCON
> >  	 * and DOTCLOCK drivers.
> >  	 */
> > -	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE)
> > +	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
> >  		clk_set_phase(tcon->dclk, 240);
> >  
> > -	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
> > +	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
> >  		clk_set_phase(tcon->dclk, 0);
> >  
> >  	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
> > diff --git a/drivers/gpu/drm/tve200/tve200_display.c b/drivers/gpu/drm/tve200/tve200_display.c
> > index e8723a2412a6..d775d10dbe6a 100644
> > --- a/drivers/gpu/drm/tve200/tve200_display.c
> > +++ b/drivers/gpu/drm/tve200/tve200_display.c
> > @@ -149,7 +149,8 @@ static void tve200_display_enable(struct drm_simple_display_pipe *pipe,
> >  	/* Vsync IRQ at start of Vsync at first */
> >  	ctrl1 |= TVE200_VSTSTYPE_VSYNC;
> >  
> > -	if (connector->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
> > +	if (connector->display_info.bus_flags &
> > +	    DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
> >  		ctrl1 |= TVE200_CTRL_TVCLKP;
> >  
> >  	if ((mode->hdisplay == 352 && mode->vdisplay == 240) || /* SIF(525) */
> > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > index 9da8c93f7976..5e5129206f40 100644
> > --- a/include/drm/drm_bridge.h
> > +++ b/include/drm/drm_bridge.h
> > @@ -246,10 +246,11 @@ struct drm_bridge_timings {
> >  	/**
> >  	 * @sampling_edge:
> >  	 *
> > -	 * Tells whether the bridge samples the digital input signal
> > -	 * from the display engine on the positive or negative edge of the
> > -	 * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE
> > -	 * bitwise flags from the DRM connector (bit 2 and 3 valid).
> > +	 * Tells whether the bridge samples the digital input signals from the> +	 * display engine on the positive or negative edge of the clock. This
> > +	 * should use the DRM_BUS_FLAG_PIXDATA_SAMPLE_[POS|NEG]EDGE and
> > +	 * DRM_BUS_FLAG_SYNC_SAMPLE_[POS|NEG]EDGE bitwise flags from the DRM
> > +	 * connector (bit 2, 3, 6 and 7 valid).
> >  	 */
> >  	u32 sampling_edge;
> >  	/**

-- 
Regards,

Laurent Pinchart