[Sound-open-firmware] [PATCH 2/2] cnl: dma: refine dma interrupt processing on cnl

[rander.wang]

Hi Liam,

On 3/8/2018 6:59 PM, Liam Girdwood wrote:
> On Thu, 2018-03-08 at 17:47 +0800, rander.wang wrote:
>> Hi Liam,
>>
>> On 3/8/2018 4:11 PM, Liam Girdwood wrote:
>>> On Thu, 2018-03-08 at 14:39 +0800, rander.wang wrote:
>>>> hi Liam,
>>>>
>>> Please don't top post.
>>>
>>>>        you mentioned :  using IRQ core as 1st level IRQ handler
>>>>
>>>> i don't get your idea.  to check which dma triggers the interrupt,
>>>> dma
>>>> status register
>>>>
>>>> would be queried. Do this in 1st level IRQ handler is very ugly.
>>>>
>>>>
>>> 1st level handler does not check the DMAC. It only masks the 1st level
>>> IRQ and then calls all the 2nd level handlers. Each second level
>>> handler then checks if the IRQ is for itself and returns immediately if
>>>    its not the IRQ owner.
>>       i have discuss with Keyon. our idea is: maintain a list of
>> interrupt function and private data
>>
>>      on second level data structure. In second level interrupt handler,
>> call the irq functions
>>
>>      for each list member.
>>
>>           can you give some advice?
>>
> Btw, the levels I'm referring to here are parent/child level and not priority
> level (so it's also worth sending an earlier patch that does some renaming i.e.
> parent_level3_handler -> level2_priority3_handler and IRQ_NUM_EXT_LEVEL2 ->
> IRQ_NUM_EXT_PRI2) as this should clear up levels and priorities.
> .
>
> 1) 1st level will be DSP interrupt - we then call second level handler..
>
> 2) 2nd level will be IRQ controller level (i.e handler will determine which
> child HW IP has the IRQ, DMAC, IPC, clock, SSP etc and call the 3rd level child
> handler).
>
> 3) 3rd level handler will iterate through it's list of client handlers. In this
> case it calls the handler for each DMAC.
>
> This means we need the following :-
>
> 4) generic level3 handler code needs to be added to
> src/platform/cannonlake/interrupt.c. It will be similar to level2 handler in
> that it will loop through an array of clients.
>
> 5) platform_irq_get_parent() will need modified to support 3rd level handlers.
> It will return 3rd level handler for DW DMAC clients.
>
> Does this make sense ?

   i have some questions:

     (1) in interrupt register function:

          parent->child[REEF_IRQ_BIT(irq)]->handler = handler

         parent->child[REEF_IRQ_BIT(irq)]->handler_arg = arg

       for two DMA share the same irq bit, only one arg of dma can be 
recorded.

       the issue also exists in interrupt unregister, disable, enable


   (2) in interrupt function each child should check whether parent is 
register to system.

        if not, register handler to system. The code only works for two 
level architecture,

        not three level, and also for unregister.


     I discuss with Keyon, to make three level interrupt algorithm 
work,  the core code of

    interrupt need to be refined. Anther method is to modify the 
irq_child, add a child list in it.

   For linux kernel, interrupts are also recorded by a list, and check 
the list when interrupted

    Or, to only make the modification in Cannonlake platform for irq 
difference,  DMA irq

    can be checked in parent_level5_handler, call the dma irq function 
two times with different

   DMA data.


    this is my idea, can you give some advice?

Rander



> Liam