Re: [PATCH V2 2/2] drm/bridge: samsung-dsim: Fix porch calcalcuation rounding

From: Marek Vasut
Date: Mon Apr 22 2024 - 15:48:46 EST


On 4/22/24 3:04 PM, Adam Ford wrote:
On Mon, Apr 22, 2024 at 8:01 AM Marek Vasut <marex@xxxxxxx> wrote:

On 4/22/24 2:09 PM, Adam Ford wrote:
On Sun, Apr 21, 2024 at 9:36 AM Marek Vasut <marex@xxxxxxx> wrote:

On 2/12/24 12:09 AM, Adam Ford wrote:
When using video sync pulses, the HFP, HBP, and HSA are divided between
the available lanes if there is more than one lane. For certain
timings and lane configurations, the HFP may not be evenly divisible.
If the HFP is rounded down, it ends up being too small which can cause
some monitors to not sync properly. In these instances, adjust htotal
and hsync to round the HFP up, and recalculate the htotal.

Tested-by: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx> # Kontron BL i.MX8MM with HDMI monitor
Signed-off-by: Adam Ford <aford173@xxxxxxxxx>
---
V2: No changes

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 8476650c477c..52939211fe93 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1606,6 +1606,27 @@ static int samsung_dsim_atomic_check(struct drm_bridge *bridge,
adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC);
}

+ /*
+ * When using video sync pulses, the HFP, HBP, and HSA are divided between
+ * the available lanes if there is more than one lane. For certain
+ * timings and lane configurations, the HFP may not be evenly divisible.
+ * If the HFP is rounded down, it ends up being too small which can cause
+ * some monitors to not sync properly. In these instances, adjust htotal
+ * and hsync to round the HFP up, and recalculate the htotal. Through trial
+ * and error, it appears that the HBP and HSA do not appearto need the same
+ * correction that HFP does.
+ */
+ if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE && dsi->lanes > 1) {

Does this also apply to mode with sync events (I suspect it does), so
the condition here should likely be if (!...burst mode) , right ?

Thanks for the review!

I was only able to test it with the DSI->ADV6535 bridge, and I'll
admit I don't know a lot about DSI interface since I don't have a copy
of the spec to read.

Are you proposing this should be:

if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) && dsi->lanes > 1) {

I just want to make sure I understand what you're requesting.

Yes, exactly this.

Do you think it should also include checks for
MIPI_DSI_MODE_VIDEO_NO_HFP, MIPI_DSI_MODE_VIDEO_NO_HBP or
MIPI_DSI_MODE_VIDEO_NO_HSA?

It seems like if any of these are set, we should skip this rounding stuff.

Now that you ask this question, shouldn't this actually apply unconditionally , no matter which mode is in use ?