[PATCH 0/4] Raspberry Pi Touchscreen bridge/panel drivers

[PATCH 0/4] Raspberry Pi Touchscreen bridge/panel drivers

[Eric Anholt]

Here's the rewrite of the Raspberry Pi display support to split out a
bridge driver representing the toshiba+atmel pair.  It depends on the
panel-bridge layer I've submitted.

The RPi DSI stack isn't completely working yet -- I've got some
flickery pixels on the display where it seems some color values are
unstable (on the default debian swirl desktop image, they're
particularly visible as pink sparkles near the edge of the swirls).
So far messing with clocks, not setting clocks at all, forcing the
firmware's DSI timings, and a bunch of other hacks haven't managed to
resolve it.  Still, I think this driver stack is useful to have, and I
assume that fixing this will be a small bugfix somewhere later.

Eric Anholt (4):
  drm/vc4: Adjust modes in DSI to work around the integer PLL divider.
  dt-bindings: Document the Raspberry Pi Touchscreen nodes.
  drm/bridge: Add support for the Raspberry Pi 7" Touchscreen.
  drm/panel: Add the Raspberry Pi 7" touchscreen's panel.

 .../raspberrypi,7inch-touchscreen-bridge.txt       |  68 +++
 .../panel/raspberrypi,7inch-touchscreen-panel.txt  |   7 +
 drivers/gpu/drm/bridge/Kconfig                     |   9 +
 drivers/gpu/drm/bridge/Makefile                    |   1 +
 drivers/gpu/drm/bridge/raspberrypi-touchscreen.c   | 460 +++++++++++++++++++++
 drivers/gpu/drm/panel/panel-simple.c               |  30 ++
 drivers/gpu/drm/vc4/vc4_dsi.c                      | 112 +++--
 7 files changed, 661 insertions(+), 26 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchscreen-bridge.txt
 create mode 100644 Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen-panel.txt
 create mode 100644 drivers/gpu/drm/bridge/raspberrypi-touchscreen.c

-- 
2.11.0

[PATCH 1/4] drm/vc4: Adjust modes in DSI to work around the integer PLL divider.

[Eric Anholt]

