On Wed, 2020-09-16 at 10:25 -0400, Jon Mason wrote: > On Wed, Sep 16, 2020 at 9:49 AM Richard Purdie > <[email protected]> wrote: > > On Wed, 2020-09-16 at 09:45 -0400, Jon Mason wrote: > > > On Wed, Sep 16, 2020 at 9:26 AM Richard Purdie > > > <[email protected]> wrote: > > > > On Mon, 2020-09-14 at 11:13 -0400, Jon Mason wrote: > > > > > There is a large number of Arm Tune files located in > > > > > meta/conf/machine/include/, and to support the current and > > > > > upcoming Arm > > > > > cores, more are needed. Adding more files is simply going to > > > > > make it > > > > > harder to find the relevant ones for an OE/YP > > > > > developer/user. Also, > > > > > there are problems with stale and erroneous configs (see my > > > > > previous > > > > > series), which will only be exacerbated by having more files. > > > > > > > > > > I am proposing a reorganization of the existing tune files by > > > > > including > > > > > them in the generic family include file. For example, the > > > > > tune-cortexa55.inc would be moved into arch-armv8-2a.inc, and > > > > > tune-cortexa57.inc would be moved into arch-armv8a.inc. This > > > > > reduces > > > > > the number of files from 12 to 2 for ARMv8a, and that is > > > > > excluding the > > > > > 13 I am adding in this series that would otherwise be unique > > > > > files. > > > > > > > > > > To use, simply add > > > > > ... > > > > > DEFAULTTUNE ?= "neoversen1" > > > > > require conf/machine/include/arm/arch-armv8-2a.inc > > > > > ... > > > > > > > > > > Which is arguably what should be done anyway (instead of > > > > > taking > > > > > the > > > > > default of the tune include file). > > > > > See the qemuarm64 patch in the series for a working example. > > > > > > > > > > Of course, by removing the existing tune files, current users > > > > > are > > > > > going > > > > > to break. A simple script can be written to use sed (or > > > > > similar) > > > > > to > > > > > replace the relevant parts for those users that would be > > > > > affected > > > > > (at > > > > > least for those that are in the layer index and update > > > > > regularly). > > > > > > > > I've just looked at this in a bit more details and I'm worried. > > > > > > > > The intent is the BSP includes the processor/core tune file > > > > that > > > > its > > > > based upon. The BSP should usually know which one is present. > > > > > > > > That tune then presents the possible options which the BSPs > > > > selects > > > > from but the end user or distro can override. > > > > > > > > With this change, you get all tunes and you don't know which > > > > are > > > > compatible with a given core/processor. I'm not sure that is an > > > > improvement. > > > > > > Before you were selecting an A75 inc file (for example), now you > > > are > > > specifying an ARMv8.2 inc file and saying it is an A75 from the > > > available list. Either way, you needed to know which core you > > > were > > > running. > > > > > > In addition to this, you have the ability to run the generic > > > default > > > for the family, which will work for all of them. And for ARMv8, > > > even > > > the more generic armv8 will work for all of them. So, there > > > isn't an > > > incompatibility problem. > > > > You should have the ability to access the generic tune with either > > approach? > > > > > > I appreciate not wanting "lots of files" but we need to > > > > consider > > > > the > > > > usability too. > > > > > > > > As others have mentioned, the errors are fairly obvious with > > > > diff > > > > between the files (or a GUI like meld). > > > > > > > > What are the advantages this brings other than fewer files? Am > > > > I > > > > missing something? > > > > > > This is the main benefit of it, TBH. If I add 25 more arm tunes, > > > it's going to get ugly. And if Arm keeps adding cores at this > > > same > > > pace every year, it's going to get even uglier. However, I'm > > > fine to > > > send out those patches if it only bothers me to have so many > > > files. > > > > My worry is that the current system: > > > > a) tells BSP developers to select their processor, not an arch > > b) only shows them tunes that work on that processor > > > > So we're changing the model, but only for armv8XXX. This is going > > to be > > confusing. I do like only showing people things they can use too. > > > > How about we split the difference and add the new tune files into > > subdirs by architecture? > > So it would look something like > meta/conf/machine/arm/armv8.0/tune-cortexa57.inc > meta/conf/machine/arm/armv8.2/tune-cortexa75.inc
Yes. With an open question on whether we move any existing files. Maybe just the armv8.0+ ones? > And inside of those, it would reference > meta/conf/machine/include/arm/arch-armv8.inc or > meta/conf/machine/include/arm/arch-armv8-2a.inc (just as it does > now). Correct? Yes. > Assuming so, does it make sense to try and match this with all the > other CPU architectures (e.g., x86, ppc, mips, armv7, etc)? I'd say not worth it, just do this either for new ones or the armv8* onwards ones. Cheers, Richard
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#142617): https://lists.openembedded.org/g/openembedded-core/message/142617 Mute This Topic: https://lists.openembedded.org/mt/76844469/21656 Group Owner: [email protected] Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
