On 06/01/2015 12:44 PM, Sakari Ailus wrote: > Hi Hans, > > Thanks for the RFC. > > On Mon, Jun 01, 2015 at 11:44:51AM +0200, Hans Verkuil wrote: >> One of the things that is really irritating is the fact that drivers that >> use contig-dma sometimes want to support USERPTR, allowing applications to >> pass pointers to the driver that point to physically contiguous memory that >> was somehow obtained, and that userspace has no way of knowing whether the >> driver has this requirement or not. >> >> A related issue is that, depending on the DMA engine, the user pointer might >> have some alignment requirements (page aligned, or at minimum 16 bytes aligned) >> that userspace has no way of knowing. >> >> The same alignment issue is present also for dma-buf. >> >> I propose to take one reserved field from struct v4l2_create_buffers and >> from struct v4l2_requestbuffers and change it to this: >> >> __u32 flags; >> >> #define V4L2_REQBUFS_FL_ALIGNMENT_MSK 0x3f > > How about V4L2_REQBUFS_FL_ALIGN_MASK instead? It's shorter, and that msk > part looks odd to me. Hmm, how to do this. Currently it masks out 6 bits which form the power-of-two that determines the alignment. How about this: #define V4L2_REQBUFS_FL_ALIGN_EXP(flags) ((flags) & 0x3f) #define V4L2_REQBUFS_FL_ALIGN_MASK(flags) ((1ULL << (flags & 0x3f)) - 1) That gives you both mask and the exponent. Better names are welcome :-) ALIGN_PWR? PWR2? ALIGN_AT_BIT? > >> #define V4L2_REQBUFS_FL_PHYS_CONTIG (1 << 31) >> >> Where the alignment is a power of 2 (and if 0 the alignment is unknown). The max >> is 2^63, which should be enough for the foreseeable future :-) >> >> If the physically contiguous flag is set, then the buffer must be physically >> contiguous. > > Both only apply to userptr buffers. I guess saying this in documentation > only is enough. I don't follow you. Perhaps there is some confusion here? The flags field is set by the driver, not by userspace. So PHYS_CONTIG applies to any type of buffer if the driver uses dma-contig. And the alignment is valid for all drivers as well. > > The approach looks good to me. > >> dma-contig: the PHYS_CONTIG flag is always set and the alignment is (unless overridden >> by the driver) page aligned. >> >> dma-sg: the PHYS_CONTIG flag is 0 and the alignment will depend on the driver DMA >> implementation. Note: malloc will align the buffer to 8 bytes on a 32 bit OS and 16 bytes >> on a 64 bit OS. >> >> vmalloc: PHYS_CONFIG is 0 and the alignment should be 3 (2^3 == 8) for 32 bit and >> 4 (2^4=16) for 64 bit OS. This matches malloc() which will align the buffer to >> 8 bytes on a 32 bit OS and 16 bytes on a 64 bit OS. > > Ack. Many dma-sg drivers actually can handle physically contiguous memory > since they're behind an IOMMU; the drivers can then set the flag if needed. All dma-sg drivers can handle phys contig memory since that's just one DMA descriptor. The flag is meant to say that the buffer *has* to be phys contig, not that it might be. So dma-sg drivers will not set it, since they don't have that requirement. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
