Re: [PATCH v4 00/17] drm/bridge: cdns-dsi: Make it work a bit better

[Jayesh Choudhary <j-choudhary@ti.com>]

Hello Tomi,

On 18/06/25 15:29, Tomi Valkeinen wrote:
> While trying to get the cdns-dsi to work on Toradex's AM69 Aquila
> platform, I hit multiple issues in the driver. Basicaly nothing worked
> for with the board.
> 
> This series fixes those issues. While I itch to make much larger changes
> to the cdns-dsi driver, I opted to keep this series relatively simple to
> make the fixes more clear and possibly help with backporting.
> 
> The series also touches tidss, but those changes are not strictly
> needed, and can be merged separately. And the series also touches
> cdns-dphy, and those changes are needed.
> 
> This has been tested on Toradex AM69 Aquila (upstream) and AM62P Verdin
> (Toradex's BSP), with:
> - HDMI output using lontium lt8912b
> - LVDS panel (sn65dsi84 + panel-lvds)
> 
>   Tomi
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
> Changes in v4:
> - Rebased on top of drm-misc-next, which has most of the dependencies
>    merged
> - Moved one dependency, "drm/bridge: cdns-dsi: Fix the _atomic_check()"
>    into this series
> - Dropped "drm/tidss: Adjust the pclk based on the HW capabilities".
>    This causes a regression with OLDI outputs, and is not strictly
>    required. Fixing this needs restructuring tidss clock handling.

Here upon further investigating the issue for OLDI, that caused this
patch to be dropped, I saw that it was due to the VP clock owned by
OLDI driver. It does not actually go into the determine_rate() call
like we would expect it to.

Tested on J784S4-EVM platform (along with the DSI support as posted in
https://lore.kernel.org/all/20250624082619.324851-1-j-choudhary@ti.com/)
I can see that display comes up for 800x600 and 1280x1024 resolution.


Tested-by: Jayesh Choudhary <j-choudhary@ti.com>


I observed that we still need something like drm_mode_set_crtcinfo()
to propagate correct crtc_* fields even when we do not call round_rate()
Since mode_fixup() is not preferred now, we need it in atomic_check()
like you had it in v3:[3/17]

I have posted a delta patch on top of this series:
https://lore.kernel.org/all/20250624080402.302526-1-j-choudhary@ti.com/
that doe sit and helps in further enabling other modes.

Warm Regards,
Jayesh



> - Link to v3: https://lore.kernel.org/r/20250414-cdns-dsi-impro-v3-0-4e52551d4f07@ideasonboard.com
> 
> Changes in v3:
> - Add Aradhya's "drm/bridge: cdns-dsi: Fix the _atomic_check()" to the
>    dependencies
> - The above patch from Aradhya allowed adding "drm/bridge: cdns-dsi:
>    Drop crtc_* code", which resulted in quite large changes in the
>    commits, even if the end result doesn't really differ.
> - Reordered commits to decrease back-and-forth (e.g. fixing something in
>    a a code that will be removed in the next commits)
> - The reordering caused quite big changes in the commits (even if the
>    final end result is more or less the same), so I chose not to add
>    tested-by tags.
> - Rename 'cdns_get_dphy_pll_cfg' to 'cdns_dphy_get_pll_cfg'
> - Use div_u64() instead of div64_u64()
> - Drop "Fail if HS rate changed when validating PHY config". This was
>    too strict, as clock rounding (especially with DRM's 1kHz
>    resolution...) leads to clock rates that do not match exactly.
>    However, the rate mismatch should be fine as the commits adjust the
>    pixel clock, and the resulting differences should be so small that we
>    can't even improve the timings match by adjusting the DSI HFP, as the
>    adjustment rounds to 0.
> - Link to v2: https://lore.kernel.org/r/20250402-cdns-dsi-impro-v2-0-4a093eaa5e27@ideasonboard.com
> 
> Changes in v2:
> - Change the tidss clock adjustment from mode_fixup() to atomic_check()
> - Link to v1: https://lore.kernel.org/r/20250320-cdns-dsi-impro-v1-0-725277c5f43b@ideasonboard.com
> 
> ---
> Aradhya Bhatia (1):
>        drm/bridge: cdns-dsi: Fix the _atomic_check()
> 
> Tomi Valkeinen (16):
>        drm/tidss: Fix missing includes and struct decls
>        drm/tidss: Use the crtc_* timings when programming the HW
>        phy: cdns-dphy: Store hs_clk_rate and return it
>        phy: cdns-dphy: Remove leftover code
>        drm/bridge: cdns-dsi: Remove extra line at the end of the file
>        drm/bridge: cdns-dsi: Drop crtc_* code
>        drm/bridge: cdns-dsi: Remove broken fifo emptying check
>        drm/bridge: cdns-dsi: Drop checks that shouldn't be in .mode_valid()
>        drm/bridge: cdns-dsi: Update htotal in cdns_dsi_mode2cfg()
>        drm/bridge: cdns-dsi: Drop cdns_dsi_adjust_phy_config()
>        drm/bridge: cdns-dsi: Adjust mode to negative syncs
>        drm/bridge: cdns-dsi: Fix REG_WAKEUP_TIME value
>        drm/bridge: cdns-dsi: Use video mode and clean up cdns_dsi_mode2cfg()
>        drm/bridge: cdns-dsi: Fix event mode
>        drm/bridge: cdns-dsi: Tune adjusted_mode->clock according to dsi needs
>        drm/bridge: cdns-dsi: Don't fail on MIPI_DSI_MODE_VIDEO_BURST
> 
>   drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 211 +++++++++++--------------
>   drivers/gpu/drm/tidss/tidss_crtc.c             |   2 +-
>   drivers/gpu/drm/tidss/tidss_dispc.c            |  16 +-
>   drivers/gpu/drm/tidss/tidss_dispc.h            |   3 +
>   drivers/gpu/drm/tidss/tidss_drv.h              |   2 +
>   drivers/gpu/drm/tidss/tidss_plane.h            |   2 +
>   drivers/gpu/drm/tidss/tidss_scale_coefs.h      |   2 +
>   drivers/phy/cadence/cdns-dphy.c                |  24 ++-
>   8 files changed, 115 insertions(+), 147 deletions(-)
> ---
> base-commit: 261a603062a87bad0b54904c76dacb15fa126c74
> change-id: 20250320-cdns-dsi-impro-3d8fbd7848d1
> 
> Best regards,