On Fri, May 14, 2010 at 10:27 PM, Felipe Contreras
<felipe.contreras@xxxxxxxxx> wrote:
> I spent some time looking deeper into this patch series, and I have some doubts.
>
> On Sun, May 2, 2010 at 8:47 PM, Ohad Ben-Cohen <ohad@xxxxxxxxxx> wrote:
>> Basically you're right, but the patches currently silently allow
>> today's user space
>> to somehow work (because if you don't use dma bounce buffers and you feel lucky
>> about speculative prefetching then you might get away with not calling
>> dma unmap).
>> I did that on purpose, to ease testing & migration, but didn't
>> explicitely said it because
>> frankly it's just wrong.
>
> I looked into the dma code (I guess it's in arch/arm/mm/dma-mapping.c)
> and I don't understand what dma_unmap_sg is supposed to do. I see that
> first some "safe buffer" is searched, and if there isn't any... then
> it depends on the direction whether something is actually done or not.
>
> I guess it depends whether our arch has dmabounce or not, which I have
> no idea, but if we do, wouldn't skiping dma_unmap calls leak some
> massive amount of "safe buffers"?
Now I understand better; arch/arm/mm/dma-mapping.c will not be used
unless CONFIG_DMABOUNCE=y, which is not the case for OMAP3.
So, as you can see in arch/arm/include/asm/dma-mapping.h, dma_unmap_sg
is a noop.
static inline void dma_unmap_single(struct device *dev, dma_addr_t handle,
size_t size, enum dma_data_direction dir)
{
/* nothing to do */
}
So, in the end, PROC_BEGINDMATODSP and PROC_BEGINDMAFROMDSP would do
exactly the same as PROC_FLUSHMEMORY and PROC_INVALIDATEMEMORY
(dmac_op_range/outer_io_range). And
PROC_ENDDMATODSP/PROC_ENDDMAFROMDSP don't do anything. Therefore even
if user-space updates to the new API, there's no change.
I don't think it makes sense to push for this new API since there will
be absolutely no gain.
>> What do you say about the following plan then:
>> - I'll add a single pair of begin_dma and end_dma commands that will
>> take the additional
>> direction parameter as described above. I'll then covert the existing
>> flush & invalidate commands to use this begin_dma command supplying it
>> the appropriate direction argument.
>
> Sounds perfect, but I wonder if the deprecated flush & invalidate
> would really work. See previous comments.
Actually it would work. I like this approach because it doesn't break
ABI, and doesn't change the semantics unless the new ioctls are used.
>> - In a separate patch, I'll deprecate flush & invalidate entirely
>> (usage of this deprecated
>> API will fail, resulting in a guiding error message).
I don't think there's any need for deprecation.
>> We get a sane and versatile (and platform-independent) implementation
>> that always work,
>> but breaks user space. If anyone would need to work with current user space,
>> the deprecating patch can always be reverted locally to get back a
>> flush & invalidate
>> implementations that (somehow) work.
I still would like the new API for the reason I mentioned before: so
that user-space can clean/invalidate/flush.
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
