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 12/18] drm/bridge: cdns-dsi: Drop checks that shouldn't be in .mode_valid()
Date: Thu, 20 Mar 2025 18:00:07 +0200 [thread overview]
Message-ID: <20250320-cdns-dsi-impro-v1-12-725277c5f43b@ideasonboard.com> (raw)
In-Reply-To: <20250320-cdns-dsi-impro-v1-0-725277c5f43b@ideasonboard.com>
The docs say about mode_valid():
"it is not allowed to look at anything else but the passed-in mode, and
validate it against configuration-invariant hardware constraints"
We're doing a lot more than just looking at the mode. The main issue
here is that we're doing checks based on the pixel clock, before we know
what the pixel clock from the crtc actually is.
So, drop the checks from .mode_valid(). This also allows us to remove
the 'mode_valid_check' parameter from internal functions, and the
related code.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 44 ++++++++------------------
1 file changed, 14 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
index e85c8652c96e..cf783680b1b4 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
@@ -469,25 +469,17 @@ static unsigned int dpi_to_dsi_timing(unsigned int dpi_timing,
static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
const struct drm_display_mode *mode,
- struct cdns_dsi_cfg *dsi_cfg,
- bool mode_valid_check)
+ struct cdns_dsi_cfg *dsi_cfg)
{
struct cdns_dsi_output *output = &dsi->output;
u32 dpi_hsa, dpi_hbp, dpi_hfp, dpi_hact;
bool sync_pulse;
int bpp;
- if (mode_valid_check) {
- dpi_hsa = mode->hsync_end - mode->hsync_start;
- dpi_hbp = mode->htotal - mode->hsync_end;
- dpi_hfp = mode->hsync_start - mode->hdisplay;
- dpi_hact = mode->hdisplay;
- } else {
- dpi_hsa = mode->crtc_hsync_end - mode->crtc_hsync_start;
- dpi_hbp = mode->crtc_htotal - mode->crtc_hsync_end;
- dpi_hfp = mode->crtc_hsync_start - mode->crtc_hdisplay;
- dpi_hact = mode->crtc_hdisplay;
- }
+ dpi_hsa = mode->crtc_hsync_end - mode->crtc_hsync_start;
+ dpi_hbp = mode->crtc_htotal - mode->crtc_hsync_end;
+ dpi_hfp = mode->crtc_hsync_start - mode->crtc_hdisplay;
+ dpi_hact = mode->crtc_hdisplay;
memset(dsi_cfg, 0, sizeof(*dsi_cfg));
@@ -518,8 +510,7 @@ static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
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 drm_display_mode *mode,
- bool mode_valid_check)
+ const struct drm_display_mode *mode)
{
struct cdns_dsi_output *output = &dsi->output;
unsigned long long dlane_bps;
@@ -549,11 +540,11 @@ static int cdns_dsi_adjust_phy_config(struct cdns_dsi *dsi,
if (dsi_htotal % lanes)
adj_dsi_htotal += lanes - (dsi_htotal % lanes);
- dpi_hz = (mode_valid_check ? mode->clock : mode->crtc_clock) * 1000;
+ dpi_hz = mode->crtc_clock * 1000;
dlane_bps = (unsigned long long)dpi_hz * adj_dsi_htotal;
/* data rate in bytes/sec is not an integer, refuse the mode. */
- dpi_htotal = mode_valid_check ? mode->htotal : mode->crtc_htotal;
+ dpi_htotal = mode->crtc_htotal;
if (do_div(dlane_bps, lanes * dpi_htotal))
return -EINVAL;
@@ -569,27 +560,25 @@ static int cdns_dsi_adjust_phy_config(struct cdns_dsi *dsi,
static int cdns_dsi_check_conf(struct cdns_dsi *dsi,
const struct drm_display_mode *mode,
- struct cdns_dsi_cfg *dsi_cfg,
- bool mode_valid_check)
+ struct cdns_dsi_cfg *dsi_cfg)
{
struct cdns_dsi_output *output = &dsi->output;
struct phy_configure_opts_mipi_dphy *phy_cfg = &output->phy_opts.mipi_dphy;
unsigned int nlanes = output->dev->lanes;
- int mode_clock = (mode_valid_check ? mode->clock : mode->crtc_clock);
unsigned long req_hs_clk_rate;
int ret;
- ret = cdns_dsi_mode2cfg(dsi, mode, dsi_cfg, mode_valid_check);
+ ret = cdns_dsi_mode2cfg(dsi, mode, dsi_cfg);
if (ret)
return ret;
- ret = phy_mipi_dphy_get_default_config(mode_clock * 1000,
+ ret = phy_mipi_dphy_get_default_config(mode->crtc_clock * 1000,
mipi_dsi_pixel_format_to_bpp(output->dev->format),
nlanes, phy_cfg);
if (ret)
return ret;
- ret = cdns_dsi_adjust_phy_config(dsi, dsi_cfg, phy_cfg, mode, mode_valid_check);
+ ret = cdns_dsi_adjust_phy_config(dsi, dsi_cfg, phy_cfg, mode);
if (ret)
return ret;
@@ -635,8 +624,7 @@ cdns_dsi_bridge_mode_valid(struct drm_bridge *bridge,
struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge);
struct cdns_dsi *dsi = input_to_dsi(input);
struct cdns_dsi_output *output = &dsi->output;
- struct cdns_dsi_cfg dsi_cfg;
- int bpp, ret;
+ int bpp;
/*
* VFP_DSI should be less than VFP_DPI and VFP_DSI should be at
@@ -654,10 +642,6 @@ cdns_dsi_bridge_mode_valid(struct drm_bridge *bridge,
if ((mode->hdisplay * bpp) % 32)
return MODE_H_ILLEGAL;
- ret = cdns_dsi_check_conf(dsi, mode, &dsi_cfg, true);
- if (ret)
- return MODE_BAD;
-
return MODE_OK;
}
@@ -996,7 +980,7 @@ static int cdns_dsi_bridge_atomic_check(struct drm_bridge *bridge,
adjusted_crtc_mode->flags &= ~(DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC);
adjusted_crtc_mode->flags |= DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC;
- return cdns_dsi_check_conf(dsi, mode, dsi_cfg, false);
+ return cdns_dsi_check_conf(dsi, mode, dsi_cfg);
}
static struct drm_bridge_state *
--
2.43.0
next prev parent reply other threads:[~2025-03-20 16:00 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 ` Tomi Valkeinen [this message]
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 ` [PATCH 17/18] drm/bridge: cdns-dsi: Drop cdns_dsi_adjust_phy_config() Tomi Valkeinen
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-12-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).