Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

14 participants
3704 discussions

Re: [PATCH v4 00/25] accel/qda: Qualcomm DSP Accelerator driver

by Krzysztof Kozlowski

On 01/11/2026 23:21, Ekansh Gupta wrote: > drivers/accel/qda/qda_soc.c | 66 ++ > drivers/accel/qda/qda_soc.h | 61 ++ > drivers/misc/Kconfig | 1 + > include/uapi/drm/qda_accel.h | 282 +++++++ > 33 files changed, 5877 insertions(+), 1 deletion(-) > --- > base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c > change-id: 20261102-v4-e8feb321263f What is happening here? How are we supposed to work with this? v1: 20260223-qda-firstpost-4ab05249e2cc v2: 20260519-qda-series-78c2bf0ed78b v2 again: 20260817-qda-v2-78e2d1f10529 And now completely different one? This makes the change id completely unusable. b4 diff obviously does not work. Any tracking of this does not work. NAK, really, I already complained internally months ago about such practice, so apparently not enough. Best regards, Krzysztof

5 days, 15 hours

Re: [PATCH v4 21/25] accel/qda: Add Audio protection domain attach and its remote heap

by Krzysztof Kozlowski

On 01/11/2026 23:21, Ekansh Gupta wrote: > > +/* > + * The Audio PD keeps the remote heap across daemon restarts, so only the first > + * attach on a device hands it to the DSP. The lock is held across the attach > + * so that a concurrent one cannot skip the heap while the first is still in > + * flight, and the flag is set only once the DSP has accepted it. > + */ > +static int qda_attach_audio_pd(struct qda_session *session, struct drm_qda_remote_attach *args) > +{ > + struct qda_dev *qdev = session->qda_dev; > + struct qda_static_pd_args sargs = {}; > + char *name; > + int err; > + > + if (strcmp(qdev->dsp_name, "adsp") || !qdev->remote_heap_size) That's value from DTB, so where did you document the ABI? I see such undocumented ABI in multiple places. Usually requirement would be to document it, but not label. Why label 'ADSP' is not correct? Why "Audio DSP" would not be correct? Drop all this label dependency, completely (and do not introduce other names, like node name). Best regards, Krzysztof

5 days, 15 hours

Re: [PATCH v4 16/25] accel/qda: Add DSP process creation and release to the legacy shim

by Krzysztof Kozlowski

On 01/11/2026 23:21, Ekansh Gupta wrote: > snode->sdev = sdev; > + snode->secure = secure; > snode->miscdev.minor = MISC_DYNAMIC_MINOR; > snode->miscdev.name = snode->name; > snode->miscdev.fops = &qda_shim_fops; > @@ -293,8 +346,6 @@ static int qda_shim_probe(struct auxiliary_device *auxdev, > const struct auxiliary_device_id *id) > { > struct qda_dev *qdev = dev_get_platdata(&auxdev->dev); > - bool secure_channel = !of_property_read_bool(qdev->dev->of_node, > - "qcom,non-secure-domain"); You just added these lines two patches earlier. You are adding COMPLETELY new driver, so adding completely new code and then removing it is confusing. It means you added something which was not needed. Best regards, Krzysztof

5 days, 15 hours

Re: [PATCH v4 23/25] accel/qda: Add DSP capability query

by Arnd Bergmann

On Sun, Nov 1, 2026, at 23:21, Ekansh Gupta wrote: > * > * This structure is used with DRM_IOCTL_QDA_QUERY to query device attributes > - * based on @query_type. > + * based on @query_type. @dsp_name and @cap share storage, as only one query > + * is made per call. > */ > struct drm_qda_query { > __u32 query_type; > - __u8 dsp_name[16]; > + union { > + __u8 dsp_name[16]; > + struct { > + __u32 attribute_id; > + __u32 capability; > + } cap; > + }; > }; I think it would be better to make this separate ioctl commands instead of a multiplexer for each thing you query. ioctl numbers are cheap, but complexity is not. Arnd

5 days, 15 hours

Re: [PATCH v4 04/25] accel/qda: Assign a context bank to each session

by Krzysztof Kozlowski

On 01/11/2026 23:21, Ekansh Gupta wrote: > + > +static int qda_count_cb_nodes(struct device_node *parent_node) > +{ > + int count = 0; > + > + for_each_available_child_of_node_scoped(parent_node, child) I don't see a need for scoped loop. > + if (of_device_is_compatible(child, "qcom,fastrpc-compute-cb")) > + count++; > + > + return count; > } Best regards, Krzysztof

5 days, 15 hours

Re: [PATCH v4 03/25] accel/qda: Add compute context bank devices

by Krzysztof Kozlowski

On 01/11/2026 23:21, Ekansh Gupta wrote: > diff --git a/drivers/accel/qda/qda_rpmsg.c b/drivers/accel/qda/qda_rpmsg.c > index 6bd977e8d68a..78b5f31974c1 100644 > --- a/drivers/accel/qda/qda_rpmsg.c > +++ b/drivers/accel/qda/qda_rpmsg.c > @@ -5,18 +5,18 @@ > #include <linux/rpmsg.h> > #include <drm/drm_print.h> > > +#include "qda_cb.h" > #include "qda_drv.h" > +#include "qda_soc.h" > > /* > * Both compatibles the binding defines are matched here. The glymur, hawi, > * maili and nord nodes name "qcom,kaanapali-fastrpc" as their fallback > - * compatible and so are matched by that entry. Per-SoC DSP parameters are > - * attached as match data by a later patch, once there is something that > - * consumes them. You just added this in previous commit. Why adding invalid documentation, which is immediately removed? > + * compatible and so are matched by that entry. Best regards, Krzysztof

5 days, 15 hours

Re: [PATCH v4 21/25] accel/qda: Add Audio protection domain attach and its remote heap

by Arnd Bergmann

On Sun, Nov 1, 2026, at 23:21, Ekansh Gupta wrote: > struct drm_qda_remote_attach { > __u32 pd_type; > - __u32 pad; > + __u32 namelen; > + __u64 name; > }; > This is an ABI change in the middle of the series, which isn't great even though it is harmless as there are no users that would see the initial ABI. I'd suggest defining the structure in the final length from the start. Arnd

5 days, 15 hours

Re: [PATCH v4 02/25] accel/qda: Add initial QDA DRM accelerator driver

by Krzysztof Kozlowski

On 01/11/2026 23:21, Ekansh Gupta wrote: > + > +#include "qda_drv.h" > + > +/* > + * Both compatibles the binding defines are matched here. The glymur, hawi, > + * maili and nord nodes name "qcom,kaanapali-fastrpc" as their fallback > + * compatible and so are matched by that entry. Per-SoC DSP parameters are > + * attached as match data by a later patch, once there is something that So you add empty or non-working code. No, add these once you actually have complete and working table. > + * consumes them. > + */ Drop comment, redundant. Referencing future patches makes no sense in source code, because there is no "patch" when looking at the code. > +static const struct of_device_id qda_rpmsg_of_match[] = { > + { .compatible = "qcom,kaanapali-fastrpc" }, > + { .compatible = "qcom,fastrpc" }, > + { } Best regards, Krzysztof

5 days, 15 hours

Re: [PATCH 2/4] media: videobuf2: add opt-in DMA-BUF CPU access

by Nicolas Dufresne

Le jeudi 22 octobre 2026 à 06:04 +0200, Karl Mehltretter a écrit : > On Wed, Oct 21, 2026 at 05:03:19PM +0100, Nicolas Dufresne wrote: > > > I think the dma-buf API doesn't give us what we need. Cleaning the CPU > > > cache through end_cpu_access() when the producer is done writting to the > > > buffer is right in some use cases, but not in all of them. It assumes > > > that the next consumer will access the data through DMA, which isn't > > > always the case. What I think we need is to track the state of the > > > cache(s) in the dma_buf, and perform cache management operations > > > accordingly when needed. In the uvcvideo -> display use case for > > > instance, the dma_buf should track that the CPU cache is dirty when > > > finishing CPU access, and should be cleaned on the display side before > > > performing the DMA. In a use case where the producer would DMA to the > > > buffer, cache management would be skipped as the dma_buf would see that > > > the cache is not dirty. > > > > I also second this as the only viable option. Its not only about uvcvideo, but > > any memory imported by a driver doing DMA (which is not also the exporter). That > > includes udmabuf usage. These days, cache artifacts are visible with Intel > > display controllers, and Intel mobile GPU drivers. Note that GStreamer properly > > signal CPU read/write using DMABuf sync ioctl. > > > > For most Intel laptop user with GStreamer 1.28, these example produce artifacts > > on movement. To me, the solution is to track the cache state according to the > > access signalling (either userspace or driver), and let DMA user/consumer decide > > if a cache flush is needed. There is a obvious challenge in keeping this state > > coherent whenever there is multiple consumer, of if some broken application do > > concurrent write. Result for the last will always be undefined, but the dmabuf > > CPU cache state should not be undefined. > > > > For a persistently mapped attachment, where would the transition to > device access happen? > > dma_buf_map_attachment() may only run once, while the importer can > submit the buffer many times. Would this need a new per-attachment > operation before each submission, or can an existing event be used? In my perception, the mechanism in place, attach/detach and begin/end only work for very specific cases and breaks in most other case I can think of. > > I tested that distinction with one persistent V3D attachment on my Pi > 400 and Pi 500+: > > - If end_cpu_access() only recorded dirty state, with no sync before > submission, V3D read stale data in all 64 frames on both boards. > > - Synchronizing the attachment immediately before submission produced > 64/64 matching frames on both boards. > > This only covers V3D, but it seems that a dirty bit still needs a > device-side use event. Is that the model you had in mind? In my mind, we need to introduce new API. The problem with the being/end design is that it only works for specific pattern. Typically, on begin_cpu_access, you want to do specific action that ensure the CPU will see the right value in memory. One of these action could be to flush a device write cache for CPU read access. But unless only the exporter see these calls, and unless the device is the exporter, eveyrthing we have fall appart. With the new trend of always allocating from unified memory using dma heap, udmabuf and other non-device specific allocator, both device cache just breaks everything. Just like, an importer that need CPU write to be flushed breaks today. This last one seems obvious, just flush, but if the data came from another device, you might just kill the performance. I think if its just for the CPU cache state, to let importer that needs it know, we could introduce some sort of a bit mask (and some locking probably?) to help solve that. But we still leave the concept of device side memory cache broken. Nicolas > > Thanks, > Karl

1 week, 1 day

Re: [RFC PATCH 1/3] dma-buf: keep the DMA flags in the DMABUF_DEBUG copy

by Christian König

On 10/5/26 08:41, Karl Mehltretter wrote: > With DMABUF_DEBUG, dma_buf_map_attachment() hands the importer a copy > of the sg_table that keeps only sg_dma_address() and sg_dma_len(). The > dma_flags (SG_DMA_BUS_ADDRESS, SG_DMA_SWIOTLB) are dropped. > > The flags describe the DMA side of an entry, which is the side > importers may use. Copy them as well. > > This is not a bug fix. No importer reads the flags today. Their only > readers are dma-iommu, dma-direct and iommu_map_sg(), which are not > supposed to see an importer's copy at all. The warn mode added in the > next patch keeps its marker in dma_flags and lets importers that do > get there continue. They should then see the same flags as without > DMABUF_DEBUG. > > Assisted-by: LLM > Signed-off-by: Karl Mehltretter <kmehltretter(a)gmail.com> I've reviewed the patch and pushed it to drm-misc-fixes. Sorry for the delay, Christian. > --- > > Notes: > Built alone on x86_64 with NEED_SG_DMA_FLAGS=y (dma-buf.o, W=1). The > KUnit test in patch 3 checks the flags in the copy. It passes on x86_64 > under QEMU in strict and in warn mode. > > drivers/dma-buf/dma-buf.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index 4c9add51f9ef..b3d311acb883 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -904,6 +904,10 @@ static int dma_buf_wrap_sg_table(struct sg_table **sg_table) > sg_assign_page(to_sg, NULL); > sg_dma_address(to_sg) = sg_dma_address(from_sg); > sg_dma_len(to_sg) = sg_dma_len(from_sg); > +#ifdef CONFIG_NEED_SG_DMA_FLAGS > + /* the flags describe the DMA side, e.g. SG_DMA_BUS_ADDRESS */ > + to_sg->dma_flags = from_sg->dma_flags; > +#endif > to_sg = sg_next(to_sg); > } >

1 week, 3 days

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig