* [PATCH] drm/bridge: nwl-dsi: Use vsync/hsync polarity from display mode @ 2024-08-14 10:37 Esben Haabendal 2024-09-13 7:55 ` Neil Armstrong 2025-01-20 12:42 ` Alexander Stein 0 siblings, 2 replies; 4+ messages in thread From: Esben Haabendal @ 2024-08-14 10:37 UTC (permalink / raw) To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter Cc: dri-devel, linux-kernel, Esben Haabendal Using the correct bit helps. The documentation specifies bit 0 in both registers to be controlling polarity of dpi_vsync_input and dpi_hsync_input polarity. Bit 1 is reserved, and should therefore not be set. Tested with panel that requires active high vsync and hsync. Signed-off-by: Esben Haabendal <esben@geanix.com> --- drivers/gpu/drm/bridge/nwl-dsi.c | 8 ++++---- drivers/gpu/drm/bridge/nwl-dsi.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c index 8d54091ec66e..5f05647a3bea 100644 --- a/drivers/gpu/drm/bridge/nwl-dsi.c +++ b/drivers/gpu/drm/bridge/nwl-dsi.c @@ -289,13 +289,13 @@ static int nwl_dsi_config_dpi(struct nwl_dsi *dsi) nwl_dsi_write(dsi, NWL_DSI_INTERFACE_COLOR_CODING, NWL_DSI_DPI_24_BIT); nwl_dsi_write(dsi, NWL_DSI_PIXEL_FORMAT, color_format); - /* - * Adjusting input polarity based on the video mode results in - * a black screen so always pick active low: - */ nwl_dsi_write(dsi, NWL_DSI_VSYNC_POLARITY, + dsi->mode.flags & DRM_MODE_FLAG_PVSYNC ? + NWL_DSI_VSYNC_POLARITY_ACTIVE_HIGH : NWL_DSI_VSYNC_POLARITY_ACTIVE_LOW); nwl_dsi_write(dsi, NWL_DSI_HSYNC_POLARITY, + dsi->mode.flags & DRM_MODE_FLAG_PHSYNC ? + NWL_DSI_HSYNC_POLARITY_ACTIVE_HIGH : NWL_DSI_HSYNC_POLARITY_ACTIVE_LOW); burst_mode = (dsi->dsi_mode_flags & MIPI_DSI_MODE_VIDEO_BURST) && diff --git a/drivers/gpu/drm/bridge/nwl-dsi.h b/drivers/gpu/drm/bridge/nwl-dsi.h index a247a8a11c7c..61e7d65cb1eb 100644 --- a/drivers/gpu/drm/bridge/nwl-dsi.h +++ b/drivers/gpu/drm/bridge/nwl-dsi.h @@ -30,11 +30,11 @@ #define NWL_DSI_PIXEL_FORMAT 0x20c #define NWL_DSI_VSYNC_POLARITY 0x210 #define NWL_DSI_VSYNC_POLARITY_ACTIVE_LOW 0 -#define NWL_DSI_VSYNC_POLARITY_ACTIVE_HIGH BIT(1) +#define NWL_DSI_VSYNC_POLARITY_ACTIVE_HIGH BIT(0) #define NWL_DSI_HSYNC_POLARITY 0x214 #define NWL_DSI_HSYNC_POLARITY_ACTIVE_LOW 0 -#define NWL_DSI_HSYNC_POLARITY_ACTIVE_HIGH BIT(1) +#define NWL_DSI_HSYNC_POLARITY_ACTIVE_HIGH BIT(0) #define NWL_DSI_VIDEO_MODE 0x218 #define NWL_DSI_HFP 0x21c --- base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba change-id: 20240814-nwl-dsi-sync-polarity-ddc58435a4c4 Best regards, -- Esben Haabendal <esben@geanix.com> ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/bridge: nwl-dsi: Use vsync/hsync polarity from display mode 2024-08-14 10:37 [PATCH] drm/bridge: nwl-dsi: Use vsync/hsync polarity from display mode Esben Haabendal @ 2024-09-13 7:55 ` Neil Armstrong 2025-01-20 12:42 ` Alexander Stein 1 sibling, 0 replies; 4+ messages in thread From: Neil Armstrong @ 2024-09-13 7:55 UTC (permalink / raw) To: Esben Haabendal, Andrzej Hajda, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter Cc: dri-devel, linux-kernel On 14/08/2024 12:37, Esben Haabendal wrote: > Using the correct bit helps. The documentation specifies bit 0 in both > registers to be controlling polarity of dpi_vsync_input and > dpi_hsync_input polarity. Bit 1 is reserved, and should therefore not be > set. > > Tested with panel that requires active high vsync and hsync. > > Signed-off-by: Esben Haabendal <esben@geanix.com> > --- > drivers/gpu/drm/bridge/nwl-dsi.c | 8 ++++---- > drivers/gpu/drm/bridge/nwl-dsi.h | 4 ++-- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c > index 8d54091ec66e..5f05647a3bea 100644 > --- a/drivers/gpu/drm/bridge/nwl-dsi.c > +++ b/drivers/gpu/drm/bridge/nwl-dsi.c > @@ -289,13 +289,13 @@ static int nwl_dsi_config_dpi(struct nwl_dsi *dsi) > > nwl_dsi_write(dsi, NWL_DSI_INTERFACE_COLOR_CODING, NWL_DSI_DPI_24_BIT); > nwl_dsi_write(dsi, NWL_DSI_PIXEL_FORMAT, color_format); > - /* > - * Adjusting input polarity based on the video mode results in > - * a black screen so always pick active low: > - */ > nwl_dsi_write(dsi, NWL_DSI_VSYNC_POLARITY, > + dsi->mode.flags & DRM_MODE_FLAG_PVSYNC ? > + NWL_DSI_VSYNC_POLARITY_ACTIVE_HIGH : > NWL_DSI_VSYNC_POLARITY_ACTIVE_LOW); > nwl_dsi_write(dsi, NWL_DSI_HSYNC_POLARITY, > + dsi->mode.flags & DRM_MODE_FLAG_PHSYNC ? > + NWL_DSI_HSYNC_POLARITY_ACTIVE_HIGH : > NWL_DSI_HSYNC_POLARITY_ACTIVE_LOW); > > burst_mode = (dsi->dsi_mode_flags & MIPI_DSI_MODE_VIDEO_BURST) && > diff --git a/drivers/gpu/drm/bridge/nwl-dsi.h b/drivers/gpu/drm/bridge/nwl-dsi.h > index a247a8a11c7c..61e7d65cb1eb 100644 > --- a/drivers/gpu/drm/bridge/nwl-dsi.h > +++ b/drivers/gpu/drm/bridge/nwl-dsi.h > @@ -30,11 +30,11 @@ > #define NWL_DSI_PIXEL_FORMAT 0x20c > #define NWL_DSI_VSYNC_POLARITY 0x210 > #define NWL_DSI_VSYNC_POLARITY_ACTIVE_LOW 0 > -#define NWL_DSI_VSYNC_POLARITY_ACTIVE_HIGH BIT(1) > +#define NWL_DSI_VSYNC_POLARITY_ACTIVE_HIGH BIT(0) > > #define NWL_DSI_HSYNC_POLARITY 0x214 > #define NWL_DSI_HSYNC_POLARITY_ACTIVE_LOW 0 > -#define NWL_DSI_HSYNC_POLARITY_ACTIVE_HIGH BIT(1) > +#define NWL_DSI_HSYNC_POLARITY_ACTIVE_HIGH BIT(0) > > #define NWL_DSI_VIDEO_MODE 0x218 > #define NWL_DSI_HFP 0x21c > > --- > base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba > change-id: 20240814-nwl-dsi-sync-polarity-ddc58435a4c4 > > Best regards, Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: drm/bridge: nwl-dsi: Use vsync/hsync polarity from display mode 2024-08-14 10:37 [PATCH] drm/bridge: nwl-dsi: Use vsync/hsync polarity from display mode Esben Haabendal 2024-09-13 7:55 ` Neil Armstrong @ 2025-01-20 12:42 ` Alexander Stein 2025-01-22 7:15 ` Esben Haabendal 1 sibling, 1 reply; 4+ messages in thread From: Alexander Stein @ 2025-01-20 12:42 UTC (permalink / raw) To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, Esben Haabendal Cc: dri-devel, linux-kernel, Esben Haabendal Hi, I'm sorry I'm late to the party. Am Mittwoch, 14. August 2024, 12:37:26 CET schrieb Esben Haabendal: > Using the correct bit helps. The documentation specifies bit 0 in both > registers to be controlling polarity of dpi_vsync_input and > dpi_hsync_input polarity. Bit 1 is reserved, and should therefore not be > set. > > Tested with panel that requires active high vsync and hsync. > > Signed-off-by: Esben Haabendal <esben@geanix.com> > Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> I just noticed this commit causes a regression on my platform TQMa8Mx/MBa8Mx. DT overlay: arch/arm64/boot/dts/freescale/imx8mq-tqma8mq-mba8mx-lvds-tm070jvhg33.dtso My bridges are configured as follow: > $ cat /sys/kernel/debug/dri/30320000.lcd-controller/encoder-0/bridges > bridge[0]: nwl_dsi_bridge_funcs [nwl_dsi] > > type: [0] Unknown > OF: /soc@0/bus@30800000/dsi@30a00000:fsl,imx8mq-nwl-dsi > ops: [0x0] > > bridge[1]: sn65dsi83_funcs [ti_sn65dsi83] > > type: [0] Unknown > OF: /soc@0/bus@30800000/i2c@30a40000/bridge@2d:ti,sn65dsi84 > ops: [0x0] > > bridge[2]: panel_bridge_bridge_funcs > > type: [7] LVDS > OF: /panel-lvds:tianma,tm070jvhg33 > ops: [0x8] modes The display needs active-low sync signals, otherwise the image is shifted by the amount of sync pulse. The patch itself looks good. But there is also nwl_dsi_bridge_atomic_check() unconditionally enabling DRM_MODE_FLAG_PHSYNC and DRM_MODE_FLAG_PVSYNC. Reverting the patch immediately restores the display image correctly. Best regards, Alexander > --- > drivers/gpu/drm/bridge/nwl-dsi.c | 8 ++++---- > drivers/gpu/drm/bridge/nwl-dsi.h | 4 ++-- > 2 files changed, 6 insertions(+), 6 deletions(-) > > > --- > base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba > change-id: 20240814-nwl-dsi-sync-polarity-ddc58435a4c4 > > Best regards, > > diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c > index 8d54091ec66e..5f05647a3bea 100644 > --- a/drivers/gpu/drm/bridge/nwl-dsi.c > +++ b/drivers/gpu/drm/bridge/nwl-dsi.c > @@ -289,13 +289,13 @@ static int nwl_dsi_config_dpi(struct nwl_dsi *dsi) > > nwl_dsi_write(dsi, NWL_DSI_INTERFACE_COLOR_CODING, NWL_DSI_DPI_24_BIT); > nwl_dsi_write(dsi, NWL_DSI_PIXEL_FORMAT, color_format); > - /* > - * Adjusting input polarity based on the video mode results in > - * a black screen so always pick active low: > - */ > nwl_dsi_write(dsi, NWL_DSI_VSYNC_POLARITY, > + dsi->mode.flags & DRM_MODE_FLAG_PVSYNC ? > + NWL_DSI_VSYNC_POLARITY_ACTIVE_HIGH : > NWL_DSI_VSYNC_POLARITY_ACTIVE_LOW); > nwl_dsi_write(dsi, NWL_DSI_HSYNC_POLARITY, > + dsi->mode.flags & DRM_MODE_FLAG_PHSYNC ? > + NWL_DSI_HSYNC_POLARITY_ACTIVE_HIGH : > NWL_DSI_HSYNC_POLARITY_ACTIVE_LOW); > > burst_mode = (dsi->dsi_mode_flags & MIPI_DSI_MODE_VIDEO_BURST) && > diff --git a/drivers/gpu/drm/bridge/nwl-dsi.h b/drivers/gpu/drm/bridge/nwl-dsi.h > index a247a8a11c7c..61e7d65cb1eb 100644 > --- a/drivers/gpu/drm/bridge/nwl-dsi.h > +++ b/drivers/gpu/drm/bridge/nwl-dsi.h > @@ -30,11 +30,11 @@ > #define NWL_DSI_PIXEL_FORMAT 0x20c > #define NWL_DSI_VSYNC_POLARITY 0x210 > #define NWL_DSI_VSYNC_POLARITY_ACTIVE_LOW 0 > -#define NWL_DSI_VSYNC_POLARITY_ACTIVE_HIGH BIT(1) > +#define NWL_DSI_VSYNC_POLARITY_ACTIVE_HIGH BIT(0) > > #define NWL_DSI_HSYNC_POLARITY 0x214 > #define NWL_DSI_HSYNC_POLARITY_ACTIVE_LOW 0 > -#define NWL_DSI_HSYNC_POLARITY_ACTIVE_HIGH BIT(1) > +#define NWL_DSI_HSYNC_POLARITY_ACTIVE_HIGH BIT(0) > > #define NWL_DSI_VIDEO_MODE 0x218 > #define NWL_DSI_HFP 0x21c > -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider http://www.tq-group.com/ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: drm/bridge: nwl-dsi: Use vsync/hsync polarity from display mode 2025-01-20 12:42 ` Alexander Stein @ 2025-01-22 7:15 ` Esben Haabendal 0 siblings, 0 replies; 4+ messages in thread From: Esben Haabendal @ 2025-01-22 7:15 UTC (permalink / raw) To: Alexander Stein Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, dri-devel, linux-kernel Alexander Stein <alexander.stein@ew.tq-group.com> writes: > Hi, > > I'm sorry I'm late to the party. > > Am Mittwoch, 14. August 2024, 12:37:26 CET schrieb Esben Haabendal: >> Using the correct bit helps. The documentation specifies bit 0 in both >> registers to be controlling polarity of dpi_vsync_input and >> dpi_hsync_input polarity. Bit 1 is reserved, and should therefore not be >> set. >> >> Tested with panel that requires active high vsync and hsync. >> >> Signed-off-by: Esben Haabendal <esben@geanix.com> >> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> > > I just noticed this commit causes a regression on my platform TQMa8Mx/MBa8Mx. > DT overlay: arch/arm64/boot/dts/freescale/imx8mq-tqma8mq-mba8mx-lvds-tm070jvhg33.dtso > My bridges are configured as follow: >> $ cat /sys/kernel/debug/dri/30320000.lcd-controller/encoder-0/bridges >> bridge[0]: nwl_dsi_bridge_funcs [nwl_dsi] >> >> type: [0] Unknown >> OF: /soc@0/bus@30800000/dsi@30a00000:fsl,imx8mq-nwl-dsi >> ops: [0x0] >> >> bridge[1]: sn65dsi83_funcs [ti_sn65dsi83] >> >> type: [0] Unknown >> OF: /soc@0/bus@30800000/i2c@30a40000/bridge@2d:ti,sn65dsi84 >> ops: [0x0] >> >> bridge[2]: panel_bridge_bridge_funcs >> >> type: [7] LVDS >> OF: /panel-lvds:tianma,tm070jvhg33 >> ops: [0x8] modes > > The display needs active-low sync signals, otherwise the image is shifted > by the amount of sync pulse. > The patch itself looks good. But there is also nwl_dsi_bridge_atomic_check() > unconditionally enabling DRM_MODE_FLAG_PHSYNC and DRM_MODE_FLAG_PVSYNC. Yes, the code you mention does look quite suspicious to me. /* At least LCDIF + NWL needs active high sync */ adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC); adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC) Why would we want to unconditionally enable active high sync signals in .atomic_check()? It is perfectly valid to have active-low sync signals, which your case perfectly proves. Could we simply drop this, and thus require that the sync signals are properly configured? > Reverting the patch immediately restores the display image correctly. And breaks it in other cases :( We need a way to be able to select the sync signal polarity. /Esben > > Best regards, > Alexander >> --- >> drivers/gpu/drm/bridge/nwl-dsi.c | 8 ++++---- >> drivers/gpu/drm/bridge/nwl-dsi.h | 4 ++-- >> 2 files changed, 6 insertions(+), 6 deletions(-) >> >> >> --- >> base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba >> change-id: 20240814-nwl-dsi-sync-polarity-ddc58435a4c4 >> >> Best regards, >> >> diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c >> index 8d54091ec66e..5f05647a3bea 100644 >> --- a/drivers/gpu/drm/bridge/nwl-dsi.c >> +++ b/drivers/gpu/drm/bridge/nwl-dsi.c >> @@ -289,13 +289,13 @@ static int nwl_dsi_config_dpi(struct nwl_dsi *dsi) >> >> nwl_dsi_write(dsi, NWL_DSI_INTERFACE_COLOR_CODING, NWL_DSI_DPI_24_BIT); >> nwl_dsi_write(dsi, NWL_DSI_PIXEL_FORMAT, color_format); >> - /* >> - * Adjusting input polarity based on the video mode results in >> - * a black screen so always pick active low: >> - */ >> nwl_dsi_write(dsi, NWL_DSI_VSYNC_POLARITY, >> + dsi->mode.flags & DRM_MODE_FLAG_PVSYNC ? >> + NWL_DSI_VSYNC_POLARITY_ACTIVE_HIGH : >> NWL_DSI_VSYNC_POLARITY_ACTIVE_LOW); >> nwl_dsi_write(dsi, NWL_DSI_HSYNC_POLARITY, >> + dsi->mode.flags & DRM_MODE_FLAG_PHSYNC ? >> + NWL_DSI_HSYNC_POLARITY_ACTIVE_HIGH : >> NWL_DSI_HSYNC_POLARITY_ACTIVE_LOW); >> >> burst_mode = (dsi->dsi_mode_flags & MIPI_DSI_MODE_VIDEO_BURST) && >> diff --git a/drivers/gpu/drm/bridge/nwl-dsi.h b/drivers/gpu/drm/bridge/nwl-dsi.h >> index a247a8a11c7c..61e7d65cb1eb 100644 >> --- a/drivers/gpu/drm/bridge/nwl-dsi.h >> +++ b/drivers/gpu/drm/bridge/nwl-dsi.h >> @@ -30,11 +30,11 @@ >> #define NWL_DSI_PIXEL_FORMAT 0x20c >> #define NWL_DSI_VSYNC_POLARITY 0x210 >> #define NWL_DSI_VSYNC_POLARITY_ACTIVE_LOW 0 >> -#define NWL_DSI_VSYNC_POLARITY_ACTIVE_HIGH BIT(1) >> +#define NWL_DSI_VSYNC_POLARITY_ACTIVE_HIGH BIT(0) >> >> #define NWL_DSI_HSYNC_POLARITY 0x214 >> #define NWL_DSI_HSYNC_POLARITY_ACTIVE_LOW 0 >> -#define NWL_DSI_HSYNC_POLARITY_ACTIVE_HIGH BIT(1) >> +#define NWL_DSI_HSYNC_POLARITY_ACTIVE_HIGH BIT(0) >> >> #define NWL_DSI_VIDEO_MODE 0x218 >> #define NWL_DSI_HFP 0x21c >> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-01-22 7:16 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-14 10:37 [PATCH] drm/bridge: nwl-dsi: Use vsync/hsync polarity from display mode Esben Haabendal 2024-09-13 7:55 ` Neil Armstrong 2025-01-20 12:42 ` Alexander Stein 2025-01-22 7:15 ` Esben Haabendal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox