[PATCH v6 00/18] Add T114 video support

[PATCH v6 00/18] Add T114 video support

[Svyatoslav Ryhel]

T114 is not that different from T30 and all T30 drivers will work
on T114 as well with some adjustments.

Patches propose general improvements for existing Tegra DC and DSI
drivers as well Tegra 114 video support (experimentl).

Commits pass buildman for tegra.

---
Changes from v5:
 - backlight enable moved to the last step of setup for DSI
 - parameterized V- and H-sync polarities
 - added framebuffer clearing on probe to avoid glitches
 - backlight enable moved after DC is fully configured
 - fixed printing framebuffer pointer instead of address
 - moved scdiv calculation to tegra DSI if it is used

Changes from v4:
 - fixed typo in max rate to be divided (400KHz > 400MHz)

Changes from v3:
 - fixed the clock divider calculation if PLLD/D2 is used
 - removed unnecessary pre-configuration
 - set correct video FIFO depth for DSI

Changes from v2:
 - fixed image distortion on devices with 180deg rotation

Changes from v1:
 - reworked patchset entirely
 - diverged DC configuration per-SOC
 - consolidated dc headers from different SOC gen
 - initial support of DC detection (tegra has 2 DC)
 - added PLLD2 support, resets and powergating
 - added T114+ MIPI calibration
 - added DSI detection (tegra has 2 DSI) and resets
---

Jonas Schwöbel (6):
  video: tegra20: dc: fix printing of framebuffer address
  video: tegra20: dc: enable backlight after DC is configured
  video: tegra20: dc: clean framebuffer memory block
  video: tegra20: dsi: remove pre-configuration
  video: tegra20: dsi: set correct fifo depth
  video: tegra20: dsi: use set_backlight for backlight only

Svyatoslav Ryhel (12):
  video: tegra20: dc: diverge DC per-SOC
  video: tegra20: dc: fix image shift on rotated panels
  video: tegra20: consolidate DC header
  video: tegra20: dc: pass DC id to internal devices
  video: tegra20: dc: add PLLD2 parent support
  video: tegra20: dc: add reset support
  video: tegra20: dc: add powergate
  video: tegra20: dc: configure behavior if PLLD/D2 is used
  video: tegra20: dc: parameterize V- and H-sync polarities
  video: tegra20: add MIPI calibration driver
  video: tegra20: dsi: add T114 support
  video: tegra20: dsi: add reset support

 arch/arm/dts/tegra114-u-boot.dtsi             |  13 +
 arch/arm/dts/tegra114.dtsi                    |   4 +-
 arch/arm/dts/tegra30-u-boot.dtsi              |   4 +
 arch/arm/dts/tegra30.dtsi                     |   2 +-
 arch/arm/include/asm/arch-tegra/dc.h          |  13 +-
 arch/arm/include/asm/arch-tegra114/pwm.h      |  13 +
 arch/arm/include/asm/arch-tegra20/display.h   |  28 --
 arch/arm/include/asm/arch-tegra30/display.h   |  28 --
 drivers/video/tegra20/Makefile                |   2 +-
 drivers/video/tegra20/tegra-dc.c              | 239 +++++++++++++-----
 drivers/video/tegra20/tegra-dc.h              |  45 ++++
 drivers/video/tegra20/tegra-dsi.c             | 122 ++++++++-
 .../video/tegra20/tegra-dsi.h                 |  24 +-
 drivers/video/tegra20/tegra-mipi.c            | 188 ++++++++++++++
 drivers/video/tegra20/tegra-pwm-backlight.c   |   3 +-
 15 files changed, 583 insertions(+), 145 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-tegra114/pwm.h
 delete mode 100644 arch/arm/include/asm/arch-tegra20/display.h
 delete mode 100644 arch/arm/include/asm/arch-tegra30/display.h
 create mode 100644 drivers/video/tegra20/tegra-dc.h
 rename arch/arm/include/asm/arch-tegra30/dsi.h => drivers/video/tegra20/tegra-dsi.h (90%)
 create mode 100644 drivers/video/tegra20/tegra-mipi.c

-- 
2.40.1

[PATCH v6 01/18] video: tegra20: dc: diverge DC per-SOC

[Svyatoslav Ryhel]

Diverge DC driver setup to better fit each of supported generations
of Tegra SOC.

Tested-by: Agneli <poczt@protonmail.ch> # Toshiba AC100 T20
Tested-by: Robert Eckelmann <longnoserob@gmail.com> # ASUS TF101
Tested-by: Andreas Westman Dorcsak <hedmoo@yahoo.com> # ASUS Grouper E1565
Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 arch/arm/dts/tegra114-u-boot.dtsi            |  13 +++
 arch/arm/dts/tegra114.dtsi                   |   4 +-
 arch/arm/dts/tegra30-u-boot.dtsi             |   4 +
 arch/arm/dts/tegra30.dtsi                    |   2 +-
 arch/arm/include/asm/arch-tegra114/display.h |  28 +++++
 arch/arm/include/asm/arch-tegra114/pwm.h     |  13 +++
 drivers/video/tegra20/tegra-dc.c             | 107 +++++++++++++------
 7 files changed, 133 insertions(+), 38 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-tegra114/display.h
 create mode 100644 arch/arm/include/asm/arch-tegra114/pwm.h

diff --git a/arch/arm/dts/tegra114-u-boot.dtsi b/arch/arm/dts/tegra114-u-boot.dtsi
index 7c11972552..6a02714a25 100644
--- a/arch/arm/dts/tegra114-u-boot.dtsi
+++ b/arch/arm/dts/tegra114-u-boot.dtsi
@@ -1,3 +1,16 @@
 #include <config.h>
 
 #include "tegra-u-boot.dtsi"
+
+/ {
+	host1x@50000000 {
+		bootph-all;
+		dc@54200000 {
+			bootph-all;
+		};
+
+		dc@54240000 {
+			bootph-all;
+		};
+	};
+};
diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi
index 68ee7f3165..250d692f6b 100644
--- a/arch/arm/dts/tegra114.dtsi
+++ b/arch/arm/dts/tegra114.dtsi
@@ -42,7 +42,7 @@
 		};
 
 		dc@54200000 {
-			compatible = "nvidia,tegra114-dc", "nvidia,tegra20-dc";
+			compatible = "nvidia,tegra114-dc";
 			reg = <0x54200000 0x00040000>;
 			interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA114_CLK_DISP1>,
@@ -61,7 +61,7 @@
 		};
 
 		dc@54240000 {
-			compatible = "nvidia,tegra114-dc", "nvidia,tegra20-dc";
+			compatible = "nvidia,tegra114-dc";
 			reg = <0x54240000 0x00040000>;
 			interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA114_CLK_DISP2>,
diff --git a/arch/arm/dts/tegra30-u-boot.dtsi b/arch/arm/dts/tegra30-u-boot.dtsi
index 3038227dbe..6a02714a25 100644
--- a/arch/arm/dts/tegra30-u-boot.dtsi
+++ b/arch/arm/dts/tegra30-u-boot.dtsi
@@ -8,5 +8,9 @@
 		dc@54200000 {
 			bootph-all;
 		};
+
+		dc@54240000 {
+			bootph-all;
+		};
 	};
 };
diff --git a/arch/arm/dts/tegra30.dtsi b/arch/arm/dts/tegra30.dtsi
index f198bc0edb..1177e2ab1f 100644
--- a/arch/arm/dts/tegra30.dtsi
+++ b/arch/arm/dts/tegra30.dtsi
@@ -158,7 +158,7 @@
 		};
 
 		dc@54200000 {
-			compatible = "nvidia,tegra30-dc", "nvidia,tegra20-dc";
+			compatible = "nvidia,tegra30-dc";
 			reg = <0x54200000 0x00040000>;
 			interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA30_CLK_DISP1>,
diff --git a/arch/arm/include/asm/arch-tegra114/display.h b/arch/arm/include/asm/arch-tegra114/display.h
new file mode 100644
index 0000000000..9411525799
--- /dev/null
+++ b/arch/arm/include/asm/arch-tegra114/display.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ *  (C) Copyright 2010
+ *  NVIDIA Corporation <www.nvidia.com>
+ */
+
+#ifndef __ASM_ARCH_TEGRA_DISPLAY_H
+#define __ASM_ARCH_TEGRA_DISPLAY_H
+
+#include <asm/arch-tegra/dc.h>
+
+/* This holds information about a window which can be displayed */
+struct disp_ctl_win {
+	enum win_color_depth_id fmt;	/* Color depth/format */
+	unsigned int	bpp;		/* Bits per pixel */
+	phys_addr_t	phys_addr;	/* Physical address in memory */
+	unsigned int	x;		/* Horizontal address offset (bytes) */
+	unsigned int	y;		/* Veritical address offset (bytes) */
+	unsigned int	w;		/* Width of source window */
+	unsigned int	h;		/* Height of source window */
+	unsigned int	stride;		/* Number of bytes per line */
+	unsigned int	out_x;		/* Left edge of output window (col) */
+	unsigned int	out_y;		/* Top edge of output window (row) */
+	unsigned int	out_w;		/* Width of output window in pixels */
+	unsigned int	out_h;		/* Height of output window in pixels */
+};
+
+#endif /*__ASM_ARCH_TEGRA_DISPLAY_H*/
diff --git a/arch/arm/include/asm/arch-tegra114/pwm.h b/arch/arm/include/asm/arch-tegra114/pwm.h
new file mode 100644
index 0000000000..af39151803
--- /dev/null
+++ b/arch/arm/include/asm/arch-tegra114/pwm.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Tegra pulse width frequency modulator definitions
+ *
+ * Copyright (c) 2011 The Chromium OS Authors.
+ */
+
+#ifndef __ASM_ARCH_TEGRA114_PWM_H
+#define __ASM_ARCH_TEGRA114_PWM_H
+
+#include <asm/arch-tegra/pwm.h>
+
+#endif	/* __ASM_ARCH_TEGRA114_PWM_H */
diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
index f53ad46397..7605e77bc1 100644
--- a/drivers/video/tegra20/tegra-dc.c
+++ b/drivers/video/tegra20/tegra-dc.c
@@ -3,7 +3,6 @@
  * Copyright (c) 2011 The Chromium OS Authors.
  */
 
-#include <common.h>
 #include <backlight.h>
 #include <dm.h>
 #include <fdtdec.h>
@@ -23,10 +22,15 @@
 #include <asm/arch/pinmux.h>
 #include <asm/arch/pwm.h>
 #include <asm/arch/display.h>
-#include <asm/arch-tegra/timer.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
+/* Holder of Tegra per-SOC DC differences */
+struct tegra_dc_soc_info {
+	bool has_timer;
+	bool has_rgb;
+};
+
 /* Information about the display controller */
 struct tegra_lcd_priv {
 	int width;			/* width in pixels */
@@ -35,6 +39,7 @@ struct tegra_lcd_priv {
 	struct display_timing timing;
 	struct udevice *panel;
 	struct dc_ctlr *dc;		/* Display controller regmap */
+	const struct tegra_dc_soc_info *soc;
 	fdt_addr_t frame_buffer;	/* Address of frame buffer */
 	unsigned pixel_clock;		/* Pixel clock in Hz */
 	int dc_clk[2];			/* Contains clk and its parent */
@@ -43,8 +48,8 @@ struct tegra_lcd_priv {
 
 enum {
 	/* Maximum LCD size we support */
-	LCD_MAX_WIDTH		= 1920,
-	LCD_MAX_HEIGHT		= 1200,
+	LCD_MAX_WIDTH		= 2560,
+	LCD_MAX_HEIGHT		= 1600,
 	LCD_MAX_LOG2_BPP	= VIDEO_BPP16,
 };
 
@@ -110,9 +115,9 @@ static void update_window(struct tegra_lcd_priv *priv,
 	writel(val, &dc->cmd.state_ctrl);
 }
 
-static int update_display_mode(struct dc_disp_reg *disp,
-			       struct tegra_lcd_priv *priv)
+static int update_display_mode(struct tegra_lcd_priv *priv)
 {
+	struct dc_disp_reg *disp = &priv->dc->disp;
 	struct display_timing *dt = &priv->timing;
 	unsigned long val;
 	unsigned long rate;
@@ -128,14 +133,16 @@ static int update_display_mode(struct dc_disp_reg *disp,
 	       &disp->front_porch);
 	writel(dt->hactive.typ | (dt->vactive.typ << 16), &disp->disp_active);
 
-	val = DE_SELECT_ACTIVE << DE_SELECT_SHIFT;
-	val |= DE_CONTROL_NORMAL << DE_CONTROL_SHIFT;
-	writel(val, &disp->data_enable_opt);
+	if (priv->soc->has_rgb) {
+		val = DE_SELECT_ACTIVE << DE_SELECT_SHIFT;
+		val |= DE_CONTROL_NORMAL << DE_CONTROL_SHIFT;
+		writel(val, &disp->data_enable_opt);
 
-	val = DATA_FORMAT_DF1P1C << DATA_FORMAT_SHIFT;
-	val |= DATA_ALIGNMENT_MSB << DATA_ALIGNMENT_SHIFT;
-	val |= DATA_ORDER_RED_BLUE << DATA_ORDER_SHIFT;
-	writel(val, &disp->disp_interface_ctrl);
+		val = DATA_FORMAT_DF1P1C << DATA_FORMAT_SHIFT;
+		val |= DATA_ALIGNMENT_MSB << DATA_ALIGNMENT_SHIFT;
+		val |= DATA_ORDER_RED_BLUE << DATA_ORDER_SHIFT;
+		writel(val, &disp->disp_interface_ctrl);
+	}
 
 	/*
 	 * The pixel clock divider is in 7.1 format (where the bottom bit
@@ -147,7 +154,8 @@ static int update_display_mode(struct dc_disp_reg *disp,
 	div = ((rate * 2 + priv->pixel_clock / 2) / priv->pixel_clock) - 2;
 	debug("Display clock %lu, divider %lu\n", rate, div);
 
-	writel(0x00010001, &disp->shift_clk_opt);
+	if (priv->soc->has_rgb)
+		writel(0x00010001, &disp->shift_clk_opt);
 
 	val = PIXEL_CLK_DIVIDER_PCD1 << PIXEL_CLK_DIVIDER_SHIFT;
 	val |= div << SHIFT_CLK_DIVIDER_SHIFT;
@@ -174,6 +182,7 @@ static void basic_init(struct dc_cmd_reg *cmd)
 	writel(val, &cmd->disp_pow_ctrl);
 
 	val = readl(&cmd->disp_cmd);
+	val &= ~CTRL_MODE_MASK;
 	val |= CTRL_MODE_C_DISPLAY << CTRL_MODE_SHIFT;
 	writel(val, &cmd->disp_cmd);
 }
@@ -229,8 +238,8 @@ static void rgb_enable(struct dc_com_reg *com)
 		writel(rgb_sel_tab[i], &com->pin_output_sel[i]);
 }
 
-static int setup_window(struct disp_ctl_win *win,
-			struct tegra_lcd_priv *priv)
+static int setup_window(struct tegra_lcd_priv *priv,
+			struct disp_ctl_win *win)
 {
 	if (priv->rotation) {
 		win->x = priv->width * 2;
@@ -274,12 +283,11 @@ static int setup_window(struct disp_ctl_win *win,
  * You should pass in the U-Boot address here, and check the contents of
  * struct tegra_lcd_priv to see what was actually chosen.
  *
- * @param blob			Device tree blob
  * @param priv			Driver's private data
  * @param default_lcd_base	Default address of LCD frame buffer
  * Return: 0 if ok, -1 on error (unsupported bits per pixel)
  */
-static int tegra_display_probe(const void *blob, struct tegra_lcd_priv *priv,
+static int tegra_display_probe(struct tegra_lcd_priv *priv,
 			       void *default_lcd_base)
 {
 	struct disp_ctl_win window;
@@ -288,7 +296,7 @@ static int tegra_display_probe(const void *blob, struct tegra_lcd_priv *priv,
 	priv->frame_buffer = (u32)default_lcd_base;
 
 	/*
-	 * We halve the rate if DISP1 paret is PLLD, since actual parent
+	 * We halve the rate if DISP1 parent is PLLD, since actual parent
 	 * is plld_out0 which is PLLD divided by 2.
 	 */
 	if (priv->dc_clk[1] == CLOCK_ID_DISPLAY)
@@ -303,13 +311,17 @@ static int tegra_display_probe(const void *blob, struct tegra_lcd_priv *priv,
 			       rate);
 
 	basic_init(&priv->dc->cmd);
-	basic_init_timer(&priv->dc->disp);
-	rgb_enable(&priv->dc->com);
+
+	if (priv->soc->has_timer)
+		basic_init_timer(&priv->dc->disp);
+
+	if (priv->soc->has_rgb)
+		rgb_enable(&priv->dc->com);
 
 	if (priv->pixel_clock)
-		update_display_mode(&priv->dc->disp, priv);
+		update_display_mode(priv);
 
-	if (setup_window(&window, priv))
+	if (setup_window(priv, &window))
 		return -1;
 
 	update_window(priv, &window);
@@ -322,7 +334,6 @@ static int tegra_lcd_probe(struct udevice *dev)
 	struct video_uc_plat *plat = dev_get_uclass_plat(dev);
 	struct video_priv *uc_priv = dev_get_uclass_priv(dev);
 	struct tegra_lcd_priv *priv = dev_get_priv(dev);
-	const void *blob = gd->fdt_blob;
 	int ret;
 
 	/* Initialize the Tegra display controller */
@@ -330,8 +341,8 @@ static int tegra_lcd_probe(struct udevice *dev)
 	funcmux_select(PERIPH_ID_DISP1, FUNCMUX_DEFAULT);
 #endif
 
-	if (tegra_display_probe(blob, priv, (void *)plat->base)) {
-		printf("%s: Failed to probe display driver\n", __func__);
+	if (tegra_display_probe(priv, (void *)plat->base)) {
+		debug("%s: Failed to probe display driver\n", __func__);
 		return -1;
 	}
 
@@ -383,6 +394,8 @@ static int tegra_lcd_of_to_plat(struct udevice *dev)
 		return -EINVAL;
 	}
 
+	priv->soc = (struct tegra_dc_soc_info *)dev_get_driver_data(dev);
+
 	ret = clock_decode_pair(dev, priv->dc_clk);
 	if (ret < 0) {
 		debug("%s: Cannot decode clocks for '%s' (ret = %d)\n",
@@ -464,19 +477,43 @@ static int tegra_lcd_bind(struct udevice *dev)
 static const struct video_ops tegra_lcd_ops = {
 };
 
+static const struct tegra_dc_soc_info tegra20_dc_soc_info = {
+	.has_timer = true,
+	.has_rgb = true,
+};
+
+static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
+	.has_timer = false,
+	.has_rgb = true,
+};
+
+static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
+	.has_timer = false,
+	.has_rgb = false,
+};
+
 static const struct udevice_id tegra_lcd_ids[] = {
-	{ .compatible = "nvidia,tegra20-dc" },
-	{ .compatible = "nvidia,tegra30-dc" },
-	{ }
+	{
+		.compatible = "nvidia,tegra20-dc",
+		.data = (ulong)&tegra20_dc_soc_info
+	}, {
+		.compatible = "nvidia,tegra30-dc",
+		.data = (ulong)&tegra30_dc_soc_info
+	}, {
+		.compatible = "nvidia,tegra114-dc",
+		.data = (ulong)&tegra114_dc_soc_info
+	}, {
+		/* sentinel */
+	}
 };
 
 U_BOOT_DRIVER(tegra_lcd) = {
-	.name	= "tegra_lcd",
-	.id	= UCLASS_VIDEO,
-	.of_match = tegra_lcd_ids,
-	.ops	= &tegra_lcd_ops,
-	.bind	= tegra_lcd_bind,
-	.probe	= tegra_lcd_probe,
+	.name		= "tegra_lcd",
+	.id		= UCLASS_VIDEO,
+	.of_match	= tegra_lcd_ids,
+	.ops		= &tegra_lcd_ops,
+	.bind		= tegra_lcd_bind,
+	.probe		= tegra_lcd_probe,
 	.of_to_plat	= tegra_lcd_of_to_plat,
 	.priv_auto	= sizeof(struct tegra_lcd_priv),
 };
-- 
2.40.1

Re: [PATCH v6 01/18] video: tegra20: dc: diverge DC per-SOC

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
[...]
> diff --git a/arch/arm/include/asm/arch-tegra114/display.h b/arch/arm/include/asm/arch-tegra114/display.h
> new file mode 100644
> index 0000000000..9411525799
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-tegra114/display.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + *  (C) Copyright 2010
> + *  NVIDIA Corporation <www.nvidia.com>
> + */
> +
> +#ifndef __ASM_ARCH_TEGRA_DISPLAY_H
> +#define __ASM_ARCH_TEGRA_DISPLAY_H
> +
> +#include <asm/arch-tegra/dc.h>
> +
> +/* This holds information about a window which can be displayed */
> +struct disp_ctl_win {
> +	enum win_color_depth_id fmt;	/* Color depth/format */
> +	unsigned int	bpp;		/* Bits per pixel */
> +	phys_addr_t	phys_addr;	/* Physical address in memory */
> +	unsigned int	x;		/* Horizontal address offset (bytes) */
> +	unsigned int	y;		/* Veritical address offset (bytes) */
> +	unsigned int	w;		/* Width of source window */
> +	unsigned int	h;		/* Height of source window */
> +	unsigned int	stride;		/* Number of bytes per line */
> +	unsigned int	out_x;		/* Left edge of output window (col) */
> +	unsigned int	out_y;		/* Top edge of output window (row) */
> +	unsigned int	out_w;		/* Width of output window in pixels */
> +	unsigned int	out_h;		/* Height of output window in pixels */
> +};
> +
> +#endif /*__ASM_ARCH_TEGRA_DISPLAY_H*/

One of the earlier patches in the series gets rid of this per-SoC header
file in favor of a common one. Did this end up here by mistake? It
doesn't seem to be used.

> diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
> index f53ad46397..7605e77bc1 100644
> --- a/drivers/video/tegra20/tegra-dc.c
> +++ b/drivers/video/tegra20/tegra-dc.c
> @@ -3,7 +3,6 @@
>   * Copyright (c) 2011 The Chromium OS Authors.
>   */
>  
> -#include <common.h>
>  #include <backlight.h>
>  #include <dm.h>
>  #include <fdtdec.h>
> @@ -23,10 +22,15 @@
>  #include <asm/arch/pinmux.h>
>  #include <asm/arch/pwm.h>
>  #include <asm/arch/display.h>
> -#include <asm/arch-tegra/timer.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> +/* Holder of Tegra per-SOC DC differences */
> +struct tegra_dc_soc_info {
> +	bool has_timer;
> +	bool has_rgb;
> +};
> +
>  /* Information about the display controller */
>  struct tegra_lcd_priv {
>  	int width;			/* width in pixels */
> @@ -35,6 +39,7 @@ struct tegra_lcd_priv {
>  	struct display_timing timing;
>  	struct udevice *panel;
>  	struct dc_ctlr *dc;		/* Display controller regmap */
> +	const struct tegra_dc_soc_info *soc;
>  	fdt_addr_t frame_buffer;	/* Address of frame buffer */
>  	unsigned pixel_clock;		/* Pixel clock in Hz */
>  	int dc_clk[2];			/* Contains clk and its parent */
> @@ -43,8 +48,8 @@ struct tegra_lcd_priv {
>  
>  enum {
>  	/* Maximum LCD size we support */
> -	LCD_MAX_WIDTH		= 1920,
> -	LCD_MAX_HEIGHT		= 1200,
> +	LCD_MAX_WIDTH		= 2560,
> +	LCD_MAX_HEIGHT		= 1600,
>  	LCD_MAX_LOG2_BPP	= VIDEO_BPP16,
>  };
>  
> @@ -110,9 +115,9 @@ static void update_window(struct tegra_lcd_priv *priv,
>  	writel(val, &dc->cmd.state_ctrl);
>  }
>  
> -static int update_display_mode(struct dc_disp_reg *disp,
> -			       struct tegra_lcd_priv *priv)
> +static int update_display_mode(struct tegra_lcd_priv *priv)
>  {
> +	struct dc_disp_reg *disp = &priv->dc->disp;
>  	struct display_timing *dt = &priv->timing;
>  	unsigned long val;
>  	unsigned long rate;
> @@ -128,14 +133,16 @@ static int update_display_mode(struct dc_disp_reg *disp,
>  	       &disp->front_porch);
>  	writel(dt->hactive.typ | (dt->vactive.typ << 16), &disp->disp_active);
>  
> -	val = DE_SELECT_ACTIVE << DE_SELECT_SHIFT;
> -	val |= DE_CONTROL_NORMAL << DE_CONTROL_SHIFT;
> -	writel(val, &disp->data_enable_opt);
> +	if (priv->soc->has_rgb) {
> +		val = DE_SELECT_ACTIVE << DE_SELECT_SHIFT;
> +		val |= DE_CONTROL_NORMAL << DE_CONTROL_SHIFT;
> +		writel(val, &disp->data_enable_opt);
>  
> -	val = DATA_FORMAT_DF1P1C << DATA_FORMAT_SHIFT;
> -	val |= DATA_ALIGNMENT_MSB << DATA_ALIGNMENT_SHIFT;
> -	val |= DATA_ORDER_RED_BLUE << DATA_ORDER_SHIFT;
> -	writel(val, &disp->disp_interface_ctrl);
> +		val = DATA_FORMAT_DF1P1C << DATA_FORMAT_SHIFT;
> +		val |= DATA_ALIGNMENT_MSB << DATA_ALIGNMENT_SHIFT;
> +		val |= DATA_ORDER_RED_BLUE << DATA_ORDER_SHIFT;
> +		writel(val, &disp->disp_interface_ctrl);
> +	}
>  
>  	/*
>  	 * The pixel clock divider is in 7.1 format (where the bottom bit
> @@ -147,7 +154,8 @@ static int update_display_mode(struct dc_disp_reg *disp,
>  	div = ((rate * 2 + priv->pixel_clock / 2) / priv->pixel_clock) - 2;
>  	debug("Display clock %lu, divider %lu\n", rate, div);
>  
> -	writel(0x00010001, &disp->shift_clk_opt);
> +	if (priv->soc->has_rgb)
> +		writel(0x00010001, &disp->shift_clk_opt);
>  
>  	val = PIXEL_CLK_DIVIDER_PCD1 << PIXEL_CLK_DIVIDER_SHIFT;
>  	val |= div << SHIFT_CLK_DIVIDER_SHIFT;
> @@ -174,6 +182,7 @@ static void basic_init(struct dc_cmd_reg *cmd)
>  	writel(val, &cmd->disp_pow_ctrl);
>  
>  	val = readl(&cmd->disp_cmd);
> +	val &= ~CTRL_MODE_MASK;
>  	val |= CTRL_MODE_C_DISPLAY << CTRL_MODE_SHIFT;

This seems unrelated to the rest here, but probably not worth splitting
it into a separate patch. I vaguely recall that this wasn't really
necessary because we do the reset prior to initialization and the
register is all zeroes by default?

>  	writel(val, &cmd->disp_cmd);
>  }
> @@ -229,8 +238,8 @@ static void rgb_enable(struct dc_com_reg *com)
>  		writel(rgb_sel_tab[i], &com->pin_output_sel[i]);
>  }
>  
> -static int setup_window(struct disp_ctl_win *win,
> -			struct tegra_lcd_priv *priv)
> +static int setup_window(struct tegra_lcd_priv *priv,
> +			struct disp_ctl_win *win)
>  {
>  	if (priv->rotation) {
>  		win->x = priv->width * 2;
> @@ -274,12 +283,11 @@ static int setup_window(struct disp_ctl_win *win,
>   * You should pass in the U-Boot address here, and check the contents of
>   * struct tegra_lcd_priv to see what was actually chosen.
>   *
> - * @param blob			Device tree blob
>   * @param priv			Driver's private data
>   * @param default_lcd_base	Default address of LCD frame buffer
>   * Return: 0 if ok, -1 on error (unsupported bits per pixel)
>   */
> -static int tegra_display_probe(const void *blob, struct tegra_lcd_priv *priv,
> +static int tegra_display_probe(struct tegra_lcd_priv *priv,
>  			       void *default_lcd_base)
>  {
>  	struct disp_ctl_win window;
> @@ -288,7 +296,7 @@ static int tegra_display_probe(const void *blob, struct tegra_lcd_priv *priv,
>  	priv->frame_buffer = (u32)default_lcd_base;
>  
>  	/*
> -	 * We halve the rate if DISP1 paret is PLLD, since actual parent
> +	 * We halve the rate if DISP1 parent is PLLD, since actual parent
>  	 * is plld_out0 which is PLLD divided by 2.
>  	 */
>  	if (priv->dc_clk[1] == CLOCK_ID_DISPLAY)
> @@ -303,13 +311,17 @@ static int tegra_display_probe(const void *blob, struct tegra_lcd_priv *priv,
>  			       rate);
>  
>  	basic_init(&priv->dc->cmd);
> -	basic_init_timer(&priv->dc->disp);
> -	rgb_enable(&priv->dc->com);
> +
> +	if (priv->soc->has_timer)
> +		basic_init_timer(&priv->dc->disp);
> +
> +	if (priv->soc->has_rgb)
> +		rgb_enable(&priv->dc->com);
>  
>  	if (priv->pixel_clock)
> -		update_display_mode(&priv->dc->disp, priv);
> +		update_display_mode(priv);
>  
> -	if (setup_window(&window, priv))
> +	if (setup_window(priv, &window))
>  		return -1;
>  
>  	update_window(priv, &window);
> @@ -322,7 +334,6 @@ static int tegra_lcd_probe(struct udevice *dev)
>  	struct video_uc_plat *plat = dev_get_uclass_plat(dev);
>  	struct video_priv *uc_priv = dev_get_uclass_priv(dev);
>  	struct tegra_lcd_priv *priv = dev_get_priv(dev);
> -	const void *blob = gd->fdt_blob;
>  	int ret;
>  
>  	/* Initialize the Tegra display controller */
> @@ -330,8 +341,8 @@ static int tegra_lcd_probe(struct udevice *dev)
>  	funcmux_select(PERIPH_ID_DISP1, FUNCMUX_DEFAULT);
>  #endif
>  
> -	if (tegra_display_probe(blob, priv, (void *)plat->base)) {
> -		printf("%s: Failed to probe display driver\n", __func__);
> +	if (tegra_display_probe(priv, (void *)plat->base)) {
> +		debug("%s: Failed to probe display driver\n", __func__);

Shouldn't this remain a printf() to make it more visible in the logs
what's going on? I guess people will notice anyway when the display
doesn't turn on, but this is good information to know when
troubleshooting.

>  		return -1;
>  	}
>  
> @@ -383,6 +394,8 @@ static int tegra_lcd_of_to_plat(struct udevice *dev)
>  		return -EINVAL;
>  	}
>  
> +	priv->soc = (struct tegra_dc_soc_info *)dev_get_driver_data(dev);
> +
>  	ret = clock_decode_pair(dev, priv->dc_clk);
>  	if (ret < 0) {
>  		debug("%s: Cannot decode clocks for '%s' (ret = %d)\n",
> @@ -464,19 +477,43 @@ static int tegra_lcd_bind(struct udevice *dev)
>  static const struct video_ops tegra_lcd_ops = {
>  };
>  
> +static const struct tegra_dc_soc_info tegra20_dc_soc_info = {
> +	.has_timer = true,
> +	.has_rgb = true,
> +};
> +
> +static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
> +	.has_timer = false,
> +	.has_rgb = true,
> +};
> +
> +static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
> +	.has_timer = false,
> +	.has_rgb = false,
> +};

My recollection is that technically Tegra114 still supports RGB, but it
ends up never being used on any of the platforms that I know of. On
Linux we base the decision to initialize the corresponding registers on
the status property of the "rgb" node of each display controller.
Perhaps that's something that U-Boot could do as well? That would avoid
programming these registers on Tegra20 and Tegra30 devices that don't
use RGB.

Thierry

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

Re: [PATCH v6 01/18] video: tegra20: dc: diverge DC per-SOC

[Thierry Reding]

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

On Fri Apr 19, 2024 at 6:26 PM CEST, Thierry Reding wrote:
> On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> [...]
> > diff --git a/arch/arm/include/asm/arch-tegra114/display.h b/arch/arm/include/asm/arch-tegra114/display.h
> > new file mode 100644
> > index 0000000000..9411525799
> > --- /dev/null
> > +++ b/arch/arm/include/asm/arch-tegra114/display.h
> > @@ -0,0 +1,28 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + *  (C) Copyright 2010
> > + *  NVIDIA Corporation <www.nvidia.com>
> > + */
> > +
> > +#ifndef __ASM_ARCH_TEGRA_DISPLAY_H
> > +#define __ASM_ARCH_TEGRA_DISPLAY_H
> > +
> > +#include <asm/arch-tegra/dc.h>
> > +
> > +/* This holds information about a window which can be displayed */
> > +struct disp_ctl_win {
> > +	enum win_color_depth_id fmt;	/* Color depth/format */
> > +	unsigned int	bpp;		/* Bits per pixel */
> > +	phys_addr_t	phys_addr;	/* Physical address in memory */
> > +	unsigned int	x;		/* Horizontal address offset (bytes) */
> > +	unsigned int	y;		/* Veritical address offset (bytes) */
> > +	unsigned int	w;		/* Width of source window */
> > +	unsigned int	h;		/* Height of source window */
> > +	unsigned int	stride;		/* Number of bytes per line */
> > +	unsigned int	out_x;		/* Left edge of output window (col) */
> > +	unsigned int	out_y;		/* Top edge of output window (row) */
> > +	unsigned int	out_w;		/* Width of output window in pixels */
> > +	unsigned int	out_h;		/* Height of output window in pixels */
> > +};
> > +
> > +#endif /*__ASM_ARCH_TEGRA_DISPLAY_H*/
>
> One of the earlier patches in the series gets rid of this per-SoC header
> file in favor of a common one. Did this end up here by mistake? It
> doesn't seem to be used.

Nevermind, my MUA sorted these patches weirdly, so it appeared as if
this was later in the series than it actually was.

Thierry

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

Re: [PATCH v6 01/18] video: tegra20: dc: diverge DC per-SOC

[Svyatoslav Ryhel]

пт, 19 квіт. 2024 р. о 19:26 Thierry Reding <thierry.reding@gmail.com> пише:
>
> On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> [...]
> > diff --git a/arch/arm/include/asm/arch-tegra114/display.h b/arch/arm/include/asm/arch-tegra114/display.h
> > new file mode 100644
> > index 0000000000..9411525799
> > --- /dev/null
> > +++ b/arch/arm/include/asm/arch-tegra114/display.h
> > @@ -0,0 +1,28 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + *  (C) Copyright 2010
> > + *  NVIDIA Corporation <www.nvidia.com>
> > + */
> > +
> > +#ifndef __ASM_ARCH_TEGRA_DISPLAY_H
> > +#define __ASM_ARCH_TEGRA_DISPLAY_H
> > +
> > +#include <asm/arch-tegra/dc.h>
> > +
> > +/* This holds information about a window which can be displayed */
> > +struct disp_ctl_win {
> > +     enum win_color_depth_id fmt;    /* Color depth/format */
> > +     unsigned int    bpp;            /* Bits per pixel */
> > +     phys_addr_t     phys_addr;      /* Physical address in memory */
> > +     unsigned int    x;              /* Horizontal address offset (bytes) */
> > +     unsigned int    y;              /* Veritical address offset (bytes) */
> > +     unsigned int    w;              /* Width of source window */
> > +     unsigned int    h;              /* Height of source window */
> > +     unsigned int    stride;         /* Number of bytes per line */
> > +     unsigned int    out_x;          /* Left edge of output window (col) */
> > +     unsigned int    out_y;          /* Top edge of output window (row) */
> > +     unsigned int    out_w;          /* Width of output window in pixels */
> > +     unsigned int    out_h;          /* Height of output window in pixels */
> > +};
> > +
> > +#endif /*__ASM_ARCH_TEGRA_DISPLAY_H*/
>
> One of the earlier patches in the series gets rid of this per-SoC header
> file in favor of a common one. Did this end up here by mistake? It
> doesn't seem to be used.
>
> > diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
> > index f53ad46397..7605e77bc1 100644
> > --- a/drivers/video/tegra20/tegra-dc.c
> > +++ b/drivers/video/tegra20/tegra-dc.c
> > @@ -3,7 +3,6 @@
> >   * Copyright (c) 2011 The Chromium OS Authors.
> >   */
> >
> > -#include <common.h>
> >  #include <backlight.h>
> >  #include <dm.h>
> >  #include <fdtdec.h>
> > @@ -23,10 +22,15 @@
> >  #include <asm/arch/pinmux.h>
> >  #include <asm/arch/pwm.h>
> >  #include <asm/arch/display.h>
> > -#include <asm/arch-tegra/timer.h>
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> > +/* Holder of Tegra per-SOC DC differences */
> > +struct tegra_dc_soc_info {
> > +     bool has_timer;
> > +     bool has_rgb;
> > +};
> > +
> >  /* Information about the display controller */
> >  struct tegra_lcd_priv {
> >       int width;                      /* width in pixels */
> > @@ -35,6 +39,7 @@ struct tegra_lcd_priv {
> >       struct display_timing timing;
> >       struct udevice *panel;
> >       struct dc_ctlr *dc;             /* Display controller regmap */
> > +     const struct tegra_dc_soc_info *soc;
> >       fdt_addr_t frame_buffer;        /* Address of frame buffer */
> >       unsigned pixel_clock;           /* Pixel clock in Hz */
> >       int dc_clk[2];                  /* Contains clk and its parent */
> > @@ -43,8 +48,8 @@ struct tegra_lcd_priv {
> >
> >  enum {
> >       /* Maximum LCD size we support */
> > -     LCD_MAX_WIDTH           = 1920,
> > -     LCD_MAX_HEIGHT          = 1200,
> > +     LCD_MAX_WIDTH           = 2560,
> > +     LCD_MAX_HEIGHT          = 1600,
> >       LCD_MAX_LOG2_BPP        = VIDEO_BPP16,
> >  };
> >
> > @@ -110,9 +115,9 @@ static void update_window(struct tegra_lcd_priv *priv,
> >       writel(val, &dc->cmd.state_ctrl);
> >  }
> >
> > -static int update_display_mode(struct dc_disp_reg *disp,
> > -                            struct tegra_lcd_priv *priv)
> > +static int update_display_mode(struct tegra_lcd_priv *priv)
> >  {
> > +     struct dc_disp_reg *disp = &priv->dc->disp;
> >       struct display_timing *dt = &priv->timing;
> >       unsigned long val;
> >       unsigned long rate;
> > @@ -128,14 +133,16 @@ static int update_display_mode(struct dc_disp_reg *disp,
> >              &disp->front_porch);
> >       writel(dt->hactive.typ | (dt->vactive.typ << 16), &disp->disp_active);
> >
> > -     val = DE_SELECT_ACTIVE << DE_SELECT_SHIFT;
> > -     val |= DE_CONTROL_NORMAL << DE_CONTROL_SHIFT;
> > -     writel(val, &disp->data_enable_opt);
> > +     if (priv->soc->has_rgb) {
> > +             val = DE_SELECT_ACTIVE << DE_SELECT_SHIFT;
> > +             val |= DE_CONTROL_NORMAL << DE_CONTROL_SHIFT;
> > +             writel(val, &disp->data_enable_opt);
> >
> > -     val = DATA_FORMAT_DF1P1C << DATA_FORMAT_SHIFT;
> > -     val |= DATA_ALIGNMENT_MSB << DATA_ALIGNMENT_SHIFT;
> > -     val |= DATA_ORDER_RED_BLUE << DATA_ORDER_SHIFT;
> > -     writel(val, &disp->disp_interface_ctrl);
> > +             val = DATA_FORMAT_DF1P1C << DATA_FORMAT_SHIFT;
> > +             val |= DATA_ALIGNMENT_MSB << DATA_ALIGNMENT_SHIFT;
> > +             val |= DATA_ORDER_RED_BLUE << DATA_ORDER_SHIFT;
> > +             writel(val, &disp->disp_interface_ctrl);
> > +     }
> >
> >       /*
> >        * The pixel clock divider is in 7.1 format (where the bottom bit
> > @@ -147,7 +154,8 @@ static int update_display_mode(struct dc_disp_reg *disp,
> >       div = ((rate * 2 + priv->pixel_clock / 2) / priv->pixel_clock) - 2;
> >       debug("Display clock %lu, divider %lu\n", rate, div);
> >
> > -     writel(0x00010001, &disp->shift_clk_opt);
> > +     if (priv->soc->has_rgb)
> > +             writel(0x00010001, &disp->shift_clk_opt);
> >
> >       val = PIXEL_CLK_DIVIDER_PCD1 << PIXEL_CLK_DIVIDER_SHIFT;
> >       val |= div << SHIFT_CLK_DIVIDER_SHIFT;
> > @@ -174,6 +182,7 @@ static void basic_init(struct dc_cmd_reg *cmd)
> >       writel(val, &cmd->disp_pow_ctrl);
> >
> >       val = readl(&cmd->disp_cmd);
> > +     val &= ~CTRL_MODE_MASK;
> >       val |= CTRL_MODE_C_DISPLAY << CTRL_MODE_SHIFT;
>
> This seems unrelated to the rest here, but probably not worth splitting
> it into a separate patch. I vaguely recall that this wasn't really
> necessary because we do the reset prior to initialization and the
> register is all zeroes by default?
>
> >       writel(val, &cmd->disp_cmd);
> >  }
> > @@ -229,8 +238,8 @@ static void rgb_enable(struct dc_com_reg *com)
> >               writel(rgb_sel_tab[i], &com->pin_output_sel[i]);
> >  }
> >
> > -static int setup_window(struct disp_ctl_win *win,
> > -                     struct tegra_lcd_priv *priv)
> > +static int setup_window(struct tegra_lcd_priv *priv,
> > +                     struct disp_ctl_win *win)
> >  {
> >       if (priv->rotation) {
> >               win->x = priv->width * 2;
> > @@ -274,12 +283,11 @@ static int setup_window(struct disp_ctl_win *win,
> >   * You should pass in the U-Boot address here, and check the contents of
> >   * struct tegra_lcd_priv to see what was actually chosen.
> >   *
> > - * @param blob                       Device tree blob
> >   * @param priv                       Driver's private data
> >   * @param default_lcd_base   Default address of LCD frame buffer
> >   * Return: 0 if ok, -1 on error (unsupported bits per pixel)
> >   */
> > -static int tegra_display_probe(const void *blob, struct tegra_lcd_priv *priv,
> > +static int tegra_display_probe(struct tegra_lcd_priv *priv,
> >                              void *default_lcd_base)
> >  {
> >       struct disp_ctl_win window;
> > @@ -288,7 +296,7 @@ static int tegra_display_probe(const void *blob, struct tegra_lcd_priv *priv,
> >       priv->frame_buffer = (u32)default_lcd_base;
> >
> >       /*
> > -      * We halve the rate if DISP1 paret is PLLD, since actual parent
> > +      * We halve the rate if DISP1 parent is PLLD, since actual parent
> >        * is plld_out0 which is PLLD divided by 2.
> >        */
> >       if (priv->dc_clk[1] == CLOCK_ID_DISPLAY)
> > @@ -303,13 +311,17 @@ static int tegra_display_probe(const void *blob, struct tegra_lcd_priv *priv,
> >                              rate);
> >
> >       basic_init(&priv->dc->cmd);
> > -     basic_init_timer(&priv->dc->disp);
> > -     rgb_enable(&priv->dc->com);
> > +
> > +     if (priv->soc->has_timer)
> > +             basic_init_timer(&priv->dc->disp);
> > +
> > +     if (priv->soc->has_rgb)
> > +             rgb_enable(&priv->dc->com);
> >
> >       if (priv->pixel_clock)
> > -             update_display_mode(&priv->dc->disp, priv);
> > +             update_display_mode(priv);
> >
> > -     if (setup_window(&window, priv))
> > +     if (setup_window(priv, &window))
> >               return -1;
> >
> >       update_window(priv, &window);
> > @@ -322,7 +334,6 @@ static int tegra_lcd_probe(struct udevice *dev)
> >       struct video_uc_plat *plat = dev_get_uclass_plat(dev);
> >       struct video_priv *uc_priv = dev_get_uclass_priv(dev);
> >       struct tegra_lcd_priv *priv = dev_get_priv(dev);
> > -     const void *blob = gd->fdt_blob;
> >       int ret;
> >
> >       /* Initialize the Tegra display controller */
> > @@ -330,8 +341,8 @@ static int tegra_lcd_probe(struct udevice *dev)
> >       funcmux_select(PERIPH_ID_DISP1, FUNCMUX_DEFAULT);
> >  #endif
> >
> > -     if (tegra_display_probe(blob, priv, (void *)plat->base)) {
> > -             printf("%s: Failed to probe display driver\n", __func__);
> > +     if (tegra_display_probe(priv, (void *)plat->base)) {
> > +             debug("%s: Failed to probe display driver\n", __func__);
>
> Shouldn't this remain a printf() to make it more visible in the logs
> what's going on? I guess people will notice anyway when the display
> doesn't turn on, but this is good information to know when
> troubleshooting.
>
debug() is preferred to reduce resulting image size.

> >               return -1;
> >       }
> >
> > @@ -383,6 +394,8 @@ static int tegra_lcd_of_to_plat(struct udevice *dev)
> >               return -EINVAL;
> >       }
> >
> > +     priv->soc = (struct tegra_dc_soc_info *)dev_get_driver_data(dev);
> > +
> >       ret = clock_decode_pair(dev, priv->dc_clk);
> >       if (ret < 0) {
> >               debug("%s: Cannot decode clocks for '%s' (ret = %d)\n",
> > @@ -464,19 +477,43 @@ static int tegra_lcd_bind(struct udevice *dev)
> >  static const struct video_ops tegra_lcd_ops = {
> >  };
> >
> > +static const struct tegra_dc_soc_info tegra20_dc_soc_info = {
> > +     .has_timer = true,
> > +     .has_rgb = true,
> > +};
> > +
> > +static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
> > +     .has_timer = false,
> > +     .has_rgb = true,
> > +};
> > +
> > +static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
> > +     .has_timer = false,
> > +     .has_rgb = false,
> > +};
>
> My recollection is that technically Tegra114 still supports RGB, but it
> ends up never being used on any of the platforms that I know of. On
> Linux we base the decision to initialize the corresponding registers on
> the status property of the "rgb" node of each display controller.
> Perhaps that's something that U-Boot could do as well? That would avoid
> programming these registers on Tegra20 and Tegra30 devices that don't
> use RGB.
>
This may be implemented in follow up. Please keep in mind that video subsystem
existing in U-Boot is much simpler then Linux framework and some stuff may not
be easy to implement right away. Thanks.

> Thierry

[PATCH v6 02/18] video: tegra20: dc: fix image shift on rotated panels

[Svyatoslav Ryhel]

Subtracting 1 from x and y fixes image shifting on rotated
panels.

Tested-by: Robert Eckelmann <longnoserob@gmail.com> # ASUS Grouper E1565
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/video/tegra20/tegra-dc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
index 7605e77bc1..bcc2748623 100644
--- a/drivers/video/tegra20/tegra-dc.c
+++ b/drivers/video/tegra20/tegra-dc.c
@@ -242,8 +242,8 @@ static int setup_window(struct tegra_lcd_priv *priv,
 			struct disp_ctl_win *win)
 {
 	if (priv->rotation) {
-		win->x = priv->width * 2;
-		win->y = priv->height;
+		win->x = priv->width * 2 - 1;
+		win->y = priv->height - 1;
 	} else {
 		win->x = 0;
 		win->y = 0;
-- 
2.40.1

Re: [PATCH v6 02/18] video: tegra20: dc: fix image shift on rotated panels

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> Subtracting 1 from x and y fixes image shifting on rotated
> panels.
>
> Tested-by: Robert Eckelmann <longnoserob@gmail.com> # ASUS Grouper E1565
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/video/tegra20/tegra-dc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Yeah, looks like we have that same fixup in Linux:

Reviewed-by: Thierry Reding <treding@nvidia.com>

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

[PATCH v6 03/18] video: tegra20: consolidate DC header

[Svyatoslav Ryhel]

Consolidate HD headers and place the result into video/tegra20
since it is used only by devices from this directory.

Tested-by: Agneli <poczt@protonmail.ch> # Toshiba AC100 T20
Tested-by: Robert Eckelmann <longnoserob@gmail.com> # ASUS TF101
Tested-by: Andreas Westman Dorcsak <hedmoo@yahoo.com> # ASUS Grouper E1565
Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 arch/arm/include/asm/arch-tegra/dc.h         |  8 ----
 arch/arm/include/asm/arch-tegra114/display.h | 28 -------------
 arch/arm/include/asm/arch-tegra20/display.h  | 28 -------------
 arch/arm/include/asm/arch-tegra30/display.h  | 28 -------------
 drivers/video/tegra20/tegra-dc.c             |  3 +-
 drivers/video/tegra20/tegra-dc.h             | 41 ++++++++++++++++++++
 drivers/video/tegra20/tegra-dsi.c            |  2 +-
 drivers/video/tegra20/tegra-pwm-backlight.c  |  3 +-
 8 files changed, 46 insertions(+), 95 deletions(-)
 delete mode 100644 arch/arm/include/asm/arch-tegra114/display.h
 delete mode 100644 arch/arm/include/asm/arch-tegra20/display.h
 delete mode 100644 arch/arm/include/asm/arch-tegra30/display.h
 create mode 100644 drivers/video/tegra20/tegra-dc.h

diff --git a/arch/arm/include/asm/arch-tegra/dc.h b/arch/arm/include/asm/arch-tegra/dc.h
index 7613d84f22..6444af2993 100644
--- a/arch/arm/include/asm/arch-tegra/dc.h
+++ b/arch/arm/include/asm/arch-tegra/dc.h
@@ -569,12 +569,4 @@ enum {
 #define DC_N_WINDOWS			5
 #define DC_REG_SAVE_SPACE		(DC_N_WINDOWS + 5)
 
-#define TEGRA_DSI_A		"dsi@54300000"
-#define TEGRA_DSI_B		"dsi@54400000"
-
-struct tegra_dc_plat {
-	struct udevice *dev;		/* Display controller device */
-	struct dc_ctlr *dc;		/* Display controller regmap */
-};
-
 #endif /* __ASM_ARCH_TEGRA_DC_H */
diff --git a/arch/arm/include/asm/arch-tegra114/display.h b/arch/arm/include/asm/arch-tegra114/display.h
deleted file mode 100644
index 9411525799..0000000000
--- a/arch/arm/include/asm/arch-tegra114/display.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-/*
- *  (C) Copyright 2010
- *  NVIDIA Corporation <www.nvidia.com>
- */
-
-#ifndef __ASM_ARCH_TEGRA_DISPLAY_H
-#define __ASM_ARCH_TEGRA_DISPLAY_H
-
-#include <asm/arch-tegra/dc.h>
-
-/* This holds information about a window which can be displayed */
-struct disp_ctl_win {
-	enum win_color_depth_id fmt;	/* Color depth/format */
-	unsigned int	bpp;		/* Bits per pixel */
-	phys_addr_t	phys_addr;	/* Physical address in memory */
-	unsigned int	x;		/* Horizontal address offset (bytes) */
-	unsigned int	y;		/* Veritical address offset (bytes) */
-	unsigned int	w;		/* Width of source window */
-	unsigned int	h;		/* Height of source window */
-	unsigned int	stride;		/* Number of bytes per line */
-	unsigned int	out_x;		/* Left edge of output window (col) */
-	unsigned int	out_y;		/* Top edge of output window (row) */
-	unsigned int	out_w;		/* Width of output window in pixels */
-	unsigned int	out_h;		/* Height of output window in pixels */
-};
-
-#endif /*__ASM_ARCH_TEGRA_DISPLAY_H*/
diff --git a/arch/arm/include/asm/arch-tegra20/display.h b/arch/arm/include/asm/arch-tegra20/display.h
deleted file mode 100644
index e7b3cffd46..0000000000
--- a/arch/arm/include/asm/arch-tegra20/display.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-/*
- *  (C) Copyright 2010
- *  NVIDIA Corporation <www.nvidia.com>
- */
-
-#ifndef __ASM_ARCH_TEGRA_DISPLAY_H
-#define __ASM_ARCH_TEGRA_DISPLAY_H
-
-#include <asm/arch-tegra/dc.h>
-
-/* This holds information about a window which can be displayed */
-struct disp_ctl_win {
-	enum win_color_depth_id fmt;	/* Color depth/format */
-	unsigned	bpp;		/* Bits per pixel */
-	phys_addr_t	phys_addr;	/* Physical address in memory */
-	unsigned	x;		/* Horizontal address offset (bytes) */
-	unsigned	y;		/* Veritical address offset (bytes) */
-	unsigned	w;		/* Width of source window */
-	unsigned	h;		/* Height of source window */
-	unsigned	stride;		/* Number of bytes per line */
-	unsigned	out_x;		/* Left edge of output window (col) */
-	unsigned	out_y;		/* Top edge of output window (row) */
-	unsigned	out_w;		/* Width of output window in pixels */
-	unsigned	out_h;		/* Height of output window in pixels */
-};
-
-#endif /*__ASM_ARCH_TEGRA_DISPLAY_H*/
diff --git a/arch/arm/include/asm/arch-tegra30/display.h b/arch/arm/include/asm/arch-tegra30/display.h
deleted file mode 100644
index 9411525799..0000000000
--- a/arch/arm/include/asm/arch-tegra30/display.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-/*
- *  (C) Copyright 2010
- *  NVIDIA Corporation <www.nvidia.com>
- */
-
-#ifndef __ASM_ARCH_TEGRA_DISPLAY_H
-#define __ASM_ARCH_TEGRA_DISPLAY_H
-
-#include <asm/arch-tegra/dc.h>
-
-/* This holds information about a window which can be displayed */
-struct disp_ctl_win {
-	enum win_color_depth_id fmt;	/* Color depth/format */
-	unsigned int	bpp;		/* Bits per pixel */
-	phys_addr_t	phys_addr;	/* Physical address in memory */
-	unsigned int	x;		/* Horizontal address offset (bytes) */
-	unsigned int	y;		/* Veritical address offset (bytes) */
-	unsigned int	w;		/* Width of source window */
-	unsigned int	h;		/* Height of source window */
-	unsigned int	stride;		/* Number of bytes per line */
-	unsigned int	out_x;		/* Left edge of output window (col) */
-	unsigned int	out_y;		/* Top edge of output window (row) */
-	unsigned int	out_w;		/* Width of output window in pixels */
-	unsigned int	out_h;		/* Height of output window in pixels */
-};
-
-#endif /*__ASM_ARCH_TEGRA_DISPLAY_H*/
diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
index bcc2748623..5d8874f323 100644
--- a/drivers/video/tegra20/tegra-dc.c
+++ b/drivers/video/tegra20/tegra-dc.c
@@ -21,7 +21,8 @@
 #include <asm/arch/funcmux.h>
 #include <asm/arch/pinmux.h>
 #include <asm/arch/pwm.h>
-#include <asm/arch/display.h>
+
+#include "tegra-dc.h"
 
 DECLARE_GLOBAL_DATA_PTR;
 
diff --git a/drivers/video/tegra20/tegra-dc.h b/drivers/video/tegra20/tegra-dc.h
new file mode 100644
index 0000000000..5c05221038
--- /dev/null
+++ b/drivers/video/tegra20/tegra-dc.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ *  (C) Copyright 2010
+ *  NVIDIA Corporation <www.nvidia.com>
+ */
+
+#ifndef _TEGRA_DC_H
+#define _TEGRA_DC_H
+
+#ifndef __ASSEMBLY__
+#include <linux/bitops.h>
+#endif
+
+/* arch-tegra/dc exists only because T124 uses it */
+#include <asm/arch-tegra/dc.h>
+
+#define TEGRA_DSI_A		"dsi@54300000"
+#define TEGRA_DSI_B		"dsi@54400000"
+
+struct tegra_dc_plat {
+	struct udevice *dev;		/* Display controller device */
+	struct dc_ctlr *dc;		/* Display controller regmap */
+};
+
+/* This holds information about a window which can be displayed */
+struct disp_ctl_win {
+	enum win_color_depth_id fmt;	/* Color depth/format */
+	unsigned int bpp;		/* Bits per pixel */
+	phys_addr_t phys_addr;		/* Physical address in memory */
+	unsigned int x;			/* Horizontal address offset (bytes) */
+	unsigned int y;			/* Veritical address offset (bytes) */
+	unsigned int w;			/* Width of source window */
+	unsigned int h;			/* Height of source window */
+	unsigned int stride;		/* Number of bytes per line */
+	unsigned int out_x;		/* Left edge of output window (col) */
+	unsigned int out_y;		/* Top edge of output window (row) */
+	unsigned int out_w;		/* Width of output window in pixels */
+	unsigned int out_h;		/* Height of output window in pixels */
+};
+
+#endif /* _TEGRA_DC_H */
diff --git a/drivers/video/tegra20/tegra-dsi.c b/drivers/video/tegra20/tegra-dsi.c
index a48f9c85d0..72b91ed26b 100644
--- a/drivers/video/tegra20/tegra-dsi.c
+++ b/drivers/video/tegra20/tegra-dsi.c
@@ -20,9 +20,9 @@
 #include <asm/gpio.h>
 #include <asm/io.h>
 #include <asm/arch/clock.h>
-#include <asm/arch/display.h>
 #include <asm/arch-tegra30/dsi.h>
 
+#include "tegra-dc.h"
 #include "mipi-phy.h"
 
 struct tegra_dsi_priv {
diff --git a/drivers/video/tegra20/tegra-pwm-backlight.c b/drivers/video/tegra20/tegra-pwm-backlight.c
index bb677daa8a..5f93f57fe9 100644
--- a/drivers/video/tegra20/tegra-pwm-backlight.c
+++ b/drivers/video/tegra20/tegra-pwm-backlight.c
@@ -15,7 +15,8 @@
 
 #include <asm/io.h>
 #include <asm/gpio.h>
-#include <asm/arch/display.h>
+
+#include "tegra-dc.h"
 
 #define TEGRA_DISPLAY_A_BASE		0x54200000
 #define TEGRA_DISPLAY_B_BASE		0x54240000
-- 
2.40.1

Re: [PATCH v6 03/18] video: tegra20: consolidate DC header

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> Consolidate HD headers and place the result into video/tegra20

Was this supposed to be "DC" headers like in the subject?

Other than that this makes sense, so:

Reviewed-by: Thierry Reding <treding@nvidia.com>

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

[PATCH v6 04/18] video: tegra20: dc: pass DC id to internal devices

[Svyatoslav Ryhel]

Tegra SoC has 2 independent display controllers called DC_A and
DC_B, they are handled differently by internal video devices like
DSI and HDMI controllers so it is important for last to know
which display controller is used to properly set up registers.
To achieve this, a pipe field was added to pdata to pass display
controller id to internal Tegra SoC devices.

Tested-by: Agneli <poczt@protonmail.ch> # Toshiba AC100 T20
Tested-by: Robert Eckelmann <longnoserob@gmail.com> # ASUS TF101
Tested-by: Andreas Westman Dorcsak <hedmoo@yahoo.com> # ASUS Grouper E1565
Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/video/tegra20/tegra-dc.c | 6 ++++++
 drivers/video/tegra20/tegra-dc.h | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
index 5d8874f323..0e94e665ef 100644
--- a/drivers/video/tegra20/tegra-dc.c
+++ b/drivers/video/tegra20/tegra-dc.c
@@ -45,6 +45,7 @@ struct tegra_lcd_priv {
 	unsigned pixel_clock;		/* Pixel clock in Hz */
 	int dc_clk[2];			/* Contains clk and its parent */
 	bool rotation;			/* 180 degree panel turn */
+	bool pipe;			/* DC controller: 0 for A, 1 for B */
 };
 
 enum {
@@ -406,6 +407,9 @@ static int tegra_lcd_of_to_plat(struct udevice *dev)
 
 	priv->rotation = dev_read_bool(dev, "nvidia,180-rotation");
 
+	if (!strcmp(dev->name, TEGRA_DC_B))
+		priv->pipe = 1;
+
 	rgb = fdt_subnode_offset(blob, node, "rgb");
 	if (rgb < 0) {
 		debug("%s: Cannot find rgb subnode for '%s' (ret=%d)\n",
@@ -431,12 +435,14 @@ static int tegra_lcd_of_to_plat(struct udevice *dev)
 		return ret;
 	}
 
+	/* Fill the platform data for internal devices */
 	if (!strcmp(priv->panel->name, TEGRA_DSI_A) ||
 	    !strcmp(priv->panel->name, TEGRA_DSI_B)) {
 		struct tegra_dc_plat *dc_plat = dev_get_plat(priv->panel);
 
 		dc_plat->dev = dev;
 		dc_plat->dc = priv->dc;
+		dc_plat->pipe = priv->pipe;
 	}
 
 	ret = panel_get_display_timing(priv->panel, &priv->timing);
diff --git a/drivers/video/tegra20/tegra-dc.h b/drivers/video/tegra20/tegra-dc.h
index 5c05221038..75fc0fa4de 100644
--- a/drivers/video/tegra20/tegra-dc.h
+++ b/drivers/video/tegra20/tegra-dc.h
@@ -14,12 +14,15 @@
 /* arch-tegra/dc exists only because T124 uses it */
 #include <asm/arch-tegra/dc.h>
 
+#define TEGRA_DC_A		"dc@54200000"
+#define TEGRA_DC_B		"dc@54240000"
 #define TEGRA_DSI_A		"dsi@54300000"
 #define TEGRA_DSI_B		"dsi@54400000"
 
 struct tegra_dc_plat {
 	struct udevice *dev;		/* Display controller device */
 	struct dc_ctlr *dc;		/* Display controller regmap */
+	bool pipe;			/* DC number: 0 for A, 1 for B */
 };
 
 /* This holds information about a window which can be displayed */
-- 
2.40.1

Re: [PATCH v6 04/18] video: tegra20: dc: pass DC id to internal devices

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> Tegra SoC has 2 independent display controllers called DC_A and
> DC_B, they are handled differently by internal video devices like
> DSI and HDMI controllers so it is important for last to know
> which display controller is used to properly set up registers.
> To achieve this, a pipe field was added to pdata to pass display
> controller id to internal Tegra SoC devices.
>
> Tested-by: Agneli <poczt@protonmail.ch> # Toshiba AC100 T20
> Tested-by: Robert Eckelmann <longnoserob@gmail.com> # ASUS TF101
> Tested-by: Andreas Westman Dorcsak <hedmoo@yahoo.com> # ASUS Grouper E1565
> Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
> Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/video/tegra20/tegra-dc.c | 6 ++++++
>  drivers/video/tegra20/tegra-dc.h | 3 +++
>  2 files changed, 9 insertions(+)
>
> diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
> index 5d8874f323..0e94e665ef 100644
> --- a/drivers/video/tegra20/tegra-dc.c
> +++ b/drivers/video/tegra20/tegra-dc.c
> @@ -45,6 +45,7 @@ struct tegra_lcd_priv {
>  	unsigned pixel_clock;		/* Pixel clock in Hz */
>  	int dc_clk[2];			/* Contains clk and its parent */
>  	bool rotation;			/* 180 degree panel turn */
> +	bool pipe;			/* DC controller: 0 for A, 1 for B */

Bool is a poor choice, even if there's only two of them. This is a
proper index, so it should be some sort of integer.

Also, the device tree bindings for the display controller specify a
"nvidia,head" property that can be used to identify these. If you add
that to the U-Boot DT you can avoid looking up by name to map this
value.

Thierry

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

Re: [PATCH v6 04/18] video: tegra20: dc: pass DC id to internal devices

[Svyatoslav Ryhel]

пт, 19 квіт. 2024 р. о 19:38 Thierry Reding <thierry.reding@gmail.com> пише:
>
> On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> > Tegra SoC has 2 independent display controllers called DC_A and
> > DC_B, they are handled differently by internal video devices like
> > DSI and HDMI controllers so it is important for last to know
> > which display controller is used to properly set up registers.
> > To achieve this, a pipe field was added to pdata to pass display
> > controller id to internal Tegra SoC devices.
> >
> > Tested-by: Agneli <poczt@protonmail.ch> # Toshiba AC100 T20
> > Tested-by: Robert Eckelmann <longnoserob@gmail.com> # ASUS TF101
> > Tested-by: Andreas Westman Dorcsak <hedmoo@yahoo.com> # ASUS Grouper E1565
> > Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
> > Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> >  drivers/video/tegra20/tegra-dc.c | 6 ++++++
> >  drivers/video/tegra20/tegra-dc.h | 3 +++
> >  2 files changed, 9 insertions(+)
> >
> > diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
> > index 5d8874f323..0e94e665ef 100644
> > --- a/drivers/video/tegra20/tegra-dc.c
> > +++ b/drivers/video/tegra20/tegra-dc.c
> > @@ -45,6 +45,7 @@ struct tegra_lcd_priv {
> >       unsigned pixel_clock;           /* Pixel clock in Hz */
> >       int dc_clk[2];                  /* Contains clk and its parent */
> >       bool rotation;                  /* 180 degree panel turn */
> > +     bool pipe;                      /* DC controller: 0 for A, 1 for B */
>
> Bool is a poor choice, even if there's only two of them. This is a
> proper index, so it should be some sort of integer.
>
> Also, the device tree bindings for the display controller specify a
> "nvidia,head" property that can be used to identify these. If you add
> that to the U-Boot DT you can avoid looking up by name to map this
> value.
>

Thanks for pointing to this property. May we apply this patch set as is
since it is well tested and confirmed to work and I will prepare a follow
up patches to adjust device tree relations? Would what be ok?

> Thierry

Re: [PATCH v6 04/18] video: tegra20: dc: pass DC id to internal devices

[Thierry Reding]

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

On Fri Apr 19, 2024 at 6:44 PM CEST, Svyatoslav Ryhel wrote:
> пт, 19 квіт. 2024 р. о 19:38 Thierry Reding <thierry.reding@gmail.com> пише:
> >
> > On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> > > Tegra SoC has 2 independent display controllers called DC_A and
> > > DC_B, they are handled differently by internal video devices like
> > > DSI and HDMI controllers so it is important for last to know
> > > which display controller is used to properly set up registers.
> > > To achieve this, a pipe field was added to pdata to pass display
> > > controller id to internal Tegra SoC devices.
> > >
> > > Tested-by: Agneli <poczt@protonmail.ch> # Toshiba AC100 T20
> > > Tested-by: Robert Eckelmann <longnoserob@gmail.com> # ASUS TF101
> > > Tested-by: Andreas Westman Dorcsak <hedmoo@yahoo.com> # ASUS Grouper E1565
> > > Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
> > > Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > >  drivers/video/tegra20/tegra-dc.c | 6 ++++++
> > >  drivers/video/tegra20/tegra-dc.h | 3 +++
> > >  2 files changed, 9 insertions(+)
> > >
> > > diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
> > > index 5d8874f323..0e94e665ef 100644
> > > --- a/drivers/video/tegra20/tegra-dc.c
> > > +++ b/drivers/video/tegra20/tegra-dc.c
> > > @@ -45,6 +45,7 @@ struct tegra_lcd_priv {
> > >       unsigned pixel_clock;           /* Pixel clock in Hz */
> > >       int dc_clk[2];                  /* Contains clk and its parent */
> > >       bool rotation;                  /* 180 degree panel turn */
> > > +     bool pipe;                      /* DC controller: 0 for A, 1 for B */
> >
> > Bool is a poor choice, even if there's only two of them. This is a
> > proper index, so it should be some sort of integer.
> >
> > Also, the device tree bindings for the display controller specify a
> > "nvidia,head" property that can be used to identify these. If you add
> > that to the U-Boot DT you can avoid looking up by name to map this
> > value.
> >
>
> Thanks for pointing to this property. May we apply this patch set as is
> since it is well tested and confirmed to work and I will prepare a follow
> up patches to adjust device tree relations? Would what be ok?

Well, there's a few other things that I think should be addressed, but
if you'd like to keep this one patch as-is and clean this up later, I
guess that's fine.

Thierry

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

Re: [PATCH v6 04/18] video: tegra20: dc: pass DC id to internal devices

[Svyatoslav Ryhel]

пт, 19 квіт. 2024 р. о 19:58 Thierry Reding <thierry.reding@gmail.com> пише:
>
> On Fri Apr 19, 2024 at 6:44 PM CEST, Svyatoslav Ryhel wrote:
> > пт, 19 квіт. 2024 р. о 19:38 Thierry Reding <thierry.reding@gmail.com> пише:
> > >
> > > On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> > > > Tegra SoC has 2 independent display controllers called DC_A and
> > > > DC_B, they are handled differently by internal video devices like
> > > > DSI and HDMI controllers so it is important for last to know
> > > > which display controller is used to properly set up registers.
> > > > To achieve this, a pipe field was added to pdata to pass display
> > > > controller id to internal Tegra SoC devices.
> > > >
> > > > Tested-by: Agneli <poczt@protonmail.ch> # Toshiba AC100 T20
> > > > Tested-by: Robert Eckelmann <longnoserob@gmail.com> # ASUS TF101
> > > > Tested-by: Andreas Westman Dorcsak <hedmoo@yahoo.com> # ASUS Grouper E1565
> > > > Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
> > > > Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > ---
> > > >  drivers/video/tegra20/tegra-dc.c | 6 ++++++
> > > >  drivers/video/tegra20/tegra-dc.h | 3 +++
> > > >  2 files changed, 9 insertions(+)
> > > >
> > > > diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
> > > > index 5d8874f323..0e94e665ef 100644
> > > > --- a/drivers/video/tegra20/tegra-dc.c
> > > > +++ b/drivers/video/tegra20/tegra-dc.c
> > > > @@ -45,6 +45,7 @@ struct tegra_lcd_priv {
> > > >       unsigned pixel_clock;           /* Pixel clock in Hz */
> > > >       int dc_clk[2];                  /* Contains clk and its parent */
> > > >       bool rotation;                  /* 180 degree panel turn */
> > > > +     bool pipe;                      /* DC controller: 0 for A, 1 for B */
> > >
> > > Bool is a poor choice, even if there's only two of them. This is a
> > > proper index, so it should be some sort of integer.
> > >
> > > Also, the device tree bindings for the display controller specify a
> > > "nvidia,head" property that can be used to identify these. If you add
> > > that to the U-Boot DT you can avoid looking up by name to map this
> > > value.
> > >
> >
> > Thanks for pointing to this property. May we apply this patch set as is
> > since it is well tested and confirmed to work and I will prepare a follow
> > up patches to adjust device tree relations? Would what be ok?
>
> Well, there's a few other things that I think should be addressed, but
> if you'd like to keep this one patch as-is and clean this up later, I
> guess that's fine.

May you send me a list of stuff which you think may be improved?
I would gladly adjust whatever I can in follow up.

> Thierry

[PATCH v6 05/18] video: tegra20: dc: add PLLD2 parent support

[Svyatoslav Ryhel]

T30+ SOC have second PLLD - PLLD2 which can be actively used by
DC and act as main DISP1/2 clock parent.

Tested-by: Agneli <poczt@protonmail.ch> # Toshiba AC100 T20
Tested-by: Robert Eckelmann <longnoserob@gmail.com> # ASUS TF101
Tested-by: Andreas Westman Dorcsak <hedmoo@yahoo.com> # ASUS Grouper E1565
Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/video/tegra20/tegra-dc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
index 0e94e665ef..56a23b3c97 100644
--- a/drivers/video/tegra20/tegra-dc.c
+++ b/drivers/video/tegra20/tegra-dc.c
@@ -304,6 +304,12 @@ static int tegra_display_probe(struct tegra_lcd_priv *priv,
 	if (priv->dc_clk[1] == CLOCK_ID_DISPLAY)
 		rate /= 2;
 
+#ifndef CONFIG_TEGRA20
+	/* PLLD2 obeys same rules as PLLD but it is present only on T30+ */
+	if (priv->dc_clk[1] == CLOCK_ID_DISPLAY2)
+		rate /= 2;
+#endif
+
 	/*
 	 * HOST1X is init by default at 150MHz with PLLC as parent
 	 */
-- 
2.40.1

Re: [PATCH v6 05/18] video: tegra20: dc: add PLLD2 parent support

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> T30+ SOC have second PLLD - PLLD2 which can be actively used by
> DC and act as main DISP1/2 clock parent.
>
> Tested-by: Agneli <poczt@protonmail.ch> # Toshiba AC100 T20
> Tested-by: Robert Eckelmann <longnoserob@gmail.com> # ASUS TF101
> Tested-by: Andreas Westman Dorcsak <hedmoo@yahoo.com> # ASUS Grouper E1565
> Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
> Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/video/tegra20/tegra-dc.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Reviewed-by: Thierry Reding <treding@nvidia.com>

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

[PATCH v6 06/18] video: tegra20: dc: add reset support

[Svyatoslav Ryhel]

Implement reset use to discard any changes which could have been
applied to DC before and can interfere with current configuration.

Tested-by: Agneli <poczt@protonmail.ch> # Toshiba AC100 T20
Tested-by: Robert Eckelmann <longnoserob@gmail.com> # ASUS TF101
Tested-by: Andreas Westman Dorcsak <hedmoo@yahoo.com> # ASUS Grouper E1565
Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/video/tegra20/tegra-dc.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
index 56a23b3c97..35abb6fe46 100644
--- a/drivers/video/tegra20/tegra-dc.c
+++ b/drivers/video/tegra20/tegra-dc.c
@@ -10,7 +10,9 @@
 #include <panel.h>
 #include <part.h>
 #include <pwm.h>
+#include <reset.h>
 #include <video.h>
+#include <linux/delay.h>
 #include <asm/cache.h>
 #include <asm/global_data.h>
 #include <asm/system.h>
@@ -342,6 +344,7 @@ static int tegra_lcd_probe(struct udevice *dev)
 	struct video_uc_plat *plat = dev_get_uclass_plat(dev);
 	struct video_priv *uc_priv = dev_get_uclass_priv(dev);
 	struct tegra_lcd_priv *priv = dev_get_priv(dev);
+	struct reset_ctl reset_ctl;
 	int ret;
 
 	/* Initialize the Tegra display controller */
@@ -349,6 +352,20 @@ static int tegra_lcd_probe(struct udevice *dev)
 	funcmux_select(PERIPH_ID_DISP1, FUNCMUX_DEFAULT);
 #endif
 
+	ret = reset_get_by_name(dev, "dc", &reset_ctl);
+	if (ret) {
+		log_err("reset_get_by_name() failed: %d\n", ret);
+		return ret;
+	}
+
+	clock_disable(priv->dc_clk[0]);
+
+	/* Reset everything set before */
+	reset_assert(&reset_ctl);
+	mdelay(4);
+	reset_deassert(&reset_ctl);
+	mdelay(4);
+
 	if (tegra_display_probe(priv, (void *)plat->base)) {
 		debug("%s: Failed to probe display driver\n", __func__);
 		return -1;
-- 
2.40.1

Re: [PATCH v6 06/18] video: tegra20: dc: add reset support

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> Implement reset use to discard any changes which could have been
> applied to DC before and can interfere with current configuration.
>
> Tested-by: Agneli <poczt@protonmail.ch> # Toshiba AC100 T20
> Tested-by: Robert Eckelmann <longnoserob@gmail.com> # ASUS TF101
> Tested-by: Andreas Westman Dorcsak <hedmoo@yahoo.com> # ASUS Grouper E1565
> Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
> Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/video/tegra20/tegra-dc.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
> index 56a23b3c97..35abb6fe46 100644
> --- a/drivers/video/tegra20/tegra-dc.c
> +++ b/drivers/video/tegra20/tegra-dc.c
> @@ -10,7 +10,9 @@
>  #include <panel.h>
>  #include <part.h>
>  #include <pwm.h>
> +#include <reset.h>
>  #include <video.h>
> +#include <linux/delay.h>
>  #include <asm/cache.h>
>  #include <asm/global_data.h>
>  #include <asm/system.h>
> @@ -342,6 +344,7 @@ static int tegra_lcd_probe(struct udevice *dev)
>  	struct video_uc_plat *plat = dev_get_uclass_plat(dev);
>  	struct video_priv *uc_priv = dev_get_uclass_priv(dev);
>  	struct tegra_lcd_priv *priv = dev_get_priv(dev);
> +	struct reset_ctl reset_ctl;
>  	int ret;
>  
>  	/* Initialize the Tegra display controller */
> @@ -349,6 +352,20 @@ static int tegra_lcd_probe(struct udevice *dev)
>  	funcmux_select(PERIPH_ID_DISP1, FUNCMUX_DEFAULT);
>  #endif
>  
> +	ret = reset_get_by_name(dev, "dc", &reset_ctl);
> +	if (ret) {
> +		log_err("reset_get_by_name() failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	clock_disable(priv->dc_clk[0]);
> +
> +	/* Reset everything set before */
> +	reset_assert(&reset_ctl);
> +	mdelay(4);
> +	reset_deassert(&reset_ctl);
> +	mdelay(4);

Are you sure this works as intended? It's been a long time since I
worked on this, but I seem to recall that most of these resets are
actually synchronous, so in order for them to do what they're supposed
to the clock needs to be kept running.

The Linux driver certainly does this differently.

Thierry

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

Re: [PATCH v6 06/18] video: tegra20: dc: add reset support

[Svyatoslav Ryhel]

пт, 19 квіт. 2024 р. о 19:05 Thierry Reding <thierry.reding@gmail.com> пише:
>
> On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> > Implement reset use to discard any changes which could have been
> > applied to DC before and can interfere with current configuration.
> >
> > Tested-by: Agneli <poczt@protonmail.ch> # Toshiba AC100 T20
> > Tested-by: Robert Eckelmann <longnoserob@gmail.com> # ASUS TF101
> > Tested-by: Andreas Westman Dorcsak <hedmoo@yahoo.com> # ASUS Grouper E1565
> > Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
> > Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> >  drivers/video/tegra20/tegra-dc.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
> > index 56a23b3c97..35abb6fe46 100644
> > --- a/drivers/video/tegra20/tegra-dc.c
> > +++ b/drivers/video/tegra20/tegra-dc.c
> > @@ -10,7 +10,9 @@
> >  #include <panel.h>
> >  #include <part.h>
> >  #include <pwm.h>
> > +#include <reset.h>
> >  #include <video.h>
> > +#include <linux/delay.h>
> >  #include <asm/cache.h>
> >  #include <asm/global_data.h>
> >  #include <asm/system.h>
> > @@ -342,6 +344,7 @@ static int tegra_lcd_probe(struct udevice *dev)
> >       struct video_uc_plat *plat = dev_get_uclass_plat(dev);
> >       struct video_priv *uc_priv = dev_get_uclass_priv(dev);
> >       struct tegra_lcd_priv *priv = dev_get_priv(dev);
> > +     struct reset_ctl reset_ctl;
> >       int ret;
> >
> >       /* Initialize the Tegra display controller */
> > @@ -349,6 +352,20 @@ static int tegra_lcd_probe(struct udevice *dev)
> >       funcmux_select(PERIPH_ID_DISP1, FUNCMUX_DEFAULT);
> >  #endif
> >
> > +     ret = reset_get_by_name(dev, "dc", &reset_ctl);
> > +     if (ret) {
> > +             log_err("reset_get_by_name() failed: %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     clock_disable(priv->dc_clk[0]);
> > +
> > +     /* Reset everything set before */
> > +     reset_assert(&reset_ctl);
> > +     mdelay(4);
> > +     reset_deassert(&reset_ctl);
> > +     mdelay(4);
>
> Are you sure this works as intended? It's been a long time since I
> worked on this, but I seem to recall that most of these resets are
> actually synchronous, so in order for them to do what they're supposed
> to the clock needs to be kept running.
>
> The Linux driver certainly does this differently.

You have point, but I have tried to mostly adapt Linux tegra dc driver,
which has same logic in probe. Maybe I have not understood it properly.
Testing on T20, T30 and T114 passed without issues so far.

> Thierry

Re: [PATCH v6 06/18] video: tegra20: dc: add reset support

[Thierry Reding]

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

On Fri Apr 19, 2024 at 6:37 PM CEST, Svyatoslav Ryhel wrote:
> пт, 19 квіт. 2024 р. о 19:05 Thierry Reding <thierry.reding@gmail.com> пише:
> >
> > On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> > > Implement reset use to discard any changes which could have been
> > > applied to DC before and can interfere with current configuration.
> > >
> > > Tested-by: Agneli <poczt@protonmail.ch> # Toshiba AC100 T20
> > > Tested-by: Robert Eckelmann <longnoserob@gmail.com> # ASUS TF101
> > > Tested-by: Andreas Westman Dorcsak <hedmoo@yahoo.com> # ASUS Grouper E1565
> > > Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
> > > Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > >  drivers/video/tegra20/tegra-dc.c | 17 +++++++++++++++++
> > >  1 file changed, 17 insertions(+)
> > >
> > > diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
> > > index 56a23b3c97..35abb6fe46 100644
> > > --- a/drivers/video/tegra20/tegra-dc.c
> > > +++ b/drivers/video/tegra20/tegra-dc.c
> > > @@ -10,7 +10,9 @@
> > >  #include <panel.h>
> > >  #include <part.h>
> > >  #include <pwm.h>
> > > +#include <reset.h>
> > >  #include <video.h>
> > > +#include <linux/delay.h>
> > >  #include <asm/cache.h>
> > >  #include <asm/global_data.h>
> > >  #include <asm/system.h>
> > > @@ -342,6 +344,7 @@ static int tegra_lcd_probe(struct udevice *dev)
> > >       struct video_uc_plat *plat = dev_get_uclass_plat(dev);
> > >       struct video_priv *uc_priv = dev_get_uclass_priv(dev);
> > >       struct tegra_lcd_priv *priv = dev_get_priv(dev);
> > > +     struct reset_ctl reset_ctl;
> > >       int ret;
> > >
> > >       /* Initialize the Tegra display controller */
> > > @@ -349,6 +352,20 @@ static int tegra_lcd_probe(struct udevice *dev)
> > >       funcmux_select(PERIPH_ID_DISP1, FUNCMUX_DEFAULT);
> > >  #endif
> > >
> > > +     ret = reset_get_by_name(dev, "dc", &reset_ctl);
> > > +     if (ret) {
> > > +             log_err("reset_get_by_name() failed: %d\n", ret);
> > > +             return ret;
> > > +     }
> > > +
> > > +     clock_disable(priv->dc_clk[0]);
> > > +
> > > +     /* Reset everything set before */
> > > +     reset_assert(&reset_ctl);
> > > +     mdelay(4);
> > > +     reset_deassert(&reset_ctl);
> > > +     mdelay(4);
> >
> > Are you sure this works as intended? It's been a long time since I
> > worked on this, but I seem to recall that most of these resets are
> > actually synchronous, so in order for them to do what they're supposed
> > to the clock needs to be kept running.
> >
> > The Linux driver certainly does this differently.
>
> You have point, but I have tried to mostly adapt Linux tegra dc driver,
> which has same logic in probe. Maybe I have not understood it properly.
> Testing on T20, T30 and T114 passed without issues so far.

Maybe look again. What it does is (basically):

	clock_enable();
	mdelay(4);
	reset_assert();
	mdelay(4);
	clock_disable();

That should ensure that it's completely reset at that point. Now before
any subsequent register accesses happen it will do this:

	clock_enable();
	reset_deassert();

to take it out of reset again. Perhaps that's something you want to keep
doing in probe() in U-Boot. In that case maybe you want something like
this instead:

	clock_enable();
	mdelay(4);
	reset_assert();
	mdelay(4);
	reset_deassert();

Thierry

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

Re: [PATCH v6 06/18] video: tegra20: dc: add reset support

[Svyatoslav Ryhel]

пт, 19 квіт. 2024 р. о 19:46 Thierry Reding <thierry.reding@gmail.com> пише:
>
> On Fri Apr 19, 2024 at 6:37 PM CEST, Svyatoslav Ryhel wrote:
> > пт, 19 квіт. 2024 р. о 19:05 Thierry Reding <thierry.reding@gmail.com> пише:
> > >
> > > On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> > > > Implement reset use to discard any changes which could have been
> > > > applied to DC before and can interfere with current configuration.
> > > >
> > > > Tested-by: Agneli <poczt@protonmail.ch> # Toshiba AC100 T20
> > > > Tested-by: Robert Eckelmann <longnoserob@gmail.com> # ASUS TF101
> > > > Tested-by: Andreas Westman Dorcsak <hedmoo@yahoo.com> # ASUS Grouper E1565
> > > > Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
> > > > Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > ---
> > > >  drivers/video/tegra20/tegra-dc.c | 17 +++++++++++++++++
> > > >  1 file changed, 17 insertions(+)
> > > >
> > > > diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
> > > > index 56a23b3c97..35abb6fe46 100644
> > > > --- a/drivers/video/tegra20/tegra-dc.c
> > > > +++ b/drivers/video/tegra20/tegra-dc.c
> > > > @@ -10,7 +10,9 @@
> > > >  #include <panel.h>
> > > >  #include <part.h>
> > > >  #include <pwm.h>
> > > > +#include <reset.h>
> > > >  #include <video.h>
> > > > +#include <linux/delay.h>
> > > >  #include <asm/cache.h>
> > > >  #include <asm/global_data.h>
> > > >  #include <asm/system.h>
> > > > @@ -342,6 +344,7 @@ static int tegra_lcd_probe(struct udevice *dev)
> > > >       struct video_uc_plat *plat = dev_get_uclass_plat(dev);
> > > >       struct video_priv *uc_priv = dev_get_uclass_priv(dev);
> > > >       struct tegra_lcd_priv *priv = dev_get_priv(dev);
> > > > +     struct reset_ctl reset_ctl;
> > > >       int ret;
> > > >
> > > >       /* Initialize the Tegra display controller */
> > > > @@ -349,6 +352,20 @@ static int tegra_lcd_probe(struct udevice *dev)
> > > >       funcmux_select(PERIPH_ID_DISP1, FUNCMUX_DEFAULT);
> > > >  #endif
> > > >
> > > > +     ret = reset_get_by_name(dev, "dc", &reset_ctl);
> > > > +     if (ret) {
> > > > +             log_err("reset_get_by_name() failed: %d\n", ret);
> > > > +             return ret;
> > > > +     }
> > > > +
> > > > +     clock_disable(priv->dc_clk[0]);
> > > > +
> > > > +     /* Reset everything set before */
> > > > +     reset_assert(&reset_ctl);
> > > > +     mdelay(4);
> > > > +     reset_deassert(&reset_ctl);
> > > > +     mdelay(4);
> > >
> > > Are you sure this works as intended? It's been a long time since I
> > > worked on this, but I seem to recall that most of these resets are
> > > actually synchronous, so in order for them to do what they're supposed
> > > to the clock needs to be kept running.
> > >
> > > The Linux driver certainly does this differently.
> >
> > You have point, but I have tried to mostly adapt Linux tegra dc driver,
> > which has same logic in probe. Maybe I have not understood it properly.
> > Testing on T20, T30 and T114 passed without issues so far.
>
> Maybe look again. What it does is (basically):
>
>         clock_enable();
>         mdelay(4);
>         reset_assert();
>         mdelay(4);
>         clock_disable();
>
> That should ensure that it's completely reset at that point. Now before
> any subsequent register accesses happen it will do this:
>
>         clock_enable();
>         reset_deassert();
>
> to take it out of reset again. Perhaps that's something you want to keep
> doing in probe() in U-Boot. In that case maybe you want something like
> this instead:
>
>         clock_enable();
>         mdelay(4);
>         reset_assert();
>         mdelay(4);
>         reset_deassert();
>

You are correct. I assume This patch may be dropped entirely. Thanks.

> Thierry

[PATCH v6 07/18] video: tegra20: dc: add powergate

[Svyatoslav Ryhel]

Add powergate use on T114 to complete resetting of DC.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/video/tegra20/tegra-dc.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
index 35abb6fe46..9a18e38cd8 100644
--- a/drivers/video/tegra20/tegra-dc.c
+++ b/drivers/video/tegra20/tegra-dc.c
@@ -22,6 +22,7 @@
 #include <asm/arch/clock.h>
 #include <asm/arch/funcmux.h>
 #include <asm/arch/pinmux.h>
+#include <asm/arch/powergate.h>
 #include <asm/arch/pwm.h>
 
 #include "tegra-dc.h"
@@ -32,6 +33,7 @@ DECLARE_GLOBAL_DATA_PTR;
 struct tegra_dc_soc_info {
 	bool has_timer;
 	bool has_rgb;
+	bool has_pgate;
 };
 
 /* Information about the display controller */
@@ -360,6 +362,28 @@ static int tegra_lcd_probe(struct udevice *dev)
 
 	clock_disable(priv->dc_clk[0]);
 
+	if (priv->soc->has_pgate) {
+		uint powergate;
+
+		if (priv->pipe)
+			powergate = TEGRA_POWERGATE_DISB;
+		else
+			powergate = TEGRA_POWERGATE_DIS;
+
+		ret = tegra_powergate_power_off(powergate);
+		if (ret < 0) {
+			log_err("failed to power off DISP gate: %d", ret);
+			return ret;
+		}
+
+		ret = tegra_powergate_sequence_power_up(powergate,
+							priv->dc_clk[0]);
+		if (ret < 0) {
+			log_err("failed to power up DISP gate: %d", ret);
+			return ret;
+		}
+	}
+
 	/* Reset everything set before */
 	reset_assert(&reset_ctl);
 	mdelay(4);
@@ -510,16 +534,19 @@ static const struct video_ops tegra_lcd_ops = {
 static const struct tegra_dc_soc_info tegra20_dc_soc_info = {
 	.has_timer = true,
 	.has_rgb = true,
+	.has_pgate = false,
 };
 
 static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
 	.has_timer = false,
 	.has_rgb = true,
+	.has_pgate = false,
 };
 
 static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
 	.has_timer = false,
 	.has_rgb = false,
+	.has_pgate = true,
 };
 
 static const struct udevice_id tegra_lcd_ids[] = {
-- 
2.40.1

Re: [PATCH v6 07/18] video: tegra20: dc: add powergate

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> Add powergate use on T114 to complete resetting of DC.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/video/tegra20/tegra-dc.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)

Seems correct:

Reviewed-by: Thierry Reding <treding@nvidia.com>

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

[PATCH v6 08/18] video: tegra20: dc: configure behavior if PLLD/D2 is used

[Svyatoslav Ryhel]

If DISP1 is a PLLD/D2 child, it cannot go over 370MHz. The cause
of this is not quite clear. This can be overcomed by further
halving the PLLD/D2 if the target parent rate is over 800MHz.
This way DISP1 and DSI clocks will have the same frequency. The
shift divider in this case has to be calculated from the
original PLLD/D2 frequency and is passed from the DSI driver.

Tested-by: Andreas Westman Dorcsak <hedmoo@yahoo.com> # ASUS Grouper E1565
Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
Tested-by: Jonas Schwöbel <jonasschwoebel@yahoo.de> # Microsoft Surface 2
Signed-off-by: Jonas Schwöbel <jonasschwoebel@yahoo.de>
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/video/tegra20/tegra-dc.c  | 34 +++++++++++++++++++------------
 drivers/video/tegra20/tegra-dc.h  |  1 +
 drivers/video/tegra20/tegra-dsi.c | 14 +++++++++++++
 3 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
index 9a18e38cd8..a8e32e6893 100644
--- a/drivers/video/tegra20/tegra-dc.c
+++ b/drivers/video/tegra20/tegra-dc.c
@@ -48,6 +48,7 @@ struct tegra_lcd_priv {
 	fdt_addr_t frame_buffer;	/* Address of frame buffer */
 	unsigned pixel_clock;		/* Pixel clock in Hz */
 	int dc_clk[2];			/* Contains clk and its parent */
+	ulong scdiv;			/* Clock divider used by disp_clk_ctrl */
 	bool rotation;			/* 180 degree panel turn */
 	bool pipe;			/* DC controller: 0 for A, 1 for B */
 };
@@ -126,8 +127,6 @@ static int update_display_mode(struct tegra_lcd_priv *priv)
 	struct dc_disp_reg *disp = &priv->dc->disp;
 	struct display_timing *dt = &priv->timing;
 	unsigned long val;
-	unsigned long rate;
-	unsigned long div;
 
 	writel(0x0, &disp->disp_timing_opt);
 
@@ -150,21 +149,11 @@ static int update_display_mode(struct tegra_lcd_priv *priv)
 		writel(val, &disp->disp_interface_ctrl);
 	}
 
-	/*
-	 * The pixel clock divider is in 7.1 format (where the bottom bit
-	 * represents 0.5). Here we calculate the divider needed to get from
-	 * the display clock (typically 600MHz) to the pixel clock. We round
-	 * up or down as requried.
-	 */
-	rate = clock_get_periph_rate(priv->dc_clk[0], priv->dc_clk[1]);
-	div = ((rate * 2 + priv->pixel_clock / 2) / priv->pixel_clock) - 2;
-	debug("Display clock %lu, divider %lu\n", rate, div);
-
 	if (priv->soc->has_rgb)
 		writel(0x00010001, &disp->shift_clk_opt);
 
 	val = PIXEL_CLK_DIVIDER_PCD1 << PIXEL_CLK_DIVIDER_SHIFT;
-	val |= div << SHIFT_CLK_DIVIDER_SHIFT;
+	val |= priv->scdiv << SHIFT_CLK_DIVIDER_SHIFT;
 	writel(val, &disp->disp_clk_ctrl);
 
 	return 0;
@@ -314,6 +303,17 @@ static int tegra_display_probe(struct tegra_lcd_priv *priv,
 		rate /= 2;
 #endif
 
+	/*
+	 * The pixel clock divider is in 7.1 format (where the bottom bit
+	 * represents 0.5). Here we calculate the divider needed to get from
+	 * the display clock (typically 600MHz) to the pixel clock. We round
+	 * up or down as required.
+	 */
+	if (!priv->scdiv)
+		priv->scdiv = ((rate * 2 + priv->pixel_clock / 2)
+						/ priv->pixel_clock) - 2;
+	debug("Display clock %lu, divider %lu\n", rate, priv->scdiv);
+
 	/*
 	 * HOST1X is init by default at 150MHz with PLLC as parent
 	 */
@@ -390,6 +390,14 @@ static int tegra_lcd_probe(struct udevice *dev)
 	reset_deassert(&reset_ctl);
 	mdelay(4);
 
+	/* Get shift clock divider from Tegra DSI if used */
+	if (!strcmp(priv->panel->name, TEGRA_DSI_A) ||
+	    !strcmp(priv->panel->name, TEGRA_DSI_B)) {
+		struct tegra_dc_plat *dc_plat = dev_get_plat(priv->panel);
+
+		priv->scdiv = dc_plat->scdiv;
+	}
+
 	if (tegra_display_probe(priv, (void *)plat->base)) {
 		debug("%s: Failed to probe display driver\n", __func__);
 		return -1;
diff --git a/drivers/video/tegra20/tegra-dc.h b/drivers/video/tegra20/tegra-dc.h
index 75fc0fa4de..05042dab1c 100644
--- a/drivers/video/tegra20/tegra-dc.h
+++ b/drivers/video/tegra20/tegra-dc.h
@@ -23,6 +23,7 @@ struct tegra_dc_plat {
 	struct udevice *dev;		/* Display controller device */
 	struct dc_ctlr *dc;		/* Display controller regmap */
 	bool pipe;			/* DC number: 0 for A, 1 for B */
+	ulong scdiv;			/* Shift clock divider */
 };
 
 /* This holds information about a window which can be displayed */
diff --git a/drivers/video/tegra20/tegra-dsi.c b/drivers/video/tegra20/tegra-dsi.c
index 72b91ed26b..de225ed376 100644
--- a/drivers/video/tegra20/tegra-dsi.c
+++ b/drivers/video/tegra20/tegra-dsi.c
@@ -743,6 +743,7 @@ static int tegra_dsi_panel_timings(struct udevice *dev,
 static void tegra_dsi_init_clocks(struct udevice *dev)
 {
 	struct tegra_dsi_priv *priv = dev_get_priv(dev);
+	struct tegra_dc_plat *dc_plat = dev_get_plat(dev);
 	struct mipi_dsi_device *device = &priv->device;
 	unsigned int mul, div;
 	unsigned long bclk, plld;
@@ -754,6 +755,19 @@ static void tegra_dsi_init_clocks(struct udevice *dev)
 
 	plld = DIV_ROUND_UP(bclk * 8, USEC_PER_SEC);
 
+	dc_plat->scdiv = ((plld * USEC_PER_SEC +
+		priv->timing.pixelclock.typ / 2) /
+		priv->timing.pixelclock.typ) - 2;
+
+	/*
+	 * BUG: If DISP1 is a PLLD/D2 child, it cannot go over 370MHz. The
+	 * cause of this is not quite clear. This can be overcomed by
+	 * halving the PLLD/D2 if the target rate is > 800MHz. This way
+	 * DISP1 and DSI clocks will be equal.
+	 */
+	if (plld > 800)
+		plld /= 2;
+
 	switch (clock_get_osc_freq()) {
 	case CLOCK_OSC_FREQ_12_0: /* OSC is 12Mhz */
 	case CLOCK_OSC_FREQ_48_0: /* OSC is 48Mhz */
-- 
2.40.1

Re: [PATCH v6 08/18] video: tegra20: dc: configure behavior if PLLD/D2 is used

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> If DISP1 is a PLLD/D2 child, it cannot go over 370MHz. The cause
> of this is not quite clear. This can be overcomed by further
> halving the PLLD/D2 if the target parent rate is over 800MHz.
> This way DISP1 and DSI clocks will have the same frequency. The
> shift divider in this case has to be calculated from the
> original PLLD/D2 frequency and is passed from the DSI driver.
>
> Tested-by: Andreas Westman Dorcsak <hedmoo@yahoo.com> # ASUS Grouper E1565
> Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
> Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
> Tested-by: Jonas Schwöbel <jonasschwoebel@yahoo.de> # Microsoft Surface 2
> Signed-off-by: Jonas Schwöbel <jonasschwoebel@yahoo.de>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/video/tegra20/tegra-dc.c  | 34 +++++++++++++++++++------------
>  drivers/video/tegra20/tegra-dc.h  |  1 +
>  drivers/video/tegra20/tegra-dsi.c | 14 +++++++++++++
>  3 files changed, 36 insertions(+), 13 deletions(-)

I'm not very familiar with these clocks, but seeing that this was
extensively tested, I guess this is okay, so:

Acked-by: Thierry Reding <treding@nvidia.com>

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

[PATCH v6 09/18] video: tegra20: dc: fix printing of framebuffer address

[Svyatoslav Ryhel]

From: Jonas Schwöbel <jonasschwoebel@yahoo.de>

Framebuffer address should not be a pointer.

Signed-off-by: Jonas Schwöbel <jonasschwoebel@yahoo.de>
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/video/tegra20/tegra-dc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
index a8e32e6893..000a0e02f8 100644
--- a/drivers/video/tegra20/tegra-dc.c
+++ b/drivers/video/tegra20/tegra-dc.c
@@ -429,7 +429,7 @@ static int tegra_lcd_probe(struct udevice *dev)
 	uc_priv->xsize = priv->width;
 	uc_priv->ysize = priv->height;
 	uc_priv->bpix = priv->log2_bpp;
-	debug("LCD frame buffer at %pa, size %x\n", &priv->frame_buffer,
+	debug("LCD frame buffer at %08x, size %x\n", priv->frame_buffer,
 	      plat->size);
 
 	return 0;
-- 
2.40.1

Re: [PATCH v6 09/18] video: tegra20: dc: fix printing of framebuffer address

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> From: Jonas Schwöbel <jonasschwoebel@yahoo.de>
>
> Framebuffer address should not be a pointer.
>
> Signed-off-by: Jonas Schwöbel <jonasschwoebel@yahoo.de>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/video/tegra20/tegra-dc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
> index a8e32e6893..000a0e02f8 100644
> --- a/drivers/video/tegra20/tegra-dc.c
> +++ b/drivers/video/tegra20/tegra-dc.c
> @@ -429,7 +429,7 @@ static int tegra_lcd_probe(struct udevice *dev)
>  	uc_priv->xsize = priv->width;
>  	uc_priv->ysize = priv->height;
>  	uc_priv->bpix = priv->log2_bpp;
> -	debug("LCD frame buffer at %pa, size %x\n", &priv->frame_buffer,
> +	debug("LCD frame buffer at %08x, size %x\n", priv->frame_buffer,

%pa is usually treated in a special way. At least it is on Linux, not
sure if the U-Boot printf implementation uses this, too. Looking at
doc/develop/printf.rst, it seems like this is supported, and given that
priv->frame_buffer is fdt_addr_t, %pa seems totally appropriate here.

Thierry

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

[PATCH v6 10/18] video: tegra20: dc: enable backlight after DC is configured

[Svyatoslav Ryhel]

From: Jonas Schwöbel <jonasschwoebel@yahoo.de>

The goal of panel_set_backlight() is to enable backlight. Hence,
it should be called at the probe end.

Signed-off-by: Jonas Schwöbel <jonasschwoebel@yahoo.de>
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/video/tegra20/tegra-dc.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
index 000a0e02f8..ac6833229a 100644
--- a/drivers/video/tegra20/tegra-dc.c
+++ b/drivers/video/tegra20/tegra-dc.c
@@ -414,12 +414,6 @@ static int tegra_lcd_probe(struct udevice *dev)
 		return ret;
 	}
 
-	ret = panel_set_backlight(priv->panel, BACKLIGHT_DEFAULT);
-	if (ret) {
-		debug("%s: Cannot set backlight to default, ret=%d\n", __func__, ret);
-		return ret;
-	}
-
 	mmu_set_region_dcache_behaviour(priv->frame_buffer, plat->size,
 					DCACHE_WRITETHROUGH);
 
@@ -432,7 +426,7 @@ static int tegra_lcd_probe(struct udevice *dev)
 	debug("LCD frame buffer at %08x, size %x\n", priv->frame_buffer,
 	      plat->size);
 
-	return 0;
+	return panel_set_backlight(priv->panel, BACKLIGHT_DEFAULT);
 }
 
 static int tegra_lcd_of_to_plat(struct udevice *dev)
-- 
2.40.1

Re: [PATCH v6 10/18] video: tegra20: dc: enable backlight after DC is configured

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> From: Jonas Schwöbel <jonasschwoebel@yahoo.de>
>
> The goal of panel_set_backlight() is to enable backlight. Hence,
> it should be called at the probe end.
>
> Signed-off-by: Jonas Schwöbel <jonasschwoebel@yahoo.de>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/video/tegra20/tegra-dc.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)

Reviewed-by: Thierry Reding <treding@nvidia.com>

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

[PATCH v6 11/18] video: tegra20: dc: clean framebuffer memory block

[Svyatoslav Ryhel]

From: Jonas Schwöbel <jonasschwoebel@yahoo.de>

Fill the framebuffer memory with zeros to avoid visual glitches.

Signed-off-by: Jonas Schwöbel <jonasschwoebel@yahoo.de>
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/video/tegra20/tegra-dc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
index ac6833229a..caeec94468 100644
--- a/drivers/video/tegra20/tegra-dc.c
+++ b/drivers/video/tegra20/tegra-dc.c
@@ -4,6 +4,7 @@
  */
 
 #include <backlight.h>
+#include <cpu_func.h>
 #include <dm.h>
 #include <fdtdec.h>
 #include <log.h>
@@ -398,6 +399,10 @@ static int tegra_lcd_probe(struct udevice *dev)
 		priv->scdiv = dc_plat->scdiv;
 	}
 
+	/* Clean the framebuffer area */
+	memset((u8 *)plat->base, 0, plat->size);
+	flush_dcache_all();
+
 	if (tegra_display_probe(priv, (void *)plat->base)) {
 		debug("%s: Failed to probe display driver\n", __func__);
 		return -1;
-- 
2.40.1

Re: [PATCH v6 11/18] video: tegra20: dc: clean framebuffer memory block

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> From: Jonas Schwöbel <jonasschwoebel@yahoo.de>
>
> Fill the framebuffer memory with zeros to avoid visual glitches.
>
> Signed-off-by: Jonas Schwöbel <jonasschwoebel@yahoo.de>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/video/tegra20/tegra-dc.c | 5 +++++
>  1 file changed, 5 insertions(+)

Makes sense:

Reviewed-by: Thierry Reding <treding@nvidia.com>

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

[PATCH v6 12/18] video: tegra20: dc: parameterize V- and H-sync polarities

[Svyatoslav Ryhel]

Based on Thierry Reding's Linux commit:

'commit 1716b1891e1de05e2c20ccafa9f58550f3539717
("drm/tegra: rgb: Parameterize V- and H-sync polarities")'

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 arch/arm/include/asm/arch-tegra/dc.h |  5 +++++
 drivers/video/tegra20/tegra-dc.c     | 22 ++++++++++++++++++++--
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/arch-tegra/dc.h b/arch/arm/include/asm/arch-tegra/dc.h
index 6444af2993..ca3718411a 100644
--- a/arch/arm/include/asm/arch-tegra/dc.h
+++ b/arch/arm/include/asm/arch-tegra/dc.h
@@ -443,6 +443,11 @@ enum win_color_depth_id {
 #define	WINDOW_D_SELECT		BIT(7)
 #define	WINDOW_H_SELECT		BIT(8)
 
+/* DC_COM_PIN_OUTPUT_POLARITY1 0x307 */
+#define LHS_OUTPUT_POLARITY_LOW		BIT(30)
+#define LVS_OUTPUT_POLARITY_LOW		BIT(28)
+#define LSC0_OUTPUT_POLARITY_LOW	BIT(24)
+
 /* DC_DISP_DISP_WIN_OPTIONS 0x402 */
 #define	CURSOR_ENABLE		BIT(16)
 #define	SOR_ENABLE		BIT(25)
diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
index caeec94468..ffa81eeac3 100644
--- a/drivers/video/tegra20/tegra-dc.c
+++ b/drivers/video/tegra20/tegra-dc.c
@@ -220,8 +220,11 @@ static const u32 rgb_sel_tab[PIN_OUTPUT_SEL_COUNT] = {
 	0x00020000,
 };
 
-static void rgb_enable(struct dc_com_reg *com)
+static void rgb_enable(struct tegra_lcd_priv *priv)
 {
+	struct dc_com_reg *com = &priv->dc->com;
+	struct display_timing *dt = &priv->timing;
+	u32 value;
 	int i;
 
 	for (i = 0; i < PIN_REG_COUNT; i++) {
@@ -230,6 +233,21 @@ static void rgb_enable(struct dc_com_reg *com)
 		writel(rgb_data_tab[i], &com->pin_output_data[i]);
 	}
 
+	/* configure H- and V-sync signal polarities */
+	value = readl(&com->pin_output_polarity[1]);
+
+	if (dt->flags & DISPLAY_FLAGS_HSYNC_LOW)
+		value |= LHS_OUTPUT_POLARITY_LOW;
+	else
+		value &= ~LHS_OUTPUT_POLARITY_LOW;
+
+	if (dt->flags & DISPLAY_FLAGS_VSYNC_LOW)
+		value |= LVS_OUTPUT_POLARITY_LOW;
+	else
+		value &= ~LVS_OUTPUT_POLARITY_LOW;
+
+	writel(value, &com->pin_output_polarity[1]);
+
 	for (i = 0; i < PIN_OUTPUT_SEL_COUNT; i++)
 		writel(rgb_sel_tab[i], &com->pin_output_sel[i]);
 }
@@ -329,7 +347,7 @@ static int tegra_display_probe(struct tegra_lcd_priv *priv,
 		basic_init_timer(&priv->dc->disp);
 
 	if (priv->soc->has_rgb)
-		rgb_enable(&priv->dc->com);
+		rgb_enable(priv);
 
 	if (priv->pixel_clock)
 		update_display_mode(priv);
-- 
2.40.1

Re: [PATCH v6 12/18] video: tegra20: dc: parameterize V- and H-sync polarities

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> Based on Thierry Reding's Linux commit:
>
> 'commit 1716b1891e1de05e2c20ccafa9f58550f3539717
> ("drm/tegra: rgb: Parameterize V- and H-sync polarities")'
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  arch/arm/include/asm/arch-tegra/dc.h |  5 +++++
>  drivers/video/tegra20/tegra-dc.c     | 22 ++++++++++++++++++++--
>  2 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-tegra/dc.h b/arch/arm/include/asm/arch-tegra/dc.h
> index 6444af2993..ca3718411a 100644
> --- a/arch/arm/include/asm/arch-tegra/dc.h
> +++ b/arch/arm/include/asm/arch-tegra/dc.h
> @@ -443,6 +443,11 @@ enum win_color_depth_id {
>  #define	WINDOW_D_SELECT		BIT(7)
>  #define	WINDOW_H_SELECT		BIT(8)
>  
> +/* DC_COM_PIN_OUTPUT_POLARITY1 0x307 */
> +#define LHS_OUTPUT_POLARITY_LOW		BIT(30)
> +#define LVS_OUTPUT_POLARITY_LOW		BIT(28)
> +#define LSC0_OUTPUT_POLARITY_LOW	BIT(24)

This definition seems to be unused, other than that:

Reviewed-by: Thierry Reding <treding@nvidia.com>

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

[PATCH v6 13/18] video: tegra20: add MIPI calibration driver

[Svyatoslav Ryhel]

Dedicated MIPI calibration driver is used on T114 and newer. Before
T114 MIPI calibration registers were part of VI and CSI.

Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/video/tegra20/Makefile     |   2 +-
 drivers/video/tegra20/tegra-mipi.c | 188 +++++++++++++++++++++++++++++
 2 files changed, 189 insertions(+), 1 deletion(-)
 create mode 100644 drivers/video/tegra20/tegra-mipi.c

diff --git a/drivers/video/tegra20/Makefile b/drivers/video/tegra20/Makefile
index f0b534c579..a75aea2a87 100644
--- a/drivers/video/tegra20/Makefile
+++ b/drivers/video/tegra20/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0+
 
 obj-$(CONFIG_VIDEO_TEGRA20) += tegra-dc.o
-obj-$(CONFIG_VIDEO_DSI_TEGRA30) += tegra-dsi.o mipi-phy.o
+obj-$(CONFIG_VIDEO_DSI_TEGRA30) += tegra-dsi.o tegra-mipi.o mipi-phy.o
 obj-$(CONFIG_TEGRA_BACKLIGHT_PWM) += tegra-pwm-backlight.o
diff --git a/drivers/video/tegra20/tegra-mipi.c b/drivers/video/tegra20/tegra-mipi.c
new file mode 100644
index 0000000000..2df3c1a994
--- /dev/null
+++ b/drivers/video/tegra20/tegra-mipi.c
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2013 NVIDIA Corporation
+ * Copyright (c) 2023 Svyatoslav Ryhel <clamor95@gmail.com>
+ */
+
+#include <dm.h>
+#include <clk.h>
+#include <misc.h>
+#include <linux/delay.h>
+#include <linux/iopoll.h>
+
+#include <asm/io.h>
+
+/* MIPI control registers 0x00 ~ 0x60 */
+struct mipi_ctlr {
+	uint mipi_cal_ctrl;
+	uint mipi_cal_autocal_ctrl;
+	uint mipi_cal_status;
+
+	uint unused1[2];
+
+	uint mipi_cal_config_csia;
+	uint mipi_cal_config_csib;
+	uint mipi_cal_config_csic;
+	uint mipi_cal_config_csid;
+	uint mipi_cal_config_csie;
+
+	uint unused2[4];
+
+	uint mipi_cal_config_dsia;
+	uint mipi_cal_config_dsib;
+	uint mipi_cal_config_dsic;
+	uint mipi_cal_config_dsid;
+
+	uint unused3[4];
+
+	uint mipi_cal_bias_pad_cfg0;
+	uint mipi_cal_bias_pad_cfg1;
+	uint mipi_cal_bias_pad_cfg2;
+};
+
+#define MIPI_CAL_CTRL_NOISE_FILTER(x)	(((x) & 0xf) << 26)
+#define MIPI_CAL_CTRL_PRESCALE(x)	(((x) & 0x3) << 24)
+#define MIPI_CAL_CTRL_CLKEN_OVR		BIT(4)
+#define MIPI_CAL_CTRL_START		BIT(0)
+
+#define MIPI_CAL_STATUS_DONE		BIT(16)
+#define MIPI_CAL_STATUS_ACTIVE		BIT(0)
+
+#define MIPI_CAL_OVERIDE(x)		(((x) & 0x1) << 30)
+#define MIPI_CAL_SEL(x)			(((x) & 0x1) << 21)
+#define MIPI_CAL_HSPDOS(x)		(((x) & 0x1f) << 16)
+#define MIPI_CAL_HSPUOS(x)		(((x) & 0x1f) << 8)
+#define MIPI_CAL_TERMOS(x)		(((x) & 0x1f) << 0)
+
+#define MIPI_CAL_BIAS_PAD_PDVCLAMP	BIT(1)
+#define MIPI_CAL_BIAS_PAD_E_VCLAMP_REF	BIT(0)
+
+#define MIPI_CAL_BIAS_PAD_DRV_DN_REF(x) (((x) & 0x7) << 16)
+#define MIPI_CAL_BIAS_PAD_DRV_UP_REF(x) (((x) & 0x7) << 8)
+
+#define MIPI_CAL_BIAS_PAD_VCLAMP(x)	(((x) & 0x7) << 16)
+#define MIPI_CAL_BIAS_PAD_VAUXP(x)	(((x) & 0x7) << 4)
+#define MIPI_CAL_BIAS_PAD_PDVREG	BIT(1)
+
+struct tegra_mipi_priv {
+	struct mipi_ctlr	*mipi;
+	struct clk		*mipi_cal;
+};
+
+static int tegra_mipi_calibrate(struct udevice *dev, int offset, const void *buf,
+				int size)
+{
+	struct tegra_mipi_priv *priv = dev_get_priv(dev);
+	u32 value;
+
+	value = MIPI_CAL_BIAS_PAD_DRV_DN_REF(0x2) |
+		MIPI_CAL_BIAS_PAD_DRV_UP_REF(0x0);
+	writel(value, &priv->mipi->mipi_cal_bias_pad_cfg1);
+
+	value = readl(&priv->mipi->mipi_cal_bias_pad_cfg2);
+	value &= ~MIPI_CAL_BIAS_PAD_VCLAMP(0x7);
+	value &= ~MIPI_CAL_BIAS_PAD_VAUXP(0x7);
+	writel(value, &priv->mipi->mipi_cal_bias_pad_cfg2);
+
+	value = MIPI_CAL_OVERIDE(0x0) | MIPI_CAL_SEL(0x1) |
+		MIPI_CAL_HSPDOS(0x0) | MIPI_CAL_HSPUOS(0x4) |
+		MIPI_CAL_TERMOS(0x5);
+	writel(value, &priv->mipi->mipi_cal_config_dsia);
+	writel(value, &priv->mipi->mipi_cal_config_dsib);
+
+	/* Deselect PAD C */
+	value = readl(&priv->mipi->mipi_cal_config_dsic);
+	value &= ~(MIPI_CAL_SEL(0x1));
+	writel(value, &priv->mipi->mipi_cal_config_dsic);
+
+	/* Deselect PAD D */
+	value = readl(&priv->mipi->mipi_cal_config_dsid);
+	value &= ~(MIPI_CAL_SEL(0x1));
+	writel(value, &priv->mipi->mipi_cal_config_dsid);
+
+	value = readl(&priv->mipi->mipi_cal_ctrl);
+	value &= ~MIPI_CAL_CTRL_NOISE_FILTER(0xf);
+	value &= ~MIPI_CAL_CTRL_PRESCALE(0x3);
+	value |= MIPI_CAL_CTRL_NOISE_FILTER(0xa) |
+		 MIPI_CAL_CTRL_PRESCALE(0x2) |
+		 MIPI_CAL_CTRL_CLKEN_OVR;
+	writel(value, &priv->mipi->mipi_cal_ctrl);
+
+	/* clear any pending status bits */
+	value = readl(&priv->mipi->mipi_cal_status);
+	writel(value, &priv->mipi->mipi_cal_status);
+
+	value = readl(&priv->mipi->mipi_cal_ctrl);
+	value |= MIPI_CAL_CTRL_START;
+	writel(value, &priv->mipi->mipi_cal_ctrl);
+
+	/*
+	 * Wait for min 72uS to let calibration logic finish calibration
+	 * sequence codes before waiting for pads idle state to apply the
+	 * results.
+	 */
+	udelay(80);
+
+	return readl_poll_sleep_timeout(&priv->mipi->mipi_cal_status, value,
+					!(value & MIPI_CAL_STATUS_ACTIVE) &&
+					(value & MIPI_CAL_STATUS_DONE), 100,
+					250000);
+}
+
+static int tegra_mipi_enable(struct udevice *dev, bool val)
+{
+	struct tegra_mipi_priv *priv = dev_get_priv(dev);
+	u32 value;
+
+	clk_enable(priv->mipi_cal);
+
+	value = readl(&priv->mipi->mipi_cal_bias_pad_cfg0);
+	value &= ~MIPI_CAL_BIAS_PAD_PDVCLAMP;
+	value |= MIPI_CAL_BIAS_PAD_E_VCLAMP_REF;
+	writel(value, &priv->mipi->mipi_cal_bias_pad_cfg0);
+
+	value = readl(&priv->mipi->mipi_cal_bias_pad_cfg2);
+	value &= ~MIPI_CAL_BIAS_PAD_PDVREG;
+	writel(value, &priv->mipi->mipi_cal_bias_pad_cfg2);
+
+	return 0;
+}
+
+static const struct misc_ops tegra_mipi_ops = {
+	.write = tegra_mipi_calibrate,
+	.set_enabled = tegra_mipi_enable,
+};
+
+static int tegra_mipi_probe(struct udevice *dev)
+{
+	struct tegra_mipi_priv *priv = dev_get_priv(dev);
+
+	priv->mipi = (struct mipi_ctlr *)dev_read_addr_ptr(dev);
+	if (!priv->mipi) {
+		log_debug("%s: no MIPI controller address\n", __func__);
+		return -EINVAL;
+	}
+
+	priv->mipi_cal = devm_clk_get(dev, NULL);
+	if (IS_ERR(priv->mipi_cal)) {
+		log_debug("%s: Could not get MIPI clock: %ld\n",
+			  __func__, PTR_ERR(priv->mipi_cal));
+		return PTR_ERR(priv->mipi_cal);
+	}
+
+	return 0;
+}
+
+static const struct udevice_id tegra_mipi_ids[] = {
+	{ .compatible = "nvidia,tegra114-mipi" },
+	{ }
+};
+
+U_BOOT_DRIVER(tegra_mipi) = {
+	.name           = "tegra_mipi",
+	.id             = UCLASS_MISC,
+	.ops		= &tegra_mipi_ops,
+	.of_match       = tegra_mipi_ids,
+	.probe          = tegra_mipi_probe,
+	.priv_auto	= sizeof(struct tegra_mipi_priv),
+};
-- 
2.40.1

Re: [PATCH v6 13/18] video: tegra20: add MIPI calibration driver

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> Dedicated MIPI calibration driver is used on T114 and newer. Before
> T114 MIPI calibration registers were part of VI and CSI.
>
> Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/video/tegra20/Makefile     |   2 +-
>  drivers/video/tegra20/tegra-mipi.c | 188 +++++++++++++++++++++++++++++
>  2 files changed, 189 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/video/tegra20/tegra-mipi.c

This looks like an adequate copy of what we have in the Linux kernel.
It's slightly suboptimal because most of the register values are hard-
coded for Tegra114, but I guess that can always be improved later on.

Reviewed-by: Thierry Reding <treding@nvidia.com>

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

[PATCH v6 14/18] video: tegra20: dsi: add T114 support

[Svyatoslav Ryhel]

Existing Tegra DSI driver mostly fits T114 apart MIPI calibration
which on T114 has dedicated driver. To resolve this MIPI calibration
logic was split for pre-T114 and T114+ devices.

Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/video/tegra20/tegra-dsi.c             | 78 ++++++++++++++++++-
 .../video/tegra20/tegra-dsi.h                 | 24 +++++-
 2 files changed, 96 insertions(+), 6 deletions(-)
 rename arch/arm/include/asm/arch-tegra30/dsi.h => drivers/video/tegra20/tegra-dsi.h (90%)

diff --git a/drivers/video/tegra20/tegra-dsi.c b/drivers/video/tegra20/tegra-dsi.c
index de225ed376..25a629535e 100644
--- a/drivers/video/tegra20/tegra-dsi.c
+++ b/drivers/video/tegra20/tegra-dsi.c
@@ -20,17 +20,24 @@
 #include <asm/gpio.h>
 #include <asm/io.h>
 #include <asm/arch/clock.h>
-#include <asm/arch-tegra30/dsi.h>
 
 #include "tegra-dc.h"
+#include "tegra-dsi.h"
 #include "mipi-phy.h"
 
+/* List of supported DSI bridges */
+enum {
+	DSI_V0,
+	DSI_V1,
+};
+
 struct tegra_dsi_priv {
 	struct mipi_dsi_host host;
 	struct mipi_dsi_device device;
 	struct mipi_dphy_timing dphy_timing;
 
 	struct udevice *panel;
+	struct udevice *mipi;
 	struct display_timing timing;
 
 	struct dsi_ctlr *dsi;
@@ -41,6 +48,8 @@ struct tegra_dsi_priv {
 	int dsi_clk;
 	int video_fifo_depth;
 	int host_fifo_depth;
+
+	u32 version;
 };
 
 static void tegra_dc_enable_controller(struct udevice *dev)
@@ -501,6 +510,41 @@ static void tegra_dsi_pad_calibrate(struct dsi_pad_ctrl_reg *pad)
 	writel(value, TEGRA_VI_BASE + (CSI_CIL_PAD_CONFIG << 2));
 }
 
+static void tegra_dsi_mipi_calibrate(struct tegra_dsi_priv *priv)
+{
+	struct dsi_pad_ctrl_reg *pad = &priv->dsi->pad;
+	u32 value;
+	int ret;
+
+	ret = misc_set_enabled(priv->mipi, true);
+	if (ret)
+		log_debug("%s: failed to enable MIPI calibration: %d\n",
+			  __func__, ret);
+
+	writel(0, &pad->pad_ctrl);
+	writel(0, &pad->pad_ctrl_1);
+	writel(0, &pad->pad_ctrl_2);
+	writel(0, &pad->pad_ctrl_3);
+	writel(0, &pad->pad_ctrl_4);
+
+	/* DSI pad enable */
+	value = DSI_PAD_CONTROL_VS1_PULLDN(0) | DSI_PAD_CONTROL_VS1_PDIO(0);
+	writel(value, &pad->pad_ctrl);
+
+	value = DSI_PAD_SLEW_UP(0x7) | DSI_PAD_SLEW_DN(0x7) |
+		DSI_PAD_LP_UP(0x1) | DSI_PAD_LP_DN(0x1) |
+		DSI_PAD_OUT_CLK(0x0);
+	writel(value, &pad->pad_ctrl_2);
+
+	value = DSI_PAD_PREEMP_PD_CLK(0x3) | DSI_PAD_PREEMP_PU_CLK(0x3) |
+		DSI_PAD_PREEMP_PD(0x03) | DSI_PAD_PREEMP_PU(0x3);
+	writel(value, &pad->pad_ctrl_3);
+
+	ret = misc_write(priv->mipi, 0, NULL, 0);
+	if (ret)
+		log_debug("%s: MIPI calibration failed %d\n", __func__, ret);
+}
+
 static void tegra_dsi_set_timeout(struct dsi_timeout_reg *rtimeout,
 				  unsigned long bclk,
 				  unsigned int vrefresh)
@@ -664,10 +708,25 @@ static int tegra_dsi_encoder_enable(struct udevice *dev)
 	u32 value;
 	int ret;
 
+	/* If for some reasone DSI is enabled then it needs to
+	 * be disabled in order for the panel initialization
+	 * commands to be properly sent.
+	 */
+	value = readl(&misc->dsi_pwr_ctrl);
+
+	if (value & DSI_POWER_CONTROL_ENABLE) {
+		value = readl(&misc->dsi_pwr_ctrl);
+		value &= ~DSI_POWER_CONTROL_ENABLE;
+		writel(value, &misc->dsi_pwr_ctrl);
+	}
+
 	/* Disable interrupt */
 	writel(0, &misc->int_enable);
 
-	tegra_dsi_pad_calibrate(&priv->dsi->pad);
+	if (priv->version)
+		tegra_dsi_mipi_calibrate(priv);
+	else
+		tegra_dsi_pad_calibrate(&priv->dsi->pad);
 
 	tegra_dsi_get_muldiv(device->format, &mul, &div);
 
@@ -806,6 +865,8 @@ static int tegra_dsi_bridge_probe(struct udevice *dev)
 	struct mipi_dsi_panel_plat *mipi_plat;
 	int ret;
 
+	priv->version = dev_get_driver_data(dev);
+
 	priv->dsi = (struct dsi_ctlr *)dev_read_addr_ptr(dev);
 	if (!priv->dsi) {
 		printf("%s: No display controller address\n", __func__);
@@ -828,6 +889,16 @@ static int tegra_dsi_bridge_probe(struct udevice *dev)
 		return log_ret(ret);
 	}
 
+	if (priv->version) {
+		ret = uclass_get_device_by_phandle(UCLASS_MISC, dev,
+						   "nvidia,mipi-calibrate",
+						   &priv->mipi);
+		if (ret) {
+			log_debug("%s: cannot get MIPI: error %d\n", __func__, ret);
+			return ret;
+		}
+	}
+
 	panel_get_display_timing(priv->panel, &priv->timing);
 
 	mipi_plat = dev_get_plat(priv->panel);
@@ -859,7 +930,8 @@ static const struct panel_ops tegra_dsi_bridge_ops = {
 };
 
 static const struct udevice_id tegra_dsi_bridge_ids[] = {
-	{ .compatible = "nvidia,tegra30-dsi" },
+	{ .compatible = "nvidia,tegra30-dsi", .data = DSI_V0 },
+	{ .compatible = "nvidia,tegra114-dsi", .data = DSI_V1 },
 	{ }
 };
 
diff --git a/arch/arm/include/asm/arch-tegra30/dsi.h b/drivers/video/tegra20/tegra-dsi.h
similarity index 90%
rename from arch/arm/include/asm/arch-tegra30/dsi.h
rename to drivers/video/tegra20/tegra-dsi.h
index 7ade132613..69dac4bd1b 100644
--- a/arch/arm/include/asm/arch-tegra30/dsi.h
+++ b/drivers/video/tegra20/tegra-dsi.h
@@ -4,8 +4,8 @@
  *  NVIDIA Corporation <www.nvidia.com>
  */
 
-#ifndef __ASM_ARCH_TEGRA_DSI_H
-#define __ASM_ARCH_TEGRA_DSI_H
+#ifndef _TEGRA_DSI_H
+#define _TEGRA_DSI_H
 
 #ifndef __ASSEMBLY__
 #include <linux/bitops.h>
@@ -105,6 +105,10 @@ struct dsi_pad_ctrl_reg {
 	uint pad_ctrl_cd;		/* _PAD_CONTROL_CD_0 */
 	uint pad_cd_status;		/* _PAD_CD_STATUS_0 */
 	uint dsi_vid_mode_control;	/* _DSI_VID_MODE_CONTROL_0 */
+	uint pad_ctrl_1;		/* _PAD_CONTROL_1 */
+	uint pad_ctrl_2;		/* _PAD_CONTROL_2 */
+	uint pad_ctrl_3;		/* _PAD_CONTROL_3 */
+	uint pad_ctrl_4;		/* _PAD_CONTROL_4 */
 };
 
 /* Display Serial Interface (DSI_) regs */
@@ -184,6 +188,20 @@ struct dsi_ctlr {
 #define DSI_PAD_CONTROL_PAD_LPUPADJ(x)		(((x) & 0x3) << 14)
 #define DSI_PAD_CONTROL_PAD_LPDNADJ(x)		(((x) & 0x3) << 12)
 
+#define DSI_PAD_CONTROL_VS1_PDIO(x)	(((x) & 0xf) <<  0)
+#define DSI_PAD_CONTROL_VS1_PULLDN(x)	(((x) & 0xf) << 16)
+
+#define DSI_PAD_OUT_CLK(x)		(((x) & 0x7) <<  0)
+#define DSI_PAD_LP_DN(x)		(((x) & 0x7) <<  4)
+#define DSI_PAD_LP_UP(x)		(((x) & 0x7) <<  8)
+#define DSI_PAD_SLEW_DN(x)		(((x) & 0x7) << 12)
+#define DSI_PAD_SLEW_UP(x)		(((x) & 0x7) << 16)
+
+#define DSI_PAD_PREEMP_PD_CLK(x)	(((x) & 0x3) << 12)
+#define DSI_PAD_PREEMP_PU_CLK(x)	(((x) & 0x3) << 8)
+#define DSI_PAD_PREEMP_PD(x)		(((x) & 0x3) << 4)
+#define DSI_PAD_PREEMP_PU(x)		(((x) & 0x3) << 0)
+
 /*
  * pixel format as used in the DSI_CONTROL_FORMAT field
  */
@@ -214,4 +232,4 @@ enum tegra_dsi_format {
 #define  PAD_DRIV_DN_REF(x)		(((x) & 0x7) << 16)
 #define  PAD_DRIV_UP_REF(x)		(((x) & 0x7) << 8)
 
-#endif /* __ASM_ARCH_TEGRA_DSI_H */
+#endif /* _TEGRA_DSI_H */
-- 
2.40.1

Re: [PATCH v6 14/18] video: tegra20: dsi: add T114 support

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> Existing Tegra DSI driver mostly fits T114 apart MIPI calibration
> which on T114 has dedicated driver. To resolve this MIPI calibration
> logic was split for pre-T114 and T114+ devices.
>
> Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
> Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/video/tegra20/tegra-dsi.c             | 78 ++++++++++++++++++-
>  .../video/tegra20/tegra-dsi.h                 | 24 +++++-
>  2 files changed, 96 insertions(+), 6 deletions(-)
>  rename arch/arm/include/asm/arch-tegra30/dsi.h => drivers/video/tegra20/tegra-dsi.h (90%)

Reviewed-by: Thierry Reding <treding@nvidia.com>

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

[PATCH v6 15/18] video: tegra20: dsi: add reset support

[Svyatoslav Ryhel]

Implement reset use to discard any changes which could have been
applied to DSI before and can interfere with current configuration.

Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/video/tegra20/tegra-dsi.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/video/tegra20/tegra-dsi.c b/drivers/video/tegra20/tegra-dsi.c
index 25a629535e..fc9ca1310a 100644
--- a/drivers/video/tegra20/tegra-dsi.c
+++ b/drivers/video/tegra20/tegra-dsi.c
@@ -12,6 +12,7 @@
 #include <mipi_dsi.h>
 #include <backlight.h>
 #include <panel.h>
+#include <reset.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/time.h>
@@ -863,6 +864,7 @@ static int tegra_dsi_bridge_probe(struct udevice *dev)
 	struct tegra_dsi_priv *priv = dev_get_priv(dev);
 	struct mipi_dsi_device *device = &priv->device;
 	struct mipi_dsi_panel_plat *mipi_plat;
+	struct reset_ctl reset_ctl;
 	int ret;
 
 	priv->version = dev_get_driver_data(dev);
@@ -876,6 +878,13 @@ static int tegra_dsi_bridge_probe(struct udevice *dev)
 	priv->video_fifo_depth = 480;
 	priv->host_fifo_depth = 64;
 
+	ret = reset_get_by_name(dev, "dsi", &reset_ctl);
+	if (ret) {
+		log_debug("%s: reset_get_by_name() failed: %d\n",
+			  __func__, ret);
+		return ret;
+	}
+
 	ret = uclass_get_device_by_phandle(UCLASS_REGULATOR, dev,
 					   "avdd-dsi-csi-supply", &priv->avdd);
 	if (ret)
@@ -914,12 +923,17 @@ static int tegra_dsi_bridge_probe(struct udevice *dev)
 
 	tegra_dsi_get_format(device->format, &priv->format);
 
+	reset_assert(&reset_ctl);
+
 	ret = regulator_set_enable_if_allowed(priv->avdd, true);
 	if (ret && ret != -ENOSYS)
 		return ret;
 
 	tegra_dsi_init_clocks(dev);
 
+	mdelay(2);
+	reset_deassert(&reset_ctl);
+
 	return 0;
 }
 
-- 
2.40.1

Re: [PATCH v6 15/18] video: tegra20: dsi: add reset support

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> Implement reset use to discard any changes which could have been
> applied to DSI before and can interfere with current configuration.
>
> Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
> Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/video/tegra20/tegra-dsi.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/video/tegra20/tegra-dsi.c b/drivers/video/tegra20/tegra-dsi.c
> index 25a629535e..fc9ca1310a 100644
> --- a/drivers/video/tegra20/tegra-dsi.c
> +++ b/drivers/video/tegra20/tegra-dsi.c
> @@ -12,6 +12,7 @@
>  #include <mipi_dsi.h>
>  #include <backlight.h>
>  #include <panel.h>
> +#include <reset.h>
>  #include <linux/delay.h>
>  #include <linux/err.h>
>  #include <linux/time.h>
> @@ -863,6 +864,7 @@ static int tegra_dsi_bridge_probe(struct udevice *dev)
>  	struct tegra_dsi_priv *priv = dev_get_priv(dev);
>  	struct mipi_dsi_device *device = &priv->device;
>  	struct mipi_dsi_panel_plat *mipi_plat;
> +	struct reset_ctl reset_ctl;
>  	int ret;
>  
>  	priv->version = dev_get_driver_data(dev);
> @@ -876,6 +878,13 @@ static int tegra_dsi_bridge_probe(struct udevice *dev)
>  	priv->video_fifo_depth = 480;
>  	priv->host_fifo_depth = 64;
>  
> +	ret = reset_get_by_name(dev, "dsi", &reset_ctl);
> +	if (ret) {
> +		log_debug("%s: reset_get_by_name() failed: %d\n",
> +			  __func__, ret);
> +		return ret;
> +	}
> +
>  	ret = uclass_get_device_by_phandle(UCLASS_REGULATOR, dev,
>  					   "avdd-dsi-csi-supply", &priv->avdd);
>  	if (ret)
> @@ -914,12 +923,17 @@ static int tegra_dsi_bridge_probe(struct udevice *dev)
>  
>  	tegra_dsi_get_format(device->format, &priv->format);
>  
> +	reset_assert(&reset_ctl);
> +
>  	ret = regulator_set_enable_if_allowed(priv->avdd, true);
>  	if (ret && ret != -ENOSYS)
>  		return ret;
>  
>  	tegra_dsi_init_clocks(dev);
>  
> +	mdelay(2);
> +	reset_deassert(&reset_ctl);
> +
>  	return 0;
>  }

Looks like tegra_dsi_init_clocks() already does the mdelay() and
reset_deassert()? Or perhaps I'm again looking at things in the wrong
order?

Thierry

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

[PATCH v6 16/18] video: tegra20: dsi: remove pre-configuration

[Svyatoslav Ryhel]

From: Jonas Schwöbel <jonasschwoebel@yahoo.de>

Configuration for DC driver command mode is not required for
every panel. Removed.

Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
Signed-off-by: Jonas Schwöbel <jonasschwoebel@yahoo.de>
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/video/tegra20/tegra-dsi.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/video/tegra20/tegra-dsi.c b/drivers/video/tegra20/tegra-dsi.c
index fc9ca1310a..ddd74540f2 100644
--- a/drivers/video/tegra20/tegra-dsi.c
+++ b/drivers/video/tegra20/tegra-dsi.c
@@ -766,8 +766,6 @@ static int tegra_dsi_encoder_enable(struct udevice *dev)
 	if (ret)
 		return ret;
 
-	tegra_dsi_configure(dev, 0);
-
 	ret = panel_set_backlight(priv->panel, BACKLIGHT_DEFAULT);
 	if (ret)
 		return ret;
-- 
2.40.1

Re: [PATCH v6 16/18] video: tegra20: dsi: remove pre-configuration

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> From: Jonas Schwöbel <jonasschwoebel@yahoo.de>
>
> Configuration for DC driver command mode is not required for
> every panel. Removed.
>
> Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
> Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
> Signed-off-by: Jonas Schwöbel <jonasschwoebel@yahoo.de>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/video/tegra20/tegra-dsi.c | 2 --
>  1 file changed, 2 deletions(-)

Yeah, this seems completely unnecessary.

Reviewed-by: Thierry Reding <treding@nvidia.com>

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

[PATCH v6 17/18] video: tegra20: dsi: set correct fifo depth

[Svyatoslav Ryhel]

From: Jonas Schwöbel <jonasschwoebel@yahoo.de>

According to Thierry Reding's commit in the linux kernel

976cebc35bed0456a42bf96073a26f251d23b264
"drm/tegra: dsi: Make FIFO depths host parameters"

correct depth of the video FIFO is 1920 *words* no *bytes*

Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
Signed-off-by: Jonas Schwöbel <jonasschwoebel@yahoo.de>
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/video/tegra20/tegra-dsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/tegra20/tegra-dsi.c b/drivers/video/tegra20/tegra-dsi.c
index ddd74540f2..7d63557d1b 100644
--- a/drivers/video/tegra20/tegra-dsi.c
+++ b/drivers/video/tegra20/tegra-dsi.c
@@ -873,7 +873,7 @@ static int tegra_dsi_bridge_probe(struct udevice *dev)
 		return -EINVAL;
 	}
 
-	priv->video_fifo_depth = 480;
+	priv->video_fifo_depth = 1920;
 	priv->host_fifo_depth = 64;
 
 	ret = reset_get_by_name(dev, "dsi", &reset_ctl);
-- 
2.40.1

Re: [PATCH v6 17/18] video: tegra20: dsi: set correct fifo depth

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> From: Jonas Schwöbel <jonasschwoebel@yahoo.de>
>
> According to Thierry Reding's commit in the linux kernel
>
> 976cebc35bed0456a42bf96073a26f251d23b264
> "drm/tegra: dsi: Make FIFO depths host parameters"
>
> correct depth of the video FIFO is 1920 *words* no *bytes*
>
> Tested-by: Ion Agorria <ion@agorria.com> # HTC One X
> Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Nvidia Tegratab T114
> Signed-off-by: Jonas Schwöbel <jonasschwoebel@yahoo.de>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/video/tegra20/tegra-dsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Thierry Reding <treding@nvidia.com>

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

[PATCH v6 18/18] video: tegra20: dsi: use set_backlight for backlight only

[Svyatoslav Ryhel]

From: Jonas Schwöbel <jonasschwoebel@yahoo.de>

Shift the backlight set further to prevent visual glitches on
panel init.

Signed-off-by: Jonas Schwöbel <jonasschwoebel@yahoo.de>
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/video/tegra20/tegra-dsi.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/video/tegra20/tegra-dsi.c b/drivers/video/tegra20/tegra-dsi.c
index 7d63557d1b..13dae37806 100644
--- a/drivers/video/tegra20/tegra-dsi.c
+++ b/drivers/video/tegra20/tegra-dsi.c
@@ -766,10 +766,6 @@ static int tegra_dsi_encoder_enable(struct udevice *dev)
 	if (ret)
 		return ret;
 
-	ret = panel_set_backlight(priv->panel, BACKLIGHT_DEFAULT);
-	if (ret)
-		return ret;
-
 	tegra_dsi_configure(dev, device->mode_flags);
 
 	tegra_dc_enable_controller(dev);
@@ -784,8 +780,10 @@ static int tegra_dsi_encoder_enable(struct udevice *dev)
 
 static int tegra_dsi_bridge_set_panel(struct udevice *dev, int percent)
 {
-	/* Is not used in tegra dc */
-	return 0;
+	struct tegra_dsi_priv *priv = dev_get_priv(dev);
+
+	/* Turn on/off backlight */
+	return panel_set_backlight(priv->panel, percent);
 }
 
 static int tegra_dsi_panel_timings(struct udevice *dev,
-- 
2.40.1

Re: [PATCH v6 18/18] video: tegra20: dsi: use set_backlight for backlight only

[Thierry Reding]

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

On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> From: Jonas Schwöbel <jonasschwoebel@yahoo.de>
>
> Shift the backlight set further to prevent visual glitches on
> panel init.
>
> Signed-off-by: Jonas Schwöbel <jonasschwoebel@yahoo.de>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/video/tegra20/tegra-dsi.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)

I vaguely recall that some devices may have had the panel and the
backlight hooked up to the same regulator or enable GPIO, so calling
panel_set_backlight() too late may cause the DSI panel to malfunction.

That said, I'm not familiar with panel_set_backlight(), so perhaps it
always only sets the brightness and the power may already be on earlier?

Ah... nevermind... I see that panel_enable_backlight() is called prior
to panel_set_backlight(), so this looks like it should be fine.

Reviewed-by: Thierry Reding <treding@nvidia.com>

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

Re: [PATCH v6 00/18] Add T114 video support

[Svyatoslav Ryhel]

Hello Tom! This patch set is hanging in patchwork for 3 month
without any comments. If no one has anything to say, may you
pick it into master?

Best regards,
Svyatoslav R.

вт, 23 січ. 2024 р. о 19:17 Svyatoslav Ryhel <clamor95@gmail.com> пише:
>
> T114 is not that different from T30 and all T30 drivers will work
> on T114 as well with some adjustments.
>
> Patches propose general improvements for existing Tegra DC and DSI
> drivers as well Tegra 114 video support (experimentl).
>
> Commits pass buildman for tegra.
>
> ---
> Changes from v5:
>  - backlight enable moved to the last step of setup for DSI
>  - parameterized V- and H-sync polarities
>  - added framebuffer clearing on probe to avoid glitches
>  - backlight enable moved after DC is fully configured
>  - fixed printing framebuffer pointer instead of address
>  - moved scdiv calculation to tegra DSI if it is used
>
> Changes from v4:
>  - fixed typo in max rate to be divided (400KHz > 400MHz)
>
> Changes from v3:
>  - fixed the clock divider calculation if PLLD/D2 is used
>  - removed unnecessary pre-configuration
>  - set correct video FIFO depth for DSI
>
> Changes from v2:
>  - fixed image distortion on devices with 180deg rotation
>
> Changes from v1:
>  - reworked patchset entirely
>  - diverged DC configuration per-SOC
>  - consolidated dc headers from different SOC gen
>  - initial support of DC detection (tegra has 2 DC)
>  - added PLLD2 support, resets and powergating
>  - added T114+ MIPI calibration
>  - added DSI detection (tegra has 2 DSI) and resets
> ---
>
> Jonas Schwöbel (6):
>   video: tegra20: dc: fix printing of framebuffer address
>   video: tegra20: dc: enable backlight after DC is configured
>   video: tegra20: dc: clean framebuffer memory block
>   video: tegra20: dsi: remove pre-configuration
>   video: tegra20: dsi: set correct fifo depth
>   video: tegra20: dsi: use set_backlight for backlight only
>
> Svyatoslav Ryhel (12):
>   video: tegra20: dc: diverge DC per-SOC
>   video: tegra20: dc: fix image shift on rotated panels
>   video: tegra20: consolidate DC header
>   video: tegra20: dc: pass DC id to internal devices
>   video: tegra20: dc: add PLLD2 parent support
>   video: tegra20: dc: add reset support
>   video: tegra20: dc: add powergate
>   video: tegra20: dc: configure behavior if PLLD/D2 is used
>   video: tegra20: dc: parameterize V- and H-sync polarities
>   video: tegra20: add MIPI calibration driver
>   video: tegra20: dsi: add T114 support
>   video: tegra20: dsi: add reset support
>
>  arch/arm/dts/tegra114-u-boot.dtsi             |  13 +
>  arch/arm/dts/tegra114.dtsi                    |   4 +-
>  arch/arm/dts/tegra30-u-boot.dtsi              |   4 +
>  arch/arm/dts/tegra30.dtsi                     |   2 +-
>  arch/arm/include/asm/arch-tegra/dc.h          |  13 +-
>  arch/arm/include/asm/arch-tegra114/pwm.h      |  13 +
>  arch/arm/include/asm/arch-tegra20/display.h   |  28 --
>  arch/arm/include/asm/arch-tegra30/display.h   |  28 --
>  drivers/video/tegra20/Makefile                |   2 +-
>  drivers/video/tegra20/tegra-dc.c              | 239 +++++++++++++-----
>  drivers/video/tegra20/tegra-dc.h              |  45 ++++
>  drivers/video/tegra20/tegra-dsi.c             | 122 ++++++++-
>  .../video/tegra20/tegra-dsi.h                 |  24 +-
>  drivers/video/tegra20/tegra-mipi.c            | 188 ++++++++++++++
>  drivers/video/tegra20/tegra-pwm-backlight.c   |   3 +-
>  15 files changed, 583 insertions(+), 145 deletions(-)
>  create mode 100644 arch/arm/include/asm/arch-tegra114/pwm.h
>  delete mode 100644 arch/arm/include/asm/arch-tegra20/display.h
>  delete mode 100644 arch/arm/include/asm/arch-tegra30/display.h
>  create mode 100644 drivers/video/tegra20/tegra-dc.h
>  rename arch/arm/include/asm/arch-tegra30/dsi.h => drivers/video/tegra20/tegra-dsi.h (90%)
>  create mode 100644 drivers/video/tegra20/tegra-mipi.c
>
> --
> 2.40.1
>

Re: [PATCH v6 00/18] Add T114 video support

[Anatolij Gustschin]

On Tue, 23 Jan 2024 19:16:15 +0200
Svyatoslav Ryhel clamor95@gmail.com wrote:

> T114 is not that different from T30 and all T30 drivers will work
> on T114 as well with some adjustments.
> 
> Patches propose general improvements for existing Tegra DC and DSI
> drivers as well Tegra 114 video support (experimentl).
> 
> Commits pass buildman for tegra.

Series applied to u-boot-video, except patch 06/18.

--
Anatolij