BCM2835's PLLD_DSI1 divider doesn't give us many choices for our pixel
clocks, so to support panels on the Raspberry Pi we need to set a
higher pixel clock rate than requested and adjust the mode we program
to extend out the HFP so that the refresh rate matches.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_dsi.c | 112 ++++++++++++++++++++++++++++++++----------
 1 file changed, 86 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index fb54a9d10360..62cb3b0d0345 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -519,7 +519,8 @@ struct vc4_dsi {
 	/* DSI channel for the panel we're connected to. */
 	u32 channel;
 	u32 lanes;
-	enum mipi_dsi_pixel_format format;
+	u32 format;
+	u32 divider;
 	u32 mode_flags;
 
 	/* Input clock from CPRMAN to the digital PHY, for the DSI
@@ -817,13 +818,67 @@ static void vc4_dsi_encoder_disable(struct drm_encoder *encoder)
 	pm_runtime_put(dev);
 }
 
+/* Extends the mode's blank intervals to handle BCM2835's integer-only
+ * DSI PLL divider.
+ *
+ * On 2835, PLLD is set to 2Ghz, and may not be changed by the display
+ * driver since most peripherals are hanging off of the PLLD_PER
+ * divider.  PLLD_DSI1, which drives our DSI bit clock (and therefore
+ * the pixel clock), only has an integer divider off of DSI.
+ *
+ * To get our panel mode to refresh at the expected 60Hz, we need to
+ * extend the horizontal blank time.  This means we drive a
+ * higher-than-expected clock rate to the panel, but that's what the
+ * firmware (which ) does too.
+ */
+static bool vc4_dsi_encoder_mode_fixup(struct drm_encoder *encoder,
+				       const struct drm_display_mode *mode,
+				       struct drm_display_mode *adjusted_mode)
+{
+	struct vc4_dsi_encoder *vc4_encoder = to_vc4_dsi_encoder(encoder);
+	struct vc4_dsi *dsi = vc4_encoder->dsi;
+	struct clk *phy_parent = clk_get_parent(dsi->pll_phy_clock);
+	unsigned long parent_rate = clk_get_rate(phy_parent);
+	unsigned long pixel_clock_hz = mode->clock * 1000;
+	unsigned long pll_clock = pixel_clock_hz * dsi->divider;
+	int divider;
+
+	/* Find what divider gets us a faster clock than the requested
+	 * pixel clock.
+	 */
+	for (divider = 1; divider < 8; divider++) {
+		if (parent_rate / divider < pll_clock) {
+			divider--;
+			break;
+		}
+	}
+
+	/* Now that we've picked a PLL divider, calculate back to its
+	 * pixel clock.
+	 */
+	pll_clock = parent_rate / divider;
+	pixel_clock_hz = pll_clock / dsi->divider;
+
+	/* Round up the clk_set_rate() request slightly, since
+	 * PLLD_DSI1 is an integer divider and its rate selection will
+	 * never round up.
+	 */
+	adjusted_mode->clock = pixel_clock_hz / 1000 + 1;
+
+	/* Given the new pixel clock, adjust HFP to keep vrefresh the same. */
+	adjusted_mode->htotal = pixel_clock_hz / (mode->vrefresh * mode->vtotal);
+	adjusted_mode->hsync_end += adjusted_mode->htotal - mode->htotal;
+	adjusted_mode->hsync_start += adjusted_mode->htotal - mode->htotal;
+
+	return true;
+}
+
 static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
 {
-	struct drm_display_mode *mode = &encoder->crtc->mode;
+	struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
 	struct vc4_dsi_encoder *vc4_encoder = to_vc4_dsi_encoder(encoder);
 	struct vc4_dsi *dsi = vc4_encoder->dsi;
 	struct device *dev = &dsi->pdev->dev;
-	u32 format = 0, divider = 0;
 	bool debug_dump_regs = false;
 	unsigned long hs_clock;
 	u32 ui_ns;
@@ -845,26 +900,7 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
 		vc4_dsi_dump_regs(dsi);
 	}
 
-	switch (dsi->format) {
-	case MIPI_DSI_FMT_RGB888:
-		format = DSI_PFORMAT_RGB888;
-		divider = 24 / dsi->lanes;
-		break;
-	case MIPI_DSI_FMT_RGB666:
-		format = DSI_PFORMAT_RGB666;
-		divider = 24 / dsi->lanes;
-		break;
-	case MIPI_DSI_FMT_RGB666_PACKED:
-		format = DSI_PFORMAT_RGB666_PACKED;
-		divider = 18 / dsi->lanes;
-		break;
-	case MIPI_DSI_FMT_RGB565:
-		format = DSI_PFORMAT_RGB565;
-		divider = 16 / dsi->lanes;
-		break;
-	}
-
-	phy_clock = pixel_clock_hz * divider;
+	phy_clock = pixel_clock_hz * dsi->divider;
 	ret = clk_set_rate(dsi->pll_phy_clock, phy_clock);
 	if (ret) {
 		dev_err(&dsi->pdev->dev,
@@ -1049,8 +1085,9 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
 
 	if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
 		DSI_PORT_WRITE(DISP0_CTRL,
-			       VC4_SET_FIELD(divider, DSI_DISP0_PIX_CLK_DIV) |
-			       VC4_SET_FIELD(format, DSI_DISP0_PFORMAT) |
+			       VC4_SET_FIELD(dsi->divider,
+					     DSI_DISP0_PIX_CLK_DIV) |
+			       VC4_SET_FIELD(dsi->format, DSI_DISP0_PFORMAT) |
 			       VC4_SET_FIELD(DSI_DISP0_LP_STOP_PERFRAME,
 					     DSI_DISP0_LP_STOP_CTRL) |
 			       DSI_DISP0_ST_END |
@@ -1255,9 +1292,31 @@ static int vc4_dsi_host_attach(struct mipi_dsi_host *host,
 
 	dsi->lanes = device->lanes;
 	dsi->channel = device->channel;
-	dsi->format = device->format;
 	dsi->mode_flags = device->mode_flags;
 
+	switch (device->format) {
+	case MIPI_DSI_FMT_RGB888:
+		dsi->format = DSI_PFORMAT_RGB888;
+		dsi->divider = 24 / dsi->lanes;
+		break;
+	case MIPI_DSI_FMT_RGB666:
+		dsi->format = DSI_PFORMAT_RGB666;
+		dsi->divider = 24 / dsi->lanes;
+		break;
+	case MIPI_DSI_FMT_RGB666_PACKED:
+		dsi->format = DSI_PFORMAT_RGB666_PACKED;
+		dsi->divider = 18 / dsi->lanes;
+		break;
+	case MIPI_DSI_FMT_RGB565:
+		dsi->format = DSI_PFORMAT_RGB565;
+		dsi->divider = 16 / dsi->lanes;
+		break;
+	default:
+		dev_err(&dsi->pdev->dev, "Unknown DSI format: %d.\n",
+			dsi->format);
+		return 0;
+	}
+
 	if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO)) {
 		dev_err(&dsi->pdev->dev,
 			"Only VIDEO mode panels supported currently.\n");
@@ -1304,6 +1363,7 @@ static const struct mipi_dsi_host_ops vc4_dsi_host_ops = {
 static const struct drm_encoder_helper_funcs vc4_dsi_encoder_helper_funcs = {
 	.disable = vc4_dsi_encoder_disable,
 	.enable = vc4_dsi_encoder_enable,
+	.mode_fixup = vc4_dsi_encoder_mode_fixup,
 };
 
 static const struct of_device_id vc4_dsi_dt_match[] = {
-- 
2.11.0

Re: [PATCH 1/4] drm/vc4: Adjust modes in DSI to work around the integer PLL divider.

[Daniel Vetter]

On Thu, May 11, 2017 at 04:56:22PM -0700, Eric Anholt wrote:
> BCM2835's PLLD_DSI1 divider doesn't give us many choices for our pixel
> clocks, so to support panels on the Raspberry Pi we need to set a
> higher pixel clock rate than requested and adjust the mode we program
> to extend out the HFP so that the refresh rate matches.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>

Yeah, this is what mode_fixup is for (or the fancier atomic_check, but no
benefit with using that one here).

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/vc4/vc4_dsi.c | 112 ++++++++++++++++++++++++++++++++----------
>  1 file changed, 86 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
> index fb54a9d10360..62cb3b0d0345 100644
> --- a/drivers/gpu/drm/vc4/vc4_dsi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dsi.c
> @@ -519,7 +519,8 @@ struct vc4_dsi {
>  	/* DSI channel for the panel we're connected to. */
>  	u32 channel;
>  	u32 lanes;
> -	enum mipi_dsi_pixel_format format;
> +	u32 format;
> +	u32 divider;
>  	u32 mode_flags;
>  
>  	/* Input clock from CPRMAN to the digital PHY, for the DSI
> @@ -817,13 +818,67 @@ static void vc4_dsi_encoder_disable(struct drm_encoder *encoder)
>  	pm_runtime_put(dev);
>  }
>  
> +/* Extends the mode's blank intervals to handle BCM2835's integer-only
> + * DSI PLL divider.
> + *
> + * On 2835, PLLD is set to 2Ghz, and may not be changed by the display
> + * driver since most peripherals are hanging off of the PLLD_PER
> + * divider.  PLLD_DSI1, which drives our DSI bit clock (and therefore
> + * the pixel clock), only has an integer divider off of DSI.
> + *
> + * To get our panel mode to refresh at the expected 60Hz, we need to
> + * extend the horizontal blank time.  This means we drive a
> + * higher-than-expected clock rate to the panel, but that's what the
> + * firmware (which ) does too.
> + */
> +static bool vc4_dsi_encoder_mode_fixup(struct drm_encoder *encoder,
> +				       const struct drm_display_mode *mode,
> +				       struct drm_display_mode *adjusted_mode)
> +{
> +	struct vc4_dsi_encoder *vc4_encoder = to_vc4_dsi_encoder(encoder);
> +	struct vc4_dsi *dsi = vc4_encoder->dsi;
> +	struct clk *phy_parent = clk_get_parent(dsi->pll_phy_clock);
> +	unsigned long parent_rate = clk_get_rate(phy_parent);
> +	unsigned long pixel_clock_hz = mode->clock * 1000;
> +	unsigned long pll_clock = pixel_clock_hz * dsi->divider;
> +	int divider;
> +
> +	/* Find what divider gets us a faster clock than the requested
> +	 * pixel clock.
> +	 */
> +	for (divider = 1; divider < 8; divider++) {
> +		if (parent_rate / divider < pll_clock) {
> +			divider--;
> +			break;
> +		}
> +	}
> +
> +	/* Now that we've picked a PLL divider, calculate back to its
> +	 * pixel clock.
> +	 */
> +	pll_clock = parent_rate / divider;
> +	pixel_clock_hz = pll_clock / dsi->divider;
> +
> +	/* Round up the clk_set_rate() request slightly, since
> +	 * PLLD_DSI1 is an integer divider and its rate selection will
> +	 * never round up.
> +	 */
> +	adjusted_mode->clock = pixel_clock_hz / 1000 + 1;
> +
> +	/* Given the new pixel clock, adjust HFP to keep vrefresh the same. */
> +	adjusted_mode->htotal = pixel_clock_hz / (mode->vrefresh * mode->vtotal);
> +	adjusted_mode->hsync_end += adjusted_mode->htotal - mode->htotal;
> +	adjusted_mode->hsync_start += adjusted_mode->htotal - mode->htotal;
> +
> +	return true;
> +}
> +
>  static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
>  {
> -	struct drm_display_mode *mode = &encoder->crtc->mode;
> +	struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
>  	struct vc4_dsi_encoder *vc4_encoder = to_vc4_dsi_encoder(encoder);
>  	struct vc4_dsi *dsi = vc4_encoder->dsi;
>  	struct device *dev = &dsi->pdev->dev;
> -	u32 format = 0, divider = 0;
>  	bool debug_dump_regs = false;
>  	unsigned long hs_clock;
>  	u32 ui_ns;
> @@ -845,26 +900,7 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
>  		vc4_dsi_dump_regs(dsi);
>  	}
>  
> -	switch (dsi->format) {
> -	case MIPI_DSI_FMT_RGB888:
> -		format = DSI_PFORMAT_RGB888;
> -		divider = 24 / dsi->lanes;
> -		break;
> -	case MIPI_DSI_FMT_RGB666:
> -		format = DSI_PFORMAT_RGB666;
> -		divider = 24 / dsi->lanes;
> -		break;
> -	case MIPI_DSI_FMT_RGB666_PACKED:
> -		format = DSI_PFORMAT_RGB666_PACKED;
> -		divider = 18 / dsi->lanes;
> -		break;
> -	case MIPI_DSI_FMT_RGB565:
> -		format = DSI_PFORMAT_RGB565;
> -		divider = 16 / dsi->lanes;
> -		break;
> -	}
> -
> -	phy_clock = pixel_clock_hz * divider;
> +	phy_clock = pixel_clock_hz * dsi->divider;
>  	ret = clk_set_rate(dsi->pll_phy_clock, phy_clock);
>  	if (ret) {
>  		dev_err(&dsi->pdev->dev,
> @@ -1049,8 +1085,9 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
>  
>  	if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
>  		DSI_PORT_WRITE(DISP0_CTRL,
> -			       VC4_SET_FIELD(divider, DSI_DISP0_PIX_CLK_DIV) |
> -			       VC4_SET_FIELD(format, DSI_DISP0_PFORMAT) |
> +			       VC4_SET_FIELD(dsi->divider,
> +					     DSI_DISP0_PIX_CLK_DIV) |
> +			       VC4_SET_FIELD(dsi->format, DSI_DISP0_PFORMAT) |
>  			       VC4_SET_FIELD(DSI_DISP0_LP_STOP_PERFRAME,
>  					     DSI_DISP0_LP_STOP_CTRL) |
>  			       DSI_DISP0_ST_END |
> @@ -1255,9 +1292,31 @@ static int vc4_dsi_host_attach(struct mipi_dsi_host *host,
>  
>  	dsi->lanes = device->lanes;
>  	dsi->channel = device->channel;
> -	dsi->format = device->format;
>  	dsi->mode_flags = device->mode_flags;
>  
> +	switch (device->format) {
> +	case MIPI_DSI_FMT_RGB888:
> +		dsi->format = DSI_PFORMAT_RGB888;
> +		dsi->divider = 24 / dsi->lanes;
> +		break;
> +	case MIPI_DSI_FMT_RGB666:
> +		dsi->format = DSI_PFORMAT_RGB666;
> +		dsi->divider = 24 / dsi->lanes;
> +		break;
> +	case MIPI_DSI_FMT_RGB666_PACKED:
> +		dsi->format = DSI_PFORMAT_RGB666_PACKED;
> +		dsi->divider = 18 / dsi->lanes;
> +		break;
> +	case MIPI_DSI_FMT_RGB565:
> +		dsi->format = DSI_PFORMAT_RGB565;
> +		dsi->divider = 16 / dsi->lanes;
> +		break;
> +	default:
> +		dev_err(&dsi->pdev->dev, "Unknown DSI format: %d.\n",
> +			dsi->format);
> +		return 0;
> +	}
> +
>  	if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO)) {
>  		dev_err(&dsi->pdev->dev,
>  			"Only VIDEO mode panels supported currently.\n");
> @@ -1304,6 +1363,7 @@ static const struct mipi_dsi_host_ops vc4_dsi_host_ops = {
>  static const struct drm_encoder_helper_funcs vc4_dsi_encoder_helper_funcs = {
>  	.disable = vc4_dsi_encoder_disable,
>  	.enable = vc4_dsi_encoder_enable,
> +	.mode_fixup = vc4_dsi_encoder_mode_fixup,
>  };
>  
>  static const struct of_device_id vc4_dsi_dt_match[] = {
> -- 
> 2.11.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 1/4] drm/vc4: Adjust modes in DSI to work around the integer PLL divider.

[Noralf Trønnes]

Den 12.05.2017 01.56, skrev Eric Anholt:
> BCM2835's PLLD_DSI1 divider doesn't give us many choices for our pixel
> clocks, so to support panels on the Raspberry Pi we need to set a
> higher pixel clock rate than requested and adjust the mode we program
> to extend out the HFP so that the refresh rate matches.
>
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>   drivers/gpu/drm/vc4/vc4_dsi.c | 112 ++++++++++++++++++++++++++++++++----------
>   1 file changed, 86 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
> index fb54a9d10360..62cb3b0d0345 100644
> --- a/drivers/gpu/drm/vc4/vc4_dsi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dsi.c
> @@ -519,7 +519,8 @@ struct vc4_dsi {
>   	/* DSI channel for the panel we're connected to. */
>   	u32 channel;
>   	u32 lanes;
> -	enum mipi_dsi_pixel_format format;
> +	u32 format;
> +	u32 divider;
>   	u32 mode_flags;
>   
>   	/* Input clock from CPRMAN to the digital PHY, for the DSI
> @@ -817,13 +818,67 @@ static void vc4_dsi_encoder_disable(struct drm_encoder *encoder)
>   	pm_runtime_put(dev);
>   }
>   
> +/* Extends the mode's blank intervals to handle BCM2835's integer-only
> + * DSI PLL divider.
> + *
> + * On 2835, PLLD is set to 2Ghz, and may not be changed by the display
> + * driver since most peripherals are hanging off of the PLLD_PER
> + * divider.  PLLD_DSI1, which drives our DSI bit clock (and therefore
> + * the pixel clock), only has an integer divider off of DSI.
> + *
> + * To get our panel mode to refresh at the expected 60Hz, we need to
> + * extend the horizontal blank time.  This means we drive a
> + * higher-than-expected clock rate to the panel, but that's what the
> + * firmware (which ) does too.

Something missing in the comment here.

Noralf.

> + */
> +static bool vc4_dsi_encoder_mode_fixup(struct drm_encoder *encoder,
> +				       const struct drm_display_mode *mode,
> +				       struct drm_display_mode *adjusted_mode)
> +{
> +	struct vc4_dsi_encoder *vc4_encoder = to_vc4_dsi_encoder(encoder);
> +	struct vc4_dsi *dsi = vc4_encoder->dsi;
> +	struct clk *phy_parent = clk_get_parent(dsi->pll_phy_clock);
> +	unsigned long parent_rate = clk_get_rate(phy_parent);
> +	unsigned long pixel_clock_hz = mode->clock * 1000;
> +	unsigned long pll_clock = pixel_clock_hz * dsi->divider;
> +	int divider;
> +
> +	/* Find what divider gets us a faster clock than the requested
> +	 * pixel clock.
> +	 */
> +	for (divider = 1; divider < 8; divider++) {
> +		if (parent_rate / divider < pll_clock) {
> +			divider--;
> +			break;
> +		}
> +	}
> +
> +	/* Now that we've picked a PLL divider, calculate back to its
> +	 * pixel clock.
> +	 */
> +	pll_clock = parent_rate / divider;
> +	pixel_clock_hz = pll_clock / dsi->divider;
> +
> +	/* Round up the clk_set_rate() request slightly, since
> +	 * PLLD_DSI1 is an integer divider and its rate selection will
> +	 * never round up.
> +	 */
> +	adjusted_mode->clock = pixel_clock_hz / 1000 + 1;
> +
> +	/* Given the new pixel clock, adjust HFP to keep vrefresh the same. */
> +	adjusted_mode->htotal = pixel_clock_hz / (mode->vrefresh * mode->vtotal);
> +	adjusted_mode->hsync_end += adjusted_mode->htotal - mode->htotal;
> +	adjusted_mode->hsync_start += adjusted_mode->htotal - mode->htotal;
> +
> +	return true;
> +}
> +
>   static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
>   {
> -	struct drm_display_mode *mode = &encoder->crtc->mode;
> +	struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
>   	struct vc4_dsi_encoder *vc4_encoder = to_vc4_dsi_encoder(encoder);
>   	struct vc4_dsi *dsi = vc4_encoder->dsi;
>   	struct device *dev = &dsi->pdev->dev;
> -	u32 format = 0, divider = 0;
>   	bool debug_dump_regs = false;
>   	unsigned long hs_clock;
>   	u32 ui_ns;
> @@ -845,26 +900,7 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
>   		vc4_dsi_dump_regs(dsi);
>   	}
>   
> -	switch (dsi->format) {
> -	case MIPI_DSI_FMT_RGB888:
> -		format = DSI_PFORMAT_RGB888;
> -		divider = 24 / dsi->lanes;
> -		break;
> -	case MIPI_DSI_FMT_RGB666:
> -		format = DSI_PFORMAT_RGB666;
> -		divider = 24 / dsi->lanes;
> -		break;
> -	case MIPI_DSI_FMT_RGB666_PACKED:
> -		format = DSI_PFORMAT_RGB666_PACKED;
> -		divider = 18 / dsi->lanes;
> -		break;
> -	case MIPI_DSI_FMT_RGB565:
> -		format = DSI_PFORMAT_RGB565;
> -		divider = 16 / dsi->lanes;
> -		break;
> -	}
> -
> -	phy_clock = pixel_clock_hz * divider;
> +	phy_clock = pixel_clock_hz * dsi->divider;
>   	ret = clk_set_rate(dsi->pll_phy_clock, phy_clock);
>   	if (ret) {
>   		dev_err(&dsi->pdev->dev,
> @@ -1049,8 +1085,9 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
>   
>   	if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
>   		DSI_PORT_WRITE(DISP0_CTRL,
> -			       VC4_SET_FIELD(divider, DSI_DISP0_PIX_CLK_DIV) |
> -			       VC4_SET_FIELD(format, DSI_DISP0_PFORMAT) |
> +			       VC4_SET_FIELD(dsi->divider,
> +					     DSI_DISP0_PIX_CLK_DIV) |
> +			       VC4_SET_FIELD(dsi->format, DSI_DISP0_PFORMAT) |
>   			       VC4_SET_FIELD(DSI_DISP0_LP_STOP_PERFRAME,
>   					     DSI_DISP0_LP_STOP_CTRL) |
>   			       DSI_DISP0_ST_END |
> @@ -1255,9 +1292,31 @@ static int vc4_dsi_host_attach(struct mipi_dsi_host *host,
>   
>   	dsi->lanes = device->lanes;
>   	dsi->channel = device->channel;
> -	dsi->format = device->format;
>   	dsi->mode_flags = device->mode_flags;
>   
> +	switch (device->format) {
> +	case MIPI_DSI_FMT_RGB888:
> +		dsi->format = DSI_PFORMAT_RGB888;
> +		dsi->divider = 24 / dsi->lanes;
> +		break;
> +	case MIPI_DSI_FMT_RGB666:
> +		dsi->format = DSI_PFORMAT_RGB666;
> +		dsi->divider = 24 / dsi->lanes;
> +		break;
> +	case MIPI_DSI_FMT_RGB666_PACKED:
> +		dsi->format = DSI_PFORMAT_RGB666_PACKED;
> +		dsi->divider = 18 / dsi->lanes;
> +		break;
> +	case MIPI_DSI_FMT_RGB565:
> +		dsi->format = DSI_PFORMAT_RGB565;
> +		dsi->divider = 16 / dsi->lanes;
> +		break;
> +	default:
> +		dev_err(&dsi->pdev->dev, "Unknown DSI format: %d.\n",
> +			dsi->format);
> +		return 0;
> +	}
> +
>   	if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO)) {
>   		dev_err(&dsi->pdev->dev,
>   			"Only VIDEO mode panels supported currently.\n");
> @@ -1304,6 +1363,7 @@ static const struct mipi_dsi_host_ops vc4_dsi_host_ops = {
>   static const struct drm_encoder_helper_funcs vc4_dsi_encoder_helper_funcs = {
>   	.disable = vc4_dsi_encoder_disable,
>   	.enable = vc4_dsi_encoder_enable,
> +	.mode_fixup = vc4_dsi_encoder_mode_fixup,
>   };
>   
>   static const struct of_device_id vc4_dsi_dt_match[] = {

[PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

[Eric Anholt]

The Raspberry Pi 7" Touchscreen is a DPI touchscreen panel with
DSI->DPI bridge and touchscreen controller integrated, that connects
to the Raspberry Pi through its 15-pin "DSI" connector (some lines are
DSI, some lines are I2C).

This device is represented in the DT as three nodes (DSI device, I2C
device, panel).  Input will be left to a separate binding later, as it
will be a basic I2C client device.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 .../raspberrypi,7inch-touchscreen-bridge.txt       | 68 ++++++++++++++++++++++
 .../panel/raspberrypi,7inch-touchscreen-panel.txt  |  7 +++
 2 files changed, 75 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchscreen-bridge.txt
 create mode 100644 Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen-panel.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchscreen-bridge.txt b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchscreen-bridge.txt
new file mode 100644
index 000000000000..a5669beaf68f
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchscreen-bridge.txt
@@ -0,0 +1,68 @@
+Official 7" (800x480) Raspberry Pi touchscreen panel's bridge.
+
+This DSI panel contains:
+
+- TC358762 DSI->DPI bridge
+- Atmel microcontroller on I2C for power sequencing the DSI bridge and
+  controlling backlight
+- Touchscreen controller on I2C for touch input
+
+and this covers the TC358762 bridge and Atmel microcontroller, while
+../panel/raspberrypi,7inch-touchscreen-panel.txt covers the panel.
+
+Required properties:
+- compatible:	Must be "raspberrypi,7inch-touchscreen-bridge"
+- raspberrypi,touchscreen-bridge:
+		Handle to the I2C device for Atmel microcontroller
+
+Example:
+
+dsi1: dsi@7e700000 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	<...>
+
+	lcd-bridge@0 {
+		compatible = "raspberrypi,7inch-touchscreen-bridge";
+		reg = <0>;
+
+		raspberrypi,touchscreen-bridge = <&pitouchscreen_bridge>;
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			port@0 {
+				reg = <0>;
+				pitouchscreen_bridge_port: endpoint {
+					remote-endpoint = <&pitouchscreen_panel_port>;
+				};
+			};
+		};
+	};
+};
+
+i2c_dsi: i2c {
+	compatible = "i2c-gpio";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	gpios = <&gpio 28 0
+		 &gpio 29 0>;
+
+	pitouchscreen_bridge: bridge@45 {
+		compatible = "raspberrypi,touchscreen-bridge-i2c";
+		reg = <0x45>;
+	};
+};
+
+lcd {
+	compatible = "raspberrypi,7inch-touchscreen-panel";
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		port@0 {
+			reg = <0>;
+			pitouchscreen_panel_port: endpoint {
+				remote-endpoint = <&pitouchscreen_bridge_port>;
+			};
+		};
+	};
+};
diff --git a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen-panel.txt b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen-panel.txt
new file mode 100644
index 000000000000..1e84f97b3b20
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen-panel.txt
@@ -0,0 +1,7 @@
+Official 7" (800x480) Raspberry Pi touchscreen panel's panel.
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
+
+Required properties:
+- compatible:	Must be "raspberrypi,7inch-touchscreen-panel"
-- 
2.11.0

Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

[Rob Herring]

On Thu, May 11, 2017 at 04:56:23PM -0700, Eric Anholt wrote:
> The Raspberry Pi 7" Touchscreen is a DPI touchscreen panel with
> DSI->DPI bridge and touchscreen controller integrated, that connects
> to the Raspberry Pi through its 15-pin "DSI" connector (some lines are
> DSI, some lines are I2C).
> 
> This device is represented in the DT as three nodes (DSI device, I2C
> device, panel).  Input will be left to a separate binding later, as it
> will be a basic I2C client device.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>  .../raspberrypi,7inch-touchscreen-bridge.txt       | 68 ++++++++++++++++++++++
>  .../panel/raspberrypi,7inch-touchscreen-panel.txt  |  7 +++
>  2 files changed, 75 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchscreen-bridge.txt
>  create mode 100644 Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen-panel.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchscreen-bridge.txt b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchscreen-bridge.txt
> new file mode 100644
> index 000000000000..a5669beaf68f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchscreen-bridge.txt
> @@ -0,0 +1,68 @@
> +Official 7" (800x480) Raspberry Pi touchscreen panel's bridge.
> +
> +This DSI panel contains:
> +
> +- TC358762 DSI->DPI bridge
> +- Atmel microcontroller on I2C for power sequencing the DSI bridge and
> +  controlling backlight
> +- Touchscreen controller on I2C for touch input

This is 1 uC or 2?

> +
> +and this covers the TC358762 bridge and Atmel microcontroller, while
> +../panel/raspberrypi,7inch-touchscreen-panel.txt covers the panel.
> +
> +Required properties:
> +- compatible:	Must be "raspberrypi,7inch-touchscreen-bridge"
> +- raspberrypi,touchscreen-bridge:
> +		Handle to the I2C device for Atmel microcontroller
> +
> +Example:
> +
> +dsi1: dsi@7e700000 {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	<...>
> +
> +	lcd-bridge@0 {
> +		compatible = "raspberrypi,7inch-touchscreen-bridge";
> +		reg = <0>;
> +
> +		raspberrypi,touchscreen-bridge = <&pitouchscreen_bridge>;

I think this should be a port with a graph connection from the DSI 
node to the i2c bridge device (and then to the panel).

It's also how other DSI bridges like the tc358767 are done.

> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			port@0 {
> +				reg = <0>;

BTW, you don't need this when there is only 1.

> +				pitouchscreen_bridge_port: endpoint {
> +					remote-endpoint = <&pitouchscreen_panel_port>;
> +				};
> +			};
> +		};
> +	};
> +};
> +
> +i2c_dsi: i2c {
> +	compatible = "i2c-gpio";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	gpios = <&gpio 28 0
> +		 &gpio 29 0>;
> +
> +	pitouchscreen_bridge: bridge@45 {
> +		compatible = "raspberrypi,touchscreen-bridge-i2c";
> +		reg = <0x45>;
> +	};
> +};
> +
> +lcd {
> +	compatible = "raspberrypi,7inch-touchscreen-panel";
> +	ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		port@0 {
> +			reg = <0>;
> +			pitouchscreen_panel_port: endpoint {
> +				remote-endpoint = <&pitouchscreen_bridge_port>;
> +			};
> +		};
> +	};
> +};
> diff --git a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen-panel.txt b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen-panel.txt
> new file mode 100644
> index 000000000000..1e84f97b3b20
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen-panel.txt
> @@ -0,0 +1,7 @@
> +Official 7" (800x480) Raspberry Pi touchscreen panel's panel.
> +
> +This binding is compatible with the simple-panel binding, which is specified
> +in simple-panel.txt in this directory.
> +
> +Required properties:
> +- compatible:	Must be "raspberrypi,7inch-touchscreen-panel"
> -- 
> 2.11.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

[Laurent Pinchart]

Hi Rob,

On Monday 15 May 2017 15:44:57 Rob Herring wrote:
> On Thu, May 11, 2017 at 04:56:23PM -0700, Eric Anholt wrote:
> > The Raspberry Pi 7" Touchscreen is a DPI touchscreen panel with
> > DSI->DPI bridge and touchscreen controller integrated, that connects
> > to the Raspberry Pi through its 15-pin "DSI" connector (some lines are
> > DSI, some lines are I2C).
> > 
> > This device is represented in the DT as three nodes (DSI device, I2C
> > device, panel).  Input will be left to a separate binding later, as it
> > will be a basic I2C client device.
> > 
> > Signed-off-by: Eric Anholt <eric@anholt.net>
> > ---
> > 
> >  .../raspberrypi,7inch-touchscreen-bridge.txt       | 68
> >  ++++++++++++++++++++++ .../panel/raspberrypi,7inch-touchscreen-panel.txt
> >   |  7 +++
> >  2 files changed, 75 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touch
> >  screen-bridge.txt create mode 100644
> >  Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchs
> >  creen-panel.txt> 
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touc
> > hscreen-bridge.txt
> > b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touc
> > hscreen-bridge.txt new file mode 100644
> > index 000000000000..a5669beaf68f
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touc
> > hscreen-bridge.txt @@ -0,0 +1,68 @@
> > +Official 7" (800x480) Raspberry Pi touchscreen panel's bridge.
> > +
> > +This DSI panel contains:
> > +
> > +- TC358762 DSI->DPI bridge
> > +- Atmel microcontroller on I2C for power sequencing the DSI bridge and
> > +  controlling backlight
> > +- Touchscreen controller on I2C for touch input
> 
> This is 1 uC or 2?
> 
> > +
> > +and this covers the TC358762 bridge and Atmel microcontroller, while
> > +../panel/raspberrypi,7inch-touchscreen-panel.txt covers the panel.
> > +
> > +Required properties:
> > +- compatible:	Must be "raspberrypi,7inch-touchscreen-bridge"
> > +- raspberrypi,touchscreen-bridge:
> > +		Handle to the I2C device for Atmel microcontroller
> > +
> > +Example:
> > +
> > +dsi1: dsi@7e700000 {
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +	<...>
> > +
> > +	lcd-bridge@0 {
> > +		compatible = "raspberrypi,7inch-touchscreen-bridge";
> > +		reg = <0>;
> > +
> > +		raspberrypi,touchscreen-bridge = <&pitouchscreen_bridge>;
> 
> I think this should be a port with a graph connection from the DSI
> node to the i2c bridge device (and then to the panel).

No, this should actually not exist :-) The property references the I2C device 
DT node corresponding to the microcontroller that controls the backlight (and, 
if I understand correctly, handles power sequencing). The DSI driver (in patch 
3/4) then grabs the I2C device and talks to it. I don't think this is right, 
as commented separately on this patch, the bindings should be split, with a 
standalone binding for the TC358762 (and a separate standalone driver too).

> It's also how other DSI bridges like the tc358767 are done.
> 
> > +		ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +			port@0 {
> > +				reg = <0>;
> 
> BTW, you don't need this when there is only 1.
> 
> > +				pitouchscreen_bridge_port: endpoint {
> > +					remote-endpoint = 
<&pitouchscreen_panel_port>;
> > +				};
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +i2c_dsi: i2c {
> > +	compatible = "i2c-gpio";
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +	gpios = <&gpio 28 0
> > +		 &gpio 29 0>;
> > +
> > +	pitouchscreen_bridge: bridge@45 {
> > +		compatible = "raspberrypi,touchscreen-bridge-i2c";
> > +		reg = <0x45>;
> > +	};
> > +};
> > +
> > +lcd {
> > +	compatible = "raspberrypi,7inch-touchscreen-panel";
> > +	ports {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		port@0 {
> > +			reg = <0>;
> > +			pitouchscreen_panel_port: endpoint {
> > +				remote-endpoint = 
<&pitouchscreen_bridge_port>;
> > +			};
> > +		};
> > +	};
> > +};
> > diff --git
> > a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touch
> > screen-panel.txt
> > b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touch
> > screen-panel.txt new file mode 100644
> > index 000000000000..1e84f97b3b20
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touch
> > screen-panel.txt @@ -0,0 +1,7 @@
> > +Official 7" (800x480) Raspberry Pi touchscreen panel's panel.
> > +
> > +This binding is compatible with the simple-panel binding, which is
> > specified +in simple-panel.txt in this directory.
> > +
> > +Required properties:
> > +- compatible:	Must be "raspberrypi,7inch-touchscreen-panel"

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

[Laurent Pinchart]

Hi Eric,

Thank you for the patch.

On Thursday 11 May 2017 16:56:23 Eric Anholt wrote:
> The Raspberry Pi 7" Touchscreen is a DPI touchscreen panel with
> DSI->DPI bridge and touchscreen controller integrated, that connects
> to the Raspberry Pi through its 15-pin "DSI" connector (some lines are
> DSI, some lines are I2C).
> 
> This device is represented in the DT as three nodes (DSI device, I2C
> device, panel).  Input will be left to a separate binding later, as it
> will be a basic I2C client device.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>  .../raspberrypi,7inch-touchscreen-bridge.txt       | 68 +++++++++++++++++++
>  .../panel/raspberrypi,7inch-touchscreen-panel.txt  |  7 +++
>  2 files changed, 75 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchscr
> een-bridge.txt create mode 100644
> Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscre
> en-panel.txt
> 
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchs
> creen-bridge.txt
> b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchs
> creen-bridge.txt new file mode 100644
> index 000000000000..a5669beaf68f
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchs
> creen-bridge.txt @@ -0,0 +1,68 @@
> +Official 7" (800x480) Raspberry Pi touchscreen panel's bridge.
> +
> +This DSI panel contains:
> +
> +- TC358762 DSI->DPI bridge
> +- Atmel microcontroller on I2C for power sequencing the DSI bridge and
> +  controlling backlight
> +- Touchscreen controller on I2C for touch input
> +
> +and this covers the TC358762 bridge and Atmel microcontroller, while
> +../panel/raspberrypi,7inch-touchscreen-panel.txt covers the panel.

The TC358762 is a standalone bridge that doesn't depend on the ATTiny 
microcontroller used by the RPI. As it's usable standalone, I believe this 
binding should be split in two.

> +Required properties:
> +- compatible:	Must be "raspberrypi,7inch-touchscreen-bridge"
> +- raspberrypi,touchscreen-bridge:
> +		Handle to the I2C device for Atmel microcontroller
> +
> +Example:
> +
> +dsi1: dsi@7e700000 {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	<...>
> +
> +	lcd-bridge@0 {
> +		compatible = "raspberrypi,7inch-touchscreen-bridge";
> +		reg = <0>;
> +
> +		raspberrypi,touchscreen-bridge = <&pitouchscreen_bridge>;
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			port@0 {
> +				reg = <0>;
> +				pitouchscreen_bridge_port: endpoint {
> +					remote-endpoint = 
<&pitouchscreen_panel_port>;
> +				};
> +			};
> +		};
> +	};
> +};
> +
> +i2c_dsi: i2c {
> +	compatible = "i2c-gpio";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	gpios = <&gpio 28 0
> +		 &gpio 29 0>;
> +
> +	pitouchscreen_bridge: bridge@45 {
> +		compatible = "raspberrypi,touchscreen-bridge-i2c";
> +		reg = <0x45>;
> +	};
> +};
> +
> +lcd {
> +	compatible = "raspberrypi,7inch-touchscreen-panel";
> +	ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		port@0 {
> +			reg = <0>;
> +			pitouchscreen_panel_port: endpoint {
> +				remote-endpoint = 
<&pitouchscreen_bridge_port>;
> +			};
> +		};
> +	};
> +};
> diff --git
> a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchsc
> reen-panel.txt
> b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchsc
> reen-panel.txt new file mode 100644
> index 000000000000..1e84f97b3b20
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchsc
> reen-panel.txt @@ -0,0 +1,7 @@
> +Official 7" (800x480) Raspberry Pi touchscreen panel's panel.
> +
> +This binding is compatible with the simple-panel binding, which is
> specified +in simple-panel.txt in this directory.
> +
> +Required properties:
> +- compatible:	Must be "raspberrypi,7inch-touchscreen-panel"

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

[Eric Anholt]

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

Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> Hi Eric,
>
> Thank you for the patch.
>
> On Thursday 11 May 2017 16:56:23 Eric Anholt wrote:
>> The Raspberry Pi 7" Touchscreen is a DPI touchscreen panel with
>> DSI->DPI bridge and touchscreen controller integrated, that connects
>> to the Raspberry Pi through its 15-pin "DSI" connector (some lines are
>> DSI, some lines are I2C).
>> 
>> This device is represented in the DT as three nodes (DSI device, I2C
>> device, panel).  Input will be left to a separate binding later, as it
>> will be a basic I2C client device.
>> 
>> Signed-off-by: Eric Anholt <eric@anholt.net>
>> ---
>>  .../raspberrypi,7inch-touchscreen-bridge.txt       | 68 +++++++++++++++++++
>>  .../panel/raspberrypi,7inch-touchscreen-panel.txt  |  7 +++
>>  2 files changed, 75 insertions(+)
>>  create mode 100644
>> Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchscr
>> een-bridge.txt create mode 100644
>> Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscre
>> en-panel.txt
>> 
>> diff --git
>> a/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchs
>> creen-bridge.txt
>> b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchs
>> creen-bridge.txt new file mode 100644
>> index 000000000000..a5669beaf68f
>> --- /dev/null
>> +++
>> b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchs
>> creen-bridge.txt @@ -0,0 +1,68 @@
>> +Official 7" (800x480) Raspberry Pi touchscreen panel's bridge.
>> +
>> +This DSI panel contains:
>> +
>> +- TC358762 DSI->DPI bridge
>> +- Atmel microcontroller on I2C for power sequencing the DSI bridge and
>> +  controlling backlight
>> +- Touchscreen controller on I2C for touch input
>> +
>> +and this covers the TC358762 bridge and Atmel microcontroller, while
>> +../panel/raspberrypi,7inch-touchscreen-panel.txt covers the panel.
>
> The TC358762 is a standalone bridge that doesn't depend on the ATTiny 
> microcontroller used by the RPI. As it's usable standalone, I believe this 
> binding should be split in two.

Do you have a plan for how I would implement a driver on top of that
binding change, though?  Note that we don't program the Toshiba
directly, we only send commands to the Atmel.

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

Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

[Rob Herring]

On Mon, May 15, 2017 at 7:03 PM, Eric Anholt <eric@anholt.net> wrote:
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
>
>> Hi Eric,
>>
>> Thank you for the patch.
>>
>> On Thursday 11 May 2017 16:56:23 Eric Anholt wrote:
>>> The Raspberry Pi 7" Touchscreen is a DPI touchscreen panel with
>>> DSI->DPI bridge and touchscreen controller integrated, that connects
>>> to the Raspberry Pi through its 15-pin "DSI" connector (some lines are
>>> DSI, some lines are I2C).
>>>
>>> This device is represented in the DT as three nodes (DSI device, I2C
>>> device, panel).  Input will be left to a separate binding later, as it
>>> will be a basic I2C client device.
>>>
>>> Signed-off-by: Eric Anholt <eric@anholt.net>
>>> ---
>>>  .../raspberrypi,7inch-touchscreen-bridge.txt       | 68 +++++++++++++++++++
>>>  .../panel/raspberrypi,7inch-touchscreen-panel.txt  |  7 +++
>>>  2 files changed, 75 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchscr
>>> een-bridge.txt create mode 100644
>>> Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscre
>>> en-panel.txt
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchs
>>> creen-bridge.txt
>>> b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchs
>>> creen-bridge.txt new file mode 100644
>>> index 000000000000..a5669beaf68f
>>> --- /dev/null
>>> +++
>>> b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchs
>>> creen-bridge.txt @@ -0,0 +1,68 @@
>>> +Official 7" (800x480) Raspberry Pi touchscreen panel's bridge.
>>> +
>>> +This DSI panel contains:
>>> +
>>> +- TC358762 DSI->DPI bridge
>>> +- Atmel microcontroller on I2C for power sequencing the DSI bridge and
>>> +  controlling backlight
>>> +- Touchscreen controller on I2C for touch input
>>> +
>>> +and this covers the TC358762 bridge and Atmel microcontroller, while
>>> +../panel/raspberrypi,7inch-touchscreen-panel.txt covers the panel.
>>
>> The TC358762 is a standalone bridge that doesn't depend on the ATTiny
>> microcontroller used by the RPI. As it's usable standalone, I believe this
>> binding should be split in two.
>
> Do you have a plan for how I would implement a driver on top of that
> binding change, though?  Note that we don't program the Toshiba
> directly, we only send commands to the Atmel.

I agree. If it is a black box and the interface to the host is defined
by the Atmel uC firmware, then that's what the DT should describe.
Perhaps a diagram here or pointer to one would help and remove
mentioning what kind of bridge chip it is.

Rob

Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

[Eric Anholt]

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

Rob Herring <robh+dt@kernel.org> writes:

> On Mon, May 15, 2017 at 7:03 PM, Eric Anholt <eric@anholt.net> wrote:
>> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
>>
>>> Hi Eric,
>>>
>>> Thank you for the patch.
>>>
>>> On Thursday 11 May 2017 16:56:23 Eric Anholt wrote:
>>>> The Raspberry Pi 7" Touchscreen is a DPI touchscreen panel with
>>>> DSI->DPI bridge and touchscreen controller integrated, that connects
>>>> to the Raspberry Pi through its 15-pin "DSI" connector (some lines are
>>>> DSI, some lines are I2C).
>>>>
>>>> This device is represented in the DT as three nodes (DSI device, I2C
>>>> device, panel).  Input will be left to a separate binding later, as it
>>>> will be a basic I2C client device.
>>>>
>>>> Signed-off-by: Eric Anholt <eric@anholt.net>
>>>> ---
>>>>  .../raspberrypi,7inch-touchscreen-bridge.txt       | 68 +++++++++++++++++++
>>>>  .../panel/raspberrypi,7inch-touchscreen-panel.txt  |  7 +++
>>>>  2 files changed, 75 insertions(+)
>>>>  create mode 100644
>>>> Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchscr
>>>> een-bridge.txt create mode 100644
>>>> Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscre
>>>> en-panel.txt
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchs
>>>> creen-bridge.txt
>>>> b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchs
>>>> creen-bridge.txt new file mode 100644
>>>> index 000000000000..a5669beaf68f
>>>> --- /dev/null
>>>> +++
>>>> b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchs
>>>> creen-bridge.txt @@ -0,0 +1,68 @@
>>>> +Official 7" (800x480) Raspberry Pi touchscreen panel's bridge.
>>>> +
>>>> +This DSI panel contains:
>>>> +
>>>> +- TC358762 DSI->DPI bridge
>>>> +- Atmel microcontroller on I2C for power sequencing the DSI bridge and
>>>> +  controlling backlight
>>>> +- Touchscreen controller on I2C for touch input
>>>> +
>>>> +and this covers the TC358762 bridge and Atmel microcontroller, while
>>>> +../panel/raspberrypi,7inch-touchscreen-panel.txt covers the panel.
>>>
>>> The TC358762 is a standalone bridge that doesn't depend on the ATTiny
>>> microcontroller used by the RPI. As it's usable standalone, I believe this
>>> binding should be split in two.
>>
>> Do you have a plan for how I would implement a driver on top of that
>> binding change, though?  Note that we don't program the Toshiba
>> directly, we only send commands to the Atmel.
>
> I agree. If it is a black box and the interface to the host is defined
> by the Atmel uC firmware, then that's what the DT should describe.
> Perhaps a diagram here or pointer to one would help and remove
> mentioning what kind of bridge chip it is.

It's a *very* black box.  I have some non-public schematics that don't
even say what panel is involved, and no documentation of the uc
interface.  The driver code is just replicating the firmware's
programming sequence.

I would certainly love to be building a generic TC358762 driver, which
would be a lot more satisfying.  I just don't think it's doable for this
panel.  Given that, what do I need to do to the DT?  Should I just drop
mention of the Toshiba and talk about this being a bridge with a custom
microcontroller firmware?

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

Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

[Laurent Pinchart]

Hi Eric,

On Tuesday 16 May 2017 09:47:49 Eric Anholt wrote:
> Rob Herring <robh+dt@kernel.org> writes:
> > On Mon, May 15, 2017 at 7:03 PM, Eric Anholt <eric@anholt.net> wrote:
> >> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
> >>> Hi Eric,
> >>> 
> >>> Thank you for the patch.
> >>> 
> >>> On Thursday 11 May 2017 16:56:23 Eric Anholt wrote:
> >>>> The Raspberry Pi 7" Touchscreen is a DPI touchscreen panel with
> >>>> DSI->DPI bridge and touchscreen controller integrated, that connects
> >>>> to the Raspberry Pi through its 15-pin "DSI" connector (some lines are
> >>>> DSI, some lines are I2C).
> >>>> 
> >>>> This device is represented in the DT as three nodes (DSI device, I2C
> >>>> device, panel).  Input will be left to a separate binding later, as it
> >>>> will be a basic I2C client device.
> >>>> 
> >>>> Signed-off-by: Eric Anholt <eric@anholt.net>
> >>>> ---
> >>>> 
> >>>>  .../raspberrypi,7inch-touchscreen-bridge.txt       | 68 ++++++++++++++
> >>>>  .../panel/raspberrypi,7inch-touchscreen-panel.txt  |  7 +++
> >>>>  2 files changed, 75 insertions(+)
> >>>>  create mode 100644
> >>>> 
> >>>> Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touc
> >>>> hscreen-bridge.txt create mode 100644
> >>>> Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touch
> >>>> screen-panel.txt
> >>>> 
> >>>> diff --git
> >>>> a/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-to
> >>>> uchscreen-bridge.txt
> >>>> b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-to
> >>>> uchscreen-bridge.txt new file mode 100644
> >>>> index 000000000000..a5669beaf68f
> >>>> --- /dev/null
> >>>> +++
> >>>> b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-to
> >>>> uchscreen-bridge.txt
> >>>> @@ -0,0 +1,68 @@
> >>>> +Official 7" (800x480) Raspberry Pi touchscreen panel's bridge.
> >>>> +
> >>>> +This DSI panel contains:
> >>>> +
> >>>> +- TC358762 DSI->DPI bridge
> >>>> +- Atmel microcontroller on I2C for power sequencing the DSI bridge and
> >>>> +  controlling backlight
> >>>> +- Touchscreen controller on I2C for touch input
> >>>> +
> >>>> +and this covers the TC358762 bridge and Atmel microcontroller, while
> >>>> +../panel/raspberrypi,7inch-touchscreen-panel.txt covers the panel.
> >>> 
> >>> The TC358762 is a standalone bridge that doesn't depend on the ATTiny
> >>> microcontroller used by the RPI. As it's usable standalone, I believe
> >>> this binding should be split in two.
> >> 
> >> Do you have a plan for how I would implement a driver on top of that
> >> binding change, though?  Note that we don't program the Toshiba
> >> directly, we only send commands to the Atmel.
> > 
> > I agree. If it is a black box and the interface to the host is defined
> > by the Atmel uC firmware, then that's what the DT should describe.
> > Perhaps a diagram here or pointer to one would help and remove
> > mentioning what kind of bridge chip it is.
> 
> It's a *very* black box.  I have some non-public schematics that don't
> even say what panel is involved, and no documentation of the uc
> interface.  The driver code is just replicating the firmware's
> programming sequence.
> 
> I would certainly love to be building a generic TC358762 driver, which
> would be a lot more satisfying.  I just don't think it's doable for this
> panel.  Given that, what do I need to do to the DT?  Should I just drop
> mention of the Toshiba and talk about this being a bridge with a custom
> microcontroller firmware?

I think that would be best, yes. Could you share a simple block-diagram of the 
hardware ? It would help turning my random advices into semi-random advices 
:-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

[Eric Anholt]

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

Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> Hi Eric,
>
> On Tuesday 16 May 2017 09:47:49 Eric Anholt wrote:
>> Rob Herring <robh+dt@kernel.org> writes:
>> > On Mon, May 15, 2017 at 7:03 PM, Eric Anholt <eric@anholt.net> wrote:
>> >> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
>> >>> Hi Eric,
>> >>> 
>> >>> Thank you for the patch.
>> >>> 
>> >>> On Thursday 11 May 2017 16:56:23 Eric Anholt wrote:
>> >>>> The Raspberry Pi 7" Touchscreen is a DPI touchscreen panel with
>> >>>> DSI->DPI bridge and touchscreen controller integrated, that connects
>> >>>> to the Raspberry Pi through its 15-pin "DSI" connector (some lines are
>> >>>> DSI, some lines are I2C).
>> >>>> 
>> >>>> This device is represented in the DT as three nodes (DSI device, I2C
>> >>>> device, panel).  Input will be left to a separate binding later, as it
>> >>>> will be a basic I2C client device.
>> >>>> 
>> >>>> Signed-off-by: Eric Anholt <eric@anholt.net>
>> >>>> ---
>> >>>> 
>> >>>>  .../raspberrypi,7inch-touchscreen-bridge.txt       | 68 ++++++++++++++
>> >>>>  .../panel/raspberrypi,7inch-touchscreen-panel.txt  |  7 +++
>> >>>>  2 files changed, 75 insertions(+)
>> >>>>  create mode 100644
>> >>>> 
>> >>>> Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touc
>> >>>> hscreen-bridge.txt create mode 100644
>> >>>> Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touch
>> >>>> screen-panel.txt
>> >>>> 
>> >>>> diff --git
>> >>>> a/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-to
>> >>>> uchscreen-bridge.txt
>> >>>> b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-to
>> >>>> uchscreen-bridge.txt new file mode 100644
>> >>>> index 000000000000..a5669beaf68f
>> >>>> --- /dev/null
>> >>>> +++
>> >>>> b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-to
>> >>>> uchscreen-bridge.txt
>> >>>> @@ -0,0 +1,68 @@
>> >>>> +Official 7" (800x480) Raspberry Pi touchscreen panel's bridge.
>> >>>> +
>> >>>> +This DSI panel contains:
>> >>>> +
>> >>>> +- TC358762 DSI->DPI bridge
>> >>>> +- Atmel microcontroller on I2C for power sequencing the DSI bridge and
>> >>>> +  controlling backlight
>> >>>> +- Touchscreen controller on I2C for touch input
>> >>>> +
>> >>>> +and this covers the TC358762 bridge and Atmel microcontroller, while
>> >>>> +../panel/raspberrypi,7inch-touchscreen-panel.txt covers the panel.
>> >>> 
>> >>> The TC358762 is a standalone bridge that doesn't depend on the ATTiny
>> >>> microcontroller used by the RPI. As it's usable standalone, I believe
>> >>> this binding should be split in two.
>> >> 
>> >> Do you have a plan for how I would implement a driver on top of that
>> >> binding change, though?  Note that we don't program the Toshiba
>> >> directly, we only send commands to the Atmel.
>> > 
>> > I agree. If it is a black box and the interface to the host is defined
>> > by the Atmel uC firmware, then that's what the DT should describe.
>> > Perhaps a diagram here or pointer to one would help and remove
>> > mentioning what kind of bridge chip it is.
>> 
>> It's a *very* black box.  I have some non-public schematics that don't
>> even say what panel is involved, and no documentation of the uc
>> interface.  The driver code is just replicating the firmware's
>> programming sequence.
>> 
>> I would certainly love to be building a generic TC358762 driver, which
>> would be a lot more satisfying.  I just don't think it's doable for this
>> panel.  Given that, what do I need to do to the DT?  Should I just drop
>> mention of the Toshiba and talk about this being a bridge with a custom
>> microcontroller firmware?
>
> I think that would be best, yes. Could you share a simple block-diagram of the 
> hardware ? It would help turning my random advices into semi-random advices 
> :-)

In terms of physical connections:

   [15-pin "DSI" connector on 2835]
    |                   |
    | I2C               | DSI
    |                   |
   / \        SPI       |
[TS]  [Atmel]------[TC358762]
       \                |
        \PWM            |
         \              | DPI
[some backlight]------[some unknown panel]

The binding I'm trying to create is to expose what's necessary for a
driver that talks I2C to the Atmel, which then controls the PWM and does
the command sequence over SPI to the Toshiba that sets up its end of the
DSI link.

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

Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

[Archit Taneja]

Hi,

On 05/17/2017 12:16 AM, Eric Anholt wrote:
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
>
>> Hi Eric,
>>
>> On Tuesday 16 May 2017 09:47:49 Eric Anholt wrote:
>>> Rob Herring <robh+dt@kernel.org> writes:
>>>> On Mon, May 15, 2017 at 7:03 PM, Eric Anholt <eric@anholt.net> wrote:
>>>>> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
>>>>>> Hi Eric,
>>>>>>
>>>>>> Thank you for the patch.
>>>>>>
>>>>>> On Thursday 11 May 2017 16:56:23 Eric Anholt wrote:
>>>>>>> The Raspberry Pi 7" Touchscreen is a DPI touchscreen panel with
>>>>>>> DSI->DPI bridge and touchscreen controller integrated, that connects
>>>>>>> to the Raspberry Pi through its 15-pin "DSI" connector (some lines are
>>>>>>> DSI, some lines are I2C).
>>>>>>>
>>>>>>> This device is represented in the DT as three nodes (DSI device, I2C
>>>>>>> device, panel).  Input will be left to a separate binding later, as it
>>>>>>> will be a basic I2C client device.
>>>>>>>
>>>>>>> Signed-off-by: Eric Anholt <eric@anholt.net>
>>>>>>> ---
>>>>>>>
>>>>>>>  .../raspberrypi,7inch-touchscreen-bridge.txt       | 68 ++++++++++++++
>>>>>>>  .../panel/raspberrypi,7inch-touchscreen-panel.txt  |  7 +++
>>>>>>>  2 files changed, 75 insertions(+)
>>>>>>>  create mode 100644
>>>>>>>
>>>>>>> Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touc
>>>>>>> hscreen-bridge.txt create mode 100644
>>>>>>> Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touch
>>>>>>> screen-panel.txt
>>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-to
>>>>>>> uchscreen-bridge.txt
>>>>>>> b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-to
>>>>>>> uchscreen-bridge.txt new file mode 100644
>>>>>>> index 000000000000..a5669beaf68f
>>>>>>> --- /dev/null
>>>>>>> +++
>>>>>>> b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-to
>>>>>>> uchscreen-bridge.txt
>>>>>>> @@ -0,0 +1,68 @@
>>>>>>> +Official 7" (800x480) Raspberry Pi touchscreen panel's bridge.
>>>>>>> +
>>>>>>> +This DSI panel contains:
>>>>>>> +
>>>>>>> +- TC358762 DSI->DPI bridge
>>>>>>> +- Atmel microcontroller on I2C for power sequencing the DSI bridge and
>>>>>>> +  controlling backlight
>>>>>>> +- Touchscreen controller on I2C for touch input
>>>>>>> +
>>>>>>> +and this covers the TC358762 bridge and Atmel microcontroller, while
>>>>>>> +../panel/raspberrypi,7inch-touchscreen-panel.txt covers the panel.
>>>>>>
>>>>>> The TC358762 is a standalone bridge that doesn't depend on the ATTiny
>>>>>> microcontroller used by the RPI. As it's usable standalone, I believe
>>>>>> this binding should be split in two.
>>>>>
>>>>> Do you have a plan for how I would implement a driver on top of that
>>>>> binding change, though?  Note that we don't program the Toshiba
>>>>> directly, we only send commands to the Atmel.
>>>>
>>>> I agree. If it is a black box and the interface to the host is defined
>>>> by the Atmel uC firmware, then that's what the DT should describe.
>>>> Perhaps a diagram here or pointer to one would help and remove
>>>> mentioning what kind of bridge chip it is.
>>>
>>> It's a *very* black box.  I have some non-public schematics that don't
>>> even say what panel is involved, and no documentation of the uc
>>> interface.  The driver code is just replicating the firmware's
>>> programming sequence.
>>>
>>> I would certainly love to be building a generic TC358762 driver, which
>>> would be a lot more satisfying.  I just don't think it's doable for this
>>> panel.  Given that, what do I need to do to the DT?  Should I just drop
>>> mention of the Toshiba and talk about this being a bridge with a custom
>>> microcontroller firmware?
>>
>> I think that would be best, yes. Could you share a simple block-diagram of the
>> hardware ? It would help turning my random advices into semi-random advices
>> :-)
>
> In terms of physical connections:
>
>    [15-pin "DSI" connector on 2835]
>     |                   |
>     | I2C               | DSI
>     |                   |
>    / \        SPI       |
> [TS]  [Atmel]------[TC358762]
>        \                |
>         \PWM            |
>          \              | DPI
> [some backlight]------[some unknown panel]
>
> The binding I'm trying to create is to expose what's necessary for a
> driver that talks I2C to the Atmel, which then controls the PWM and does
> the command sequence over SPI to the Toshiba that sets up its end of the
> DSI link.
>

The bridge (Atmel + TC358762 combination) here looks like it's primarily
an i2c device (i.e, the control bus is i2c). Therefore, the drm-bridge
driver here should be an i2c driver instead of a mipi_dsi_driver.

We have the facility to create a mipi DSI device without the need to have
a corresponding node in DT. The ADV7533 and TC358767 drivers are examples
of that.

The following is what the binding could look like, it's same as what Rob
also mentioned previously in the thread.

Thanks,
Archit

dsi1: dsi@7e700000 {
	#address-cells = <1>;
	#size-cells = <0>;
	<...>

	/* The SoC's DSI input/output port */
	ports {
		#address-cells = <1>;
		#size-cells = <0>;

		/* port@0 if needed */

		port@1 {
			dsi_out_port: endpoint {
				reg = <1>;
				remote-endpoint = <&bridge_dsi_port>;
			};
		};
	};
};

i2c_dsi: i2c {
	compatible = "i2c-gpio";
	#address-cells = <1>;
	#size-cells = <0>;
	gpios = <&gpio 28 0
		 &gpio 29 0>;

	/* the Atmel + TC35872 bridge */
	pitouchscreen_bridge: bridge@45 {
		compatible = "raspberrypi,touchscreen-bridge";
		reg = <0x45>;

		ports {
			#address-cells = <1>;
			#size-cells = <0>;

			port@0 {
				reg = <0>;
				bridge_dsi_port: endpoint {
					remote-endpoint = <&dsi_out_port>;
				};
			};
			port@1 {
				reg = <1>;
				bridge_dpi_port: endpoint {
					remote-endpoint = <&pitouchscreen_panel_port>;
				};
			};
		};
	};
};

lcd {
	compatible = "raspberrypi,7inch-touchscreen-panel";
	ports {
		#address-cells = <1>;
		#size-cells = <0>;
		port@0 {
			reg = <0>;
			pitouchscreen_panel_port: endpoint {
				remote-endpoint = <&bridge_dpi_port>;
			};
		};
	};
};

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

[Laurent Pinchart]

Hi Archit,

On Thursday 18 May 2017 13:56:19 Archit Taneja wrote:
> On 05/17/2017 12:16 AM, Eric Anholt wrote:

[snip]

> > In terms of physical connections:
> >    [15-pin "DSI" connector on 2835]
> >    
> >     | I2C               | DSI
> >    
> >    / \        SPI       |
> > 
> > [TS]  [Atmel]------[TC358762]
> > 
> >        \                |
> >        
> >         \PWM            |
> >         
> >          \              | DPI
> > 
> > [some backlight]------[some unknown panel]
> > 
> > The binding I'm trying to create is to expose what's necessary for a
> > driver that talks I2C to the Atmel, which then controls the PWM and does
> > the command sequence over SPI to the Toshiba that sets up its end of the
> > DSI link.
> 
> The bridge (Atmel + TC358762 combination) here looks like it's primarily
> an i2c device (i.e, the control bus is i2c). Therefore, the drm-bridge
> driver here should be an i2c driver instead of a mipi_dsi_driver.

Glad to see we agree, that's what I've proposed in a separate answer :-) I'd 
go one step further though, there should be no DRM bridge, just a DRM panel.

> We have the facility to create a mipi DSI device without the need to have
> a corresponding node in DT. The ADV7533 and TC358767 drivers are examples
> of that.
> 
> The following is what the binding could look like, it's same as what Rob
> also mentioned previously in the thread.
> 
> Thanks,
> Archit
> 
> dsi1: dsi@7e700000 {
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 	<...>
> 
> 	/* The SoC's DSI input/output port */
> 	ports {
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		/* port@0 if needed */
> 
> 		port@1 {
> 			dsi_out_port: endpoint {
> 				reg = <1>;
> 				remote-endpoint = <&bridge_dsi_port>;
> 			};
> 		};
> 	};
> };
> 
> i2c_dsi: i2c {
> 	compatible = "i2c-gpio";
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 	gpios = <&gpio 28 0
> 		 &gpio 29 0>;
> 
> 	/* the Atmel + TC35872 bridge */
> 	pitouchscreen_bridge: bridge@45 {

This should thus be lcd@45.

> 		compatible = "raspberrypi,touchscreen-bridge";

And this raspberrypi,7inch-touchscreen-panel. Shame we haven't standardized 
the vendor name prefix to rpi :-/

> 		reg = <0x45>;
> 
> 		ports {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 
> 			port@0 {
> 				reg = <0>;
> 				bridge_dsi_port: endpoint {

This should be named panel_dsi_port.

> 					remote-endpoint = <&dsi_out_port>;
> 				};
> 			};
> 			port@1 {
> 				reg = <1>;
> 				bridge_dpi_port: endpoint {
> 					remote-endpoint = 
<&pitouchscreen_panel_port>;
> 				};
> 			};

The second port is thus not needed.

> 		};

So we can simplify this to

		port {
			panel_dsi_port: endpoint {
				remote-endpoint = <&dsi_out_port>;
			};
		};

(no need for a ports node when there's a single port)

> 	};
> };
> 
> lcd {
> 	compatible = "raspberrypi,7inch-touchscreen-panel";
> 	ports {
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 		port@0 {
> 			reg = <0>;
> 			pitouchscreen_panel_port: endpoint {
> 				remote-endpoint = <&bridge_dpi_port>;
> 			};
> 		};
> 	};
> };

And this node can go away.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

[Archit Taneja]

On 05/18/2017 08:25 PM, Laurent Pinchart wrote:
> Hi Archit,
>
> On Thursday 18 May 2017 13:56:19 Archit Taneja wrote:
>> On 05/17/2017 12:16 AM, Eric Anholt wrote:
>
> [snip]
>
>>> In terms of physical connections:
>>>    [15-pin "DSI" connector on 2835]
>>>
>>>     | I2C               | DSI
>>>
>>>    / \        SPI       |
>>>
>>> [TS]  [Atmel]------[TC358762]
>>>
>>>        \                |
>>>
>>>         \PWM            |
>>>
>>>          \              | DPI
>>>
>>> [some backlight]------[some unknown panel]
>>>
>>> The binding I'm trying to create is to expose what's necessary for a
>>> driver that talks I2C to the Atmel, which then controls the PWM and does
>>> the command sequence over SPI to the Toshiba that sets up its end of the
>>> DSI link.
>>
>> The bridge (Atmel + TC358762 combination) here looks like it's primarily
>> an i2c device (i.e, the control bus is i2c). Therefore, the drm-bridge
>> driver here should be an i2c driver instead of a mipi_dsi_driver.
>
> Glad to see we agree, that's what I've proposed in a separate answer :-) I'd
> go one step further though, there should be no DRM bridge, just a DRM panel.

If the PCB containing the controller chips and the panel are part of a single
casing, and the set up won't work with another panel, then yeah, I agree. If the
bridge chips are on a separate adapter board, and there is a possibility to connect
other panels, then maybe a separate DRM bridge and a DRM panel might be a safer bet.

Thanks,
Archit

>
>> We have the facility to create a mipi DSI device without the need to have
>> a corresponding node in DT. The ADV7533 and TC358767 drivers are examples
>> of that.
>>
>> The following is what the binding could look like, it's same as what Rob
>> also mentioned previously in the thread.
>>
>> Thanks,
>> Archit
>>
>> dsi1: dsi@7e700000 {
>> 	#address-cells = <1>;
>> 	#size-cells = <0>;
>> 	<...>
>>
>> 	/* The SoC's DSI input/output port */
>> 	ports {
>> 		#address-cells = <1>;
>> 		#size-cells = <0>;
>>
>> 		/* port@0 if needed */
>>
>> 		port@1 {
>> 			dsi_out_port: endpoint {
>> 				reg = <1>;
>> 				remote-endpoint = <&bridge_dsi_port>;
>> 			};
>> 		};
>> 	};
>> };
>>
>> i2c_dsi: i2c {
>> 	compatible = "i2c-gpio";
>> 	#address-cells = <1>;
>> 	#size-cells = <0>;
>> 	gpios = <&gpio 28 0
>> 		 &gpio 29 0>;
>>
>> 	/* the Atmel + TC35872 bridge */
>> 	pitouchscreen_bridge: bridge@45 {
>
> This should thus be lcd@45.
>
>> 		compatible = "raspberrypi,touchscreen-bridge";
>
> And this raspberrypi,7inch-touchscreen-panel. Shame we haven't standardized
> the vendor name prefix to rpi :-/
>
>> 		reg = <0x45>;
>>
>> 		ports {
>> 			#address-cells = <1>;
>> 			#size-cells = <0>;
>>
>> 			port@0 {
>> 				reg = <0>;
>> 				bridge_dsi_port: endpoint {
>
> This should be named panel_dsi_port.
>
>> 					remote-endpoint = <&dsi_out_port>;
>> 				};
>> 			};
>> 			port@1 {
>> 				reg = <1>;
>> 				bridge_dpi_port: endpoint {
>> 					remote-endpoint =
> <&pitouchscreen_panel_port>;
>> 				};
>> 			};
>
> The second port is thus not needed.
>
>> 		};
>
> So we can simplify this to
>
> 		port {
> 			panel_dsi_port: endpoint {
> 				remote-endpoint = <&dsi_out_port>;
> 			};
> 		};
>
> (no need for a ports node when there's a single port)
>
>> 	};
>> };
>>
>> lcd {
>> 	compatible = "raspberrypi,7inch-touchscreen-panel";
>> 	ports {
>> 		#address-cells = <1>;
>> 		#size-cells = <0>;
>> 		port@0 {
>> 			reg = <0>;
>> 			pitouchscreen_panel_port: endpoint {
>> 				remote-endpoint = <&bridge_dpi_port>;
>> 			};
>> 		};
>> 	};
>> };
>
> And this node can go away.
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

[Laurent Pinchart]

Hi Archit,

On Friday 19 May 2017 14:24:36 Archit Taneja wrote:
> On 05/18/2017 08:25 PM, Laurent Pinchart wrote:
> > On Thursday 18 May 2017 13:56:19 Archit Taneja wrote:
> >> On 05/17/2017 12:16 AM, Eric Anholt wrote:
> >
> > [snip]
> > 
> >>> In terms of physical connections:
> >>>    [15-pin "DSI" connector on 2835]
> >>>     | I2C               | DSI
> >>>    / \        SPI       |
> >>> [TS]  [Atmel]------[TC358762]
> >>>        \                |
> >>>         \PWM            |
> >>>          \              | DPI
> >>> 
> >>> [some backlight]------[some unknown panel]
> >>> 
> >>> The binding I'm trying to create is to expose what's necessary for a
> >>> driver that talks I2C to the Atmel, which then controls the PWM and does
> >>> the command sequence over SPI to the Toshiba that sets up its end of the
> >>> DSI link.
> >> 
> >> The bridge (Atmel + TC358762 combination) here looks like it's primarily
> >> an i2c device (i.e, the control bus is i2c). Therefore, the drm-bridge
> >> driver here should be an i2c driver instead of a mipi_dsi_driver.
> > 
> > Glad to see we agree, that's what I've proposed in a separate answer :-)
> > I'd go one step further though, there should be no DRM bridge, just a DRM
> > panel.
>
> If the PCB containing the controller chips and the panel are part of a
> single casing, and the set up won't work with another panel, then yeah, I
> agree. If the bridge chips are on a separate adapter board, and there is a
> possibility to connect other panels, then maybe a separate DRM bridge and a
> DRM panel might be a safer bet.

I thought it was a single black box, but upon closer inspection there's a 
separate PCB with the Microcontroller and TC358762.

Eric, do you know if it's possible to exchange the panel for another one (and 
not just an model with identical features from another vendor, but another 
panel with a different mode for instance) without reprogramming the 
microcontroller, or is the bridge board tied to the panel model ?

> >> We have the facility to create a mipi DSI device without the need to have
> >> a corresponding node in DT. The ADV7533 and TC358767 drivers are examples
> >> of that.
> >> 
> >> The following is what the binding could look like, it's same as what Rob
> >> also mentioned previously in the thread.
> >> 
> >> Thanks,
> >> Archit
> >> 
> >> dsi1: dsi@7e700000 {
> >> 
> >> 	#address-cells = <1>;
> >> 	#size-cells = <0>;
> >> 	<...>
> >> 	
> >> 	/* The SoC's DSI input/output port */
> >> 	ports {
> >> 		#address-cells = <1>;
> >> 		#size-cells = <0>;
> >> 		
> >> 		/* port@0 if needed */
> >> 		port@1 {
> >> 			dsi_out_port: endpoint {
> >> 				reg = <1>;
> >> 				remote-endpoint = <&bridge_dsi_port>;
> >> 			};
> >> 		};
> >> 	};
> >> };
> >> 
> >> i2c_dsi: i2c {
> >> 	compatible = "i2c-gpio";
> >> 	#address-cells = <1>;
> >> 	#size-cells = <0>;
> >> 	gpios = <&gpio 28 0
> >> 		 &gpio 29 0>;
> >> 	
> >> 	/* the Atmel + TC35872 bridge */
> >> 	pitouchscreen_bridge: bridge@45 {
> > 
> > This should thus be lcd@45.
> > 
> >> 		compatible = "raspberrypi,touchscreen-bridge";
> > 
> > And this raspberrypi,7inch-touchscreen-panel. Shame we haven't
> > standardized
> > the vendor name prefix to rpi :-/
> > 
> >> 		reg = <0x45>;
> >> 		
> >> 		ports {
> >> 			#address-cells = <1>;
> >> 			#size-cells = <0>;
> >> 			port@0 {
> >> 				reg = <0>;
> >> 				bridge_dsi_port: endpoint {
> > 
> > This should be named panel_dsi_port.
> > 
> >> 					remote-endpoint = <&dsi_out_port>;
> >> 				};
> >> 			};
> >> 			port@1 {
> >> 				reg = <1>;
> >> 				bridge_dpi_port: endpoint {
> >> 					remote-endpoint =
> > <&pitouchscreen_panel_port>;
> >> 				};
> >> 			};
> > 
> > The second port is thus not needed.
> > 
> >> 		};
> > 
> > So we can simplify this to
> > 
> > 		port {
> > 			panel_dsi_port: endpoint {
> > 				remote-endpoint = <&dsi_out_port>;
> > 			};
> > 		};
> > 
> > (no need for a ports node when there's a single port)
> > 
> >> 	};
> >> };
> >> 
> >> lcd {
> >> 	compatible = "raspberrypi,7inch-touchscreen-panel";
> >> 	ports {
> >> 		#address-cells = <1>;
> >> 		#size-cells = <0>;
> >> 		port@0 {
> >> 			reg = <0>;
> >> 			pitouchscreen_panel_port: endpoint {
> >> 				remote-endpoint = <&bridge_dpi_port>;
> >> 			};
> >> 		};
> >> 	};
> >> };
> > 
> > And this node can go away.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

[Eric Anholt]

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

Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> Hi Archit,
>
> On Friday 19 May 2017 14:24:36 Archit Taneja wrote:
>> On 05/18/2017 08:25 PM, Laurent Pinchart wrote:
>> > On Thursday 18 May 2017 13:56:19 Archit Taneja wrote:
>> >> On 05/17/2017 12:16 AM, Eric Anholt wrote:
>> >
>> > [snip]
>> > 
>> >>> In terms of physical connections:
>> >>>    [15-pin "DSI" connector on 2835]
>> >>>     | I2C               | DSI
>> >>>    / \        SPI       |
>> >>> [TS]  [Atmel]------[TC358762]
>> >>>        \                |
>> >>>         \PWM            |
>> >>>          \              | DPI
>> >>> 
>> >>> [some backlight]------[some unknown panel]
>> >>> 
>> >>> The binding I'm trying to create is to expose what's necessary for a
>> >>> driver that talks I2C to the Atmel, which then controls the PWM and does
>> >>> the command sequence over SPI to the Toshiba that sets up its end of the
>> >>> DSI link.
>> >> 
>> >> The bridge (Atmel + TC358762 combination) here looks like it's primarily
>> >> an i2c device (i.e, the control bus is i2c). Therefore, the drm-bridge
>> >> driver here should be an i2c driver instead of a mipi_dsi_driver.
>> > 
>> > Glad to see we agree, that's what I've proposed in a separate answer :-)
>> > I'd go one step further though, there should be no DRM bridge, just a DRM
>> > panel.
>>
>> If the PCB containing the controller chips and the panel are part of a
>> single casing, and the set up won't work with another panel, then yeah, I
>> agree. If the bridge chips are on a separate adapter board, and there is a
>> possibility to connect other panels, then maybe a separate DRM bridge and a
>> DRM panel might be a safer bet.
>
> I thought it was a single black box, but upon closer inspection there's a 
> separate PCB with the Microcontroller and TC358762.
>
> Eric, do you know if it's possible to exchange the panel for another one (and 
> not just an model with identical features from another vendor, but another 
> panel with a different mode for instance) without reprogramming the 
> microcontroller, or is the bridge board tied to the panel model ?

Not without finding some other panel with equivalent non-standard
connectors / doing your own soldering, at a minimum.  And we don't know
what kind of programming the microcontroller does, since it's a black
box.

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

Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

[Laurent Pinchart]

Hi Eric,

On Tuesday 16 May 2017 11:46:36 Eric Anholt wrote:

[snip]

> In terms of physical connections:
> 
>    [15-pin "DSI" connector on 2835]
> 
>     | I2C               | DSI
>    / \        SPI       |
> [TS]  [Atmel]------[TC358762]
>        \                |
>         \PWM            |
>          \              | DPI
> [some backlight]------[some unknown panel]
> 
> The binding I'm trying to create is to expose what's necessary for a
> driver that talks I2C to the Atmel, which then controls the PWM and does
> the command sequence over SPI to the Toshiba that sets up its end of the
> DSI link.

According to the documentation I've been able to find, the TC358762 has an SPI 
master port through which it can output the commands DCS received from the DSI 
port, and an I2C slave port through which it can be configured by an external 
device. If the connection between the microcontroller and the TC358762 is 
indeed SPI and not I2C, I assume it's used by the microcontroller to receive 
the DCS commands and perform control of the backlight (and possibly other 
components) accordingly. By the way, is there any place where I can find a 
leaked version of the non-public panel schematics ? ;-)

As far as I can tell from your patch series, you don't need to send any 
command to the TC358762 over DSI. In that case I would model the panel in DT 
as an I2C device, as all control goes through the I2C bus. The DSI video data 
connection should then be modelled using the OF graph DT bindings. The result 
will be a black box panel with a custom black box panel driver, using a single 
DT node. There's no need for a separate bridge instance. That's the cleanest 
option I can come up with so far, and I agree that splitting TC358762 support 
into a standalone bridge driver makes no sense in this case.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

[Eric Anholt]

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

Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> Hi Eric,
>
> On Tuesday 16 May 2017 11:46:36 Eric Anholt wrote:
>
> [snip]
>
>> In terms of physical connections:
>> 
>>    [15-pin "DSI" connector on 2835]
>> 
>>     | I2C               | DSI
>>    / \        SPI       |
>> [TS]  [Atmel]------[TC358762]
>>        \                |
>>         \PWM            |
>>          \              | DPI
>> [some backlight]------[some unknown panel]
>> 
>> The binding I'm trying to create is to expose what's necessary for a
>> driver that talks I2C to the Atmel, which then controls the PWM and does
>> the command sequence over SPI to the Toshiba that sets up its end of the
>> DSI link.
>
> According to the documentation I've been able to find, the TC358762 has an SPI 
> master port through which it can output the commands DCS received from the DSI 
> port, and an I2C slave port through which it can be configured by an external 
> device. If the connection between the microcontroller and the TC358762 is 
> indeed SPI and not I2C, I assume it's used by the microcontroller to receive 
> the DCS commands and perform control of the backlight (and possibly other 
> components) accordingly. By the way, is there any place where I can find a 
> leaked version of the non-public panel schematics ? ;-)

Not that I know of.

I don't know that you can control the backlight over DCS, given that I
have no docs.  We only send commands from Atmel to TC, not the other way
around.

> As far as I can tell from your patch series, you don't need to send any 
> command to the TC358762 over DSI. In that case I would model the panel in DT 
> as an I2C device, as all control goes through the I2C bus. The DSI video data 
> connection should then be modelled using the OF graph DT bindings. The result 
> will be a black box panel with a custom black box panel driver, using a single 
> DT node. There's no need for a separate bridge instance. That's the cleanest 
> option I can come up with so far, and I agree that splitting TC358762 support 
> into a standalone bridge driver makes no sense in this case.

I agree, it's just that when I submitted to drm-panel I was told that it
didn't make sense as a single driver, so I went to all of this work
instead.

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

Re: [PATCH 2/4] dt-bindings: Document the Raspberry Pi Touchscreen nodes.

[Laurent Pinchart]

Hi Eric,

On Monday 15 May 2017 17:03:29 Eric Anholt wrote:
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
> > On Thursday 11 May 2017 16:56:23 Eric Anholt wrote:
> >> The Raspberry Pi 7" Touchscreen is a DPI touchscreen panel with
> >> DSI->DPI bridge and touchscreen controller integrated, that connects
> >> to the Raspberry Pi through its 15-pin "DSI" connector (some lines are
> >> DSI, some lines are I2C).
> >> 
> >> This device is represented in the DT as three nodes (DSI device, I2C
> >> device, panel).  Input will be left to a separate binding later, as it
> >> will be a basic I2C client device.
> >> 
> >> Signed-off-by: Eric Anholt <eric@anholt.net>
> >> ---
> >> 
> >>  .../raspberrypi,7inch-touchscreen-bridge.txt       | 68 +++++++++++++++
> >>  .../panel/raspberrypi,7inch-touchscreen-panel.txt  |  7 +++
> >>  2 files changed, 75 insertions(+)
> >>  create mode 100644
> >> 
> >> Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touchs
> >> cr
> >> een-bridge.txt create mode 100644
> >> Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchsc
> >> re
> >> en-panel.txt
> >> 
> >> diff --git
> >> a/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touc
> >> hs
> >> creen-bridge.txt
> >> b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touc
> >> hs
> >> creen-bridge.txt new file mode 100644
> >> index 000000000000..a5669beaf68f
> >> --- /dev/null
> >> +++
> >> b/Documentation/devicetree/bindings/display/bridge/raspberrypi,7inch-touc
> >> hs
> >> creen-bridge.txt @@ -0,0 +1,68 @@
> >> +Official 7" (800x480) Raspberry Pi touchscreen panel's bridge.
> >> +
> >> +This DSI panel contains:
> >> +
> >> +- TC358762 DSI->DPI bridge
> >> +- Atmel microcontroller on I2C for power sequencing the DSI bridge and
> >> +  controlling backlight
> >> +- Touchscreen controller on I2C for touch input
> >> +
> >> +and this covers the TC358762 bridge and Atmel microcontroller, while
> >> +../panel/raspberrypi,7inch-touchscreen-panel.txt covers the panel.
> > 
> > The TC358762 is a standalone bridge that doesn't depend on the ATTiny
> > microcontroller used by the RPI. As it's usable standalone, I believe this
> > binding should be split in two.
> 
> Do you have a plan for how I would implement a driver on top of that
> binding change, though?  Note that we don't program the Toshiba
> directly, we only send commands to the Atmel.

I'm not too familiar with the hardware architecture, let me have a look. Is 
the schematics publicly available somewhere ?

-- 
Regards,

Laurent Pinchart

[PATCH 3/4] drm/bridge: Add support for the Raspberry Pi 7" Touchscreen.

[Eric Anholt]

This driver communicates with the Atmel microcontroller for sequencing
the poweron of the TC358762 DSI-DPI bridge and controlling the
backlight PWM.

The following lines are required in config.txt, to keep the firmware
from trying to bash our I2C lines and steal the DSI interrupts:

    disable_touchscreen=1
    ignore_lcd=2
    mask_gpu_interrupt1=0x1000

This means that the firmware won't power on the panel at boot time (no
rainbow) and the touchscreen input won't work.  A native input driver
for the touchscreen still needs to be written.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/bridge/Kconfig                   |   9 +
 drivers/gpu/drm/bridge/Makefile                  |   1 +
 drivers/gpu/drm/bridge/raspberrypi-touchscreen.c | 460 +++++++++++++++++++++++
 3 files changed, 470 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/raspberrypi-touchscreen.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index c4daca38743c..9adda62f45df 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -76,6 +76,15 @@ config DRM_SIL_SII8620
 	help
 	  Silicon Image SII8620 HDMI/MHL bridge chip driver.
 
+config DRM_RASPBERRYPI_TOUCHSCREEN_BRIDGE
+	tristate "Raspberry Pi 7-inch touchscreen bridge"
+	depends on OF
+	select DRM_KMS_HELPER
+	help
+	  Say Y here if you want to enable support for the Raspberry
+	  Pi 7" Touchscreen.  To compile this driver as a module,
+	  choose M here.
+
 config DRM_SII902X
 	tristate "Silicon Image sii902x RGB/HDMI bridge"
 	depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 3fe2226ee2f2..6226ed41f4d1 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o
 obj-$(CONFIG_DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW) += megachips-stdpxxxx-ge-b850v3-fw.o
 obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
 obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
+obj-$(CONFIG_DRM_RASPBERRYPI_TOUCHSCREEN_BRIDGE) += raspberrypi-touchscreen.o
 obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
 obj-$(CONFIG_DRM_SII902X) += sii902x.o
 obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
diff --git a/drivers/gpu/drm/bridge/raspberrypi-touchscreen.c b/drivers/gpu/drm/bridge/raspberrypi-touchscreen.c
new file mode 100644
index 000000000000..b19b5a83f210
--- /dev/null
+++ b/drivers/gpu/drm/bridge/raspberrypi-touchscreen.c
@@ -0,0 +1,460 @@
+/*
+ * Copyright © 2016-2017 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Portions of this file (derived from panel-simple.c) are:
+ *
+ * Copyright (C) 2013, NVIDIA Corporation.  All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sub license,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+/**
+ * Raspberry Pi 7" touchscreen panel bridge driver.
+ *
+ * The 7" touchscreen consists of a DPI LCD panel, a Toshiba
+ * TC358762XBG DSI-DPI bridge, and an I2C-connected Atmel ATTINY88-MUR
+ * controlling power management, the LCD PWM, and initial register
+ * setup of the Tohsiba.
+ *
+ * This driver controls the TC358762 and ATTINY88, bridging between
+ * the DSI host and the LCD panel.  The panel-simple driver has the
+ * actual panel.
+ */
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/fb.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_graph.h>
+#include <linux/pm.h>
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_of.h>
+#include <drm/drm_panel.h>
+
+/* I2C registers of the Atmel microcontroller. */
+enum REG_ADDR {
+	REG_ID = 0x80,
+	REG_PORTA, /* BIT(2) for horizontal flip, BIT(3) for vertical flip */
+	REG_PORTB,
+	REG_PORTC,
+	REG_PORTD,
+	REG_POWERON,
+	REG_PWM,
+	REG_DDRA,
+	REG_DDRB,
+	REG_DDRC,
+	REG_DDRD,
+	REG_TEST,
+	REG_WR_ADDRL,
+	REG_WR_ADDRH,
+	REG_READH,
+	REG_READL,
+	REG_WRITEH,
+	REG_WRITEL,
+	REG_ID2,
+};
+
+/* We only turn the PWM on or off, without varying values. */
+#define RPI_TOUCHSCREEN_MAX_BRIGHTNESS 1
+
+/* DSI D-PHY Layer Registers */
+#define D0W_DPHYCONTTX		0x0004
+#define CLW_DPHYCONTRX		0x0020
+#define D0W_DPHYCONTRX		0x0024
+#define D1W_DPHYCONTRX		0x0028
+#define COM_DPHYCONTRX		0x0038
+#define CLW_CNTRL		0x0040
+#define D0W_CNTRL		0x0044
+#define D1W_CNTRL		0x0048
+#define DFTMODE_CNTRL		0x0054
+
+/* DSI PPI Layer Registers */
+#define PPI_STARTPPI		0x0104
+#define PPI_BUSYPPI		0x0108
+#define PPI_LINEINITCNT		0x0110
+#define PPI_LPTXTIMECNT		0x0114
+#define PPI_LANEENABLE		0x0134
+#define PPI_TX_RX_TA		0x013C
+#define PPI_CLS_ATMR		0x0140
+#define PPI_D0S_ATMR		0x0144
+#define PPI_D1S_ATMR		0x0148
+#define PPI_D0S_CLRSIPOCOUNT	0x0164
+#define PPI_D1S_CLRSIPOCOUNT	0x0168
+#define CLS_PRE			0x0180
+#define D0S_PRE			0x0184
+#define D1S_PRE			0x0188
+#define CLS_PREP		0x01A0
+#define D0S_PREP		0x01A4
+#define D1S_PREP		0x01A8
+#define CLS_ZERO		0x01C0
+#define D0S_ZERO		0x01C4
+#define D1S_ZERO		0x01C8
+#define PPI_CLRFLG		0x01E0
+#define PPI_CLRSIPO		0x01E4
+#define HSTIMEOUT		0x01F0
+#define HSTIMEOUTENABLE		0x01F4
+
+/* DSI Protocol Layer Registers */
+#define DSI_STARTDSI		0x0204
+#define DSI_BUSYDSI		0x0208
+#define DSI_LANEENABLE		0x0210
+# define DSI_LANEENABLE_CLOCK		BIT(0)
+# define DSI_LANEENABLE_D0		BIT(1)
+# define DSI_LANEENABLE_D1		BIT(2)
+
+#define DSI_LANESTATUS0		0x0214
+#define DSI_LANESTATUS1		0x0218
+#define DSI_INTSTATUS		0x0220
+#define DSI_INTMASK		0x0224
+#define DSI_INTCLR		0x0228
+#define DSI_LPTXTO		0x0230
+#define DSI_MODE		0x0260
+#define DSI_PAYLOAD0		0x0268
+#define DSI_PAYLOAD1		0x026C
+#define DSI_SHORTPKTDAT		0x0270
+#define DSI_SHORTPKTREQ		0x0274
+#define DSI_BTASTA		0x0278
+#define DSI_BTACLR		0x027C
+
+/* DSI General Registers */
+#define DSIERRCNT		0x0300
+#define DSISIGMOD		0x0304
+
+/* DSI Application Layer Registers */
+#define APLCTRL			0x0400
+#define APLSTAT			0x0404
+#define APLERR			0x0408
+#define PWRMOD			0x040C
+#define RDPKTLN			0x0410
+#define PXLFMT			0x0414
+#define MEMWRCMD		0x0418
+
+/* LCDC/DPI Host Registers */
+#define LCDCTRL			0x0420
+#define HSR			0x0424
+#define HDISPR			0x0428
+#define VSR			0x042C
+#define VDISPR			0x0430
+#define VFUEN			0x0434
+
+/* DBI-B Host Registers */
+#define DBIBCTRL		0x0440
+
+/* SPI Master Registers */
+#define SPICMR			0x0450
+#define SPITCR			0x0454
+
+/* System Controller Registers */
+#define SYSSTAT			0x0460
+#define SYSCTRL			0x0464
+#define SYSPLL1			0x0468
+#define SYSPLL2			0x046C
+#define SYSPLL3			0x0470
+#define SYSPMCTRL		0x047C
+
+/* GPIO Registers */
+#define GPIOC			0x0480
+#define GPIOO			0x0484
+#define GPIOI			0x0488
+
+/* I2C Registers */
+#define I2CCLKCTRL		0x0490
+
+/* Chip/Rev Registers */
+#define IDREG			0x04A0
+
+/* Debug Registers */
+#define WCMDQUEUE		0x0500
+#define RCMDQUEUE		0x0504
+
+struct rpi_touchscreen {
+	struct drm_bridge base;
+	struct drm_bridge *panel_bridge;
+	struct mipi_dsi_device *dsi;
+	struct i2c_client *bridge_i2c;
+};
+
+static struct rpi_touchscreen *bridge_to_ts(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct rpi_touchscreen, base);
+}
+
+static u8 rpi_touchscreen_i2c_read(struct rpi_touchscreen *ts, u8 reg)
+{
+	return i2c_smbus_read_byte_data(ts->bridge_i2c, reg);
+}
+
+static void rpi_touchscreen_i2c_write(struct rpi_touchscreen *ts,
+				      u8 reg, u8 val)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(ts->bridge_i2c, reg, val);
+	if (ret)
+		dev_err(&ts->dsi->dev, "I2C write failed: %d\n", ret);
+}
+
+static int rpi_touchscreen_write(struct rpi_touchscreen *ts, u16 reg, u32 val)
+{
+#if 0
+	/* The firmware uses LP DSI transactions like this to bring up
+	 * the hardware, which should be faster than using I2C to then
+	 * pass to the Toshiba.  However, I was unable to get it to
+	 * work.
+	 */
+	u8 msg[] = {
+		reg,
+		reg >> 8,
+		val,
+		val >> 8,
+		val >> 16,
+		val >> 24,
+	};
+
+	mipi_dsi_dcs_write_buffer(ts->dsi, msg, sizeof(msg));
+#else
+	rpi_touchscreen_i2c_write(ts, REG_WR_ADDRH, reg >> 8);
+	rpi_touchscreen_i2c_write(ts, REG_WR_ADDRL, reg);
+	rpi_touchscreen_i2c_write(ts, REG_WRITEH, val >> 8);
+	rpi_touchscreen_i2c_write(ts, REG_WRITEL, val);
+#endif
+
+	return 0;
+}
+
+static void rpi_bridge_disable(struct drm_bridge *bridge)
+{
+	struct rpi_touchscreen *ts = bridge_to_ts(bridge);
+
+	rpi_touchscreen_i2c_write(ts, REG_PWM, 0);
+
+	rpi_touchscreen_i2c_write(ts, REG_POWERON, 0);
+	udelay(1);
+}
+
+static void rpi_bridge_enable(struct drm_bridge *bridge)
+{
+	struct rpi_touchscreen *ts = bridge_to_ts(bridge);
+	int i;
+
+	rpi_touchscreen_i2c_write(ts, REG_POWERON, 1);
+	/* Wait for nPWRDWN to go low to indicate poweron is done. */
+	for (i = 0; i < 100; i++) {
+		if (rpi_touchscreen_i2c_read(ts, REG_PORTB) & 1)
+			break;
+	}
+
+	rpi_touchscreen_write(ts, DSI_LANEENABLE,
+			      DSI_LANEENABLE_CLOCK |
+			      DSI_LANEENABLE_D0);
+	rpi_touchscreen_write(ts, PPI_D0S_CLRSIPOCOUNT, 0x05);
+	rpi_touchscreen_write(ts, PPI_D1S_CLRSIPOCOUNT, 0x05);
+	rpi_touchscreen_write(ts, PPI_D0S_ATMR, 0x00);
+	rpi_touchscreen_write(ts, PPI_D1S_ATMR, 0x00);
+	rpi_touchscreen_write(ts, PPI_LPTXTIMECNT, 0x03);
+
+	rpi_touchscreen_write(ts, SPICMR, 0x00);
+	rpi_touchscreen_write(ts, LCDCTRL, 0x00100150);
+	rpi_touchscreen_write(ts, SYSCTRL, 0x040f);
+	msleep(100);
+
+	rpi_touchscreen_write(ts, PPI_STARTPPI, 0x01);
+	rpi_touchscreen_write(ts, DSI_STARTDSI, 0x01);
+	msleep(100);
+
+	/* Turn on the backlight. */
+	rpi_touchscreen_i2c_write(ts, REG_PWM, 255);
+
+	/* Default to the same orientation as the closed source
+	 * firmware used for the panel.  Runtime rotation
+	 * configuration will be supported using VC4's plane
+	 * orientation bits.
+	 */
+	rpi_touchscreen_i2c_write(ts, REG_PORTA, BIT(2));
+}
+
+static int rpi_bridge_attach(struct drm_bridge *bridge)
+{
+	struct rpi_touchscreen *ts = bridge_to_ts(bridge);
+
+	return drm_bridge_attach(bridge->encoder, ts->panel_bridge, bridge);
+}
+
+static struct drm_bridge_funcs rpi_bridge_funcs = {
+	.enable = rpi_bridge_enable,
+	.disable = rpi_bridge_disable,
+	.attach = rpi_bridge_attach,
+};
+
+static struct i2c_client *rpi_touchscreen_get_i2c(struct device *dev,
+						  const char *name)
+{
+	struct device_node *node;
+	struct i2c_client *client;
+
+	node = of_parse_phandle(dev->of_node, name, 0);
+	if (!node)
+		return ERR_PTR(-ENODEV);
+
+	client = of_find_i2c_device_by_node(node);
+	of_node_put(node);
+	if (!client)
+		return ERR_PTR(-ENODEV);
+
+	return client;
+}
+
+static int rpi_touchscreen_dsi_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct drm_panel *panel;
+	struct rpi_touchscreen *ts;
+	int ret, ver;
+
+	ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
+	if (!ts)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, ts);
+
+	ts->dsi = dsi;
+	dsi->mode_flags = (MIPI_DSI_MODE_VIDEO |
+			   MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
+			   MIPI_DSI_MODE_LPM);
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->lanes = 1;
+
+	ts->bridge_i2c =
+		rpi_touchscreen_get_i2c(dev, "raspberrypi,touchscreen-bridge");
+	if (IS_ERR(ts->bridge_i2c)) {
+		ret = -EPROBE_DEFER;
+		return ret;
+	}
+
+	ver = rpi_touchscreen_i2c_read(ts, REG_ID);
+	if (ver < 0) {
+		dev_err(dev, "Atmel I2C read failed: %d\n", ver);
+		return -ENODEV;
+	}
+
+	switch (ver) {
+	case 0xde: /* ver 1 */
+	case 0xc3: /* ver 2 */
+		break;
+	default:
+		dev_err(dev, "Unknown Atmel firmware revision: 0x%02x\n", ver);
+		ret = -ENODEV;
+		goto err_release_i2c;
+	}
+
+	drm_of_find_panel_or_bridge(dev->of_node, 0, 0, &panel, NULL);
+	if (!panel)
+		return -EPROBE_DEFER;
+
+	ts->panel_bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_DSI);
+	if (IS_ERR(ts->panel_bridge)) {
+		dev_err(dev, "Failed to create panel bridge\n");
+		ret = PTR_ERR(ts->panel_bridge);
+		goto err_release_i2c;
+	}
+
+	/* Turn off at boot, so we can cleanly sequence powering on. */
+	rpi_touchscreen_i2c_write(ts, REG_POWERON, 0);
+
+	ts->base.funcs = &rpi_bridge_funcs;
+	ts->base.of_node = dev->of_node;
+
+	ret = drm_bridge_add(&ts->base);
+	if (ret) {
+		dev_err(dev, "Failed to add bridge\n");
+		goto err_remove_bridge;
+	}
+
+	return mipi_dsi_attach(dsi);
+
+err_remove_bridge:
+	drm_panel_bridge_remove(ts->panel_bridge);
+err_release_i2c:
+	put_device(&ts->bridge_i2c->dev);
+	return ret;
+}
+
+static int rpi_touchscreen_dsi_remove(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct rpi_touchscreen *ts = dev_get_drvdata(dev);
+	int ret;
+
+	ret = mipi_dsi_detach(dsi);
+	if (ret < 0) {
+		dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", ret);
+		return ret;
+	}
+
+	drm_panel_bridge_remove(ts->panel_bridge);
+	drm_bridge_remove(&ts->base);
+
+	put_device(&ts->bridge_i2c->dev);
+
+	return 0;
+}
+
+static void rpi_touchscreen_dsi_shutdown(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct rpi_touchscreen *ts = dev_get_drvdata(dev);
+
+	rpi_touchscreen_i2c_write(ts, REG_POWERON, 0);
+}
+
+static const struct of_device_id rpi_touchscreen_of_match[] = {
+	{ .compatible = "raspberrypi,7inch-touchscreen-bridge" },
+	{ } /* sentinel */
+};
+MODULE_DEVICE_TABLE(of, rpi_touchscreen_of_match);
+
+static struct mipi_dsi_driver rpi_touchscreen_driver = {
+	.driver = {
+		.name = "raspberrypi-touchscreen-bridge",
+		.of_match_table = rpi_touchscreen_of_match,
+	},
+	.probe = rpi_touchscreen_dsi_probe,
+	.remove = rpi_touchscreen_dsi_remove,
+	.shutdown = rpi_touchscreen_dsi_shutdown,
+};
+module_mipi_dsi_driver(rpi_touchscreen_driver);
+
+MODULE_AUTHOR("Eric Anholt <eric@anholt.net>");
+MODULE_DESCRIPTION("Raspberry Pi 7-inch touchscreen driver");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0

