[PATCH 0/5] R-Car DU: Fix DPAD output routing on D3 and E3

[PATCH 0/5] R-Car DU: Fix DPAD output routing on D3 and E3

[Laurent Pinchart]

Hello,

DU channels are routed to DPAD outputs in an SoC-dependent way. The routing
can be fixed (e.g. DU3 to DPAD0 on H3) or configurable (e.g. DU0 or DU1 to
DPAD0 on D3/E3). The hardware offers no option to disconnect DPAD outputs,
which are thus always driven by a DU channel.

On SoCs that have less DU channels than DU outputs, such as D3 and E3, the
DPAD output is always driven when all channels are in used by other outputs
(such as the internal LVDS and HDMI encoders). This creates an unwanted clone
on the DPAD output.

However, the parallel output of the DU channels routed to DPAD can be set to
fixed levels in the DU channels themselves through the DOFLR group register.
This patch series uses this feature to fix the problem and get rid of unwanted
clones.

Patch 1/5 is a small cleanup not strictly required by the series, but included
as it was developed at the same time. Patch 2/5 moves output routing
information from the CRTC structure to the CRTC state, in order to make the
information available early enough for output routing configuration. Patch 3/5
then fixes the DPAD output routing issue using the DOFLR register.

Patches 4/5 and 5/5 add backlight and panel support to the Draak board DT, in
order to test the series. Only patch 4/5 should be upstreamed at this time as
5/5 should be rewritten using DT overlays. It can be merged separately from
the rest of the series as code and DT are not dependent.

Laurent Pinchart (5):
  drm: rcar-du: Replace EXT_CTRL_REGS feature flag with generation check
  drm: rcar-du: Move CRTC outputs bitmask to private CRTC state
  drm: rcar-du: Disable unused DPAD outputs
  arm64: dts: renesas: r8a77995: draak: Add backlight
  [HACK] arm64: dts: r8a77995: draak: Add panel to DT

 .../arm64/boot/dts/renesas/r8a77995-draak.dts | 31 ++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c        | 42 ++++++++--------
 drivers/gpu/drm/rcar-du/rcar_du_crtc.h        |  7 +--
 drivers/gpu/drm/rcar-du/rcar_du_drv.c         | 14 +-----
 drivers/gpu/drm/rcar-du/rcar_du_drv.h         |  8 +--
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c     | 10 ----
 drivers/gpu/drm/rcar-du/rcar_du_group.c       | 50 +++++++++++++++++--
 drivers/gpu/drm/rcar-du/rcar_du_kms.c         | 22 ++++++++
 8 files changed, 126 insertions(+), 58 deletions(-)

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/5] drm: rcar-du: Replace EXT_CTRL_REGS feature flag with generation check

[Laurent Pinchart]

