* [PATCH 0/2] drm/meson: vclk: revert and re-fix vclk calculations @ 2025-01-10 7:44 ` Christian Hewitt 0 siblings, 0 replies; 20+ messages in thread From: Christian Hewitt @ 2025-01-10 7:44 UTC (permalink / raw) To: Neil Armstrong, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel Patch 1 reverts a previous fix for loss of HDMI sync when playing YUV420 @ 59.94 media. The patch does resolve a calculation issue. It also makes all fractional rates invalid which is a bigger problem. Patch 2 provides a proper fix after I figured out the actual root cause of the original problem. This time the maths checks out. Apologies to the stable team as the original patch has been applied to a wide range of older and LTS kernels (as far as 5.10). Please let me know if combining the two patches into a single 'fix the previous fix' patch would be preferred? Christian Christian Hewitt (2): Revert "drm/meson: vclk: fix calculation of 59.94 fractional rates" drm/meson: vclk: fix precision in vclk calculations drivers/gpu/drm/meson/meson_encoder_hdmi.c | 42 +++++++++++----------- drivers/gpu/drm/meson/meson_vclk.c | 7 ++-- 2 files changed, 25 insertions(+), 24 deletions(-) -- 2.34.1 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/2] drm/meson: vclk: revert and re-fix vclk calculations @ 2025-01-10 7:44 ` Christian Hewitt 0 siblings, 0 replies; 20+ messages in thread From: Christian Hewitt @ 2025-01-10 7:44 UTC (permalink / raw) To: Neil Armstrong, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel Patch 1 reverts a previous fix for loss of HDMI sync when playing YUV420 @ 59.94 media. The patch does resolve a calculation issue. It also makes all fractional rates invalid which is a bigger problem. Patch 2 provides a proper fix after I figured out the actual root cause of the original problem. This time the maths checks out. Apologies to the stable team as the original patch has been applied to a wide range of older and LTS kernels (as far as 5.10). Please let me know if combining the two patches into a single 'fix the previous fix' patch would be preferred? Christian Christian Hewitt (2): Revert "drm/meson: vclk: fix calculation of 59.94 fractional rates" drm/meson: vclk: fix precision in vclk calculations drivers/gpu/drm/meson/meson_encoder_hdmi.c | 42 +++++++++++----------- drivers/gpu/drm/meson/meson_vclk.c | 7 ++-- 2 files changed, 25 insertions(+), 24 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] Revert "drm/meson: vclk: fix calculation of 59.94 fractional rates" 2025-01-10 7:44 ` Christian Hewitt @ 2025-01-10 7:44 ` Christian Hewitt -1 siblings, 0 replies; 20+ messages in thread From: Christian Hewitt @ 2025-01-10 7:44 UTC (permalink / raw) To: Neil Armstrong, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel Cc: stable This reverts commit bfbc68e4d8695497f858a45a142665e22a512ea3. The patch does permit the offending YUV420 @ 59.94 phy_freq and vclk_freq mode to match in calculations. It also results in all fractional rates being unavailable for use. This was unintended and requires the patch to be reverted. Cc: <stable@vger.kernel.org> Signed-off-by: Christian Hewitt <christianshewitt@gmail.com> --- drivers/gpu/drm/meson/meson_vclk.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/meson/meson_vclk.c b/drivers/gpu/drm/meson/meson_vclk.c index 2a942dc6a6dc..2a82119eb58e 100644 --- a/drivers/gpu/drm/meson/meson_vclk.c +++ b/drivers/gpu/drm/meson/meson_vclk.c @@ -790,13 +790,13 @@ meson_vclk_vic_supported_freq(struct meson_drm *priv, unsigned int phy_freq, FREQ_1000_1001(params[i].pixel_freq)); DRM_DEBUG_DRIVER("i = %d phy_freq = %d alt = %d\n", i, params[i].phy_freq, - FREQ_1000_1001(params[i].phy_freq/1000)*1000); + FREQ_1000_1001(params[i].phy_freq/10)*10); /* Match strict frequency */ if (phy_freq == params[i].phy_freq && vclk_freq == params[i].vclk_freq) return MODE_OK; /* Match 1000/1001 variant */ - if (phy_freq == (FREQ_1000_1001(params[i].phy_freq/1000)*1000) && + if (phy_freq == (FREQ_1000_1001(params[i].phy_freq/10)*10) && vclk_freq == FREQ_1000_1001(params[i].vclk_freq)) return MODE_OK; } @@ -1070,7 +1070,7 @@ void meson_vclk_setup(struct meson_drm *priv, unsigned int target, for (freq = 0 ; params[freq].pixel_freq ; ++freq) { if ((phy_freq == params[freq].phy_freq || - phy_freq == FREQ_1000_1001(params[freq].phy_freq/1000)*1000) && + phy_freq == FREQ_1000_1001(params[freq].phy_freq/10)*10) && (vclk_freq == params[freq].vclk_freq || vclk_freq == FREQ_1000_1001(params[freq].vclk_freq))) { if (vclk_freq != params[freq].vclk_freq) -- 2.34.1 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 1/2] Revert "drm/meson: vclk: fix calculation of 59.94 fractional rates" @ 2025-01-10 7:44 ` Christian Hewitt 0 siblings, 0 replies; 20+ messages in thread From: Christian Hewitt @ 2025-01-10 7:44 UTC (permalink / raw) To: Neil Armstrong, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel Cc: stable This reverts commit bfbc68e4d8695497f858a45a142665e22a512ea3. The patch does permit the offending YUV420 @ 59.94 phy_freq and vclk_freq mode to match in calculations. It also results in all fractional rates being unavailable for use. This was unintended and requires the patch to be reverted. Cc: <stable@vger.kernel.org> Signed-off-by: Christian Hewitt <christianshewitt@gmail.com> --- drivers/gpu/drm/meson/meson_vclk.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/meson/meson_vclk.c b/drivers/gpu/drm/meson/meson_vclk.c index 2a942dc6a6dc..2a82119eb58e 100644 --- a/drivers/gpu/drm/meson/meson_vclk.c +++ b/drivers/gpu/drm/meson/meson_vclk.c @@ -790,13 +790,13 @@ meson_vclk_vic_supported_freq(struct meson_drm *priv, unsigned int phy_freq, FREQ_1000_1001(params[i].pixel_freq)); DRM_DEBUG_DRIVER("i = %d phy_freq = %d alt = %d\n", i, params[i].phy_freq, - FREQ_1000_1001(params[i].phy_freq/1000)*1000); + FREQ_1000_1001(params[i].phy_freq/10)*10); /* Match strict frequency */ if (phy_freq == params[i].phy_freq && vclk_freq == params[i].vclk_freq) return MODE_OK; /* Match 1000/1001 variant */ - if (phy_freq == (FREQ_1000_1001(params[i].phy_freq/1000)*1000) && + if (phy_freq == (FREQ_1000_1001(params[i].phy_freq/10)*10) && vclk_freq == FREQ_1000_1001(params[i].vclk_freq)) return MODE_OK; } @@ -1070,7 +1070,7 @@ void meson_vclk_setup(struct meson_drm *priv, unsigned int target, for (freq = 0 ; params[freq].pixel_freq ; ++freq) { if ((phy_freq == params[freq].phy_freq || - phy_freq == FREQ_1000_1001(params[freq].phy_freq/1000)*1000) && + phy_freq == FREQ_1000_1001(params[freq].phy_freq/10)*10) && (vclk_freq == params[freq].vclk_freq || vclk_freq == FREQ_1000_1001(params[freq].vclk_freq))) { if (vclk_freq != params[freq].vclk_freq) -- 2.34.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] Revert "drm/meson: vclk: fix calculation of 59.94 fractional rates" 2025-01-10 7:44 ` Christian Hewitt @ 2025-01-10 10:13 ` Neil Armstrong -1 siblings, 0 replies; 20+ messages in thread From: Neil Armstrong @ 2025-01-10 10:13 UTC (permalink / raw) To: Christian Hewitt, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel Cc: stable On 10/01/2025 08:44, Christian Hewitt wrote: > This reverts commit bfbc68e4d8695497f858a45a142665e22a512ea3. > > The patch does permit the offending YUV420 @ 59.94 phy_freq and > vclk_freq mode to match in calculations. It also results in all > fractional rates being unavailable for use. This was unintended > and requires the patch to be reverted. > > Cc: <stable@vger.kernel.org> > Signed-off-by: Christian Hewitt <christianshewitt@gmail.com> > --- > drivers/gpu/drm/meson/meson_vclk.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/meson/meson_vclk.c b/drivers/gpu/drm/meson/meson_vclk.c > index 2a942dc6a6dc..2a82119eb58e 100644 > --- a/drivers/gpu/drm/meson/meson_vclk.c > +++ b/drivers/gpu/drm/meson/meson_vclk.c > @@ -790,13 +790,13 @@ meson_vclk_vic_supported_freq(struct meson_drm *priv, unsigned int phy_freq, > FREQ_1000_1001(params[i].pixel_freq)); > DRM_DEBUG_DRIVER("i = %d phy_freq = %d alt = %d\n", > i, params[i].phy_freq, > - FREQ_1000_1001(params[i].phy_freq/1000)*1000); > + FREQ_1000_1001(params[i].phy_freq/10)*10); > /* Match strict frequency */ > if (phy_freq == params[i].phy_freq && > vclk_freq == params[i].vclk_freq) > return MODE_OK; > /* Match 1000/1001 variant */ > - if (phy_freq == (FREQ_1000_1001(params[i].phy_freq/1000)*1000) && > + if (phy_freq == (FREQ_1000_1001(params[i].phy_freq/10)*10) && > vclk_freq == FREQ_1000_1001(params[i].vclk_freq)) > return MODE_OK; > } > @@ -1070,7 +1070,7 @@ void meson_vclk_setup(struct meson_drm *priv, unsigned int target, > > for (freq = 0 ; params[freq].pixel_freq ; ++freq) { > if ((phy_freq == params[freq].phy_freq || > - phy_freq == FREQ_1000_1001(params[freq].phy_freq/1000)*1000) && > + phy_freq == FREQ_1000_1001(params[freq].phy_freq/10)*10) && > (vclk_freq == params[freq].vclk_freq || > vclk_freq == FREQ_1000_1001(params[freq].vclk_freq))) { > if (vclk_freq != params[freq].vclk_freq) I wonder if a Fixes is also required here _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] Revert "drm/meson: vclk: fix calculation of 59.94 fractional rates" @ 2025-01-10 10:13 ` Neil Armstrong 0 siblings, 0 replies; 20+ messages in thread From: Neil Armstrong @ 2025-01-10 10:13 UTC (permalink / raw) To: Christian Hewitt, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel Cc: stable On 10/01/2025 08:44, Christian Hewitt wrote: > This reverts commit bfbc68e4d8695497f858a45a142665e22a512ea3. > > The patch does permit the offending YUV420 @ 59.94 phy_freq and > vclk_freq mode to match in calculations. It also results in all > fractional rates being unavailable for use. This was unintended > and requires the patch to be reverted. > > Cc: <stable@vger.kernel.org> > Signed-off-by: Christian Hewitt <christianshewitt@gmail.com> > --- > drivers/gpu/drm/meson/meson_vclk.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/meson/meson_vclk.c b/drivers/gpu/drm/meson/meson_vclk.c > index 2a942dc6a6dc..2a82119eb58e 100644 > --- a/drivers/gpu/drm/meson/meson_vclk.c > +++ b/drivers/gpu/drm/meson/meson_vclk.c > @@ -790,13 +790,13 @@ meson_vclk_vic_supported_freq(struct meson_drm *priv, unsigned int phy_freq, > FREQ_1000_1001(params[i].pixel_freq)); > DRM_DEBUG_DRIVER("i = %d phy_freq = %d alt = %d\n", > i, params[i].phy_freq, > - FREQ_1000_1001(params[i].phy_freq/1000)*1000); > + FREQ_1000_1001(params[i].phy_freq/10)*10); > /* Match strict frequency */ > if (phy_freq == params[i].phy_freq && > vclk_freq == params[i].vclk_freq) > return MODE_OK; > /* Match 1000/1001 variant */ > - if (phy_freq == (FREQ_1000_1001(params[i].phy_freq/1000)*1000) && > + if (phy_freq == (FREQ_1000_1001(params[i].phy_freq/10)*10) && > vclk_freq == FREQ_1000_1001(params[i].vclk_freq)) > return MODE_OK; > } > @@ -1070,7 +1070,7 @@ void meson_vclk_setup(struct meson_drm *priv, unsigned int target, > > for (freq = 0 ; params[freq].pixel_freq ; ++freq) { > if ((phy_freq == params[freq].phy_freq || > - phy_freq == FREQ_1000_1001(params[freq].phy_freq/1000)*1000) && > + phy_freq == FREQ_1000_1001(params[freq].phy_freq/10)*10) && > (vclk_freq == params[freq].vclk_freq || > vclk_freq == FREQ_1000_1001(params[freq].vclk_freq))) { > if (vclk_freq != params[freq].vclk_freq) I wonder if a Fixes is also required here ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations 2025-01-10 7:44 ` Christian Hewitt @ 2025-01-10 7:44 ` Christian Hewitt -1 siblings, 0 replies; 20+ messages in thread From: Christian Hewitt @ 2025-01-10 7:44 UTC (permalink / raw) To: Neil Armstrong, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel Playing YUV420 @ 59.94 media causes HDMI output to lose sync with a fatal error reported: [ 89.610280] Fatal Error, invalid HDMI vclk freq 593406 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. 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. Fixes: e5fab2ec9ca4 ("drm/meson: vclk: add support for YUV420 setup") Signed-off-by: Christian Hewitt <christianshewitt@gmail.com> --- drivers/gpu/drm/meson/meson_encoder_hdmi.c | 42 +++++++++++----------- drivers/gpu/drm/meson/meson_vclk.c | 3 +- 2 files changed, 23 insertions(+), 22 deletions(-) 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 = encoder_hdmi->priv; int vic = 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; - vclk_freq = mode->clock; + vclk_freq = mode->clock * 1000ULL; /* For 420, pixel clock is half unlike venc clock */ if (encoder_hdmi->output_bus_fmt == 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 = vclk_freq * 10; 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; } @@ -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 /= 2; - dev_dbg(priv->dev, "vclk:%d phy=%d venc=%d hdmi=%d enci=%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); } 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 = bridge_to_meson_encoder_hdmi(bridge); struct meson_drm *priv = encoder_hdmi->priv; bool is_hdmi2_sink = 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 = drm_match_cea_mode(mode); enum drm_mode_status status; @@ -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; - vclk_freq = mode->clock; + vclk_freq = mode->clock * 1000ULL; /* For 420, pixel clock is half unlike venc clock */ if (drm_mode_is_420_only(display_info, mode) || @@ -179,10 +177,12 @@ static enum drm_mode_status meson_encoder_hdmi_mode_valid(struct drm_bridge *bri if (mode->flags & DRM_MODE_FLAG_DBLCLK) venc_freq /= 2; - dev_dbg(priv->dev, "%s: vclk:%d phy=%d venc=%d hdmi=%d\n", - __func__, phy_freq, vclk_freq, venc_freq, hdmi_freq); + dev_dbg(priv->dev, "%s: phy=%lld vclk=%lld venc=%lld hdmi=%lld\n", + __func__, phy_freq / 1000ULL, vclk_freq / 1000ULL, + venc_freq / 1000ULL, hdmi_freq / 1000ULL); - return meson_vclk_vic_supported_freq(priv, phy_freq, vclk_freq); + return meson_vclk_vic_supported_freq(priv, phy_freq / 1000ULL, + vclk_freq / 1000ULL); } static void meson_encoder_hdmi_atomic_enable(struct drm_bridge *bridge, diff --git a/drivers/gpu/drm/meson/meson_vclk.c b/drivers/gpu/drm/meson/meson_vclk.c index 2a82119eb58e..a2e93b500ed6 100644 --- a/drivers/gpu/drm/meson/meson_vclk.c +++ b/drivers/gpu/drm/meson/meson_vclk.c @@ -1070,7 +1070,8 @@ void meson_vclk_setup(struct meson_drm *priv, unsigned int target, for (freq = 0 ; params[freq].pixel_freq ; ++freq) { if ((phy_freq == params[freq].phy_freq || - phy_freq == FREQ_1000_1001(params[freq].phy_freq/10)*10) && + phy_freq == FREQ_1000_1001(params[freq].phy_freq/10)*10 || + ((phy_freq/10)*10) == FREQ_1000_1001(params[freq].phy_freq/10)*10) && (vclk_freq == params[freq].vclk_freq || vclk_freq == FREQ_1000_1001(params[freq].vclk_freq))) { if (vclk_freq != params[freq].vclk_freq) -- 2.34.1 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations @ 2025-01-10 7:44 ` Christian Hewitt 0 siblings, 0 replies; 20+ messages in thread From: Christian Hewitt @ 2025-01-10 7:44 UTC (permalink / raw) To: Neil Armstrong, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel Playing YUV420 @ 59.94 media causes HDMI output to lose sync with a fatal error reported: [ 89.610280] Fatal Error, invalid HDMI vclk freq 593406 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. 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. Fixes: e5fab2ec9ca4 ("drm/meson: vclk: add support for YUV420 setup") Signed-off-by: Christian Hewitt <christianshewitt@gmail.com> --- drivers/gpu/drm/meson/meson_encoder_hdmi.c | 42 +++++++++++----------- drivers/gpu/drm/meson/meson_vclk.c | 3 +- 2 files changed, 23 insertions(+), 22 deletions(-) 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 = encoder_hdmi->priv; int vic = 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; - vclk_freq = mode->clock; + vclk_freq = mode->clock * 1000ULL; /* For 420, pixel clock is half unlike venc clock */ if (encoder_hdmi->output_bus_fmt == 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 = vclk_freq * 10; 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; } @@ -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 /= 2; - dev_dbg(priv->dev, "vclk:%d phy=%d venc=%d hdmi=%d enci=%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); } 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 = bridge_to_meson_encoder_hdmi(bridge); struct meson_drm *priv = encoder_hdmi->priv; bool is_hdmi2_sink = 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 = drm_match_cea_mode(mode); enum drm_mode_status status; @@ -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; - vclk_freq = mode->clock; + vclk_freq = mode->clock * 1000ULL; /* For 420, pixel clock is half unlike venc clock */ if (drm_mode_is_420_only(display_info, mode) || @@ -179,10 +177,12 @@ static enum drm_mode_status meson_encoder_hdmi_mode_valid(struct drm_bridge *bri if (mode->flags & DRM_MODE_FLAG_DBLCLK) venc_freq /= 2; - dev_dbg(priv->dev, "%s: vclk:%d phy=%d venc=%d hdmi=%d\n", - __func__, phy_freq, vclk_freq, venc_freq, hdmi_freq); + dev_dbg(priv->dev, "%s: phy=%lld vclk=%lld venc=%lld hdmi=%lld\n", + __func__, phy_freq / 1000ULL, vclk_freq / 1000ULL, + venc_freq / 1000ULL, hdmi_freq / 1000ULL); - return meson_vclk_vic_supported_freq(priv, phy_freq, vclk_freq); + return meson_vclk_vic_supported_freq(priv, phy_freq / 1000ULL, + vclk_freq / 1000ULL); } static void meson_encoder_hdmi_atomic_enable(struct drm_bridge *bridge, diff --git a/drivers/gpu/drm/meson/meson_vclk.c b/drivers/gpu/drm/meson/meson_vclk.c index 2a82119eb58e..a2e93b500ed6 100644 --- a/drivers/gpu/drm/meson/meson_vclk.c +++ b/drivers/gpu/drm/meson/meson_vclk.c @@ -1070,7 +1070,8 @@ void meson_vclk_setup(struct meson_drm *priv, unsigned int target, for (freq = 0 ; params[freq].pixel_freq ; ++freq) { if ((phy_freq == params[freq].phy_freq || - phy_freq == FREQ_1000_1001(params[freq].phy_freq/10)*10) && + phy_freq == FREQ_1000_1001(params[freq].phy_freq/10)*10 || + ((phy_freq/10)*10) == FREQ_1000_1001(params[freq].phy_freq/10)*10) && (vclk_freq == params[freq].vclk_freq || vclk_freq == FREQ_1000_1001(params[freq].vclk_freq))) { if (vclk_freq != params[freq].vclk_freq) -- 2.34.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations 2025-01-10 7:44 ` Christian Hewitt @ 2025-01-10 8:36 ` Maxime Ripard -1 siblings, 0 replies; 20+ messages in thread From: Maxime Ripard @ 2025-01-10 8:36 UTC (permalink / raw) To: Christian Hewitt Cc: Neil Armstrong, Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 4289 bytes --] Hi, 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: > > [ 89.610280] Fatal Error, invalid HDMI vclk freq 593406 > > 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. > > 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. > > Fixes: e5fab2ec9ca4 ("drm/meson: vclk: add support for YUV420 setup") > Signed-off-by: Christian Hewitt <christianshewitt@gmail.com> > --- > drivers/gpu/drm/meson/meson_encoder_hdmi.c | 42 +++++++++++----------- > drivers/gpu/drm/meson/meson_vclk.c | 3 +- > 2 files changed, 23 insertions(+), 22 deletions(-) > > 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 = encoder_hdmi->priv; > int vic = 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; > > - vclk_freq = mode->clock; > + vclk_freq = mode->clock * 1000ULL; You should be using drm_hdmi_compute_mode_clock() here > /* For 420, pixel clock is half unlike venc clock */ > if (encoder_hdmi->output_bus_fmt == 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 = vclk_freq * 10; > > 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; > } > > @@ -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 /= 2; > > - dev_dbg(priv->dev, "vclk:%d phy=%d venc=%d hdmi=%d enci=%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); > } > > 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 = bridge_to_meson_encoder_hdmi(bridge); > struct meson_drm *priv = encoder_hdmi->priv; > bool is_hdmi2_sink = 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 = drm_match_cea_mode(mode); > enum drm_mode_status status; > > @@ -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; > > - vclk_freq = mode->clock; > + vclk_freq = mode->clock * 1000ULL; And here too. Maxime [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 273 bytes --] [-- Attachment #2: Type: text/plain, Size: 167 bytes --] _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations @ 2025-01-10 8:36 ` Maxime Ripard 0 siblings, 0 replies; 20+ messages in thread From: Maxime Ripard @ 2025-01-10 8:36 UTC (permalink / raw) To: Christian Hewitt Cc: Neil Armstrong, Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 4289 bytes --] Hi, 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: > > [ 89.610280] Fatal Error, invalid HDMI vclk freq 593406 > > 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. > > 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. > > Fixes: e5fab2ec9ca4 ("drm/meson: vclk: add support for YUV420 setup") > Signed-off-by: Christian Hewitt <christianshewitt@gmail.com> > --- > drivers/gpu/drm/meson/meson_encoder_hdmi.c | 42 +++++++++++----------- > drivers/gpu/drm/meson/meson_vclk.c | 3 +- > 2 files changed, 23 insertions(+), 22 deletions(-) > > 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 = encoder_hdmi->priv; > int vic = 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; > > - vclk_freq = mode->clock; > + vclk_freq = mode->clock * 1000ULL; You should be using drm_hdmi_compute_mode_clock() here > /* For 420, pixel clock is half unlike venc clock */ > if (encoder_hdmi->output_bus_fmt == 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 = vclk_freq * 10; > > 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; > } > > @@ -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 /= 2; > > - dev_dbg(priv->dev, "vclk:%d phy=%d venc=%d hdmi=%d enci=%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); > } > > 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 = bridge_to_meson_encoder_hdmi(bridge); > struct meson_drm *priv = encoder_hdmi->priv; > bool is_hdmi2_sink = 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 = drm_match_cea_mode(mode); > enum drm_mode_status status; > > @@ -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; > > - vclk_freq = mode->clock; > + vclk_freq = mode->clock * 1000ULL; And here too. Maxime [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 273 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations 2025-01-10 8:36 ` Maxime Ripard @ 2025-01-10 9:35 ` Christian Hewitt -1 siblings, 0 replies; 20+ messages in thread From: Christian Hewitt @ 2025-01-10 9:35 UTC (permalink / raw) To: Maxime Ripard Cc: Neil Armstrong, Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel > On 10 Jan 2025, at 12:36 pm, Maxime Ripard <mripard@kernel.org> wrote: > > Hi, > > 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: >> >> [ 89.610280] Fatal Error, invalid HDMI vclk freq 593406 >> >> 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. >> >> 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. >> >> Fixes: e5fab2ec9ca4 ("drm/meson: vclk: add support for YUV420 setup") >> Signed-off-by: Christian Hewitt <christianshewitt@gmail.com> >> --- >> drivers/gpu/drm/meson/meson_encoder_hdmi.c | 42 +++++++++++----------- >> drivers/gpu/drm/meson/meson_vclk.c | 3 +- >> 2 files changed, 23 insertions(+), 22 deletions(-) >> >> 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 = encoder_hdmi->priv; >> int vic = 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; >> >> - vclk_freq = mode->clock; >> + vclk_freq = mode->clock * 1000ULL; > > You should be using drm_hdmi_compute_mode_clock() here Hello Maxime, If we’re talking a one-two line change than I’d 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 == 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 = vclk_freq * 10; >> >> 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; >> } >> >> @@ -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 /= 2; >> >> - dev_dbg(priv->dev, "vclk:%d phy=%d venc=%d hdmi=%d enci=%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); >> } >> >> 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 = bridge_to_meson_encoder_hdmi(bridge); >> struct meson_drm *priv = encoder_hdmi->priv; >> bool is_hdmi2_sink = 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 = drm_match_cea_mode(mode); >> enum drm_mode_status status; >> >> @@ -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; >> >> - vclk_freq = mode->clock; >> + vclk_freq = mode->clock * 1000ULL; > > And here too. > > Maxime _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations @ 2025-01-10 9:35 ` Christian Hewitt 0 siblings, 0 replies; 20+ messages in thread From: Christian Hewitt @ 2025-01-10 9:35 UTC (permalink / raw) To: Maxime Ripard Cc: Neil Armstrong, Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel > On 10 Jan 2025, at 12:36 pm, Maxime Ripard <mripard@kernel.org> wrote: > > Hi, > > 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: >> >> [ 89.610280] Fatal Error, invalid HDMI vclk freq 593406 >> >> 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. >> >> 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. >> >> Fixes: e5fab2ec9ca4 ("drm/meson: vclk: add support for YUV420 setup") >> Signed-off-by: Christian Hewitt <christianshewitt@gmail.com> >> --- >> drivers/gpu/drm/meson/meson_encoder_hdmi.c | 42 +++++++++++----------- >> drivers/gpu/drm/meson/meson_vclk.c | 3 +- >> 2 files changed, 23 insertions(+), 22 deletions(-) >> >> 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 = encoder_hdmi->priv; >> int vic = 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; >> >> - vclk_freq = mode->clock; >> + vclk_freq = mode->clock * 1000ULL; > > You should be using drm_hdmi_compute_mode_clock() here Hello Maxime, If we’re talking a one-two line change than I’d 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 == 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 = vclk_freq * 10; >> >> 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; >> } >> >> @@ -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 /= 2; >> >> - dev_dbg(priv->dev, "vclk:%d phy=%d venc=%d hdmi=%d enci=%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); >> } >> >> 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 = bridge_to_meson_encoder_hdmi(bridge); >> struct meson_drm *priv = encoder_hdmi->priv; >> bool is_hdmi2_sink = 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 = drm_match_cea_mode(mode); >> enum drm_mode_status status; >> >> @@ -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; >> >> - vclk_freq = mode->clock; >> + vclk_freq = mode->clock * 1000ULL; > > And here too. > > Maxime ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations 2025-01-10 8:36 ` Maxime Ripard @ 2025-01-10 10:12 ` Neil Armstrong -1 siblings, 0 replies; 20+ messages in thread From: Neil Armstrong @ 2025-01-10 10:12 UTC (permalink / raw) To: Maxime Ripard, Christian Hewitt Cc: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel On 10/01/2025 09:36, Maxime Ripard wrote: > Hi, > > 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: >> >> [ 89.610280] Fatal Error, invalid HDMI vclk freq 593406 >> >> 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. >> >> 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. >> >> Fixes: e5fab2ec9ca4 ("drm/meson: vclk: add support for YUV420 setup") >> Signed-off-by: Christian Hewitt <christianshewitt@gmail.com> >> --- >> drivers/gpu/drm/meson/meson_encoder_hdmi.c | 42 +++++++++++----------- >> drivers/gpu/drm/meson/meson_vclk.c | 3 +- >> 2 files changed, 23 insertions(+), 22 deletions(-) >> >> 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 = encoder_hdmi->priv; >> int vic = 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; >> >> - vclk_freq = mode->clock; >> + vclk_freq = mode->clock * 1000ULL; > > You should be using drm_hdmi_compute_mode_clock() here Yes in a second time, this simply fixes the actual calculations so it can be backported to stable kernel to make the feature work again. And yes at some point we should switch to hdmi helpers in a separate forward changeset. Neil > > > >> /* For 420, pixel clock is half unlike venc clock */ >> if (encoder_hdmi->output_bus_fmt == 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 = vclk_freq * 10; >> >> 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; >> } >> >> @@ -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 /= 2; >> >> - dev_dbg(priv->dev, "vclk:%d phy=%d venc=%d hdmi=%d enci=%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); >> } >> >> 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 = bridge_to_meson_encoder_hdmi(bridge); >> struct meson_drm *priv = encoder_hdmi->priv; >> bool is_hdmi2_sink = 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 = drm_match_cea_mode(mode); >> enum drm_mode_status status; >> >> @@ -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; >> >> - vclk_freq = mode->clock; >> + vclk_freq = mode->clock * 1000ULL; > > And here too. > > Maxime _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations @ 2025-01-10 10:12 ` Neil Armstrong 0 siblings, 0 replies; 20+ messages in thread From: Neil Armstrong @ 2025-01-10 10:12 UTC (permalink / raw) To: Maxime Ripard, Christian Hewitt Cc: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel On 10/01/2025 09:36, Maxime Ripard wrote: > Hi, > > 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: >> >> [ 89.610280] Fatal Error, invalid HDMI vclk freq 593406 >> >> 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. >> >> 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. >> >> Fixes: e5fab2ec9ca4 ("drm/meson: vclk: add support for YUV420 setup") >> Signed-off-by: Christian Hewitt <christianshewitt@gmail.com> >> --- >> drivers/gpu/drm/meson/meson_encoder_hdmi.c | 42 +++++++++++----------- >> drivers/gpu/drm/meson/meson_vclk.c | 3 +- >> 2 files changed, 23 insertions(+), 22 deletions(-) >> >> 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 = encoder_hdmi->priv; >> int vic = 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; >> >> - vclk_freq = mode->clock; >> + vclk_freq = mode->clock * 1000ULL; > > You should be using drm_hdmi_compute_mode_clock() here Yes in a second time, this simply fixes the actual calculations so it can be backported to stable kernel to make the feature work again. And yes at some point we should switch to hdmi helpers in a separate forward changeset. Neil > > > >> /* For 420, pixel clock is half unlike venc clock */ >> if (encoder_hdmi->output_bus_fmt == 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 = vclk_freq * 10; >> >> 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; >> } >> >> @@ -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 /= 2; >> >> - dev_dbg(priv->dev, "vclk:%d phy=%d venc=%d hdmi=%d enci=%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); >> } >> >> 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 = bridge_to_meson_encoder_hdmi(bridge); >> struct meson_drm *priv = encoder_hdmi->priv; >> bool is_hdmi2_sink = 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 = drm_match_cea_mode(mode); >> enum drm_mode_status status; >> >> @@ -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; >> >> - vclk_freq = mode->clock; >> + vclk_freq = mode->clock * 1000ULL; > > And here too. > > Maxime ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations 2025-01-10 7:44 ` Christian Hewitt @ 2025-01-20 21:56 ` kernel test robot -1 siblings, 0 replies; 20+ messages in thread From: kernel test robot @ 2025-01-20 21:56 UTC (permalink / raw) To: Christian Hewitt, Neil Armstrong, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel Cc: oe-kbuild-all Hi Christian, kernel test robot noticed the following build errors: [auto build test ERROR on linus/master] [also build test ERROR on v6.13 next-20250120] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Christian-Hewitt/Revert-drm-meson-vclk-fix-calculation-of-59-94-fractional-rates/20250110-154701 base: linus/master patch link: https://lore.kernel.org/r/20250110074458.3624094-3-christianshewitt%40gmail.com patch subject: [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations config: arm-randconfig-r072-20250118 (https://download.01.org/0day-ci/archive/20250121/202501210513.GCec6kts-lkp@intel.com/config) compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project c23f2417dc5f6dc371afb07af5627ec2a9d373a0) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250121/202501210513.GCec6kts-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202501210513.GCec6kts-lkp@intel.com/ All errors (new ones prefixed by >>, old ones prefixed by <<): WARNING: modpost: missing MODULE_DESCRIPTION() in arch/arm/probes/kprobes/test-kprobes.o WARNING: modpost: missing MODULE_DESCRIPTION() in lib/slub_kunit.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/staging/gpib/fmh_gpib/fmh_gpib.o >> ERROR: modpost: "__aeabi_uldivmod" [drivers/gpu/drm/meson/meson-drm.ko] undefined! -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations @ 2025-01-20 21:56 ` kernel test robot 0 siblings, 0 replies; 20+ messages in thread From: kernel test robot @ 2025-01-20 21:56 UTC (permalink / raw) To: Christian Hewitt, Neil Armstrong, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel Cc: oe-kbuild-all Hi Christian, kernel test robot noticed the following build errors: [auto build test ERROR on linus/master] [also build test ERROR on v6.13 next-20250120] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Christian-Hewitt/Revert-drm-meson-vclk-fix-calculation-of-59-94-fractional-rates/20250110-154701 base: linus/master patch link: https://lore.kernel.org/r/20250110074458.3624094-3-christianshewitt%40gmail.com patch subject: [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations config: arm-randconfig-r072-20250118 (https://download.01.org/0day-ci/archive/20250121/202501210513.GCec6kts-lkp@intel.com/config) compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project c23f2417dc5f6dc371afb07af5627ec2a9d373a0) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250121/202501210513.GCec6kts-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202501210513.GCec6kts-lkp@intel.com/ All errors (new ones prefixed by >>, old ones prefixed by <<): WARNING: modpost: missing MODULE_DESCRIPTION() in arch/arm/probes/kprobes/test-kprobes.o WARNING: modpost: missing MODULE_DESCRIPTION() in lib/slub_kunit.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/staging/gpib/fmh_gpib/fmh_gpib.o >> ERROR: modpost: "__aeabi_uldivmod" [drivers/gpu/drm/meson/meson-drm.ko] undefined! -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations 2025-01-20 21:56 ` kernel test robot @ 2025-02-24 8:52 ` Neil Armstrong -1 siblings, 0 replies; 20+ messages in thread From: Neil Armstrong @ 2025-02-24 8:52 UTC (permalink / raw) To: kernel test robot, Christian Hewitt, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel Cc: oe-kbuild-all Hi, On 20/01/2025 22:56, kernel test robot wrote: > Hi Christian, > > kernel test robot noticed the following build errors: > > [auto build test ERROR on linus/master] > [also build test ERROR on v6.13 next-20250120] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: https://github.com/intel-lab-lkp/linux/commits/Christian-Hewitt/Revert-drm-meson-vclk-fix-calculation-of-59-94-fractional-rates/20250110-154701 > base: linus/master > patch link: https://lore.kernel.org/r/20250110074458.3624094-3-christianshewitt%40gmail.com > patch subject: [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations > config: arm-randconfig-r072-20250118 (https://download.01.org/0day-ci/archive/20250121/202501210513.GCec6kts-lkp@intel.com/config) > compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project c23f2417dc5f6dc371afb07af5627ec2a9d373a0) > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250121/202501210513.GCec6kts-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202501210513.GCec6kts-lkp@intel.com/ > > All errors (new ones prefixed by >>, old ones prefixed by <<): > > WARNING: modpost: missing MODULE_DESCRIPTION() in arch/arm/probes/kprobes/test-kprobes.o > WARNING: modpost: missing MODULE_DESCRIPTION() in lib/slub_kunit.o > WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/staging/gpib/fmh_gpib/fmh_gpib.o >>> ERROR: modpost: "__aeabi_uldivmod" [drivers/gpu/drm/meson/meson-drm.ko] undefined! > Please use do_div() when dividing 64bit numbers to please 32-bit platforms. Neil _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations @ 2025-02-24 8:52 ` Neil Armstrong 0 siblings, 0 replies; 20+ messages in thread From: Neil Armstrong @ 2025-02-24 8:52 UTC (permalink / raw) To: kernel test robot, Christian Hewitt, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel Cc: oe-kbuild-all Hi, On 20/01/2025 22:56, kernel test robot wrote: > Hi Christian, > > kernel test robot noticed the following build errors: > > [auto build test ERROR on linus/master] > [also build test ERROR on v6.13 next-20250120] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: https://github.com/intel-lab-lkp/linux/commits/Christian-Hewitt/Revert-drm-meson-vclk-fix-calculation-of-59-94-fractional-rates/20250110-154701 > base: linus/master > patch link: https://lore.kernel.org/r/20250110074458.3624094-3-christianshewitt%40gmail.com > patch subject: [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations > config: arm-randconfig-r072-20250118 (https://download.01.org/0day-ci/archive/20250121/202501210513.GCec6kts-lkp@intel.com/config) > compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project c23f2417dc5f6dc371afb07af5627ec2a9d373a0) > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250121/202501210513.GCec6kts-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202501210513.GCec6kts-lkp@intel.com/ > > All errors (new ones prefixed by >>, old ones prefixed by <<): > > WARNING: modpost: missing MODULE_DESCRIPTION() in arch/arm/probes/kprobes/test-kprobes.o > WARNING: modpost: missing MODULE_DESCRIPTION() in lib/slub_kunit.o > WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/staging/gpib/fmh_gpib/fmh_gpib.o >>> ERROR: modpost: "__aeabi_uldivmod" [drivers/gpu/drm/meson/meson-drm.ko] undefined! > Please use do_div() when dividing 64bit numbers to please 32-bit platforms. Neil ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/2] drm/meson: vclk: revert and re-fix vclk calculations 2025-01-10 7:44 ` Christian Hewitt @ 2025-02-24 8:39 ` Neil Armstrong -1 siblings, 0 replies; 20+ messages in thread From: Neil Armstrong @ 2025-02-24 8:39 UTC (permalink / raw) To: Christian Hewitt, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel On 10/01/2025 08:44, Christian Hewitt wrote: > Patch 1 reverts a previous fix for loss of HDMI sync when playing YUV420 > @ 59.94 media. The patch does resolve a calculation issue. It also makes > all fractional rates invalid which is a bigger problem. > > Patch 2 provides a proper fix after I figured out the actual root cause > of the original problem. This time the maths checks out. > > Apologies to the stable team as the original patch has been applied to > a wide range of older and LTS kernels (as far as 5.10). Please let me > know if combining the two patches into a single 'fix the previous fix' > patch would be preferred? > > Christian > > Christian Hewitt (2): > Revert "drm/meson: vclk: fix calculation of 59.94 fractional rates" > drm/meson: vclk: fix precision in vclk calculations > > drivers/gpu/drm/meson/meson_encoder_hdmi.c | 42 +++++++++++----------- > drivers/gpu/drm/meson/meson_vclk.c | 7 ++-- > 2 files changed, 25 insertions(+), 24 deletions(-) > LGTM Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/2] drm/meson: vclk: revert and re-fix vclk calculations @ 2025-02-24 8:39 ` Neil Armstrong 0 siblings, 0 replies; 20+ messages in thread From: Neil Armstrong @ 2025-02-24 8:39 UTC (permalink / raw) To: Christian Hewitt, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jernej Skrabec, dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel On 10/01/2025 08:44, Christian Hewitt wrote: > Patch 1 reverts a previous fix for loss of HDMI sync when playing YUV420 > @ 59.94 media. The patch does resolve a calculation issue. It also makes > all fractional rates invalid which is a bigger problem. > > Patch 2 provides a proper fix after I figured out the actual root cause > of the original problem. This time the maths checks out. > > Apologies to the stable team as the original patch has been applied to > a wide range of older and LTS kernels (as far as 5.10). Please let me > know if combining the two patches into a single 'fix the previous fix' > patch would be preferred? > > Christian > > Christian Hewitt (2): > Revert "drm/meson: vclk: fix calculation of 59.94 fractional rates" > drm/meson: vclk: fix precision in vclk calculations > > drivers/gpu/drm/meson/meson_encoder_hdmi.c | 42 +++++++++++----------- > drivers/gpu/drm/meson/meson_vclk.c | 7 ++-- > 2 files changed, 25 insertions(+), 24 deletions(-) > LGTM Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2025-02-24 9:12 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-01-10 7:44 [PATCH 0/2] drm/meson: vclk: revert and re-fix vclk calculations Christian Hewitt 2025-01-10 7:44 ` Christian Hewitt 2025-01-10 7:44 ` [PATCH 1/2] Revert "drm/meson: vclk: fix calculation of 59.94 fractional rates" Christian Hewitt 2025-01-10 7:44 ` Christian Hewitt 2025-01-10 10:13 ` Neil Armstrong 2025-01-10 10:13 ` Neil Armstrong 2025-01-10 7:44 ` [PATCH 2/2] drm/meson: vclk: fix precision in vclk calculations Christian Hewitt 2025-01-10 7:44 ` Christian Hewitt 2025-01-10 8:36 ` Maxime Ripard 2025-01-10 8:36 ` Maxime Ripard 2025-01-10 9:35 ` Christian Hewitt 2025-01-10 9:35 ` Christian Hewitt 2025-01-10 10:12 ` Neil Armstrong 2025-01-10 10:12 ` Neil Armstrong 2025-01-20 21:56 ` kernel test robot 2025-01-20 21:56 ` kernel test robot 2025-02-24 8:52 ` Neil Armstrong 2025-02-24 8:52 ` Neil Armstrong 2025-02-24 8:39 ` [PATCH 0/2] drm/meson: vclk: revert and re-fix " Neil Armstrong 2025-02-24 8:39 ` Neil Armstrong
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.