[PATCH 4/4] drm/panel: Add the Raspberry Pi 7" touchscreen's panel.

[Eric Anholt]

The timings are those that the firmware defines as the baseline, which
will be modified by the bridge/host as necessary to get the clocking
to work.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/panel/panel-simple.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index c4566ce8fda7..5bd026cf432b 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1530,6 +1530,33 @@ static const struct panel_desc qd43003c0_40 = {
 	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
 };
 
+static const struct drm_display_mode raspberrypi_7inch_mode = {
+	/* Modeline comes from the Raspberry Pi firmware, with HFP=1
+	 * plugged in and clock re-computed from that.
+	 */
+	.clock = 25979400 / 1000,
+	.hdisplay = 800,
+	.hsync_start = 800 + 1,
+	.hsync_end = 800 + 1 + 2,
+	.htotal = 800 + 1 + 2 + 46,
+	.vdisplay = 480,
+	.vsync_start = 480 + 7,
+	.vsync_end = 480 + 7 + 2,
+	.vtotal = 480 + 7 + 2 + 21,
+	.vrefresh = 60,
+};
+
+static const struct panel_desc raspberrypi_7inch = {
+	.modes = &raspberrypi_7inch_mode,
+	.num_modes = 1,
+	.bpc = 8,
+	.size = {
+		.width = 154,
+		.height = 86,
+	},
+	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+};
+
 static const struct drm_display_mode samsung_lsn122dl01_c01_mode = {
 	.clock = 271560,
 	.hdisplay = 2560,
@@ -1996,6 +2023,9 @@ static const struct of_device_id platform_of_match[] = {
 		.compatible = "qiaodian,qd43003c0-40",
 		.data = &qd43003c0_40,
 	}, {
+		.compatible = "raspberrypi,7inch-touchscreen-panel",
+		.data = &raspberrypi_7inch,
+	}, {
 		.compatible = "samsung,lsn122dl01-c01",
 		.data = &samsung_lsn122dl01_c01,
 	}, {
-- 
2.11.0