Hi!
Considering that the linux DMA-API states that information in an sg-list
may be destroyed when it's mapped,
it seems to me that at least one of the drm prime functions use invalid
assumptions.
In particular, I don't think it's safe to assume that pages in a single
sg-list segment are contigous after mapping, so
if we want struct page pointers we should use
pfn = dma_to_phys((sg_dma_address(sg) + p_offset*PAGE_SIZE)) >> PAGE_SHIFT
and if the pfn is valid, convert it to a struct page.
(Incorrect code is, for example, in drm_prime_sg_to_page_addr_arrays)
Or does dma-buf require that page info in sg-lists need to be kept
across the map operation?
BTW this brings up another question: It's stated that the above function
is needed by the TTM driver in order to do
correct fault handling. This seems odd, TTM shouldn't be able to mmap()
or fault an imported dma-buf, right?
Thanks,
Thomas
On Thu, 2013-10-17 at 13:37 -0500, Matt Sealey wrote:
> This may be late, but please can you consider re-using the CHRP
> reserved node (i.e. device_type = "reserved")?
>
> Since it does exactly the same thing, is well defined since the dark ages?
>
> It's CHRP 1.7 section 5.9 by the way (just before /chosen gets defined).
>
> It would solve a selection of the issues; and require zero binding
> work except describing perhaps a couple freakish Linux-specific
> properties that may be only as intrusive as, say, linux,initrd would
> be in /chosen.
>
> The most effective, multi-OS way of using it ("available" property not
> currently implemented in Linux for some reason, but it could come in
> so handy - not only because it matches the way Linux resource
> structures are handled)
First, the original /reserved on CHRP was supposed to be about reserved
bus space for things like hidden HW devices, but yes, it could be used for
that. However it would be nice to enrich the binding to provide at least
some kind of specific identification of what a given reserved area is about,
see my comments about that in the previous threads.
The available property is of no use to us. It purely indicates what is
available while OFW is still running. Once we get rid of OFW its content
is utterly meaningless.
The original OFW was design with the idea that OFW remains alive along
with the operating system, and that has been done on some platforms, but
that idea has been ditched very early on in powerpc space for many
reasons, one of them being that most implementations of OFW around were
way to broken to bother.
> memory@0x70000000 {
> device_type = "memory";
> reg = <0x70000000 0x40000000>;
> available = <0x70000000 0x10000000
> 0x90000000 0x1ffc00000>; /* top 16KiB of memory
> is where the secure firmware keeps it's mailboxes */
> };
> freaky-codec-memory: reserved@0x80000000 {
> device_type = "reserved";
> reg = <0x80000000 0x10000000>;
> available = <0x80000000 0x8000000
> 0x88000000 0x8000000>; /* two 128MiB buffers */
> non-objectionable-mark-as-contiguous-property-name-here;
> cacheable;
> };
>
> Any driver that has, previously, required a bunch of it's own memory
> carved out of DDR *should* be gaining a phandle reference to that
> reserved node however it likes (it would be up to that devices'
> binding).
>
> On Linux under CMA, it may well be ignored and just stuffed into the
> generic CMA regions, and the driver MAY allocate anywhere it likes,
> but it COULD ask for memory based on a region phandle or, horribly, by
> name (since there's no other way to search for it, the OF "name" for
> reserved SHALL be "reserved") and be given memory in that region
> defined by the reserved node if it had any addressing restrictions.
>
> /videodecoder@0x43f01000 {
> compatible = "freaky,codec";
> :
> decode-buffer = &freaky-codec-memory;
> };
>
> On another OS it may manually map and use a custom allocator for that
> memory region, since otherwise the OS would not have even looked at
> it.
>
> Also this discussion of Jeremy Kerr's proposal seems to be 'missing'
> on Google. Do you happen to have a link to it?
>
> Thanks,
> Matt Sealey <neko(a)bakuhatsu.net>
Hello,
I thought that v6 series is really ready for merging, but I found today
a stupid bug which broke compilation when CONFIG_CMA was not enabled, so
another respin is needed. I've also took the opportunity and fixed
some minor issues in the documentation (renamed all nodes to 'region')
and rebased the patches onto Aneesh Kumar K.V patch queued earlier for
merging.
For more information, please refer to the V6 patches thread:
http://thread.gmane.org/gmane.linux.ports.arm.kernel/261518
Best regards
Marek Szyprowski
Samsung R&D Institute Poland
Changelog:
v7:
- fixed compilation break when CMA support has been disabled
- rebased onto Aneesh Kumar K.V CMA patches queued earlier for merging
- more cleanups in the binding documentation (thanks to Stephen Warren)
v6: http://thread.gmane.org/gmane.linux.ports.arm.kernel/261518
- added support for 'status' property, so memory regions can be disabled
like any other nodes
- fixed issues pointed by Rob: removed annotations from function
declarations and replaced macros with static inline functions.
- restored of_scan_flat_dt_by_path() function to simplify reserved memory
scanning function
- the code now uses #size-cells/#address-cells properties from root node
for interpreting 'reg' property in reserved memory regions
- fixed some issues in dt binding documentation
v5: http://thread.gmane.org/gmane.linux.ports.arm.kernel/259278
- renamed "contiguous-memory-region" compatibility string to
"linux,contiguous-memory-region" (this one is really specific to Linux
kernel implementation)
- renamed "dma-memory-region" property to "memory-region" (suggested by
Kumar Gala)
- added support for #address-cells, #size-cells for memory regions
(thanks to Rob Herring for suggestion)
- removed generic code to scan specific path in flat device tree (cannot
be used to fdt one-pass scan based initialization of memory regions with
#address-cells and #size-cells parsing)
- replaced dma_contiguous_set_default_area() and dma_contiguous_add_device()
functions with dev_set_cma_area() call
v4: http://thread.gmane.org/gmane.linux.ports.arm.kernel/256491
- corrected Devcie Tree mailing list address (resend)
- moved back contiguous-memory bindings from /chosen/contiguous-memory
to /memory nodes as suggested by Grant (see
http://article.gmane.org/gmane.linux.drivers.devicetree/41030
for more details)
- added support for DMA reserved memory with dma_declare_coherent()
- moved code to drivers/of/of_reserved_mem.c
- added generic code to scan specific path in flat device tree
v3: http://thread.gmane.org/gmane.linux.drivers.devicetree/40013/
- fixed issues pointed by Laura and updated documentation
v2: http://thread.gmane.org/gmane.linux.drivers.devicetree/34075
- moved contiguous-memory bindings from /memory to /chosen/contiguous-memory/
node to avoid spreading Linux specific parameters over the whole device
tree definitions
- added support for autoconfigured regions (use zero base)
- fixes minor bugs
v1: http://thread.gmane.org/gmane.linux.drivers.devicetree/30111/
- initial proposal
Patch summary:
Marek Szyprowski (4):
drivers: dma-contiguous: clean source code and prepare for device
tree
drivers: of: add function to scan fdt nodes given by path
drivers: of: add initialization code for dma reserved memory
ARM: init: add support for reserved memory defined by device tree
Documentation/devicetree/bindings/memory.txt | 168 +++++++++++++++++++++++++
arch/arm/include/asm/dma-contiguous.h | 1 -
arch/arm/mm/init.c | 3 +
arch/x86/include/asm/dma-contiguous.h | 1 -
drivers/base/dma-contiguous.c | 119 +++++++-----------
drivers/of/Kconfig | 6 +
drivers/of/Makefile | 1 +
drivers/of/fdt.c | 76 +++++++++++
drivers/of/of_reserved_mem.c | 175 ++++++++++++++++++++++++++
drivers/of/platform.c | 4 +
include/asm-generic/dma-contiguous.h | 28 -----
include/linux/device.h | 2 +-
include/linux/dma-contiguous.h | 62 ++++++++-
include/linux/of_fdt.h | 3 +
include/linux/of_reserved_mem.h | 14 +++
15 files changed, 555 insertions(+), 108 deletions(-)
create mode 100644 Documentation/devicetree/bindings/memory.txt
create mode 100644 drivers/of/of_reserved_mem.c
delete mode 100644 include/asm-generic/dma-contiguous.h
create mode 100644 include/linux/of_reserved_mem.h
--
1.7.9.5
Hello,
This is a third version of my proposal for device tree integration for
Contiguous Memory Allocator. Over 2 month has passed from the time I've
posted the second version, but now some things about using /chosen
device tree node has been clarified, so I expect to get this code merged
soon this time.
Just a few words for those who see this code for the first time:
The proposed bindings allows to define contiguous memory regions of
specified base address and size. Then, the defined regions can be
assigned to the given device(s) by adding a property with a phanle to
the defined contiguous memory region. From the device tree perspective
that's all. Once the bindings are added, all the memory allocations from
dma-mapping subsystem will be served from the defined contiguous memory
regions.
Contiguous Memory Allocator is a framework, which lets to provide a
large contiguous memory buffers for (usually a multimedia) devices. The
contiguous memory is reserved during early boot and then shared with
kernel, which is allowed to allocate it for movable pages. Then, when
device driver requests a contigouous buffer, the framework migrates
movable pages out of contiguous region and gives it to the driver. When
device driver frees the buffer, it is added to kernel memory pool again.
For more information, please refer to commit c64be2bb1c6eb43c838b2c6d57
("drivers: add Contiguous Memory Allocator") and d484864dd96e1830e76895
(CMA merge commit).
Why we need device tree bindings for CMA at all?
Older ARM kernels used so called board-based initialization. Those board
files contained a definition of all hardware blocks available on the
target system and particular kernel and driver software configuration
selected by the board maintainer.
In the new approach the board files will be removed completely and
Device Tree approach is used to describe all hardware blocks available
on the target system. By definition, the bindings should be software
independent, so at least in theory it should be possible to use those
bindings with other operating systems than Linux kernel.
However we also need to pass somehow the information about kernel and
device driver software-only configuration data, which were earlier
encoded in the board file. For such data I decided to use /chosen node,
where kernel command line has been already stored. Future bootloaders
will allow to modify or replace particular nodes and one will be able to
use custom /chosen node to configure his system. The proposed patches
introduce /chosen/contiguous-memory node and related bindings, to avoid
complicated encoding of CMA related configuration to kernel command
line.
Some rationale for using /chosen/ node for kernel configuration entities
has been already suggested by Grant Likely in the following thread:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/155296/focus=34363
Best regards
Marek Szyprowski
Samsung R&D Institute Poland
Changelog:
v3:
- fixed issues pointed by Laura and updated documentation
v2: http://thread.gmane.org/gmane.linux.drivers.devicetree/34075
- moved contiguous-memory bindings from /memory to /chosen/contiguous-memory/
node to avoid spreading Linux specific parameters over the whole device
tree definitions
- added support for autoconfigured regions (use zero base)
- fixes minor bugs
v1: http://thread.gmane.org/gmane.linux.drivers.devicetree/30111/
- initial proposal
Patch summary:
Marek Szyprowski (2):
drivers: dma-contiguous: clean source code and prepare for device
tree
drivers: dma-contiguous: add initialization from device tree
.../devicetree/bindings/contiguous-memory.txt | 94 ++++++
arch/arm/boot/dts/skeleton.dtsi | 7 +-
drivers/base/dma-contiguous.c | 325 +++++++++++++-------
include/asm-generic/dma-contiguous.h | 2 -
include/linux/dma-contiguous.h | 32 +-
5 files changed, 352 insertions(+), 108 deletions(-)
create mode 100644 Documentation/devicetree/bindings/contiguous-memory.txt
--
1.7.9.5
Hi,
On Tue, 2013-08-06 at 12:31 +0100, Tom Cooksey wrote:
> > >> On Fri, Jul 26, 2013 at 11:58 AM, Tom Cooksey <tom.cooksey(a)arm.com>
> > >> wrote:
> > that was part of the reason to punt this problem to userspace ;-)
> >
> > In practice, the kernel drivers doesn't usually know too much about
> > the dimensions/format/etc.. that is really userspace level knowledge.
> > There are a few exceptions when the kernel needs to know how to setup
> > GTT/etc for tiled buffers, but normally this sort of information is up
> > at the next level up (userspace, and drm_framebuffer in case of
> > scanout). Userspace media frameworks like GStreamer already have a
> > concept of format/caps negotiation. For non-display<->gpu sharing, I
> > think this is probably where this sort of constraint negotiation
> > should be handled.
Egads. GStreamer's caps negotiation is already close to unbounded time;
seems like most of the optimisation work that goes into it these days is
all about _reducing_ the complexity of caps negotiation!
> I agree that user-space will know which devices will access the buffer
> and thus can figure out at least a common pixel format.
Hm, are you sure about that? The answer is yes for 'userspace' as a
broad handwave, but not necessarily for individual processes. Take, for
instance, media decode through GStreamer, being displayed by Wayland
using a KMS plane/overlay/sprite/cursor/etc. The media player knows
that the buffers are coming from the decode engine, and Wayland knows
that the buffers are going to a KMS plane, but neither of them knows the
full combination of the two.
Though this kinda feeds into an idea I've been kicking around for a
while, which is an 'optimal hint' mechanism in the Wayland protocol. So
for our hypothetical dmabuf-using protocol, we'd start off with buffers
which satisfied all the constraints of our media decode engine, but
perhaps just the GPU rather than display controller. At this point,
we'd note that we could place the video in a plane if only the buffers
were better-allocated, and send an event to the client letting it know
how to tweak its buffer allocation for more optimal display.
But ...
> Though I'm not
> so sure userspace can figure out more low-level details like alignment
> and placement in physical memory, etc.
>
> Anyway, assuming user-space can figure out how a buffer should be
> stored in memory, how does it indicate this to a kernel driver and
> actually allocate it? Which ioctl on which device does user-space
> call, with what parameters? Are you suggesting using something like
> ION which exposes the low-level details of how buffers are laid out in
> physical memory to userspace? If not, what?
... this is still rather unresolved. ;)
Cheers,
Daniel
>
> Cheers,
>
> Tom
>
>
>
>
>
>
> _______________________________________________
> Linaro-mm-sig mailing list
> Linaro-mm-sig(a)lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-mm-sig
It is not needed to include asm/dma-contiguous.h header to compile
reserved memory initialization code, so remove it to avoid build break
on architectures without CMA support.
Signed-off-by: Marek Szyprowski <m.szyprowski(a)samsung.com>
---
drivers/of/of_reserved_mem.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index a754b84..0fe40c7 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -11,8 +11,6 @@
* License or (at your optional) any later version of the license.
*/
-#include <asm/dma-contiguous.h>
-
#include <linux/memblock.h>
#include <linux/err.h>
#include <linux/of.h>
--
1.7.9.5
Hi Linus,
Here's the 3.12 pull request for dma-buf framework updates. Its yet another
small one - dma-buf framework now supports size discovery of the buffer via
llseek.
Could you please pull?
Thanks and best regards,
~Sumit.
The following changes since commit 26b0332e30c7f93e780aaa054bd84e3437f84354:
Merge tag 'dmaengine-3.12' of git://
git.kernel.org/pub/scm/linux/kernel/git/djbw/dmaengine (2013-09-09 18:07:15
-0700)
are available in the git repository at:
git://git.linaro.org/people/sumitsemwal/linux-dma-buf.git tags/for-3.12
for you to fetch changes up to 19e8697ba45e7bcdb04f2adf6110fbf4882863e5:
dma-buf: Expose buffer size to userspace (v2) (2013-09-10 11:36:45 +0530)
----------------------------------------------------------------
dma-buf updates for 3.12
----------------------------------------------------------------
Christopher James Halse Rogers (1):
dma-buf: Expose buffer size to userspace (v2)
Tuomas Tynkkynen (1):
dma-buf: Check return value of anon_inode_getfile
Documentation/dma-buf-sharing.txt | 12 ++++++++++++
drivers/base/dma-buf.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 44 insertions(+)
Each dma-buf has an associated size and it's reasonable for userspace
to want to know what it is.
Since userspace already has an fd, expose the size using the
size = lseek(fd, SEEK_END, 0); lseek(fd, SEEK_CUR, 0);
idiom.
v2: Added Daniel's sugeested documentation, with minor fixups
Signed-off-by: Christopher James Halse Rogers <christopher.halse.rogers(a)canonical.com>
Reviewed-by: Daniel Vetter <daniel.vetter(a)ffwll.ch>
Tested-by: Daniel Vetter <daniel.vetter(a)ffwll.ch>
---
Documentation/dma-buf-sharing.txt | 12 ++++++++++++
drivers/base/dma-buf.c | 28 ++++++++++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/Documentation/dma-buf-sharing.txt b/Documentation/dma-buf-sharing.txt
index 0b23261..849e982 100644
--- a/Documentation/dma-buf-sharing.txt
+++ b/Documentation/dma-buf-sharing.txt
@@ -407,6 +407,18 @@ Being able to mmap an export dma-buf buffer object has 2 main use-cases:
interesting ways depending upong the exporter (if userspace starts depending
upon this implicit synchronization).
+Other Interfaces Exposed to Userspace on the dma-buf FD
+------------------------------------------------------
+
+- Since kernel 3.12 the dma-buf FD supports the llseek system call, but only
+ with offset=0 and whence=SEEK_END|SEEK_SET. SEEK_SET is supported to allow
+ the usual size discover pattern size = SEEK_END(0); SEEK_SET(0). Every other
+ llseek operation will report -EINVAL.
+
+ If llseek on dma-buf FDs isn't support the kernel will report -ESPIPE for all
+ cases. Userspace can use this to detect support for discovering the dma-buf
+ size using llseek.
+
Miscellaneous notes
-------------------
diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index 6687ba7..c33a857 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -77,9 +77,36 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
return dmabuf->ops->mmap(dmabuf, vma);
}
+static loff_t dma_buf_llseek(struct file *file, loff_t offset, int whence)
+{
+ struct dma_buf *dmabuf;
+ loff_t base;
+
+ if (!is_dma_buf_file(file))
+ return -EBADF;
+
+ dmabuf = file->private_data;
+
+ /* only support discovering the end of the buffer,
+ but also allow SEEK_SET to maintain the idiomatic
+ SEEK_END(0), SEEK_CUR(0) pattern */
+ if (whence == SEEK_END)
+ base = dmabuf->size;
+ else if (whence == SEEK_SET)
+ base = 0;
+ else
+ return -EINVAL;
+
+ if (offset != 0)
+ return -EINVAL;
+
+ return base + offset;
+}
+
static const struct file_operations dma_buf_fops = {
.release = dma_buf_release,
.mmap = dma_buf_mmap_internal,
+ .llseek = dma_buf_llseek,
};
/*
@@ -133,6 +160,7 @@ struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops *ops,
dmabuf->exp_name = exp_name;
file = anon_inode_getfile("dmabuf", &dma_buf_fops, dmabuf, flags);
+ file->f_mode |= FMODE_LSEEK;
dmabuf->file = file;
--
1.8.3.2