On Sat, Mar 9, 2024 at 3:34 PM David Laight David.Laight@aculab.com wrote:
From: Maxime Ripard
Sent: 04 March 2024 11:46
On Mon, Mar 04, 2024 at 12:11:36PM +0100, Arnd Bergmann wrote:
On Mon, Mar 4, 2024, at 09:07, Naresh Kamboju wrote:
The arm defconfig builds failed on today's Linux next tag next-20240304.
Build log:
ERROR: modpost: "__aeabi_uldivmod" [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined!
Apparently caused by the 64-bit division in 358e76fd613a ("drm/sun4i: hdmi: Consolidate atomic_check and mode_valid"):
+static enum drm_mode_status +sun4i_hdmi_connector_clock_valid(const struct drm_connector *connector,
const struct drm_display_mode *mode,unsigned long long clock){
struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder);unsigned long rate = mode->clock * 1000;unsigned long diff = rate / 200; /* +-0.5% allowed by HDMI spec */
const struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);unsigned long diff = clock / 200; /* +-0.5% allowed by HDMI spec */ long rounded_rate;This used to be a 32-bit division. If the rate is never more than 4.2GHz, clock could be turned back into 'unsigned long' to avoid the expensive div_u64().
I sent a fix for it this morning: https://lore.kernel.org/r/20240304091225.366325-1-mripard@kernel.org
The framework will pass an unsigned long long because HDMI character rates can go up to 5.9GHz.
You could do: /* The max clock is 5.9GHz, split the divide */ u32 diff = (u32)(clock / 8) / (200/8);
+1, as the issue is still present in current next, as per the recent nagging from the build bots.
The code should really use u32 and u64. Otherwise the sizes are different on 32bit.
The code is already using a variety of types (long, unsigned long long, unsigned long) :-(
max_t(unsigned long, rounded_rate, clock) - min_t(unsigned long, rounded_rate, clock) < diff)
At least u64 should make it very clear clock does not fit in 32-bit.
Gr{oetje,eeting}s,
Geert