From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755068AbbCXQYw (ORCPT ); Tue, 24 Mar 2015 12:24:52 -0400 Received: from mga14.intel.com ([192.55.52.115]:43639 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755052AbbCXQYs (ORCPT ); Tue, 24 Mar 2015 12:24:48 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.11,459,1422950400"; d="asc'?scan'208";a="471925272" Date: Tue, 24 Mar 2015 21:50:43 +0530 From: Vinod Koul To: Maxime Ripard Cc: dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org, Laurent Pinchart , Ludovic Desroches Subject: Re: [PATCH RFC 1/2] dmaengine: Introduce scheduled DMA framework Message-ID: <20150324162043.GC32683@intel.com> References: <1426966927-30833-1-git-send-email-maxime.ripard@free-electrons.com> <1426966927-30833-2-git-send-email-maxime.ripard@free-electrons.com> <20150323163843.GZ32683@intel.com> <20150323215140.GF15676@lukather> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="7LkOrbQMr4cezO2T" Content-Disposition: inline In-Reply-To: <20150323215140.GF15676@lukather> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --7LkOrbQMr4cezO2T Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Mar 23, 2015 at 02:51:40PM -0700, Maxime Ripard wrote: > > > + if (vd) { > > > + lli =3D desc->v_lli; > > > + while (true) { > > > + bytes +=3D sdma->ops->lli_size(lli); > > > + > > > + if (!sdma->ops->lli_has_next(lli)) > > > + break; > > > + > > > + lli =3D sdma->ops->lli_next(lli); > > > > wouldn't it be nicer to just use a link list here which is embedded > > in a generic struct which in turn can be embedded in a driver > > specific one, so we actually don't care what driver specific > > implementation is... >=20 > I'm not sure to get what you mean here. We will always have to care > about the driver specific implementation. by providing a callback driver can do whatever it wants whereas the framewo= rk does generic stuff >=20 > > that way we cna get rid of lli_size and lli_has_next and lli_next > > ones. Further that will allow drivers/framework to break a txn into > > multiple smaller txns internally. >=20 > I chose this accessors/iterators pattern because it actually make this > very easy to add new features on the LLI (like debugging, we just have > to add a new lli_dump) and that works. >=20 > We can also manage more easily the LLI, by allocating ourself the LLI > items, and freeing it afterwards, which is something that also is > often wrong, or at least addressed in different ways from one driver > to another (for example, the Atmel drivers have their own descriptor > allocation logic, while the others mostly rely on the dma_pools. Which > one is right, I don't know, but the right one should be shared by > everyone). >=20 > That would also be much easier to add software-emulated LLI, since you > just have to set the lli management functions to some common library > functions, and we're done. >=20 > I feel like it would be much more flexible, and would remove logic > from the driver itself, which is the point of this whole > "sub-framework". Yes the intent is right, I was thinking that using a kernel link frees us =66rom managing the list, lesser chances of bugs that way > > typically driver would do calculations for descriptor setting up the > > registers values which cna be programmed, so a driver callback would be= nice > > here. That way lesser work in start function and which is good as I am > > assuming start should be done from irq context >=20 > I'm not really sure that would make sense. You don't have the > guarantee that the dma_chan pointer you are passing to it is actually > mapped to a physical channel at this time. >=20 > And I thought that the prep functions could be called from irq context > too? Yes that is true but if you have descriptor prepared and only need to program the hw that it is the fastest way. Plus the HW is idle so faster we can submit a new transaction the better it is. Btw the recommendation is that new txn should be submitted in ISR and not wait till tasklet, most driver do latter with few doing former. The computation part of registers to be programmed should be anyway independent of channel, while actual programming and starting of channel we need to ensure right channel and thereby offsets are used :) >=20 > > Also this needs to take into account the dma_slave_config, which is all= owed > > to me modified for subsequent txns >=20 > memcpy doesn't rely on dma_slave_config, does it? Nope but slave does :) this comment is for slave txn :) --=20 ~Vinod --7LkOrbQMr4cezO2T Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iQIcBAEBAgAGBQJVEY7bAAoJEHwUBw8lI4NHjCEP/AtZ59mxeW1fr+tkjwsyI547 sYEXhIn1vLej0dShgqijdgClEBefoHa9C1PUQb44uS8F6NGddqAlzdKq9ue6MeXZ AC4UzjJIySzdj4kiMWnYDywDmZNJZvIdwJ9povvFeZ2j1yOEfGdqEVWC79XN5Tmi AGLsHqcOL3DXmtkTRBfcxBGoKyMun5NsNmFrE97XNqMokcOGW7G3s2srKk1aL6Kt WjUZH500qSx+P7I5DaQ/IA1L1H3fPjOLvEdnAJHbIGNWqWw7JNcOMqiOGc8+ogTK CBvCgLp6qLfY53kTEYAJ1L5OZrHT6kanflkM7L6PzeJWH7Zw2RNS6AtT7KH1a/Nk U2Lla8Du/qbbD5UVN3uoroQ6aDpgLUxS5EcKmJrzTAJlDL0UL9iOmeap+4YzNp7t A3oGQipxNS6YX6qoPGyGm5FIbluHDVzguZpYhbD7eCODuR+iTmPYb9K5XB+dI+mo 31cLAKyeldJ/1JkrTIKWlvtLivcBGDp22LgfybyIiIxmKnPEo845JyH6MH3AZDKo 7QbH5tijW0FbyCmZ7NSsTzz0OLt/tuJwfPU4na2mX44ECWM3F8Ndb4QBOxwCUoFp 52E0XbKXQHtR5JFe9joha214NlkKYnpHsjmGGCGOKECZoet7lVtgbp3tKpuv7CSJ gmbf78CErveoH/J3ff6B =tJ6U -----END PGP SIGNATURE----- --7LkOrbQMr4cezO2T--