Re: [EXT] Re: [PATCH] media: videobuf2-dma-sg: limit the sg segment size

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux