| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3a73aada-48ad-45ad-8144-b58760113d6f@suse.de>
Date: Tue, 2 Sep 2025 16:06:45 +0200
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Maxime Ripard <mripard@...nel.org>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Andrzej Hajda <andrzej.hajda@...el.com>,
Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>,
Jyri Sarha <jyri.sarha@....fi>,
Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Cc: Devarsh Thakkar <devarsht@...com>, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 24/29] drm/tidss: dispc: Improve mode checking logs
Am 02.09.25 um 10:32 schrieb Maxime Ripard:
> The dispc_vp_mode_valid() function checks whether a mode can be handled
> by the display controller.
>
> There's a whole bunch of criteria, and it's not clear when a rejection
> happens why it did. Let's add a bunch of logs on error to make it
> clearer.
>
> Signed-off-by: Maxime Ripard <mripard@...nel.org>
> ---
> drivers/gpu/drm/tidss/tidss_dispc.c | 47 +++++++++++++++++++++++++++++--------
> 1 file changed, 37 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
> index 32248b5f71b7566dc33d7a7db0efb26d3a9ed1c3..ef948e3041e10bc65cf2c4794a4e4cffa7e3fb3a 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
> @@ -1349,47 +1349,63 @@ static void dispc_vp_set_default_color(struct dispc_device *dispc,
>
> enum drm_mode_status dispc_vp_mode_valid(struct dispc_device *dispc,
> u32 hw_videoport,
> const struct drm_display_mode *mode)
> {
> + struct tidss_device *tidss = dispc->tidss;
> + struct drm_device *dev = &tidss->ddev;
> u32 hsw, hfp, hbp, vsw, vfp, vbp;
> enum dispc_vp_bus_type bus_type;
> int max_pclk;
>
> bus_type = dispc->feat->vp_bus_type[hw_videoport];
>
> max_pclk = dispc->feat->max_pclk_khz[bus_type];
>
> - if (WARN_ON(max_pclk == 0))
> + if (WARN_ON(max_pclk == 0)) {
Better remove this WARN_ON(). User-space could trigger it and spam the
kernel logs. It's also a driver bug, I think.
> + drm_dbg(dev, "Invalid maximum pixel clock");
> return MODE_BAD;
> + }
>
> - if (mode->clock < dispc->feat->min_pclk_khz)
> + if (mode->clock < dispc->feat->min_pclk_khz) {
> + drm_dbg(dev, "Mode pixel clock below hardware minimum pixel clock");
> return MODE_CLOCK_LOW;
> + }
>
> - if (mode->clock > max_pclk)
> + if (mode->clock > max_pclk) {
> + drm_dbg(dev, "Mode pixel clock above hardware maximum pixel clock");
> return MODE_CLOCK_HIGH;
> + }
>
> - if (mode->hdisplay > 4096)
> + if (mode->hdisplay > 4096) {
> + drm_dbg(dev, "Number of active horizontal pixels above hardware limits.");
> return MODE_BAD;
> + }
>
> - if (mode->vdisplay > 4096)
> + if (mode->vdisplay > 4096) {
> + drm_dbg(dev, "Number of active vertical lines above hardware limits.");
> return MODE_BAD;
> + }
>
> /* TODO: add interlace support */
> - if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> + if (mode->flags & DRM_MODE_FLAG_INTERLACE) {
> + drm_dbg(dev, "Interlace modes not suppported.");
> return MODE_NO_INTERLACE;
> + }
>
> /*
> * Enforce the output width is divisible by 2. Actually this
> * is only needed in following cases:
> * - YUV output selected (BT656, BT1120)
> * - Dithering enabled
> * - TDM with TDMCycleFormat == 3
> * But for simplicity we enforce that always.
> */
> - if ((mode->hdisplay % 2) != 0)
> + if ((mode->hdisplay % 2) != 0) {
> + drm_dbg(dev, "Number of active horizontal pixels must be even.");
> return MODE_BAD_HVALUE;
> + }
>
> hfp = mode->hsync_start - mode->hdisplay;
> hsw = mode->hsync_end - mode->hsync_start;
> hbp = mode->htotal - mode->hsync_end;
>
> @@ -1397,29 +1413,40 @@ enum drm_mode_status dispc_vp_mode_valid(struct dispc_device *dispc,
> vsw = mode->vsync_end - mode->vsync_start;
> vbp = mode->vtotal - mode->vsync_end;
>
> if (hsw < 1 || hsw > 256 ||
> hfp < 1 || hfp > 4096 ||
> - hbp < 1 || hbp > 4096)
> + hbp < 1 || hbp > 4096) {
> + drm_dbg(dev,
> + "Horizontal blanking or sync outside of hardware limits (fp: %u, sw: %u, bp: %u).",
> + hfp, hsw, hbp);
> return MODE_BAD_HVALUE;
> + }
>
> if (vsw < 1 || vsw > 256 ||
> - vfp > 4095 || vbp > 4095)
> + vfp > 4095 || vbp > 4095) {
> + drm_dbg(dev,
> + "Vertical blanking or sync outside of hardware limits (fp: %u, sw: %u, bp: %u).",
> + vfp, vsw, vbp);
> return MODE_BAD_VVALUE;
> + }
>
> if (dispc->memory_bandwidth_limit) {
> const unsigned int bpp = 4;
> u64 bandwidth;
>
> bandwidth = 1000 * mode->clock;
> bandwidth = bandwidth * mode->hdisplay * mode->vdisplay * bpp;
> bandwidth = div_u64(bandwidth, mode->htotal * mode->vtotal);
>
> - if (dispc->memory_bandwidth_limit < bandwidth)
> + if (dispc->memory_bandwidth_limit < bandwidth) {
> + drm_dbg(dev, "Required memory bandwidth outside of hardware limits.");
> return MODE_BAD;
> + }
> }
>
> + drm_dbg(dev, "Mode is valid.");
> return MODE_OK;
> }
>
> int dispc_vp_enable_clk(struct dispc_device *dispc, u32 hw_videoport)
> {
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Powered by blists - more mailing lists