From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: "Cristian Ciocaltea" <cristian.ciocaltea@collabora.com>,
"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
"Maxime Ripard" <mripard@kernel.org>,
"Thomas Zimmermann" <tzimmermann@suse.de>,
"David Airlie" <airlied@gmail.com>,
"Simona Vetter" <simona@ffwll.ch>,
"Sandy Huang" <hjc@rock-chips.com>,
"Heiko Stübner" <heiko@sntech.de>,
"Andy Yan" <andy.yan@rock-chips.com>,
"Louis Chauvet" <louis.chauvet@bootlin.com>,
"Haneen Mohammed" <hamohammed.sa@gmail.com>,
"Melissa Wen" <melissa.srw@gmail.com>,
"Robert Mader" <robert.mader@collabora.com>,
kernel@collabora.com, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org,
"Nícolas F. R. A. Prado" <nfraprado@collabora.com>,
"Diederik de Haas" <diederik@cknow-tech.com>
Subject: Re: [PATCH v5 1/4] uapi: Provide DIV_ROUND_CLOSEST()
Date: Tue, 27 Jan 2026 16:38:00 +0200 [thread overview]
Message-ID: <aXjNyHaJDHoCPRJO@smile.fi.intel.com> (raw)
In-Reply-To: <baf075b2370a13cddd597a3d421b5f39290d87a4@intel.com>
On Tue, Jan 27, 2026 at 03:58:13PM +0200, Jani Nikula wrote:
> On Tue, 27 Jan 2026, Cristian Ciocaltea <cristian.ciocaltea@collabora.com> wrote:
> > Currently DIV_ROUND_CLOSEST() is only available for the kernel via
> > include/linux/math.h.
> >
> > Expose it to userland as well by adding __KERNEL_DIV_ROUND_CLOSEST() as
> > a common definition in uapi.
> >
> > Additionally, ensure it allows building ISO C applications by switching
> > from the 'typeof' GNU extension to the ISO-friendly __typeof__.
>
> I am not convinced that it's a good idea to make the implementation of
> kernel DIV_ROUND_CLOSEST() part of the kernel UAPI, which is what this
> change effectively does.
>
> I'd at least like to get an ack from Andy Shevchenko first (Cc'd).
Thanks for Cc'ing me!
So, the history of the DIV_ROUND_UP() to appear in UAPI is a response to
the ethtool change that missed the fact that this was a kernel internal macro.
Giving a precedent there is no technical issues to add DIV_ROUND_CLOSEST()
to UAPI as proposed. Main question here is: Does DRM headers in question
(that are going to use it) really need this?
Interestingly that DRM also started using __KERNEL_DIV_ROUND_UP() in UAPI
at some point, which kinda makes an argument for allowing the other one.
Also fun fact: this series plead for a new macro for division while ignoring
existing (UAPI) macros for masks and bits.
0xffffU is effectively __GENMASK(15, 0). (And if you change the code, avoid
using variables inside GENMASK() macros, it may generate an awful code, the
GENMASK($HI, $LO) << foo is preferred over GENMASK(foo + $DELTA, foo) case.
GENMASK(foo - 1, 0) OTOH is fine, however be always careful against overflows
with left shifts, as BIT(foo) - 1 may not work for foo == 32, while GENMASK()
may not work for foo == 0).
So, I have no objections for either choice
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
...
But if you go that direction, please, fix up the style.
> > +/*
> > + * Divide positive or negative dividend by positive or negative divisor
> > + * and round to closest integer. Result is undefined for negative
> > + * divisors if the dividend variable type is unsigned and for negative
> > + * dividends if the divisor variable type is unsigned.
> > + */
> > +#define __KERNEL_DIV_ROUND_CLOSEST(x, divisor)( \
> > +{ \
Use ({ on this line together...
> > + __typeof__(x) __x = x; \
> > + __typeof__(divisor) __d = divisor; \
+ blank line here.
> > + (((__typeof__(x))-1) > 0 || \
> > + ((__typeof__(divisor))-1) > 0 || \
> > + (((__x) > 0) == ((__d) > 0))) ? \
> > + (((__x) + ((__d) / 2)) / (__d)) : \
> > + (((__x) - ((__d) / 2)) / (__d)); \
> > +} \
> > +)
...as here join }) to be a single line.
+ blank line.
> > #endif /* _UAPI_LINUX_CONST_H */
--
With Best Regards,
Andy Shevchenko
WARNING: multiple messages have this Message-ID (diff)
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: "Simona Vetter" <simona@ffwll.ch>,
"Haneen Mohammed" <hamohammed.sa@gmail.com>,
"Louis Chauvet" <louis.chauvet@bootlin.com>,
kernel@collabora.com, "Heiko Stübner" <heiko@sntech.de>,
"Nícolas F. R. A. Prado" <nfraprado@collabora.com>,
"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
"Sandy Huang" <hjc@rock-chips.com>,
"Maxime Ripard" <mripard@kernel.org>,
linux-kernel@vger.kernel.org,
"Melissa Wen" <melissa.srw@gmail.com>,
linux-rockchip@lists.infradead.org,
"Diederik de Haas" <diederik@cknow-tech.com>,
dri-devel@lists.freedesktop.org,
"Thomas Zimmermann" <tzimmermann@suse.de>,
"Andy Yan" <andy.yan@rock-chips.com>,
"Robert Mader" <robert.mader@collabora.com>,
"David Airlie" <airlied@gmail.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v5 1/4] uapi: Provide DIV_ROUND_CLOSEST()
Date: Tue, 27 Jan 2026 16:38:00 +0200 [thread overview]
Message-ID: <aXjNyHaJDHoCPRJO@smile.fi.intel.com> (raw)
In-Reply-To: <baf075b2370a13cddd597a3d421b5f39290d87a4@intel.com>
On Tue, Jan 27, 2026 at 03:58:13PM +0200, Jani Nikula wrote:
> On Tue, 27 Jan 2026, Cristian Ciocaltea <cristian.ciocaltea@collabora.com> wrote:
> > Currently DIV_ROUND_CLOSEST() is only available for the kernel via
> > include/linux/math.h.
> >
> > Expose it to userland as well by adding __KERNEL_DIV_ROUND_CLOSEST() as
> > a common definition in uapi.
> >
> > Additionally, ensure it allows building ISO C applications by switching
> > from the 'typeof' GNU extension to the ISO-friendly __typeof__.
>
> I am not convinced that it's a good idea to make the implementation of
> kernel DIV_ROUND_CLOSEST() part of the kernel UAPI, which is what this
> change effectively does.
>
> I'd at least like to get an ack from Andy Shevchenko first (Cc'd).
Thanks for Cc'ing me!
So, the history of the DIV_ROUND_UP() to appear in UAPI is a response to
the ethtool change that missed the fact that this was a kernel internal macro.
Giving a precedent there is no technical issues to add DIV_ROUND_CLOSEST()
to UAPI as proposed. Main question here is: Does DRM headers in question
(that are going to use it) really need this?
Interestingly that DRM also started using __KERNEL_DIV_ROUND_UP() in UAPI
at some point, which kinda makes an argument for allowing the other one.
Also fun fact: this series plead for a new macro for division while ignoring
existing (UAPI) macros for masks and bits.
0xffffU is effectively __GENMASK(15, 0). (And if you change the code, avoid
using variables inside GENMASK() macros, it may generate an awful code, the
GENMASK($HI, $LO) << foo is preferred over GENMASK(foo + $DELTA, foo) case.
GENMASK(foo - 1, 0) OTOH is fine, however be always careful against overflows
with left shifts, as BIT(foo) - 1 may not work for foo == 32, while GENMASK()
may not work for foo == 0).
So, I have no objections for either choice
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
...
But if you go that direction, please, fix up the style.
> > +/*
> > + * Divide positive or negative dividend by positive or negative divisor
> > + * and round to closest integer. Result is undefined for negative
> > + * divisors if the dividend variable type is unsigned and for negative
> > + * dividends if the divisor variable type is unsigned.
> > + */
> > +#define __KERNEL_DIV_ROUND_CLOSEST(x, divisor)( \
> > +{ \
Use ({ on this line together...
> > + __typeof__(x) __x = x; \
> > + __typeof__(divisor) __d = divisor; \
+ blank line here.
> > + (((__typeof__(x))-1) > 0 || \
> > + ((__typeof__(divisor))-1) > 0 || \
> > + (((__x) > 0) == ((__d) > 0))) ? \
> > + (((__x) + ((__d) / 2)) / (__d)) : \
> > + (((__x) - ((__d) / 2)) / (__d)); \
> > +} \
> > +)
...as here join }) to be a single line.
+ blank line.
> > #endif /* _UAPI_LINUX_CONST_H */
--
With Best Regards,
Andy Shevchenko
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
next prev parent reply other threads:[~2026-01-27 14:38 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-27 8:45 [PATCH v5 0/4] Introduce BACKGROUND_COLOR DRM CRTC property Cristian Ciocaltea
2026-01-27 8:45 ` Cristian Ciocaltea
2026-01-27 8:45 ` [PATCH v5 1/4] uapi: Provide DIV_ROUND_CLOSEST() Cristian Ciocaltea
2026-01-27 8:45 ` Cristian Ciocaltea
2026-01-27 12:35 ` AngeloGioacchino Del Regno
2026-01-27 12:35 ` AngeloGioacchino Del Regno
2026-01-27 13:58 ` Jani Nikula
2026-01-27 13:58 ` Jani Nikula
2026-01-27 14:38 ` Andy Shevchenko [this message]
2026-01-27 14:38 ` Andy Shevchenko
2026-01-27 17:39 ` David Laight
2026-01-27 17:39 ` David Laight
2026-01-29 1:14 ` Cristian Ciocaltea
2026-01-29 1:14 ` Cristian Ciocaltea
2026-01-27 8:45 ` [PATCH v5 2/4] drm: Add CRTC background color property Cristian Ciocaltea
2026-01-27 8:45 ` Cristian Ciocaltea
2026-01-27 8:45 ` [PATCH v5 3/4] drm/vkms: Support setting custom background color Cristian Ciocaltea
2026-01-27 8:45 ` Cristian Ciocaltea
2026-01-27 8:45 ` [PATCH v5 4/4] drm/rockchip: vop2: " Cristian Ciocaltea
2026-01-27 8:45 ` Cristian Ciocaltea
2026-01-27 14:45 ` Andy Shevchenko
2026-01-27 14:45 ` Andy Shevchenko
2026-01-29 1:29 ` Cristian Ciocaltea
2026-01-29 1:29 ` Cristian Ciocaltea
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aXjNyHaJDHoCPRJO@smile.fi.intel.com \
--to=andriy.shevchenko@linux.intel.com \
--cc=airlied@gmail.com \
--cc=andy.yan@rock-chips.com \
--cc=cristian.ciocaltea@collabora.com \
--cc=diederik@cknow-tech.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=hamohammed.sa@gmail.com \
--cc=heiko@sntech.de \
--cc=hjc@rock-chips.com \
--cc=jani.nikula@linux.intel.com \
--cc=kernel@collabora.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=louis.chauvet@bootlin.com \
--cc=maarten.lankhorst@linux.intel.com \
--cc=melissa.srw@gmail.com \
--cc=mripard@kernel.org \
--cc=nfraprado@collabora.com \
--cc=robert.mader@collabora.com \
--cc=simona@ffwll.ch \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.