2024-06-01 14:41:23

by Adam Ford

[permalink] [raw]
Subject: [PATCH V3 1/2] drm/bridge: samsung-dsim: Set P divider based on min/max of fin pll

The P divider should be set based on the min and max values of
the fin pll which may vary between different platforms.
These ranges are defined per platform, but hard-coded values
were used instead which resulted in a smaller range available
on the i.MX8M[MNP] than what was possible.

As noted by Frieder, there are descripencies between the reference
manuals of the Mini, Nano and Plus, so I reached out to my NXP
rep and got the following response regarding the varing notes
in the documentation.

"Yes it is definitely wrong, the one that is part of the NOTE in
MIPI_DPHY_M_PLLPMS register table against PMS_P, PMS_M and PMS_S is
not correct. I will report this to Doc team, the one customer should
be take into account is the Table 13-40 DPHY PLL Parameters and the
Note above."

With this patch, the clock rates now match the values used in NXP's
downstream kernel.

Fixes: 846307185f0f ("drm/bridge: samsung-dsim: update PLL reference clock")
Signed-off-by: Adam Ford <[email protected]>
Reviewed-by: Frieder Schrempf <[email protected]>
Tested-by: Frieder Schrempf <[email protected]>
Reviewed-by: Marek Vasut <[email protected]>
Tested-by: Marek Szyprowski <[email protected]>

---
V2: Only update the commit message to reflect why these values
were chosen. No code change present

V3: No Changes

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 95fedc68b0ae..8476650c477c 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -574,8 +574,8 @@ static unsigned long samsung_dsim_pll_find_pms(struct samsung_dsim *dsi,
u16 _m, best_m;
u8 _s, best_s;

- p_min = DIV_ROUND_UP(fin, (12 * MHZ));
- p_max = fin / (6 * MHZ);
+ p_min = DIV_ROUND_UP(fin, (driver_data->pll_fin_max * MHZ));
+ p_max = fin / (driver_data->pll_fin_min * MHZ);

for (_p = p_min; _p <= p_max; ++_p) {
for (_s = 0; _s <= 5; ++_s) {
--
2.43.0



2024-06-01 14:41:33

by Adam Ford

[permalink] [raw]
Subject: [PATCH V3 2/2] drm/bridge: samsung-dsim: Fix porch calcalcuation rounding

The VFP, 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.

This allows 720P-60 to operation on an i.MX8MP with a four-lane
configuration.

Tested-by: Frieder Schrempf <[email protected]> # Kontron BL i.MX8MM with HDMI monitor
Signed-off-by: Adam Ford <[email protected]>
---
V2: No changes
V3: Remove the MIPI_DSI_MODE_VIDEO_SYNC_PULSE conditional as
requested by Marek Vasut.
Removed the Tested-by from Szyprowski because I don't know
if removing the conditional will impact his boards.
Update commit message

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 8476650c477c..e7e53a9e42af 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->lanes > 1) {
+ int hfp = adjusted_mode->hsync_start - adjusted_mode->hdisplay;
+ int remainder = hfp % dsi->lanes;
+
+ if (remainder) {
+ adjusted_mode->hsync_start += remainder;
+ adjusted_mode->hsync_end += remainder;
+ adjusted_mode->htotal += remainder;
+ }
+ }
+
return 0;
}

--
2.43.0


2024-06-10 14:35:02

by Robert Foss

[permalink] [raw]
Subject: Re: [PATCH V3 1/2] drm/bridge: samsung-dsim: Set P divider based on min/max of fin pll

On Sat, 1 Jun 2024 09:41:01 -0500, Adam Ford wrote:
> The P divider should be set based on the min and max values of
> the fin pll which may vary between different platforms.
> These ranges are defined per platform, but hard-coded values
> were used instead which resulted in a smaller range available
> on the i.MX8M[MNP] than what was possible.
>
> As noted by Frieder, there are descripencies between the reference
> manuals of the Mini, Nano and Plus, so I reached out to my NXP
> rep and got the following response regarding the varing notes
> in the documentation.
>
> [...]

Applied, thanks!

[1/2] drm/bridge: samsung-dsim: Set P divider based on min/max of fin pll
(no commit info)
[2/2] drm/bridge: samsung-dsim: Fix porch calcalcuation rounding
(no commit info)



Rob