Hi Linus,
This isn't the drm merge, however a few projects have been working together on a buffer sharing mechanism for kernel drivers, the maim interested parties are the Linaro/ARM SoC folks, V4L, DRM, and fbdev. Sumit Semwal wrote the code, and its been reviewed on various lists a fair few times.
Now we've all agreed that the initial implementation is a good baseline for us to move forward on, but its messy working with others when the core code is out of tree. So we'd like to merge the core dma-buf code now so we can all build on top of it for 3.4. I know some people would say we shouldn't merge things with no users, but it will really make our lives easier going forward to get this baseline into the tree.
Currently I've got most of a drm inter-driver buffer sharing mechanism built on top and there is also got a v4l consumer built. The code doesn't introduce any new userspace interfaces, its purely a kernel internal layer, that the consumer/exported layers like drm/v4l will plug into to offer services.
Dave.
The following changes since commit 805a6af8dba5dfdd35ec35dc52ec0122400b2610:
Linux 3.2 (2012-01-04 15:55:44 -0800)
are available in the git repository at: git://people.freedesktop.org/~airlied/linux dma-buf-merge
Sumit Semwal (3): dma-buf: Introduce dma buffer sharing mechanism dma-buf: Documentation for buffer sharing framework dma-buf: mark EXPERIMENTAL for 1st release.
Documentation/dma-buf-sharing.txt | 224 ++++++++++++++++++++++++++++ drivers/base/Kconfig | 11 ++ drivers/base/Makefile | 1 + drivers/base/dma-buf.c | 291 +++++++++++++++++++++++++++++++++++++ include/linux/dma-buf.h | 176 ++++++++++++++++++++++ 5 files changed, 703 insertions(+), 0 deletions(-) create mode 100644 Documentation/dma-buf-sharing.txt create mode 100644 drivers/base/dma-buf.c create mode 100644 include/linux/dma-buf.h
On Fri, Jan 6, 2012 at 7:06 AM, Dave Airlie airlied@linux.ie wrote:
Now we've all agreed that the initial implementation is a good baseline for us to move forward on, but its messy working with others when the core code is out of tree. So we'd like to merge the core dma-buf code now so we can all build on top of it for 3.4. I know some people would say we shouldn't merge things with no users, but it will really make our lives easier going forward to get this baseline into the tree.
Ok, merged.
However, I ask myself whether it's ever sane to ask the user to enable this? Isn't this very much a "drivers will use 'select' to pick up the intfrastructure code" kind of thing?
Linus
On 9 January 2012 03:38, Linus Torvalds torvalds@linux-foundation.org wrote:
On Fri, Jan 6, 2012 at 7:06 AM, Dave Airlie airlied@linux.ie wrote:
Now we've all agreed that the initial implementation is a good baseline for us to move forward on, but its messy working with others when the core code is out of tree. So we'd like to merge the core dma-buf code now so we can all build on top of it for 3.4. I know some people would say we shouldn't merge things with no users, but it will really make our lives easier going forward to get this baseline into the tree.
Ok, merged.
Hi Linus, Thanks!
However, I ask myself whether it's ever sane to ask the user to enable this? Isn't this very much a "drivers will use 'select' to pick up the intfrastructure code" kind of thing?
Yes, you're right; I will submit a patch to change appropriately, as well as document it as well. Would it be ok to submit this change alongwith one of the first users of the framework, or would you rather prefer it in the rc2 / rc3 time frame?
Linus
Best regards, ~Sumit.
On Mon, Jan 9, 2012 at 6:44 AM, Sumit Semwal sumit.semwal@linaro.org wrote:
On 9 January 2012 03:38, Linus Torvalds torvalds@linux-foundation.org wrote:
On Fri, Jan 6, 2012 at 7:06 AM, Dave Airlie airlied@linux.ie wrote:
Now we've all agreed that the initial implementation is a good baseline for us to move forward on, but its messy working with others when the core code is out of tree. So we'd like to merge the core dma-buf code now so we can all build on top of it for 3.4. I know some people would say we shouldn't merge things with no users, but it will really make our lives easier going forward to get this baseline into the tree.
Ok, merged.
Hi Linus, Thanks!
However, I ask myself whether it's ever sane to ask the user to enable this? Isn't this very much a "drivers will use 'select' to pick up the intfrastructure code" kind of thing?
Yes, you're right; I will submit a patch to change appropriately, as well as document it as well. Would it be ok to submit this change alongwith one of the first users of the framework, or would you rather prefer it in the rc2 / rc3 time frame?
I'll just sent this patch to Linus in -fixes if okay
commit 3b32a592ea6e49145d4dc610b85dd9042226896d Author: Dave Airlie airlied@redhat.com Date: Fri Jan 13 09:05:14 2012 +0000
dma-buf: drop option text so users don't select it.
This is going to be used by other subsystems so they should select it.
Signed-off-by: Dave Airlie airlied@redhat.com
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index fcbec8a..7be9f79 100644 --- a/drivers/base/Kconfig +++ b/drivers/base/Kconfig @@ -179,7 +179,7 @@ config GENERIC_CPU_DEVICES source "drivers/base/regmap/Kconfig"
config DMA_SHARED_BUFFER - bool "Buffer framework to be shared between drivers" + bool default n select ANON_INODES depends on EXPERIMENTAL
Hi Dave,
On 13 January 2012 14:38, Dave Airlie airlied@gmail.com wrote:
On Mon, Jan 9, 2012 at 6:44 AM, Sumit Semwal sumit.semwal@linaro.org wrote:
On 9 January 2012 03:38, Linus Torvalds torvalds@linux-foundation.org wrote:
<snip>
However, I ask myself whether it's ever sane to ask the user to enable this? Isn't this very much a "drivers will use 'select' to pick up the intfrastructure code" kind of thing?
Yes, you're right; I will submit a patch to change appropriately, as well as document it as well. Would it be ok to submit this change alongwith one of the first users of the framework, or would you rather prefer it in the rc2 / rc3 time frame?
I'll just sent this patch to Linus in -fixes if okay
<snip> This looks ok; I have a small documentation update patch as well for this - I will send it to this list now. Could you please add that in your -fixes too? Best regards, ~Sumit.
On Fri, Jan 13, 2012 at 9:38 AM, Sumit Semwal sumit.semwal@linaro.org wrote:
Hi Dave,
On 13 January 2012 14:38, Dave Airlie airlied@gmail.com wrote:
On Mon, Jan 9, 2012 at 6:44 AM, Sumit Semwal sumit.semwal@linaro.org wrote:
On 9 January 2012 03:38, Linus Torvalds torvalds@linux-foundation.org wrote:
<snip> >>> However, I ask myself whether it's ever sane to ask the user to enable >>> this? Isn't this very much a "drivers will use 'select' to pick up the >>> intfrastructure code" kind of thing? >> Yes, you're right; I will submit a patch to change appropriately, as >> well as document it as well. Would it be ok to submit this change >> alongwith one of the first users of the framework, or would you rather >> prefer it in the rc2 / rc3 time frame? > > I'll just sent this patch to Linus in -fixes if okay > <snip> This looks ok; I have a small documentation update patch as well for this - I will send it to this list now. Could you please add that in your -fixes too?
Yes please do,
Dave.
The DMA buffer infrastructure (dma-buf) currently exposes its interface with EXPORT_SYMBOL_GPL. The documentation for EXPORT_SYMBOL_GPL says: "It implies that the function is considered an internal implementation issue, and not really an interface." This interface is clearly not just an "implementation issue" but an interface to be used across drivers/subsystems, so I think it makes sense for it to use EXPORT_SYMBOL instead.
Work on dma-buf was originally started with the goal of unifying several competing "memory management" systems developed with different ARM SoCs in mind. It would be unfortunate if restricting its use to only GPL-licensed modules caused dma-buf adoption to be limited.
For convenience, I'll send the trivial patch to implement this change. I'd like to see this in the first release with dma-buf in 3.3.
Thanks, Robert
EXPORT_SYMBOL_GPL is intended to be used for "an internal implementation issue, and not really an interface". The dma-buf infrastructure is explicitly intended as an interface between modules/drivers, so it should use EXPORT_SYMBOL instead.
Signed-off-by: Robert Morell rmorell@nvidia.com --- drivers/base/dma-buf.c | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c index e38ad24..4ed5a5d 100644 --- a/drivers/base/dma-buf.c +++ b/drivers/base/dma-buf.c @@ -101,7 +101,7 @@ struct dma_buf *dma_buf_export(void *priv, struct dma_buf_ops *ops,
return dmabuf; } -EXPORT_SYMBOL_GPL(dma_buf_export); +EXPORT_SYMBOL(dma_buf_export);
/** @@ -126,7 +126,7 @@ int dma_buf_fd(struct dma_buf *dmabuf)
return fd; } -EXPORT_SYMBOL_GPL(dma_buf_fd); +EXPORT_SYMBOL(dma_buf_fd);
/** * dma_buf_get - returns the dma_buf structure related to an fd @@ -152,7 +152,7 @@ struct dma_buf *dma_buf_get(int fd)
return file->private_data; } -EXPORT_SYMBOL_GPL(dma_buf_get); +EXPORT_SYMBOL(dma_buf_get);
/** * dma_buf_put - decreases refcount of the buffer @@ -167,7 +167,7 @@ void dma_buf_put(struct dma_buf *dmabuf)
fput(dmabuf->file); } -EXPORT_SYMBOL_GPL(dma_buf_put); +EXPORT_SYMBOL(dma_buf_put);
/** * dma_buf_attach - Add the device to dma_buf's attachments list; optionally, @@ -213,7 +213,7 @@ err_attach: mutex_unlock(&dmabuf->lock); return ERR_PTR(ret); } -EXPORT_SYMBOL_GPL(dma_buf_attach); +EXPORT_SYMBOL(dma_buf_attach);
/** * dma_buf_detach - Remove the given attachment from dmabuf's attachments list; @@ -235,7 +235,7 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) mutex_unlock(&dmabuf->lock); kfree(attach); } -EXPORT_SYMBOL_GPL(dma_buf_detach); +EXPORT_SYMBOL(dma_buf_detach);
/** * dma_buf_map_attachment - Returns the scatterlist table of the attachment; @@ -265,7 +265,7 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
return sg_table; } -EXPORT_SYMBOL_GPL(dma_buf_map_attachment); +EXPORT_SYMBOL(dma_buf_map_attachment);
/** * dma_buf_unmap_attachment - unmaps and decreases usecount of the buffer;might @@ -288,4 +288,4 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach, mutex_unlock(&attach->dmabuf->lock);
} -EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment); +EXPORT_SYMBOL(dma_buf_unmap_attachment);
On Wed, Jan 18, 2012 at 5:38 AM, Robert Morell rmorell@nvidia.com wrote:
EXPORT_SYMBOL_GPL is intended to be used for "an internal implementation issue, and not really an interface". The dma-buf infrastructure is explicitly intended as an interface between modules/drivers, so it should use EXPORT_SYMBOL instead.
+ Konrad, Arnd, Mauro: there were strong objections on using EXPORT_SYMBOL in place of EXPORT_SYMBOL_GPL by all 3 of them; I suggest we first arrive at a consensus before merging this patch.
Best regards, ~Sumit. <snip>
On Wednesday 18 January 2012, Semwal, Sumit wrote:
On Wed, Jan 18, 2012 at 5:38 AM, Robert Morell rmorell@nvidia.com wrote:
EXPORT_SYMBOL_GPL is intended to be used for "an internal implementation issue, and not really an interface". The dma-buf infrastructure is explicitly intended as an interface between modules/drivers, so it should use EXPORT_SYMBOL instead.
- Konrad, Arnd, Mauro: there were strong objections on using
EXPORT_SYMBOL in place of EXPORT_SYMBOL_GPL by all 3 of them; I suggest we first arrive at a consensus before merging this patch.
We discussed this before. The reason for using EXPORT_SYMBOL_GPL here is that the interface is low-level and that it's meant to be used by subsystems that export user-level interface based on that and come with their own device driver interface, such as V4L or DRM.
While there is an eternal debate over whether there should or should not be out of tree device drivers, I think there is very little to gain by allowing dma_buf to be used by out of tree *subsystems*. Further, a device driver that tries to use the interface but sits outside of the regular subsystems is a bad technical choice and we should not encourage those either.
NAK
Arnd
On Wed, Jan 18, 2012 at 12:14 PM, Arnd Bergmann arnd@arndb.de wrote:
On Wednesday 18 January 2012, Semwal, Sumit wrote:
On Wed, Jan 18, 2012 at 5:38 AM, Robert Morell rmorell@nvidia.com wrote:
EXPORT_SYMBOL_GPL is intended to be used for "an internal implementation issue, and not really an interface". The dma-buf infrastructure is explicitly intended as an interface between modules/drivers, so it should use EXPORT_SYMBOL instead.
- Konrad, Arnd, Mauro: there were strong objections on using
EXPORT_SYMBOL in place of EXPORT_SYMBOL_GPL by all 3 of them; I suggest we first arrive at a consensus before merging this patch.
We discussed this before. The reason for using EXPORT_SYMBOL_GPL here is that the interface is low-level and that it's meant to be used by subsystems that export user-level interface based on that and come with their own device driver interface, such as V4L or DRM.
While there is an eternal debate over whether there should or should not be out of tree device drivers, I think there is very little to gain by allowing dma_buf to be used by out of tree *subsystems*. Further, a device driver that tries to use the interface but sits outside of the regular subsystems is a bad technical choice and we should not encourage those either.
The problem is the x86 nvidia binary driver does sit outside of subsystems, and I forsee wanting to share buffers with it from the Intel driver in light of the optimus hardware. Although nouveau exists and I'd much rather nvidia get behind that wrt the kernel stuff, I don't forsee that happening.
From an X.org point of view it would be useful to end-users to allow
this sort of thing, nouveau is a good solution its just not going to beat the binary driver in a lot of ways, just not sure whether I care enough yet.
Dave.
On Wed, 18 Jan 2012, Dave Airlie wrote:
The problem is the x86 nvidia binary driver does sit outside of subsystems, and I forsee wanting to share buffers with it from the Intel driver in light of the optimus hardware. Although nouveau exists and I'd much rather nvidia get behind that wrt the kernel stuff, I don't forsee that happening.
Please correct me if I blab a nonsense here, but just the other day, we have seen a different thread in which it was decided that user cannot turn on buffer sharing at compile time explicitly, but rather a driver that needs it would turn it on automatically.
Doesn't that alone exclude out-of-tree drivers? In other words if you have two out-of-tree drivers that want to use DMA buffer sharing, and no other enabled driver in the kernel enables it implicitly, then such a kernel won't make it possible for said two drivers to work.
On a related note, EXPORT_SYMBOL_GPL will still happily link with out-of-tree driver, for as long as that driver comes under GPL-compatible license. So it's not really a question of whether the driver is out-of-tree or in-tree, but it's a question of driver's license.
Frankly, I never understood this "low-level interface" argument that is kicked around when EXPORT_SYMBOL_GPL topic is brought up. My view to EXPORT_SYMBOL vs. EXPORT_SYMBOL_GPL is that it really boils down to license controversy about binary/proprietary modules in Linux kernel. To me it's about whether the authors of certain code (for mostly phylosophical reasons) agree that their (GPL) code is OK or not OK to link against non-GPL module.
From that angle, I am not sure if it is ethical at all to modify how the
symbol is exported without explicit consent of the original author (regardless of what we think about GPL/proprietary modules covtroversy). So if NVidia needs to link DMA buffer sharing against their proprietary driver, they should have explicit permission from the original author to turn its symbols into EXPORT_SYMBOL.
-- Ilija
On Wed, Jan 18, 2012 at 1:55 PM, Ilija Hadzic ihadzic@research.bell-labs.com wrote:
On Wed, 18 Jan 2012, Dave Airlie wrote:
The problem is the x86 nvidia binary driver does sit outside of subsystems, and I forsee wanting to share buffers with it from the Intel driver in light of the optimus hardware. Although nouveau exists and I'd much rather nvidia get behind that wrt the kernel stuff, I don't forsee that happening.
Please correct me if I blab a nonsense here, but just the other day, we have seen a different thread in which it was decided that user cannot turn on buffer sharing at compile time explicitly, but rather a driver that needs it would turn it on automatically.
Doesn't that alone exclude out-of-tree drivers? In other words if you have two out-of-tree drivers that want to use DMA buffer sharing, and no other enabled driver in the kernel enables it implicitly, then such a kernel won't make it possible for said two drivers to work.
Well the use case at least on x86 would be open x86 driver sharing with closed nvidia driver, if two closed drivers wanted to share I'd except them to do it internally anyways.
Frankly, I never understood this "low-level interface" argument that is kicked around when EXPORT_SYMBOL_GPL topic is brought up. My view to EXPORT_SYMBOL vs. EXPORT_SYMBOL_GPL is that it really boils down to license controversy about binary/proprietary modules in Linux kernel. To me it's about whether the authors of certain code (for mostly phylosophical reasons) agree that their (GPL) code is OK or not OK to link against non-GPL module.
From that angle, I am not sure if it is ethical at all to modify how the symbol is exported without explicit consent of the original author (regardless of what we think about GPL/proprietary modules covtroversy). So if NVidia needs to link DMA buffer sharing against their proprietary driver, they should have explicit permission from the original author to turn its symbols into EXPORT_SYMBOL.
Which is the point of their patch, to ask permission from the author.
Dave.
On Wed, Jan 18, 2012 at 06:00:54AM -0800, Dave Airlie wrote:
On Wed, Jan 18, 2012 at 1:55 PM, Ilija Hadzic ihadzic@research.bell-labs.com wrote:
On Wed, 18 Jan 2012, Dave Airlie wrote:
The problem is the x86 nvidia binary driver does sit outside of subsystems, and I forsee wanting to share buffers with it from the Intel driver in light of the optimus hardware. Although nouveau exists and I'd much rather nvidia get behind that wrt the kernel stuff, I don't forsee that happening.
Yes, this is one potential use case that I have in mind.
This is digressing a bit, but the binary nvidia driver is the best way that I see that we can support our users with a feature set compatible to that available to other operating systems. For technical reasons, we've chosen to leverage a lot of common code written internally, which allows us to release support for new hardware and software features much more quickly than if those of us working on the Linux/FreeBSD/Solaris drivers wrote it all from scratch. This means that we share a lot with other NVIDIA drivers, but we for better or worse can't share much infrastructure like DRI.
Please correct me if I blab a nonsense here, but just the other day, we have seen a different thread in which it was decided that user cannot turn on buffer sharing at compile time explicitly, but rather a driver that needs it would turn it on automatically.
Doesn't that alone exclude out-of-tree drivers? In other words if you have two out-of-tree drivers that want to use DMA buffer sharing, and no other enabled driver in the kernel enables it implicitly, then such a kernel won't make it possible for said two drivers to work.
Well the use case at least on x86 would be open x86 driver sharing with closed nvidia driver, if two closed drivers wanted to share I'd except them to do it internally anyways.
Correct. Right now, that covers Optimus laptops with Intel integrated graphics and an NVIDIA GPU. We'd only use the dma-buf interface in the case of interoperating with the Intel device.
I only see such hardware becoming more common. For example, in the future, if we can't agree on using EXPORT_SYMBOL, then if somebody were to introduce a laptop that had a Tegra GPU (which uses GPL-compatible open-source Linux drivers) and a GeForce GPU (which is, as described above, supported by our existing binary driver) then I imagine we'd have no choice but to re-implement a different open-source buffer allocation mechanism for Tegra that could be used between the two, or just continue using our existing nvhost code. This, along with every other SoC's version, is exactly what the dma-buf project was intended to replace.
Frankly, I never understood this "low-level interface" argument that is kicked around when EXPORT_SYMBOL_GPL topic is brought up. My view to EXPORT_SYMBOL vs. EXPORT_SYMBOL_GPL is that it really boils down to license controversy about binary/proprietary modules in Linux kernel. To me it's about whether the authors of certain code (for mostly phylosophical reasons) agree that their (GPL) code is OK or not OK to link against non-GPL module.
From that angle, I am not sure if it is ethical at all to modify how the symbol is exported without explicit consent of the original author (regardless of what we think about GPL/proprietary modules covtroversy). So if NVidia needs to link DMA buffer sharing against their proprietary driver, they should have explicit permission from the original author to turn its symbols into EXPORT_SYMBOL.
Which is the point of their patch, to ask permission from the author.
Right. I never intended to submit this patch behind anyone's back, I just wanted to bring this to their attention and ask if the change could be made so that we could better serve shared-graphics users.
Thanks, Robert
On Wednesday 18 January 2012, Ilija Hadzic wrote:
On Wed, 18 Jan 2012, Dave Airlie wrote:
The problem is the x86 nvidia binary driver does sit outside of subsystems, and I forsee wanting to share buffers with it from the Intel driver in light of the optimus hardware. Although nouveau exists and I'd much rather nvidia get behind that wrt the kernel stuff, I don't forsee that happening.
A lot of good stuff is coming out of nvidia these days regarding for their tegra ARM socs. I wouldn't be surprised to see them in the same boat as Intel and AMD regarding their support for free drivers in the near future and I think it's worth supporting the people inside of nvidia that are fighting for that instead of the others that are against it.
Having more interesting features in nouveau that are not in the proprietary driver would probably help on that front, too.
Frankly, I never understood this "low-level interface" argument that is kicked around when EXPORT_SYMBOL_GPL topic is brought up. My view to EXPORT_SYMBOL vs. EXPORT_SYMBOL_GPL is that it really boils down to license controversy about binary/proprietary modules in Linux kernel. To me it's about whether the authors of certain code (for mostly phylosophical reasons) agree that their (GPL) code is OK or not OK to link against non-GPL module.
The controversy is about whether a module is a derived work of the kernel when you link it in. Traditionally, some modules were not considered derived works by a lot of people arguing that we have a public module interface that acts as a barrier for the license in the same way that the syscall interface lets you run proprietary applications.
The EXPORT_SYMBOL_GPL is all about symbols that are too low-level to be considered part of that public module API (assuming that this API exists). The argument is that symbols which are not meant to be used in third-party modules can never be a license barrier and anything using them is a derived work even if you consider other modules not to be a derived work of the kernel. Note that this argument can still hold for low-level symbols that are marked as EXPORT_SYMBOL, using EXPORT_SYMBOL_GPL just makes it explicit.
Arnd
Em 18-01-2012 10:14, Arnd Bergmann escreveu:
On Wednesday 18 January 2012, Semwal, Sumit wrote:
On Wed, Jan 18, 2012 at 5:38 AM, Robert Morell rmorell@nvidia.com wrote:
EXPORT_SYMBOL_GPL is intended to be used for "an internal implementation issue, and not really an interface". The dma-buf infrastructure is explicitly intended as an interface between modules/drivers, so it should use EXPORT_SYMBOL instead.
- Konrad, Arnd, Mauro: there were strong objections on using
EXPORT_SYMBOL in place of EXPORT_SYMBOL_GPL by all 3 of them; I suggest we first arrive at a consensus before merging this patch.
We discussed this before. The reason for using EXPORT_SYMBOL_GPL here is that the interface is low-level and that it's meant to be used by subsystems that export user-level interface based on that and come with their own device driver interface, such as V4L or DRM.
While there is an eternal debate over whether there should or should not be out of tree device drivers, I think there is very little to gain by allowing dma_buf to be used by out of tree *subsystems*. Further, a device driver that tries to use the interface but sits outside of the regular subsystems is a bad technical choice and we should not encourage those either.
NAK
I fully agree with Arnd.
A bug on a driver using such low-level interface could cause side effects at the wrong places. In order to handle such bugs, the developers and the maintainers of both subsystems need to see the source code of the entire pipeline, with is not possible if is there any non-GPL'd driver.
NAK
Mauro.
On Wed, Jan 18, 2012 at 12:23 PM, Mauro Carvalho Chehab mchehab@redhat.com wrote:
Em 18-01-2012 10:14, Arnd Bergmann escreveu:
On Wednesday 18 January 2012, Semwal, Sumit wrote:
On Wed, Jan 18, 2012 at 5:38 AM, Robert Morell rmorell@nvidia.com wrote:
EXPORT_SYMBOL_GPL is intended to be used for "an internal implementation issue, and not really an interface". The dma-buf infrastructure is explicitly intended as an interface between modules/drivers, so it should use EXPORT_SYMBOL instead.
- Konrad, Arnd, Mauro: there were strong objections on using
EXPORT_SYMBOL in place of EXPORT_SYMBOL_GPL by all 3 of them; I suggest we first arrive at a consensus before merging this patch.
We discussed this before. The reason for using EXPORT_SYMBOL_GPL here is that the interface is low-level and that it's meant to be used by subsystems that export user-level interface based on that and come with their own device driver interface, such as V4L or DRM.
While there is an eternal debate over whether there should or should not be out of tree device drivers, I think there is very little to gain by allowing dma_buf to be used by out of tree *subsystems*. Further, a device driver that tries to use the interface but sits outside of the regular subsystems is a bad technical choice and we should not encourage those either.
NAK
I fully agree with Arnd.
A bug on a driver using such low-level interface could cause side effects at the wrong places. In order to handle such bugs, the developers and the maintainers of both subsystems need to see the source code of the entire pipeline, with is not possible if is there any non-GPL'd driver.
Just as an aside you shouldn't be debugging tainted kernels anyways.
The thing is if we don't provide the interface that is known working for this, then we end up requiring the nvidia binary to jump through hoops which is more likely to make thing unstable.
At that point I'd start to seriously consider whether drm needs dma-buf since one of the things I will get pressure to share buffers with is for optimus. The pressure isn't from nvidia, but from users and customers.
I'd like to at least remove debuggabilty from the argument, that is why we have tainting, if you ignore taint + lsmod output then export_symbol won't matter.
Dave.
NAK
Mauro.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Wed, Jan 18, 2012 at 01:10:04AM -0800, Semwal, Sumit wrote:
On Wed, Jan 18, 2012 at 5:38 AM, Robert Morell rmorell@nvidia.com wrote:
EXPORT_SYMBOL_GPL is intended to be used for "an internal implementation issue, and not really an interface". The dma-buf infrastructure is explicitly intended as an interface between modules/drivers, so it should use EXPORT_SYMBOL instead.
- Konrad, Arnd, Mauro: there were strong objections on using
EXPORT_SYMBOL in place of EXPORT_SYMBOL_GPL by all 3 of them; I suggest we first arrive at a consensus before merging this patch.
This discussion seems to have stagnated; how do we move forward here?
Sumit, as the primary author and new maintainer (congrats!) of the dma-buf infrastructure, it seems like it's really your call how to proceed. I'd still like to see this be something that we can use from the nvidia and fglrx drivers for Xorg buffer sharing, as I and Dave have argued in this thread. It really seems to me that this change on a technical level won't have any adverse effect on the scenarios where it can be used today, but it will allow it to be used more widely, which will prevent duplication and fragmentation in the future and be greatly appreciated by users of hardware such as Optimus.
Let me know if you have any questions.
Thanks, Robert
On Fri, Jan 20, 2012 at 10:04:57AM -0800, Robert Morell wrote:
On Wed, Jan 18, 2012 at 01:10:04AM -0800, Semwal, Sumit wrote:
On Wed, Jan 18, 2012 at 5:38 AM, Robert Morell rmorell@nvidia.com wrote:
EXPORT_SYMBOL_GPL is intended to be used for "an internal implementation issue, and not really an interface". The dma-buf infrastructure is explicitly intended as an interface between modules/drivers, so it should use EXPORT_SYMBOL instead.
- Konrad, Arnd, Mauro: there were strong objections on using
EXPORT_SYMBOL in place of EXPORT_SYMBOL_GPL by all 3 of them; I
I am going to defer to what David Airlie recommends.
suggest we first arrive at a consensus before merging this patch.
This discussion seems to have stagnated; how do we move forward here?
Sumit, as the primary author and new maintainer (congrats!) of the dma-buf infrastructure, it seems like it's really your call how to proceed. I'd still like to see this be something that we can use from the nvidia and fglrx drivers for Xorg buffer sharing, as I and Dave have argued in this thread. It really seems to me that this change on a technical level won't have any adverse effect on the scenarios where it can be used today, but it will allow it to be used more widely, which will prevent duplication and fragmentation in the future and be greatly appreciated by users of hardware such as Optimus.
Let me know if you have any questions.
Thanks, Robert
On Fri, Jan 20, 2012 at 10:04:57AM -0800, Robert Morell wrote:
On Wed, Jan 18, 2012 at 01:10:04AM -0800, Semwal, Sumit wrote:
On Wed, Jan 18, 2012 at 5:38 AM, Robert Morell rmorell@nvidia.com wrote:
EXPORT_SYMBOL_GPL is intended to be used for "an internal implementation issue, and not really an interface". The dma-buf infrastructure is explicitly intended as an interface between modules/drivers, so it should use EXPORT_SYMBOL instead.
- Konrad, Arnd, Mauro: there were strong objections on using
EXPORT_SYMBOL in place of EXPORT_SYMBOL_GPL by all 3 of them; I suggest we first arrive at a consensus before merging this patch.
This discussion seems to have stagnated; how do we move forward here?
Sumit, as the primary author and new maintainer (congrats!) of the dma-buf infrastructure, it seems like it's really your call how to proceed. I'd still like to see this be something that we can use from the nvidia and fglrx drivers for Xorg buffer sharing, as I and Dave have argued in this thread. It really seems to me that this change on a technical level won't have any adverse effect on the scenarios where it can be used today, but it will allow it to be used more widely, which will prevent duplication and fragmentation in the future and be greatly appreciated by users of hardware such as Optimus.
Given that I've participated quite a bit in the design of dma_buf as-is, let me throw in my totally irrelevant opinion, too ;-)
I'll refrain from comment on the actual patch, it's obviously a hot topic. Furthermore I might need to ask Intel's legal dep for guidance to asses things wrt my own contributions to dma_buf.
Otoh I'd like nvidia to be on board, especially when we're desingned additions to dma_buf required to make it really work for multiple gpus. In additions it looks like that the nvidia blob will only be an importer of a dma_buf, at least for the use-cases discussed here.
So why don't you just ditch this patch here and add a small shim to your blob to interface with drm's prime as an importing driver? I personally would deem that acceptable and I think Dave wouldn't mind too much, either.
Yours, Daniel
Disclaimer: This is my own opinion and I do not speak as an Intel employee here.
On Sat, Jan 21, 2012 at 11:02 PM, Daniel Vetter daniel@ffwll.ch wrote:
On Fri, Jan 20, 2012 at 10:04:57AM -0800, Robert Morell wrote:
On Wed, Jan 18, 2012 at 01:10:04AM -0800, Semwal, Sumit wrote:
On Wed, Jan 18, 2012 at 5:38 AM, Robert Morell rmorell@nvidia.com wrote:
EXPORT_SYMBOL_GPL is intended to be used for "an internal implementation issue, and not really an interface". The dma-buf infrastructure is explicitly intended as an interface between modules/drivers, so it should use EXPORT_SYMBOL instead.
- Konrad, Arnd, Mauro: there were strong objections on using
EXPORT_SYMBOL in place of EXPORT_SYMBOL_GPL by all 3 of them; I suggest we first arrive at a consensus before merging this patch.
This discussion seems to have stagnated; how do we move forward here?
Sumit, as the primary author and new maintainer (congrats!) of the dma-buf infrastructure, it seems like it's really your call how to proceed. I'd still like to see this be something that we can use from the nvidia and fglrx drivers for Xorg buffer sharing, as I and Dave have argued in this thread. It really seems to me that this change on a technical level won't have any adverse effect on the scenarios where it can be used today, but it will allow it to be used more widely, which will prevent duplication and fragmentation in the future and be greatly appreciated by users of hardware such as Optimus.
Given that I've participated quite a bit in the design of dma_buf as-is, let me throw in my totally irrelevant opinion, too ;-)
I'll refrain from comment on the actual patch, it's obviously a hot topic. Furthermore I might need to ask Intel's legal dep for guidance to asses things wrt my own contributions to dma_buf.
Otoh I'd like nvidia to be on board, especially when we're desingned additions to dma_buf required to make it really work for multiple gpus. In additions it looks like that the nvidia blob will only be an importer of a dma_buf, at least for the use-cases discussed here.
So why don't you just ditch this patch here and add a small shim to your blob to interface with drm's prime as an importing driver? I personally would deem that acceptable and I think Dave wouldn't mind too much, either.
Hi Everyone!
(Apologies for delay in replying; was OoO for past couple of days)
Thanks very much for this discussion - a couple of things:
1. I am definitely willing to make changes as needed to get as many devices / subsystems / frameworks to use the dma-buf infrastructure. This could include changing the way symbols are exported, if that is the *only* way to get things done.
2. With that premise, I quite like the idea that Daniel gave (of course, in his capacity as one of the top contributors behind dma-buf infrastructure, and like he said, not as an Intel employee) - so let me ask the following:
Robert, Dave, Technically speaking, is there no way that the EXPORT_SYMBOL_GPLed symbols can be used by the binary blobs, possibly with an open-sourced shim which provides the buffer-sharing interface to the binary blobs? Are there any reasons to not consider this approach?
Also, if some of you are going to be at ELC mid-Feb at SFO, we could meet up face-to-face and thrash out possible ways forward.
Yours, Daniel
Thanks, and best regards, ~Sumit.
Disclaimer: This is my own opinion and I do not speak as an Intel employee here. -- Daniel Vetter Mail: daniel@ffwll.ch Mobile: +41 (0)79 365 57 48
Technically speaking, is there no way that the EXPORT_SYMBOL_GPLed symbols can be used by the binary blobs, possibly with an open-sourced shim which provides the buffer-sharing interface to the binary blobs? Are there any reasons to not consider this approach?
The GPL requires all the code of a work is source. All of it, no shims no magic glue. EXPORT_SYMBOL isn't an indication you can use it for binary modules. The GPL licence is quite clear on what is covered.
Since you've asked this I'm advised by my lawyer to respond to all such assumptions of legality of binary modules...
For a Linux kernel containing any code I own the code is under the GNU public license v2 (in some cases or later), I have never given permission for that code to be used as part of a combined or derivative work which contains binary chunks. I have never said that modules are somehow magically outside the GPL and I am doubtful that in most cases a work containing binary modules for a Linux kernel is compatible with the licensing, although I accept there may be some cases that it is.
Alan
Em 25-01-2012 10:30, Alan Cox escreveu:
Technically speaking, is there no way that the EXPORT_SYMBOL_GPLed symbols can be used by the binary blobs, possibly with an open-sourced shim which provides the buffer-sharing interface to the binary blobs? Are there any reasons to not consider this approach?
The GPL requires all the code of a work is source. All of it, no shims no magic glue. EXPORT_SYMBOL isn't an indication you can use it for binary modules. The GPL licence is quite clear on what is covered.
Agreed. Such patch won't change anything. The discussions, patch reviews, etc were under the assumption that the code will be GPL'd, plus the subsystems that are exposed by this interface also assumes that. Any trials to circumvent it seem to violate Kernel owner's rights.
Since you've asked this I'm advised by my lawyer to respond to all such assumptions of legality of binary modules...
For a Linux kernel containing any code I own the code is under the GNU public license v2 (in some cases or later), I have never given permission for that code to be used as part of a combined or derivative work which contains binary chunks. I have never said that modules are somehow magically outside the GPL and I am doubtful that in most cases a work containing binary modules for a Linux kernel is compatible with the licensing, although I accept there may be some cases that it is.
Alan
I second Alan: For a Linux kernel containing any code I own the code is under the GNU public license v2 (in a few cases, where explicitly said GPLv2 or later or dual GNU/BSD), I have never given permission for that code to be used as part of a combined or derivative work which contains binary chunks.
Regards, Mauro.
Em 25-01-2012 11:46, Mauro Carvalho Chehab escreveu:
Em 25-01-2012 10:30, Alan Cox escreveu:
Technically speaking, is there no way that the EXPORT_SYMBOL_GPLed symbols can be used by the binary blobs, possibly with an open-sourced shim which provides the buffer-sharing interface to the binary blobs? Are there any reasons to not consider this approach?
The GPL requires all the code of a work is source. All of it, no shims no magic glue. EXPORT_SYMBOL isn't an indication you can use it for binary modules. The GPL licence is quite clear on what is covered.
Agreed. Such patch won't change anything. The discussions, patch reviews, etc were under the assumption that the code will be GPL'd, plus the subsystems that are exposed by this interface also assumes that.
Any trials to circumvent it seem to violate Kernel owner's rights.
In time, let me rephrase it:
Any trial to circumvent it seems to be an attempt to violate Kernel owner's rights.
Since you've asked this I'm advised by my lawyer to respond to all such assumptions of legality of binary modules...
For a Linux kernel containing any code I own the code is under the GNU public license v2 (in some cases or later), I have never given permission for that code to be used as part of a combined or derivative work which contains binary chunks. I have never said that modules are somehow magically outside the GPL and I am doubtful that in most cases a work containing binary modules for a Linux kernel is compatible with the licensing, although I accept there may be some cases that it is.
Alan
I second Alan: For a Linux kernel containing any code I own the code is under the GNU public license v2 (in a few cases, where explicitly said GPLv2 or later or dual GNU/BSD), I have never given permission for that code to be used as part of a combined or derivative work which contains binary chunks.
Regards, Mauro.
On Tue, 17 Jan 2012 16:08:17 -0800 Robert Morell rmorell@nvidia.com wrote:
EXPORT_SYMBOL_GPL is intended to be used for "an internal implementation issue, and not really an interface". The dma-buf infrastructure is explicitly intended as an interface between modules/drivers, so it should use EXPORT_SYMBOL instead.
Signed-off-by: Robert Morell rmorell@nvidia.com
Please get a Signed-off-by: from the original authors. In some eyes you are making what is a licensing change, and that needs the rights holders permissions.
So this patch at the very least needs approval from TI and Linaro.
On Tue, Jan 17, 2012 at 6:08 PM, Robert Morell rmorell@nvidia.com wrote:
EXPORT_SYMBOL_GPL is intended to be used for "an internal implementation issue, and not really an interface". The dma-buf infrastructure is explicitly intended as an interface between modules/drivers, so it should use EXPORT_SYMBOL instead.
We discussed this topic at the kernel-gfx mini-summit at ELC. Following the discussion, I agree that dma-buf infrastructure is intended as an interface between driver subsystems. And because (for now) all the other arm SoC gl(es) stacks unfortunately involve a closed src userspace, and since I consider userspace and kernel as tightly coupled in the realm of graphics stacks, I don't think we can really claim the moral high-ground here. So I can't object to use of EXPORT_SYMBOL() instead of EXPORT_SYMBOL_GPL().
That said, I expect the dma-buf infrastructure to still be evolving and it is the responsibility for out-of-tree users of the API (GPL or otherwise) to adapt as the dma-buf API changes. And there isn't much the in-tree drivers can do when it comes to debugging of buffer sharing issues with out-of-tree drivers (binary or otherwise), leaving the debugging responsibility to the owners of different out-of-tree drivers.
BR, -R
Signed-off-by: Robert Morell rmorell@nvidia.com
drivers/base/dma-buf.c | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c index e38ad24..4ed5a5d 100644 --- a/drivers/base/dma-buf.c +++ b/drivers/base/dma-buf.c @@ -101,7 +101,7 @@ struct dma_buf *dma_buf_export(void *priv, struct dma_buf_ops *ops,
return dmabuf; } -EXPORT_SYMBOL_GPL(dma_buf_export); +EXPORT_SYMBOL(dma_buf_export);
/** @@ -126,7 +126,7 @@ int dma_buf_fd(struct dma_buf *dmabuf)
return fd; } -EXPORT_SYMBOL_GPL(dma_buf_fd); +EXPORT_SYMBOL(dma_buf_fd);
/** * dma_buf_get - returns the dma_buf structure related to an fd @@ -152,7 +152,7 @@ struct dma_buf *dma_buf_get(int fd)
return file->private_data; } -EXPORT_SYMBOL_GPL(dma_buf_get); +EXPORT_SYMBOL(dma_buf_get);
/** * dma_buf_put - decreases refcount of the buffer @@ -167,7 +167,7 @@ void dma_buf_put(struct dma_buf *dmabuf)
fput(dmabuf->file); } -EXPORT_SYMBOL_GPL(dma_buf_put); +EXPORT_SYMBOL(dma_buf_put);
/** * dma_buf_attach - Add the device to dma_buf's attachments list; optionally, @@ -213,7 +213,7 @@ err_attach: mutex_unlock(&dmabuf->lock); return ERR_PTR(ret); } -EXPORT_SYMBOL_GPL(dma_buf_attach); +EXPORT_SYMBOL(dma_buf_attach);
/** * dma_buf_detach - Remove the given attachment from dmabuf's attachments list; @@ -235,7 +235,7 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) mutex_unlock(&dmabuf->lock); kfree(attach); } -EXPORT_SYMBOL_GPL(dma_buf_detach); +EXPORT_SYMBOL(dma_buf_detach);
/** * dma_buf_map_attachment - Returns the scatterlist table of the attachment; @@ -265,7 +265,7 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
return sg_table; } -EXPORT_SYMBOL_GPL(dma_buf_map_attachment); +EXPORT_SYMBOL(dma_buf_map_attachment);
/** * dma_buf_unmap_attachment - unmaps and decreases usecount of the buffer;might @@ -288,4 +288,4 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach, mutex_unlock(&attach->dmabuf->lock);
} -EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
+EXPORT_SYMBOL(dma_buf_unmap_attachment);
1.7.3.4
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
in mind. It would be unfortunate if restricting its use to only GPL-licensed modules caused dma-buf adoption to be limited.
You appear confused. I'll say this again is my lawyer advises me that for the benefit of any future enforcement actions I should remind people each time it comes up.
For a Linux kernel containing any code I own the code is under the GNU public license v2 (in some cases or later), I have never given permission for that code to be used as part of a combined or derivative work which contains binary chunks. I have never said that modules are somehow magically outside the GPL and I am doubtful that in most cases a work containing binary modules for a Linux kernel is compatible with the licensing, although I accept there may be some cases that it is.
Alan
On Wed, Jan 18, 2012 at 2:08 AM, Robert Morell rmorell@nvidia.com wrote:
The DMA buffer infrastructure (dma-buf) currently exposes its interface with EXPORT_SYMBOL_GPL. The documentation for EXPORT_SYMBOL_GPL says: "It implies that the function is considered an internal implementation issue, and not really an interface." This interface is clearly not just an "implementation issue" but an interface to be used across drivers/subsystems, so I think it makes sense for it to use EXPORT_SYMBOL instead.
Work on dma-buf was originally started with the goal of unifying several competing "memory management" systems developed with different ARM SoCs in mind. It would be unfortunate if restricting its use to only GPL-licensed modules caused dma-buf adoption to be limited.
For convenience, I'll send the trivial patch to implement this change. I'd like to see this in the first release with dma-buf in 3.3.
How about you open up your driver code instead? You'd get access to all EXPORT_SYMBOL_GPL symbols instantly!
Pekka
dri-devel@lists.freedesktop.org
-
Alan Cox -
Arnd Bergmann -
Daniel Vetter -
Dave Airlie -
Dave Airlie -
Ilija Hadzic -
Konrad Rzeszutek Wilk -
Linus Torvalds -
Mauro Carvalho Chehab -
Pekka Enberg -
Rob Clark -
Robert Morell -
Semwal, Sumit -
Sumit Semwal