dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
To: Jyri Sarha <jyri.sarha@iki.fi>,
	 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	 Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	 David Airlie <airlied@gmail.com>,
	Simona Vetter <simona@ffwll.ch>,  Vinod Koul <vkoul@kernel.org>,
	Kishon Vijay Abraham I <kishon@kernel.org>,
	 Andrzej Hajda <andrzej.hajda@intel.com>,
	 Neil Armstrong <neil.armstrong@linaro.org>,
	Robert Foss <rfoss@kernel.org>,
	 Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	 Jonas Karlman <jonas@kwiboo.se>,
	Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	 linux-phy@lists.infradead.org,
	Francesco Dolcini <francesco@dolcini.it>,
	 Aradhya Bhatia <aradhya.bhatia@linux.dev>,
	 Devarsh Thakkar <devarsht@ti.com>,
	 Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Subject: [PATCH 17/18] drm/bridge: cdns-dsi: Drop cdns_dsi_adjust_phy_config()
Date: Thu, 20 Mar 2025 18:00:12 +0200	[thread overview]
Message-ID: <20250320-cdns-dsi-impro-v1-17-725277c5f43b@ideasonboard.com> (raw)
In-Reply-To: <20250320-cdns-dsi-impro-v1-0-725277c5f43b@ideasonboard.com>

cdns_dsi_adjust_phy_config() is called from
cdns_dsi_adjust_phy_config(), which is called from .atomic_check(). It
checks the DSI htotal and adjusts it to align on the DSI lane boundary
by changing hfp and then recalculating htotal and HS clock rate.

This has a few problems.

First is the fact that the whole thing is not needed: we do not need to
align on the lane boundary. The whole frame is sent in HS mode, and it
is fine if the line's last byte clock tick fills, say, only 2 of the 4
lanes. The next line will just continue from there. Assuming the
DSI timing values have been calculated to match the incoming DPI stream,
and the HS clock is compatible with the DPI pixel clock, the "uneven"
DSI lines will even out when multiple lines are being sent.

But we could do the align, aligning is not a problem as such. However,
adding more bytes to the hfp, as the function currently does, makes the
DSI line time longer, so the function then adjusts the HS clock rate.
This is where things fail: we don't know what rates we can get from the
HS clock, and at least in TI K3 SoC case the rates are quite coarsely
grained. Thus small adjustment to hfp will lead to a big change in HS
clock rate, and things break down.

We could do a loop here, adjusting hfp, adjusting clock, checking clock
rate, adjusting hfp again, etc., but considering that the whole
adjustment shouldn't be needed at all, it's easier to just remove the
function.

Something like this function should be added back later, when adding
burst mode support, but that's a bigger change and I don't think this
function would help that work in any way.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 48 --------------------------
 1 file changed, 48 deletions(-)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
index 2a272fd8ea3e..0bb55584cb44 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
@@ -517,50 +517,6 @@ static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
 	return 0;
 }
 
-static int cdns_dsi_adjust_phy_config(struct cdns_dsi *dsi,
-			      struct cdns_dsi_cfg *dsi_cfg,
-			      struct phy_configure_opts_mipi_dphy *phy_cfg,
-			      const struct videomode *vm)
-{
-	struct cdns_dsi_output *output = &dsi->output;
-	unsigned long long dlane_bps;
-	unsigned long adj_dsi_htotal;
-	unsigned long dsi_htotal;
-	unsigned long dpi_htotal;
-	unsigned long dpi_hz;
-	unsigned int dsi_hfp_ext;
-	unsigned int lanes = output->dev->lanes;
-
-	dsi_htotal = dsi_cfg->htotal;
-
-	/*
-	 * Make sure DSI htotal is aligned on a lane boundary when calculating
-	 * the expected data rate. This is done by extending HFP in case of
-	 * misalignment.
-	 */
-	adj_dsi_htotal = dsi_htotal;
-	if (dsi_htotal % lanes)
-		adj_dsi_htotal += lanes - (dsi_htotal % lanes);
-
-	dpi_hz = vm->pixelclock;
-	dlane_bps = (unsigned long long)dpi_hz * adj_dsi_htotal;
-
-	/* data rate in bytes/sec is not an integer, refuse the mode. */
-	dpi_htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
-		     vm->hsync_len;
-	if (do_div(dlane_bps, lanes * dpi_htotal))
-		return -EINVAL;
-
-	/* data rate was in bytes/sec, convert to bits/sec. */
-	phy_cfg->hs_clk_rate = dlane_bps * 8;
-
-	dsi_hfp_ext = adj_dsi_htotal - dsi_htotal;
-	dsi_cfg->hfp += dsi_hfp_ext;
-	dsi_cfg->htotal = dsi_htotal + dsi_hfp_ext;
-
-	return 0;
-}
-
 static int cdns_dsi_check_conf(struct cdns_dsi *dsi,
 			       const struct videomode *vm,
 			       struct cdns_dsi_cfg *dsi_cfg)
@@ -581,10 +537,6 @@ static int cdns_dsi_check_conf(struct cdns_dsi *dsi,
 	if (ret)
 		return ret;
 
-	ret = cdns_dsi_adjust_phy_config(dsi, dsi_cfg, phy_cfg, vm);
-	if (ret)
-		return ret;
-
 	req_hs_clk_rate = output->phy_opts.mipi_dphy.hs_clk_rate;
 	ret = phy_validate(dsi->dphy, PHY_MODE_MIPI_DPHY, 0, &output->phy_opts);
 	if (ret)

-- 
2.43.0


  parent reply	other threads:[~2025-03-20 16:01 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-20 15:59 [PATCH 00/18] drm/bridge: cdns-dsi: Make it work a bit better Tomi Valkeinen
2025-03-20 15:59 ` [PATCH 01/18] drm/tidss: Fix missing includes and struct decls Tomi Valkeinen
2025-03-20 15:59 ` [PATCH 02/18] drm/tidss: Use the crtc_* timings when programming the HW Tomi Valkeinen
2025-03-20 15:59 ` [PATCH 03/18] drm/tidss: Add mode_fixup to adjust the clock based on HW Tomi Valkeinen
2025-03-21 14:17   ` Maxime Ripard
2025-04-02 12:16     ` Tomi Valkeinen
2025-03-20 15:59 ` [PATCH 04/18] phy: cdns-dphy: Store hs_clk_rate and return it Tomi Valkeinen
2025-03-20 16:00 ` [PATCH 05/18] phy: cdns-dphy: Remove leftover code Tomi Valkeinen
2025-03-20 16:00 ` [PATCH 06/18] drm/bridge: cdns-dsi: Adjust mode to negative syncs Tomi Valkeinen
2025-03-20 16:00 ` [PATCH 07/18] drm/bridge: cdns-dsi: Fail if HS rate changed when validating PHY config Tomi Valkeinen
2025-03-20 16:00 ` [PATCH 08/18] drm/bridge: cdns-dsi: Clean up cdns_dsi_mode2cfg() Tomi Valkeinen
2025-03-20 16:00 ` [PATCH 09/18] drm/bridge: cdns-dsi: Fix REG_WAKEUP_TIME value Tomi Valkeinen
2025-03-20 16:00 ` [PATCH 10/18] drm/bridge: cdns-dsi: Fix event mode Tomi Valkeinen
2025-03-20 16:00 ` [PATCH 11/18] drm/bridge: cdns-dsi: Remove broken fifo emptying check Tomi Valkeinen
2025-03-20 16:00 ` [PATCH 12/18] drm/bridge: cdns-dsi: Drop checks that shouldn't be in .mode_valid() Tomi Valkeinen
2025-03-20 16:00 ` [PATCH 13/18] drm/bridge: cdns-dsi: Do not use crtc_* values Tomi Valkeinen
2025-03-20 16:00 ` [PATCH 14/18] drm/bridge: cdns-dsi: Use videomode internally Tomi Valkeinen
2025-03-20 16:00 ` [PATCH 15/18] drm/bridge: cdns-dsi: Tune adjusted_mode->clock according to dsi needs Tomi Valkeinen
2025-03-20 16:00 ` [PATCH 16/18] drm/bridge: cdns-dsi: Update htotal in cdns_dsi_mode2cfg() Tomi Valkeinen
2025-03-20 16:00 ` Tomi Valkeinen [this message]
2025-03-20 16:00 ` [PATCH 18/18] drm/bridge: cdns-dsi: Don't fail on MIPI_DSI_MODE_VIDEO_BURST Tomi Valkeinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250320-cdns-dsi-impro-v1-17-725277c5f43b@ideasonboard.com \
    --to=tomi.valkeinen@ideasonboard.com \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=airlied@gmail.com \
    --cc=andrzej.hajda@intel.com \
    --cc=aradhya.bhatia@linux.dev \
    --cc=devarsht@ti.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=francesco@dolcini.it \
    --cc=jernej.skrabec@gmail.com \
    --cc=jonas@kwiboo.se \
    --cc=jyri.sarha@iki.fi \
    --cc=kishon@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=neil.armstrong@linaro.org \
    --cc=rfoss@kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tzimmermann@suse.de \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).