On 2023-08-30 04:22, Hui Fang wrote:
Hi, guys
Thanks for your time.
I wonder if only NXP met the "swiotlb buffer full" issue.
In theory, when format is YUYV, those resolutions no greater than 320x240 (153600 bytes, which less than the max mapping size 256K ) can't meet the issue.
But resolutions no less than 640x480 (307200 bytes), may have chances to trigger the issue.
On our side, we tested on 1080p, easy to reproduce.
Or maybe "dma_max_mapping_size(dev)" is much bigger than 256K on your side?
I would imagine that in most cases, people either have an IOMMU, or they
don't have more RAM than the device can access directly.
And in fact I think we've missed looking deep enough and that's the real
problem here - the overall context is a buffer allocator, so if SWIOTLB
is bouncing anything in that dma_map_sg() call, then it means we're
allocating a buffer for a device out of pages that that device can't
actually access, which seems fundamentally bad. Unfortunately there
isn't currently an easy way to do the right thing - dma-debug would
probably get very unhappy about scraping a bunch of dma_alloc_pages()
allocations into a scatterlist and subsequently calling dma_sync_sg() on
it, while dma_alloc_noncontiguous() does at least return a scatterlist
but would be overly restrictive since we don't need it to be
dma-contiguous either. I guess we could do with some sort of
dma_alloc_sgt() API that joins the relevant internal bits together to
replace vb2_dma_sg_alloc_compacted() with proper DMA-mask-aware
allocation, and probably also allocates the sg_table as well?
Thanks,
Robin.
BRs,
Fang Hui
________________________________
From: Christoph Hellwig <hch@xxxxxx>
Sent: Tuesday, August 29, 2023 11:04 PM
To: Robin Murphy <robin.murphy@xxxxxxx>
Cc: Tomasz Figa <tfiga@xxxxxxxxxxxx>; Anle Pan <anle.pan@xxxxxxx>; Christoph Hellwig <hch@xxxxxx>; m.szyprowski@xxxxxxxxxxx <m.szyprowski@xxxxxxxxxxx>; mchehab@xxxxxxxxxx <mchehab@xxxxxxxxxx>; linux-media@xxxxxxxxxxxxxxx <linux-media@xxxxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>; Hui Fang <hui.fang@xxxxxxx>
Subject: [EXT] Re: [PATCH] media: videobuf2-dma-sg: limit the sg segment size
Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
On Tue, Aug 29, 2023 at 12:14:44PM +0100, Robin Murphy wrote:
dma_get_max_seg_size() represents a capability of the device itself, namely
the largest contiguous range it can be programmed to access in a single DMA
descriptor/register/whatever.
Yes. In a way it's a bit odd that it ended up in a field in
struct device, as the feature might actually be different for different
DMA engines or features in a device. If I was to redesign it from
scratch I'd just pass it to dma_map_sg.
Generally looking at videobuf2-dma-sg, I feel like we would benefit
from some kind of dma_alloc_table_from_pages() that simply takes the
struct dev pointer and does everything necessary.
Possibly; this code already looks lifted from drm_prime_pages_to_sg(), and
if it's needed here then presumably vb2_dma_sg_get_userptr() also needs it,
at the very least.
Yes, there's tons of them. But I'd feel really bad adding even more
struct scatterlist based APIs given how bad of a data structure that is.
