Re: [PATCH v2 05/14] arm: common: edma: Select event queue 1 as default when booted with DT

[Sekhar Nori]

On Friday 11 April 2014 03:12 PM, Vinod Koul wrote:
> On Fri, Apr 11, 2014 at 12:38:00PM +0300, Peter Ujfalusi wrote:
>> On 04/11/2014 11:56 AM, Sekhar Nori wrote:
>>> On Friday 11 April 2014 02:20 PM, Peter Ujfalusi wrote:
>>>> On 04/11/2014 11:17 AM, Sekhar Nori wrote:
>>>>> On Tuesday 01 April 2014 06:36 PM, Peter Ujfalusi wrote:
>>>>>> Use the EVENTQ_1 for default and leave the EVENTQ_0 to be used by high
>>>>>> priority channels, like audio.
>>>>>>
>>>>>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@xxxxxx>
>>>>>
>>>>> Acked-by: Sekhar Nori <nsekhar@xxxxxx>
>>>>>
>>>>>> ---
>>>>>>  arch/arm/common/edma.c | 3 ++-
>>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
>>>>>> index 86a8b263278f..19520e2519d9 100644
>>>>>> --- a/arch/arm/common/edma.c
>>>>>> +++ b/arch/arm/common/edma.c
>>>>>> @@ -1546,7 +1546,8 @@ static int edma_of_parse_dt(struct device *dev,
>>>>>>  
>>>>>>  	pdata->queue_priority_mapping = queue_priority_map;
>>>>>>  
>>>>>> -	pdata->default_queue = 0;
>>>>>> +	/* select queue 1 as default */
>>>>>
>>>>> It will be nice to expand the comment with explanation of why this is
>>>>> being chosen as default (lower priority queue by default for typical
>>>>> bulk data transfer).
>>>>
>>>> Yes, extended comment is a good idea.
>>>>
>>>> For the next version I think I'm going to change the code around default
>>>> TC/Queue and the non default queue selection, mostly based on Joel's comment:
>>>>
>>>> EVENTQ_1 as default queue.
>>>> Set the EVENTQ_1 priority to 7
>>>> EVENTQ_0 priority is going to stay 0 and EVENTQ_2 as 2
>>>>
>>>> Add new member to struct edma, like high_pri_queue.
>>>> When we set the queue priorities in edma_probe() we look for the highest
>>>> priority queue and save the number in high_pri_queue.
>>>>
>>>> I will rename the edma_request_non_default_queue() to
>>>> edma_request_high_pri_queue() and it will assign the channel to the high
>>>> priority queue.
>>>>
>>>> I think this way it is going to be more explicit and IMHO a bit more safer in
>>>> a sense the we are going to get high priority when we ask for it.
>>>
>>> Sounds much better. I had posted some ideas about adding support for
>>> channel priority in the core code but we can leave that for Vinod and
>>> Dan to say if they really see a need for that.
> Is it part of this series?

No, the current series has an EDMA specific way of managing priority.

> 
>> If we do it via the dmaengine core I think it would be better to have a new
>> flag to be passed to dmaengine_prep_dma_*().
>> We could have for example:
>> DMA_PREP_HIGH_PRI as flag to indicate that we need high priority DMA if it is
>> possible.
>> We can watch for this flag in the edma driver and act accordingly.
>> ALSA's dmaengine_pcm_prepare_and_submit() could set this flag unconditionally
>> since audio should be treated in this way if the DMA IP can do this.
> Will the priority be different for each descriptor or would be based on channel
> usage. If not then we can add this in dma_slave_config ?

The priority will be per-channel not per-transaction (at least for the
use case we are talking about here).

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/