On Wed, Sep 16, 2020 at 10:25:49AM -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

This additional level will still force all BSPs to update the
include/require line in MACHINE configs.

> 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?

Isn't this enough to see which tune-coretex* file belongs to which
family? (e.g. git grep arch-armv8-2a.inc to see all available tune files
from armv8.2a family instead of armv8.2 subdirectory)

> Assuming so, does it make sense to try and match this with all the
> other CPU architectures (e.g., x86, ppc, mips, armv7, etc)?
> 
> Thanks,
> Jon
> 
> 
> > Cheers,
> >
> > Richard
> >

> 

Attachment: signature.asc
Description: PGP signature

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#142616): 
https://lists.openembedded.org/g/openembedded-core/message/142616
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]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to