Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support
Hi Sumit, On 01/25/2012 06:35 AM, Semwal, Sumit wrote: Hi Tomasz, On Mon, Jan 23, 2012 at 8:55 PM, Tomasz Stanislawski t.stanisl...@samsung.com wrote: Hi Mauro, snip Ok. I should have given more details about the patch. I am sorry for missing it. My kernel tree failed to compile after applying patches from [1] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/42966/focus=42968 I had to generate this patch to compile the code and test it. Most of the fixes refer to Sumit's code and I think he will take care of those bugs. Is your kernel tree a mainline kernel? I am pretty sure I posted out the RFC after compile testing. Our development kernel often contains patches that are not posted to opensource. The tree presented in the cover letter contains only patches that were approved for opensource submission. Some of the patches that are not merged into the mainline may break compilation if patches from the mailing list are applied on the top. The example is 'media: vb2: remove plane argument from call_memop and cleanup mempriv usage'. I had to add fixes to compile the code. Moreover I had to test a working application that makes use of DMABUF exporting/importing via V4L2 API. So I had to fix other issues that are not only compilation related. As I remember we agreed that I had to post an incremental patchset. Therefore all needed fixes had to be present in the tree. The fixes were posted in this patchset to keep the whole work together. I expect that you already prepared a patch fixing majority of issues from this patch. Many of them were mentioned in Pawel's and Laurent's and Sakari's reviews. If you find fixes in this patch useful you can merge them into next version of RFC 'v4l: DMA buffer sharing support as a user'. snip I wanted to post the complete set of patches that produce compilable kernel. Therefore most important bugs/issues had to be fixed and attached to the patchset. Some of the issues in [1] were mentioned by Laurent and Sakari. I hope Sumit will take care of those problems. I must've misunderstood when you said 'I would like to take care of these patches'. Please let me know if you'd like me to submit next version of my RFC separately with fixes for these issues, or would you manage that as part of your RFC patch series submission. This patchset is an RFC. It was my big mistake that I forgot to add this to the title of the patchset. I am not going to post the patch with fixes to your part any more. It would be great if you merged it into new version of 'DMA buffer sharing support as a user'. IMO, some parts should go as separate threads: - extension to DMA subsystem, introduction of dma_get_pages. This would probably go to DMA mailing list. - redesign of dma-contig allocator (w/o dmabuf exporting/importing) - buffer importing via dmabuf in V4L2 and vb2-dma-contig - buffer exporting via dmabuf in V4L2 and vb2-dma-contig BTW. Could you state your opinion on presented solution for dma-buf exporting in vb2-core and vb2-dma-contig allocator? Regards, Tomasz Stanislawski Failing to do that will mean that important fixes for upstream will be missed. Ok. It will be fixed. Regards, Mauro Regards, Tomasz Stanislawski Best regards, ~Sumit. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support
Hi Tomasz, On Wed, Jan 25, 2012 at 4:04 PM, Tomasz Stanislawski t.stanisl...@samsung.com wrote: Hi Sumit, On 01/25/2012 06:35 AM, Semwal, Sumit wrote: Hi Tomasz, On Mon, Jan 23, 2012 at 8:55 PM, Tomasz Stanislawski t.stanisl...@samsung.com wrote: Hi Mauro, snip Ok. I should have given more details about the patch. I am sorry for missing it. My kernel tree failed to compile after applying patches from [1] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/42966/focus=42968 I had to generate this patch to compile the code and test it. Most of the fixes refer to Sumit's code and I think he will take care of those bugs. Is your kernel tree a mainline kernel? I am pretty sure I posted out the RFC after compile testing. Our development kernel often contains patches that are not posted to opensource. The tree presented in the cover letter contains only patches that were approved for opensource submission. Right; I understand that - I just wanted to make sure you didn't hit some problem with my patches that I didn't. Thanks for confirming that it was your dev kernel. Some of the patches that are not merged into the mainline may break compilation if patches from the mailing list are applied on the top. The example is 'media: vb2: remove plane argument from call_memop and cleanup mempriv usage'. I had to add fixes to compile the code. Moreover I had to test a working application that makes use of DMABUF exporting/importing via V4L2 API. So I had to fix other issues that are not only compilation related. I understand. As I remember we agreed that I had to post an incremental patchset. Therefore all needed fixes had to be present in the tree. The fixes were posted in this patchset to keep the whole work together. I expect that you already prepared a patch fixing majority of issues from this patch. Many of them were mentioned in Pawel's and Laurent's and Sakari's reviews. If you find fixes in this patch useful you can merge them into next version of RFC 'v4l: DMA buffer sharing support as a user'. OK - this makes it quite clear; I will re-work my RFC then. snip I wanted to post the complete set of patches that produce compilable kernel. Therefore most important bugs/issues had to be fixed and attached to the patchset. Some of the issues in [1] were mentioned by Laurent and Sakari. I hope Sumit will take care of those problems. I must've misunderstood when you said 'I would like to take care of these patches'. Please let me know if you'd like me to submit next version of my RFC separately with fixes for these issues, or would you manage that as part of your RFC patch series submission. This patchset is an RFC. It was my big mistake that I forgot to add this to the title of the patchset. I am not going to post the patch with fixes to your part any more. It would be great if you merged it into new version of 'DMA buffer sharing support as a user'. That's OK with me. IMO, some parts should go as separate threads: - extension to DMA subsystem, introduction of dma_get_pages. This would probably go to DMA mailing list. - redesign of dma-contig allocator (w/o dmabuf exporting/importing) - buffer importing via dmabuf in V4L2 and vb2-dma-contig - buffer exporting via dmabuf in V4L2 and vb2-dma-contig BTW. Could you state your opinion on presented solution for dma-buf exporting in vb2-core and vb2-dma-contig allocator? I agree with your ordering of these parts; Also, with this ordering, I guess I should pay more attention to parts 1. (extension to DMA...) and 2. (redesign of dma-contig allocator...) - which I hope you are going to do? I can then base out my next version of RFC on these. I was OoO for past couple of days, hence missed all the action :) - I will try and go through your approach, and comment as soon as I can. Hopefully in a couple of days. Regards, Tomasz Stanislawski snip Best regards, ~Sumit. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support
Hi Tomasz, On Mon, Jan 23, 2012 at 8:55 PM, Tomasz Stanislawski t.stanisl...@samsung.com wrote: Hi Mauro, snip Ok. I should have given more details about the patch. I am sorry for missing it. My kernel tree failed to compile after applying patches from [1] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/42966/focus=42968 I had to generate this patch to compile the code and test it. Most of the fixes refer to Sumit's code and I think he will take care of those bugs. Is your kernel tree a mainline kernel? I am pretty sure I posted out the RFC after compile testing. snip I wanted to post the complete set of patches that produce compilable kernel. Therefore most important bugs/issues had to be fixed and attached to the patchset. Some of the issues in [1] were mentioned by Laurent and Sakari. I hope Sumit will take care of those problems. I must've misunderstood when you said 'I would like to take care of these patches'. Please let me know if you'd like me to submit next version of my RFC separately with fixes for these issues, or would you manage that as part of your RFC patch series submission. Failing to do that will mean that important fixes for upstream will be missed. Ok. It will be fixed. Regards, Mauro Regards, Tomasz Stanislawski Best regards, ~Sumit. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support
Em 23-01-2012 11:51, Tomasz Stanislawski escreveu: Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com Please better describe this patch. What is it supposing to fix? --- drivers/media/video/videobuf2-core.c | 21 + include/media/videobuf2-core.h |6 +++--- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index cb85874..59bb1bc 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -119,7 +119,7 @@ static void __vb2_buf_dmabuf_put(struct vb2_buffer *vb) void *mem_priv = vb-planes[plane].mem_priv; if (mem_priv) { - call_memop(q, plane, detach_dmabuf, mem_priv); + call_memop(q, detach_dmabuf, mem_priv); Huh? You're not removing the plane parameter on this patch, but, instead, on a previous patch. No patch is allowed to break compilation, as it breaks git bisect. dma_buf_put(vb-planes[plane].dbuf); vb-planes[plane].dbuf = NULL; vb-planes[plane].mem_priv = NULL; @@ -907,6 +907,8 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b, if (b-memory == V4L2_MEMORY_DMABUF) { for (plane = 0; plane vb-num_planes; ++plane) { v4l2_planes[plane].m.fd = b-m.planes[plane].m.fd; + v4l2_planes[plane].length = + b-m.planes[plane].length; } } } else { @@ -1055,15 +1057,10 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b) if (IS_ERR_OR_NULL(dbuf)) { dprintk(1, qbuf: invalid dmabuf fd for plane %d\n, plane); - ret = PTR_ERR(dbuf); + ret = -EINVAL; goto err; } - /* this doesn't get filled in until __fill_vb2_buffer(), - * since it isn't known until after dma_buf_get().. - */ - planes[plane].length = dbuf-size; - /* Skip the plane if already verified */ if (dbuf == vb-planes[plane].dbuf) { dma_buf_put(dbuf); @@ -1075,7 +1072,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b) /* Release previously acquired memory if present */ if (vb-planes[plane].mem_priv) { - call_memop(q, plane, detach_dmabuf, + call_memop(q, detach_dmabuf, vb-planes[plane].mem_priv); dma_buf_put(vb-planes[plane].dbuf); } @@ -1083,8 +1080,8 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b) vb-planes[plane].mem_priv = NULL; /* Acquire each plane's memory */ - mem_priv = q-mem_ops-attach_dmabuf( - q-alloc_ctx[plane], dbuf); + mem_priv = q-mem_ops-attach_dmabuf(q-alloc_ctx[plane], + dbuf, planes[plane].length, write); if (IS_ERR(mem_priv)) { dprintk(1, qbuf: failed acquiring dmabuf memory for plane %d\n, plane); @@ -1102,7 +1099,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b) */ for (plane = 0; plane vb-num_planes; ++plane) { ret = q-mem_ops-map_dmabuf( - vb-planes[plane].mem_priv, write); + vb-planes[plane].mem_priv); if (ret) { dprintk(1, qbuf: failed mapping dmabuf memory for plane %d\n, plane); @@ -1497,7 +1494,7 @@ int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking) */ if (q-memory == V4L2_MEMORY_DMABUF) for (plane = 0; plane vb-num_planes; ++plane) - call_memop(q, plane, unmap_dmabuf, + call_memop(q, unmap_dmabuf, vb-planes[plane].mem_priv); switch (vb-state) { diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index d8b8171..412c6a4 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -88,10 +88,10 @@ struct vb2_mem_ops { * in the vb2 core, and vb2_mem_ops really just need to get/put the * sglist (and make sure that the sglist fits it's needs..) */ - void*(*attach_dmabuf)(void *alloc_ctx, - struct dma_buf *dbuf); + void
Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support
Hi Mauro, On 01/23/2012 03:22 PM, Mauro Carvalho Chehab wrote: Em 23-01-2012 11:51, Tomasz Stanislawski escreveu: Signed-off-by: Tomasz Stanislawskit.stanisl...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com Please better describe this patch. What is it supposing to fix? Usually compilation error or bugs discovered in previous vb2-dma-contig patches adding support for dma-buf. --- drivers/media/video/videobuf2-core.c | 21 + include/media/videobuf2-core.h |6 +++--- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index cb85874..59bb1bc 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -119,7 +119,7 @@ static void __vb2_buf_dmabuf_put(struct vb2_buffer *vb) void *mem_priv = vb-planes[plane].mem_priv; if (mem_priv) { - call_memop(q, plane, detach_dmabuf, mem_priv); + call_memop(q, detach_dmabuf, mem_priv); Huh? You're not removing the plane parameter on this patch, but, instead, on a previous patch. No patch is allowed to break compilation, as it breaks git bisect. I agree that patches should not break compilation if patches are accepted to the mainline. There is a big compilation failure at patch 07 where videobuf2-dma-contig.c disappears. Note that these are proof-of-concept patches for support of dma-buf exporting/importing dma-buf in V4L2. It would be a waste of time polished the patches if they are going to be rejected due to design flaws. Regards, Tomasz Stanislawski -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support
Em 23-01-2012 12:32, Tomasz Stanislawski escreveu: Hi Mauro, On 01/23/2012 03:22 PM, Mauro Carvalho Chehab wrote: Em 23-01-2012 11:51, Tomasz Stanislawski escreveu: Signed-off-by: Tomasz Stanislawskit.stanisl...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com Please better describe this patch. What is it supposing to fix? Usually compilation error or bugs discovered in previous vb2-dma-contig patches adding support for dma-buf. --- drivers/media/video/videobuf2-core.c | 21 + include/media/videobuf2-core.h |6 +++--- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index cb85874..59bb1bc 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -119,7 +119,7 @@ static void __vb2_buf_dmabuf_put(struct vb2_buffer *vb) void *mem_priv = vb-planes[plane].mem_priv; if (mem_priv) { -call_memop(q, plane, detach_dmabuf, mem_priv); +call_memop(q, detach_dmabuf, mem_priv); Huh? You're not removing the plane parameter on this patch, but, instead, on a previous patch. No patch is allowed to break compilation, as it breaks git bisect. I agree that patches should not break compilation if patches are accepted to the mainline. There is a big compilation failure at patch 07 where videobuf2-dma-contig.c disappears. Note that these are proof-of-concept patches for support of dma-buf exporting/importing dma-buf in V4L2. It would be a waste of time polished the patches if they are going to be rejected due to design flaws. It is a waste of time for the reviewers to see a patch like this one, as: - No description of what is inside the patch is provided; - changes that should be happening inside the other patches are mixed here. It is also a waste of your time to submit a patch that will need to be later polished, as you'll need to work with the same thing twice. So, please fix your patch workflow. As a general rule, you should compile every patch you're applying and fix the breakages on them. Also, if you found bugs that need to be fixed and that aren't directly related to your current task, those should generate their own patches, and submitted in separate, in order to be applied sooner upstream and to stable. Failing to do that will mean that important fixes for upstream will be missed. Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support
Hi Mauro, On 01/23/2012 03:52 PM, Mauro Carvalho Chehab wrote: Em 23-01-2012 12:32, Tomasz Stanislawski escreveu: Hi Mauro, On 01/23/2012 03:22 PM, Mauro Carvalho Chehab wrote: Em 23-01-2012 11:51, Tomasz Stanislawski escreveu: Signed-off-by: Tomasz Stanislawskit.stanisl...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com Please better describe this patch. What is it supposing to fix? Usually compilation error or bugs discovered in previous vb2-dma-contig patches adding support for dma-buf. --- drivers/media/video/videobuf2-core.c | 21 + include/media/videobuf2-core.h |6 +++--- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index cb85874..59bb1bc 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -119,7 +119,7 @@ static void __vb2_buf_dmabuf_put(struct vb2_buffer *vb) void *mem_priv = vb-planes[plane].mem_priv; if (mem_priv) { -call_memop(q, plane, detach_dmabuf, mem_priv); +call_memop(q, detach_dmabuf, mem_priv); Huh? You're not removing the plane parameter on this patch, but, instead, on a previous patch. No patch is allowed to break compilation, as it breaks git bisect. I agree that patches should not break compilation if patches are accepted to the mainline. There is a big compilation failure at patch 07 where videobuf2-dma-contig.c disappears. Note that these are proof-of-concept patches for support of dma-buf exporting/importing dma-buf in V4L2. It would be a waste of time polished the patches if they are going to be rejected due to design flaws. It is a waste of time for the reviewers to see a patch like this one, as: - No description of what is inside the patch is provided; Ok. I should have given more details about the patch. I am sorry for missing it. My kernel tree failed to compile after applying patches from [1] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/42966/focus=42968 I had to generate this patch to compile the code and test it. Most of the fixes refer to Sumit's code and I think he will take care of those bugs. I admit that I focused on other patches. Like DMA extension, exporting in vb2-core and vb2-dma-contig. Sorry for putting so little attention to bugfixing patch. - changes that should be happening inside the other patches are mixed here. Right. I missed call_memop here. It is also a waste of your time to submit a patch that will need to be later polished, as you'll need to work with the same thing twice. The problem is that those patches were not intended to be accepted. The were intended for discussion. The other problem is that there are many people waiting for those patches. The dma-buf was already accepted to the mainline. Me and Sumit are trying to help V4L2 to catch-up. The dma-buf and its support in vb2-core seams to change very dynamically. Polishing the patch takes much time. If the dma-buf API changes the design of vb2-core may have to be changed. Therefore time spent on polishing would be lost. I am sorry for patch flaws. All of them would be fixed when the design is stabilized. So, please fix your patch workflow. As a general rule, you should compile every patch you're applying and fix the breakages on them. Also, if you found bugs that need to be fixed and that aren't directly related to your current task, those should generate their own patches, and submitted in separate, in order to be applied sooner upstream and to stable. I wanted to post the complete set of patches that produce compilable kernel. Therefore most important bugs/issues had to be fixed and attached to the patchset. Some of the issues in [1] were mentioned by Laurent and Sakari. I hope Sumit will take care of those problems. Failing to do that will mean that important fixes for upstream will be missed. Ok. It will be fixed. Regards, Mauro Regards, Tomasz Stanislawski -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support
Em 23-01-2012 13:25, Tomasz Stanislawski escreveu: Hi Mauro, On 01/23/2012 03:52 PM, Mauro Carvalho Chehab wrote: Em 23-01-2012 12:32, Tomasz Stanislawski escreveu: Hi Mauro, On 01/23/2012 03:22 PM, Mauro Carvalho Chehab wrote: Em 23-01-2012 11:51, Tomasz Stanislawski escreveu: Signed-off-by: Tomasz Stanislawskit.stanisl...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com Please better describe this patch. What is it supposing to fix? Usually compilation error or bugs discovered in previous vb2-dma-contig patches adding support for dma-buf. --- drivers/media/video/videobuf2-core.c | 21 + include/media/videobuf2-core.h |6 +++--- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index cb85874..59bb1bc 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -119,7 +119,7 @@ static void __vb2_buf_dmabuf_put(struct vb2_buffer *vb) void *mem_priv = vb-planes[plane].mem_priv; if (mem_priv) { -call_memop(q, plane, detach_dmabuf, mem_priv); +call_memop(q, detach_dmabuf, mem_priv); Huh? You're not removing the plane parameter on this patch, but, instead, on a previous patch. No patch is allowed to break compilation, as it breaks git bisect. I agree that patches should not break compilation if patches are accepted to the mainline. There is a big compilation failure at patch 07 where videobuf2-dma-contig.c disappears. Note that these are proof-of-concept patches for support of dma-buf exporting/importing dma-buf in V4L2. It would be a waste of time polished the patches if they are going to be rejected due to design flaws. It is a waste of time for the reviewers to see a patch like this one, as: - No description of what is inside the patch is provided; Ok. I should have given more details about the patch. I am sorry for missing it. My kernel tree failed to compile after applying patches from [1] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/42966/focus=42968 I had to generate this patch to compile the code and test it. Most of the fixes refer to Sumit's code and I think he will take care of those bugs. Ok. I admit that I focused on other patches. Like DMA extension, exporting in vb2-core and vb2-dma-contig. Sorry for putting so little attention to bugfixing patch. - changes that should be happening inside the other patches are mixed here. Right. I missed call_memop here. It is also a waste of your time to submit a patch that will need to be later polished, as you'll need to work with the same thing twice. The problem is that those patches were not intended to be accepted. The were intended for discussion. The patch subjects were not marked with RFC. Please prefix the subject with something like git send-email --subject-prefix PATCH RFC When submitting such patches. The other problem is that there are many people waiting for those patches. The dma-buf was already accepted to the mainline. Me and Sumit are trying to help V4L2 to catch-up. The dma-buf and its support in vb2-core seams to change very dynamically. Polishing the patch takes much time. If the dma-buf API changes the design of vb2-core may have to be changed. Therefore time spent on polishing would be lost. Ok. I am sorry for patch flaws. All of them would be fixed when the design is stabilized. No problem. So, please fix your patch workflow. As a general rule, you should compile every patch you're applying and fix the breakages on them. Also, if you found bugs that need to be fixed and that aren't directly related to your current task, those should generate their own patches, and submitted in separate, in order to be applied sooner upstream and to stable. I wanted to post the complete set of patches that produce compilable kernel. herefore most important bugs/issues had to be fixed and attached to the patchset. Some of the issues in [1] were mentioned by Laurent and Sakari. I hope Sumit will take care of those problems. Ok. My main concern was not with the driver bits, but with: 1) if fixes are needed at the vb2 core, to ensure that they'll go upstream earlier; 2) The userspace API changes to properly support for dma buffers. If you're not ready to discuss (2), that's ok, but I'd like to follow the discussions for it with care, not only for reviewing the actual patches, but also since I want to be sure that it will address the needs for xawtv and for the Xorg v4l driver. Failing to do that will mean that important fixes for upstream will be missed. Ok. It will be fixed. Thanks! Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo
Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support
Hi Mauro, On 01/23/2012 05:06 PM, Mauro Carvalho Chehab wrote: Em 23-01-2012 13:25, Tomasz Stanislawski escreveu: Hi Mauro, On 01/23/2012 03:52 PM, Mauro Carvalho Chehab wrote: Em 23-01-2012 12:32, Tomasz Stanislawski escreveu: Hi Mauro, On 01/23/2012 03:22 PM, Mauro Carvalho Chehab wrote: Em 23-01-2012 11:51, Tomasz Stanislawski escreveu: Signed-off-by: Tomasz Stanislawskit.stanisl...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com Please better describe this patch. What is it supposing to fix? [snip] It is also a waste of your time to submit a patch that will need to be later polished, as you'll need to work with the same thing twice. The problem is that those patches were not intended to be accepted. The were intended for discussion. The patch subjects were not marked with RFC. Please prefix the subject with something like git send-email --subject-prefix PATCH RFC When submitting such patches. Right!!! Sorry, I forgot about it. Probably, if I had added those 3 letter we would have avoided this whole misunderstanding about the purpose of the patches :) The other problem is that there are many people waiting for those patches. The dma-buf was already accepted to the mainline. Me and Sumit are trying to help V4L2 to catch-up. The dma-buf and its support in vb2-core seams to change very dynamically. Polishing the patch takes much time. If the dma-buf API changes the design of vb2-core may have to be changed. Therefore time spent on polishing would be lost. Ok. I am sorry for patch flaws. All of them would be fixed when the design is stabilized. No problem. So, please fix your patch workflow. As a general rule, you should compile every patch you're applying and fix the breakages on them. Also, if you found bugs that need to be fixed and that aren't directly related to your current task, those should generate their own patches, and submitted in separate, in order to be applied sooner upstream and to stable. I wanted to post the complete set of patches that produce compilable kernel. herefore most important bugs/issues had to be fixed and attached to the patchset. Some of the issues in [1] were mentioned by Laurent and Sakari. I hope Sumit will take care of those problems. Ok. My main concern was not with the driver bits, but with: 1) if fixes are needed at the vb2 core, to ensure that they'll go upstream earlier; First, we should select patches not related to DMABUF. The fixes related to DMABUF could be postponed because they will be applied in new version for Sumit's patches. Next videobuf2-dma-contig.c file is patched. Fixes to handling of mmap and userptr would go first with all DMABUF related features removed. Lack of DMABUF related callbacks will not break backward compatibility. Next, DMABUF support is added to vb2-core. Finally, that DMABUF exporting/importing patches would be applied. 2) The userspace API changes to properly support for dma buffers. If you're not ready to discuss (2), that's ok, but I'd like to follow the discussions for it with care, not only for reviewing the actual patches, but also since I want to be sure that it will address the needs for xawtv and for the Xorg v4l driver. The support of dmabuf could be easily added to framebuffer API. I expect that it would not be difficult to add it to Xv. The selection API could be used to control scaling and composing of video stream into framebuffer or a texture for composing manager. Regards, Tomasz Stanislawski Failing to do that will mean that important fixes for upstream will be missed. Ok. It will be fixed. Thanks! Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support
Em 23-01-2012 14:37, Tomasz Stanislawski escreveu: Hi Mauro, On 01/23/2012 05:06 PM, Mauro Carvalho Chehab wrote: Em 23-01-2012 13:25, Tomasz Stanislawski escreveu: Hi Mauro, On 01/23/2012 03:52 PM, Mauro Carvalho Chehab wrote: Em 23-01-2012 12:32, Tomasz Stanislawski escreveu: Hi Mauro, On 01/23/2012 03:22 PM, Mauro Carvalho Chehab wrote: Em 23-01-2012 11:51, Tomasz Stanislawski escreveu: Signed-off-by: Tomasz Stanislawskit.stanisl...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com Please better describe this patch. What is it supposing to fix? [snip] It is also a waste of your time to submit a patch that will need to be later polished, as you'll need to work with the same thing twice. The problem is that those patches were not intended to be accepted. The were intended for discussion. The patch subjects were not marked with RFC. Please prefix the subject with something like git send-email --subject-prefix PATCH RFC When submitting such patches. Right!!! Sorry, I forgot about it. Probably, if I had added those 3 letter we would have avoided this whole misunderstanding about the purpose of the patches :) The other problem is that there are many people waiting for those patches. The dma-buf was already accepted to the mainline. Me and Sumit are trying to help V4L2 to catch-up. The dma-buf and its support in vb2-core seams to change very dynamically. Polishing the patch takes much time. If the dma-buf API changes the design of vb2-core may have to be changed. Therefore time spent on polishing would be lost. Ok. I am sorry for patch flaws. All of them would be fixed when the design is stabilized. No problem. So, please fix your patch workflow. As a general rule, you should compile every patch you're applying and fix the breakages on them. Also, if you found bugs that need to be fixed and that aren't directly related to your current task, those should generate their own patches, and submitted in separate, in order to be applied sooner upstream and to stable. I wanted to post the complete set of patches that produce compilable kernel. herefore most important bugs/issues had to be fixed and attached to the patchset. Some of the issues in [1] were mentioned by Laurent and Sakari. I hope Sumit will take care of those problems. Ok. My main concern was not with the driver bits, but with: 1) if fixes are needed at the vb2 core, to ensure that they'll go upstream earlier; First, we should select patches not related to DMABUF. The fixes related to DMABUF could be postponed because they will be applied in new version for Sumit's patches. Next videobuf2-dma-contig.c file is patched. Fixes to handling of mmap and userptr would go first with all DMABUF related features removed. Lack of DMABUF related callbacks will not break backward compatibility. Next, DMABUF support is added to vb2-core. Finally, that DMABUF exporting/importing patches would be applied. Sounds like a plan. Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html