Re: [PATCH v4 4/7] drm/tegra: dc: Expand available registers layouts

[Mikko Perttunen <mperttunen@nvidia.com>]

On Thursday, October 22, 2026 10:22 PM Svyatoslav Ryhel wrote:
> Expand existing DC register definitions with additional fields in
> preparation for adding the 8-bit CPU interface.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/gpu/drm/tegra/dc.c |  3 ++-
>  drivers/gpu/drm/tegra/dc.h | 49 +++++++++++++++++++++++++++++++++-----
>  2 files changed, 45 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index b0bfa946e6979..5c67928bcabfa 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -2384,7 +2384,8 @@ static void tegra_crtc_atomic_enable(struct drm_crtc *crtc,
>  
>  	if (dc->rgb) {
>  		/* XXX: parameterize? */
> -		value = SC0_H_QUALIFIER_NONE | SC1_H_QUALIFIER_NONE;
> +		value = SC0_H_QUALIFIER(SC_H_QUALIFIER_NONE) |
> +			SC1_H_QUALIFIER(SC_H_QUALIFIER_NONE);
>  		tegra_dc_writel(dc, value, DC_DISP_SHIFT_CLOCK_OPTIONS);
>  	}
>  
> diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
> index 0cb0515968b35..7005df32f485f 100644
> --- a/drivers/gpu/drm/tegra/dc.h
> +++ b/drivers/gpu/drm/tegra/dc.h
> @@ -274,8 +274,11 @@ int tegra_dc_rgb_exit(struct tegra_dc *dc);
>  #define DC_COM_CRC_CHECKSUM			0x301
>  #define DC_COM_PIN_OUTPUT_ENABLE(x) (0x302 + (x))
>  #define DC_COM_PIN_OUTPUT_POLARITY(x) (0x306 + (x))
> +#define LSC0_OUTPUT_POLARITY_LOW BIT(24)
>  #define LVS_OUTPUT_POLARITY_LOW (1 << 28)
>  #define LHS_OUTPUT_POLARITY_LOW (1 << 30)
> +#define LSPI_OUTPUT_POLARITY_LOW BIT(8)

Please update the existing defines to use BIT(n) as well, and align the 
define values (incl. the lines around here that are already there) like 
elsewhere in the file. Also, currently it's hard to tell which register 
each bit define corresponds to, so I'd like some comments like ..

/* DC_COM_PIN_OUTPUT_POLARITY(0): */
#define ...
#define ...

> +#define LDC_OUTPUT_SELECT_V_PULSE1 BIT(14)

Please move this below the DC_COM_PIN_OUTPUT_SELECT(x) define.

>  #define DC_COM_PIN_OUTPUT_DATA(x) (0x30a + (x))
>  #define DC_COM_PIN_INPUT_ENABLE(x) (0x30e + (x))
>  #define DC_COM_PIN_INPUT_DATA(x) (0x312 + (x))
> @@ -303,9 +306,15 @@ int tegra_dc_rgb_exit(struct tegra_dc *dc);
>  #define  UNDERFLOW_REPORT_ENABLE (1 << 0)
>  
>  #define DC_DISP_DISP_SIGNAL_OPTIONS0		0x400
> -#define H_PULSE0_ENABLE (1 <<  8)
> -#define H_PULSE1_ENABLE (1 << 10)
> -#define H_PULSE2_ENABLE (1 << 12)
> +#define H_PULSE0_ENABLE				BIT(8)
> +#define H_PULSE1_ENABLE				BIT(10)
> +#define H_PULSE2_ENABLE				BIT(12)
> +#define V_PULSE0_ENABLE				BIT(16)
> +#define V_PULSE1_ENABLE				BIT(18)
> +#define V_PULSE2_ENABLE				BIT(19)
> +#define V_PULSE3_ENABLE				BIT(20)
> +#define M0_ENABLE				BIT(24)
> +#define M1_ENABLE				BIT(26)
>  
>  #define DC_DISP_DISP_SIGNAL_OPTIONS1		0x401
>  
> @@ -451,12 +460,26 @@ int tegra_dc_rgb_exit(struct tegra_dc *dc);
>  #define BASE_COLOR_SIZE_888    (  8 << 0)
>  #define BASE_COLOR_SIZE_101010 ( 10 << 0)
>  #define BASE_COLOR_SIZE_121212 ( 12 << 0)
> +#define DISP_COLOR_SWAP_BGR	BIT(16)
>  #define CMU_ENABLE_ENABLE      (1 << 20)
>  
>  #define DC_DISP_SHIFT_CLOCK_OPTIONS		0x431
> -#define  SC1_H_QUALIFIER_NONE	(1 << 16)
> -#define  SC0_H_QUALIFIER_NONE	(1 <<  0)
> -
> +#define SC0_H_QUALIFIER(x)			(((x) & 0x7) <<  0)
> +#define SC1_H_QUALIFIER(x)			(((x) & 0x7) << 16)
> +#define SC_H_QUALIFIER_DISABLE			0
> +#define SC_H_QUALIFIER_NONE			1
> +#define SC_H_QUALIFIER_HACTIVE			2
> +#define SC_H_QUALIFIER_EXT_HACTIVE		3
> +#define SC_H_QUALIFIER_HPULSE			4
> +#define SC_H_QUALIFIER_EXT_HPULSE		5
> +#define SC0_V_QUALIFIER(x)			(((x) & 0x7) <<  3)
> +#define SC1_V_QUALIFIER(x)			(((x) & 0x7) <<  19)
> +#define SC_V_QUALIFIER_NONE			0
> +#define SC_V_QUALIFIER_RSVD			1
> +#define SC_V_QUALIFIER_VACTIVE			2
> +#define SC_V_QUALIFIER_EXT_VACTIVE		3
> +#define SC_V_QUALIFIER_VPULSE			4
> +#define SC_V_QUALIFIER_EXT_VPULSE		5
>  #define DC_DISP_DATA_ENABLE_OPTIONS		0x432
>  #define DE_SELECT_ACTIVE_BLANK  (0 << 0)
>  #define DE_SELECT_ACTIVE        (1 << 0)
> @@ -493,6 +516,20 @@ int tegra_dc_rgb_exit(struct tegra_dc *dc);
>  #define DC_DISP_CURSOR_POSITION_NS		0x441
>  
>  #define DC_DISP_INIT_SEQ_CONTROL		0x442
> +#define SEND_INIT_SEQUENCE			BIT(0)
> +#define INIT_SEQUENCE_MODE_SPI			BIT(1)
> +#define INIT_SEQUENCE_MODE_PLCD			0x0
> +#define INIT_SEQ_DC_SIGNAL_SHIFT		4
> +#define INIT_SEQ_DC_SIGNAL_MASK			(0x7 << INIT_SEQ_DC_SIGNAL_SHIFT)
> +#define NO_DC_SIGNAL				0
> +#define DC_SIGNAL_VSYNC				1
> +#define DC_SIGNAL_VPULSE0			2
> +#define DC_SIGNAL_VPULSE1			3
> +#define DC_SIGNAL_VPULSE2			4
> +#define DC_SIGNAL_VPULSE3			5
> +#define INIT_SEQ_DC_CONTROL_SHIFT		7
> +#define FRAME_INIT_SEQ_CYCLES_SHIFT		8
> +
>  #define DC_DISP_SPI_INIT_SEQ_DATA_A		0x443
>  #define DC_DISP_SPI_INIT_SEQ_DATA_B		0x444
>  #define DC_DISP_SPI_INIT_SEQ_DATA_C		0x445
> -- 
> 2.53.0
> 
>