[PATCH] drm/panel: sitronix-st7789v: fix sync flags for t28cp45tn89

[PATCH] drm/panel: sitronix-st7789v: fix sync flags for t28cp45tn89

[Sebastian Reichel]

From: Sebastian Reichel <sebastian.reichel@collabora.com>

I planned to set the polarity of horizontal and vertical sync, but
accidentally described vertical sync twice with different polarity
instead.

Note, that there is no functional change, because the driver only
makes use of DRM_MODE_FLAG_P[HV]SYNC to divert from the default
active-low polarity.

Reported-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Closes: https://lore.kernel.org/all/20250923132616.GH20765@pendragon.ideasonboard.com/
Fixes: a411558cc143 ("drm/panel: sitronix-st7789v: add Inanbo T28CP45TN89 support")
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
index 04d91929eedda092b966b8cffdef5b267748f190..dedf0a390a88dd45a8179e2d22e872128c87cfda 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -261,7 +261,7 @@ static const struct drm_display_mode t28cp45tn89_mode = {
 	.vtotal = 320 + 8 + 4 + 4,
 	.width_mm = 43,
 	.height_mm = 57,
-	.flags = DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC,
+	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC,
 };
 
 static const struct drm_display_mode et028013dma_mode = {

---
base-commit: 07e27ad16399afcd693be20211b0dfae63e0615f
change-id: 20250924-t28cp45tn89-fix-0931500ee88a

Best regards,
-- 
Sebastian Reichel <sebastian.reichel@collabora.com>

Re: [PATCH] drm/panel: sitronix-st7789v: fix sync flags for t28cp45tn89

[Laurent Pinchart]

On Wed, Sep 24, 2025 at 11:46:30PM +0200, Sebastian Reichel wrote:
> From: Sebastian Reichel <sebastian.reichel@collabora.com>
> 
> I planned to set the polarity of horizontal and vertical sync, but
> accidentally described vertical sync twice with different polarity
> instead.
> 
> Note, that there is no functional change, because the driver only
> makes use of DRM_MODE_FLAG_P[HV]SYNC to divert from the default
> active-low polarity.
> 
> Reported-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Closes: https://lore.kernel.org/all/20250923132616.GH20765@pendragon.ideasonboard.com/
> Fixes: a411558cc143 ("drm/panel: sitronix-st7789v: add Inanbo T28CP45TN89 support")
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
>  drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> index 04d91929eedda092b966b8cffdef5b267748f190..dedf0a390a88dd45a8179e2d22e872128c87cfda 100644
> --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> @@ -261,7 +261,7 @@ static const struct drm_display_mode t28cp45tn89_mode = {
>  	.vtotal = 320 + 8 + 4 + 4,
>  	.width_mm = 43,
>  	.height_mm = 57,
> -	.flags = DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC,
> +	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC,
>  };
>  
>  static const struct drm_display_mode et028013dma_mode = {
> 
> ---
> base-commit: 07e27ad16399afcd693be20211b0dfae63e0615f
> change-id: 20250924-t28cp45tn89-fix-0931500ee88a

-- 
Regards,

Laurent Pinchart

Re: [PATCH] drm/panel: sitronix-st7789v: fix sync flags for t28cp45tn89

[Marek Vasut]

On 9/24/25 11:46 PM, Sebastian Reichel wrote:
> From: Sebastian Reichel <sebastian.reichel@collabora.com>
> 
> I planned to set the polarity of horizontal and vertical sync, but
> accidentally described vertical sync twice with different polarity
> instead.
> 
> Note, that there is no functional change, because the driver only
> makes use of DRM_MODE_FLAG_P[HV]SYNC to divert from the default
> active-low polarity.
> 
> Reported-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Closes: https://lore.kernel.org/all/20250923132616.GH20765@pendragon.ideasonboard.com/
> Fixes: a411558cc143 ("drm/panel: sitronix-st7789v: add Inanbo T28CP45TN89 support")
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> index 04d91929eedda092b966b8cffdef5b267748f190..dedf0a390a88dd45a8179e2d22e872128c87cfda 100644
> --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> @@ -261,7 +261,7 @@ static const struct drm_display_mode t28cp45tn89_mode = {
>   	.vtotal = 320 + 8 + 4 + 4,
>   	.width_mm = 43,
>   	.height_mm = 57,
> -	.flags = DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC,
> +	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC,
Is this panel somehow special with its NHSYNC / PVSYNC , compared to the 
other supported panels, which all use PHSYNC / PVSYNC ? I would expect 
all of these DSI TCON to use the same polarity, how come this one needs 
NHSYNC ?

Re: [PATCH] drm/panel: sitronix-st7789v: fix sync flags for t28cp45tn89

[Sebastian Reichel]

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

Hello Marek,

On Thu, Sep 25, 2025 at 02:15:41AM +0200, Marek Vasut wrote:
> On 9/24/25 11:46 PM, Sebastian Reichel wrote:
> > From: Sebastian Reichel <sebastian.reichel@collabora.com>
> > 
> > I planned to set the polarity of horizontal and vertical sync, but
> > accidentally described vertical sync twice with different polarity
> > instead.
> > 
> > Note, that there is no functional change, because the driver only
> > makes use of DRM_MODE_FLAG_P[HV]SYNC to divert from the default
> > active-low polarity.
> > 
> > Reported-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > Closes: https://lore.kernel.org/all/20250923132616.GH20765@pendragon.ideasonboard.com/
> > Fixes: a411558cc143 ("drm/panel: sitronix-st7789v: add Inanbo T28CP45TN89 support")
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > ---
> >   drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > index 04d91929eedda092b966b8cffdef5b267748f190..dedf0a390a88dd45a8179e2d22e872128c87cfda 100644
> > --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > @@ -261,7 +261,7 @@ static const struct drm_display_mode t28cp45tn89_mode = {
> >   	.vtotal = 320 + 8 + 4 + 4,
> >   	.width_mm = 43,
> >   	.height_mm = 57,
> > -	.flags = DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC,
> > +	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC,
> Is this panel somehow special with its NHSYNC / PVSYNC , compared to the
> other supported panels, which all use PHSYNC / PVSYNC ? I would expect all
> of these DSI TCON to use the same polarity, how come this one needs NHSYNC ?

I wrote this based on reverse engineering incl. quite a bit try-and-error and
don't have very good data about the panel. Using DRM_MODE_FLAG_PHSYNC like all
the other panels results in garbage data on the display.

Greetings,

-- Sebastian

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

Re: [PATCH] drm/panel: sitronix-st7789v: fix sync flags for t28cp45tn89

[Marek Vasut]

On 9/25/25 2:48 PM, Sebastian Reichel wrote:

Hello Sebastian,

> On Thu, Sep 25, 2025 at 02:15:41AM +0200, Marek Vasut wrote:
>> On 9/24/25 11:46 PM, Sebastian Reichel wrote:
>>> From: Sebastian Reichel <sebastian.reichel@collabora.com>
>>>
>>> I planned to set the polarity of horizontal and vertical sync, but
>>> accidentally described vertical sync twice with different polarity
>>> instead.
>>>
>>> Note, that there is no functional change, because the driver only
>>> makes use of DRM_MODE_FLAG_P[HV]SYNC to divert from the default
>>> active-low polarity.
>>>
>>> Reported-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>>> Closes: https://lore.kernel.org/all/20250923132616.GH20765@pendragon.ideasonboard.com/
>>> Fixes: a411558cc143 ("drm/panel: sitronix-st7789v: add Inanbo T28CP45TN89 support")
>>> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
>>> ---
>>>    drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
>>> index 04d91929eedda092b966b8cffdef5b267748f190..dedf0a390a88dd45a8179e2d22e872128c87cfda 100644
>>> --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
>>> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
>>> @@ -261,7 +261,7 @@ static const struct drm_display_mode t28cp45tn89_mode = {
>>>    	.vtotal = 320 + 8 + 4 + 4,
>>>    	.width_mm = 43,
>>>    	.height_mm = 57,
>>> -	.flags = DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC,
>>> +	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC,
>> Is this panel somehow special with its NHSYNC / PVSYNC , compared to the
>> other supported panels, which all use PHSYNC / PVSYNC ? I would expect all
>> of these DSI TCON to use the same polarity, how come this one needs NHSYNC ?
> 
> I wrote this based on reverse engineering incl. quite a bit try-and-error and
> don't have very good data about the panel. Using DRM_MODE_FLAG_PHSYNC like all
> the other panels results in garbage data on the display.
Interesting.

Can you please add that information as a code comment in V2 and also add 
to V2 my

Reviewed-by: Marek Vasut <marek.vasut+renesas@mailbox.org>

Thank you