Re: [PATCH 3/3] drm/bridge: ti-sn65dsi83: add test pattern generation support

[Louis Chauvet <louis.chauvet@bootlin.com>]

On 2/26/26 17:16, Luca Ceresoli wrote:
> Generation of a test pattern output is a useful tool for panel bringup and
> debugging, and very simple to support with this chip.
> 
> The value of REG_VID_CHA_ACTIVE_LINE_LENGTH_LOW needs to be divided by two
> for the test pattern to work in dual LVDS mode. While not clearly stated in
> the datasheet, this is needed according to the DSI Tuner [0] output. And
> some dual-LVDS panels refuse to show any picture without this division by
> two.
> 
> [0] https://www.ti.com/tool/DSI-TUNER
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>   drivers/gpu/drm/bridge/ti-sn65dsi83.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index 17a885244e1e..ddc8b5e1dd15 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -114,6 +114,7 @@
>   #define REG_VID_CHA_HORIZONTAL_FRONT_PORCH	0x38
>   #define REG_VID_CHA_VERTICAL_FRONT_PORCH	0x3a
>   #define REG_VID_CHA_TEST_PATTERN		0x3c
> +#define  REG_VID_CHA_TEST_PATTERN_EN		BIT(4)
>   /* IRQ registers */
>   #define REG_IRQ_GLOBAL				0xe0
>   #define  REG_IRQ_GLOBAL_IRQ_EN			BIT(0)
> @@ -134,6 +135,9 @@
>   #define  REG_IRQ_STAT_CHA_SOT_BIT_ERR		BIT(2)
>   #define  REG_IRQ_STAT_CHA_PLL_UNLOCK		BIT(0)
>   
> +static bool sn65dsi83_test_pattern;
> +module_param_named(test_pattern, sn65dsi83_test_pattern, bool, 0644);
> +
>   enum sn65dsi83_channel {
>   	CHANNEL_A,
>   	CHANNEL_B
> @@ -645,7 +649,11 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge,
>   			  REG_LVDS_LANE_CHB_LVDS_TERM : 0));
>   	regmap_write(ctx->regmap, REG_LVDS_CM, 0x00);
>   
> -	le16val = cpu_to_le16(mode->hdisplay);
> +	/*
> +	 * Active line length needs to be halved for test pattern
> +	 * generation in dual LVDS output.
> +	 */
> +	le16val = cpu_to_le16(mode->hdisplay / (sn65dsi83_test_pattern ? 2 : 1));

This will always half the line length, even for "mono" LVDS output no? 
 From the comment I expect something like

if (dual && test_pattern)
	val = hdisplay/2
else
	val = hdisplay

>   	regmap_bulk_write(ctx->regmap, REG_VID_CHA_ACTIVE_LINE_LENGTH_LOW,
>   			  &le16val, 2);
>   	le16val = cpu_to_le16(mode->vdisplay);
> @@ -668,7 +676,8 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge,
>   		     (mode->hsync_start - mode->hdisplay) / dual_factor);
>   	regmap_write(ctx->regmap, REG_VID_CHA_VERTICAL_FRONT_PORCH,
>   		     mode->vsync_start - mode->vdisplay);
> -	regmap_write(ctx->regmap, REG_VID_CHA_TEST_PATTERN, 0x00);
> +	regmap_write(ctx->regmap, REG_VID_CHA_TEST_PATTERN,
> +		     sn65dsi83_test_pattern ? REG_VID_CHA_TEST_PATTERN_EN : 0);
>   

(sn65dsi83_test_pattern is not "atomic", but you already saw it)

>   	/* Enable PLL */
>   	regmap_write(ctx->regmap, REG_RC_PLL_EN, REG_RC_PLL_EN_PLL_EN);
>