RE: [REGRESSION] QXL display malfunction
[AMD Official Use Only - AMD Internal Distribution Only]
> -Original Message-
> From: Thomas Zimmermann
> Sent: Wednesday, June 12, 2024 9:26 AM
> To: Linux regressions mailing list
> Cc: Petkov, Borislav ;
> zack.ru...@broadcom.com; dmitry.osipe...@collabora.com; Kaplan, David
> ; Koenig, Christian ;
> Dave Airlie ; Maarten Lankhorst
> ; Maxime Ripard
> ; LKML ; ML dri-devel
> ; spice-de...@lists.freedesktop.org;
> virtualizat...@lists.linux.dev
> Subject: Re: [REGRESSION] QXL display malfunction
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> Hi
>
> Am 12.06.24 um 14:41 schrieb Linux regression tracking (Thorsten Leemhuis):
> > [CCing a few more people and lists that get_maintainers pointed out
> > for qxl]
> >
> > Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
> > for once, to make this easily accessible to everyone.
> >
> > Thomas, from here it looks like this report that apparently is caused
> > by a change of yours that went into 6.10-rc1 (b33651a5c98dbd
> > ("drm/qxl: Do not pin buffer objects for vmap")) fell through the
> > cracks. Or was progress made to resolve this and I just missed this?
> >
> > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker'
> > hat)
> > --
> > Everything you wanna know about Linux kernel regression tracking:
> > https://linux-regtracking.leemhuis.info/about/#tldr
> > If I did something stupid, please tell me, as explained on that page.
> >
> > #regzbot poke
> >
> >
> > On 03.06.24 04:29, Kaplan, David wrote:
> >>> -Original Message-
> >>> From: Kaplan, David
> >>> Sent: Sunday, June 2, 2024 9:25 PM
> >>> To: tzimmerm...@suse.de; dmitry.osipe...@collabora.com; Koenig,
> >>> Christian ; zach.ru...@broadcom.com
> >>> Cc: Petkov, Borislav ;
> >>> regressi...@list.linux.dev
> >>> Subject: [REGRESSION] QXL display malfunction
> >>>
> >>> Hi,
> >>>
> >>> I am running an Ubuntu 19.10 VM with a tip kernel using QXL video
> >>> and I've observed the VM graphics often malfunction after boot,
> >>> sometimes failing to load the Ubuntu desktop or even immediately
> shutting the guest down.
> >>> When it does load, the guest dmesg log often contains errors like
> >>>
> >>> [4.303586] [drm:drm_atomic_helper_commit_planes] *ERROR* head
> 1
> >>> wrong: 65376256x16777216+0+0
> >>> [4.586883] [drm:drm_atomic_helper_commit_planes] *ERROR* head
> 1
> >>> wrong: 65376256x16777216+0+0
> >>> [4.904036] [drm:drm_atomic_helper_commit_planes] *ERROR* head
> 1
> >>> wrong: 65335296x16777216+0+0
>
> I don't see how these messages are related. Did they already appear before
> the broken commit was there?
No, I did not observe them prior to the broken commit.
>
> >>> [5.374347] [drm:qxl_release_from_id_locked] *ERROR* failed to find
> id in
> >>> release_idr
>
> Is there only one such message in the log? Or multiple/frequent ones.
I would usually only see one.
>
> Could you provide a stack trace of what happens before?
Here's the top of a backtrace when the error occurs:
#0 qxl_release_from_id_locked (qdev=qdev@entry=0x88810126e000,
id=id@entry=262151)
at drivers/gpu/drm/qxl/qxl_release.c:373
#1 0x819f5b6a in qxl_garbage_collect (qdev=0x88810126e000)
at drivers/gpu/drm/qxl/qxl_cmd.c:222
#2 0x810e3aa8 in process_one_work
(worker=worker@entry=0x888101680300,
work=0x88810126f340) at kernel/workqueue.c:3231
#3 0x810e6281 in process_scheduled_works (worker=)
at kernel/workqueue.c:3312
#4 worker_thread (__worker=0x888101680300) at kernel/workqueue.c:3393
>
> We sometimes draw into the buffer object from the CPU. For accessing the
> buffer object's pages from the CPU, only a vmap operation should be
> necessary. It appears as if qxl also requires a pin. My guess is that the pin
> inserts the buffer-object's host-side pages and the code around
> qxl_release_from_id_locked() appears to be garbage-collecting them.
> Hence without the pin, the GC complains about inconsistent state.
> >>>
> >>> I bisected the issue down to "drm/qxl: Do not pin buffer objects for
> vmap"
> >>> (b33651a5c98dbd5a919219d8c129d0674ef74299).
>
> Thanks for bisecting. Does it work if you revert that commit?
Yes
Thanks --David Kaplan
Re: [PATCH net-next v10 02/14] net: page_pool: create hooks for custom page providers
On 6/12/24 6:06 AM, Jason Gunthorpe wrote: > On Tue, Jun 11, 2024 at 11:09:15AM -0700, Mina Almasry wrote: > >> Just curious: in Pavel's effort, io_uring - which is not a device - is >> trying to share memory with the page_pool, which is also not a device. >> And Pavel is being asked to wrap the memory in a dmabuf. Is dmabuf >> going to be the kernel's standard for any memory sharing between any 2 >> components in the future, even when they're not devices? > > dmabuf is how we are refcounting non-struct page memory, there is > nothing about it that says it has to be MMIO memory, or even that the > memory doesn't have struct pages. > > All it says is that the memory is alive according to dmabuf > refcounting rules. And the importer obviously don't get to touch the > underlying folios, if any. > In addition, the io_uring developers should be considering the use case of device memory. There is no reason for this design to be limited to host memory. io_uring should not care (it is not peeking inside the memory buffers); it is just memory references. One of io_uring's primary benefits is avoiding system calls. io_uring works with TCP sockets. Let it work with any dmabuf without concern of memory type. The performance benefits the Google crowd sees with system call based apps should be even better with io_uring. Focus on primitives, building blocks with solid APIs for other subsystems to leverage and let them be wired up in ways you cannot imagine today.
Re: [PATCH 2/6] drm/nouveau: remove unused struct 'init_exec'
* Danilo Krummrich (d...@redhat.com) wrote:
> On 5/18/24 01:26, li...@treblig.org wrote:
> > From: "Dr. David Alan Gilbert"
> >
> > 'init_exec' is unused since
> > commit cb75d97e9c77 ("drm/nouveau: implement devinit subdev, and new
> > init table parser")
> > Remove it.
> >
> > Signed-off-by: Dr. David Alan Gilbert
>
> Acked-by: Danilo Krummrich
>
> To which series does this patch belong?
Actually all of them were independent patches on drm
some of which are all in, so it can be taken by itself.
> Need me to apply it?
Yes please!
Thanks,
Dave
> - Danilo
>
> > ---
> > drivers/gpu/drm/nouveau/nouveau_bios.c | 5 -
> > 1 file changed, 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c
> > b/drivers/gpu/drm/nouveau/nouveau_bios.c
> > index 79cfab53f80e..8c3c1f1e01c5 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_bios.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_bios.c
> > @@ -43,11 +43,6 @@
> > #define BIOSLOG(sip, fmt, arg...) NV_DEBUG(sip->dev, fmt, ##arg)
> > #define LOG_OLD_VALUE(x)
> > -struct init_exec {
> > - bool execute;
> > - bool repeat;
> > -};
> > -
> > static bool nv_cksum(const uint8_t *data, unsigned int length)
> > {
> > /*
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH net-next v10 02/14] net: page_pool: create hooks for custom page providers
On 6/10/24 6:16 AM, Jason Gunthorpe wrote: > On Mon, Jun 10, 2024 at 02:07:01AM +0100, Pavel Begunkov wrote: >> On 6/10/24 01:37, David Wei wrote: >>> On 2024-06-07 17:52, Jason Gunthorpe wrote: >>>> IMHO it seems to compose poorly if you can only use the io_uring >>>> lifecycle model with io_uring registered memory, and not with DMABUF >>>> memory registered through Mina's mechanism. >>> >>> By this, do you mean io_uring must be exclusively used to use this >>> feature? >>> >>> And you'd rather see the two decoupled, so userspace can register w/ say >>> dmabuf then pass it to io_uring? >> >> Personally, I have no clue what Jason means. You can just as >> well say that it's poorly composable that write(2) to a disk >> cannot post a completion into a XDP ring, or a netlink socket, >> or io_uring's main completion queue, or name any other API. > > There is no reason you shouldn't be able to use your fast io_uring > completion and lifecycle flow with DMABUF backed memory. Those are not > widly different things and there is good reason they should work > together. > > Pretending they are totally different just because two different > people wrote them is a very siloed view. > >> The devmem TCP callback can implement it in a way feasible to >> the project, but it cannot directly post events to an unrelated >> API like io_uring. And devmem attaches buffers to a socket, >> for which a ring for returning buffers might even be a nuisance. > > If you can't compose your io_uring completion mechanism with a DMABUF > provided backing store then I think it needs more work. > exactly. io_uring, page_pool, dmabuf - all kernel building blocks for solutions. This why I was pushing for Mina's set not to be using the name `devmem` - it is but one type of memory and with dmabuf it should not matter if it is gpu or host (or something else later on - cxl?).
Re: [PATCH net-next v10 02/14] net: page_pool: create hooks for custom page providers
On 2024-06-07 17:52, Jason Gunthorpe wrote: > IMHO it seems to compose poorly if you can only use the io_uring > lifecycle model with io_uring registered memory, and not with DMABUF > memory registered through Mina's mechanism. By this, do you mean io_uring must be exclusively used to use this feature? And you'd rather see the two decoupled, so userspace can register w/ say dmabuf then pass it to io_uring? > > Jason
Re: [PATCH net-next v10 02/14] net: page_pool: create hooks for custom page providers
On 2024-06-07 17:27, David Ahern wrote: > I also do not understand why the ifq cache and overloading xdp functions > have stuck around; I always thought both were added by Jonathan to > simplify kernel ports during early POC days. Setting up an Rx queue for ZC w/ a different pp will be done properly using the new queue API that Mina merged recently. Those custom XDP hooks will be gone in a non-RFC patchset.
Re: [PATCH net-next v10 02/14] net: page_pool: create hooks for custom page providers
On 6/7/24 7:42 AM, Pavel Begunkov wrote: > I haven't seen any arguments against from the (net) maintainers so > far. Nor I see any objection against callbacks from them (considering > that either option adds an if). I have said before I do not understand why the dmabuf paradigm is not sufficient for both device memory and host memory. A less than ideal control path to put hostmem in a dmabuf wrapper vs extra checks and changes in the datapath. The former should always be preferred. I also do not understand why the ifq cache and overloading xdp functions have stuck around; I always thought both were added by Jonathan to simplify kernel ports during early POC days.
Re: [PATCH 2/6] drm/nouveau: remove unused struct 'init_exec'
* li...@treblig.org (li...@treblig.org) wrote:
> From: "Dr. David Alan Gilbert"
>
> 'init_exec' is unused since
> commit cb75d97e9c77 ("drm/nouveau: implement devinit subdev, and new
> init table parser")
> Remove it.
>
> Signed-off-by: Dr. David Alan Gilbert
Ping.
> ---
> drivers/gpu/drm/nouveau/nouveau_bios.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c
> b/drivers/gpu/drm/nouveau/nouveau_bios.c
> index 79cfab53f80e..8c3c1f1e01c5 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bios.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bios.c
> @@ -43,11 +43,6 @@
> #define BIOSLOG(sip, fmt, arg...) NV_DEBUG(sip->dev, fmt, ##arg)
> #define LOG_OLD_VALUE(x)
>
> -struct init_exec {
> - bool execute;
> - bool repeat;
> -};
> -
> static bool nv_cksum(const uint8_t *data, unsigned int length)
> {
> /*
> --
> 2.45.1
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 3/6] drm/vmwgfx: remove unused struct 'vmw_stdu_dma'
* li...@treblig.org (li...@treblig.org) wrote:
> From: "Dr. David Alan Gilbert"
>
> 'vmw_stdu_dma' is unused since
> commit 39985eea5a6d ("drm/vmwgfx: Abstract placement selection")
> Remove it.
Ping.
> Signed-off-by: Dr. David Alan Gilbert
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index 2041c4d48daa..50022e9e3519 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -85,11 +85,6 @@ struct vmw_stdu_update {
> SVGA3dCmdUpdateGBScreenTarget body;
> };
>
> -struct vmw_stdu_dma {
> - SVGA3dCmdHeader header;
> - SVGA3dCmdSurfaceDMA body;
> -};
> -
> struct vmw_stdu_surface_copy {
> SVGA3dCmdHeader header;
> SVGA3dCmdSurfaceCopy body;
> --
> 2.45.1
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH net-next v10 01/14] netdev: add netdev_rx_queue_restart()
On 2024-05-30 13:16, Mina Almasry wrote:
[...]
> +err_start_queue:
> + /* Restarting the queue with old_mem should be successful as we haven't
> + * changed any of the queue configuration, and there is not much we can
> + * do to recover from a failure here.
> + *
> + * WARN if the we fail to recover the old rx queue, and at least free
> + * old_mem so we don't also leak that.
> + */
> + if (dev->queue_mgmt_ops->ndo_queue_start(dev, old_mem, rxq_idx)) {
> + WARN(1,
> + "Failed to restart old queue in error path. RX queue %d
> may be unhealthy.",
> + rxq_idx);
> + dev->queue_mgmt_ops->ndo_queue_mem_free(dev, _mem);
This should be ->ndo_queue_mem_free(dev, old_mem).
Re: [linux-next:master] BUILD REGRESSION 6dc544b66971c7f9909ff038b62149105272d26a
On Wed, May 29, 2024 at 02:19:47AM +0800, kernel test robot wrote: > tree/branch: > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master > branch HEAD: 6dc544b66971c7f9909ff038b62149105272d26a Add linux-next > specific files for 20240528 > > Error/Warning reports: > > https://lore.kernel.org/oe-kbuild-all/202405282036.maedo54q-...@intel.com > https://lore.kernel.org/oe-kbuild-all/202405282148.jaf0flhu-...@intel.com > https://lore.kernel.org/oe-kbuild-all/202405282308.uezt6hqc-...@intel.com > > Error/Warning: (recently discovered and may have been fixed) > > drivers/dma-buf/udmabuf.c:45:(.text+0x140): undefined reference to > `vmf_insert_pfn' > fs/btrfs/fiemap.c:822:26: warning: 'last_extent_end' may be used > uninitialized [-Wmaybe-uninitialized] The report says it's gcc 13.2, that one I use (and expect others as well as it's a recent one) and we also have -Wmaybe-uninitialized enabled in fs/btrfs/ to catch such warnings. Yet this is reported on mips64, is there something special about that compiler+architecture? The warning is IMO a false positive, the maybe-uninitialized variable is passed as pointer but initialized on success and never used on failure. We can safely silence the warning by initializing the variable to 0 but this may be pointing to a problem with mips64+gcc namely because other compiler+host combinations do not warn abou that.
Re: [PATCH] udmabuf: add CONFIG_MMU dependency
Am 28.05.24 um 15:31 schrieb Arnd Bergmann:
From: Arnd Bergmann
There is no !CONFIG_MMU version of vmf_insert_pfn():
arm-linux-gnueabi-ld: drivers/dma-buf/udmabuf.o: in function `udmabuf_vm_fault':
udmabuf.c:(.text+0xaa): undefined reference to `vmf_insert_pfn'
Fixes: f7254e043ff1 ("udmabuf: use vmf_insert_pfn and VM_PFNMAP for handling
mmap")
Signed-off-by: Arnd Bergmann
---
drivers/dma-buf/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig
index e4dc53a36428..b46eb8a552d7 100644
--- a/drivers/dma-buf/Kconfig
+++ b/drivers/dma-buf/Kconfig
@@ -35,6 +35,7 @@ config UDMABUF
default n
depends on DMA_SHARED_BUFFER
depends on MEMFD_CREATE || COMPILE_TEST
+ depends on MMU
help
A driver to let userspace turn memfd regions into dma-bufs.
Qemu can use this to create host dmabufs for guest framebuffers.
Acked-by: David Hildenbrand
--
Thanks,
David / dhildenb
Re: [PATCH net-next v9 11/14] tcp: RX path for devmem TCP
On 2024-05-10 16:21, Mina Almasry wrote:
> +/* On error, returns the -errno. On success, returns number of bytes sent to
> the
> + * user. May not consume all of @remaining_len.
> + */
> +static int tcp_recvmsg_dmabuf(struct sock *sk, const struct sk_buff *skb,
> + unsigned int offset, struct msghdr *msg,
> + int remaining_len)
> +{
> + struct dmabuf_cmsg dmabuf_cmsg = { 0 };
> + struct tcp_xa_pool tcp_xa_pool;
> + unsigned int start;
> + int i, copy, n;
> + int sent = 0;
> + int err = 0;
> +
> + tcp_xa_pool.max = 0;
> + tcp_xa_pool.idx = 0;
> + do {
> + start = skb_headlen(skb);
> +
> + if (skb_frags_readable(skb)) {
> + err = -ENODEV;
> + goto out;
> + }
> +
> + /* Copy header. */
> + copy = start - offset;
> + if (copy > 0) {
> + copy = min(copy, remaining_len);
> +
> + n = copy_to_iter(skb->data + offset, copy,
> + >msg_iter);
> + if (n != copy) {
> + err = -EFAULT;
> + goto out;
> + }
> +
> + offset += copy;
> + remaining_len -= copy;
> +
> + /* First a dmabuf_cmsg for # bytes copied to user
> + * buffer.
> + */
> + memset(_cmsg, 0, sizeof(dmabuf_cmsg));
> + dmabuf_cmsg.frag_size = copy;
> + err = put_cmsg(msg, SOL_SOCKET, SO_DEVMEM_LINEAR,
> +sizeof(dmabuf_cmsg), _cmsg);
> + if (err || msg->msg_flags & MSG_CTRUNC) {
> + msg->msg_flags &= ~MSG_CTRUNC;
> + if (!err)
> + err = -ETOOSMALL;
> + goto out;
> + }
> +
> + sent += copy;
> +
> + if (remaining_len == 0)
> + goto out;
> + }
> +
> + /* after that, send information of dmabuf pages through a
> + * sequence of cmsg
> + */
> + for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
> + skb_frag_t *frag = _shinfo(skb)->frags[i];
> + struct net_iov *niov;
> + u64 frag_offset;
> + int end;
> +
> + /* !skb_frags_readable() should indicate that ALL the
> + * frags in this skb are dmabuf net_iovs. We're checking
> + * for that flag above, but also check individual frags
> + * here. If the tcp stack is not setting
> + * skb_frags_readable() correctly, we still don't want
> + * to crash here.
> + */
> + if (!skb_frag_net_iov(frag)) {
> + net_err_ratelimited("Found non-dmabuf skb with
> net_iov");
> + err = -ENODEV;
> + goto out;
> + }
> +
> + niov = skb_frag_net_iov(frag);
Sorry if we've already discussed this.
We have this additional hunk:
+ if (niov->pp->mp_ops != _devmem_ops) {
+ err = -ENODEV;
+ goto out;
+ }
In case one of our skbs end up here, skb_frag_is_net_iov() and
!skb_frags_readable(). Does this even matter? And if so then is there a
better way to distinguish between our two types of net_iovs?
Re: [PATCH v2] drm/bridge: analogix: remove unused struct 'bridge_init'
* Dmitry Baryshkov (dmitry.barysh...@linaro.org) wrote:
> On Mon, May 20, 2024 at 01:55:51PM +0100, li...@treblig.org wrote:
> > From: "Dr. David Alan Gilbert"
> >
> > commit 6a1688ae8794 ("drm/bridge: ptn3460: Convert to I2C driver model")
> > has dropped all the users of the struct bridge_init from the
> > exynos_dp_core, while retaining unused structure definition.
> > Later on the driver was reworked and the definition migrated
> > to the analogix_dp driver. Remove unused struct bridge_init definition.
> >
> > Signed-off-by: Dr. David Alan Gilbert
> > ---
> > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 -
> > 1 file changed, 5 deletions(-)
> >
>
> Reviewed-by: Dmitry Baryshkov
Thanks.
Dave
>
> --
> With best wishes
> Dmitry
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 1/6] drm/bridge: analogix: remove unused struct 'bridge_init'
* Dmitry Baryshkov (dmitry.barysh...@linaro.org) wrote:
> On Sun, May 19, 2024 at 10:43:44PM +, Dr. David Alan Gilbert wrote:
> > * Dmitry Baryshkov (dmitry.barysh...@linaro.org) wrote:
> > > On Sat, May 18, 2024 at 12:24:27AM +0100, li...@treblig.org wrote:
> > > > From: "Dr. David Alan Gilbert"
> > > >
> > > > 'bridge_init' is unused, I think following:
> > > > commit 6a1688ae8794 ("drm/bridge: ptn3460: Convert to I2C driver model")
> > > > (which is where a git --follow finds it)
> > > > Remove it.
> > >
> > > Please rephrase the commit message following guidelines in
> > > Documentation/process. Use Fixes tags if suitable.
> >
> > I specifically don't want to use Fixes in these set because
> > there's no need for someone to backport this to older
> > kernels that use the original, and many backporters
> > use 'Fixes' as an automated means to find stuff they should
> > backport.
> >
> > Other than that, is there something specific you think I've
> > missed?
>
> It's not about missing things. It's about a way it is written.
> Consider something like:
>
> The commit aaa ("drm/bridge: foo bar") has dropped all the users of
> the struct bridge_init from the exynos_dp_core, while retainng unused
> structure definition. Later on the driver was reworked and the
> definition migrated to the analogix_dp driver. Remove unused struct
> bridge_init definition.
OK, v2 sent with text close to that.
>
> >
> > (I'm also purposely being less certain here, because --follow
> > is showing it in a ptn3460 and I don't quite follow
> > why that changes it here).
>
> The mentioned commit is a correct one. Historically exynos_dp_core had
> been creating the ptn3460 bridge manually. Later on this was fixed in
> the ptn3640 driver and the code was dropped from exynos_dp_core.
Ah OK; remember I don't know the actual structure of these devices
or the history.
Dave
> >
> > Dave
> >
> > >
> > > >
> > > > Build tested.
> > > >
> > > > Signed-off-by: Dr. David Alan Gilbert
> > > > ---
> > > > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 -
> > > > 1 file changed, 5 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > > index df9370e0ff23..1e03f3525a92 100644
> > > > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > > @@ -36,11 +36,6 @@
> > > >
> > > > static const bool verify_fast_training;
> > > >
> > > > -struct bridge_init {
> > > > - struct i2c_client *client;
> > > > - struct device_node *node;
> > > > -};
> > > > -
> > > > static int analogix_dp_init_dp(struct analogix_dp_device *dp)
> > > > {
> > > > int ret;
> > > > --
> > > > 2.45.1
> > > >
> > >
> > > --
> > > With best wishes
> > > Dmitry
> > --
> > -Open up your eyes, open up your mind, open up your code ---
> > / Dr. David Alan Gilbert| Running GNU/Linux | Happy \
> > \dave @ treblig.org | | In Hex /
> > \ _|_ http://www.treblig.org |___/
>
> --
> With best wishes
> Dmitry
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 1/6] drm/bridge: analogix: remove unused struct 'bridge_init'
* Dmitry Baryshkov (dmitry.barysh...@linaro.org) wrote:
> On Sat, May 18, 2024 at 12:24:27AM +0100, li...@treblig.org wrote:
> > From: "Dr. David Alan Gilbert"
> >
> > 'bridge_init' is unused, I think following:
> > commit 6a1688ae8794 ("drm/bridge: ptn3460: Convert to I2C driver model")
> > (which is where a git --follow finds it)
> > Remove it.
>
> Please rephrase the commit message following guidelines in
> Documentation/process. Use Fixes tags if suitable.
I specifically don't want to use Fixes in these set because
there's no need for someone to backport this to older
kernels that use the original, and many backporters
use 'Fixes' as an automated means to find stuff they should
backport.
Other than that, is there something specific you think I've
missed?
(I'm also purposely being less certain here, because --follow
is showing it in a ptn3460 and I don't quite follow
why that changes it here).
Dave
>
> >
> > Build tested.
> >
> > Signed-off-by: Dr. David Alan Gilbert
> > ---
> > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 -
> > 1 file changed, 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > index df9370e0ff23..1e03f3525a92 100644
> > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > @@ -36,11 +36,6 @@
> >
> > static const bool verify_fast_training;
> >
> > -struct bridge_init {
> > - struct i2c_client *client;
> > - struct device_node *node;
> > -};
> > -
> > static int analogix_dp_init_dp(struct analogix_dp_device *dp)
> > {
> > int ret;
> > --
> > 2.45.1
> >
>
> --
> With best wishes
> Dmitry
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH net-next v9 04/14] netdev: support binding dma-buf to netdevice
On 2024-05-10 16:21, Mina Almasry wrote:
> -/* Stub */
> int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
> {
> - return 0;
> + struct nlattr *tb[ARRAY_SIZE(netdev_queue_dmabuf_nl_policy)];
> + struct net_devmem_dmabuf_binding *out_binding;
> + struct list_head *sock_binding_list;
> + u32 ifindex, dmabuf_fd, rxq_idx;
> + struct net_device *netdev;
> + struct sk_buff *rsp;
> + struct nlattr *attr;
> + int rem, err = 0;
> + void *hdr;
> +
> + if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_DEV_IFINDEX) ||
> + GENL_REQ_ATTR_CHECK(info, NETDEV_A_BIND_DMABUF_DMABUF_FD) ||
> + GENL_REQ_ATTR_CHECK(info, NETDEV_A_BIND_DMABUF_QUEUES))
> + return -EINVAL;
> +
> + ifindex = nla_get_u32(info->attrs[NETDEV_A_DEV_IFINDEX]);
> + dmabuf_fd = nla_get_u32(info->attrs[NETDEV_A_BIND_DMABUF_DMABUF_FD]);
> +
> + rtnl_lock();
> +
> + netdev = __dev_get_by_index(genl_info_net(info), ifindex);
> + if (!netdev) {
> + err = -ENODEV;
> + goto err_unlock;
> + }
> +
> + err = net_devmem_bind_dmabuf(netdev, dmabuf_fd, _binding);
> + if (err)
> + goto err_unlock;
> +
> + nla_for_each_attr(attr, genlmsg_data(info->genlhdr),
> + genlmsg_len(info->genlhdr), rem) {
> + if (nla_type(attr) != NETDEV_A_BIND_DMABUF_QUEUES)
> + continue;
> +
> + err = nla_parse_nested(
> + tb, ARRAY_SIZE(netdev_queue_dmabuf_nl_policy) - 1, attr,
> + netdev_queue_dmabuf_nl_policy, info->extack);
> + if (err < 0)
> + goto err_unbind;
> +
> + rxq_idx = nla_get_u32(tb[NETDEV_A_QUEUE_DMABUF_IDX]);
> + if (rxq_idx >= netdev->num_rx_queues) {
> + err = -ERANGE;
> + goto err_unbind;
> + }
net_devmem_bind_dmabuf_to_queue() checks for rxq_idx >=
netdev->num_rx_queues as well. I'd say remove the one in
netdev_nl_bind_rx_doit().
Also we may want a generic netdev function e.g. netdev_rx_queue_set_mp()
since I need the same functionality.
> +
> + err = net_devmem_bind_dmabuf_to_queue(netdev, rxq_idx,
> + out_binding);
> + if (err)
> + goto err_unbind;
> + }
> +
> + sock_binding_list = genl_sk_priv_get(_nl_family,
> + NETLINK_CB(skb).sk);
> + if (IS_ERR(sock_binding_list)) {
> + err = PTR_ERR(sock_binding_list);
> + goto err_unbind;
> + }
> +
> + list_add(_binding->list, sock_binding_list);
> +
> + rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> + if (!rsp) {
> + err = -ENOMEM;
> + goto err_unbind;
> + }
> +
> + hdr = genlmsg_iput(rsp, info);
> + if (!hdr) {
> + err = -EMSGSIZE;
> + goto err_genlmsg_free;
> + }
> +
> + nla_put_u32(rsp, NETDEV_A_BIND_DMABUF_DMABUF_ID, out_binding->id);
> + genlmsg_end(rsp, hdr);
> +
> + rtnl_unlock();
> +
> + return genlmsg_reply(rsp, info);
> +
> +err_genlmsg_free:
> + nlmsg_free(rsp);
> +err_unbind:
> + net_devmem_unbind_dmabuf(out_binding);
> +err_unlock:
> + rtnl_unlock();
> + return err;
> }
Re: [PATCH net-next v9 04/14] netdev: support binding dma-buf to netdevice
On 2024-05-10 16:21, Mina Almasry wrote:
> +void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding)
> +{
> + struct netdev_rx_queue *rxq;
> + unsigned long xa_idx;
> + unsigned int rxq_idx;
> +
> + if (!binding)
> + return;
> +
> + if (binding->list.next)
> + list_del(>list);
> +
> + xa_for_each(>bound_rxq_list, xa_idx, rxq) {
> + if (rxq->mp_params.mp_priv == binding) {
> + /* We hold the rtnl_lock while binding/unbinding
> + * dma-buf, so we can't race with another thread that
> + * is also modifying this value. However, the page_pool
> + * may read this config while it's creating its
> + * rx-queues. WRITE_ONCE() here to match the
> + * READ_ONCE() in the page_pool.
> + */
> + WRITE_ONCE(rxq->mp_params.mp_ops, NULL);
> + WRITE_ONCE(rxq->mp_params.mp_priv, NULL);
> +
> + rxq_idx = get_netdev_rx_queue_index(rxq);
> +
> + netdev_rx_queue_restart(binding->dev, rxq_idx);
What if netdev_rx_queue_restart() fails? Depending on where it failed, a
queue might still be filled from struct net_devmem_dmabuf_binding. This
is one downside of the current situation with netdev_rx_queue_restart()
needing to do allocations each time.
Perhaps a full reset if individual queue restart fails?
Re: [PATCH net-next v9 05/14] netdev: netdevice devmem allocator
On 2024-05-10 16:21, Mina Almasry wrote:
> +/* This returns the absolute dma_addr_t calculated from
> + * net_iov_owner(niov)->owner->base_dma_addr, not the page_pool-owned
> + * niov->dma_addr.
> + *
> + * The absolute dma_addr_t is a dma_addr_t that is always uncompressed.
> + *
> + * The page_pool-owner niov->dma_addr is the absolute dma_addr compressed
> into
> + * an unsigned long. Special handling is done when the unsigned long is
> 32-bit
> + * but the dma_addr_t is 64-bit.
> + *
> + * In general code looking for the dma_addr_t should use net_iov_dma_addr(),
> + * while page_pool code looking for the unsigned long dma_addr which mirrors
> + * the field in struct page should use niov->dma_addr.
> + */
> +static inline dma_addr_t net_iov_dma_addr(const struct net_iov *niov)
> +{
> + struct dmabuf_genpool_chunk_owner *owner = net_iov_owner(niov);
> +
> + return owner->base_dma_addr +
> +((dma_addr_t)net_iov_idx(niov) << PAGE_SHIFT);
> +}
This part feels like devmem TCP specific, yet the function is in
netmem.h. Please consider moving it into devmem.{h,c} which makes it
less likely that people not reading your comment will try using it.
> +
> +static inline struct net_devmem_dmabuf_binding *
> +net_iov_binding(const struct net_iov *niov)
> +{
> + return net_iov_owner(niov)->binding;
> +}
> +
> /* netmem */
>
> /**
> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index d82f92d7cf9ce..1f90e23a81441 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c
> @@ -54,6 +54,42 @@ void __net_devmem_dmabuf_binding_free(struct
> net_devmem_dmabuf_binding *binding)
> kfree(binding);
> }
>
> +struct net_iov *
> +net_devmem_alloc_dmabuf(struct net_devmem_dmabuf_binding *binding)
> +{
> + struct dmabuf_genpool_chunk_owner *owner;
> + unsigned long dma_addr;
> + struct net_iov *niov;
> + ssize_t offset;
> + ssize_t index;
> +
> + dma_addr = gen_pool_alloc_owner(binding->chunk_pool, PAGE_SIZE,
> + (void **));
> + if (!dma_addr)
> + return NULL;
> +
> + offset = dma_addr - owner->base_dma_addr;
> + index = offset / PAGE_SIZE;
> + niov = >niovs[index];
> +
> + niov->dma_addr = 0;
> +
> + net_devmem_dmabuf_binding_get(binding);
> +
> + return niov;
> +}
> +
> +void net_devmem_free_dmabuf(struct net_iov *niov)
> +{
> + struct net_devmem_dmabuf_binding *binding = net_iov_binding(niov);
> + unsigned long dma_addr = net_iov_dma_addr(niov);
> +
> + if (gen_pool_has_addr(binding->chunk_pool, dma_addr, PAGE_SIZE))
> + gen_pool_free(binding->chunk_pool, dma_addr, PAGE_SIZE);
> +
> + net_devmem_dmabuf_binding_put(binding);
> +}
> +
> /* Protected by rtnl_lock() */
> static DEFINE_XARRAY_FLAGS(net_devmem_dmabuf_bindings, XA_FLAGS_ALLOC1);
>
Re: [PATCH 1/6] drm/bridge: analogix: remove unused struct 'bridge_init'
(oops the patch numbering in these 3 are wrong, they're all independent
patches)
Dave
* li...@treblig.org (li...@treblig.org) wrote:
> From: "Dr. David Alan Gilbert"
>
> 'bridge_init' is unused, I think following:
> commit 6a1688ae8794 ("drm/bridge: ptn3460: Convert to I2C driver model")
> (which is where a git --follow finds it)
> Remove it.
>
> Build tested.
>
> Signed-off-by: Dr. David Alan Gilbert
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index df9370e0ff23..1e03f3525a92 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -36,11 +36,6 @@
>
> static const bool verify_fast_training;
>
> -struct bridge_init {
> - struct i2c_client *client;
> - struct device_node *node;
> -};
> -
> static int analogix_dp_init_dp(struct analogix_dp_device *dp)
> {
> int ret;
> --
> 2.45.1
>
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
RE: [Linaro-mm-sig] Re: [PATCH] epoll: try to be a _bit_ better about file lifetimes
From: Christian Brauner > Sent: 10 May 2024 11:55 > > > For the uapi issue you describe below my take would be that we should just > > try, and hope that everyone's been dutifully using O_CLOEXEC. But maybe > > I'm biased from the gpu world, where we've been hammering it in that > > "O_CLOEXEC or bust" mantra since well over a decade. Really the only valid > > Oh, we're very much on the same page. All new file descriptor types that > I've added over the years are O_CLOEXEC by default. IOW, you need to > remove O_CLOEXEC explicitly (see pidfd as an example). And imho, any new > fd type that's added should just be O_CLOEXEC by default. For fd a shell redirect creates you may want so be able to say 'this fd will have O_CLOEXEC set after the next exec'. Also (possibly) a flag that can't be cleared once set and that gets kept by dup() etc. But maybe that is excessive? I've certainly used: # ip netns exec ns command 3
RE: [PATCH] dmabuf: fix dmabuf file poll uaf issue
From: Christian König
> Sent: 07 May 2024 15:05
...
> I actually have been telling people to (ab)use the epoll behavior to
> check if two file descriptors point to the same underlying file when
> KCMP isn't available.
In what way?
You can add both fd to the same epoll fd.
Relying on the implicit EPOLL_CTL_DEL not happening until both fd are
closed is a recipe for disaster.
(And I can't see an obvious way of testing it.)
Q6/A6 on epoll(7) should always have had a caveat that it is an
'implementation detail' and shouldn't be relied on.
(it is written as a 'beware of' ...)
The other point is that there are two ways to get multiple fd that
reference the same underlying file.
dup() fork() etc share the file offset, but open("/dev/fd/n") adds
a reference count later and has a separate file offset.
I don't know which structure epoll is using, but I suspect it is
the former.
So it may not tell you what you want to know.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
RE: get_file() unsafe under epoll (was Re: [syzbot] [fs?] [io-uring?] general protection fault in __ep_remove)
From: Linus Torvalds > Sent: 06 May 2024 19:18 ... > Which is why I applied the minimal patch for just "refcount over > vfs_poll()", and am just mentioning my suggestion in the hope that > some eager beaver would like to see how painful it would do to make > the bigger surgery... I wonder if I can work out how it (doesn't) currently work... David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
RE: [PATCH] epoll: try to be a _bit_ better about file lifetimes
From: Christian Brauner > Sent: 06 May 2024 09:45 > > > The fact is, it's not dma-buf that is violating any rules. It's epoll. > > I agree that epoll() not taking a reference on the file is at least > unexpected and contradicts the usual code patterns for the sake of > performance and that it very likely is the case that most callers of > f_op->poll() don't know this. > > Note, I cleary wrote upthread that I'm ok to do it like you suggested > but raised two concerns a) there's currently only one instance of > prolonged @file lifetime in f_op->poll() afaict and b) that there's > possibly going to be some performance impact on epoll(). > > So it's at least worth discussing what's more important because epoll() > is very widely used and it's not that we haven't favored performance > before. > > But you've already said that you aren't concerned with performance on > epoll() upthread. So afaict then there's really not a lot more to > discuss other than take the patch and see whether we get any complaints. Surely there isn't a problem with epoll holding a reference to the file structure - it isn't really any different to a dup(). 'All' that needs to happen is that the 'magic' that makes epoll() remove files on the last fput happen when the close is done. I'm sure there are horrid locking issues it that code (separate from it calling ->poll() after ->release()) eg if you call close() concurrently with EPOLL_CTL_ADD. I'm not at all sure it would have mattered if epoll kept the file open. But it can't do that because it is documented not to. As well as poll/select holding a reference to all their fd for the duration of the system call, a successful mmap() holds a reference until the pages are all unmapped - usually by process exit. We (dayjob) have code that uses epoll() to monitor large numbers of UDP sockets. I was doing some tests (trying to) receive RTP (audio) data concurrently on 1 sockets with typically one packet every 20ms. There are 1 associated RCTP sockets that are usually idle. A more normal limit would be 1000 RTP sockets. All the data needs to go into a single (multithreaded) process. Just getting all the packets queued on the sockets was non-trivial. epoll is about the only way to actually read the data. (That needed multiple epoll fd so each thread could process all the events from one epoll fd then look for another unprocessed fd.) David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [Regression] 6.9.0: WARNING: workqueue: WQ_MEM_RECLAIM ttm:ttm_bo_delayed_delete [ttm] is flushing !WQ_MEM_RECLAIM events:qxl_gc_work [qxl]
The kernel warning still shows up in 6.9.0-rc7. (I think 4 high load processes on a 2-Core VM could easily trigger the kernel warning.) Thanks David
Re: [RFC PATCH net-next v8 07/14] page_pool: devmem support
On 2024-05-01 00:55, Christoph Hellwig wrote:
> On Fri, Apr 26, 2024 at 05:17:52PM -0700, David Wei wrote:
>> On 2024-04-02 5:20 pm, Mina Almasry wrote:
>>> @@ -69,20 +106,26 @@ net_iov_binding(const struct net_iov *niov)
>>> */
>>> typedef unsigned long __bitwise netmem_ref;
>>>
>>> +static inline bool netmem_is_net_iov(const netmem_ref netmem)
>>> +{
>>> +#if defined(CONFIG_PAGE_POOL) && defined(CONFIG_DMA_SHARED_BUFFER)
>>
>> I am guessing you added this to try and speed up the fast path? It's
>> overly restrictive for us since we do not need dmabuf necessarily. I
>> spent a bit too much time wondering why things aren't working only to
>> find this :(
>
> So what else do you need? I was assured last round that nothing but
> dmabuf and potentially the huge page case (that really just is the page
> provider) would get added.
I'm using userspace memory so having this gated behind
CONFIG_DMA_SHARED_BUFFER doesn't make sense for us.
>
>>
> ---end quoted text---
RE: [PATCH v2] epoll: be better about file lifetimes
From: Linus Torvalds
> Sent: 05 May 2024 18:56
>
> epoll can call out to vfs_poll() with a file pointer that may race with
> the last 'fput()'. That would make f_count go down to zero, and while
> the ep->mtx locking means that the resulting file pointer tear-down will
> be blocked until the poll returns, it means that f_count is already
> dead, and any use of it won't actually get a reference to the file any
> more: it's dead regardless.
>
> Make sure we have a valid ref on the file pointer before we call down to
> vfs_poll() from the epoll routines.
How much is the extra pair of atomics going to hurt performance?
IIRC a lot of work was done to (try to) make epoll lockless.
Perhaps the 'hook' into epoll (usually) from sys_close should be
done before any of the references are removed?
(Which is different from Q6/A6 in man epoll - but that seems to be
documenting a bug!)
Then the ->poll() callback can't happen (assuming it is properly
locked) after the ->release() one.
It seems better to add extra atomics to the close/final-fput path
rather than ever ->poll() call epoll makes.
I can get extra references to a driver by dup() open("/dev/fd/n")
and mmap() - but epoll is definitely fd based.
(Which may be why it has the fd number in its data.)
Is there another race between EPOLL_CTL_ADD and close() (from a
different thread)?
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
[Regression] 6.9.0: WARNING: workqueue: WQ_MEM_RECLAIM ttm:ttm_bo_delayed_delete [ttm] is flushing !WQ_MEM_RECLAIM events:qxl_gc_work [qxl]
:36:04 2024] worker_thread+0x273/0x390
[Mon Apr 29 21:36:04 2024] ? __pfx_worker_thread+0x10/0x10
[Mon Apr 29 21:36:04 2024] kthread+0xdd/0x110
[Mon Apr 29 21:36:04 2024] ? __pfx_kthread+0x10/0x10
[Mon Apr 29 21:36:04 2024] ret_from_fork+0x30/0x50
[Mon Apr 29 21:36:04 2024] ? __pfx_kthread+0x10/0x10
[Mon Apr 29 21:36:04 2024] ret_from_fork_asm+0x1a/0x30
[Mon Apr 29 21:36:04 2024]
[Mon Apr 29 21:36:04 2024] ---[ end trace ]---
I find that the exact warning message mentioned in
https://lore.kernel.org/lkml/20240404181448.1643-1-dreaming.about.electric.sh...@gmail.com/T/#m8c2ecc83ebba8717b1290ec28d4dc15f2fa595d5
And confirmed that the warning is caused by
07ed11afb68d94eadd4ffc082b97c2331307c5ea and reverting it can fix.
It seems that under heavy load, qxl_queue_garbage_collect would be called within
a WQ_MEM_RECLAIM worker, and flush qxl_gc_work which is a
!WQ_MEM_RECLAIM worker. This will trigger the kernel WARNING by
check_flush_dependency.
And I tried following changes, setting flush flag to false.
The warning is gone, but I am not sure whether there is any other side-effect,
especially the issue mentioned in
https://lore.kernel.org/lkml/20240404181448.1643-2-dreaming.about.electric.sh...@gmail.com/T/#m988ffad2000c794dcfdab7e60b03db93d8726391
Signed-off-by: David Wang <00107...@163.com>
---
drivers/gpu/drm/qxl/qxl_release.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index 9febc8b73f09..f372085c5aad 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -76,7 +76,7 @@ static long qxl_fence_wait(struct dma_fence *fence, bool intr,
qxl_io_notify_oom(qdev);
for (count = 0; count < 11; count++) {
- if (!qxl_queue_garbage_collect(qdev, true))
+ if (!qxl_queue_garbage_collect(qdev, false))
break;
if (dma_fence_is_signaled(fence))
--
2.39.2
David
Re: [PATCH v2 1/2] drm/nouveau/firmware: Fix SG_DEBUG error with nvkm_firmware_ctor()
> V2: > * Fixup explanation as the prior one was bogus For v2 of both patches, Reviewed-by: Dave Airlie Please apply to drm-misc-fixes Dave.
Re: [RFC PATCH net-next v8 07/14] page_pool: devmem support
On 2024-04-02 5:20 pm, Mina Almasry wrote:
> @@ -69,20 +106,26 @@ net_iov_binding(const struct net_iov *niov)
> */
> typedef unsigned long __bitwise netmem_ref;
>
> +static inline bool netmem_is_net_iov(const netmem_ref netmem)
> +{
> +#if defined(CONFIG_PAGE_POOL) && defined(CONFIG_DMA_SHARED_BUFFER)
I am guessing you added this to try and speed up the fast path? It's
overly restrictive for us since we do not need dmabuf necessarily. I
spent a bit too much time wondering why things aren't working only to
find this :(
Re: [RFC PATCH net-next v8 04/14] netdev: support binding dma-buf to netdevice
On 2024-04-02 5:20 pm, Mina Almasry wrote: > Add a netdev_dmabuf_binding struct which represents the > dma-buf-to-netdevice binding. The netlink API will bind the dma-buf to > rx queues on the netdevice. On the binding, the dma_buf_attach > & dma_buf_map_attachment will occur. The entries in the sg_table from > mapping will be inserted into a genpool to make it ready > for allocation. > > The chunks in the genpool are owned by a dmabuf_chunk_owner struct which > holds the dma-buf offset of the base of the chunk and the dma_addr of > the chunk. Both are needed to use allocations that come from this chunk. > > We create a new type that represents an allocation from the genpool: > net_iov. We setup the net_iov allocation size in the > genpool to PAGE_SIZE for simplicity: to match the PAGE_SIZE normally > allocated by the page pool and given to the drivers. > > The user can unbind the dmabuf from the netdevice by closing the netlink > socket that established the binding. We do this so that the binding is > automatically unbound even if the userspace process crashes. > > The binding and unbinding leaves an indicator in struct netdev_rx_queue > that the given queue is bound, but the binding doesn't take effect until > the driver actually reconfigures its queues, and re-initializes its page > pool. > > The netdev_dmabuf_binding struct is refcounted, and releases its > resources only when all the refs are released. > > Signed-off-by: Willem de Bruijn > Signed-off-by: Kaiyuan Zhang > Signed-off-by: Mina Almasry > > --- > > v8: > - move dmabuf_devmem_ops usage to later patch to avoid patch-by-patch > build error. > > v7: > - Use IS_ERR() instead of IS_ERR_OR_NULL() for the dma_buf_get() return > value. > - Changes netdev_* naming in devmem.c to net_devmem_* (Yunsheng). > - DMA_BIDIRECTIONAL -> DMA_FROM_DEVICE (Yunsheng). > - Added a comment around recovering of the old rx queue in > net_devmem_restart_rx_queue(), and added freeing of old_mem if the > restart of the old queue fails. (Yunsheng). > - Use kernel-family sock-priv (Jakub). > - Put pp_memory_provider_params in netdev_rx_queue instead of the > dma-buf specific binding (Pavel & David). > - Move queue management ops to queue_mgmt_ops instead of netdev_ops > (Jakub). > - Remove excess whitespaces (Jakub). > - Use genlmsg_iput (Jakub). > > v6: > - Validate rx queue index > - Refactor new functions into devmem.c (Pavel) > > v5: > - Renamed page_pool_iov to net_iov, and moved that support to devmem.h > or netmem.h. > > v1: > > - Introduce devmem.h instead of bloating netdevice.h (Jakub) > - ENOTSUPP -> EOPNOTSUPP (checkpatch.pl I think) > - Remove unneeded rcu protection for binding->list (rtnl protected) > - Removed extraneous err_binding_put: label. > - Removed dma_addr += len (Paolo). > - Don't override err on netdev_bind_dmabuf_to_queue failure. > - Rename devmem -> dmabuf (David). > - Add id to dmabuf binding (David/Stan). > - Fix missing xa_destroy bound_rq_list. > - Use queue api to reset bound RX queues (Jakub). > - Update netlink API for rx-queue type (tx/re) (Jakub). > > RFC v3: > - Support multi rx-queue binding > > --- > Documentation/netlink/specs/netdev.yaml | 4 + > include/net/devmem.h| 111 + > include/net/netdev_rx_queue.h | 2 + > include/net/netmem.h| 10 + > include/net/page_pool/types.h | 5 + > net/core/Makefile | 2 +- > net/core/dev.c | 3 + > net/core/devmem.c | 303 > net/core/netdev-genl-gen.c | 4 + > net/core/netdev-genl-gen.h | 4 + > net/core/netdev-genl.c | 105 +++- > 11 files changed, 550 insertions(+), 3 deletions(-) > create mode 100644 include/net/devmem.h > create mode 100644 net/core/devmem.c > > diff --git a/Documentation/netlink/specs/netdev.yaml > b/Documentation/netlink/specs/netdev.yaml > index 275d1faa87a6..bf4e58dfe9dd 100644 > --- a/Documentation/netlink/specs/netdev.yaml > +++ b/Documentation/netlink/specs/netdev.yaml > @@ -550,6 +550,10 @@ operations: > - tx-packets > - tx-bytes > > +kernel-family: > + headers: [ "linux/list.h"] > + sock-priv: struct list_head > + > mcast-groups: >list: > - > diff --git a/include/net/devmem.h b/include/net/devmem.h > new file mode 100644 > index ..fa03bdabdffd > --- /dev/null > +++ b/include/net/devmem.h > @@ -0,0 +1,111 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > +
[PATCH v4 2/2] drm/panel: Add driver for EDO RM69380 OLED panel
Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a
Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro 2021.
Reviewed-by: Dmitry Baryshkov
Signed-off-by: David Wronek
---
drivers/gpu/drm/panel/Kconfig | 12 +
drivers/gpu/drm/panel/Makefile| 1 +
drivers/gpu/drm/panel/panel-raydium-rm69380.c | 344 ++
3 files changed, 357 insertions(+)
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 154f5bf82980..e2a66c21349f 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -542,6 +542,18 @@ config DRM_PANEL_RAYDIUM_RM692E5
Say Y here if you want to enable support for Raydium RM692E5-based
display panels, such as the one found in the Fairphone 5 smartphone.
+config DRM_PANEL_RAYDIUM_RM69380
+ tristate "Raydium RM69380-based DSI panel"
+ depends on OF && GPIOLIB
+ depends on DRM_MIPI_DSI
+ depends on BACKLIGHT_CLASS_DEVICE
+ help
+ Say Y here if you want to enable support for Raydium RM69380-based
+ display panels.
+
+ This panel controller can be found in the Lenovo Xiaoxin Pad Pro 2021
+ in combination with an EDO OLED panel.
+
config DRM_PANEL_RONBO_RB070D30
tristate "Ronbo Electronics RB070D30 panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 24a02655d726..e2a2807d4ef0 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) +=
panel-raspberrypi-touchscreen
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM692E5) += panel-raydium-rm692e5.o
+obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM69380) += panel-raydium-rm69380.o
obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o
obj-$(CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20) += panel-samsung-atna33xc20.o
obj-$(CONFIG_DRM_PANEL_SAMSUNG_DB7430) += panel-samsung-db7430.o
diff --git a/drivers/gpu/drm/panel/panel-raydium-rm69380.c
b/drivers/gpu/drm/panel/panel-raydium-rm69380.c
new file mode 100644
index ..4dca6802faef
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-raydium-rm69380.c
@@ -0,0 +1,344 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Generated with linux-mdss-dsi-panel-driver-generator from vendor device
tree.
+ * Copyright (c) 2024 David Wronek
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include
+#include
+#include
+#include
+
+struct rm69380_panel {
+ struct drm_panel panel;
+ struct mipi_dsi_device *dsi[2];
+ struct regulator_bulk_data supplies[2];
+ struct gpio_desc *reset_gpio;
+};
+
+static inline
+struct rm69380_panel *to_rm69380_panel(struct drm_panel *panel)
+{
+ return container_of(panel, struct rm69380_panel, panel);
+}
+
+static void rm69380_reset(struct rm69380_panel *ctx)
+{
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ usleep_range(15000, 16000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+ usleep_range(1, 11000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ msleep(30);
+}
+
+static int rm69380_on(struct rm69380_panel *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi[0];
+ struct device *dev = >dev;
+ int ret;
+
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+ if (ctx->dsi[1])
+ ctx->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd4);
+ mipi_dsi_dcs_write_seq(dsi, 0x00, 0x80);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd0);
+ mipi_dsi_dcs_write_seq(dsi, 0x48, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0x26);
+ mipi_dsi_dcs_write_seq(dsi, 0x75, 0x3f);
+ mipi_dsi_dcs_write_seq(dsi, 0x1d, 0x1a);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x28);
+ mipi_dsi_dcs_write_seq(dsi, 0xc2, 0x08);
+
+ ret = mipi_dsi_dcs_set_tear_on(dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set tear on: %d\n", ret);
+ return ret;
+ }
+
+ ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
+ return ret;
+ }
+ msleep(20);
+
+ ret = mipi_dsi_dcs_set_display_on(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set display on: %d\n", ret);
+ return ret;
+ }
+ msleep(36);
+
+ return 0;
+}
+
+static int rm69380_off(struct rm69380_panel *ctx)
+{
+ struct mipi_dsi_device *ds
[PATCH v4 0/2] Add driver for Raydium RM69380-based DSI panels
This patch adds support the 2560x1600@90Hz dual DSI command mode panel by EDO in combination with a Raydium RM69380 driver IC. This driver IC can be found in the following devices: * Lenovo Xiaoxin Pad Pro 2021 (TB-J716F) with EDO panel * Lenovo Tab P11 Pro (TB-J706F) with EDO panel * Robo & Kala 2-in-1 Laptop with Sharp panel Signed-off-by: David Wronek --- Changes in v4: - Fix up Kconfig - Switch to devm_mipi_dsi_attach to benefit from automatic detaching - Initialize panel at a lower brightness - Dropped debug logs - Signify second DSI interface in mipi_dsi_device_info as "RM69380 DSI1" - Changed 'addtionalProperties' to 'unevaluatedProperties' in dt-binding - Dropped 'ports' in dt-binding - Link to v3: https://lore.kernel.org/r/20240416-raydium-rm69380-driver-v3-0-21600ac4c...@mainlining.org Changes in v3: - Removed unneeded curly brackets from some if statments - Fix error handling code in probe function - Include video/mipi_display.h and make use of MIPI command definitions - Removed DRM_MODE_TYPE_PREFERRED - Dropped 'prepared' bool entirely - Register second DSI host using mipi_dsi_device_register_full() - Link to v2: https://lore.kernel.org/r/20240415-raydium-rm69380-driver-v2-0-524216461...@mainlining.org Changes in v2: - Fixed typo in Kconfig - Removed ctx->prepared = true; in prepare function - Switched to drm_connector_helper_get_modes_fixed in get_modes function - Changed dev_notice() to dev_dbg() - Add description for compatible and reset-gpio in the dt-binding - Always require 'ports' node in the dt-binding regardless of compatible - Link to v1: https://lore.kernel.org/r/20240414-raydium-rm69380-driver-v1-0-5e86ba249...@mainlining.org --- David Wronek (2): dt-bindings: display: panel: Add Raydium RM69380 drm/panel: Add driver for EDO RM69380 OLED panel .../bindings/display/panel/raydium,rm69380.yaml| 89 ++ drivers/gpu/drm/panel/Kconfig | 12 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-raydium-rm69380.c | 344 + 4 files changed, 446 insertions(+) --- base-commit: 4eab358930711bbeb85bf5ee267d0d42d3394c2c change-id: 20240414-raydium-rm69380-driver-47f22b6f24fe Best regards, -- David Wronek
[PATCH v4 1/2] dt-bindings: display: panel: Add Raydium RM69380
Raydium RM69380 is a display driver IC used to drive OLED DSI panels.
Add a dt-binding for it.
Signed-off-by: David Wronek
---
Note:
Depends on commit 48a516363e29 ("dt-bindings: display: panel: add common
dual-link schema")
---
.../bindings/display/panel/raydium,rm69380.yaml| 89 ++
1 file changed, 89 insertions(+)
diff --git
a/Documentation/devicetree/bindings/display/panel/raydium,rm69380.yaml
b/Documentation/devicetree/bindings/display/panel/raydium,rm69380.yaml
new file mode 100644
index ..b17765b2b351
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/raydium,rm69380.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/raydium,rm69380.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Raydium RM69380-based DSI display panels
+
+maintainers:
+ - David Wronek
+
+description:
+ The Raydium RM69380 is a generic DSI panel IC used to control
+ OLED panels.
+
+allOf:
+ - $ref: panel-common-dual.yaml#
+
+properties:
+ compatible:
+items:
+ - enum:
+ - lenovo,j716f-edo-rm69380
+ - const: raydium,rm69380
+description: This indicates the panel manufacturer of the panel
+ that is in turn using the RM69380 panel driver. The compatible
+ string determines how the RM69380 panel driver shall be configured
+ to work with the indicated panel. The raydium,rm69380 compatible shall
+ always be provided as a fallback.
+
+ avdd-supply:
+description: Analog voltage rail
+
+ vddio-supply:
+description: I/O voltage rail
+
+ reset-gpios:
+maxItems: 1
+description: phandle of gpio for reset line - This should be active low
+
+ reg: true
+
+required:
+ - compatible
+ - reg
+ - avdd-supply
+ - vddio-supply
+ - reset-gpios
+
+unevaluatedProperties: false
+
+examples:
+ - |
+#include
+
+dsi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+panel@0 {
+compatible = "lenovo,j716f-edo-rm69380", "raydium,rm69380";
+reg = <0>;
+
+avdd-supply = <_avdd_regulator>;
+vddio-supply = <_l14a>;
+reset-gpios = < 75 GPIO_ACTIVE_LOW>;
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+
+port@0 {
+reg = <0>;
+panel_in_0: endpoint {
+remote-endpoint = <_dsi0_out>;
+};
+};
+
+port@1 {
+reg = <1>;
+panel_in_1: endpoint {
+remote-endpoint = <_dsi1_out>;
+};
+};
+};
+};
+};
+
+...
--
2.44.0
Re: [PATCH v3 2/2] drm/panel: Add driver for EDO RM69380 OLED panel
W dniu 2024-04-16 22:52, Marijn Suijten napisał(a):
On 2024-04-16 20:30:49, David Wronek wrote:
Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a
Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro
2021.
Signed-off-by: David Wronek
---
drivers/gpu/drm/panel/Kconfig | 14 +
drivers/gpu/drm/panel/Makefile| 1 +
drivers/gpu/drm/panel/panel-raydium-rm69380.c | 367
++
3 files changed, 382 insertions(+)
diff --git a/drivers/gpu/drm/panel/Kconfig
b/drivers/gpu/drm/panel/Kconfig
index 154f5bf82980..5b3eeb93b1a2 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -542,6 +542,20 @@ config DRM_PANEL_RAYDIUM_RM692E5
Say Y here if you want to enable support for Raydium RM692E5-based
display panels, such as the one found in the Fairphone 5
smartphone.
+config DRM_PANEL_RAYDIUM_RM69380
+ tristate "Raydium RM69380-based DSI panel"
+ depends on BACKLIGHT_CLASS_DEVICE
+ depends on DRM_DISPLAY_DP_HELPER
"DRM DisplayPort helpers"
But you said that this is a DSI device?
Looking in -next from yesterday, Raydium-RM692E5 and Visionox-R66451
get this
wrong as well.
+ depends on DRM_DISPLAY_HELPER
This also looks unused? The only helpers in the non-DP non-HDMI helper
points
to more DP AUX code.
+ depends on DRM_MIPI_DSI
+ depends on OF
As I've shown in the SOFEF00 cleanup patch, devm_gpiod_get() is used
which is
behind GPIOLIB. This should probably be a dependency.
+ help
+ Say Y here if you want to enable support for Raydium RM69380-based
+ display panels.
+
+ This panel controller can be found in the Lenovo Xiaoxin Pad Pro
2021
+ in combination with an EDO OLED panel.
+
config DRM_PANEL_RONBO_RB070D30
tristate "Ronbo Electronics RB070D30 panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile
b/drivers/gpu/drm/panel/Makefile
index 24a02655d726..e2a2807d4ef0 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) +=
panel-raspberrypi-touchscreen
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM692E5) += panel-raydium-rm692e5.o
+obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM69380) += panel-raydium-rm69380.o
obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o
obj-$(CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20) +=
panel-samsung-atna33xc20.o
obj-$(CONFIG_DRM_PANEL_SAMSUNG_DB7430) += panel-samsung-db7430.o
diff --git a/drivers/gpu/drm/panel/panel-raydium-rm69380.c
b/drivers/gpu/drm/panel/panel-raydium-rm69380.c
new file mode 100644
index ..f89230c969b7
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-raydium-rm69380.c
@@ -0,0 +1,367 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Generated with linux-mdss-dsi-panel-driver-generator from vendor
device tree.
+ * Copyright (c) 2024 David Wronek
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include
+#include
+#include
+#include
+
+struct rm69380_panel {
+ struct drm_panel panel;
+ struct mipi_dsi_device *dsi[2];
+ struct regulator_bulk_data supplies[2];
+ struct gpio_desc *reset_gpio;
+};
+
+static inline
+struct rm69380_panel *to_rm69380_panel(struct drm_panel *panel)
+{
+ return container_of(panel, struct rm69380_panel, panel);
+}
+
+static void rm69380_reset(struct rm69380_panel *ctx)
+{
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ usleep_range(15000, 16000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+ usleep_range(1, 11000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ msleep(30);
+}
+
+static int rm69380_on(struct rm69380_panel *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi[0];
+ struct device *dev = >dev;
+ int ret;
+
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+ if (ctx->dsi[1])
+ ctx->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd4);
+ mipi_dsi_dcs_write_seq(dsi, 0x00, 0x80);
Is this MIPI_DCS_NOP? Strange to see that with a parameter.
I don't think it is. 0xfe is the command to switch the 'manufacture
command set'. As you can see, it switches to a different MCS right after
this one. Given that I don't have a datasheet for this perticular driver
IC, I can't be absolutely certain, but considering that the drivers for
rm67191 and rm68200 are defining 0xfe as the command to switch the
commands sets, it's probably the same here.
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd0);
+ mipi_dsi_dcs_write_seq(dsi, 0x48, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0x
[PATCH v3 1/2] dt-bindings: display: panel: Add Raydium RM69380
Raydium RM69380 is a display driver IC used to drive OLED DSI panels.
Add a dt-binding for it.
Signed-off-by: David Wronek
---
Note:
Depends on commit 48a516363e29 ("dt-bindings: display: panel: add common
dual-link schema")
---
.../bindings/display/panel/raydium,rm69380.yaml| 91 ++
1 file changed, 91 insertions(+)
diff --git
a/Documentation/devicetree/bindings/display/panel/raydium,rm69380.yaml
b/Documentation/devicetree/bindings/display/panel/raydium,rm69380.yaml
new file mode 100644
index ..0ac7d033cbe0
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/raydium,rm69380.yaml
@@ -0,0 +1,91 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/raydium,rm69380.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Raydium RM6380-based DSI display panels
+
+maintainers:
+ - David Wronek
+
+description:
+ The Raydium RM69380 is a generic DSI panel IC used to control
+ OLED panels.
+
+allOf:
+ - $ref: panel-common-dual.yaml#
+
+properties:
+ compatible:
+items:
+ - enum:
+ - lenovo,j716f-edo-rm69380
+ - const: raydium,rm69380
+description: This indicates the panel manufacturer of the panel
+ that is in turn using the RM69380 panel driver. The compatible
+ string determines how the RM69380 panel driver shall be configured
+ to work with the indicated panel. The raydium,rm69380 compatible shall
+ always be provided as a fallback.
+
+ avdd-supply:
+description: Analog voltage rail
+
+ vddio-supply:
+description: I/O voltage rail
+
+ reset-gpios:
+maxItems: 1
+description: phandle of gpio for reset line - This should be active low
+
+ ports: true
+ reg: true
+
+required:
+ - compatible
+ - reg
+ - avdd-supply
+ - vddio-supply
+ - reset-gpios
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+#include
+
+dsi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+panel@0 {
+compatible = "lenovo,j716f-edo-rm69380", "raydium,rm69380";
+reg = <0>;
+
+avdd-supply = <_avdd_regulator>;
+vddio-supply = <_l14a>;
+reset-gpios = < 75 GPIO_ACTIVE_LOW>;
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+
+port@0 {
+reg = <0>;
+panel_in_0: endpoint {
+remote-endpoint = <_dsi0_out>;
+};
+};
+
+port@1 {
+reg = <1>;
+panel_in_1: endpoint {
+remote-endpoint = <_dsi1_out>;
+};
+};
+};
+};
+};
+
+...
--
2.44.0
[PATCH v3 2/2] drm/panel: Add driver for EDO RM69380 OLED panel
Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a
Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro 2021.
Signed-off-by: David Wronek
---
drivers/gpu/drm/panel/Kconfig | 14 +
drivers/gpu/drm/panel/Makefile| 1 +
drivers/gpu/drm/panel/panel-raydium-rm69380.c | 367 ++
3 files changed, 382 insertions(+)
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 154f5bf82980..5b3eeb93b1a2 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -542,6 +542,20 @@ config DRM_PANEL_RAYDIUM_RM692E5
Say Y here if you want to enable support for Raydium RM692E5-based
display panels, such as the one found in the Fairphone 5 smartphone.
+config DRM_PANEL_RAYDIUM_RM69380
+ tristate "Raydium RM69380-based DSI panel"
+ depends on BACKLIGHT_CLASS_DEVICE
+ depends on DRM_DISPLAY_DP_HELPER
+ depends on DRM_DISPLAY_HELPER
+ depends on DRM_MIPI_DSI
+ depends on OF
+ help
+ Say Y here if you want to enable support for Raydium RM69380-based
+ display panels.
+
+ This panel controller can be found in the Lenovo Xiaoxin Pad Pro 2021
+ in combination with an EDO OLED panel.
+
config DRM_PANEL_RONBO_RB070D30
tristate "Ronbo Electronics RB070D30 panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 24a02655d726..e2a2807d4ef0 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) +=
panel-raspberrypi-touchscreen
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM692E5) += panel-raydium-rm692e5.o
+obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM69380) += panel-raydium-rm69380.o
obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o
obj-$(CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20) += panel-samsung-atna33xc20.o
obj-$(CONFIG_DRM_PANEL_SAMSUNG_DB7430) += panel-samsung-db7430.o
diff --git a/drivers/gpu/drm/panel/panel-raydium-rm69380.c
b/drivers/gpu/drm/panel/panel-raydium-rm69380.c
new file mode 100644
index ..f89230c969b7
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-raydium-rm69380.c
@@ -0,0 +1,367 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Generated with linux-mdss-dsi-panel-driver-generator from vendor device
tree.
+ * Copyright (c) 2024 David Wronek
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include
+#include
+#include
+#include
+
+struct rm69380_panel {
+ struct drm_panel panel;
+ struct mipi_dsi_device *dsi[2];
+ struct regulator_bulk_data supplies[2];
+ struct gpio_desc *reset_gpio;
+};
+
+static inline
+struct rm69380_panel *to_rm69380_panel(struct drm_panel *panel)
+{
+ return container_of(panel, struct rm69380_panel, panel);
+}
+
+static void rm69380_reset(struct rm69380_panel *ctx)
+{
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ usleep_range(15000, 16000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+ usleep_range(1, 11000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ msleep(30);
+}
+
+static int rm69380_on(struct rm69380_panel *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi[0];
+ struct device *dev = >dev;
+ int ret;
+
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+ if (ctx->dsi[1])
+ ctx->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd4);
+ mipi_dsi_dcs_write_seq(dsi, 0x00, 0x80);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd0);
+ mipi_dsi_dcs_write_seq(dsi, 0x48, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0x26);
+ mipi_dsi_dcs_write_seq(dsi, 0x75, 0x3f);
+ mipi_dsi_dcs_write_seq(dsi, 0x1d, 0x1a);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x28);
+ mipi_dsi_dcs_write_seq(dsi, 0xc2, 0x08);
+
+ ret = mipi_dsi_dcs_set_tear_on(dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set tear on: %d\n", ret);
+ return ret;
+ }
+
+ ret = mipi_dsi_dcs_set_display_brightness(dsi, 0x7ff);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set display brightness: %d\n", ret);
+ return ret;
+ }
+
+ ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
+ return ret;
+ }
+ msleep(20);
+
+ ret = mipi_dsi_dcs_set_display_on(dsi);
+ if (ret < 0) {
+
[PATCH v3 0/2] Add driver for Raydium RM69380-based DSI panels
This patch adds support the 2560x1600@90Hz dual DSI command mode panel by EDO in combination with a Raydium RM69380 driver IC. This driver IC can be found in the following devices: * Lenovo Xiaoxin Pad Pro 2021 (TB-J716F) with EDO panel * Lenovo Tab P11 Pro (TB-J706F) with EDO panel * Robo & Kala 2-in-1 Laptop with Sharp panel Signed-off-by: David Wronek --- Changes in v3: - Removed unneeded curly brackets from some if statments - Fix error handling code in probe function - Include video/mipi_display.h and make use of MIPI command definitions - Removed DRM_MODE_TYPE_PREFERRED - Dropped 'prepared' bool entirely - Register second DSI host using mipi_dsi_device_register_full() - Link to v2: https://lore.kernel.org/r/20240415-raydium-rm69380-driver-v2-0-524216461...@mainlining.org Changes in v2: - Fixed typo in Kconfig - Removed ctx->prepared = true; in prepare function - Switched to drm_connector_helper_get_modes_fixed in get_modes function - Changed dev_notice() to dev_dbg() - Add description for compatible and reset-gpio in the dt-binding - Always require 'ports' node in the dt-binding regardless of compatible - Link to v1: https://lore.kernel.org/r/20240414-raydium-rm69380-driver-v1-0-5e86ba249...@mainlining.org --- David Wronek (2): dt-bindings: display: panel: Add Raydium RM69380 drm/panel: Add driver for EDO RM69380 OLED panel .../bindings/display/panel/raydium,rm69380.yaml| 91 + drivers/gpu/drm/panel/Kconfig | 14 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-raydium-rm69380.c | 367 + 4 files changed, 473 insertions(+) --- base-commit: 66e4190e92ce0e4a50b2f6be0e5f5b2e47e072f4 change-id: 20240414-raydium-rm69380-driver-47f22b6f24fe Best regards, -- David Wronek
Re: [PATCH v2 2/2] drm/panel: Add driver for EDO RM69380 OLED panel
W dniu 2024-04-15 19:55, Christophe JAILLET napisał(a):
Le 15/04/2024 à 18:10, David Wronek a écrit :
Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a
Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro
2021.
Signed-off-by: David Wronek
---
drivers/gpu/drm/panel/Kconfig | 14 +
drivers/gpu/drm/panel/Makefile| 1 +
drivers/gpu/drm/panel/panel-raydium-rm69380.c | 366
++
3 files changed, 381 insertions(+)
...
+static int rm69380_on(struct rm69380_panel *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi[0];
+ struct device *dev = >dev;
+ int ret;
+
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+ if (ctx->dsi[1])
+ ctx->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd4);
+ mipi_dsi_dcs_write_seq(dsi, 0x00, 0x80);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd0);
+ mipi_dsi_dcs_write_seq(dsi, 0x48, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0x26);
+ mipi_dsi_dcs_write_seq(dsi, 0x75, 0x3f);
+ mipi_dsi_dcs_write_seq(dsi, 0x1d, 0x1a);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0x53, 0x28);
+ mipi_dsi_dcs_write_seq(dsi, 0xc2, 0x08);
+ mipi_dsi_dcs_write_seq(dsi, 0x35, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0x51, 0x07, 0xff);
+
+ ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
+ return ret;
+ }
+ msleep(20);
+
+ ret = mipi_dsi_dcs_set_display_on(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set display on: %d\n", ret);
+ return ret;
+ }
+ msleep(36);
36 and 35 below are un-usual values for msleep.
Why 2 different values?
Would using a #define for this(these) value(s) make sense here?
I am not sure of that either. This is how the panel is being set up in
Android, as well as the bootloader.
See lines 67 and 92 here:
https://github.com/ungeskriptet/QcomXblBinaries/blob/master/J716F/BOOT.XF.3.2-00354-SM8250-1/RawFiles/Panel_rm69380_amoled_2k_cmd.xml
+
+ return 0;
+}
+
+static int rm69380_off(struct rm69380_panel *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi[0];
+ struct device *dev = >dev;
+ int ret;
+
+ dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+ if (ctx->dsi[1])
+ ctx->dsi[1]->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+ ret = mipi_dsi_dcs_set_display_off(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set display off: %d\n", ret);
+ return ret;
+ }
+ msleep(35);
(here)
+
+ ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to enter sleep mode: %d\n", ret);
+ return ret;
+ }
+ msleep(20);
+
+ return 0;
+}
...
+static int rm69380_probe(struct mipi_dsi_device *dsi)
+{
+ struct mipi_dsi_host *dsi_sec_host;
+ struct rm69380_panel *ctx;
+ struct device *dev = >dev;
+ struct device_node *dsi_sec;
+ int ret, i;
+
+ ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+ if (!ctx)
+ return -ENOMEM;
+
+ ctx->supplies[0].supply = "vddio";
+ ctx->supplies[1].supply = "avdd";
+ ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
+ ctx->supplies);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "Failed to get regulators\n");
+
+ ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(ctx->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
+"Failed to get reset-gpios\n");
+
+ dsi_sec = of_graph_get_remote_node(dsi->dev.of_node, 1, -1);
+
+ if (dsi_sec) {
+ dev_dbg(dev, "Using Dual-DSI\n");
This should be after de 'info' variable below, so...
+
+ const struct mipi_dsi_device_info info = { "RM69380", 0,
+ dsi_sec };
+
+ dev_dbg(dev, "Found second DSI `%s`\n", dsi_sec->name);
... maybe merge the 2 messages into something like:
dev_dbg(dev, "Using Dual-DSI: found `%s`\n", dsi_sec->name);
+
+ dsi_sec_host = of_find_mipi_dsi_host_by_node(dsi_sec);
+ of_node_put(dsi_sec);
+ if (!dsi_sec_host) {
+ return dev_err_probe(dev, -EPROBE_DEFER,
+"Cannot get secondary DSI host\n");
+ }
+
[PATCH v2 2/2] drm/panel: Add driver for EDO RM69380 OLED panel
Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a
Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro 2021.
Signed-off-by: David Wronek
---
drivers/gpu/drm/panel/Kconfig | 14 +
drivers/gpu/drm/panel/Makefile| 1 +
drivers/gpu/drm/panel/panel-raydium-rm69380.c | 366 ++
3 files changed, 381 insertions(+)
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 154f5bf82980..5b3eeb93b1a2 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -542,6 +542,20 @@ config DRM_PANEL_RAYDIUM_RM692E5
Say Y here if you want to enable support for Raydium RM692E5-based
display panels, such as the one found in the Fairphone 5 smartphone.
+config DRM_PANEL_RAYDIUM_RM69380
+ tristate "Raydium RM69380-based DSI panel"
+ depends on BACKLIGHT_CLASS_DEVICE
+ depends on DRM_DISPLAY_DP_HELPER
+ depends on DRM_DISPLAY_HELPER
+ depends on DRM_MIPI_DSI
+ depends on OF
+ help
+ Say Y here if you want to enable support for Raydium RM69380-based
+ display panels.
+
+ This panel controller can be found in the Lenovo Xiaoxin Pad Pro 2021
+ in combination with an EDO OLED panel.
+
config DRM_PANEL_RONBO_RB070D30
tristate "Ronbo Electronics RB070D30 panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 24a02655d726..e2a2807d4ef0 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) +=
panel-raspberrypi-touchscreen
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM692E5) += panel-raydium-rm692e5.o
+obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM69380) += panel-raydium-rm69380.o
obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o
obj-$(CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20) += panel-samsung-atna33xc20.o
obj-$(CONFIG_DRM_PANEL_SAMSUNG_DB7430) += panel-samsung-db7430.o
diff --git a/drivers/gpu/drm/panel/panel-raydium-rm69380.c
b/drivers/gpu/drm/panel/panel-raydium-rm69380.c
new file mode 100644
index ..253b9a1c2800
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-raydium-rm69380.c
@@ -0,0 +1,366 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Generated with linux-mdss-dsi-panel-driver-generator from vendor device
tree.
+ * Copyright (c) 2024 David Wronek
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+struct rm69380_panel {
+ struct drm_panel panel;
+ struct mipi_dsi_device *dsi[2];
+ struct regulator_bulk_data supplies[2];
+ struct gpio_desc *reset_gpio;
+ bool prepared;
+};
+
+static inline
+struct rm69380_panel *to_rm69380_panel(struct drm_panel *panel)
+{
+ return container_of(panel, struct rm69380_panel, panel);
+}
+
+static void rm69380_reset(struct rm69380_panel *ctx)
+{
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ usleep_range(15000, 16000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+ usleep_range(1, 11000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ msleep(30);
+}
+
+static int rm69380_on(struct rm69380_panel *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi[0];
+ struct device *dev = >dev;
+ int ret;
+
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+ if (ctx->dsi[1])
+ ctx->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd4);
+ mipi_dsi_dcs_write_seq(dsi, 0x00, 0x80);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd0);
+ mipi_dsi_dcs_write_seq(dsi, 0x48, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0x26);
+ mipi_dsi_dcs_write_seq(dsi, 0x75, 0x3f);
+ mipi_dsi_dcs_write_seq(dsi, 0x1d, 0x1a);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0x53, 0x28);
+ mipi_dsi_dcs_write_seq(dsi, 0xc2, 0x08);
+ mipi_dsi_dcs_write_seq(dsi, 0x35, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0x51, 0x07, 0xff);
+
+ ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
+ return ret;
+ }
+ msleep(20);
+
+ ret = mipi_dsi_dcs_set_display_on(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set display on: %d\n", ret);
+ return ret;
+ }
+ msleep(36);
+
+ return 0;
+}
+
+static int rm69380_off(struct rm69380_panel *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi[0];
+ struct device *dev = >dev;
+ int ret;
+
+ dsi->mod
[PATCH v2 0/2] Add driver for Raydium RM69380-based DSI panels
This patch adds support the 2560x1600@90Hz dual DSI command mode panel by EDO in combination with a Raydium RM69380 driver IC. This driver IC can be found in the following devices: * Lenovo Xiaoxin Pad Pro 2021 (TB-J716F) with EDO panel * Lenovo Tab P11 Pro (TB-J706F) with EDO panel * Robo & Kala 2-in-1 Laptop with Sharp panel Signed-off-by: David Wronek --- Changes in v2: - Fixed typo in Kconfig - Removed ctx->prepared = true; in prepare function - Switched to drm_connector_helper_get_modes_fixed in get_modes function - Changed dev_notice() to dev_dbg() - Add description for compatible and reset-gpio in the dt-binding - Always require 'ports' node in the dt-binding regardless of compatible - Link to v1: https://lore.kernel.org/r/20240414-raydium-rm69380-driver-v1-0-5e86ba249...@mainlining.org --- David Wronek (2): dt-bindings: display: panel: Add Raydium RM69380 drm/panel: Add driver for EDO RM69380 OLED panel .../bindings/display/panel/raydium,rm69380.yaml| 91 + drivers/gpu/drm/panel/Kconfig | 14 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-raydium-rm69380.c | 366 + 4 files changed, 472 insertions(+) --- base-commit: 6bd343537461b57f3efe5dfc5fc193a232dfef1e change-id: 20240414-raydium-rm69380-driver-47f22b6f24fe Best regards, -- David Wronek
[PATCH v2 1/2] dt-bindings: display: panel: Add Raydium RM69380
Raydium RM69380 is a display driver IC used to drive OLED DSI panels.
Add a dt-binding for it.
Signed-off-by: David Wronek
---
.../bindings/display/panel/raydium,rm69380.yaml| 91 ++
1 file changed, 91 insertions(+)
diff --git
a/Documentation/devicetree/bindings/display/panel/raydium,rm69380.yaml
b/Documentation/devicetree/bindings/display/panel/raydium,rm69380.yaml
new file mode 100644
index ..0ac7d033cbe0
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/raydium,rm69380.yaml
@@ -0,0 +1,91 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/raydium,rm69380.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Raydium RM6380-based DSI display panels
+
+maintainers:
+ - David Wronek
+
+description:
+ The Raydium RM69380 is a generic DSI panel IC used to control
+ OLED panels.
+
+allOf:
+ - $ref: panel-common-dual.yaml#
+
+properties:
+ compatible:
+items:
+ - enum:
+ - lenovo,j716f-edo-rm69380
+ - const: raydium,rm69380
+description: This indicates the panel manufacturer of the panel
+ that is in turn using the RM69380 panel driver. The compatible
+ string determines how the RM69380 panel driver shall be configured
+ to work with the indicated panel. The raydium,rm69380 compatible shall
+ always be provided as a fallback.
+
+ avdd-supply:
+description: Analog voltage rail
+
+ vddio-supply:
+description: I/O voltage rail
+
+ reset-gpios:
+maxItems: 1
+description: phandle of gpio for reset line - This should be active low
+
+ ports: true
+ reg: true
+
+required:
+ - compatible
+ - reg
+ - avdd-supply
+ - vddio-supply
+ - reset-gpios
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+#include
+
+dsi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+panel@0 {
+compatible = "lenovo,j716f-edo-rm69380", "raydium,rm69380";
+reg = <0>;
+
+avdd-supply = <_avdd_regulator>;
+vddio-supply = <_l14a>;
+reset-gpios = < 75 GPIO_ACTIVE_LOW>;
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+
+port@0 {
+reg = <0>;
+panel_in_0: endpoint {
+remote-endpoint = <_dsi0_out>;
+};
+};
+
+port@1 {
+reg = <1>;
+panel_in_1: endpoint {
+remote-endpoint = <_dsi1_out>;
+};
+};
+};
+};
+};
+
+...
--
2.44.0
Re: [PATCH 2/2] drm/panel: Add driver for EDO RM69380 OLED panel
W dniu 2024-04-14 22:00, Dmitry Baryshkov napisał(a):
On Sun, Apr 14, 2024 at 05:22:31PM +0200, David Wronek wrote:
Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a
Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro
2021.
Signed-off-by: David Wronek
---
drivers/gpu/drm/panel/Kconfig | 14 +
drivers/gpu/drm/panel/Makefile| 1 +
drivers/gpu/drm/panel/panel-raydium-rm69380.c | 378
++
3 files changed, 393 insertions(+)
diff --git a/drivers/gpu/drm/panel/Kconfig
b/drivers/gpu/drm/panel/Kconfig
index 154f5bf82980..84cbd838f57e 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -542,6 +542,20 @@ config DRM_PANEL_RAYDIUM_RM692E5
Say Y here if you want to enable support for Raydium RM692E5-based
display panels, such as the one found in the Fairphone 5
smartphone.
+config DRM_PANEL_RAYDIUM_RM69380
+ tristate "Raydium RM69380-based DSI panel"
+ depends on BACKLIGHT_CLASS_DEVICE
+ depends on DRM_DISPLAY_DP_HELPER
+ depends on DRM_DISPLAY_HELPER
+ depends on DRM_MIPI_DSI
+ depends on OF
+ help
+ Say Y here if you want to enable support for Raydium RM69380-based
+ display panels.
+
+ This panel controller can be found in the Lenovo Xiaoxin Pad Pro
2021
+ in combiantion with an EDO OLED panel.
+
config DRM_PANEL_RONBO_RB070D30
tristate "Ronbo Electronics RB070D30 panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile
b/drivers/gpu/drm/panel/Makefile
index 24a02655d726..e2a2807d4ef0 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) +=
panel-raspberrypi-touchscreen
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM692E5) += panel-raydium-rm692e5.o
+obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM69380) += panel-raydium-rm69380.o
obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o
obj-$(CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20) +=
panel-samsung-atna33xc20.o
obj-$(CONFIG_DRM_PANEL_SAMSUNG_DB7430) += panel-samsung-db7430.o
diff --git a/drivers/gpu/drm/panel/panel-raydium-rm69380.c
b/drivers/gpu/drm/panel/panel-raydium-rm69380.c
new file mode 100644
index ..0b2d576b051d
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-raydium-rm69380.c
@@ -0,0 +1,378 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Generated with linux-mdss-dsi-panel-driver-generator from vendor
device tree.
+ * Copyright (c) 2024 David Wronek
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+struct rm69380_panel {
+ struct drm_panel panel;
+ struct mipi_dsi_device *dsi[2];
+ struct regulator_bulk_data supplies[2];
+ struct gpio_desc *reset_gpio;
+ bool prepared;
+};
+
+static inline
+struct rm69380_panel *to_rm69380_panel(struct drm_panel *panel)
+{
+ return container_of(panel, struct rm69380_panel, panel);
+}
+
+static void rm69380_reset(struct rm69380_panel *ctx)
+{
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ usleep_range(15000, 16000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+ usleep_range(1, 11000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ msleep(30);
+}
+
+static int rm69380_on(struct rm69380_panel *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi[0];
+ struct device *dev = >dev;
+ int ret;
+
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+ if (ctx->dsi[1])
+ ctx->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd4);
+ mipi_dsi_dcs_write_seq(dsi, 0x00, 0x80);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd0);
+ mipi_dsi_dcs_write_seq(dsi, 0x48, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0x26);
+ mipi_dsi_dcs_write_seq(dsi, 0x75, 0x3f);
+ mipi_dsi_dcs_write_seq(dsi, 0x1d, 0x1a);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0x53, 0x28);
+ mipi_dsi_dcs_write_seq(dsi, 0xc2, 0x08);
+ mipi_dsi_dcs_write_seq(dsi, 0x35, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0x51, 0x07, 0xff);
Does this assume that the host broadcasts commands to both DSI0 and
DSI1
or is it enough to send commands on DSI0 only?
It is enough to send the commands to DSI0 only.
+
+ ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
+ return ret;
+ }
+ msleep(20);
+
+ ret = mipi_dsi_dcs_set_display_on(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set display on:
Re: [PATCH 2/2] drm/panel: Add driver for EDO RM69380 OLED panel
W dniu 2024-04-15 07:39, Christophe JAILLET napisał(a): Le 15/04/2024 à 07:37, david-vu3dztd92roxwddmvfq...@public.gmane.org a écrit : W dniu 2024-04-14 22:22, Christophe JAILLET napisał(a): Le 14/04/2024 à 17:22, David Wronek a écrit : Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro 2021. Signed-off-by: David Wronek --- drivers/gpu/drm/panel/Kconfig | 14 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-raydium-rm69380.c | 378 ++ 3 files changed, 393 insertions(+) diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 154f5bf82980..84cbd838f57e 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -542,6 +542,20 @@ config DRM_PANEL_RAYDIUM_RM692E5 Say Y here if you want to enable support for Raydium RM692E5-based display panels, such as the one found in the Fairphone 5 smartphone. +config DRM_PANEL_RAYDIUM_RM69380 + tristate "Raydium RM69380-based DSI panel" + depends on BACKLIGHT_CLASS_DEVICE + depends on DRM_DISPLAY_DP_HELPER + depends on DRM_DISPLAY_HELPER + depends on DRM_MIPI_DSI + depends on OF + help + Say Y here if you want to enable support for Raydium RM69380-based + display panels. + + This panel controller can be found in the Lenovo Xiaoxin Pad Pro 2021 + in combiantion with an EDO OLED panel. combination? Yes, this is just one of the examples where this driver IC can be found. It can also be used with panels other than those from EDO. Hi, sorry if i was unclear. Is there a typo: s/combiantion/combination/ ? CJ Ah, now I get it. Yes, that is indeed a typo. Thanks for pointing this out. + config DRM_PANEL_RONBO_RB070D30 tristate "Ronbo Electronics RB070D30 panel" depends on OF Best regards, David Wronek -- Best regards, David Wronek
Re: [PATCH 2/2] drm/panel: Add driver for EDO RM69380 OLED panel
W dniu 2024-04-14 22:22, Christophe JAILLET napisał(a): Le 14/04/2024 à 17:22, David Wronek a écrit : Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro 2021. Signed-off-by: David Wronek --- drivers/gpu/drm/panel/Kconfig | 14 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-raydium-rm69380.c | 378 ++ 3 files changed, 393 insertions(+) diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 154f5bf82980..84cbd838f57e 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -542,6 +542,20 @@ config DRM_PANEL_RAYDIUM_RM692E5 Say Y here if you want to enable support for Raydium RM692E5-based display panels, such as the one found in the Fairphone 5 smartphone. +config DRM_PANEL_RAYDIUM_RM69380 + tristate "Raydium RM69380-based DSI panel" + depends on BACKLIGHT_CLASS_DEVICE + depends on DRM_DISPLAY_DP_HELPER + depends on DRM_DISPLAY_HELPER + depends on DRM_MIPI_DSI + depends on OF + help + Say Y here if you want to enable support for Raydium RM69380-based + display panels. + + This panel controller can be found in the Lenovo Xiaoxin Pad Pro 2021 + in combiantion with an EDO OLED panel. combination? Yes, this is just one of the examples where this driver IC can be found. It can also be used with panels other than those from EDO. + config DRM_PANEL_RONBO_RB070D30 tristate "Ronbo Electronics RB070D30 panel" depends on OF Best regards, David Wronek
[PATCH 2/2] drm/panel: Add driver for EDO RM69380 OLED panel
Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a
Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro 2021.
Signed-off-by: David Wronek
---
drivers/gpu/drm/panel/Kconfig | 14 +
drivers/gpu/drm/panel/Makefile| 1 +
drivers/gpu/drm/panel/panel-raydium-rm69380.c | 378 ++
3 files changed, 393 insertions(+)
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 154f5bf82980..84cbd838f57e 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -542,6 +542,20 @@ config DRM_PANEL_RAYDIUM_RM692E5
Say Y here if you want to enable support for Raydium RM692E5-based
display panels, such as the one found in the Fairphone 5 smartphone.
+config DRM_PANEL_RAYDIUM_RM69380
+ tristate "Raydium RM69380-based DSI panel"
+ depends on BACKLIGHT_CLASS_DEVICE
+ depends on DRM_DISPLAY_DP_HELPER
+ depends on DRM_DISPLAY_HELPER
+ depends on DRM_MIPI_DSI
+ depends on OF
+ help
+ Say Y here if you want to enable support for Raydium RM69380-based
+ display panels.
+
+ This panel controller can be found in the Lenovo Xiaoxin Pad Pro 2021
+ in combiantion with an EDO OLED panel.
+
config DRM_PANEL_RONBO_RB070D30
tristate "Ronbo Electronics RB070D30 panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 24a02655d726..e2a2807d4ef0 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) +=
panel-raspberrypi-touchscreen
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM692E5) += panel-raydium-rm692e5.o
+obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM69380) += panel-raydium-rm69380.o
obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o
obj-$(CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20) += panel-samsung-atna33xc20.o
obj-$(CONFIG_DRM_PANEL_SAMSUNG_DB7430) += panel-samsung-db7430.o
diff --git a/drivers/gpu/drm/panel/panel-raydium-rm69380.c
b/drivers/gpu/drm/panel/panel-raydium-rm69380.c
new file mode 100644
index ..0b2d576b051d
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-raydium-rm69380.c
@@ -0,0 +1,378 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Generated with linux-mdss-dsi-panel-driver-generator from vendor device
tree.
+ * Copyright (c) 2024 David Wronek
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+struct rm69380_panel {
+ struct drm_panel panel;
+ struct mipi_dsi_device *dsi[2];
+ struct regulator_bulk_data supplies[2];
+ struct gpio_desc *reset_gpio;
+ bool prepared;
+};
+
+static inline
+struct rm69380_panel *to_rm69380_panel(struct drm_panel *panel)
+{
+ return container_of(panel, struct rm69380_panel, panel);
+}
+
+static void rm69380_reset(struct rm69380_panel *ctx)
+{
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ usleep_range(15000, 16000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+ usleep_range(1, 11000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ msleep(30);
+}
+
+static int rm69380_on(struct rm69380_panel *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi[0];
+ struct device *dev = >dev;
+ int ret;
+
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+ if (ctx->dsi[1])
+ ctx->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd4);
+ mipi_dsi_dcs_write_seq(dsi, 0x00, 0x80);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd0);
+ mipi_dsi_dcs_write_seq(dsi, 0x48, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0x26);
+ mipi_dsi_dcs_write_seq(dsi, 0x75, 0x3f);
+ mipi_dsi_dcs_write_seq(dsi, 0x1d, 0x1a);
+ mipi_dsi_dcs_write_seq(dsi, 0xfe, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0x53, 0x28);
+ mipi_dsi_dcs_write_seq(dsi, 0xc2, 0x08);
+ mipi_dsi_dcs_write_seq(dsi, 0x35, 0x00);
+ mipi_dsi_dcs_write_seq(dsi, 0x51, 0x07, 0xff);
+
+ ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
+ return ret;
+ }
+ msleep(20);
+
+ ret = mipi_dsi_dcs_set_display_on(dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set display on: %d\n", ret);
+ return ret;
+ }
+ msleep(36);
+
+ return 0;
+}
+
+static int rm69380_off(struct rm69380_panel *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi[0];
+ struct device *dev = >dev;
+ int ret;
+
+ dsi->mode_flags &
[PATCH 1/2] dt-bindings: display: panel: Add Raydium RM69380
Raydium RM69380 is a display driver IC used to drive OLED DSI panels.
Add a dt-binding for it.
Signed-off-by: David Wronek
---
.../bindings/display/panel/raydium,rm69380.yaml| 94 ++
1 file changed, 94 insertions(+)
diff --git
a/Documentation/devicetree/bindings/display/panel/raydium,rm69380.yaml
b/Documentation/devicetree/bindings/display/panel/raydium,rm69380.yaml
new file mode 100644
index ..9b01b9c22ae9
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/raydium,rm69380.yaml
@@ -0,0 +1,94 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/raydium,rm69380.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Raydium RM6380-based DSI display panels
+
+maintainers:
+ - David Wronek
+
+description:
+ The Raydium RM69380 is a generic DSI Panel IC used to control
+ OLED panels.
+
+properties:
+ compatible:
+items:
+ - const: lenovo,j716f-edo-rm69380
+ - const: raydium,rm69380
+
+ avdd-supply:
+description: Analog voltage rail
+
+ vddio-supply:
+description: I/O voltage rail
+
+ reset-gpios:
+maxItems: 1
+
+ reg: true
+
+required:
+ - compatible
+ - reg
+ - avdd-supply
+ - vddio-supply
+ - reset-gpios
+
+allOf:
+ - $ref: panel-common-dual.yaml#
+ - if:
+ properties:
+compatible:
+ contains:
+enum:
+ - lenovo,j716f-edo-rm69380
+then:
+ properties:
+port: false
+ports:
+ required:
+- port@1
+
+unevaluatedProperties: false
+
+examples:
+ - |
+#include
+
+dsi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+panel@0 {
+compatible = "lenovo,j716f-edo-rm69380", "raydium,rm69380";
+reg = <0>;
+
+avdd-supply = <_avdd_regulator>;
+vddio-supply = <_l14a>;
+reset-gpios = < 75 GPIO_ACTIVE_LOW>;
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+
+port@0 {
+reg = <0>;
+panel_in_0: endpoint {
+remote-endpoint = <_dsi0_out>;
+};
+};
+
+port@1 {
+reg = <1>;
+panel_in_1: endpoint {
+remote-endpoint = <_dsi1_out>;
+};
+};
+};
+};
+};
+
+...
--
2.44.0
[PATCH 0/2] Add driver for Raydium RM69380-based DSI panels
This patch adds support the 2560x1600@90Hz dual DSI command mode panel by EDO in combination with a Raydium RM69380 driver IC. This driver IC can be found in the following devices: * Lenovo Xiaoxin Pad Pro 2021 (TB-J716F) with EDO panel * Lenovo Tab P11 Pro (TB-J706F) with EDO panel * Robo & Kala 2-in-1 Laptop with Sharp panel Signed-off-by: David Wronek --- David Wronek (2): dt-bindings: display: panel: Add Raydium RM69380 drm/panel: Add driver for EDO RM69380 OLED panel .../bindings/display/panel/raydium,rm69380.yaml| 94 + drivers/gpu/drm/panel/Kconfig | 14 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-raydium-rm69380.c | 378 + 4 files changed, 487 insertions(+) --- base-commit: 9ed46da14b9b9b2ad4edb3b0c545b6dbe5c00d39 change-id: 20240414-raydium-rm69380-driver-47f22b6f24fe Best regards, -- David Wronek
Re: [PATCH v3 04/15] kunit: Add documentation for warning backtrace suppression API
On Wed, 3 Apr 2024 at 21:19, Guenter Roeck wrote:
>
> Document API functions for suppressing warning backtraces.
>
> Tested-by: Linux Kernel Functional Testing
> Acked-by: Dan Carpenter
> Reviewed-by: Kees Cook
> Signed-off-by: Guenter Roeck
> ---
This looks good to me: thanks for adding the documentation!
If we add integration between this and the KUnit resource system,
we'll need to add that to this documentation.
I wonder if it would make sense to have an example where the
DEFINE_SUPPRESSED_WARNING() is global, e.g., in the test init/exit
functions. That might overcomplicate it a bit.
It also might be nice to document the individual macros with kerneldoc
comments. (Though, that could equally fit in patch #1).
Still, this is the most important bit, so I'm happy to have it as-is.
Reviewed-by: David Gow
Cheers,
-- David
> v2:
> - Rebased to v6.9-rc1
> - Added Tested-by:, Acked-by:, and Reviewed-by: tags
> v3:
> - Rebased to v6.9-rc2
>
> Documentation/dev-tools/kunit/usage.rst | 30 -
> 1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/dev-tools/kunit/usage.rst
> b/Documentation/dev-tools/kunit/usage.rst
> index 22955d56b379..8d3d36d4103d 100644
> --- a/Documentation/dev-tools/kunit/usage.rst
> +++ b/Documentation/dev-tools/kunit/usage.rst
> @@ -157,6 +157,34 @@ Alternatively, one can take full control over the error
> message by using
> if (some_setup_function())
> KUNIT_FAIL(test, "Failed to setup thing for testing");
>
> +Suppressing warning backtraces
> +--
> +
> +Some unit tests trigger warning backtraces either intentionally or as side
> +effect. Such backtraces are normally undesirable since they distract from
> +the actual test and may result in the impression that there is a problem.
> +
> +Such backtraces can be suppressed. To suppress a backtrace in
> some_function(),
> +use the following code.
> +
> +.. code-block:: c
> +
> + static void some_test(struct kunit *test)
> + {
> + DEFINE_SUPPRESSED_WARNING(some_function);
> +
> + START_SUPPRESSED_WARNING(some_function);
> + trigger_backtrace();
> + END_SUPPRESSED_WARNING(some_function);
> + }
> +
> +SUPPRESSED_WARNING_COUNT() returns the number of suppressed backtraces. If
> the
> +suppressed backtrace was triggered on purpose, this can be used to check if
> +the backtrace was actually triggered.
> +
> +.. code-block:: c
> +
> + KUNIT_EXPECT_EQ(test, SUPPRESSED_WARNING_COUNT(some_function), 1);
>
> Test Suites
> ~~~
> @@ -857,4 +885,4 @@ For example:
> dev_managed_string = devm_kstrdup(fake_device, "Hello,
> World!");
>
> // Everything is cleaned up automatically when the test ends.
> - }
> \ No newline at end of file
> + }
> --
> 2.39.2
>
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [PATCH v3 02/15] kunit: bug: Count suppressed warning backtraces
On Wed, 3 Apr 2024 at 21:19, Guenter Roeck wrote:
>
> Count suppressed warning backtraces to enable code which suppresses
> warning backtraces to check if the expected backtrace(s) have been
> observed.
>
> Using atomics for the backtrace count resulted in build errors on some
> architectures due to include file recursion, so use a plain integer
> for now.
>
> Acked-by: Dan Carpenter
> Reviewed-by: Kees Cook
> Tested-by: Linux Kernel Functional Testing
> Signed-off-by: Guenter Roeck
> ---
Looks good to me, thanks.
Reviewed-by: David Gow
Cheers,
-- David
> v2:
> - Rebased to v6.9-rc1
> - Added Tested-by:, Acked-by:, and Reviewed-by: tags
> - Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
> v3:
> - Rebased to v6.9-rc2
>
> include/kunit/bug.h | 7 ++-
> lib/kunit/bug.c | 4 +++-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/include/kunit/bug.h b/include/kunit/bug.h
> index bd0fe047572b..72e9fb23bbd5 100644
> --- a/include/kunit/bug.h
> +++ b/include/kunit/bug.h
> @@ -20,6 +20,7 @@
> struct __suppressed_warning {
> struct list_head node;
> const char *function;
> + int counter;
> };
>
> void __start_suppress_warning(struct __suppressed_warning *warning);
> @@ -28,7 +29,7 @@ bool __is_suppressed_warning(const char *function);
>
> #define DEFINE_SUPPRESSED_WARNING(func)\
> struct __suppressed_warning __kunit_suppress_##func = \
> - { .function = __stringify(func) }
> + { .function = __stringify(func), .counter = 0 }
>
> #define START_SUPPRESSED_WARNING(func) \
> __start_suppress_warning(&__kunit_suppress_##func)
> @@ -39,12 +40,16 @@ bool __is_suppressed_warning(const char *function);
> #define IS_SUPPRESSED_WARNING(func) \
> __is_suppressed_warning(func)
>
> +#define SUPPRESSED_WARNING_COUNT(func) \
> + (__kunit_suppress_##func.counter)
> +
> #else /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
>
> #define DEFINE_SUPPRESSED_WARNING(func)
> #define START_SUPPRESSED_WARNING(func)
> #define END_SUPPRESSED_WARNING(func)
> #define IS_SUPPRESSED_WARNING(func) (false)
> +#define SUPPRESSED_WARNING_COUNT(func) (0)
>
> #endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
> #endif /* __ASSEMBLY__ */
> diff --git a/lib/kunit/bug.c b/lib/kunit/bug.c
> index f93544d7a9d1..13b3d896c114 100644
> --- a/lib/kunit/bug.c
> +++ b/lib/kunit/bug.c
> @@ -32,8 +32,10 @@ bool __is_suppressed_warning(const char *function)
> return false;
>
> list_for_each_entry(warning, _warnings, node) {
> - if (!strcmp(function, warning->function))
> + if (!strcmp(function, warning->function)) {
> + warning->counter++;
> return true;
> + }
> }
> return false;
> }
> --
> 2.39.2
>
> --
> You received this message because you are subscribed to the Google Groups
> "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kunit-dev+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/kunit-dev/20240403131936.787234-3-linux%40roeck-us.net.
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [PATCH v3 03/15] kunit: Add test cases for backtrace warning suppression
On Wed, 3 Apr 2024 at 21:19, Guenter Roeck wrote:
>
> Add unit tests to verify that warning backtrace suppression works.
>
> If backtrace suppression does _not_ work, the unit tests will likely
> trigger unsuppressed backtraces, which should actually help to get
> the affected architectures / platforms fixed.
>
> Tested-by: Linux Kernel Functional Testing
> Acked-by: Dan Carpenter
> Reviewed-by: Kees Cook
> Signed-off-by: Guenter Roeck
> ---
There's a typo in the Makefile, which stops this from being built at
all. Otherwise, it seems good to me.
-- David
> v2:
> - Rebased to v6.9-rc1
> - Added Tested-by:, Acked-by:, and Reviewed-by: tags
> - Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
> v3:
> - Rebased to v6.9-rc2
>
> lib/kunit/Makefile | 7 +-
> lib/kunit/backtrace-suppression-test.c | 104 +
> 2 files changed, 109 insertions(+), 2 deletions(-)
> create mode 100644 lib/kunit/backtrace-suppression-test.c
>
> diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
> index 545b57c3be48..3eee1bd0ce5e 100644
> --- a/lib/kunit/Makefile
> +++ b/lib/kunit/Makefile
> @@ -16,10 +16,13 @@ endif
>
> # KUnit 'hooks' and bug handling are built-in even when KUnit is built
> # as a module.
> -obj-y += hooks.o \
> - bug.o
> +obj-y += hooks.o
> +obj-$(CONFIG_KUNIT_SUPPRESS_BACKTRACE) += bug.o
>
> obj-$(CONFIG_KUNIT_TEST) +=kunit-test.o
> +ifeq ($(CCONFIG_KUNIT_SUPPRESS_BACKTRACE),y)
s/CCONFIG_/CONFIG_/ ?
> +obj-$(CONFIG_KUNIT_TEST) +=backtrace-suppression-test.o
> +endif
>
> # string-stream-test compiles built-in only.
> ifeq ($(CONFIG_KUNIT_TEST),y)
> diff --git a/lib/kunit/backtrace-suppression-test.c
> b/lib/kunit/backtrace-suppression-test.c
> new file mode 100644
> index ..47c619283802
> --- /dev/null
> +++ b/lib/kunit/backtrace-suppression-test.c
> @@ -0,0 +1,104 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for suppressing warning tracebacks
> + *
> + * Copyright (C) 2024, Guenter Roeck
> + * Author: Guenter Roeck
> + */
> +
> +#include
> +#include
> +
> +static void backtrace_suppression_test_warn_direct(struct kunit *test)
> +{
> + DEFINE_SUPPRESSED_WARNING(backtrace_suppression_test_warn_direct);
> +
> + START_SUPPRESSED_WARNING(backtrace_suppression_test_warn_direct);
> + WARN(1, "This backtrace should be suppressed");
> + END_SUPPRESSED_WARNING(backtrace_suppression_test_warn_direct);
> +
> + KUNIT_EXPECT_EQ(test,
> SUPPRESSED_WARNING_COUNT(backtrace_suppression_test_warn_direct), 1);
> +}
> +
> +static void trigger_backtrace_warn(void)
> +{
> + WARN(1, "This backtrace should be suppressed");
> +}
> +
> +static void backtrace_suppression_test_warn_indirect(struct kunit *test)
> +{
> + DEFINE_SUPPRESSED_WARNING(trigger_backtrace_warn);
> +
> + START_SUPPRESSED_WARNING(trigger_backtrace_warn);
> + trigger_backtrace_warn();
> + END_SUPPRESSED_WARNING(trigger_backtrace_warn);
> +
> + KUNIT_EXPECT_EQ(test,
> SUPPRESSED_WARNING_COUNT(trigger_backtrace_warn), 1);
> +}
> +
> +static void backtrace_suppression_test_warn_multi(struct kunit *test)
> +{
> + DEFINE_SUPPRESSED_WARNING(trigger_backtrace_warn);
> + DEFINE_SUPPRESSED_WARNING(backtrace_suppression_test_warn_multi);
> +
> + START_SUPPRESSED_WARNING(backtrace_suppression_test_warn_multi);
> + START_SUPPRESSED_WARNING(trigger_backtrace_warn);
> + WARN(1, "This backtrace should be suppressed");
> + trigger_backtrace_warn();
> + END_SUPPRESSED_WARNING(trigger_backtrace_warn);
> + END_SUPPRESSED_WARNING(backtrace_suppression_test_warn_multi);
> +
> + KUNIT_EXPECT_EQ(test,
> SUPPRESSED_WARNING_COUNT(backtrace_suppression_test_warn_multi), 1);
> + KUNIT_EXPECT_EQ(test,
> SUPPRESSED_WARNING_COUNT(trigger_backtrace_warn), 1);
> +}
> +
> +static void backtrace_suppression_test_warn_on_direct(struct kunit *test)
> +{
> + DEFINE_SUPPRESSED_WARNING(backtrace_suppression_test_warn_on_direct);
> +
> + if (!IS_ENABLED(CONFIG_DEBUG_BUGVERBOSE) &&
> !IS_ENABLED(CONFIG_KALLSYMS))
> + kunit_skip(test, "requires CONFIG_DEBUG_BUGVERBOSE or
> CONFIG_KALLSYMS");
> +
> + START_SUPPRESSED_WARNING(backtrace_suppression_test_warn_on_direct);
> + WARN_ON(1);
> + END_SUPPRESSED_WARNING(backtrace_suppression_test_warn_on_direct);
> +
> + KUNIT_EXPECT_EQ(test,
> +
Re: [PATCH v3 01/15] bug/kunit: Core support for suppressing warning backtraces
ing
> +* backtraces, if CONFIG_DEBUG_BUGVERBOSE is not enabled, or
> if
> +* the calling code is from assembler which does not record a
> +* function name. Extracting the function name from the bug
> +* address is less than perfect since compiler optimization
> may
> +* result in 'bugaddr' pointing to a function which does not
> +* actually trigger the warning, but it is better than no
> +* suppression at all.
> +*/
> + sprint_symbol_no_offset(sym, bugaddr);
> + function = sym;
> + }
> +#endif /* defined(CONFIG_KUNIT_SUPPRESS_BACKTRACE) &&
> defined(CONFIG_KALLSYMS) */
>
> warning = (bug->flags & BUGFLAG_WARNING) != 0;
> once = (bug->flags & BUGFLAG_ONCE) != 0;
> done = (bug->flags & BUGFLAG_DONE) != 0;
>
> + if (warning && IS_SUPPRESSED_WARNING(function))
> + return BUG_TRAP_TYPE_WARN;
> +
> if (warning && once) {
> if (done)
> return BUG_TRAP_TYPE_WARN;
> diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
> index 68a6daec0aef..b1b899265acc 100644
> --- a/lib/kunit/Kconfig
> +++ b/lib/kunit/Kconfig
> @@ -15,6 +15,15 @@ menuconfig KUNIT
>
> if KUNIT
>
> +config KUNIT_SUPPRESS_BACKTRACE
> + bool "KUnit - Enable backtrace suppression"
> + default y
> + help
> + Enable backtrace suppression for KUnit. If enabled, backtraces
> + generated intentionally by KUnit tests are suppressed. Disable
> + to reduce kernel image size if image size is more important than
> + suppression of backtraces generated by KUnit tests.
> +
> config KUNIT_DEBUGFS
> bool "KUnit - Enable /sys/kernel/debug/kunit debugfs representation"
> if !KUNIT_ALL_TESTS
> default KUNIT_ALL_TESTS
> diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
> index 309659a32a78..545b57c3be48 100644
> --- a/lib/kunit/Makefile
> +++ b/lib/kunit/Makefile
> @@ -14,8 +14,10 @@ ifeq ($(CONFIG_KUNIT_DEBUGFS),y)
> kunit-objs += debugfs.o
> endif
>
> -# KUnit 'hooks' are built-in even when KUnit is built as a module.
> -obj-y += hooks.o
> +# KUnit 'hooks' and bug handling are built-in even when KUnit is built
> +# as a module.
> +obj-y += hooks.o \
> + bug.o
>
> obj-$(CONFIG_KUNIT_TEST) += kunit-test.o
>
> diff --git a/lib/kunit/bug.c b/lib/kunit/bug.c
> new file mode 100644
> index ..f93544d7a9d1
> --- /dev/null
> +++ b/lib/kunit/bug.c
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit helpers for backtrace suppression
> + *
> + * Copyright (c) 2024 Guenter Roeck
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +static LIST_HEAD(suppressed_warnings);
> +
> +void __start_suppress_warning(struct __suppressed_warning *warning)
> +{
> + list_add(>node, _warnings);
> +}
> +EXPORT_SYMBOL_GPL(__start_suppress_warning);
> +
> +void __end_suppress_warning(struct __suppressed_warning *warning)
> +{
> + list_del(>node);
> +}
> +EXPORT_SYMBOL_GPL(__end_suppress_warning);
> +
> +bool __is_suppressed_warning(const char *function)
> +{
> + struct __suppressed_warning *warning;
> +
> + if (!function)
> + return false;
> +
> + list_for_each_entry(warning, _warnings, node) {
> + if (!strcmp(function, warning->function))
> + return true;
> + }
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(__is_suppressed_warning);
> --
> 2.39.2
>
Thanks,
-- David
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [PATCH v13 2/8] mm/gup: Introduce check_and_migrate_movable_folios()
On 04.04.24 09:26, Vivek Kasireddy wrote:
This helper is the folio equivalent of check_and_migrate_movable_pages().
Therefore, all the rules that apply to check_and_migrate_movable_pages()
also apply to this one as well. Currently, this helper is only used by
memfd_pin_folios().
This patch also includes changes to rename and convert the internal
functions collect_longterm_unpinnable_pages() and
migrate_longterm_unpinnable_pages() to work on folios. As a result,
check_and_migrate_movable_pages() is now a wrapper around
check_and_migrate_movable_folios().
Cc: David Hildenbrand
Cc: Matthew Wilcox
Cc: Christoph Hellwig
Cc: Jason Gunthorpe
Cc: Peter Xu
Suggested-by: David Hildenbrand
Signed-off-by: Vivek Kasireddy
---
[...]
+/*
+ * Check whether all folios are *allowed* to be pinned indefinitely (longterm).
+ * Rather confusingly, all folios in the range are required to be pinned via
+ * FOLL_PIN, before calling this routine.
+ *
+ * If any folios in the range are not allowed to be pinned, then this routine
+ * will migrate those folios away, unpin all the folios in the range and return
+ * -EAGAIN. The caller should re-pin the entire range with FOLL_PIN and then
+ * call this routine again.
+ *
+ * If an error other than -EAGAIN occurs, this indicates a migration failure.
+ * The caller should give up, and propagate the error back up the call stack.
+ *
+ * If everything is OK and all folios in the range are allowed to be pinned,
+ * then this routine leaves all folios pinned and returns zero for success.
+ */
+static long check_and_migrate_movable_folios(unsigned long nr_folios,
+struct folio **folios)
+{
+ unsigned long collected;
+ LIST_HEAD(movable_folio_list);
+
+ collected = collect_longterm_unpinnable_folios(_folio_list,
+ nr_folios, folios);
+ if (!collected)
+ return 0;
+
+ return migrate_longterm_unpinnable_folios(_folio_list,
+ nr_folios, folios);
+}
+
/*
* Check whether all pages are *allowed* to be pinned. Rather confusingly, all
* pages in the range are required to be pinned via FOLL_PIN, before calling
Likely we should just drop that comment and refer to
check_and_migrate_movable_folios() instead. No need to duplicate all that.
@@ -2555,16 +2585,20 @@ static int migrate_longterm_unpinnable_pages(
static long check_and_migrate_movable_pages(unsigned long nr_pages,
struct page **pages)
{
- unsigned long collected;
- LIST_HEAD(movable_page_list);
+ struct folio **folios;
+ long i, ret;
- collected = collect_longterm_unpinnable_pages(_page_list,
- nr_pages, pages);
- if (!collected)
- return 0;
+ folios = kmalloc_array(nr_pages, sizeof(*folios), GFP_KERNEL);
+ if (!folios)
+ return -ENOMEM;
- return migrate_longterm_unpinnable_pages(_page_list, nr_pages,
- pages);
+ for (i = 0; i < nr_pages; i++)
+ folios[i] = page_folio(pages[i]);
I wonder if we have to handle pages[i] being NULL. Hopefully not :)
Looks straight forward now:
Acked-by: David Hildenbrand
--
Cheers,
David / dhildenb
Re: [PATCH v13 1/8] mm/gup: Introduce unpin_folio/unpin_folios helpers
On 04.04.24 09:26, Vivek Kasireddy wrote: These helpers are the folio versions of unpin_user_page/unpin_user_pages. They are currently only useful for unpinning folios pinned by memfd_pin_folios() or other associated routines. However, they could find new uses in the future, when more and more folio-only helpers are added to GUP. We should probably sanity check the folio as part of unpin similar to how it is done in unpin_user_page/unpin_user_pages but we cannot cleanly do that at the moment without also checking the subpage. Therefore, sanity checking needs to be added to these routines once we have a way to determine if any given folio is anon-exclusive (via a per folio AnonExclusive flag). Cc: David Hildenbrand Cc: Matthew Wilcox Cc: Christoph Hellwig Cc: Jason Gunthorpe Cc: Peter Xu Suggested-by: David Hildenbrand Signed-off-by: Vivek Kasireddy --- Reviewed-by: David Hildenbrand -- Cheers, David / dhildenb
Re: [PATCH v12 2/8] mm/gup: Introduce check_and_migrate_movable_folios()
On 25.02.24 08:56, Vivek Kasireddy wrote:
This helper is the folio equivalent of check_and_migrate_movable_pages().
Therefore, all the rules that apply to check_and_migrate_movable_pages()
also apply to this one as well. Currently, this helper is only used by
memfd_pin_folios().
This patch also includes changes to rename and convert the internal
functions collect_longterm_unpinnable_pages() and
migrate_longterm_unpinnable_pages() to work on folios. Since they
are also used by check_and_migrate_movable_pages(), a temporary
array is used to collect and share the folios with these functions.
Cc: David Hildenbrand
Cc: Matthew Wilcox
Cc: Christoph Hellwig
Cc: Jason Gunthorpe
Cc: Peter Xu
Suggested-by: David Hildenbrand
Signed-off-by: Vivek Kasireddy
---
mm/gup.c | 129 +++
1 file changed, 92 insertions(+), 37 deletions(-)
diff --git a/mm/gup.c b/mm/gup.c
index 0a45eda6aaeb..1410af954a4e 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2099,20 +2099,24 @@ struct page *get_dump_page(unsigned long addr)
#ifdef CONFIG_MIGRATION
/*
- * Returns the number of collected pages. Return value is always >= 0.
+ * Returns the number of collected folios. Return value is always >= 0.
*/
-static unsigned long collect_longterm_unpinnable_pages(
- struct list_head *movable_page_list,
- unsigned long nr_pages,
+static unsigned long collect_longterm_unpinnable_folios(
+ struct list_head *movable_folio_list,
+ unsigned long nr_folios,
+ struct folio **folios,
struct page **pages)
This function really shouldn't consume both folios and pages.
Either use "folios" and handle the conversion from pages->folios in the
caller, or handle it similar to release_pages() where we can pass either
and simply always do page_folio() on the given pointer, using
essentially an abstracted pointer type and always calling page_folio()
on that thing.
The easiest is likely to just do the page->folio conversion in the
caller by looping over the arrays once more. See below.
Temporary memory allocation can be avoided by using an abstracted
pointer type.
[...]
+ folio = folios[i];
if (folio == prev_folio)
continue;
prev_folio = folio;
@@ -2126,7 +2130,7 @@ static unsigned long collect_longterm_unpinnable_pages(
continue;
if (folio_test_hugetlb(folio)) {
- isolate_hugetlb(folio, movable_page_list);
+ isolate_hugetlb(folio, movable_folio_list);
continue;
}
@@ -2138,7 +2142,7 @@ static unsigned long collect_longterm_unpinnable_pages(
if (!folio_isolate_lru(folio))
continue;
- list_add_tail(>lru, movable_page_list);
+ list_add_tail(>lru, movable_folio_list);
node_stat_mod_folio(folio,
NR_ISOLATED_ANON + folio_is_file_lru(folio),
folio_nr_pages(folio));
@@ -2148,27 +2152,28 @@ static unsigned long collect_longterm_unpinnable_pages(
}
/*
- * Unpins all pages and migrates device coherent pages and movable_page_list.
- * Returns -EAGAIN if all pages were successfully migrated or -errno for
failure
- * (or partial success).
+ * Unpins all folios and migrates device coherent folios and
movable_folio_list.
+ * Returns -EAGAIN if all folios were successfully migrated or -errno for
+ * failure (or partial success).
*/
-static int migrate_longterm_unpinnable_pages(
- struct list_head *movable_page_list,
- unsigned long nr_pages,
- struct page **pages)
+static int migrate_longterm_unpinnable_folios(
+ struct list_head *movable_folio_list,
+ unsigned long nr_folios,
+ struct folio **folios)
{
int ret;
unsigned long i;
- for (i = 0; i < nr_pages; i++) {
- struct folio *folio = page_folio(pages[i]);
+ for (i = 0; i < nr_folios; i++) {
+ struct folio *folio = folios[i];
if (folio_is_device_coherent(folio)) {
/*
-* Migration will fail if the page is pinned, so convert
-* the pin on the source page to a normal reference.
+* Migration will fail if the folio is pinned, so
+* convert the pin on the source folio to a normal
+* reference.
*/
-
Re: [PATCH v12 1/8] mm/gup: Introduce unpin_folio/unpin_folios helpers
On 02.04.24 15:52, David Hildenbrand wrote:
On 25.02.24 08:56, Vivek Kasireddy wrote:
These helpers are the folio versions of unpin_user_page/unpin_user_pages.
They are currently only useful for unpinning folios pinned by
memfd_pin_folios() or other associated routines. However, they could
find new uses in the future, when more and more folio-only helpers
are added to GUP.
Cc: David Hildenbrand
Cc: Matthew Wilcox
Cc: Christoph Hellwig
Cc: Jason Gunthorpe
Cc: Peter Xu
Suggested-by: David Hildenbrand
Signed-off-by: Vivek Kasireddy
---
include/linux/mm.h | 2 ++
mm/gup.c | 81 --
2 files changed, 74 insertions(+), 9 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 6f4825d82965..36e4c2b22600 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1601,11 +1601,13 @@ static inline void put_page(struct page *page)
#define GUP_PIN_COUNTING_BIAS (1U << 10)
void unpin_user_page(struct page *page);
+void unpin_folio(struct folio *folio);
void unpin_user_pages_dirty_lock(struct page **pages, unsigned long npages,
bool make_dirty);
void unpin_user_page_range_dirty_lock(struct page *page, unsigned long
npages,
bool make_dirty);
void unpin_user_pages(struct page **pages, unsigned long npages);
+void unpin_folios(struct folio **folios, unsigned long nfolios);
static inline bool is_cow_mapping(vm_flags_t flags)
{
diff --git a/mm/gup.c b/mm/gup.c
index df83182ec72d..0a45eda6aaeb 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -30,6 +30,23 @@ struct follow_page_context {
unsigned int page_mask;
};
+static inline void sanity_check_pinned_folios(struct folio **folios,
+ unsigned long nfolios)
+{
+ if (!IS_ENABLED(CONFIG_DEBUG_VM))
+ return;
+
+ for (; nfolios; nfolios--, folios++) {
+ struct folio *folio = *folios;
+
+ if (is_zero_folio(folio) ||
+ !folio_test_anon(folio))
+ continue;
+
+ VM_BUG_ON_FOLIO(!PageAnonExclusive(>page), folio);
That change is wrong (and the split makes the check confusing).
It could be that the first subpage is no longer exclusive, but the given
(sanity_check_pinned_pages() ) subpage is exclusive for large folios.
I suggest dropping that change, and instead, in
unpin_folio()/unpin_folios(), reject any anon folios for now.
So, replace the sanity_check_pinned_folios() in unpin_folio() /
unpin_folios() by a VM_WARN_ON(folio_test_anon(folio));
After reading patch #2: drop both the sanity check and VM_WARN_ON() from
unpin_folio()/unpin_folios(), and add a comment to the patch description
that we cannot do the sanity checking without the subpage, and that we
can reintroduce it once we have a single per-folio AnonExclusive bit.
--
Cheers,
David / dhildenb
Re: [PATCH v12 1/8] mm/gup: Introduce unpin_folio/unpin_folios helpers
On 25.02.24 08:56, Vivek Kasireddy wrote:
These helpers are the folio versions of unpin_user_page/unpin_user_pages.
They are currently only useful for unpinning folios pinned by
memfd_pin_folios() or other associated routines. However, they could
find new uses in the future, when more and more folio-only helpers
are added to GUP.
Cc: David Hildenbrand
Cc: Matthew Wilcox
Cc: Christoph Hellwig
Cc: Jason Gunthorpe
Cc: Peter Xu
Suggested-by: David Hildenbrand
Signed-off-by: Vivek Kasireddy
---
include/linux/mm.h | 2 ++
mm/gup.c | 81 --
2 files changed, 74 insertions(+), 9 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 6f4825d82965..36e4c2b22600 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1601,11 +1601,13 @@ static inline void put_page(struct page *page)
#define GUP_PIN_COUNTING_BIAS (1U << 10)
void unpin_user_page(struct page *page);
+void unpin_folio(struct folio *folio);
void unpin_user_pages_dirty_lock(struct page **pages, unsigned long npages,
bool make_dirty);
void unpin_user_page_range_dirty_lock(struct page *page, unsigned long npages,
bool make_dirty);
void unpin_user_pages(struct page **pages, unsigned long npages);
+void unpin_folios(struct folio **folios, unsigned long nfolios);
static inline bool is_cow_mapping(vm_flags_t flags)
{
diff --git a/mm/gup.c b/mm/gup.c
index df83182ec72d..0a45eda6aaeb 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -30,6 +30,23 @@ struct follow_page_context {
unsigned int page_mask;
};
+static inline void sanity_check_pinned_folios(struct folio **folios,
+ unsigned long nfolios)
+{
+ if (!IS_ENABLED(CONFIG_DEBUG_VM))
+ return;
+
+ for (; nfolios; nfolios--, folios++) {
+ struct folio *folio = *folios;
+
+ if (is_zero_folio(folio) ||
+ !folio_test_anon(folio))
+ continue;
+
+ VM_BUG_ON_FOLIO(!PageAnonExclusive(>page), folio);
That change is wrong (and the split makes the check confusing).
It could be that the first subpage is no longer exclusive, but the given
(sanity_check_pinned_pages() ) subpage is exclusive for large folios.
I suggest dropping that change, and instead, in
unpin_folio()/unpin_folios(), reject any anon folios for now.
So, replace the sanity_check_pinned_folios() in unpin_folio() /
unpin_folios() by a VM_WARN_ON(folio_test_anon(folio));
It will all be better once we have a single anon-exclusive flag per
folio (which I am working on), but in the meantime, we really don't
expect code that called pin_user_pages() to call unpin_folios().
[...]
+/**
+ * unpin_folio() - release a dma-pinned folio
+ * @folio: pointer to folio to be released
+ *
+ * Folios that were pinned via memfd_pin_folios() or other similar routines
+ * must be released either using unpin_folio() or unpin_folios(). This is so
+ * that such folios can be separately tracked and uniquely handled.
I'd drop the last sentence; no need for apologies/explanations, this is
simply how ;pinning works :)
+ */
+void unpin_folio(struct folio *folio)
+{
+ sanity_check_pinned_folios(, 1);
+ gup_put_folio(folio, 1, FOLL_PIN);
+}
+EXPORT_SYMBOL(unpin_folio);
Can we restrict that to EXPORT_SYMBOL_GPL for now? memfd_pin_folios()
uses EXPORT_SYMBOL_GPL...
+
/**
* folio_add_pin - Try to get an additional pin on a pinned folio
* @folio: The folio to be pinned
@@ -488,6 +516,41 @@ void unpin_user_pages(struct page **pages, unsigned long
npages)
}
EXPORT_SYMBOL(unpin_user_pages);
+/**
+ * unpin_folios() - release an array of gup-pinned folios.
+ * @folios: array of folios to be marked dirty and released.
+ * @nfolios: number of folios in the @folios array.
+ *
+ * For each folio in the @folios array, release the folio using unpin_folio().
+ *
+ * Please see the unpin_folio() documentation for details.
+ */
+void unpin_folios(struct folio **folios, unsigned long nfolios)
+{
+ unsigned long i = 0, j;
+
+ /*
+* If this WARN_ON() fires, then the system *might* be leaking folios
+* (by leaving them pinned), but probably not. More likely, gup/pup
+* returned a hard -ERRNO error to the caller, who erroneously passed
+* it here.
+*/
+ if (WARN_ON(IS_ERR_VALUE(nfolios)))
+ return;
+
+ sanity_check_pinned_folios(folios, nfolios);
+ while (i < nfolios) {
+ for (j = i + 1; j < nfolios; j++)
+ if (folios[i] != folios[j])
+ break;
+
+ if (folios[i])
+ gup_put_folio(folios[i], j - i, FOLL_PIN);
+ i = j;
+ }
+}
+EXPORT_SYMBOL(unpin_folios);
Same thought here.
--
Cheers,
David / dhildenb
Re: [PATCH v12 0/8] mm/gup: Introduce memfd_pin_folios() for pinning memfd folios
On 29.03.24 06:38, Kasireddy, Vivek wrote: Hi David, On 25.02.24 08:56, Vivek Kasireddy wrote: Currently, some drivers (e.g, Udmabuf) that want to longterm-pin the pages/folios associated with a memfd, do so by simply taking a reference on them. This is not desirable because the pages/folios may reside in Movable zone or CMA block. Therefore, having drivers use memfd_pin_folios() API ensures that the folios are appropriately pinned via FOLL_PIN for longterm DMA. This patchset also introduces a few helpers and converts the Udmabuf driver to use folios and memfd_pin_folios() API to longterm-pin the folios for DMA. Two new Udmabuf selftests are also included to test the driver and the new API. --- Sorry Vivek, I got distracted. What's the state of this? I assume it's not in an mm tree yet. No problem. Right, they are not in any tree yet. The first two mm patches that add the unpin_folios() and check_and_migrate_movable_folios() helpers still need to be reviewed. I try to get this reviewed this week. If I fail to do that, please ping me. Ok, sounds good! .. as it's already Friday (and even a public Holiday today+Monday here), let me prioritize this for next week! -- Cheers, David / dhildenb
Re: [PATCH 4/4] drm/tiny: st7586: drop driver owner assignment
On 3/27/24 12:48 PM, Krzysztof Kozlowski wrote: > Core in spi_register_driver() already sets the .owner, so driver > does not need to. > > Signed-off-by: Krzysztof Kozlowski > --- Acked-by: David Lechner
Re: [PATCH 1/4] drm/tiny: ili9225: drop driver owner assignment
On 3/27/24 12:48 PM, Krzysztof Kozlowski wrote: > Core in spi_register_driver() already sets the .owner, so driver > does not need to. > > Signed-off-by: Krzysztof Kozlowski > --- Acked-by: David Lechner
Re: [PATCH v12 0/8] mm/gup: Introduce memfd_pin_folios() for pinning memfd folios
On 25.02.24 08:56, Vivek Kasireddy wrote: Currently, some drivers (e.g, Udmabuf) that want to longterm-pin the pages/folios associated with a memfd, do so by simply taking a reference on them. This is not desirable because the pages/folios may reside in Movable zone or CMA block. Therefore, having drivers use memfd_pin_folios() API ensures that the folios are appropriately pinned via FOLL_PIN for longterm DMA. This patchset also introduces a few helpers and converts the Udmabuf driver to use folios and memfd_pin_folios() API to longterm-pin the folios for DMA. Two new Udmabuf selftests are also included to test the driver and the new API. --- Sorry Vivek, I got distracted. What's the state of this? I assume it's not in an mm tree yet. I try to get this reviewed this week. If I fail to do that, please ping me. -- Cheers, David / dhildenb
Re: [RFC PATCH net-next v6 02/15] net: page_pool: create hooks for custom page providers
On 2024-03-17 19:02, Christoph Hellwig wrote: > On Mon, Mar 04, 2024 at 06:01:37PM -0800, Mina Almasry wrote: >> From: Jakub Kicinski >> >> The page providers which try to reuse the same pages will >> need to hold onto the ref, even if page gets released from >> the pool - as in releasing the page from the pp just transfers >> the "ownership" reference from pp to the provider, and provider >> will wait for other references to be gone before feeding this >> page back into the pool. > > The word hook always rings a giant warning bell for me, and looking into > this series I am concerned indeed. > > The only provider provided here is the dma-buf one, and that basically > is the only sensible one for the documented design. So instead of > adding hooks that random proprietary crap can hook into, why not hard > code the dma buf provide and just use a flag? That'll also avoid > expensive indirect calls. I'm working on a similar proposal for zero copy Rx but to host memory and depend on this memory provider API. Jakub also designed this API for hugepages too IIRC. Basically there's going to be at least three fancy ways of providing pages (one of which isn't actually pages, hence the merged netmem_t series) to drivers. >
Re: [PATCH] drm/nouveau/dp: Fix incorrect return code in r535_dp_aux_xfer()
Reviewed-by: Dave Airlie
On Sat, Mar 16, 2024 at 7:21 AM Lyude Paul wrote:
>
> I've recently been seeing some unexplained GSP errors on my RTX 6000 from
> failed aux transactions:
>
> [ 132.915867] nouveau :1f:00.0: gsp: cli:0xc1d2 obj:0x0073
> ctrl cmd:0x00731341 failed: 0x
>
> While the cause of these is not yet clear, these messages made me notice
> that the aux transactions causing these transactions were succeeding - not
> failing. As it turns out, this is because we're currently not returning the
> correct variable when r535_dp_aux_xfer() hits an error - causing us to
> never propagate GSP errors for failed aux transactions to userspace.
>
> So, let's fix that.
>
> Fixes: 4ae3a20102b2 ("nouveau/gsp: don't free ctrl messages on errors")
> Signed-off-by: Lyude Paul
> ---
> drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c
> b/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c
> index 6a0a4d3b8902d..027867c2a8c5b 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c
> @@ -1080,7 +1080,7 @@ r535_dp_aux_xfer(struct nvkm_outp *outp, u8 type, u32
> addr, u8 *data, u8 *psize)
> ret = nvkm_gsp_rm_ctrl_push(>rm.objcom, , sizeof(*ctrl));
> if (ret) {
> nvkm_gsp_rm_ctrl_done(>rm.objcom, ctrl);
> - return PTR_ERR(ctrl);
> + return ret;
> }
>
> memcpy(data, ctrl->data, size);
> --
> 2.43.0
>
Re: [PATCH 41/43] drm/tiny/st7735r: Use fbdev-dma
On 3/12/24 10:45 AM, Thomas Zimmermann wrote: > Implement fbdev emulation with fbdev-dma. Fbdev-dma now supports > damage handling, which is required by st7735r. Avoids the overhead of > fbdev-generic's additional shadow buffering. No functional changes. > > Signed-off-by: Thomas Zimmermann > Cc: David Lechner > --- Acked-by: David Lechner
Re: [PATCH 34/43] drm/tiny/ili9225: Use fbdev-dma
On 3/12/24 10:45 AM, Thomas Zimmermann wrote: > Implement fbdev emulation with fbdev-dma. Fbdev-dma now supports > damage handling, which is required by ili9225. Avoids the overhead of > fbdev-generic's additional shadow buffering. No functional changes. > > Signed-off-by: Thomas Zimmermann > Cc: David Lechner > --- Acked-by: David Lechner
Re: [PATCH 40/43] drm/tiny/st7586: Use fbdev-dma
On 3/12/24 10:45 AM, Thomas Zimmermann wrote: > Implement fbdev emulation with fbdev-dma. Fbdev-dma now supports > damage handling, which is required by st7586. Avoids the overhead of > fbdev-generic's additional shadow buffering. No functional changes. > > Signed-off-by: Thomas Zimmermann > Cc: David Lechner > --- Acked-by: David Lechner
Re: [RFC PATCH net-next v6 01/15] queue_api: define queue api
On 3/8/24 4:47 PM, David Wei wrote: > > I'm working to port bnxt over to using this API. What are your thoughts > on maybe pulling this out and use bnxt to drive it? > I would love to see a second nic implementation; this patch set and overall design is driven by GVE limitations.
RE: arm: ERROR: modpost: "__aeabi_uldivmod" [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined!
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-mrip...@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);
The code should really use u32 and u64.
Otherwise the sizes are different on 32bit.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
Re: [RFC PATCH net-next v6 01/15] queue_api: define queue api
On 2024-03-04 18:01, Mina Almasry wrote:
> This API enables the net stack to reset the queues used for devmem.
>
> Signed-off-by: Mina Almasry
>
> ---
> include/linux/netdevice.h | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index c41019f34179..3105c586355d 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1435,6 +1435,20 @@ struct netdev_net_notifier {
> * struct kernel_hwtstamp_config *kernel_config,
> * struct netlink_ext_ack *extack);
> * Change the hardware timestamping parameters for NIC device.
> + *
> + * void *(*ndo_queue_mem_alloc)(struct net_device *dev, int idx);
> + * Allocate memory for an RX queue. The memory returned in the form of
> + * a void * can be passed to ndo_queue_mem_free() for freeing or to
> + * ndo_queue_start to create an RX queue with this memory.
> + *
> + * void (*ndo_queue_mem_free)(struct net_device *dev, void *);
> + * Free memory from an RX queue.
> + *
> + * int (*ndo_queue_start)(struct net_device *dev, int idx, void *);
> + * Start an RX queue at the specified index.
> + *
> + * int (*ndo_queue_stop)(struct net_device *dev, int idx, void **);
> + * Stop the RX queue at the specified index.
> */
> struct net_device_ops {
> int (*ndo_init)(struct net_device *dev);
> @@ -1679,6 +1693,16 @@ struct net_device_ops {
> int (*ndo_hwtstamp_set)(struct net_device *dev,
> struct
> kernel_hwtstamp_config *kernel_config,
> struct netlink_ext_ack
> *extack);
> + void * (*ndo_queue_mem_alloc)(struct net_device *dev,
> +int idx);
> + void(*ndo_queue_mem_free)(struct net_device *dev,
> + void *queue_mem);
> + int (*ndo_queue_start)(struct net_device *dev,
> +int idx,
> +void *queue_mem);
> + int (*ndo_queue_stop)(struct net_device *dev,
> + int idx,
> + void **out_queue_mem);
> };
I'm working to port bnxt over to using this API. What are your thoughts
on maybe pulling this out and use bnxt to drive it?
>
> /**
Re: [RFC PATCH net-next v6 02/15] net: page_pool: create hooks for custom page providers
On 2024-03-04 18:01, Mina Almasry wrote:
> From: Jakub Kicinski
>
> The page providers which try to reuse the same pages will
> need to hold onto the ref, even if page gets released from
> the pool - as in releasing the page from the pp just transfers
> the "ownership" reference from pp to the provider, and provider
> will wait for other references to be gone before feeding this
> page back into the pool.
>
> Signed-off-by: Jakub Kicinski
> Signed-off-by: Mina Almasry
>
> ---
>
> This is implemented by Jakub in his RFC:
> https://lore.kernel.org/netdev/f8270765-a27b-6ccf-33ea-cda097168...@redhat.com/T/
>
> I take no credit for the idea or implementation; I only added minor
> edits to make this workable with device memory TCP, and removed some
> hacky test code. This is a critical dependency of device memory TCP
> and thus I'm pulling it into this series to make it revewable and
> mergeable.
>
> RFC v3 -> v1
> - Removed unusued mem_provider. (Yunsheng).
> - Replaced memory_provider & mp_priv with netdev_rx_queue (Jakub).
>
> ---
> include/net/page_pool/types.h | 12 ++
> net/core/page_pool.c | 43 +++
> 2 files changed, 50 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h
> index 5e43a08d3231..ffe5f31fb0da 100644
> --- a/include/net/page_pool/types.h
> +++ b/include/net/page_pool/types.h
> @@ -52,6 +52,7 @@ struct pp_alloc_cache {
> * @dev: device, for DMA pre-mapping purposes
> * @netdev: netdev this pool will serve (leave as NULL if none or multiple)
> * @napi:NAPI which is the sole consumer of pages, otherwise NULL
> + * @queue: struct netdev_rx_queue this page_pool is being created for.
> * @dma_dir: DMA mapping direction
> * @max_len: max DMA sync memory size for PP_FLAG_DMA_SYNC_DEV
> * @offset: DMA sync address offset for PP_FLAG_DMA_SYNC_DEV
> @@ -64,6 +65,7 @@ struct page_pool_params {
> int nid;
> struct device *dev;
> struct napi_struct *napi;
> + struct netdev_rx_queue *queue;
> enum dma_data_direction dma_dir;
> unsigned intmax_len;
> unsigned intoffset;
> @@ -126,6 +128,13 @@ struct page_pool_stats {
> };
> #endif
>
> +struct memory_provider_ops {
> + int (*init)(struct page_pool *pool);
> + void (*destroy)(struct page_pool *pool);
> + struct page *(*alloc_pages)(struct page_pool *pool, gfp_t gfp);
> + bool (*release_page)(struct page_pool *pool, struct page *page);
> +};
Separate question as I try to adapt bnxt to this and your queue
configuration API.
How does GVE handle the need to allocate kernel pages for headers and
dmabuf for payloads?
Reading the code, struct gve_rx_ring is the main per-ring object with a
page pool. gve_queue_page_lists are filled with page pool netmem
allocations from the page pool in gve_alloc_queue_page_list(). Are these
strictly used for payloads only?
I found a struct gve_header_buf in both gve_rx_ring and struct
gve_per_rx_queue_mem_dpo. This is allocated in gve_rx_queue_mem_alloc()
using dma_alloc_coherent(). Is this where GVE stores headers?
IOW, GVE only uses page pool to allocate memory for QPLs, and QPLs are
used by the device for split payloads. Is my understanding correct?
> +
> struct page_pool {
> struct page_pool_params_fast p;
>
> @@ -176,6 +185,9 @@ struct page_pool {
>*/
> struct ptr_ring ring;
>
> + void *mp_priv;
> + const struct memory_provider_ops *mp_ops;
> +
> #ifdef CONFIG_PAGE_POOL_STATS
> /* recycle stats are per-cpu to avoid locking */
> struct page_pool_recycle_stats __percpu *recycle_stats;
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index d706fe5548df..8776fcad064a 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -25,6 +25,8 @@
>
> #include "page_pool_priv.h"
>
> +static DEFINE_STATIC_KEY_FALSE(page_pool_mem_providers);
> +
> #define DEFER_TIME (msecs_to_jiffies(1000))
> #define DEFER_WARN_INTERVAL (60 * HZ)
>
> @@ -177,6 +179,7 @@ static int page_pool_init(struct page_pool *pool,
> int cpuid)
> {
> unsigned int ring_qsize = 1024; /* Default */
> + int err;
>
> memcpy(>p, >fast, sizeof(pool->p));
> memcpy(>slow, >slow, sizeof(pool->slow));
> @@ -248,10 +251,25 @@ static int page_pool_init(struct page_pool *pool,
> /* Driver calling page_pool_create() also call page_pool_destroy() */
> refcount_set(>user_cnt, 1);
>
> + if (pool->mp_ops) {
> + err = pool->mp_ops->init(pool);
> + if (err) {
> + pr_warn("%s() mem-provider init failed %d\n",
> + __func__, err);
> + goto free_ptr_ring;
> + }
> +
> + static_branch_inc(_pool_mem_providers);
> + }
> +
> if (pool->p.flags &
Re: [RFC PATCH net-next v6 09/15] memory-provider: dmabuf devmem memory provider
On 2024-03-05 18:42, Mina Almasry wrote:
> On Tue, Mar 5, 2024 at 6:28 PM David Wei wrote:
>>
>> On 2024-03-04 18:01, Mina Almasry wrote:
>>> + if (pool->p.queue)
>>> + binding = READ_ONCE(pool->p.queue->binding);
>>> +
>>> + if (binding) {
>>> + pool->mp_ops = _devmem_ops;
>>> + pool->mp_priv = binding;
>>> + }
>>
>> This is specific to TCP devmem. For ZC Rx we will need something more
>> generic to let us pass our own memory provider backend down to the page
>> pool.
>>
>> What about storing ops and priv void ptr in struct netdev_rx_queue
>> instead? Then we can both use it.
>
> Yes, this is dmabuf specific, I was thinking you'd define your own
> member of netdev_rx_queue, and then add something like this to
> page_pool_init:
>
> + if (pool->p.queue)
> + io_uring_metadata =
> READ_ONCE(pool->p.queue->io_uring_metadata);
> +
> + /* We don't support rx-queues that are configured for both
> io_uring & dmabuf binding */
> + BUG_ON(io_uring_metadata && binding);
> +
> + if (io_uring_metadata) {
> + pool->mp_ops = _uring_ops;
> + pool->mp_priv = io_uring_metadata;
> + }
>
> I.e., we share the pool->mp_ops and the pool->mp_priv but we don't
> really need to share the same netdev_rx_queue member. For me it's a
> dma-buf specific data structure (netdev_dmabuf_binding) and for you
> it's something else.
This adds size to struct netdev_rx_queue and requires checks on whether
both are set. There can be thousands of these structs at any one time so
if we don't need to add size unnecessarily then that would be best.
We can disambiguate by comparing _ops and then cast the void ptr to
our impl specific objects.
What do you not like about this approach?
>
> page_pool_init() probably needs to validate that the queue is
> configured for dma-buf or io_uring but not both. If it's configured
> for both then the user is doing something funky we shouldn't support.
>
> Perhaps I can make the intention clearer by renaming 'binding' to
> something more specific to dma-buf like queue->dmabuf_binding, to make
> it clear that this is the dma-buf binding and not some other binding
> like io_uring?
>
Re: [RFC PATCH net-next v6 09/15] memory-provider: dmabuf devmem memory provider
On 2024-03-04 18:01, Mina Almasry wrote:
> + if (pool->p.queue)
> + binding = READ_ONCE(pool->p.queue->binding);
> +
> + if (binding) {
> + pool->mp_ops = _devmem_ops;
> + pool->mp_priv = binding;
> + }
This is specific to TCP devmem. For ZC Rx we will need something more
generic to let us pass our own memory provider backend down to the page
pool.
What about storing ops and priv void ptr in struct netdev_rx_queue
instead? Then we can both use it.
Re: [RFC PATCH net-next v6 02/15] net: page_pool: create hooks for custom page providers
On 2024-03-04 18:01, Mina Almasry wrote:
> +struct memory_provider_ops {
> + int (*init)(struct page_pool *pool);
> + void (*destroy)(struct page_pool *pool);
> + struct page *(*alloc_pages)(struct page_pool *pool, gfp_t gfp);
> + bool (*release_page)(struct page_pool *pool, struct page *page);
For ZC Rx we added a scrub() function to memory_provider_ops that is
called from page_pool_scrub(). Does TCP devmem not custom behaviour
waiting for all netmem_refs to return before destroying the page pool?
What happens if e.g. application crashes?
Re: [PATCH] drm/ci: update device type for volteer devices
Reviewed-by: David Heidelberg If possible, please merge this ASAP, because major move of most of the devices to type acer-cp514-2h-1130g7-volteer will happen tomorrow. Thank you On 05/03/2024 11:16, Vignesh Raman wrote: Volteer devices in the collabora lab are categorized under the asus-cx9400-volteer device type. The majority of these units has an Intel Core i5-1130G7 CPU, while some of them have a Intel Core i7-1160G7 CPU instead. So due to this difference, new device type template is added for the Intel Core i5-1130G7 and i7-1160G7 variants of the Acer Chromebook Spin 514 (CP514-2H) volteer Chromebooks. So update the same in drm-ci. https://gitlab.collabora.com/lava/lava/-/merge_requests/149 Signed-off-by: Vignesh Raman --- drivers/gpu/drm/ci/test.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/ci/test.yml b/drivers/gpu/drm/ci/test.yml index 0857773e5c5f..8bc63912fddb 100644 --- a/drivers/gpu/drm/ci/test.yml +++ b/drivers/gpu/drm/ci/test.yml @@ -252,11 +252,11 @@ i915:cml: i915:tgl: extends: - .i915 - parallel: 8 + parallel: 5 variables: -DEVICE_TYPE: asus-cx9400-volteer +DEVICE_TYPE: acer-cp514-2h-1130g7-volteer GPU_VERSION: tgl -RUNNER_TAG: mesa-ci-x86-64-lava-asus-cx9400-volteer +RUNNER_TAG: mesa-ci-x86-64-lava-acer-cp514-2h-1130g7-volteer .amdgpu: extends: -- David Heidelberg Consultant Software Engineer Collabora Ltd. Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK Registered in England & Wales, no. 5513718 OpenPGP_0x69F567861C1EC014.asc Description: OpenPGP public key OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [RFC PATCH net-next v6 07/15] page_pool: convert to use netmem
Hi Mina, I recommend you cc linux-mm and Matthew Wilcox on these two patches also. David
Re: linux-next: build failure after merge of the kunit-next tree
On Thu, 29 Feb 2024 at 23:07, Shuah Khan wrote:
>
> Hi Stephen,
>
> On 2/28/24 21:26, Stephen Rothwell wrote:
> > Hi all,
> >
> > After merging the kunit-next tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:
> >
> > In file included from drivers/gpu/drm/tests/drm_buddy_test.c:7:
> > drivers/gpu/drm/tests/drm_buddy_test.c: In function
> > 'drm_test_buddy_alloc_range_bias':
> > drivers/gpu/drm/tests/drm_buddy_test.c:191:40: error: format '%u' expects a
> > matching 'unsigned int' argument [-Werror=format=]
> >191 |"buddy_alloc failed with
> > bias(%x-%x), size=%u, ps=%u\n",
> >|
> > ^~~
> > include/kunit/test.h:597:37: note: in definition of macro '_KUNIT_FAILED'
> >597 | fmt,
> > \
> >| ^~~
> > include/kunit/test.h:662:9: note: in expansion of macro
> > 'KUNIT_UNARY_ASSERTION'
> >662 | KUNIT_UNARY_ASSERTION(test,
> > \
> >| ^
> > include/kunit/test.h:1233:9: note: in expansion of macro
> > 'KUNIT_FALSE_MSG_ASSERTION'
> > 1233 | KUNIT_FALSE_MSG_ASSERTION(test,
> > \
> >| ^
> > drivers/gpu/drm/tests/drm_buddy_test.c:186:17: note: in expansion of macro
> > 'KUNIT_ASSERT_FALSE_MSG'
> >186 | KUNIT_ASSERT_FALSE_MSG(test,
> >| ^~
> > drivers/gpu/drm/tests/drm_buddy_test.c:191:91: note: format string is
> > defined here
> >191 |"buddy_alloc failed with
> > bias(%x-%x), size=%u, ps=%u\n",
> >|
> > ~^
> >|
> >|
> >|
> >unsigned int
> > cc1: all warnings being treated as errors
> >
> > Caused by commit
> >
> >806cb2270237 ("kunit: Annotate _MSG assertion variants with gnu printf
> > specifiers")
> >
>
> Thank you. I did allmodconfig build on kselftest kunit branch to make
> sure all is well, before I pushed the commits.
>
> > interacting with commit
> >
> >c70703320e55 ("drm/tests/drm_buddy: add alloc_range_bias test")
> >
> > from the drm-misc-fixes tree.
> >
> > I have applied the following patch for today (this should probably
> > actually be fixed in the drm-misc-fixes tree).
> >
>
> Danial, David,
>
> I can carry the fix through kselftest kunit if it works
> for all.
I'm happy for this to go in with the KUnit changes if that's the best
way to keep all of the printk formatting fixes together.
-- David
>
> > From: Stephen Rothwell
> > Date: Thu, 29 Feb 2024 15:18:36 +1100
> > Subject: [PATCH] fix up for "drm/tests/drm_buddy: add alloc_range_bias test"
> >
> > Signed-off-by: Stephen Rothwell
> > ---
> > drivers/gpu/drm/tests/drm_buddy_test.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c
> > b/drivers/gpu/drm/tests/drm_buddy_test.c
> > index 1e73e3f0d278..369edf587b44 100644
> > --- a/drivers/gpu/drm/tests/drm_buddy_test.c
> > +++ b/drivers/gpu/drm/tests/drm_buddy_test.c
> > @@ -188,7 +188,7 @@ static void drm_test_buddy_alloc_range_bias(struct
> > kunit *test)
> > bias_end, size,
> > ps,
> > ,
> >
> > DRM_BUDDY_RANGE_ALLOCATION),
> > -"buddy_alloc failed with bias(%x-%x),
> > size=%u, ps=%u\n",
> > +"buddy_alloc failed with bias(%x-%x),
> > size=%u\n",
> > bias_start, bias_end, size);
> > bias_rem -= size;
> >
>
> thanks,
> -- Shuah
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH v2] drm: tests: Fix invalid printf format specifiers in KUnit tests
The drm_buddy_test's alloc_contiguous test used a u64 for the page size,
which was then updated to be an 'unsigned long' to avoid 64-bit
multiplication division helpers.
However, the variable is logged by some KUNIT_ASSERT_EQ_MSG() using the
'%d' or '%llu' format specifiers, the former of which is always wrong,
and the latter is no longer correct now that ps is no longer a u64. Fix
these to all use '%lu'.
Also, drm_mm_test calls KUNIT_FAIL() with an empty string as the
message. gcc and clang warns if a printf format string is empty, so
give these some more detailed error messages, which should be more
useful anyway.
Fixes: a64056bb5a32 ("drm/tests/drm_buddy: add alloc_contiguous test")
Fixes: fca7526b7d89 ("drm/tests/drm_buddy: fix build failure on 32-bit targets")
Fixes: fc8d29e298cf ("drm: selftest: convert drm_mm selftest to KUnit")
Reviewed-by: Matthew Auld
Acked-by: Christian König
Tested-by: Guenter Roeck
Reviewed-by: Justin Stitt
Signed-off-by: David Gow
---
Changes since v1:
https://lore.kernel.org/linux-kselftest/20240221092728.1281499-8-david...@google.com/
- Split this patch out, as the others have been applied already.
- Rebase on 6.8-rc6
- Add everyone's {Reviewed,Acked,Tested}-by tags. Thanks!
---
drivers/gpu/drm/tests/drm_buddy_test.c | 14 +++---
drivers/gpu/drm/tests/drm_mm_test.c| 6 +++---
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c
b/drivers/gpu/drm/tests/drm_buddy_test.c
index 2f32fb2f12e7..3dbfa3078449 100644
--- a/drivers/gpu/drm/tests/drm_buddy_test.c
+++ b/drivers/gpu/drm/tests/drm_buddy_test.c
@@ -55,30 +55,30 @@ static void drm_test_buddy_alloc_contiguous(struct kunit
*test)
KUNIT_ASSERT_FALSE_MSG(test,
drm_buddy_alloc_blocks(, 0, mm_size,
ps, ps, list, 0),
- "buddy_alloc hit an error size=%u\n",
+ "buddy_alloc hit an error size=%lu\n",
ps);
} while (++i < n_pages);
KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
3 * ps, ps,
,
DRM_BUDDY_CONTIGUOUS_ALLOCATION),
- "buddy_alloc didn't error size=%u\n", 3 * ps);
+ "buddy_alloc didn't error size=%lu\n", 3 * ps);
drm_buddy_free_list(, );
KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
3 * ps, ps,
,
DRM_BUDDY_CONTIGUOUS_ALLOCATION),
- "buddy_alloc didn't error size=%u\n", 3 * ps);
+ "buddy_alloc didn't error size=%lu\n", 3 * ps);
KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
2 * ps, ps,
,
DRM_BUDDY_CONTIGUOUS_ALLOCATION),
- "buddy_alloc didn't error size=%u\n", 2 * ps);
+ "buddy_alloc didn't error size=%lu\n", 2 * ps);
drm_buddy_free_list(, );
KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
3 * ps, ps,
,
DRM_BUDDY_CONTIGUOUS_ALLOCATION),
- "buddy_alloc didn't error size=%u\n", 3 * ps);
+ "buddy_alloc didn't error size=%lu\n", 3 * ps);
/*
* At this point we should have enough contiguous space for 2 blocks,
* however they are never buddies (since we freed middle and right) so
@@ -87,13 +87,13 @@ static void drm_test_buddy_alloc_contiguous(struct kunit
*test)
KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
2 * ps, ps,
,
DRM_BUDDY_CONTIGUOUS_ALLOCATION),
- "buddy_alloc hit an error size=%u\n", 2 * ps);
+ "buddy_alloc hit an error size=%lu\n", 2 * ps);
drm_buddy_free_list(, );
KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
3 * ps, ps,
,
DRM_BUDDY_CONTIGUOUS_ALLOCATION),
- "buddy_alloc hit an error size=%u\n", 3 * ps);
+
RE: [PATCH next v2 03/11] minmax: Simplify signedness check
From: kernel test robot > Sent: 27 February 2024 01:34 > > kernel test robot noticed the following build warnings: > > [auto build test WARNING on drm-misc/drm-misc-next] > [also build test WARNING on linux/master mkl-can-next/testing kdave/for-next > akpm-mm/mm-nonmm-unstable > axboe-block/for-next linus/master v6.8-rc6 next-20240226] > [cannot apply to next-20240223 dtor-input/next dtor-input/for-linus > horms-ipvs/master] > [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/David-Laight/minmax-Put-all-the-clamp- > definitions-together/20240226-005902 > base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next > patch link: > https://lore.kernel.org/r/8657dd5c2264456f8a005520a3b90e2b%40AcuMS.aculab.com > patch subject: [PATCH next v2 03/11] minmax: Simplify signedness check > config: alpha-defconfig > (https://download.01.org/0day-ci/archive/20240227/202402270937.9kmO5PFt- > l...@intel.com/config) > compiler: alpha-linux-gcc (GCC) 13.2.0 > reproduce (this is a W=1 build): (https://download.01.org/0day- > ci/archive/20240227/202402270937.9kmo5pft-...@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 > | Closes: > https://lore.kernel.org/oe-kbuild-all/202402270937.9kmo5pft-...@intel.com/ > > All warnings (new ones prefixed by >>): > >In file included from include/linux/kernel.h:28, > from include/linux/cpumask.h:10, > from include/linux/smp.h:13, > from include/linux/lockdep.h:14, > from include/linux/spinlock.h:63, > from include/linux/swait.h:7, > from include/linux/completion.h:12, > from include/linux/crypto.h:15, > from include/crypto/aead.h:13, > from include/crypto/internal/aead.h:11, > from crypto/skcipher.c:12: >crypto/skcipher.c: In function 'skcipher_get_spot': > >> include/linux/minmax.h:31:70: warning: ordered comparison of pointer with > >> integer zero [-Wextra] > 31 | (is_unsigned_type(typeof(x)) || (__is_constexpr(x) ? (x) + > 0 >= 0 : 0)) Hmmm -Wextra isn't normally set. But I do wish the compiler would do dead code elimination before these warnings. Apart from stopping code using min()/max() for pointer types (all the type checking is pointless) I think that __is_constextr() can be implemented using _Generic (instead of sizeof(type)) and then the true/false return values can be specified and need not be the same types. That test can then be: (__if_constexpr(x, x, -1) >= 0) (The '+ 0' is there to convert bool to int and won't be needed for non-constant bool.) I may drop the last few patches until MIN/MAX have been removed from everywhere else to free up the names. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
RE: [PATCH next v2 11/11] minmax: min() and max() don't need to return constant expressions
From: kernel test robot > Sent: 26 February 2024 09:42 > 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 > | Closes: > https://lore.kernel.org/oe-kbuild-all/202402261720.eamc0ehm-...@intel.com/ > > All warnings (new ones prefixed by >>): > > >> arch/x86/mm/pgtable.c:437:14: warning: variable length array used [-Wvla] > 437 | pmd_t *pmds[MAX_PREALLOCATED_PMDS]; Not surprisingly I missed X86_CONFIG_PAE builds :-) David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
RE: [PATCH next v2 02/11] minmax: Use _Static_assert() instead of static_assert()
From: Jani Nikula > Sent: 26 February 2024 09:28 > > On Sun, 25 Feb 2024, David Laight wrote: > > The wrapper just adds two more lines of error output when the test fails. > > There are only a handful of places in kernel code that use > _Static_assert() directly. Nearly 900 instances of static_assert(). How many of those supply an error message? > Are we now saying it's fine to use _Static_assert() directly all over > the place? People will copy-paste and cargo cult. Is that actually a problem? The wrapper allows the error message to be omitted and substitutes the text of the conditional. But it isn't 'free'. As well as slightly slowing down the compilation, the error messages from the compiler get more difficult to interpret. Most of the static_assert() will probably never generate an error. But the ones in min()/max() will so it is best to make them as readable as possible. (Don't even look as the mess clang makes) David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
RE: [PATCH next v2 08/11] minmax: Add min_const() and max_const()
... > Yes, yes, that may end up requiring getting rid of some current users of > > #define MIN(a,b) ((a)<(b) ? (a):(b)) > > but dammit, we don't actually have _that_ many of them, and why should > we have random drivers doing that anyway? They look like they could be changed to min(). It is even likely the header gets pulled in somewhere. I'm not sure about the ones in drivers/gpu/drm/amd/display/*..*/*.c, but it wouldn't surprise me if that code doesn't use any standard kernel headers. Isn't that also the code that manages to pass 42 integer parameters to functions? David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
RE: [PATCH next v2 08/11] minmax: Add min_const() and max_const()
From: Linus Torvalds > Sent: 25 February 2024 17:14 > > On Sun, 25 Feb 2024 at 08:53, David Laight wrote: > > > > The expansions of min() and max() contain statement expressions so are > > not valid for static intialisers. > > min_const() and max_const() are expressions so can be used for static > > initialisers. > > I hate the name. Picking name is always hard... > Naming shouldn't be about an implementation detail, particularly not > an esoteric one like the "C constant expression" rule. That can be > useful for some internal helper functions or macros, but not for > something that random people are supposed to USE. > > Telling some random developer that inside an array size declaration or > a static initializer you need to use "max_const()" because it needs to > syntactically be a constant expression, and our regular "max()" > function isn't that, is just *horrid*. > > No, please just use the traditional C model of just using ALL CAPS for > macro names that don't act like a function. > > Yes, yes, that may end up requiring getting rid of some current users of > > #define MIN(a,b) ((a)<(b) ? (a):(b)) > > but dammit, we don't actually have _that_ many of them, and why should > we have random drivers doing that anyway? I'll have a look at what is there. It might take a three part patch though. Unless you apply it as a tree-wide patch? One option is to add as max_const(), then change any existing MAX() to be max() or max_const() and then finally rename to MAX()? David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
[PATCH next v2 11/11] minmax: min() and max() don't need to return constant expressions
After changing the handful of places max() was used to size an on-stack
array to use max_const() it is no longer necessary for min() and max()
to return constant expressions from constant inputs.
Remove the associated logic to reduce the expanded text.
Remove the 'hack' that allowed max(bool, bool).
Fixup the initial block comment to match current reality.
Signed-off-by: David Laight
---
include/linux/minmax.h | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
Changes for v2:
- Typographical and spelling corrections to the commit messages.
Patches unchanged.
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index c08916588425..5e65c98ff256 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -8,13 +8,10 @@
#include
/*
- * min()/max()/clamp() macros must accomplish three things:
+ * min()/max()/clamp() macros must accomplish several things:
*
* - Avoid multiple evaluations of the arguments (so side-effects like
* "x++" happen only once) when non-constant.
- * - Retain result as a constant expressions when called with only
- * constant expressions (to avoid tripping VLA warnings in stack
- * allocation usage).
* - Perform signed v unsigned type-checking (to generate compile
* errors instead of nasty runtime surprises).
* - Unsigned char/short are always promoted to signed int and can be
@@ -22,13 +19,19 @@
* - Unsigned arguments can be compared against non-negative signed constants.
* - Comparison of a signed argument against an unsigned constant fails
* even if the constant is below __INT_MAX__ and could be cast to int.
+ *
+ * The return value of min()/max() is not a constant expression for
+ * constant parameters - so will trigger a VLA warging if used to size
+ * an on-stack array.
+ * Instead use min_const() or max_const() which do generate constant
+ * expressions and are also valid for static initialisers.
*/
#define __typecheck(x, y) \
(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
/* Allow unsigned compares against non-negative signed constants. */
#define __is_ok_unsigned(x) \
- (is_unsigned_type(typeof(x)) || (__is_constexpr(x) ? (x) + 0 >= 0 : 0))
+ (is_unsigned_type(typeof(x)) || (__is_constexpr(x) ? (x) >= 0 : 0))
/* Check for signed after promoting unsigned char/short to int */
#define __is_ok_signed(x) is_signed_type(typeof((x) + 0))
@@ -53,12 +56,10 @@
typeof(y) __y_##uniq = (y); \
__cmp(op, __x_##uniq, __y_##uniq); })
-#define __careful_cmp(op, x, y, uniq) \
- __builtin_choose_expr(__is_constexpr((x) - (y)),\
- __cmp(op, x, y),\
- ({ _Static_assert(__types_ok(x, y), \
- #op "(" #x ", " #y ") signedness error, fix types or
consider u" #op "() before " #op "_t()"); \
- __cmp_once(op, x, y, uniq); }))
+#define __careful_cmp(op, x, y, uniq) ({ \
+ _Static_assert(__types_ok(x, y),\
+ #op "(" #x ", " #y ") signedness error, fix types or consider
u" #op "() before " #op "_t()"); \
+ __cmp_once(op, x, y, uniq); })
#define __careful_cmp_const(op, x, y) \
(BUILD_BUG_ON_ZERO(!__is_constexpr((x) - (y))) +\
--
2.17.1
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
[PATCH next v2 10/11] block: Use a boolean expression instead of max() on booleans
blk_stack_limits() contains: t->zoned = max(t->zoned, b->zoned); These are bool, so can be replaced by bitwise or. However it generates: error: comparison of constant '0' with boolean expression is always true [-Werror=bool-compare] inside the signedness check that max() does unless a '+ 0' is added. It is a shame the compiler generates this warning for code that will be optimised away. Change so that the extra '+ 0' can be removed. Signed-off-by: David Laight --- block/blk-settings.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Changes for v2: - Typographical and spelling corrections to the commit messages. Patches unchanged. diff --git a/block/blk-settings.c b/block/blk-settings.c index 06ea91e51b8b..9ca21fea039d 100644 --- a/block/blk-settings.c +++ b/block/blk-settings.c @@ -688,7 +688,7 @@ int blk_stack_limits(struct queue_limits *t, struct queue_limits *b, b->max_secure_erase_sectors); t->zone_write_granularity = max(t->zone_write_granularity, b->zone_write_granularity); - t->zoned = max(t->zoned, b->zoned); + t->zoned = t->zoned | b->zoned; return ret; } EXPORT_SYMBOL(blk_stack_limits); -- 2.17.1 - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
[PATCH next v2 09/11] tree-wide: minmax: Replace all the uses of max() for array sizes with max_const()
These are the only uses of max() that require a constant value
from constant parameters.
There don't seem to be any similar uses of min().
Replacing the max() by max_const() lets min()/max() be simplified
speeding up compilation.
max_const() will convert enums to int (or unsigned int) so that the
casts added by max_t() are no longer needed.
Signed-off-by: David Laight
---
drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c | 2 +-
drivers/gpu/drm/drm_color_mgmt.c | 4 ++--
drivers/input/touchscreen/cyttsp4_core.c | 2 +-
drivers/net/can/usb/etas_es58x/es58x_devlink.c | 2 +-
fs/btrfs/tree-checker.c| 2 +-
lib/vsprintf.c | 4 ++--
net/ipv4/proc.c| 2 +-
net/ipv6/proc.c| 2 +-
8 files changed, 10 insertions(+), 10 deletions(-)
Changes for v2:
- Typographical and spelling corrections to the commit messages.
Patches unchanged.
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
index 00cd615bbcdc..935fb4014f7c 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
@@ -708,7 +708,7 @@ static const char *smu_get_feature_name(struct smu_context
*smu,
size_t smu_cmn_get_pp_feature_mask(struct smu_context *smu,
char *buf)
{
- int8_t sort_feature[max(SMU_FEATURE_COUNT, SMU_FEATURE_MAX)];
+ int8_t sort_feature[max_const(SMU_FEATURE_COUNT, SMU_FEATURE_MAX)];
uint64_t feature_mask;
int i, feature_index;
uint32_t count = 0;
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index d021497841b8..43a6bd0ca960 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -532,8 +532,8 @@ int drm_plane_create_color_properties(struct drm_plane
*plane,
{
struct drm_device *dev = plane->dev;
struct drm_property *prop;
- struct drm_prop_enum_list enum_list[max_t(int, DRM_COLOR_ENCODING_MAX,
- DRM_COLOR_RANGE_MAX)];
+ struct drm_prop_enum_list enum_list[max_const(DRM_COLOR_ENCODING_MAX,
+ DRM_COLOR_RANGE_MAX)];
int i, len;
if (WARN_ON(supported_encodings == 0 ||
diff --git a/drivers/input/touchscreen/cyttsp4_core.c
b/drivers/input/touchscreen/cyttsp4_core.c
index 7cb26929dc73..c6884c3c3fca 100644
--- a/drivers/input/touchscreen/cyttsp4_core.c
+++ b/drivers/input/touchscreen/cyttsp4_core.c
@@ -871,7 +871,7 @@ static void cyttsp4_get_mt_touches(struct cyttsp4_mt_data
*md, int num_cur_tch)
struct cyttsp4_touch tch;
int sig;
int i, j, t = 0;
- int ids[max(CY_TMA1036_MAX_TCH, CY_TMA4XX_MAX_TCH)];
+ int ids[max_const(CY_TMA1036_MAX_TCH, CY_TMA4XX_MAX_TCH)];
memset(ids, 0, si->si_ofs.tch_abs[CY_TCH_T].max * sizeof(int));
for (i = 0; i < num_cur_tch; i++) {
diff --git a/drivers/net/can/usb/etas_es58x/es58x_devlink.c
b/drivers/net/can/usb/etas_es58x/es58x_devlink.c
index 635edeb8f68c..28fa87668bf8 100644
--- a/drivers/net/can/usb/etas_es58x/es58x_devlink.c
+++ b/drivers/net/can/usb/etas_es58x/es58x_devlink.c
@@ -215,7 +215,7 @@ static int es58x_devlink_info_get(struct devlink *devlink,
struct es58x_sw_version *fw_ver = _dev->firmware_version;
struct es58x_sw_version *bl_ver = _dev->bootloader_version;
struct es58x_hw_revision *hw_rev = _dev->hardware_revision;
- char buf[max(sizeof("xx.xx.xx"), sizeof("axxx/xxx"))];
+ char buf[max_const(sizeof("xx.xx.xx"), sizeof("axxx/xxx"))];
int ret = 0;
if (es58x_sw_version_is_valid(fw_ver)) {
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 6eccf8496486..aec4729a9a82 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -615,7 +615,7 @@ static int check_dir_item(struct extent_buffer *leaf,
*/
if (key->type == BTRFS_DIR_ITEM_KEY ||
key->type == BTRFS_XATTR_ITEM_KEY) {
- char namebuf[max(BTRFS_NAME_LEN, XATTR_NAME_MAX)];
+ char namebuf[max_const(BTRFS_NAME_LEN, XATTR_NAME_MAX)];
read_extent_buffer(leaf, namebuf,
(unsigned long)(di + 1), name_len);
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 552738f14275..6c3c319afd86 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1080,8 +1080,8 @@ char *resource_string(char *buf, char *end, struct
resource *res,
#define FLAG_BUF_SIZE (2 * sizeof(res->flags))
#define DECODED_BUF_SIZE sizeof("[mem - 64bit pref window disabled]")
#define RAW_BUF_SIZE sizeof("[mem - flags 0x]")
- char sym[max(2*RSRC_BUF_SIZE + DECODED_BUF_SIZE,
[PATCH next v2 08/11] minmax: Add min_const() and max_const()
The expansions of min() and max() contain statement expressions so are
not valid for static intialisers.
min_const() and max_const() are expressions so can be used for static
initialisers.
The arguments are checked for being constant and for negative signed
values being converted to large unsigned values.
Using these to size on-stack arrays lets min/max be simplified.
Zero is added before the compare to convert enum values to integers
avoinding the need for casts when enums have been used for constants.
Signed-off-by: David Laight
---
include/linux/minmax.h | 15 +++
1 file changed, 15 insertions(+)
Changes for v2:
- Typographical and spelling corrections to the commit messages.
Patches unchanged.
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 278a390b8a4c..c08916588425 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -60,19 +60,34 @@
#op "(" #x ", " #y ") signedness error, fix types or
consider u" #op "() before " #op "_t()"); \
__cmp_once(op, x, y, uniq); }))
+#define __careful_cmp_const(op, x, y) \
+ (BUILD_BUG_ON_ZERO(!__is_constexpr((x) - (y))) +\
+ BUILD_BUG_ON_ZERO(!__types_ok(x, y)) + \
+ __cmp(op, (x) + 0, (y) + 0))
+
/**
* min - return minimum of two values of the same or compatible types
* @x: first value
* @y: second value
+ *
+ * If @x and @y are constants the return value is constant, but not 'constant
+ * enough' for things like static initialisers.
+ * min_const(@x, @y) is a constant expression for constant inputs.
*/
#define min(x, y) __careful_cmp(min, x, y, __COUNTER__)
+#define min_const(x, y)__careful_cmp_const(min, x, y)
/**
* max - return maximum of two values of the same or compatible types
* @x: first value
* @y: second value
+ *
+ * If @x and @y are constants the return value is constant, but not 'constant
+ * enough' for things like static initialisers.
+ * max_const(@x, @y) is a constant expression for constant inputs.
*/
#define max(x, y) __careful_cmp(max, x, y, __COUNTER__)
+#define max_const(x, y)__careful_cmp_const(max, x, y)
/**
* umin - return minimum of two non-negative values
--
2.17.1
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
[PATCH next v2 07/11] minmax: minmax: Add __types_ok3() and optimise defines with 3 arguments
min3() and max3() were added to optimise nested min(x, min(y, z))
sequences, but only moved where the expansion was requiested.
Add a separate implementation for 3 argument calls.
These are never required to generate constant expressions to
remove that logic.
Signed-off-by: David Laight
---
include/linux/minmax.h | 23 +++
1 file changed, 19 insertions(+), 4 deletions(-)
Changes for v2:
- Typographical and spelling corrections to the commit messages.
Patches unchanged.
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 5c7fce76abe5..278a390b8a4c 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -38,6 +38,11 @@
((__is_ok_signed(x) && __is_ok_signed(y)) ||\
(__is_ok_unsigned(x) && __is_ok_unsigned(y)))
+/* Check three values for min3(), max3() and clamp() */
+#define __types_ok3(x, y, z)
\
+ ((__is_ok_signed(x) && __is_ok_signed(y) && __is_ok_signed(z)) ||
\
+(__is_ok_unsigned(x) && __is_ok_unsigned(y) && __is_ok_unsigned(z)))
+
#define __cmp_op_min <
#define __cmp_op_max >
@@ -87,13 +92,24 @@
#define umax(x, y) \
__careful_cmp(max, __zero_extend(x), _zero_extend(y), __COUNTER__)
+#define __cmp_once3(op, x, y, z, uniq) ({ \
+ typeof(x) __x_##uniq = (x); \
+ typeof(x) __y_##uniq = (y); \
+ typeof(x) __z_##uniq = (z); \
+ __cmp(op, __cmp(op, __x_##uniq, __y_##uniq), __z_##uniq); })
+
+#define __careful_cmp3(op, x, y, z, uniq) ({ \
+ static_assert(__types_ok3(x, y, z), \
+ #op "3(" #x ", " #y ", " #z ") signedness error"); \
+ __cmp_once3(op, x, y, z, uniq); })
+
/**
* min3 - return minimum of three values
* @x: first value
* @y: second value
* @z: third value
*/
-#define min3(x, y, z) min((typeof(x))min(x, y), z)
+#define min3(x, y, z) __careful_cmp3(min, x, y, z, __COUNTER__)
/**
* max3 - return maximum of three values
@@ -101,7 +117,7 @@
* @y: second value
* @z: third value
*/
-#define max3(x, y, z) max((typeof(x))max(x, y), z)
+#define max3(x, y, z) __careful_cmp3(max, x, y, z, __COUNTER__)
/**
* min_t - return minimum of two values, using the specified type
@@ -142,8 +158,7 @@
__clamp(__val_##uniq, __lo_##uniq, __hi_##uniq); })
#define __careful_clamp(val, lo, hi, uniq) ({
\
- _Static_assert(__types_ok(val, lo), "clamp() 'lo' signedness error");
\
- _Static_assert(__types_ok(val, hi), "clamp() 'hi' signedness error");
\
+ _Static_assert(__types_ok3(val, lo, hi), "clamp() signedness error");
\
__clamp_once(val, lo, hi, uniq); })
/**
--
2.17.1
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
[PATCH next v2 06/11] minmax: Remove 'constexpr' check from __careful_clamp()
Nothing requires that clamp() return a constant expression.
The logic to do so significantly increases the .i file.
Remove the check and directly expand __clamp_once() from clamp_t()
since the type check can't fail.
Signed-off-by: David Laight
---
include/linux/minmax.h | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
Changes for v2:
- Typographical and spelling corrections to the commit messages.
Patches unchanged.
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 111c52a14fe5..5c7fce76abe5 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -141,12 +141,10 @@
"clamp() low limit " #lo " greater than high limit " #hi);
\
__clamp(__val_##uniq, __lo_##uniq, __hi_##uniq); })
-#define __careful_clamp(val, lo, hi, uniq) \
- __builtin_choose_expr(__is_constexpr((val) - (lo) + (hi)), \
- __clamp(val, lo, hi), \
- ({ _Static_assert(__types_ok(val, lo), "clamp() 'lo' signedness
error");\
- _Static_assert(__types_ok(val, hi), "clamp() 'hi' signedness
error"); \
- __clamp_once(val, lo, hi, uniq); }))
+#define __careful_clamp(val, lo, hi, uniq) ({
\
+ _Static_assert(__types_ok(val, lo), "clamp() 'lo' signedness error");
\
+ _Static_assert(__types_ok(val, hi), "clamp() 'hi' signedness error");
\
+ __clamp_once(val, lo, hi, uniq); })
/**
* clamp - return a value clamped to a given range with strict typechecking
@@ -168,7 +166,9 @@
* This macro does no typechecking and uses temporary variables of type
* @type to make all the comparisons.
*/
-#define clamp_t(type, val, lo, hi) clamp((type)(val), (type)(lo), (type)(hi))
+#define __clamp_t(type, val, lo, hi, uniq) \
+ __clamp_once((type)(val), (type)(lo), (type)(hi), uniq)
+#define clamp_t(type, val, lo, hi) __clamp_t(type, val, lo, hi, __COUNTER__)
/**
* clamp_val - return a value clamped to a given range using val's type
--
2.17.1
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
[PATCH next v2 05/11] minmax: Move the signedness check out of __cmp_once() and __clamp_once()
There is no need to do the signedness/type check when the arguments
are being cast to a fixed type.
So move the check out of __xxx_once() into __careful_xxx().
Signed-off-by: David Laight
---
include/linux/minmax.h | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
Changes for v2:
- Typographical and spelling corrections to the commit messages.
Patches unchanged.
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 8ee003d8abaf..111c52a14fe5 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -46,14 +46,14 @@
#define __cmp_once(op, x, y, uniq) ({ \
typeof(x) __x_##uniq = (x); \
typeof(y) __y_##uniq = (y); \
- _Static_assert(__types_ok(x, y),\
- #op "(" #x ", " #y ") signedness error, fix types or consider
u" #op "() before " #op "_t()"); \
__cmp(op, __x_##uniq, __y_##uniq); })
#define __careful_cmp(op, x, y, uniq) \
__builtin_choose_expr(__is_constexpr((x) - (y)),\
__cmp(op, x, y),\
- __cmp_once(op, x, y, uniq))
+ ({ _Static_assert(__types_ok(x, y), \
+ #op "(" #x ", " #y ") signedness error, fix types or
consider u" #op "() before " #op "_t()"); \
+ __cmp_once(op, x, y, uniq); }))
/**
* min - return minimum of two values of the same or compatible types
@@ -139,14 +139,14 @@
_Static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)),
\
(lo) <= (hi), true),
\
"clamp() low limit " #lo " greater than high limit " #hi);
\
- _Static_assert(__types_ok(val, lo), "clamp() 'lo' signedness error");
\
- _Static_assert(__types_ok(val, hi), "clamp() 'hi' signedness error");
\
__clamp(__val_##uniq, __lo_##uniq, __hi_##uniq); })
-#define __careful_clamp(val, lo, hi, uniq) ({ \
+#define __careful_clamp(val, lo, hi, uniq) \
__builtin_choose_expr(__is_constexpr((val) - (lo) + (hi)), \
__clamp(val, lo, hi), \
- __clamp_once(val, lo, hi, uniq)); })
+ ({ _Static_assert(__types_ok(val, lo), "clamp() 'lo' signedness
error");\
+ _Static_assert(__types_ok(val, hi), "clamp() 'hi' signedness
error"); \
+ __clamp_once(val, lo, hi, uniq); }))
/**
* clamp - return a value clamped to a given range with strict typechecking
--
2.17.1
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
[PATCH next v2 04/11] minmax: Replace multiple __UNIQUE_ID() by directly using __COUNTER__
Provided __COUNTER__ is passed through an extra #define it can be pasted
onto multiple local variables to give unique names.
This saves having 3 __UNIQUE_ID() for #defines with three locals and
look less messy in general.
Stop the umin()/umax() lines being overlong by factoring out the
zero-extension logic.
Signed-off-by: David Laight
---
include/linux/minmax.h | 48 +-
1 file changed, 24 insertions(+), 24 deletions(-)
Changes for v2:
- Typographical and spelling corrections to the commit messages.
Patches unchanged.
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index c32b4b40ce01..8ee003d8abaf 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -8,7 +8,7 @@
#include
/*
- * min()/max()/clamp() macros must accomplish several things:
+ * min()/max()/clamp() macros must accomplish three things:
*
* - Avoid multiple evaluations of the arguments (so side-effects like
* "x++" happen only once) when non-constant.
@@ -43,31 +43,31 @@
#define __cmp(op, x, y)((x) __cmp_op_##op (y) ? (x) : (y))
-#define __cmp_once(op, x, y, unique_x, unique_y) ({\
- typeof(x) unique_x = (x); \
- typeof(y) unique_y = (y); \
- _Static_assert(__types_ok(x, y),\
+#define __cmp_once(op, x, y, uniq) ({ \
+ typeof(x) __x_##uniq = (x); \
+ typeof(y) __y_##uniq = (y); \
+ _Static_assert(__types_ok(x, y),\
#op "(" #x ", " #y ") signedness error, fix types or consider
u" #op "() before " #op "_t()"); \
- __cmp(op, unique_x, unique_y); })
+ __cmp(op, __x_##uniq, __y_##uniq); })
-#define __careful_cmp(op, x, y)\
+#define __careful_cmp(op, x, y, uniq) \
__builtin_choose_expr(__is_constexpr((x) - (y)),\
__cmp(op, x, y),\
- __cmp_once(op, x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y)))
+ __cmp_once(op, x, y, uniq))
/**
* min - return minimum of two values of the same or compatible types
* @x: first value
* @y: second value
*/
-#define min(x, y) __careful_cmp(min, x, y)
+#define min(x, y) __careful_cmp(min, x, y, __COUNTER__)
/**
* max - return maximum of two values of the same or compatible types
* @x: first value
* @y: second value
*/
-#define max(x, y) __careful_cmp(max, x, y)
+#define max(x, y) __careful_cmp(max, x, y, __COUNTER__)
/**
* umin - return minimum of two non-negative values
@@ -75,8 +75,9 @@
* @x: first value
* @y: second value
*/
+#define __zero_extend(x) ((x) + 0u + 0ul + 0ull)
#define umin(x, y) \
- __careful_cmp(min, (x) + 0u + 0ul + 0ull, (y) + 0u + 0ul + 0ull)
+ __careful_cmp(min, __zero_extend(x), _zero_extend(y), __COUNTER__)
/**
* umax - return maximum of two non-negative values
@@ -84,7 +85,7 @@
* @y: second value
*/
#define umax(x, y) \
- __careful_cmp(max, (x) + 0u + 0ul + 0ull, (y) + 0u + 0ul + 0ull)
+ __careful_cmp(max, __zero_extend(x), _zero_extend(y), __COUNTER__)
/**
* min3 - return minimum of three values
@@ -108,7 +109,7 @@
* @x: first value
* @y: second value
*/
-#define min_t(type, x, y) __careful_cmp(min, (type)(x), (type)(y))
+#define min_t(type, x, y) __careful_cmp(min, (type)(x), (type)(y),
__COUNTER__)
/**
* max_t - return maximum of two values, using the specified type
@@ -116,7 +117,7 @@
* @x: first value
* @y: second value
*/
-#define max_t(type, x, y) __careful_cmp(max, (type)(x), (type)(y))
+#define max_t(type, x, y) __careful_cmp(max, (type)(x), (type)(y),
__COUNTER__)
/**
* min_not_zero - return the minimum that is _not_ zero, unless both are zero
@@ -131,22 +132,21 @@
#define __clamp(val, lo, hi) \
((val) >= (hi) ? (hi) : ((val) <= (lo) ? (lo) : (val)))
-#define __clamp_once(val, lo, hi, unique_val, unique_lo, unique_hi) ({
\
- typeof(val) unique_val = (val);
\
- typeof(lo) unique_lo = (lo);
\
- typeof(hi) unique_hi = (hi);
\
+#define __clamp_once(val, lo, hi, uniq) ({
\
+ typeof(val) __val_##uniq = (val);
\
+ typeof(lo) __lo_##uniq = (lo);
\
+ typeof(hi) __hi_##uniq = (hi);
\
_Static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)),
\
(lo) <= (hi), true),
\
"clamp() low limit " #lo " greater than high lim
[PATCH next v2 03/11] minmax: Simplify signedness check
It is enough to check that both 'x' and 'y' are valid for either a signed compare or an unsigned compare. For unsigned they must be an unsigned type or a positive constant. For signed they must be signed after unsigned char/short are promoted. The predicate for _Static_assert() only needs to be a compile-time constant not a constant integeger expression. In particular the short-circuit evaluation of || && ?: can be used to avoid the non-constantness of (pointer_type)1 in is_signed_type(). The '+ 0' in '(x) + 0 > = 0' is there to convert 'bool' to 'int' and avoid a compiler warning because max() gets used for 'bool' in one place (a very expensive 'or'). (The code is optimised away by two earlier checks - but the compiler still bleats.) Signed-off-by: David Laight --- include/linux/minmax.h | 22 ++ 1 file changed, 10 insertions(+), 12 deletions(-) Changes for v2: - Typographical and spelling corrections to the commit messages. Patches unchanged. diff --git a/include/linux/minmax.h b/include/linux/minmax.h index 900eec7a28e5..c32b4b40ce01 100644 --- a/include/linux/minmax.h +++ b/include/linux/minmax.h @@ -8,7 +8,7 @@ #include /* - * min()/max()/clamp() macros must accomplish three things: + * min()/max()/clamp() macros must accomplish several things: * * - Avoid multiple evaluations of the arguments (so side-effects like * "x++" happen only once) when non-constant. @@ -26,19 +26,17 @@ #define __typecheck(x, y) \ (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) -/* is_signed_type() isn't a constexpr for pointer types */ -#define __is_signed(x) \ - __builtin_choose_expr(__is_constexpr(is_signed_type(typeof(x))), \ - is_signed_type(typeof(x)), 0) +/* Allow unsigned compares against non-negative signed constants. */ +#define __is_ok_unsigned(x) \ + (is_unsigned_type(typeof(x)) || (__is_constexpr(x) ? (x) + 0 >= 0 : 0)) -/* True for a non-negative signed int constant */ -#define __is_noneg_int(x) \ - (__builtin_choose_expr(__is_constexpr(x) && __is_signed(x), x, -1) >= 0) +/* Check for signed after promoting unsigned char/short to int */ +#define __is_ok_signed(x) is_signed_type(typeof((x) + 0)) -#define __types_ok(x, y) \ - (__is_signed(x) == __is_signed(y) ||\ - __is_signed((x) + 0) == __is_signed((y) + 0) || \ - __is_noneg_int(x) || __is_noneg_int(y)) +/* Allow if both x and y are valid for either signed or unsigned compares. */ +#define __types_ok(x, y) \ + ((__is_ok_signed(x) && __is_ok_signed(y)) ||\ +(__is_ok_unsigned(x) && __is_ok_unsigned(y))) #define __cmp_op_min < #define __cmp_op_max > -- 2.17.1 - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
[PATCH next v2 02/11] minmax: Use _Static_assert() instead of static_assert()
The wrapper just adds two more lines of error output when the test fails.
Signed-off-by: David Laight
---
include/linux/minmax.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
Changes for v2:
- Typographical and spelling corrections to the commit messages.
Patches unchanged.
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 63c45865b48a..900eec7a28e5 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -48,7 +48,7 @@
#define __cmp_once(op, x, y, unique_x, unique_y) ({\
typeof(x) unique_x = (x); \
typeof(y) unique_y = (y); \
- static_assert(__types_ok(x, y), \
+ _Static_assert(__types_ok(x, y),\
#op "(" #x ", " #y ") signedness error, fix types or consider
u" #op "() before " #op "_t()"); \
__cmp(op, unique_x, unique_y); })
@@ -137,11 +137,11 @@
typeof(val) unique_val = (val);
\
typeof(lo) unique_lo = (lo);
\
typeof(hi) unique_hi = (hi);
\
- static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)),
\
+ _Static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)),
\
(lo) <= (hi), true),
\
"clamp() low limit " #lo " greater than high limit " #hi);
\
- static_assert(__types_ok(val, lo), "clamp() 'lo' signedness error");
\
- static_assert(__types_ok(val, hi), "clamp() 'hi' signedness error");
\
+ _Static_assert(__types_ok(val, lo), "clamp() 'lo' signedness error");
\
+ _Static_assert(__types_ok(val, hi), "clamp() 'hi' signedness error");
\
__clamp(unique_val, unique_lo, unique_hi); })
#define __careful_clamp(val, lo, hi) ({
\
--
2.17.1
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
[PATCH next v2 01/11] minmax: Put all the clamp() definitions together
The defines for clamp() have got separated, move togther for readability.
Update description of signedness check.
Signed-off-by: David Laight
---
include/linux/minmax.h | 120 +++--
1 file changed, 56 insertions(+), 64 deletions(-)
Changes for v2:
- Typographical and spelling corrections to the commit messages.
Patches unchanged.
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 2ec559284a9f..63c45865b48a 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -57,26 +57,6 @@
__cmp(op, x, y),\
__cmp_once(op, x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y)))
-#define __clamp(val, lo, hi) \
- ((val) >= (hi) ? (hi) : ((val) <= (lo) ? (lo) : (val)))
-
-#define __clamp_once(val, lo, hi, unique_val, unique_lo, unique_hi) ({
\
- typeof(val) unique_val = (val);
\
- typeof(lo) unique_lo = (lo);
\
- typeof(hi) unique_hi = (hi);
\
- static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)),
\
- (lo) <= (hi), true),
\
- "clamp() low limit " #lo " greater than high limit " #hi);
\
- static_assert(__types_ok(val, lo), "clamp() 'lo' signedness error");
\
- static_assert(__types_ok(val, hi), "clamp() 'hi' signedness error");
\
- __clamp(unique_val, unique_lo, unique_hi); })
-
-#define __careful_clamp(val, lo, hi) ({
\
- __builtin_choose_expr(__is_constexpr((val) - (lo) + (hi)), \
- __clamp(val, lo, hi), \
- __clamp_once(val, lo, hi, __UNIQUE_ID(__val), \
-__UNIQUE_ID(__lo), __UNIQUE_ID(__hi))); })
-
/**
* min - return minimum of two values of the same or compatible types
* @x: first value
@@ -124,6 +104,22 @@
*/
#define max3(x, y, z) max((typeof(x))max(x, y), z)
+/**
+ * min_t - return minimum of two values, using the specified type
+ * @type: data type to use
+ * @x: first value
+ * @y: second value
+ */
+#define min_t(type, x, y) __careful_cmp(min, (type)(x), (type)(y))
+
+/**
+ * max_t - return maximum of two values, using the specified type
+ * @type: data type to use
+ * @x: first value
+ * @y: second value
+ */
+#define max_t(type, x, y) __careful_cmp(max, (type)(x), (type)(y))
+
/**
* min_not_zero - return the minimum that is _not_ zero, unless both are zero
* @x: value1
@@ -134,39 +130,60 @@
typeof(y) __y = (y);\
__x == 0 ? __y : ((__y == 0) ? __x : min(__x, __y)); })
+#define __clamp(val, lo, hi) \
+ ((val) >= (hi) ? (hi) : ((val) <= (lo) ? (lo) : (val)))
+
+#define __clamp_once(val, lo, hi, unique_val, unique_lo, unique_hi) ({
\
+ typeof(val) unique_val = (val);
\
+ typeof(lo) unique_lo = (lo);
\
+ typeof(hi) unique_hi = (hi);
\
+ static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)),
\
+ (lo) <= (hi), true),
\
+ "clamp() low limit " #lo " greater than high limit " #hi);
\
+ static_assert(__types_ok(val, lo), "clamp() 'lo' signedness error");
\
+ static_assert(__types_ok(val, hi), "clamp() 'hi' signedness error");
\
+ __clamp(unique_val, unique_lo, unique_hi); })
+
+#define __careful_clamp(val, lo, hi) ({
\
+ __builtin_choose_expr(__is_constexpr((val) - (lo) + (hi)), \
+ __clamp(val, lo, hi), \
+ __clamp_once(val, lo, hi, __UNIQUE_ID(__val), \
+__UNIQUE_ID(__lo), __UNIQUE_ID(__hi))); })
+
/**
* clamp - return a value clamped to a given range with strict typechecking
* @val: current value
* @lo: lowest allowable value
* @hi: highest allowable value
*
- * This macro does strict typechecking of @lo/@hi to make sure they are of the
- * same type as @val. See the unnecessary pointer comparisons.
+ * This macro checks that @val, @lo and @hi have the same signedness.
*/
#define clamp(val, lo, hi) __careful_clamp(val, lo, hi)
-/*
- * ..and if you can't take the strict
- * types, you can specify one yourself.
- *
- * Or not use min/max/clamp at all, of course.
- */
-
/**
- * min_t - return minimum of two values, using the specified type
- * @type: data type to use
- * @x: first value
- * @y: second value
+ * clamp_t - return a value clamped to
[PATCH next v2 00/11] minmax: Optimise to reduce .i line length
The changes to minmax.h that changed the type check to a signedness check significantly increased the length of the expansion. In some cases it has also significantly increased compile type. This is particularly noticable for nested expansions. The fact that _Static_assert() only requires a compile time constant not a constant expression allows a lot of simplification. The other thing that compilicates the expansion is the necessity of returning a constant expression from constant arguments (for VLA). I can only find a handful of places this is done. Penalising most of the code for these few cases seems 'suboptimal'. Instead I've added min_const() and max_const() for VLA and static initialisers, these check the arguments are constant to avoid misuse. Patch [9] is dependant on the earlier patches. Patch [10] isn't dependant on them. Patch [11] depends on both 9 and 10. Changes for v2: - Typographical and spelling corrections to the commit messages. Patches unchanged. David Laight (11): [1] minmax: Put all the clamp() definitions together [2] minmax: Use _Static_assert() instead of static_assert() [3] minmax: Simplify signedness check [4] minmax: Replace multiple __UNIQUE_ID() by directly using __COUNTER__ [5] minmax: Move the signedness check out of __cmp_once() and __clamp_once() [6] minmax: Remove 'constexpr' check from __careful_clamp() [7] minmax: minmax: Add __types_ok3() and optimise defines with 3 arguments [8] minmax: Add min_const() and max_const() [9] tree-wide: minmax: Replace all the uses of max() for array sizes with max_const(). [10] block: Use a boolean expression instead of max() on booleans [11] minmax: min() and max() don't need to return constant expressions block/blk-settings.c | 2 +- drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c| 2 +- drivers/gpu/drm/drm_color_mgmt.c | 4 +- drivers/input/touchscreen/cyttsp4_core.c | 2 +- .../net/can/usb/etas_es58x/es58x_devlink.c| 2 +- fs/btrfs/tree-checker.c | 2 +- include/linux/minmax.h| 211 ++ lib/vsprintf.c| 4 +- net/ipv4/proc.c | 2 +- net/ipv6/proc.c | 2 +- 10 files changed, 127 insertions(+), 106 deletions(-) -- 2.17.1 - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
[PATCH] drm/amd/display: Remove duplicated function signature from dcn3.01 DCCG
In the header file dc/dcn301/dcn301_dccg.h, the function dccg301_create is declared twice, so remove duplication. Signed-off-by: David Tadokoro --- drivers/gpu/drm/amd/display/dc/dcn301/dcn301_dccg.h | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_dccg.h b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_dccg.h index 73db962dbc03..067e49cb238e 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_dccg.h +++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_dccg.h @@ -56,10 +56,4 @@ struct dccg *dccg301_create( const struct dccg_shift *dccg_shift, const struct dccg_mask *dccg_mask); -struct dccg *dccg301_create( - struct dc_context *ctx, - const struct dccg_registers *regs, - const struct dccg_shift *dccg_shift, - const struct dccg_mask *dccg_mask); - #endif //__DCN301_DCCG_H__ -- 2.39.2
Re: [PATCH 2/9] lib/cmdline: Fix an invalid format specifier in an assertion msg
On Thu, 22 Feb 2024 at 04:10, 'Justin Stitt' via KUnit Development wrote: > > Hi, > > On Wed, Feb 21, 2024 at 05:27:15PM +0800, David Gow wrote: > > The correct format specifier for p - n (both p and n are pointers) is > > %td, as the type should be ptrdiff_t. > > I think %tu is better. d specifies a signed type. I don't doubt that the > warning is fixed but I think %tu represents the type semantics here. > While I agree that this should never be negative, I'd still lean on this being a signed type, for two reasons: - I think, if there's a bug in this code, it's easier to debug this if a 'negative' value were to appear as such. - While, as I understand it, the C spec does provide for a ptrdiff_t-sized unsigned printf specifier in '%tu', the difference between two pointers is always signed: "When two pointers are subtracted, both shall point to elements of the same array object, or one past the last element of the array object; the result is the difference of the subscripts of the two array elements. The size of the result is implementation-defined, and its type (a signed integer type) is ptrdiff_t defined in the header" (Technically, the kernel's ptrdiff_t type isn't defined in stddef.h, so a bit of deviation from the spec is happening anyway, though.) If there's a particularly good reason to make this unsigned in this case, I'd be happy to change it, of course. But I'd otherwise prefer to keep it as-is. Cheers, -- David smime.p7s Description: S/MIME Cryptographic Signature
[PATCH 9/9] kunit: Annotate _MSG assertion variants with gnu printf specifiers
KUnit's assertion macros have variants which accept a printf format
string, to allow tests to specify a more detailed message on failure.
These (and the related KUNIT_FAIL() macro) ultimately wrap the
__kunit_do_failed_assertion() function, which accepted a printf format
specifier, but did not have the __printf attribute, so gcc couldn't warn
on incorrect agruments.
It turns out there were quite a few tests with such incorrect arguments.
Add the __printf() specifier now that we've fixed these errors, to
prevent them from recurring.
Suggested-by: Linus Torvalds
Signed-off-by: David Gow
---
include/kunit/test.h | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h
index fcb4a4940ace..61637ef32302 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -579,12 +579,12 @@ void __printf(2, 3) kunit_log_append(struct string_stream
*log, const char *fmt,
void __noreturn __kunit_abort(struct kunit *test);
-void __kunit_do_failed_assertion(struct kunit *test,
- const struct kunit_loc *loc,
- enum kunit_assert_type type,
- const struct kunit_assert *assert,
- assert_format_t assert_format,
- const char *fmt, ...);
+void __printf(6, 7) __kunit_do_failed_assertion(struct kunit *test,
+ const struct kunit_loc *loc,
+ enum kunit_assert_type type,
+ const struct kunit_assert
*assert,
+ assert_format_t assert_format,
+ const char *fmt, ...);
#define _KUNIT_FAILED(test, assert_type, assert_class, assert_format,
INITIALIZER, fmt, ...) do { \
static const struct kunit_loc __loc = KUNIT_CURRENT_LOC; \
--
2.44.0.rc0.258.g7320e95886-goog
[PATCH 8/9] drm/xe/tests: Fix printf format specifiers in xe_migrate test
KUNIT_FAIL() is used to fail the xe_migrate test when an error occurs.
However, there's a mismatch in the format specifier: '%li' is used to
log 'err', which is an 'int'.
Use '%i' instead of '%li', and for the case where we're printing an
error pointer, just use '%pe', instead of extracting the error code
manually with PTR_ERR(). (This also results in a nicer output when the
error code is known.)
Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
Signed-off-by: David Gow
---
drivers/gpu/drm/xe/tests/xe_migrate.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/xe/tests/xe_migrate.c
b/drivers/gpu/drm/xe/tests/xe_migrate.c
index a6523df0f1d3..c347e2c29f81 100644
--- a/drivers/gpu/drm/xe/tests/xe_migrate.c
+++ b/drivers/gpu/drm/xe/tests/xe_migrate.c
@@ -114,21 +114,21 @@ static void test_copy(struct xe_migrate *m, struct xe_bo
*bo,
region |
XE_BO_NEEDS_CPU_ACCESS);
if (IS_ERR(remote)) {
- KUNIT_FAIL(test, "Failed to allocate remote bo for %s: %li\n",
- str, PTR_ERR(remote));
+ KUNIT_FAIL(test, "Failed to allocate remote bo for %s: %pe\n",
+ str, remote);
return;
}
err = xe_bo_validate(remote, NULL, false);
if (err) {
- KUNIT_FAIL(test, "Failed to validate system bo for %s: %li\n",
+ KUNIT_FAIL(test, "Failed to validate system bo for %s: %i\n",
str, err);
goto out_unlock;
}
err = xe_bo_vmap(remote);
if (err) {
- KUNIT_FAIL(test, "Failed to vmap system bo for %s: %li\n",
+ KUNIT_FAIL(test, "Failed to vmap system bo for %s: %i\n",
str, err);
goto out_unlock;
}
--
2.44.0.rc0.258.g7320e95886-goog
[PATCH 7/9] drm: tests: Fix invalid printf format specifiers in KUnit tests
The drm_buddy_test's alloc_contiguous test used a u64 for the page size,
which was then updated to be an 'unsigned long' to avoid 64-bit
multiplication division helpers.
However, the variable is logged by some KUNIT_ASSERT_EQ_MSG() using the
'%d' or '%llu' format specifiers, the former of which is always wrong,
and the latter is no longer correct now that ps is no longer a u64. Fix
these to all use '%lu'.
Also, drm_mm_test calls KUNIT_FAIL() with an empty string as the
message. gcc warns if a printf format string is empty (apparently), so
give these some more detailed error messages, which should be more
useful anyway.
Fixes: a64056bb5a32 ("drm/tests/drm_buddy: add alloc_contiguous test")
Fixes: fca7526b7d89 ("drm/tests/drm_buddy: fix build failure on 32-bit targets")
Fixes: fc8d29e298cf ("drm: selftest: convert drm_mm selftest to KUnit")
Signed-off-by: David Gow
---
drivers/gpu/drm/tests/drm_buddy_test.c | 14 +++---
drivers/gpu/drm/tests/drm_mm_test.c| 6 +++---
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c
b/drivers/gpu/drm/tests/drm_buddy_test.c
index 8a464f7f4c61..3dbfa3078449 100644
--- a/drivers/gpu/drm/tests/drm_buddy_test.c
+++ b/drivers/gpu/drm/tests/drm_buddy_test.c
@@ -55,30 +55,30 @@ static void drm_test_buddy_alloc_contiguous(struct kunit
*test)
KUNIT_ASSERT_FALSE_MSG(test,
drm_buddy_alloc_blocks(, 0, mm_size,
ps, ps, list, 0),
- "buddy_alloc hit an error size=%d\n",
+ "buddy_alloc hit an error size=%lu\n",
ps);
} while (++i < n_pages);
KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
3 * ps, ps,
,
DRM_BUDDY_CONTIGUOUS_ALLOCATION),
- "buddy_alloc didn't error size=%d\n", 3 * ps);
+ "buddy_alloc didn't error size=%lu\n", 3 * ps);
drm_buddy_free_list(, );
KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
3 * ps, ps,
,
DRM_BUDDY_CONTIGUOUS_ALLOCATION),
- "buddy_alloc didn't error size=%llu\n", 3 * ps);
+ "buddy_alloc didn't error size=%lu\n", 3 * ps);
KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
2 * ps, ps,
,
DRM_BUDDY_CONTIGUOUS_ALLOCATION),
- "buddy_alloc didn't error size=%llu\n", 2 * ps);
+ "buddy_alloc didn't error size=%lu\n", 2 * ps);
drm_buddy_free_list(, );
KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
3 * ps, ps,
,
DRM_BUDDY_CONTIGUOUS_ALLOCATION),
- "buddy_alloc didn't error size=%llu\n", 3 * ps);
+ "buddy_alloc didn't error size=%lu\n", 3 * ps);
/*
* At this point we should have enough contiguous space for 2 blocks,
* however they are never buddies (since we freed middle and right) so
@@ -87,13 +87,13 @@ static void drm_test_buddy_alloc_contiguous(struct kunit
*test)
KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
2 * ps, ps,
,
DRM_BUDDY_CONTIGUOUS_ALLOCATION),
- "buddy_alloc hit an error size=%d\n", 2 * ps);
+ "buddy_alloc hit an error size=%lu\n", 2 * ps);
drm_buddy_free_list(, );
KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
3 * ps, ps,
,
DRM_BUDDY_CONTIGUOUS_ALLOCATION),
- "buddy_alloc hit an error size=%d\n", 3 * ps);
+ "buddy_alloc hit an error size=%lu\n", 3 * ps);
total = 0;
list_for_each_entry(block, , link)
diff --git a/drivers/gpu/drm/tests/drm_mm_test.c
b/drivers/gpu/drm/tests/drm_mm_test.c
index 1eb0c304f960..f37c0d765865 100644
--- a/drivers/gpu/drm/tests/drm_mm_test.c
+++ b/drivers/gpu/drm/tests/drm_mm_te
[PATCH 6/9] net: test: Fix printf format specifier in skb_segment kunit test
KUNIT_FAIL() accepts a printf-style format string, but previously did
not let gcc validate it with the __printf() attribute. The use of %lld
for the result of PTR_ERR() is not correct.
Instead, use %pe and pass the actual error pointer. printk() will format
it correctly (and give a symbolic name rather than a number if
available, which should make the output more readable, too).
Fixes: b3098d32ed6e ("net: add skb_segment kunit test")
Signed-off-by: David Gow
---
net/core/gso_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/gso_test.c b/net/core/gso_test.c
index 4c2e77bd12f4..358c44680d91 100644
--- a/net/core/gso_test.c
+++ b/net/core/gso_test.c
@@ -225,7 +225,7 @@ static void gso_test_func(struct kunit *test)
segs = skb_segment(skb, features);
if (IS_ERR(segs)) {
- KUNIT_FAIL(test, "segs error %lld", PTR_ERR(segs));
+ KUNIT_FAIL(test, "segs error %pe", segs);
goto free_gso_skb;
} else if (!segs) {
KUNIT_FAIL(test, "no segments");
--
2.44.0.rc0.258.g7320e95886-goog
