On Sat, 28 Aug 2010 09:14:25 +0300 Marin Mitov <mitov@xxxxxxxxxxx> wrote: > On Friday, August 27, 2010 09:32:14 am FUJITA Tomonori wrote: > > On Fri, 27 Aug 2010 09:23:21 +0300 > > Marin Mitov <mitov@xxxxxxxxxxx> wrote: > > > > > On Friday, August 27, 2010 08:57:59 am FUJITA Tomonori wrote: > > > > On Fri, 27 Aug 2010 07:19:07 +0200 > > > > Uwe Kleine-K$(D+S(Bnig <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > > > > > > > > > Hey, > > > > > > > > > > On Fri, Aug 27, 2010 at 02:00:17PM +0900, FUJITA Tomonori wrote: > > > > > > On Fri, 27 Aug 2010 06:41:42 +0200 > > > > > > Uwe Kleine-K$(D+S(Bnig <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > > > > > > > On Thu, Aug 26, 2010 at 07:00:24PM +0900, FUJITA Tomonori wrote: > > > > > > > > On Thu, 26 Aug 2010 11:53:11 +0200 > > > > > > > > Uwe Kleine-K$(D+S(Bnig <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > > > > > > > > > > > > > > > > > > > We have currently a number of boards broken in the mainline. They must be > > > > > > > > > > > fixed for 2.6.36. I don't think the mentioned API will do this for us. So, > > > > > > > > > > > as I suggested earlier, we need either this or my patch series > > > > > > > > > > > > > > > > > > > > > > http://thread.gmane.org/gmane.linux.ports.sh.devel/8595 > > > > > > > > > > > > > > > > > > > > > > for 2.6.36. > > > > > > > > > > > > > > > > > > > > Why can't you revert a commit that causes the regression? > > > > > > > > > > > > > > > > > > > > The related DMA API wasn't changed in 2.6.36-rc1. The DMA API is not > > > > > > > > > > responsible for the regression. And the patchset even exnteds the > > > > > > > > > > definition of the DMA API (dma_declare_coherent_memory). Such change > > > > > > > > > > shouldn't applied after rc1. I think that DMA-API.txt says that > > > > > > > > > > dma_declare_coherent_memory() handles coherent memory for a particular > > > > > > > > > > device. It's not for the API that reserves coherent memory that can be > > > > > > > > > > used for any device for a single device. > > > > > > > > > The patch that made the problem obvious for ARM is > > > > > > > > > 309caa9cc6ff39d261264ec4ff10e29489afc8f8 aka v2.6.36-rc1~591^2~2^4~12. > > > > > > > > > So this went in before v2.6.36-rc1. One of the "architectures which > > > > > > > > > similar restrictions" is x86 BTW. > > > > > > > > > > > > > > > > > > And no, we won't revert 309caa9cc6ff39d261264ec4ff10e29489afc8f8 as it > > > > > > > > > addresses a hardware restriction. > > > > > > > > > > > > > > > > How these drivers were able to work without hitting the hardware restriction? > > > > > > > In my case the machine in question is an ARMv5, the hardware restriction > > > > > > > is on ARMv6+ only. You could argue that so the breaking patch for arm > > > > > > > should only break ARMv6, but I don't think this is sensible from a > > > > > > > maintainers POV. We need an API that works independant of the machine > > > > > > > that runs the code. > > > > > > > > > > > > Agreed. But insisting that the DMA API needs to be extended wrongly > > > > > > after rc2 to fix the regression is not sensible too. The related DMA > > > > > > API wasn't changed in 2.6.36-rc1. The API isn't responsible for the > > > > > > regression at all. > > > > > I think this isn't about "responsiblity". Someone in arm-land found > > > > > that the way dma memory allocation worked for some time doesn't work > > > > > anymore on new generation chips. As pointing out this problem was > > > > > expected to find some matches it was merged in the merge window. One > > > > > such match is the current usage of the DMA API that doesn't currently > > > > > offer a way to do it right, so it needs a patch, no? > > > > > > > > No, I don't think so. We are talking about a regression, right? > > > > > > > > On new generation chips, something often doesn't work (which have > > > > worked on old chips for some time). It's not a regresiion. I don't > > > > think that it's sensible to make large change (especially after rc1) > > > > to fix such issue. If you say that the DMA API doesn't work on new > > > > chips and proposes a patch for the next merge window, it's sensible, I > > > > suppose. > > > > > > > > Btw, the patch isn't a fix for the DMA API. It tries to extend the DMA > > > > API (and IMO in the wrong way). > > > > In addition, the patch might break the > > > > current code. > > > > > > To "break the current code" is simply not possible. Sorry to oppose. As you have written it > > > "extend the DMA API", so if you do not use the new API (and no current code is using it) > > > you cannot "break the current code". > > > > Looks like that the patch adds the new API that touches the exisitng > > code. It means the existing code could break. So the exsising API > > could break too. > > > > http://thread.gmane.org/gmane.linux.ports.sh.devel/8595 > > The above reference is not my patch. I am speaking for my patch: > > http://lkml.org/lkml/2010/8/19/200 I think that I already NACK'ed the patch. 1) drivers/media/videobuf-dma-contig.c should not use dma_alloc_coherent. We shouldn't support the proposed API. 2) I don't think that the DMA API (drivers/base/dma-mapping.c) is not for creating "cache". Generally, the kernel uses "pool" concept for something like that. IMHO, reverting the commit 309caa9cc6ff39d261264ec4ff10e29489afc8f8 temporary (or temporary disabling it for systems that had worked) is the most reasonable approach. I don't think that breaking systems that had worked is a good idea even if the patch does the right thing. I believe that we need to fix the broken solution (videobuf-dma-contig.c) before the commit. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
