[v1] drm/bridge: samsung-dsim: Stop controlling vsync display FIFO flush in panels

[PATCH RFC 0/4] drm/bridge: samsung-dsim: Stop controlling vsync display FIFO flush in panels

Posted by Philipp Zabel 10 months, 3 weeks ago

This series enables the vsync flush feature in the samsung-dsim driver
unconditionally and removes the MIPI_DSI_MODE_VSYNC_FLUSH flag.

Background: I've recently seen shifted display issues on two different
i.MX8MM boards (mxsfb + samsung-dsim) with different DSI panels.
The symptoms were horizonally shifted display contents, with a stable
offset, in about 0.1 to 0.6 percent of modesets.
Enabling the MIPI_DSI_MODE_VSYNC_FLUSH flag in the panels' mode_flags
fixed the issue in both cases.

The samsung-dsim driver is the only DSI bridge driver that uses this
flag: If the flag is absent, the driver sets the DSIM_MFLUSH_VS bit in
the DSIM_CONFIG_REG register, which disables the vsync flush feature.
The reset value of this bit is cleared (vsync flush is default-enabled).
According to the i.MX8MM reference manual,

    "It needs that Main display FIFO should be flushed for deleting
     garbage data."

This appears to match the comment in mxsfb_reset_block() in mxsfb_kms.c:

    /*
     * It seems, you can't re-program the controller if it is still
     * running. This may lead to shifted pictures (FIFO issue?), so
     * first stop the controller and drain its FIFOs.
     */

Now I wonder why the bit is controlled by a flag in the panel drivers.
Whether the display controller pushes up to a FIFO worth of garbage data
into the DSI bridge during initialization seems to be a property of the
display controller / DSI bridge integration (whether this is due to
hardware or driver bugs), not a specific requirement of the panel.
Surely no panel needs to receive a partial line of garbage data in front
of the first frame?

Instead of adding the flag to every panel connected to affected SoCs,
the vsync flush feature could just be enabled unconditionally.
Clearing an already-empty display FIFO should have no effect, unless
I'm missing something? With that, the MIPI_DSI_MODE_VSYNC_FLUSH flag
would not be used anymore and could be removed.

regards
Philipp

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Philipp Zabel (4):
      drm/bridge: samsung-dsim: Always flush display FIFO on vsync pulse
      drm/panel: samsung-s6d7aa0: Drop MIPI_DSI_MODE_VSYNC_FLUSH flag
      drm/panel: samsung-s6e8aa0: Drop MIPI_DSI_MODE_VSYNC_FLUSH flag
      drm/mipi-dsi: Drop MIPI_DSI_MODE_VSYNC_FLUSH flag

 drivers/gpu/drm/bridge/samsung-dsim.c         | 2 --
 drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c | 2 +-
 drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c | 2 +-
 include/drm/drm_mipi_dsi.h                    | 2 --
 4 files changed, 2 insertions(+), 6 deletions(-)
---
base-commit: 99764593528f9e0ee9509f9e4a4eb21db99d0681
change-id: 20250527-dsi-vsync-flush-8f8cc91a6f04

Best regards,
-- 
Philipp Zabel <p.zabel@pengutronix.de>

Re: [PATCH RFC 0/4] drm/bridge: samsung-dsim: Stop controlling vsync display FIFO flush in panels

Posted by Marek Szyprowski 10 months, 3 weeks ago

On 27.05.2025 16:14, Philipp Zabel wrote:
> This series enables the vsync flush feature in the samsung-dsim driver
> unconditionally and removes the MIPI_DSI_MODE_VSYNC_FLUSH flag.
>
> Background: I've recently seen shifted display issues on two different
> i.MX8MM boards (mxsfb + samsung-dsim) with different DSI panels.
> The symptoms were horizonally shifted display contents, with a stable
> offset, in about 0.1 to 0.6 percent of modesets.
> Enabling the MIPI_DSI_MODE_VSYNC_FLUSH flag in the panels' mode_flags
> fixed the issue in both cases.
>
> The samsung-dsim driver is the only DSI bridge driver that uses this
> flag: If the flag is absent, the driver sets the DSIM_MFLUSH_VS bit in
> the DSIM_CONFIG_REG register, which disables the vsync flush feature.
> The reset value of this bit is cleared (vsync flush is default-enabled).
> According to the i.MX8MM reference manual,
>
>      "It needs that Main display FIFO should be flushed for deleting
>       garbage data."
>
> This appears to match the comment in mxsfb_reset_block() in mxsfb_kms.c:
>
>      /*
>       * It seems, you can't re-program the controller if it is still
>       * running. This may lead to shifted pictures (FIFO issue?), so
>       * first stop the controller and drain its FIFOs.
>       */
>
> Now I wonder why the bit is controlled by a flag in the panel drivers.
> Whether the display controller pushes up to a FIFO worth of garbage data
> into the DSI bridge during initialization seems to be a property of the
> display controller / DSI bridge integration (whether this is due to
> hardware or driver bugs), not a specific requirement of the panel.
> Surely no panel needs to receive a partial line of garbage data in front
> of the first frame?
>
> Instead of adding the flag to every panel connected to affected SoCs,
> the vsync flush feature could just be enabled unconditionally.
> Clearing an already-empty display FIFO should have no effect, unless
> I'm missing something? With that, the MIPI_DSI_MODE_VSYNC_FLUSH flag
> would not be used anymore and could be removed.

The Exynos5433 datasheet doesn't give us anything more about this bit:

"Auto flush of display FIFO in only video mode.
It requires that the display FIFO should be flushed for
deleting garbage data in video mode."

Your reasoning seems to be correct, it probably slipped into 
MIPI_DSI_MODE flags just because it is in the same register.

Feel free to add:

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland