On Mon, Mar 04, 2024 at 11:05:14AM +0100, Geert Uytterhoeven wrote:
> Hi Maxime,
>
> Thanks for your patch!
>
> On Mon, Mar 4, 2024 at 10:12 AM Maxime Ripard <mripard@xxxxxxxxxx> wrote:
> > Commit 358e76fd613a ("drm/sun4i: hdmi: Consolidate atomic_check and
> > mode_valid") changed the clock rate from an unsigned long to an unsigned
> > long long resulting in a a 64-bit division that might not be supported
> > on all platforms.
>
> Why was this changed to unsigned long long?
> Can a valid pixel clock really not fit in 32-bit?
Yes, HDMI 2.1 supports pixel rates up until 5.940GHz, so the framework
has to use that.
> > The resulted in compilation being broken at least for m68k, xtensa and
> > some arm configurations, at least.
> >
> > Fixes: 358e76fd613a ("drm/sun4i: hdmi: Consolidate atomic_check and mode_valid")
> > Reported-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > Reported-by: Naresh Kamboju <naresh.kamboju@xxxxxxxxxx>
> > Closes: https://lore.kernel.org/r/CA+G9fYvG9KE15PGNoLu+SBVyShe+u5HBLQ81+kK9Zop6u=ywmw@xxxxxxxxxxxxxx/
> > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202403011839.KLiXh4wC-lkp@xxxxxxxxx/
> > Signed-off-by: Maxime Ripard <mripard@xxxxxxxxxx>
>
> > --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> > @@ -163,11 +163,11 @@ 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)
> > {
> > const struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);
> > - unsigned long diff = clock / 200; /* +-0.5% allowed by HDMI spec */
> > + unsigned long diff = div_u64(clock, 200); /* +-0.5% allowed by HDMI spec */
>
> I'd rather see clock changed back to unsigned long.
No, the tolerance we allow is an unsigned long. The tolerance is 0.5% of
the pixel clock, so even if that controller supported HDMI 2.1 at its
full capacity (it doesn't, at all), it would be 29.7 MHz, which fits
comfortably in an unsigned long.
Maxime
Attachment:
signature.asc
Description: PGP signature
