[PATCH] drm/panel: nt35510: Make new commands optional

[Stefan Hansson]

Hi Linus!

On 2024-09-06 23:54, Linus Walleij wrote:
> The commit introducing the Frida display started to write the
> SETVCL, BT3CTR and SETVCMOFF registers unconditionally, and some
> (not all!) Hydis display seem to be affected by ghosting after
> the commit.
> 
> Make them all optional and only send these commands on the
> Frida display for now.
> 
> Reported-by: Stefan Hansson <newbyte at postmarketos.org>
> Fixes: 219a1f49094f ("drm/panel: nt35510: support FRIDA FRD400B25025-A-CTK")
> Signed-off-by: Linus Walleij <linus.walleij at linaro.org>
> ---
> Stefan: if this patch works and you have time you can try adding
> the three flags one at a time to your device and we can see which
> command is problematic.

I did just that.

I tried different combinations of the .cmds property of 
nt35510_hydis_hva40wv1 and came to these conclusions:

Good:

     .cmds = NT35510_CMD_CORRECT_GAMMA
     .cmds = NT35510_CMD_CORRECT_GAMMA | NT35510_CMD_SETVCL
     .cmds = NT35510_CMD_CORRECT_GAMMA | NT35510_CMD_SETVCL | 
NT35510_CMD_BT3CTR

Bad:

     .cmds = NT35510_CMD_CORRECT_GAMMA | NT35510_CMD_SETVCL | 
NT35510_CMD_BT3CTR | NT35510_CMD_SETVCMOFF
     .cmds = NT35510_CMD_CORRECT_GAMMA | NT35510_CMD_SETVCMOFF

By good, I mean "no visible ghosting" and by bad I mean "visible ghosting".

Based on this, it seems to me that NT35510_CMD_SETVCMOFF is the culprit. 
However, this patch is also good as-is.

> ---
>   drivers/gpu/drm/panel/panel-novatek-nt35510.c | 45 +++++++++++++++++----------
>   1 file changed, 29 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
> index d3bfdfc9cff6..e07409dcbd1d 100644
> --- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c
> +++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
> @@ -38,6 +38,9 @@
>   
>   #define NT35510_CMD_CORRECT_GAMMA BIT(0)
>   #define NT35510_CMD_CONTROL_DISPLAY BIT(1)
> +#define NT35510_CMD_SETVCL BIT(2)
> +#define NT35510_CMD_BT3CTR BIT(3)
> +#define NT35510_CMD_SETVCMOFF BIT(4)
>   
>   #define MCS_CMD_MAUCCTR		0xF0 /* Manufacturer command enable */
>   #define MCS_CMD_READ_ID1	0xDA
> @@ -675,16 +678,23 @@ static int nt35510_setup_power(struct nt35510 *nt)
>   				nt->conf->bt2ctr);
>   	if (ret)
>   		return ret;
> -	ret = nt35510_send_long(nt, dsi, NT35510_P1_SETVCL,
> -				NT35510_P1_VCL_LEN,
> -				nt->conf->vcl);
> -	if (ret)
> -		return ret;
> -	ret = nt35510_send_long(nt, dsi, NT35510_P1_BT3CTR,
> -				NT35510_P1_BT3CTR_LEN,
> -				nt->conf->bt3ctr);
> -	if (ret)
> -		return ret;
> +
> +	if (nt->conf->cmds & NT35510_CMD_SETVCL) {
> +		ret = nt35510_send_long(nt, dsi, NT35510_P1_SETVCL,
> +					NT35510_P1_VCL_LEN,
> +					nt->conf->vcl);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (nt->conf->cmds & NT35510_CMD_BT3CTR) {
> +		ret = nt35510_send_long(nt, dsi, NT35510_P1_BT3CTR,
> +					NT35510_P1_BT3CTR_LEN,
> +					nt->conf->bt3ctr);
> +		if (ret)
> +			return ret;
> +	}
> +
>   	ret = nt35510_send_long(nt, dsi, NT35510_P1_SETVGH,
>   				NT35510_P1_VGH_LEN,
>   				nt->conf->vgh);
> @@ -721,11 +731,13 @@ static int nt35510_setup_power(struct nt35510 *nt)
>   	if (ret)
>   		return ret;
>   
> -	ret = nt35510_send_long(nt, dsi, NT35510_P1_SETVCMOFF,
> -				NT35510_P1_VCMOFF_LEN,
> -				nt->conf->vcmoff);
> -	if (ret)
> -		return ret;
> +	if (nt->conf->cmds & NT35510_CMD_SETVCMOFF) {
> +		ret = nt35510_send_long(nt, dsi, NT35510_P1_SETVCMOFF,
> +					NT35510_P1_VCMOFF_LEN,
> +					nt->conf->vcmoff);
> +		if (ret)
> +			return ret;
> +	}
>   
>   	/* Typically 10 ms */
>   	usleep_range(10000, 20000);
> @@ -1319,7 +1331,8 @@ static const struct nt35510_config nt35510_frida_frd400b25025 = {
>   	},
>   	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
>   			MIPI_DSI_MODE_LPM,
> -	.cmds = NT35510_CMD_CONTROL_DISPLAY,
> +	.cmds = NT35510_CMD_CONTROL_DISPLAY | NT35510_CMD_SETVCL |
> +			NT35510_CMD_BT3CTR | NT35510_CMD_SETVCMOFF,
>   	/* 0x03: AVDD = 6.2V */
>   	.avdd = { 0x03, 0x03, 0x03 },
>   	/* 0x46: PCK = 2 x Hsync, BTP = 2.5 x VDDB */
> 
> ---
> base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
> change-id: 20240906-fix-nt35510-a8ec6e47e036
> 
> Best regards,

Stefan Hansson