Re: [PATCH 0/5] drm/bridge: samsung-dsim: fix various modes with ADV7535 bridge

From: Adam Ford
Date: Mon Aug 28 2023 - 12:14:23 EST


On Mon, Aug 28, 2023 at 10:59 AM Michael Tretter
<m.tretter@xxxxxxxxxxxxxx> wrote:
>
> I tested the i.MX8M Nano EVK with the NXP supplied MIPI-DSI adapter,
> which uses an ADV7535 MIPI-DSI to HDMI converter. I found that a few
> modes were working, but in many modes my monitor stayed dark.
>
> This series fixes the Samsung DSIM bridge driver to bring up a few more
> modes:
>
> The driver read the rate of the PLL ref clock only during probe.
> However, if the clock is re-parented to the VIDEO_PLL, changes to the
> pixel clock have an effect on the PLL ref clock. Therefore, the driver
> must read and potentially update the PLL ref clock on every modeset.
>
> I also found that the rounding mode of the porches and active area has
> an effect on the working modes. If the driver rounds up instead of
> rounding down and be calculates them in Hz instead of kHz, more modes
> start to work.
>
> The following table shows the modes that were working in my test without
> this patch set and the modes that are working now:
>
> | Mode | Before | Now |
> | 1920x1080-60.00 | X | X |
> | 1920x1080-59.94 | | X |
> | 1920x1080-50.00 | | X |
> | 1920x1080-30.00 | | X |
> | 1920x1080-29.97 | | X |
> | 1920x1080-25.00 | | X |
> | 1920x1080-24.00 | | |
> | 1920x1080-23.98 | | |
> | 1680x1050-59.88 | | X |
> | 1280x1024-75.03 | X | X |
> | 1280x1024-60.02 | X | X |
> | 1200x960-59.99 | | X |
> | 1152x864-75.00 | X | X |
> | 1280x720-60.00 | | |
> | 1280x720-59.94 | | |
> | 1280x720-50.00 | | X |
> | 1024x768-75.03 | | X |
> | 1024x768-60.00 | | X |
> | 800x600-75.00 | X | X |
> | 800x600-60.32 | X | X |
> | 720x576-50.00 | X | X |
> | 720x480-60.00 | | |
> | 720x480-59.94 | X | |
> | 640x480-75.00 | X | X |
> | 640x480-60.00 | | X |
> | 640x480-59.94 | | X |
> | 720x400-70.08 | | |
>

Nice!

> Interestingly, the 720x480-59.94 mode stopped working. However, I am
> able to bring up the 720x480 modes by manually hacking the active area
> (hsa) to 40 and carefully adjusting the clocks, but something still
> seems to be off.

Checkout my

branch: https://github.com/aford173/linux/commit/183cf6d154afeb9b0300500b09d7b8ec53047a12

I found that certain resolutions don't properly round, and it seemed
to be related to the size of HFP. HFP of 110 when divded between 4
lanes, required a round-up, but then I had to recalculate the rest of
the horizontal timings to compensate.

I was going to push that as an RFC, but I will investigate your patch
series first. With my small rounding correction, I am able to get
720x480 and 720p on my imx8mp, but not my mini/nano, so I am hoping
your patch series fixes that issue for me.

>
> Unfortunately, a few more modes are still not working at all. The NXP
> downstream kernel has some quirks to handle some of the modes especially
> wrt. to the porches, but I cannot figure out, what the driver should
> actually do in these cases. Maybe there is still an error in the
> calculation of the porches and someone at NXP can chime in.

Hopefully the comment in my above patch explains how the calculation
is corrected and adjusted to get some of the missing resolutions.

> Michael
>
> Signed-off-by: Michael Tretter <m.tretter@xxxxxxxxxxxxxx>

I'll try to reivew this week and respond with my feedback.

adam

> ---
> Marco Felsch (1):
> drm/bridge: samsung-dsim: add more mipi-dsi device debug information
>
> Michael Tretter (4):
> drm/bridge: samsung-dsim: reread ref clock before configuring PLL
> drm/bridge: samsung-dsim: update PLL reference clock
> drm/bridge: samsung-dsim: adjust porches by rounding up
> drm/bridge: samsung-dsim: calculate porches in Hz
>
> drivers/gpu/drm/bridge/samsung-dsim.c | 42 +++++++++++++++++++++++++----------
> include/drm/bridge/samsung-dsim.h | 1 +
> 2 files changed, 31 insertions(+), 12 deletions(-)
> ---
> base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
> change-id: 20230818-samsung-dsim-42346444bce5
>
> Best regards,
> --
> Michael Tretter <m.tretter@xxxxxxxxxxxxxx>
>