From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f45.google.com (mail-wm1-f45.google.com [209.85.128.45]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3E5F21F0E40 for ; Fri, 10 Jan 2025 09:35:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.45 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736501749; cv=none; b=Fg7y1vUU7OB4rzGhLlS0Hp9NGinni/RUjMPSQ/kIw321FLpJ8RDEYaOwvpeo0+5VHSaddqHKAwphCgQzwFuoqrgh4uiMkyz9fhlGpnLkx3TNMjpdkowIOjiW4nU+GntlGeb7jnaRIMTz7g2Y47iWrqRS4N7n/mLAw0X395H0GkY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736501749; c=relaxed/simple; bh=og0eoepxO5PFfXiJyjj4BodQZH8R4r7XTi52Wq9FP0U=; h=Content-Type:Mime-Version:Subject:From:In-Reply-To:Date:Cc: Message-Id:References:To; b=P0BDrMRRtvBFTj+QqkfVBWcOZ/V5aojqs0oXDbrVFmSaPZ+gb95d6w7oHAIe59Awvr0luhjew+dr/Wi8qMOfEZlyWGXjzYXRsmlOp9KXnl/fwTw9b/gGQgNuiW6ggMjb5WWrQ4YM8eV77h0ITISecMVgrEBjIaW7SR17KIw/yrU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=kfPgTcWO; arc=none smtp.client-ip=209.85.128.45 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="kfPgTcWO" Received: by mail-wm1-f45.google.com with SMTP id 5b1f17b1804b1-436202dd730so13384265e9.2 for ; Fri, 10 Jan 2025 01:35:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736501745; x=1737106545; darn=vger.kernel.org; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=t9g79eEr03zcR3C0me3t/ZaJgbQfeT6rUS/5evYzFcY=; b=kfPgTcWO36dOg/3Zf/HnM3kLmVV5IzOi4kLkYdNJAwyps7KlocCfg2vuQcZN5icH7w Lw0/xXFkm2tDfyjO64oXmF5ztninHY49iMufE1M/axTtl2BplTmyrntwvi1nBRV9hz7O zr3AkMyzihhkVpQ7g8BdIzJlNYTanvbDFRJKIKawfkSdnSCf4HfMljvOL2biSnnWq+7C 5reZ33dxz/D7FkMdFZnBrd+CMtZJSSwCizmis9Ura9Uu8PAq5WsAoiYl9pGAiyZW4qTY XFiTvUJGVREyAwixKA232AwhngRsIIWEWk6nF2KTrKwrfbP2GCiXPY8Mdlli7VUJn0Js c7QA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736501745; x=1737106545; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=t9g79eEr03zcR3C0me3t/ZaJgbQfeT6rUS/5evYzFcY=; b=eW7iubWTTEpzWnTbhtlJ2tHcnTbKgR/7zpvV8c7qBLv6RbC0mG1iEU/hm+ivZb/F4S gFckabdZTm6T1un/xabmpjTgJr2uqS/vWNpCDkOBzNJuF25r8J+vzXRkJfeEE1Il1m78 wCKONcQ+aI61QxJ4VibmTctwWmcJEE6m1Q/q40WQ+W8R6q0Y6S2t0oCcuDRlQ+GhVCxA IQ32skB/2xU6/1NpuQ59MWXUnS6HDJFI+SrFrUrMOE6arzGcmnNNtHrrfuCSqQ5YreyR 82KSCBFRrti3st/Sh850C1bP2lrK661EzbbmD++XVt4PfsILZpEMGTwy5eJyjDylgdiw +0Fg== X-Forwarded-Encrypted: i=1; AJvYcCX2iw3nPYENoriNeuSWULdqIG2pWDs1d0WfeAWLbkeTORUSihTqCP3y7nPMVo6q3ZKO0pH7EPIQi3C7Wdo=@vger.kernel.org X-Gm-Message-State: AOJu0YwDMG5l4Te3rMOcmGWmkZuZBPkI3RAz7/R73aYhistnDXTmE+pO kt2ZP7g7YueN3O29C/kVJDuXN73bXm0PN5aQQ+DhRTX+hugFmDUZ X-Gm-Gg: ASbGncu4J4ErFwSJah2t/zMJI7DJfusPUSl7oihWR/UYP21cbUq72RvR1Aqq5Ro2KqT 72IUeWzA3SDsKPgWQls8pj6lLZqgA20jP5pPMpDoojmcqF9pybx/PxotGX6EwUewwKwy9oWp0kD 3qUpRdBQKsG5l6VO8mXRQwxWB1b1ZcRRsg0VIdLo0CttVYqZM38wgN0uYhl6VIvb0sAAQyRaux9 WDi0i3azgrgVq/Sk7sES36mcUObYiQ+4TMZidGY+YwJagCTuaFu2LcHua973ZNesPAXbgDZbNxu y2I= X-Google-Smtp-Source: AGHT+IHwl7RTDk7XCHgavjvPYzyw0+x62aQxSFel0BV13NUnPSIe70GA9HfWuDf9a/COfDkzjSeDxQ== X-Received: by 2002:a5d:6c66:0:b0:38a:4184:151a with SMTP id ffacd0b85a97d-38a87336d7emr8612113f8f.37.1736501745179; Fri, 10 Jan 2025 01:35:45 -0800 (PST) Received: from smtpclient.apple ([87.200.95.144]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38a8e38c990sm4084563f8f.56.2025.01.10.01.35.42 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 10 Jan 2025 01:35:44 -0800 (PST) Content-Type: text/plain; charset=utf-8 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.200.121\)) Subject: Re: [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations From: Christian Hewitt In-Reply-To: <20250110-flat-quantum-frigatebird-2432f1@houat> Date: Fri, 10 Jan 2025 13:35:30 +0400 Cc: Neil Armstrong , Maarten Lankhorst , Thomas Zimmermann , David Airlie , Simona Vetter , Kevin Hilman , Jerome Brunet , Martin Blumenstingl , Jernej Skrabec , dri-devel@lists.freedesktop.org, linux-amlogic@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Message-Id: <048FF805-BF7E-4D1B-B522-E783415057BD@gmail.com> References: <20250110074458.3624094-1-christianshewitt@gmail.com> <20250110074458.3624094-3-christianshewitt@gmail.com> <20250110-flat-quantum-frigatebird-2432f1@houat> To: Maxime Ripard X-Mailer: Apple Mail (2.3826.200.121) > On 10 Jan 2025, at 12:36=E2=80=AFpm, Maxime Ripard = wrote: >=20 > Hi, >=20 > On Fri, Jan 10, 2025 at 07:44:58AM +0000, Christian Hewitt wrote: >> Playing YUV420 @ 59.94 media causes HDMI output to lose sync >> with a fatal error reported: >>=20 >> [ 89.610280] Fatal Error, invalid HDMI vclk freq 593406 >>=20 >> In meson_encoder_hdmi_set_vclk the initial vclk_freq value is >> 593407 but YUV420 modes halve the value to 296703.5 and this >> is stored as int which loses precision by rounding down to >> 296703. The rounded value is later doubled to 593406 and then >> meson_encoder_hdmi_set_vclk sets an invalid vclk_freq value >> and the error triggers during meson_vlkc_setup validation. >>=20 >> Fix precision in meson_encoder_hdmi_set_vclk by switching to >> unsigned long long KHz values instead of int MHz. As values >> for phy_freq are now more accurate we also need to handle an >> additional match scenario in meson_vclk_setup. >>=20 >> Fixes: e5fab2ec9ca4 ("drm/meson: vclk: add support for YUV420 setup") >> Signed-off-by: Christian Hewitt >> --- >> drivers/gpu/drm/meson/meson_encoder_hdmi.c | 42 = +++++++++++----------- >> drivers/gpu/drm/meson/meson_vclk.c | 3 +- >> 2 files changed, 23 insertions(+), 22 deletions(-) >>=20 >> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c = b/drivers/gpu/drm/meson/meson_encoder_hdmi.c >> index 0593a1cde906..fa37cf975992 100644 >> --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c >> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c >> @@ -70,12 +70,12 @@ static void meson_encoder_hdmi_set_vclk(struct = meson_encoder_hdmi *encoder_hdmi, >> { >> struct meson_drm *priv =3D encoder_hdmi->priv; >> int vic =3D drm_match_cea_mode(mode); >> - unsigned int phy_freq; >> - unsigned int vclk_freq; >> - unsigned int venc_freq; >> - unsigned int hdmi_freq; >> + unsigned long long vclk_freq; >> + unsigned long long phy_freq; >> + unsigned long long venc_freq; >> + unsigned long long hdmi_freq; >>=20 >> - vclk_freq =3D mode->clock; >> + vclk_freq =3D mode->clock * 1000ULL; >=20 > You should be using drm_hdmi_compute_mode_clock() here Hello Maxime, If we=E2=80=99re talking a one-two line change than I=E2=80=99d like = pointers and guidance on how to go about that (offline to avoid group noise). I have only rudimentary c skills though, so anything that looks like replumbing a driver around drm_connector is way beyond my current drm code and general coding knowledge; I can only offer a fix not a proper improvement :( Christian > /* For 420, pixel clock is half unlike venc clock */ >> if (encoder_hdmi->output_bus_fmt =3D=3D MEDIA_BUS_FMT_UYYVYY8_0_5X24) >> @@ -85,8 +85,9 @@ static void meson_encoder_hdmi_set_vclk(struct = meson_encoder_hdmi *encoder_hdmi, >> phy_freq =3D vclk_freq * 10; >>=20 >> if (!vic) { >> - meson_vclk_setup(priv, MESON_VCLK_TARGET_DMT, phy_freq, >> - vclk_freq, vclk_freq, vclk_freq, false); >> + meson_vclk_setup(priv, MESON_VCLK_TARGET_DMT, phy_freq / 1000ULL, >> + vclk_freq / 1000ULL, vclk_freq / 1000ULL, >> + vclk_freq / 1000ULL, false); >> return; >> } >>=20 >> @@ -107,12 +108,9 @@ static void meson_encoder_hdmi_set_vclk(struct = meson_encoder_hdmi *encoder_hdmi, >> if (mode->flags & DRM_MODE_FLAG_DBLCLK) >> venc_freq /=3D 2; >>=20 >> - dev_dbg(priv->dev, "vclk:%d phy=3D%d venc=3D%d hdmi=3D%d = enci=3D%d\n", >> - phy_freq, vclk_freq, venc_freq, hdmi_freq, >> - priv->venc.hdmi_use_enci); >> - >> - meson_vclk_setup(priv, MESON_VCLK_TARGET_HDMI, phy_freq, vclk_freq, >> - venc_freq, hdmi_freq, priv->venc.hdmi_use_enci); >> + meson_vclk_setup(priv, MESON_VCLK_TARGET_HDMI, phy_freq / 1000ULL, >> + vclk_freq / 1000ULL, venc_freq / 1000ULL, hdmi_freq / 1000ULL, >> + priv->venc.hdmi_use_enci); >> } >>=20 >> static enum drm_mode_status meson_encoder_hdmi_mode_valid(struct = drm_bridge *bridge, >> @@ -122,10 +120,10 @@ static enum drm_mode_status = meson_encoder_hdmi_mode_valid(struct drm_bridge *bri >> struct meson_encoder_hdmi *encoder_hdmi =3D = bridge_to_meson_encoder_hdmi(bridge); >> struct meson_drm *priv =3D encoder_hdmi->priv; >> bool is_hdmi2_sink =3D display_info->hdmi.scdc.supported; >> - unsigned int phy_freq; >> - unsigned int vclk_freq; >> - unsigned int venc_freq; >> - unsigned int hdmi_freq; >> + unsigned long long vclk_freq; >> + unsigned long long phy_freq; >> + unsigned long long venc_freq; >> + unsigned long long hdmi_freq; >> int vic =3D drm_match_cea_mode(mode); >> enum drm_mode_status status; >>=20 >> @@ -149,7 +147,7 @@ static enum drm_mode_status = meson_encoder_hdmi_mode_valid(struct drm_bridge *bri >> } else if (!meson_venc_hdmi_supported_vic(vic)) >> return MODE_BAD; >>=20 >> - vclk_freq =3D mode->clock; >> + vclk_freq =3D mode->clock * 1000ULL; >=20 > And here too. >=20 > Maxime