The RCAR_DU_FEATURE_EXT_CTRL_REGS feature flag is missing for H1 only,
which is a first generation device, not a second generation device as
reported in the device information table. Fix the H1 generation and use
generation checks to replace the feature flag.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_drv.c   | 14 +-------------
 drivers/gpu/drm/rcar-du/rcar_du_drv.h   |  7 +++----
 drivers/gpu/drm/rcar-du/rcar_du_group.c |  4 ++--
 3 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 94f055186b95..814c1099dacb 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -35,7 +35,6 @@
 static const struct rcar_du_device_info rzg1_du_r8a7743_info = {
 	.gen = 2,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
 		  | RCAR_DU_FEATURE_INTERLACED
 		  | RCAR_DU_FEATURE_TVM_SYNC,
 	.channels_mask = BIT(1) | BIT(0),
@@ -58,7 +57,6 @@ static const struct rcar_du_device_info rzg1_du_r8a7743_info = {
 static const struct rcar_du_device_info rzg1_du_r8a7745_info = {
 	.gen = 2,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
 		  | RCAR_DU_FEATURE_INTERLACED
 		  | RCAR_DU_FEATURE_TVM_SYNC,
 	.channels_mask = BIT(1) | BIT(0),
@@ -80,7 +78,6 @@ static const struct rcar_du_device_info rzg1_du_r8a7745_info = {
 static const struct rcar_du_device_info rzg1_du_r8a77470_info = {
 	.gen = 2,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
 		  | RCAR_DU_FEATURE_INTERLACED
 		  | RCAR_DU_FEATURE_TVM_SYNC,
 	.channels_mask = BIT(1) | BIT(0),
@@ -105,7 +102,7 @@ static const struct rcar_du_device_info rzg1_du_r8a77470_info = {
 };
 
 static const struct rcar_du_device_info rcar_du_r8a7779_info = {
-	.gen = 2,
+	.gen = 1,
 	.features = RCAR_DU_FEATURE_INTERLACED
 		  | RCAR_DU_FEATURE_TVM_SYNC,
 	.channels_mask = BIT(1) | BIT(0),
@@ -128,7 +125,6 @@ static const struct rcar_du_device_info rcar_du_r8a7779_info = {
 static const struct rcar_du_device_info rcar_du_r8a7790_info = {
 	.gen = 2,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
 		  | RCAR_DU_FEATURE_INTERLACED
 		  | RCAR_DU_FEATURE_TVM_SYNC,
 	.quirks = RCAR_DU_QUIRK_ALIGN_128B,
@@ -158,7 +154,6 @@ static const struct rcar_du_device_info rcar_du_r8a7790_info = {
 static const struct rcar_du_device_info rcar_du_r8a7791_info = {
 	.gen = 2,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
 		  | RCAR_DU_FEATURE_INTERLACED
 		  | RCAR_DU_FEATURE_TVM_SYNC,
 	.channels_mask = BIT(1) | BIT(0),
@@ -182,7 +177,6 @@ static const struct rcar_du_device_info rcar_du_r8a7791_info = {
 static const struct rcar_du_device_info rcar_du_r8a7792_info = {
 	.gen = 2,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
 		  | RCAR_DU_FEATURE_INTERLACED
 		  | RCAR_DU_FEATURE_TVM_SYNC,
 	.channels_mask = BIT(1) | BIT(0),
@@ -202,7 +196,6 @@ static const struct rcar_du_device_info rcar_du_r8a7792_info = {
 static const struct rcar_du_device_info rcar_du_r8a7794_info = {
 	.gen = 2,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
 		  | RCAR_DU_FEATURE_INTERLACED
 		  | RCAR_DU_FEATURE_TVM_SYNC,
 	.channels_mask = BIT(1) | BIT(0),
@@ -225,7 +218,6 @@ static const struct rcar_du_device_info rcar_du_r8a7794_info = {
 static const struct rcar_du_device_info rcar_du_r8a7795_info = {
 	.gen = 3,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
 		  | RCAR_DU_FEATURE_VSP1_SOURCE
 		  | RCAR_DU_FEATURE_INTERLACED
 		  | RCAR_DU_FEATURE_TVM_SYNC,
@@ -259,7 +251,6 @@ static const struct rcar_du_device_info rcar_du_r8a7795_info = {
 static const struct rcar_du_device_info rcar_du_r8a7796_info = {
 	.gen = 3,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
 		  | RCAR_DU_FEATURE_VSP1_SOURCE
 		  | RCAR_DU_FEATURE_INTERLACED
 		  | RCAR_DU_FEATURE_TVM_SYNC,
@@ -289,7 +280,6 @@ static const struct rcar_du_device_info rcar_du_r8a7796_info = {
 static const struct rcar_du_device_info rcar_du_r8a77965_info = {
 	.gen = 3,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
 		  | RCAR_DU_FEATURE_VSP1_SOURCE
 		  | RCAR_DU_FEATURE_INTERLACED
 		  | RCAR_DU_FEATURE_TVM_SYNC,
@@ -319,7 +309,6 @@ static const struct rcar_du_device_info rcar_du_r8a77965_info = {
 static const struct rcar_du_device_info rcar_du_r8a77970_info = {
 	.gen = 3,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
 		  | RCAR_DU_FEATURE_VSP1_SOURCE
 		  | RCAR_DU_FEATURE_INTERLACED
 		  | RCAR_DU_FEATURE_TVM_SYNC,
@@ -341,7 +330,6 @@ static const struct rcar_du_device_info rcar_du_r8a77970_info = {
 static const struct rcar_du_device_info rcar_du_r8a7799x_info = {
 	.gen = 3,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
 		  | RCAR_DU_FEATURE_VSP1_SOURCE,
 	.channels_mask = BIT(1) | BIT(0),
 	.routes = {
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
index 9f5563296c5a..8ef9165957cb 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -24,10 +24,9 @@ struct drm_fbdev_cma;
 struct rcar_du_device;
 
 #define RCAR_DU_FEATURE_CRTC_IRQ_CLOCK	BIT(0)	/* Per-CRTC IRQ and clock */
-#define RCAR_DU_FEATURE_EXT_CTRL_REGS	BIT(1)	/* Has extended control registers */
-#define RCAR_DU_FEATURE_VSP1_SOURCE	BIT(2)	/* Has inputs from VSP1 */
-#define RCAR_DU_FEATURE_INTERLACED	BIT(3)	/* HW supports interlaced */
-#define RCAR_DU_FEATURE_TVM_SYNC	BIT(4)	/* Has TV switch/sync modes */
+#define RCAR_DU_FEATURE_VSP1_SOURCE	BIT(1)	/* Has inputs from VSP1 */
+#define RCAR_DU_FEATURE_INTERLACED	BIT(2)	/* HW supports interlaced */
+#define RCAR_DU_FEATURE_TVM_SYNC	BIT(3)	/* Has TV switch/sync modes */
 
 #define RCAR_DU_QUIRK_ALIGN_128B	BIT(0)	/* Align pitches to 128 bytes */
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
index cebf313c6e1f..3366cda6086c 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
@@ -147,7 +147,7 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
 
 	rcar_du_group_setup_pins(rgrp);
 
-	if (rcar_du_has(rgrp->dev, RCAR_DU_FEATURE_EXT_CTRL_REGS)) {
+	if (rcdu->info->gen >= 2) {
 		rcar_du_group_setup_defr8(rgrp);
 		rcar_du_group_setup_didsr(rgrp);
 	}
@@ -262,7 +262,7 @@ int rcar_du_set_dpad0_vsp1_routing(struct rcar_du_device *rcdu)
 	unsigned int index;
 	int ret;
 
-	if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_EXT_CTRL_REGS))
+	if (rcdu->info->gen < 2)
 		return 0;
 
 	/*
-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/5] drm: rcar-du: Move CRTC outputs bitmask to private CRTC state

[Laurent Pinchart]

The rcar_du_crtc outputs field stores a bitmask of the outputs driven by
the CRTC. This changes based on the configuration requested by
userspace, and is used for the sole purpose of configuring the hardware.
The field thus belongs to the CRTC state. Move it to the
rcar_du_crtc_state structure.

As a result the rcar_du_crtc_route_output() function loses most of its
purpose. In order to remove it, move dpad0_source calculation to
rcar_du_atomic_commit_tail(), until the field gets moved to a state
structure. In order to simplify the rcar_du_group_set_routing()
implementation, we also store the DPAD1 source in a new dpad1_source
field which will move to a state structure with dpad0_source.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c    | 42 +++++++++++------------
 drivers/gpu/drm/rcar-du/rcar_du_crtc.h    |  7 ++--
 drivers/gpu/drm/rcar-du/rcar_du_drv.h     |  1 +
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 10 ------
 drivers/gpu/drm/rcar-du/rcar_du_group.c   |  4 +--
 drivers/gpu/drm/rcar-du/rcar_du_kms.c     | 22 ++++++++++++
 6 files changed, 47 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 90dacab67be5..40b7f17159b0 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -22,6 +22,7 @@
 
 #include "rcar_du_crtc.h"
 #include "rcar_du_drv.h"
+#include "rcar_du_encoder.h"
 #include "rcar_du_kms.h"
 #include "rcar_du_plane.h"
 #include "rcar_du_regs.h"
@@ -316,26 +317,6 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
 	rcar_du_crtc_write(rcrtc, DEWR,  mode->hdisplay);
 }
 
-void rcar_du_crtc_route_output(struct drm_crtc *crtc,
-			       enum rcar_du_output output)
-{
-	struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
-	struct rcar_du_device *rcdu = rcrtc->group->dev;
-
-	/*
-	 * Store the route from the CRTC output to the DU output. The DU will be
-	 * configured when starting the CRTC.
-	 */
-	rcrtc->outputs |= BIT(output);
-
-	/*
-	 * Store RGB routing to DPAD0, the hardware will be configured when
-	 * starting the CRTC.
-	 */
-	if (output == RCAR_DU_OUTPUT_DPAD0)
-		rcdu->dpad0_source = rcrtc->index;
-}
-
 static unsigned int plane_zpos(struct rcar_du_plane *plane)
 {
 	return plane->plane.state->normalized_zpos;
@@ -655,6 +636,24 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
  * CRTC Functions
  */
 
+static int rcar_du_crtc_atomic_check(struct drm_crtc *crtc,
+				     struct drm_crtc_state *state)
+{
+	struct rcar_du_crtc_state *rstate = to_rcar_crtc_state(state);
+	struct drm_encoder *encoder;
+
+	/* Store the routes from the CRTC output to the DU outputs. */
+	rstate->outputs = 0;
+
+	drm_for_each_encoder_mask(encoder, crtc->dev, state->encoder_mask) {
+		struct rcar_du_encoder *renc = to_rcar_encoder(encoder);
+
+		rstate->outputs |= BIT(renc->output);
+	}
+
+	return 0;
+}
+
 static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
 				       struct drm_crtc_state *old_state)
 {
@@ -678,8 +677,6 @@ static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc,
 		crtc->state->event = NULL;
 	}
 	spin_unlock_irq(&crtc->dev->event_lock);
-
-	rcrtc->outputs = 0;
 }
 
 static void rcar_du_crtc_atomic_begin(struct drm_crtc *crtc,
@@ -755,6 +752,7 @@ enum drm_mode_status rcar_du_crtc_mode_valid(struct drm_crtc *crtc,
 }
 
 static const struct drm_crtc_helper_funcs crtc_helper_funcs = {
+	.atomic_check = rcar_du_crtc_atomic_check,
 	.atomic_begin = rcar_du_crtc_atomic_begin,
 	.atomic_flush = rcar_du_crtc_atomic_flush,
 	.atomic_enable = rcar_du_crtc_atomic_enable,
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
index 59ac6e7d22c9..ec47f164e69b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
@@ -37,7 +37,6 @@ struct rcar_du_vsp;
  * @vblank_lock: protects vblank_wait and vblank_count
  * @vblank_wait: wait queue used to signal vertical blanking
  * @vblank_count: number of vertical blanking interrupts to wait for
- * @outputs: bitmask of the outputs (enum rcar_du_output) driven by this CRTC
  * @group: CRTC group this CRTC belongs to
  * @vsp: VSP feeding video to this CRTC
  * @vsp_pipe: index of the VSP pipeline feeding video to this CRTC
@@ -61,8 +60,6 @@ struct rcar_du_crtc {
 	wait_queue_head_t vblank_wait;
 	unsigned int vblank_count;
 
-	unsigned int outputs;
-
 	struct rcar_du_group *group;
 	struct rcar_du_vsp *vsp;
 	unsigned int vsp_pipe;
@@ -77,11 +74,13 @@ struct rcar_du_crtc {
  * struct rcar_du_crtc_state - Driver-specific CRTC state
  * @state: base DRM CRTC state
  * @crc: CRC computation configuration
+ * @outputs: bitmask of the outputs (enum rcar_du_output) driven by this CRTC
  */
 struct rcar_du_crtc_state {
 	struct drm_crtc_state state;
 
 	struct vsp1_du_crc_config crc;
+	unsigned int outputs;
 };
 
 #define to_rcar_crtc_state(s) container_of(s, struct rcar_du_crtc_state, state)
@@ -102,8 +101,6 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int swindex,
 void rcar_du_crtc_suspend(struct rcar_du_crtc *rcrtc);
 void rcar_du_crtc_resume(struct rcar_du_crtc *rcrtc);
 
-void rcar_du_crtc_route_output(struct drm_crtc *crtc,
-			       enum rcar_du_output output);
 void rcar_du_crtc_finish_page_flip(struct rcar_du_crtc *rcrtc);
 
 void rcar_du_crtc_dsysr_clr_set(struct rcar_du_crtc *rcrtc, u32 clr, u32 set);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
index 8ef9165957cb..8b47b8546fc8 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -90,6 +90,7 @@ struct rcar_du_device {
 	} props;
 
 	unsigned int dpad0_source;
+	unsigned int dpad1_source;
 	unsigned int vspd1_sink;
 };
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
index 1877764bd6d9..a123c28ea6ed 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
@@ -22,17 +22,7 @@
  * Encoder
  */
 
-static void rcar_du_encoder_mode_set(struct drm_encoder *encoder,
-				     struct drm_crtc_state *crtc_state,
-				     struct drm_connector_state *conn_state)
-{
-	struct rcar_du_encoder *renc = to_rcar_encoder(encoder);
-
-	rcar_du_crtc_route_output(crtc_state->crtc, renc->output);
-}
-
 static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
-	.atomic_mode_set = rcar_du_encoder_mode_set,
 };
 
 static const struct drm_encoder_funcs encoder_funcs = {
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
index 3366cda6086c..7e440f61977f 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
@@ -289,7 +289,7 @@ int rcar_du_set_dpad0_vsp1_routing(struct rcar_du_device *rcdu)
 
 int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
 {
-	struct rcar_du_crtc *crtc0 = &rgrp->dev->crtcs[rgrp->index * 2];
+	struct rcar_du_device *rcdu = rgrp->dev;
 	u32 dorcr = rcar_du_group_read(rgrp, DORCR);
 
 	dorcr &= ~(DORCR_PG2T | DORCR_DK2S | DORCR_PG2D_MASK);
@@ -299,7 +299,7 @@ int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
 	 * CRTC 1 in all other cases to avoid cloning CRTC 0 to DPAD0 and DPAD1
 	 * by default.
 	 */
-	if (crtc0->outputs & BIT(RCAR_DU_OUTPUT_DPAD1))
+	if (rcdu->dpad1_source == rgrp->index * 2)
 		dorcr |= DORCR_PG2D_DS1;
 	else
 		dorcr |= DORCR_PG2T | DORCR_DK2S | DORCR_PG2D_DS2;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index fe6f65c94eef..bd3c2c5ea66f 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -285,6 +285,28 @@ static int rcar_du_atomic_check(struct drm_device *dev,
 static void rcar_du_atomic_commit_tail(struct drm_atomic_state *old_state)
 {
 	struct drm_device *dev = old_state->dev;
+	struct rcar_du_device *rcdu = dev->dev_private;
+	struct drm_crtc_state *crtc_state;
+	struct drm_crtc *crtc;
+	unsigned int i;
+
+	/*
+	 * Store RGB routing to DPAD0 and DPAD1, the hardware will be configured
+	 * when starting the CRTCs.
+	 */
+	rcdu->dpad1_source = -1;
+
+	for_each_new_crtc_in_state(old_state, crtc, crtc_state, i) {
+		struct rcar_du_crtc_state *rcrtc_state =
+			to_rcar_crtc_state(crtc_state);
+		struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
+
+		if (rcrtc_state->outputs & BIT(RCAR_DU_OUTPUT_DPAD0))
+			rcdu->dpad0_source = rcrtc->index;
+
+		if (rcrtc_state->outputs & BIT(RCAR_DU_OUTPUT_DPAD1))
+			rcdu->dpad1_source = rcrtc->index;
+	}
 
 	/* Apply the atomic update. */
 	drm_atomic_helper_commit_modeset_disables(dev, old_state);
-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/5] drm: rcar-du: Disable unused DPAD outputs

[Laurent Pinchart]

DU channels are routed to DPAD outputs in an SoC-dependent way. The
routing can be fixed (e.g. DU3 to DPAD0 on H3) or configurable (e.g. DU0
or DU1 to DPAD0 on D3/E3). The hardware offers no option to disconnect
DPAD outputs, which are thus always driven by a DU channel.

On SoCs that have less DU channels than DU outputs, such as D3 and E3,
the DPAD output is always driven when all channels are in used by other
outputs (such as the internal LVDS and HDMI encoders). This creates an
unwanted clone on the DPAD output.

However, the parallel output of the DU channels routed to DPAD can be
set to fixed levels in the DU channels themselves through the DOFLR
group register. Use this to turn the DPAD on or off by driving fixed
signals at the output of any DU channel not routed to a DPAD output.
This doesn't affect the DU output signals going to other outputs.

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

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
index 7e440f61977f..5aaf41b7a2ca 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
@@ -287,6 +287,46 @@ int rcar_du_set_dpad0_vsp1_routing(struct rcar_du_device *rcdu)
 	return 0;
 }
 
+static void rcar_du_group_set_output_levels(struct rcar_du_group *rgrp)
+{
+	static const u32 doflr_values[2] = {
+		DOFLR_HSYCFL0 | DOFLR_VSYCFL0 | DOFLR_ODDFL0 |
+		DOFLR_DISPFL0 | DOFLR_CDEFL0  | DOFLR_RGBFL0,
+		DOFLR_HSYCFL1 | DOFLR_VSYCFL1 | DOFLR_ODDFL1 |
+		DOFLR_DISPFL1 | DOFLR_CDEFL1  | DOFLR_RGBFL1,
+	};
+	static const u32 dpad_mask = BIT(RCAR_DU_OUTPUT_DPAD1)
+				   | BIT(RCAR_DU_OUTPUT_DPAD0);
+	struct rcar_du_device *rcdu = rgrp->dev;
+	u32 doflr = DOFLR_CODE;
+	unsigned int i;
+
+	if (rcdu->info->gen < 2)
+		return;
+
+	/*
+	 * The DPAD outputs can't be controlled directly. However, the parallel
+	 * output of the DU channels routed to DPAD can be set to fixed levels
+	 * through the DOFLR group register. Use this to turn the DPAD on or off
+	 * by driving fixed signals at the output of any DU channel not routed
+	 * to a DPAD output. This doesn't affect the DU output signals going to
+	 * other outputs, such as the internal LVDS and HDMI encoders.
+	 */
+
+	for (i = 0; i < rgrp->num_crtcs; ++i) {
+		struct rcar_du_crtc_state *rstate;
+		struct rcar_du_crtc *rcrtc;
+
+		rcrtc = &rcdu->crtcs[rgrp->index * 2 + i];
+		rstate = to_rcar_crtc_state(rcrtc->crtc.state);
+
+		if (!(rstate->outputs & dpad_mask))
+			doflr |= doflr_values[i];
+	}
+
+	rcar_du_group_write(rgrp, DOFLR, doflr);
+}
+
 int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
 {
 	struct rcar_du_device *rcdu = rgrp->dev;
@@ -306,5 +346,7 @@ int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
 
 	rcar_du_group_write(rgrp, DORCR, dorcr);
 
+	rcar_du_group_set_output_levels(rgrp);
+
 	return rcar_du_set_dpad0_vsp1_routing(rgrp->dev);
 }
-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/5] arm64: dts: renesas: r8a77995: draak: Add backlight

[Laurent Pinchart]

Add the backlight device for the LVDS1 output, in preparation for panel
support.

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

diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index 2405eaad0296..cd067319e6f3 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -24,6 +24,17 @@
 		stdout-path = "serial0:115200n8";
 	};
 
+	backlight: backlight {
+		compatible = "pwm-backlight";
+		pwms = <&pwm1 0 50000>;
+
+		brightness-levels = <256 128 64 16 8 4 0>;
+		default-brightness-level = <6>;
+
+		power-supply = <&reg_12p0v>;
+		enable-gpios = <&gpio4 0 GPIO_ACTIVE_HIGH>;
+	};
+
 	composite-in {
 		compatible = "composite-video-connector";
 
@@ -104,6 +115,15 @@
 		regulator-always-on;
 	};
 
+	reg_12p0v: regulator1 {
+		compatible = "regulator-fixed";
+		regulator-name = "D12.0V";
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
 	vga {
 		compatible = "vga-connector";
 
-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/5] [HACK] arm64: dts: r8a77995: draak: Add panel to DT

[Laurent Pinchart]

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

diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index cd067319e6f3..140519e39f06 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -487,3 +487,14 @@
 		};
 	};
 };
+
+#define lvds_connector lvds1_out
+#include "../../../../arm/boot/dts/r8a77xx-aa104xd12-panel.dtsi"
+
+&lvds1 {
+	status = "okay";
+};
+
+&{/panel/} {
+	backlight = <&backlight>;
+};
-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/5] drm: rcar-du: Replace EXT_CTRL_REGS feature flag with generation check

[Simon Horman]

On Sun, Nov 25, 2018 at 04:40:27PM +0200, Laurent Pinchart wrote:
> The RCAR_DU_FEATURE_EXT_CTRL_REGS feature flag is missing for H1 only,
> which is a first generation device, not a second generation device as
> reported in the device information table. Fix the H1 generation and use
> generation checks to replace the feature flag.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/5] arm64: dts: renesas: r8a77995: draak: Add backlight

[Laurent Pinchart]

Hi Simon,

Could you please consider taking this patch in your tree ? It's independent 
from the rest of the series.

On Sunday, 25 November 2018 16:40:30 EET Laurent Pinchart wrote:
> Add the backlight device for the LVDS1 output, in preparation for panel
> support.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../arm64/boot/dts/renesas/r8a77995-draak.dts | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index
> 2405eaad0296..cd067319e6f3 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> @@ -24,6 +24,17 @@
>  		stdout-path = "serial0:115200n8";
>  	};
> 
> +	backlight: backlight {
> +		compatible = "pwm-backlight";
> +		pwms = <&pwm1 0 50000>;
> +
> +		brightness-levels = <256 128 64 16 8 4 0>;
> +		default-brightness-level = <6>;
> +
> +		power-supply = <&reg_12p0v>;
> +		enable-gpios = <&gpio4 0 GPIO_ACTIVE_HIGH>;
> +	};
> +
>  	composite-in {
>  		compatible = "composite-video-connector";
> 
> @@ -104,6 +115,15 @@
>  		regulator-always-on;
>  	};
> 
> +	reg_12p0v: regulator1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "D12.0V";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +	};
> +
>  	vga {
>  		compatible = "vga-connector";

-- 
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/5] arm64: dts: renesas: r8a77995: draak: Add backlight

[Geert Uytterhoeven]

On Sun, Nov 25, 2018 at 3:40 PM Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> Add the backlight device for the LVDS1 output, in preparation for panel
> support.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

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

Gr{oetje,eeting}s,

                        Geert

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

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/5] arm64: dts: renesas: r8a77995: draak: Add backlight

[Simon Horman]

Hi Laurent,

On Tue, Dec 04, 2018 at 06:57:10PM +0200, Laurent Pinchart wrote:
> Hi Simon,
> 
> Could you please consider taking this patch in your tree ? It's independent 
> from the rest of the series.

sure, applied for v4.21.

> 
> On Sunday, 25 November 2018 16:40:30 EET Laurent Pinchart wrote:
> > Add the backlight device for the LVDS1 output, in preparation for panel
> > support.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  .../arm64/boot/dts/renesas/r8a77995-draak.dts | 20 +++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index
> > 2405eaad0296..cd067319e6f3 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > @@ -24,6 +24,17 @@
> >  		stdout-path = "serial0:115200n8";
> >  	};
> > 
> > +	backlight: backlight {
> > +		compatible = "pwm-backlight";
> > +		pwms = <&pwm1 0 50000>;
> > +
> > +		brightness-levels = <256 128 64 16 8 4 0>;
> > +		default-brightness-level = <6>;
> > +
> > +		power-supply = <&reg_12p0v>;
> > +		enable-gpios = <&gpio4 0 GPIO_ACTIVE_HIGH>;
> > +	};
> > +
> >  	composite-in {
> >  		compatible = "composite-video-connector";
> > 
> > @@ -104,6 +115,15 @@
> >  		regulator-always-on;
> >  	};
> > 
> > +	reg_12p0v: regulator1 {
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "D12.0V";
> > +		regulator-min-microvolt = <12000000>;
> > +		regulator-max-microvolt = <12000000>;
> > +		regulator-boot-on;
> > +		regulator-always-on;
> > +	};
> > +
> >  	vga {
> >  		compatible = "vga-connector";
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> 
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/5] drm: rcar-du: Replace EXT_CTRL_REGS feature flag with generation check

[Kieran Bingham]

Hi Laurent,

Thank you for the patch,

On 25/11/2018 14:40, Laurent Pinchart wrote:
> The RCAR_DU_FEATURE_EXT_CTRL_REGS feature flag is missing for H1 only,
> which is a first generation device, not a second generation device as
> reported in the device information table. Fix the H1 generation and use
> generation checks to replace the feature flag.

Looks like a good simplification

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

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

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c   | 14 +-------------
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h   |  7 +++----
>  drivers/gpu/drm/rcar-du/rcar_du_group.c |  4 ++--
>  3 files changed, 6 insertions(+), 19 deletions(-)

-13 lines ... sounds like a (small) win :D


> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index 94f055186b95..814c1099dacb 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -35,7 +35,6 @@
>  static const struct rcar_du_device_info rzg1_du_r8a7743_info = {
>  	.gen = 2,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>  		  | RCAR_DU_FEATURE_INTERLACED
>  		  | RCAR_DU_FEATURE_TVM_SYNC,
>  	.channels_mask = BIT(1) | BIT(0),
> @@ -58,7 +57,6 @@ static const struct rcar_du_device_info rzg1_du_r8a7743_info = {
>  static const struct rcar_du_device_info rzg1_du_r8a7745_info = {
>  	.gen = 2,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>  		  | RCAR_DU_FEATURE_INTERLACED
>  		  | RCAR_DU_FEATURE_TVM_SYNC,
>  	.channels_mask = BIT(1) | BIT(0),
> @@ -80,7 +78,6 @@ static const struct rcar_du_device_info rzg1_du_r8a7745_info = {
>  static const struct rcar_du_device_info rzg1_du_r8a77470_info = {
>  	.gen = 2,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>  		  | RCAR_DU_FEATURE_INTERLACED
>  		  | RCAR_DU_FEATURE_TVM_SYNC,
>  	.channels_mask = BIT(1) | BIT(0),
> @@ -105,7 +102,7 @@ static const struct rcar_du_device_info rzg1_du_r8a77470_info = {
>  };
>  
>  static const struct rcar_du_device_info rcar_du_r8a7779_info = {
> -	.gen = 2,
> +	.gen = 1,
>  	.features = RCAR_DU_FEATURE_INTERLACED
>  		  | RCAR_DU_FEATURE_TVM_SYNC,
>  	.channels_mask = BIT(1) | BIT(0),
> @@ -128,7 +125,6 @@ static const struct rcar_du_device_info rcar_du_r8a7779_info = {
>  static const struct rcar_du_device_info rcar_du_r8a7790_info = {
>  	.gen = 2,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>  		  | RCAR_DU_FEATURE_INTERLACED
>  		  | RCAR_DU_FEATURE_TVM_SYNC,
>  	.quirks = RCAR_DU_QUIRK_ALIGN_128B,
> @@ -158,7 +154,6 @@ static const struct rcar_du_device_info rcar_du_r8a7790_info = {
>  static const struct rcar_du_device_info rcar_du_r8a7791_info = {
>  	.gen = 2,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>  		  | RCAR_DU_FEATURE_INTERLACED
>  		  | RCAR_DU_FEATURE_TVM_SYNC,
>  	.channels_mask = BIT(1) | BIT(0),
> @@ -182,7 +177,6 @@ static const struct rcar_du_device_info rcar_du_r8a7791_info = {
>  static const struct rcar_du_device_info rcar_du_r8a7792_info = {
>  	.gen = 2,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>  		  | RCAR_DU_FEATURE_INTERLACED
>  		  | RCAR_DU_FEATURE_TVM_SYNC,
>  	.channels_mask = BIT(1) | BIT(0),
> @@ -202,7 +196,6 @@ static const struct rcar_du_device_info rcar_du_r8a7792_info = {
>  static const struct rcar_du_device_info rcar_du_r8a7794_info = {
>  	.gen = 2,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>  		  | RCAR_DU_FEATURE_INTERLACED
>  		  | RCAR_DU_FEATURE_TVM_SYNC,
>  	.channels_mask = BIT(1) | BIT(0),
> @@ -225,7 +218,6 @@ static const struct rcar_du_device_info rcar_du_r8a7794_info = {
>  static const struct rcar_du_device_info rcar_du_r8a7795_info = {
>  	.gen = 3,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>  		  | RCAR_DU_FEATURE_VSP1_SOURCE
>  		  | RCAR_DU_FEATURE_INTERLACED
>  		  | RCAR_DU_FEATURE_TVM_SYNC,
> @@ -259,7 +251,6 @@ static const struct rcar_du_device_info rcar_du_r8a7795_info = {
>  static const struct rcar_du_device_info rcar_du_r8a7796_info = {
>  	.gen = 3,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>  		  | RCAR_DU_FEATURE_VSP1_SOURCE
>  		  | RCAR_DU_FEATURE_INTERLACED
>  		  | RCAR_DU_FEATURE_TVM_SYNC,
> @@ -289,7 +280,6 @@ static const struct rcar_du_device_info rcar_du_r8a7796_info = {
>  static const struct rcar_du_device_info rcar_du_r8a77965_info = {
>  	.gen = 3,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>  		  | RCAR_DU_FEATURE_VSP1_SOURCE
>  		  | RCAR_DU_FEATURE_INTERLACED
>  		  | RCAR_DU_FEATURE_TVM_SYNC,
> @@ -319,7 +309,6 @@ static const struct rcar_du_device_info rcar_du_r8a77965_info = {
>  static const struct rcar_du_device_info rcar_du_r8a77970_info = {
>  	.gen = 3,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>  		  | RCAR_DU_FEATURE_VSP1_SOURCE
>  		  | RCAR_DU_FEATURE_INTERLACED
>  		  | RCAR_DU_FEATURE_TVM_SYNC,
> @@ -341,7 +330,6 @@ static const struct rcar_du_device_info rcar_du_r8a77970_info = {
>  static const struct rcar_du_device_info rcar_du_r8a7799x_info = {
>  	.gen = 3,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>  		  | RCAR_DU_FEATURE_VSP1_SOURCE,
>  	.channels_mask = BIT(1) | BIT(0),
>  	.routes = {
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> index 9f5563296c5a..8ef9165957cb 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -24,10 +24,9 @@ struct drm_fbdev_cma;
>  struct rcar_du_device;
>  
>  #define RCAR_DU_FEATURE_CRTC_IRQ_CLOCK	BIT(0)	/* Per-CRTC IRQ and clock */
> -#define RCAR_DU_FEATURE_EXT_CTRL_REGS	BIT(1)	/* Has extended control registers */
> -#define RCAR_DU_FEATURE_VSP1_SOURCE	BIT(2)	/* Has inputs from VSP1 */
> -#define RCAR_DU_FEATURE_INTERLACED	BIT(3)	/* HW supports interlaced */
> -#define RCAR_DU_FEATURE_TVM_SYNC	BIT(4)	/* Has TV switch/sync modes */
> +#define RCAR_DU_FEATURE_VSP1_SOURCE	BIT(1)	/* Has inputs from VSP1 */
> +#define RCAR_DU_FEATURE_INTERLACED	BIT(2)	/* HW supports interlaced */
> +#define RCAR_DU_FEATURE_TVM_SYNC	BIT(3)	/* Has TV switch/sync modes */
>  
>  #define RCAR_DU_QUIRK_ALIGN_128B	BIT(0)	/* Align pitches to 128 bytes */
>  
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> index cebf313c6e1f..3366cda6086c 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -147,7 +147,7 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>  
>  	rcar_du_group_setup_pins(rgrp);
>  
> -	if (rcar_du_has(rgrp->dev, RCAR_DU_FEATURE_EXT_CTRL_REGS)) {
> +	if (rcdu->info->gen >= 2) {
>  		rcar_du_group_setup_defr8(rgrp);
>  		rcar_du_group_setup_didsr(rgrp);
>  	}
> @@ -262,7 +262,7 @@ int rcar_du_set_dpad0_vsp1_routing(struct rcar_du_device *rcdu)
>  	unsigned int index;
>  	int ret;
>  
> -	if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_EXT_CTRL_REGS))
> +	if (rcdu->info->gen < 2)
>  		return 0;
>  
>  	/*
> 

-- 
Regards
--
Kieran
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/5] drm: rcar-du: Move CRTC outputs bitmask to private CRTC state

[Kieran Bingham]

Hi Laurent,

Thank you for the patch,

On 25/11/2018 14:40, Laurent Pinchart wrote:
> The rcar_du_crtc outputs field stores a bitmask of the outputs driven by
> the CRTC. This changes based on the configuration requested by
> userspace, and is used for the sole purpose of configuring the hardware.
> The field thus belongs to the CRTC state. Move it to the
> rcar_du_crtc_state structure.
> 
> As a result the rcar_du_crtc_route_output() function loses most of its
> purpose. In order to remove it, move dpad0_source calculation to
> rcar_du_atomic_commit_tail(), until the field gets moved to a state
> structure. In order to simplify the rcar_du_group_set_routing()
> implementation, we also store the DPAD1 source in a new dpad1_source
> field which will move to a state structure with dpad0_source.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

that was a fairly tough read - but aside from one comment near the
bottom regarding initialising dpad0 which I'm sure you can handle
correctly, and another comment which I think we could improve things
outside of this patch:

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


> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c    | 42 +++++++++++------------
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.h    |  7 ++--
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h     |  1 +
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 10 ------
>  drivers/gpu/drm/rcar-du/rcar_du_group.c   |  4 +--
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c     | 22 ++++++++++++
>  6 files changed, 47 insertions(+), 39 deletions(-)

+8 ... It's a good job you bought -13 lines in the previous patch ;)



> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 90dacab67be5..40b7f17159b0 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -22,6 +22,7 @@
>  
>  #include "rcar_du_crtc.h"
>  #include "rcar_du_drv.h"
> +#include "rcar_du_encoder.h"
>  #include "rcar_du_kms.h"
>  #include "rcar_du_plane.h"
>  #include "rcar_du_regs.h"
> @@ -316,26 +317,6 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
>  	rcar_du_crtc_write(rcrtc, DEWR,  mode->hdisplay);
>  }
>  
> -void rcar_du_crtc_route_output(struct drm_crtc *crtc,
> -			       enum rcar_du_output output)
> -{
> -	struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> -	struct rcar_du_device *rcdu = rcrtc->group->dev;
> -
> -	/*
> -	 * Store the route from the CRTC output to the DU output. The DU will be
> -	 * configured when starting the CRTC.
> -	 */
> -	rcrtc->outputs |= BIT(output);
> -
> -	/*
> -	 * Store RGB routing to DPAD0, the hardware will be configured when
> -	 * starting the CRTC.
> -	 */
> -	if (output == RCAR_DU_OUTPUT_DPAD0)
> -		rcdu->dpad0_source = rcrtc->index;
> -}
> -
>  static unsigned int plane_zpos(struct rcar_du_plane *plane)
>  {
>  	return plane->plane.state->normalized_zpos;
> @@ -655,6 +636,24 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
>   * CRTC Functions
>   */
>  
> +static int rcar_du_crtc_atomic_check(struct drm_crtc *crtc,
> +				     struct drm_crtc_state *state)
> +{
> +	struct rcar_du_crtc_state *rstate = to_rcar_crtc_state(state);
> +	struct drm_encoder *encoder;
> +
> +	/* Store the routes from the CRTC output to the DU outputs. */
> +	rstate->outputs = 0;
> +
> +	drm_for_each_encoder_mask(encoder, crtc->dev, state->encoder_mask) {
> +		struct rcar_du_encoder *renc = to_rcar_encoder(encoder);
> +
> +		rstate->outputs |= BIT(renc->output);
> +	}
> +
> +	return 0;
> +}
> +
>  static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
>  				       struct drm_crtc_state *old_state)
>  {
> @@ -678,8 +677,6 @@ static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc,
>  		crtc->state->event = NULL;
>  	}
>  	spin_unlock_irq(&crtc->dev->event_lock);
> -
> -	rcrtc->outputs = 0;
>  }
>  
>  static void rcar_du_crtc_atomic_begin(struct drm_crtc *crtc,
> @@ -755,6 +752,7 @@ enum drm_mode_status rcar_du_crtc_mode_valid(struct drm_crtc *crtc,
>  }
>  
>  static const struct drm_crtc_helper_funcs crtc_helper_funcs = {
> +	.atomic_check = rcar_du_crtc_atomic_check,
>  	.atomic_begin = rcar_du_crtc_atomic_begin,
>  	.atomic_flush = rcar_du_crtc_atomic_flush,
>  	.atomic_enable = rcar_du_crtc_atomic_enable,
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> index 59ac6e7d22c9..ec47f164e69b 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> @@ -37,7 +37,6 @@ struct rcar_du_vsp;
>   * @vblank_lock: protects vblank_wait and vblank_count
>   * @vblank_wait: wait queue used to signal vertical blanking
>   * @vblank_count: number of vertical blanking interrupts to wait for
> - * @outputs: bitmask of the outputs (enum rcar_du_output) driven by this CRTC
>   * @group: CRTC group this CRTC belongs to
>   * @vsp: VSP feeding video to this CRTC
>   * @vsp_pipe: index of the VSP pipeline feeding video to this CRTC
> @@ -61,8 +60,6 @@ struct rcar_du_crtc {
>  	wait_queue_head_t vblank_wait;
>  	unsigned int vblank_count;
>  
> -	unsigned int outputs;
> -
>  	struct rcar_du_group *group;
>  	struct rcar_du_vsp *vsp;
>  	unsigned int vsp_pipe;
> @@ -77,11 +74,13 @@ struct rcar_du_crtc {
>   * struct rcar_du_crtc_state - Driver-specific CRTC state
>   * @state: base DRM CRTC state
>   * @crc: CRC computation configuration
> + * @outputs: bitmask of the outputs (enum rcar_du_output) driven by this CRTC
>   */
>  struct rcar_du_crtc_state {
>  	struct drm_crtc_state state;
>  
>  	struct vsp1_du_crc_config crc;
> +	unsigned int outputs;
>  };
>  
>  #define to_rcar_crtc_state(s) container_of(s, struct rcar_du_crtc_state, state)
> @@ -102,8 +101,6 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int swindex,
>  void rcar_du_crtc_suspend(struct rcar_du_crtc *rcrtc);
>  void rcar_du_crtc_resume(struct rcar_du_crtc *rcrtc);
>  
> -void rcar_du_crtc_route_output(struct drm_crtc *crtc,
> -			       enum rcar_du_output output);
>  void rcar_du_crtc_finish_page_flip(struct rcar_du_crtc *rcrtc);
>  
>  void rcar_du_crtc_dsysr_clr_set(struct rcar_du_crtc *rcrtc, u32 clr, u32 set);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> index 8ef9165957cb..8b47b8546fc8 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -90,6 +90,7 @@ struct rcar_du_device {
>  	} props;
>  
>  	unsigned int dpad0_source;
> +	unsigned int dpad1_source;
>  	unsigned int vspd1_sink;
>  };
>  
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> index 1877764bd6d9..a123c28ea6ed 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> @@ -22,17 +22,7 @@
>   * Encoder
>   */
>  
> -static void rcar_du_encoder_mode_set(struct drm_encoder *encoder,
> -				     struct drm_crtc_state *crtc_state,
> -				     struct drm_connector_state *conn_state)
> -{
> -	struct rcar_du_encoder *renc = to_rcar_encoder(encoder);
> -
> -	rcar_du_crtc_route_output(crtc_state->crtc, renc->output);
> -}
> -
>  static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
> -	.atomic_mode_set = rcar_du_encoder_mode_set,
>  };
>  
>  static const struct drm_encoder_funcs encoder_funcs = {
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> index 3366cda6086c..7e440f61977f 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -289,7 +289,7 @@ int rcar_du_set_dpad0_vsp1_routing(struct rcar_du_device *rcdu)
>  
>  int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
>  {
> -	struct rcar_du_crtc *crtc0 = &rgrp->dev->crtcs[rgrp->index * 2];
> +	struct rcar_du_device *rcdu = rgrp->dev;
>  	u32 dorcr = rcar_du_group_read(rgrp, DORCR);
>  
>  	dorcr &= ~(DORCR_PG2T | DORCR_DK2S | DORCR_PG2D_MASK);
> @@ -299,7 +299,7 @@ int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
>  	 * CRTC 1 in all other cases to avoid cloning CRTC 0 to DPAD0 and DPAD1
>  	 * by default.
>  	 */
> -	if (crtc0->outputs & BIT(RCAR_DU_OUTPUT_DPAD1))
> +	if (rcdu->dpad1_source == rgrp->index * 2)

The magic of getting (rgrp->index * 2) is a little bit ... magic ...

The what is happening ( ({0,1} => {0,2}) ) is clear - but not the why.

This happens a bit throughout as a means to convert from CRTC to Group
indexes, and back. But it could be clearer with a helper.
	(not in this patch)


>  		dorcr |= DORCR_PG2D_DS1;
>  	else
>  		dorcr |= DORCR_PG2T | DORCR_DK2S | DORCR_PG2D_DS2;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index fe6f65c94eef..bd3c2c5ea66f 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -285,6 +285,28 @@ static int rcar_du_atomic_check(struct drm_device *dev,
>  static void rcar_du_atomic_commit_tail(struct drm_atomic_state *old_state)
>  {
>  	struct drm_device *dev = old_state->dev;
> +	struct rcar_du_device *rcdu = dev->dev_private;
> +	struct drm_crtc_state *crtc_state;
> +	struct drm_crtc *crtc;
> +	unsigned int i;
> +
> +	/*
> +	 * Store RGB routing to DPAD0 and DPAD1, the hardware will be configured
> +	 * when starting the CRTCs.
> +	 */
> +	rcdu->dpad1_source = -1;

Why do we initialise dpad1_source but not dpad0_source?

Are we *guaranteed* to always have RCAR_DU_OUTPUT_DPAD0 set in one of
the rcrtc_state->outputs ?

If so - then this isn't an issue.


> +
> +	for_each_new_crtc_in_state(old_state, crtc, crtc_state, i) {
> +		struct rcar_du_crtc_state *rcrtc_state =
> +			to_rcar_crtc_state(crtc_state);
> +		struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> +
> +		if (rcrtc_state->outputs & BIT(RCAR_DU_OUTPUT_DPAD0))
> +			rcdu->dpad0_source = rcrtc->index;
> +
> +		if (rcrtc_state->outputs & BIT(RCAR_DU_OUTPUT_DPAD1))
> +			rcdu->dpad1_source = rcrtc->index;
> +	}
>  
>  	/* Apply the atomic update. */
>  	drm_atomic_helper_commit_modeset_disables(dev, old_state);
> 

-- 
Regards
--
Kieran
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/5] drm: rcar-du: Disable unused DPAD outputs

[Kieran Bingham]

Hi Laurent,

Thank you for the patch,

On 25/11/2018 14:40, Laurent Pinchart wrote:
> DU channels are routed to DPAD outputs in an SoC-dependent way. The
> routing can be fixed (e.g. DU3 to DPAD0 on H3) or configurable (e.g. DU0
> or DU1 to DPAD0 on D3/E3). The hardware offers no option to disconnect
> DPAD outputs, which are thus always driven by a DU channel.
> 
> On SoCs that have less DU channels than DU outputs, such as D3 and E3,
> the DPAD output is always driven when all channels are in used by other

s/used/use/


> outputs (such as the internal LVDS and HDMI encoders). This creates an
> unwanted clone on the DPAD output.
> 
> However, the parallel output of the DU channels routed to DPAD can be
> set to fixed levels in the DU channels themselves through the DOFLR
> group register. Use this to turn the DPAD on or off by driving fixed
> signals at the output of any DU channel not routed to a DPAD output.
> This doesn't affect the DU output signals going to other outputs.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Only spelling and bikeshedding here - so:

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

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_group.c | 42 +++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> index 7e440f61977f..5aaf41b7a2ca 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -287,6 +287,46 @@ int rcar_du_set_dpad0_vsp1_routing(struct rcar_du_device *rcdu)
>  	return 0;
>  }
>  
> +static void rcar_du_group_set_output_levels(struct rcar_du_group *rgrp)
> +{
> +	static const u32 doflr_values[2] = {
> +		DOFLR_HSYCFL0 | DOFLR_VSYCFL0 | DOFLR_ODDFL0 |
> +		DOFLR_DISPFL0 | DOFLR_CDEFL0  | DOFLR_RGBFL0,
> +		DOFLR_HSYCFL1 | DOFLR_VSYCFL1 | DOFLR_ODDFL1 |
> +		DOFLR_DISPFL1 | DOFLR_CDEFL1  | DOFLR_RGBFL1,
> +	};
> +	static const u32 dpad_mask = BIT(RCAR_DU_OUTPUT_DPAD1)
> +				   | BIT(RCAR_DU_OUTPUT_DPAD0);
> +	struct rcar_du_device *rcdu = rgrp->dev;
> +	u32 doflr = DOFLR_CODE;
> +	unsigned int i;
> +
> +	if (rcdu->info->gen < 2)
> +		return;
> +
> +	/*
> +	 * The DPAD outputs can't be controlled directly. However, the parallel
> +	 * output of the DU channels routed to DPAD can be set to fixed levels
> +	 * through the DOFLR group register. Use this to turn the DPAD on or off
> +	 * by driving fixed signals at the output of any DU channel not routed
> +	 * to a DPAD output. This doesn't affect the DU output signals going to
> +	 * other outputs, such as the internal LVDS and HDMI encoders.

Perhaps more out of interest - what /fixed/ levels do we output.
High/Low/Hi-Z ?


> +	 */
> +
> +	for (i = 0; i < rgrp->num_crtcs; ++i) {
> +		struct rcar_du_crtc_state *rstate;
> +		struct rcar_du_crtc *rcrtc;
> +
> +		rcrtc = &rcdu->crtcs[rgrp->index * 2 + i];> +		rstate = to_rcar_crtc_state(rcrtc->crtc.state);
> +
> +		if (!(rstate->outputs & dpad_mask))
> +			doflr |= doflr_values[i];> +	}
> +
> +	rcar_du_group_write(rgrp, DOFLR, doflr);
> +}
> +
>  int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
>  {
>  	struct rcar_du_device *rcdu = rgrp->dev;
> @@ -306,5 +346,7 @@ int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
>  
>  	rcar_du_group_write(rgrp, DORCR, dorcr);
>  
> +	rcar_du_group_set_output_levels(rgrp);

Shouldn't this be:

  rcar_du_group_set_dpad_levels()

Anyway - that's just bikeshedding - I'll leave the decision (even if
that's keeping this as is) to you.

> +
>  	return rcar_du_set_dpad0_vsp1_routing(rgrp->dev);
>  }
> 

-- 
Regards
--
Kieran
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/5] drm: rcar-du: Disable unused DPAD outputs

[Laurent Pinchart]

Hi Kieran,

On Friday, 7 December 2018 14:50:47 EET Kieran Bingham wrote:
> On 25/11/2018 14:40, Laurent Pinchart wrote:
> > DU channels are routed to DPAD outputs in an SoC-dependent way. The
> > routing can be fixed (e.g. DU3 to DPAD0 on H3) or configurable (e.g. DU0
> > or DU1 to DPAD0 on D3/E3). The hardware offers no option to disconnect
> > DPAD outputs, which are thus always driven by a DU channel.
> > 
> > On SoCs that have less DU channels than DU outputs, such as D3 and E3,
> > the DPAD output is always driven when all channels are in used by other
> 
> s/used/use/
> 
> > outputs (such as the internal LVDS and HDMI encoders). This creates an
> > unwanted clone on the DPAD output.
> > 
> > However, the parallel output of the DU channels routed to DPAD can be
> > set to fixed levels in the DU channels themselves through the DOFLR
> > group register. Use this to turn the DPAD on or off by driving fixed
> > signals at the output of any DU channel not routed to a DPAD output.
> > This doesn't affect the DU output signals going to other outputs.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> 
> Only spelling and bikeshedding here - so:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> > ---
> > 
> >  drivers/gpu/drm/rcar-du/rcar_du_group.c | 42 +++++++++++++++++++++++++
> >  1 file changed, 42 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_group.c index
> > 7e440f61977f..5aaf41b7a2ca 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> > @@ -287,6 +287,46 @@ int rcar_du_set_dpad0_vsp1_routing(struct
> > rcar_du_device *rcdu)
> >  	return 0;
> >  }
> > 
> > +static void rcar_du_group_set_output_levels(struct rcar_du_group *rgrp)
> > +{
> > +	static const u32 doflr_values[2] = {
> > +		DOFLR_HSYCFL0 | DOFLR_VSYCFL0 | DOFLR_ODDFL0 |
> > +		DOFLR_DISPFL0 | DOFLR_CDEFL0  | DOFLR_RGBFL0,
> > +		DOFLR_HSYCFL1 | DOFLR_VSYCFL1 | DOFLR_ODDFL1 |
> > +		DOFLR_DISPFL1 | DOFLR_CDEFL1  | DOFLR_RGBFL1,
> > +	};
> > +	static const u32 dpad_mask = BIT(RCAR_DU_OUTPUT_DPAD1)
> > +				   | BIT(RCAR_DU_OUTPUT_DPAD0);
> > +	struct rcar_du_device *rcdu = rgrp->dev;
> > +	u32 doflr = DOFLR_CODE;
> > +	unsigned int i;
> > +
> > +	if (rcdu->info->gen < 2)
> > +		return;
> > +
> > +	/*
> > +	 * The DPAD outputs can't be controlled directly. However, the parallel
> > +	 * output of the DU channels routed to DPAD can be set to fixed levels
> > +	 * through the DOFLR group register. Use this to turn the DPAD on or 
off
> > +	 * by driving fixed signals at the output of any DU channel not routed
> > +	 * to a DPAD output. This doesn't affect the DU output signals going to
> > +	 * other outputs, such as the internal LVDS and HDMI encoders.
> 
> Perhaps more out of interest - what /fixed/ levels do we output.
> High/Low/Hi-Z ?

It's low-level, I'll update the comment.

> > +	 */
> > +
> > +	for (i = 0; i < rgrp->num_crtcs; ++i) {
> > +		struct rcar_du_crtc_state *rstate;
> > +		struct rcar_du_crtc *rcrtc;
> > +
> > +		rcrtc = &rcdu->crtcs[rgrp->index * 2 + i];> +		rstate =
> > to_rcar_crtc_state(rcrtc->crtc.state); +
> > +		if (!(rstate->outputs & dpad_mask))
> > +			doflr |= doflr_values[i];> +	}
> > +
> > +	rcar_du_group_write(rgrp, DOFLR, doflr);
> > +}
> > +
> > 
> >  int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
> >  {
> >  
> >  	struct rcar_du_device *rcdu = rgrp->dev;
> > 
> > @@ -306,5 +346,7 @@ int rcar_du_group_set_routing(struct rcar_du_group
> > *rgrp)> 
> >  	rcar_du_group_write(rgrp, DORCR, dorcr);
> > 
> > +	rcar_du_group_set_output_levels(rgrp);
> 
> Shouldn't this be:
> 
>   rcar_du_group_set_dpad_levels()
> 
> Anyway - that's just bikeshedding - I'll leave the decision (even if
> that's keeping this as is) to you.

Good idea, I'll rename the function.

> > +
> > 
> >  	return rcar_du_set_dpad0_vsp1_routing(rgrp->dev);
> >  
> >  }


-- 
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/5] drm: rcar-du: Move CRTC outputs bitmask to private CRTC state

[Laurent Pinchart]

Hi Kieran,

On Friday, 7 December 2018 14:34:58 EET Kieran Bingham wrote:
> On 25/11/2018 14:40, Laurent Pinchart wrote:
> > The rcar_du_crtc outputs field stores a bitmask of the outputs driven by
> > the CRTC. This changes based on the configuration requested by
> > userspace, and is used for the sole purpose of configuring the hardware.
> > The field thus belongs to the CRTC state. Move it to the
> > rcar_du_crtc_state structure.
> > 
> > As a result the rcar_du_crtc_route_output() function loses most of its
> > purpose. In order to remove it, move dpad0_source calculation to
> > rcar_du_atomic_commit_tail(), until the field gets moved to a state
> > structure. In order to simplify the rcar_du_group_set_routing()
> > implementation, we also store the DPAD1 source in a new dpad1_source
> > field which will move to a state structure with dpad0_source.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> 
> that was a fairly tough read - but aside from one comment near the
> bottom regarding initialising dpad0 which I'm sure you can handle
> correctly, and another comment which I think we could improve things
> outside of this patch:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> > ---
> > 
> >  drivers/gpu/drm/rcar-du/rcar_du_crtc.c    | 42 +++++++++++------------
> >  drivers/gpu/drm/rcar-du/rcar_du_crtc.h    |  7 ++--
> >  drivers/gpu/drm/rcar-du/rcar_du_drv.h     |  1 +
> >  drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 10 ------
> >  drivers/gpu/drm/rcar-du/rcar_du_group.c   |  4 +--
> >  drivers/gpu/drm/rcar-du/rcar_du_kms.c     | 22 ++++++++++++
> >  6 files changed, 47 insertions(+), 39 deletions(-)
> 
> +8 ... It's a good job you bought -13 lines in the previous patch ;)
> 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 90dacab67be5..40b7f17159b0
> > 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > @@ -22,6 +22,7 @@
> > 
> >  #include "rcar_du_crtc.h"
> >  #include "rcar_du_drv.h"
> > +#include "rcar_du_encoder.h"
> >  #include "rcar_du_kms.h"
> >  #include "rcar_du_plane.h"
> >  #include "rcar_du_regs.h"
> > @@ -316,26 +317,6 @@ static void rcar_du_crtc_set_display_timing(struct
> > rcar_du_crtc *rcrtc)
> >  	rcar_du_crtc_write(rcrtc, DEWR,  mode->hdisplay);
> >  }
> > 
> > -void rcar_du_crtc_route_output(struct drm_crtc *crtc,
> > -			       enum rcar_du_output output)
> > -{
> > -	struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> > -	struct rcar_du_device *rcdu = rcrtc->group->dev;
> > -
> > -	/*
> > -	 * Store the route from the CRTC output to the DU output. The DU will 
be
> > -	 * configured when starting the CRTC.
> > -	 */
> > -	rcrtc->outputs |= BIT(output);
> > -
> > -	/*
> > -	 * Store RGB routing to DPAD0, the hardware will be configured when
> > -	 * starting the CRTC.
> > -	 */
> > -	if (output == RCAR_DU_OUTPUT_DPAD0)
> > -		rcdu->dpad0_source = rcrtc->index;
> > -}
> > -
> >  static unsigned int plane_zpos(struct rcar_du_plane *plane)
> >  {
> >  	return plane->plane.state->normalized_zpos;
> > @@ -655,6 +636,24 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc
> > *rcrtc)
> >   * CRTC Functions
> >   */
> > 
> > +static int rcar_du_crtc_atomic_check(struct drm_crtc *crtc,
> > +				     struct drm_crtc_state *state)
> > +{
> > +	struct rcar_du_crtc_state *rstate = to_rcar_crtc_state(state);
> > +	struct drm_encoder *encoder;
> > +
> > +	/* Store the routes from the CRTC output to the DU outputs. */
> > +	rstate->outputs = 0;
> > +
> > +	drm_for_each_encoder_mask(encoder, crtc->dev, state->encoder_mask) {
> > +		struct rcar_du_encoder *renc = to_rcar_encoder(encoder);
> > +
> > +		rstate->outputs |= BIT(renc->output);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
> >  				       struct drm_crtc_state *old_state)
> >  {
> > @@ -678,8 +677,6 @@ static void rcar_du_crtc_atomic_disable(struct
> > drm_crtc *crtc,
> >  		crtc->state->event = NULL;
> >  	}
> >  	spin_unlock_irq(&crtc->dev->event_lock);
> > -
> > -	rcrtc->outputs = 0;
> >  }
> >  
> >  static void rcar_du_crtc_atomic_begin(struct drm_crtc *crtc,
> > @@ -755,6 +752,7 @@ enum drm_mode_status rcar_du_crtc_mode_valid(struct
> > drm_crtc *crtc,
> >  }
> >  
> >  static const struct drm_crtc_helper_funcs crtc_helper_funcs = {
> > +	.atomic_check = rcar_du_crtc_atomic_check,
> >  	.atomic_begin = rcar_du_crtc_atomic_begin,
> >  	.atomic_flush = rcar_du_crtc_atomic_flush,
> >  	.atomic_enable = rcar_du_crtc_atomic_enable,
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h index 59ac6e7d22c9..ec47f164e69b
> > 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> > @@ -37,7 +37,6 @@ struct rcar_du_vsp;
> >   * @vblank_lock: protects vblank_wait and vblank_count
> >   * @vblank_wait: wait queue used to signal vertical blanking
> >   * @vblank_count: number of vertical blanking interrupts to wait for
> > - * @outputs: bitmask of the outputs (enum rcar_du_output) driven by this
> > CRTC
> >   * @group: CRTC group this CRTC belongs to
> >   * @vsp: VSP feeding video to this CRTC
> >   * @vsp_pipe: index of the VSP pipeline feeding video to this CRTC
> > @@ -61,8 +60,6 @@ struct rcar_du_crtc {
> >  	wait_queue_head_t vblank_wait;
> >  	unsigned int vblank_count;
> > 
> > -	unsigned int outputs;
> > -
> >  	struct rcar_du_group *group;
> >  	struct rcar_du_vsp *vsp;
> >  	unsigned int vsp_pipe;
> > 
> > @@ -77,11 +74,13 @@ struct rcar_du_crtc {
> >   * struct rcar_du_crtc_state - Driver-specific CRTC state
> >   * @state: base DRM CRTC state
> >   * @crc: CRC computation configuration
> > + * @outputs: bitmask of the outputs (enum rcar_du_output) driven by this
> > CRTC
> >   */
> >  struct rcar_du_crtc_state {
 >  	struct drm_crtc_state state;
> >  	struct vsp1_du_crc_config crc;
> > +	unsigned int outputs;
> >  };
> >  
> >  #define to_rcar_crtc_state(s) container_of(s, struct rcar_du_crtc_state,
> >  state)
> > @@ -102,8 +101,6 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
> > unsigned int swindex,
> >  void rcar_du_crtc_suspend(struct rcar_du_crtc *rcrtc);
> >  void rcar_du_crtc_resume(struct rcar_du_crtc *rcrtc);
> > 
> > -void rcar_du_crtc_route_output(struct drm_crtc *crtc,
> > -			       enum rcar_du_output output);
> >  void rcar_du_crtc_finish_page_flip(struct rcar_du_crtc *rcrtc);
> >  
> >  void rcar_du_crtc_dsysr_clr_set(struct rcar_du_crtc *rcrtc, u32 clr, u32
> >  set);
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> > b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index 8ef9165957cb..8b47b8546fc8
> > 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> > @@ -90,6 +90,7 @@ struct rcar_du_device {
> >  	} props;
> >  	
> >  	unsigned int dpad0_source;
> > +	unsigned int dpad1_source;
> >  	unsigned int vspd1_sink;
> >  };
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c index
> > 1877764bd6d9..a123c28ea6ed 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> > @@ -22,17 +22,7 @@
> >   * Encoder
> >   */
> > 
> > -static void rcar_du_encoder_mode_set(struct drm_encoder *encoder,
> > -				     struct drm_crtc_state *crtc_state,
> > -				     struct drm_connector_state *conn_state)
> > -{
> > -	struct rcar_du_encoder *renc = to_rcar_encoder(encoder);
> > -
> > -	rcar_du_crtc_route_output(crtc_state->crtc, renc->output);
> > -}
> > -
> >  static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
> > -	.atomic_mode_set = rcar_du_encoder_mode_set,
> >  };
> >  
> >  static const struct drm_encoder_funcs encoder_funcs = {
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_group.c index
> > 3366cda6086c..7e440f61977f 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> > @@ -289,7 +289,7 @@ int rcar_du_set_dpad0_vsp1_routing(struct
> > rcar_du_device *rcdu)
> >  int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
> >  {
> > 
> > -	struct rcar_du_crtc *crtc0 = &rgrp->dev->crtcs[rgrp->index * 2];
> > +	struct rcar_du_device *rcdu = rgrp->dev;
> >  	u32 dorcr = rcar_du_group_read(rgrp, DORCR);
> >  	
> >  	dorcr &= ~(DORCR_PG2T | DORCR_DK2S | DORCR_PG2D_MASK);
> > @@ -299,7 +299,7 @@ int rcar_du_group_set_routing(struct rcar_du_group
> > *rgrp)
> >  	 * CRTC 1 in all other cases to avoid cloning CRTC 0 to DPAD0 and DPAD1
> >  	 * by default.
> >  	 */
> > -	if (crtc0->outputs & BIT(RCAR_DU_OUTPUT_DPAD1))
> > +	if (rcdu->dpad1_source == rgrp->index * 2)
> 
> The magic of getting (rgrp->index * 2) is a little bit ... magic ...
> 
> The what is happening ( ({0,1} => {0,2}) ) is clear - but not the why.
> 
> This happens a bit throughout as a means to convert from CRTC to Group
> indexes, and back. But it could be clearer with a helper.
> 	(not in this patch)

I agree. Let's see what happens first, a helper function or a complete rewrite 
of group handling based on atomic states :-)

> >  		dorcr |= DORCR_PG2D_DS1;
> >  	else
> >  		dorcr |= DORCR_PG2T | DORCR_DK2S | DORCR_PG2D_DS2;
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index fe6f65c94eef..bd3c2c5ea66f
> > 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > @@ -285,6 +285,28 @@ static int rcar_du_atomic_check(struct drm_device
> > *dev,
> >  static void rcar_du_atomic_commit_tail(struct drm_atomic_state
> >  *old_state)
> >  {
> >  	struct drm_device *dev = old_state->dev;
> > +	struct rcar_du_device *rcdu = dev->dev_private;
> > +	struct drm_crtc_state *crtc_state;
> > +	struct drm_crtc *crtc;
> > +	unsigned int i;
> > +
> > +	/*
> > +	 * Store RGB routing to DPAD0 and DPAD1, the hardware will be 
configured
> > +	 * when starting the CRTCs.
> > +	 */
> > +	rcdu->dpad1_source = -1;
> 
> Why do we initialise dpad1_source but not dpad0_source?
> 
> Are we *guaranteed* to always have RCAR_DU_OUTPUT_DPAD0 set in one of
> the rcrtc_state->outputs ?
> 
> If so - then this isn't an issue.

No, there's no guarantee, but it's still not an issue :-) If none of the CRTCs 
in this commit output to DPAD0, the value of rcdu->dpad0_source will remain 
unchanged, which is what we want.

For DPAD1 the situation is a bit different. The only R-Car SoCs to have DPAD1 
are H1, V2H and E2, and they all have fixed output routing (DU0 to DPAD0 and 
DU1 to DPAD1). There's however a way to route the output of the composer 
inside DU0 to the DU1 output, and vice-versa. The DU driver uses this feature 
to configure the routing of the DPAD1 on H1 only (I don't remember the 
historical reason, or perhaps the historical mistake). We need to only route 
DU0 to DPAD1 when explicitly requested (as explained in 
rcar_du_group_set_routing()) and DU1 to DPAD1 in all other cases. Initializing 
dpad1_source to -1 will ensure that.

The group code really needs to be rewritten to use atomic states (and I wonder 
whether the DU0 -> DPAD1 routing should be kept).

> > +
> > +	for_each_new_crtc_in_state(old_state, crtc, crtc_state, i) {
> > +		struct rcar_du_crtc_state *rcrtc_state =
> > +			to_rcar_crtc_state(crtc_state);
> > +		struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> > +
> > +		if (rcrtc_state->outputs & BIT(RCAR_DU_OUTPUT_DPAD0))
> > +			rcdu->dpad0_source = rcrtc->index;
> > +
> > +		if (rcrtc_state->outputs & BIT(RCAR_DU_OUTPUT_DPAD1))
> > +			rcdu->dpad1_source = rcrtc->index;
> > +	}
> > 
> >  	/* Apply the atomic update. */
> >  	drm_atomic_helper_commit_modeset_disables(dev, old_state);

-- 
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1.1 3/5] drm: rcar-du: Disable unused DPAD outputs

[Laurent Pinchart]

DU channels are routed to DPAD outputs in an SoC-dependent way. The
routing can be fixed (e.g. DU3 to DPAD0 on H3) or configurable (e.g. DU0
or DU1 to DPAD0 on D3/E3). The hardware offers no option to disconnect
DPAD outputs, which are thus always driven by a DU channel.

On SoCs that have less DU channels than DU outputs, such as D3 and E3,
the DPAD output is always driven when all channels are in use by other
outputs (such as the internal LVDS and HDMI encoders). This creates an
unwanted clone on the DPAD output.

However, the parallel output of the DU channels routed to DPAD can be
set to fixed levels in the DU channels themselves through the DOFLR
group register. Use this to turn the DPAD on or off by driving fixed
signals at the output of any DU channel not routed to a DPAD output.
This doesn't affect the DU output signals going to other outputs.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
Changes since v1:

- Renamed rcar_du_group_set_output_levels() to
  rcar_du_group_set_dpad_levels()
- Clarify that fixed DPAD outputs are low-level
---
 drivers/gpu/drm/rcar-du/rcar_du_group.c | 43 +++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
index 7e440f61977f..9eee47969e77 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
@@ -287,6 +287,47 @@ int rcar_du_set_dpad0_vsp1_routing(struct rcar_du_device *rcdu)
 	return 0;
 }
 
+static void rcar_du_group_set_dpad_levels(struct rcar_du_group *rgrp)
+{
+	static const u32 doflr_values[2] = {
+		DOFLR_HSYCFL0 | DOFLR_VSYCFL0 | DOFLR_ODDFL0 |
+		DOFLR_DISPFL0 | DOFLR_CDEFL0  | DOFLR_RGBFL0,
+		DOFLR_HSYCFL1 | DOFLR_VSYCFL1 | DOFLR_ODDFL1 |
+		DOFLR_DISPFL1 | DOFLR_CDEFL1  | DOFLR_RGBFL1,
+	};
+	static const u32 dpad_mask = BIT(RCAR_DU_OUTPUT_DPAD1)
+				   | BIT(RCAR_DU_OUTPUT_DPAD0);
+	struct rcar_du_device *rcdu = rgrp->dev;
+	u32 doflr = DOFLR_CODE;
+	unsigned int i;
+
+	if (rcdu->info->gen < 2)
+		return;
+
+	/*
+	 * The DPAD outputs can't be controlled directly. However, the parallel
+	 * output of the DU channels routed to DPAD can be set to fixed levels
+	 * through the DOFLR group register. Use this to turn the DPAD on or off
+	 * by driving fixed low-level signals at the output of any DU channel
+	 * not routed to a DPAD output. This doesn't affect the DU output
+	 * signals going to other outputs, such as the internal LVDS and HDMI
+	 * encoders.
+	 */
+
+	for (i = 0; i < rgrp->num_crtcs; ++i) {
+		struct rcar_du_crtc_state *rstate;
+		struct rcar_du_crtc *rcrtc;
+
+		rcrtc = &rcdu->crtcs[rgrp->index * 2 + i];
+		rstate = to_rcar_crtc_state(rcrtc->crtc.state);
+
+		if (!(rstate->outputs & dpad_mask))
+			doflr |= doflr_values[i];
+	}
+
+	rcar_du_group_write(rgrp, DOFLR, doflr);
+}
+
 int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
 {
 	struct rcar_du_device *rcdu = rgrp->dev;
@@ -306,5 +347,7 @@ int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
 
 	rcar_du_group_write(rgrp, DORCR, dorcr);
 
+	rcar_du_group_set_dpad_levels(rgrp);
+
 	return rcar_du_set_dpad0_vsp1_routing(rgrp->dev);
 }
-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/5] arm64: dts: renesas: r8a77995: draak: Add backlight

[Geert Uytterhoeven]

Hi Laurent,

On Tue, Dec 4, 2018 at 6:36 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Sun, Nov 25, 2018 at 3:40 PM Laurent Pinchart
> <laurent.pinchart+renesas@ideasonboard.com> wrote:
> > Add the backlight device for the LVDS1 output, in preparation for panel
> > support.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Oops, seems I missed the backlight node should be moved up, to preserve
sort order.

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/5] arm64: dts: renesas: r8a77995: draak: Add backlight

[Laurent Pinchart]

Hi Geert,

On Monday, 10 December 2018 14:30:22 EET Geert Uytterhoeven wrote:
> On Tue, Dec 4, 2018 at 6:36 PM Geert Uytterhoeven wrote:
> > On Sun, Nov 25, 2018 at 3:40 PM Laurent Pinchart wrote:
> >> Add the backlight device for the LVDS1 output, in preparation for panel
> >> support.
> >> 
> >> Signed-off-by: Laurent Pinchart
> >> <laurent.pinchart+renesas@ideasonboard.com>
> > 
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Oops, seems I missed the backlight node should be moved up, to preserve
> sort order.

I assumed that aliases and chosen should be kept at the top of the file. Maybe 
we don't want to keep the tradition :-)

-- 
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/5] arm64: dts: renesas: r8a77995: draak: Add backlight

[Simon Horman]

On Mon, Dec 10, 2018 at 02:33:08PM +0200, Laurent Pinchart wrote:
> Hi Geert,
> 
> On Monday, 10 December 2018 14:30:22 EET Geert Uytterhoeven wrote:
> > On Tue, Dec 4, 2018 at 6:36 PM Geert Uytterhoeven wrote:
> > > On Sun, Nov 25, 2018 at 3:40 PM Laurent Pinchart wrote:
> > >> Add the backlight device for the LVDS1 output, in preparation for panel
> > >> support.
> > >> 
> > >> Signed-off-by: Laurent Pinchart
> > >> <laurent.pinchart+renesas@ideasonboard.com>
> > > 
> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > 
> > Oops, seems I missed the backlight node should be moved up, to preserve
> > sort order.
> 
> I assumed that aliases and chosen should be kept at the top of the file.
> Maybe we don't want to keep the tradition :-)

There is precedence for this tradition in salvator-common.dtsi.
I am ambivalent at this point.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel