On Tue, May 12, 2009 at 8:38 AM, Hiroshi DOYU <[email protected]> wrote:
> From: "ext Kanigeri, Hari" <[email protected]>
> Subject: RE: [PATCH 2/2] DSPBRIDGE: add checking 128 byte alignment for dsp
> cache line size
> Date: Mon, 11 May 2009 20:26:11 +0200
>
>> To summarize the discussion.
>
> Thanks
>
>> We need to have the check for 128 bytes alignment (upper and
>> lower). The 2 places that this can be done are in flush function or
>> in Map function.
>>
>> - I prefer the check is done in Map function as Felipe
>> mentioned this function is bound to be called by MM components as
>> opposed to Flush function.
>
> Mapping is totally another thing from this problem.
>
> Any page mapping is being done, forcing PAGE_SIZE as below, because
> PAGE_SIZE(4KB) is the mininum unit from (io)mmu H/W perspective. This
> is same as ARM too. They've been aligned on 4KB already. So no need to
> worry about 128-byte alignement for mapping.
It's the same address, you cannot flush an address that has not been
mapped. Look at the dsp-dummy code[1].
DSPProcessor_ReserveMemory(b->node, to_reserve, &b->reserve);
DSPProcessor_Map(b->node, b->data, b->size, b->reserve, &b->map, 0);
DSPProcessor_FlushMemory(b->node, b->data, b->size, 0);
See? b->data is used for *both* Map and FlushMemory. The only
difference is that you can skip FlushMemory, or flush half of the
memory area:
DSPProcessor_FlushMemory(b->node, b->data + (b->size / 2), b->size, 0);
The only operation you cannot avoid is Map.
> DSP_STATUS PROC_Map(DSP_HPROCESSOR hProcessor, void *pMpuAddr, u32 ulSize,
> void *pReqAddr, void **ppMapAddr, u32 ulMapAttr)
> {
> .....
> /* Calculate the page-aligned PA, VA and size */
> vaAlign = PG_ALIGN_LOW((u32) pReqAddr, PG_SIZE_4K);
> paAlign = PG_ALIGN_LOW((u32) pMpuAddr, PG_SIZE_4K);
> sizeAlign = PG_ALIGN_HIGH(ulSize + (u32)pMpuAddr - paAlign,
> PG_SIZE_4K);
>
> The points which we should take care of are bridge cache
> operations("PROC_Flush/Invalidate..()") and we should/can handle this
> problem independently. It would cause another dependency and increase
> another complexity again if we have to assume something(ex:
> "read-only" or "write-only" buffer) on other modules or expecting
> something on its "protocol".
If we are not going to have a read-only/write-only flag then I'm
against adding the alignment check. It will only force user-space to
do memory copies unnecessarily. That would kill performance
drastically. NAK!
Even mmap has PROT_READ and PROT_WRITE, is that unnecessary complexity?
[1]
http://github.com/felipec/dsp-dummy/blob/9806fd01c3bf8129d27a673086d286011565fc2c/dmm_buffer.h
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
