On 1/10/2022 1:27 PM, Sanjay R Mehta wrote:
>
>
> On 1/3/2022 5:04 PM, Vinod Koul wrote:
>> On 17-12-21, 03:58, Sanjay R Mehta wrote:
>>> From: Sanjay R Mehta <sanju.mehta@xxxxxxx>
>>>
>>> The command should be submitted only if the engine is idle,
>>> for this, the next available descriptor is checked and set the flag
>>> to false in case the descriptor is non-empty.
>>>
>>> Also need to segregate the cases when DMA is complete or not.
>>> In case if DMA is already complete there is no need to handle it
>>> again and gracefully exit from the function.
>>>
>>> Signed-off-by: Sanjay R Mehta <sanju.mehta@xxxxxxx>
>>> ---
>>> drivers/dma/ptdma/ptdma-dmaengine.c | 24 +++++++++++++++++-------
>>> 1 file changed, 17 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/dma/ptdma/ptdma-dmaengine.c b/drivers/dma/ptdma/ptdma-dmaengine.c
>>> index c9e52f6..91b93e8 100644
>>> --- a/drivers/dma/ptdma/ptdma-dmaengine.c
>>> +++ b/drivers/dma/ptdma/ptdma-dmaengine.c
>>> @@ -100,12 +100,17 @@ static struct pt_dma_desc *pt_handle_active_desc(struct pt_dma_chan *chan,
>>> spin_lock_irqsave(&chan->vc.lock, flags);
>>>
>>> if (desc) {
>>> - if (desc->status != DMA_ERROR)
>>> - desc->status = DMA_COMPLETE;
>>> -
>>> - dma_cookie_complete(tx_desc);
>>> - dma_descriptor_unmap(tx_desc);
>>> - list_del(&desc->vd.node);
>>> + if (desc->status != DMA_COMPLETE) {
>>> + if (desc->status != DMA_ERROR)
>>> + desc->status = DMA_COMPLETE;
>>> +
>>> + dma_cookie_complete(tx_desc);
>>> + dma_descriptor_unmap(tx_desc);
>>> + list_del(&desc->vd.node);
>>> + } else {
>>> + /* Don't handle it twice */
>>> + tx_desc = NULL;
>>> + }
>>> }
>>>
>>> desc = pt_next_dma_desc(chan);
>>> @@ -233,9 +238,14 @@ static void pt_issue_pending(struct dma_chan *dma_chan)
>>> struct pt_dma_chan *chan = to_pt_chan(dma_chan);
>>> struct pt_dma_desc *desc;
>>> unsigned long flags;
>>> + bool engine_is_idle = true;
>>>
>>> spin_lock_irqsave(&chan->vc.lock, flags);
>>>
>>> + desc = pt_next_dma_desc(chan);
>>> + if (desc)
>>> + engine_is_idle = false;
>>> +
>>> vchan_issue_pending(&chan->vc);
>>>
>>> desc = pt_next_dma_desc(chan);
>>> @@ -243,7 +253,7 @@ static void pt_issue_pending(struct dma_chan *dma_chan)
>>> spin_unlock_irqrestore(&chan->vc.lock, flags);
>>>
>>> /* If there was nothing active, start processing */
>>> - if (desc)
>>> + if (engine_is_idle)
>>
>> Can you explain why do you need this flag and why desc is not
>> sufficient..
>
> Here it is required to know if the engine was idle or not before
> submitting new desc to the active list (i.e, before calling
> "vchan_issue_pending()" API). So that if there was nothing active then
> start processing this desc otherwise later.
>
> Here desc is submitted to the engine after vchan_issue_pending() API
> called which will actually put the desc into the active list and then if
> I get the next desc, the condition will always be true. Therefore used
> this flag here to solve this issue.
>
>>
>> It also sounds like 2 patches to me...
>
> Once the desc is submitted to the engine that will be handled by
> pt_handle_active_desc() function. This issue was resolved by making
> these changes together. Hence kept into the single patch.
>
> Please suggest to me, if this still needs to be split. I'll make the
> changes accordingly.
>
Hi Vinod,
Any further comments for this patch? Need your help to get this upstreamed.
> - Sanjay
>
>>
>>> pt_cmd_callback(desc, 0);
>>> }
>>>
>>> --
>>> 2.7.4
>>
