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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 14 Apr 2014 20:02:33 +0530
From:	Sekhar Nori <nsekhar@...com>
To:	Peter Ujfalusi <peter.ujfalusi@...com>,
	Vinod Koul <vinod.koul@...el.com>
CC:	<dan.j.williams@...el.com>, <dmaengine@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <joelf@...com>,
	<linux-arm-kernel@...ts.infradead.org>,
	<linux-omap@...r.kernel.org>,
	<davinci-linux-open-source@...ux.davincidsp.com>,
	<mporter@...aro.org>, Mark Brown <broonie@...nel.org>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Liam Girdwood <lgirdwood@...il.com>,
	Takashi Iwai <tiwai@...e.de>
Subject: Re: [PATCH v2 05/14] arm: common: edma: Select event queue 1 as default
 when booted with DT

On Monday 14 April 2014 06:11 PM, Peter Ujfalusi wrote:
> On 04/14/2014 03:12 PM, Sekhar Nori wrote:
>> On Monday 14 April 2014 05:26 PM, Peter Ujfalusi wrote:
>>> Hi Vinod,
>>>
>>> On 04/11/2014 03:46 PM, Vinod Koul wrote:
>>>> I think the number shouldn't be viewed in absolute terms. If we decide that (lets
>>>> say) 0-7, then any controller should map 0 to lowest and 7 to highest.
>>>>
>>>> For your case you can do this and then intermediate numbers would be medium
>>>> priority. Such a system might work well...
>>>>
>>>> Also how would a client driver know which priority to use? Would it come from
>>>> DT?
>>>
>>> I think DT would be the best place.
>>
>> In the strict sense of what DT is supposed to represent, DT is not the
>> best place for this. Priority is usecase driven rather that hardware
>> description driven.
> 
> While this is true, the DMA priority - if supported by the DMA engine - is a
> HW feature as well. If it is supported by the HW it can be used to tune and
> partition the system for the anticipated load or purpose.
> 
>> So on a reasonably less loaded system, I am sure you
>> could run audio even with a lower priority DMA queue.
> 
> Yes, you can. But as soon as you have other devices using the same priority
> (with eDMA3 at least) and asks for a 'long' transfer it can ruin the audio.
> During audio playback/capture you execute a long MMC read for example can
> introduce a glitch.
> 
>> Moreover, IMHO, encoding it in DT now will make it an ABI. Without a
>> wide variety of example use cases, I think it is too early to commit to
>> an ABI.
> 
> True, but we need to start from somewhere?

Right, and based on our IRC discussion, we are not really fixing up the
priority value space. That makes me much more comfortable with the idea.

>>> Not sure if we should set the range for this either. What I was thinking is to
>>> add an optional new property to be set by the client nodes, using DMA:
>>>
>>> mcasp0: mcasp@...38000 {
>>> 	compatible = "ti,am33xx-mcasp-audio";
>>> 	...
>>> 	dmas =	<&edma 8>,
>>> 		<&edma 9>;
>>> 	dma-names = "tx", "rx";
>>> 	dma-priorities = <2>, <2>;
>>> };
>>>

>>> We could agree that lower number means lower priority, higher is - well -
>>> higher priority.

Even this does not have to be uniform across, right? The numbers could
be left to interpretation per-SoC.

>>> If the dma-priority is missing we should assume lowest priority (0).
>>> The highest priority depends on the platform. For eDMA3 in AM335x it is three
>>> level. For designware controller you might have the range 0-8 as valid.
>>>
>>> The question is how to get this information into use?
>>> We can take the priority number in the core when the dma channel is requested
>>> and add field to "struct dma_chan" in order to store it and the DMA drivers
>>> could have access to it.

How about Vinod's idea of extending dma_slave_config? Priority is
similar to rest of the runtime setup dmaengine_slave_config() is meant
to do.

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