Re: [PATCH 1/1] conf/tune: add tune-ppce300c3

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Thu, 2012-09-06 at 18:07 -0500, Mark Hatle wrote:
> On 9/6/12 5:59 PM, Richard Purdie wrote:
> > On Thu, 2012-09-06 at 17:35 -0500, Mark Hatle wrote:
> >> On 9/6/12 5:20 PM, Bruce Ashfield wrote:
> >>> On 12-09-06 6:19 PM, Richard Purdie wrote:
> >>>> On Thu, 2012-09-06 at 14:43 -0400, Bruce Ashfield wrote:
> >>>>> It has been pointed out several times that the yocto mpc8315e-rdb
> >>>>> reference was using the wrong tuning (603e), since it is actually
> >>>>> a e300c3 board.
> >>>>>
> >>>>> This commit creates a e300c3 tune file based on the e300c2 variant
> >>>>> already in oe-core.
> >>>>>
> >>>>> This commit also inhibits altivec in flac when this new tuning is
> >>>>> enabled. It was also noticed that the existing tune based overrides
> >>>>> in the flac package would not be triggered since DEFAULTTUNE is not
> >>>>> in the overrides list. To avoid doing per-board disabling of altivec
> >>>>> DEFAULTTUNE is added to the local package OVERRIDES and then used
> >>>>> to disable altivec.
> >>>>>
> >>>>> [YOCTO #1192]
> >>>>>
> >>>>> Signed-off-by: Bruce Ashfield<bruce.ashfield@windriver.com>
> >>>>>
> >>>>> asdfkljds
> >>>>> Signed-off-by: Bruce Ashfield<bruce.ashfield@windriver.com>
> >>>>> ---
> >>>>>     meta/conf/machine/include/tune-ppce300c3.inc |   11 +++++++++++
> >>>>>     meta/recipes-multimedia/flac/flac_1.2.1.bb   |    5 +++++
> >>>>>     2 files changed, 16 insertions(+), 0 deletions(-)
> >>>>>     create mode 100644 meta/conf/machine/include/tune-ppce300c3.inc
> >>>>>
> >>>>> diff --git a/meta/conf/machine/include/tune-ppce300c3.inc b/meta/conf/machine/include/tune-ppce300c3.inc
> >>>>> new file mode 100644
> >>>>> index 0000000..3f5ac26
> >>>>> --- /dev/null
> >>>>> +++ b/meta/conf/machine/include/tune-ppce300c3.inc
> >>>>> @@ -0,0 +1,11 @@
> >>>>> +DEFAULTTUNE ?= "ppce300c3"
> >>>>> +
> >>>>> +require conf/machine/include/powerpc/arch-powerpc.inc
> >>>>> +
> >>>>> +TUNEVALID[ppce300c3] = "Enable ppce300c3 specific processor optimizations"
> >>>>> +TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "ppce300c3", "-mcpu=e300c3", "", d)}"
> >>>>> +
> >>>>> +AVAILTUNES += "ppce300c3"
> >>>>> +TUNE_FEATURES_tune-ppce300c3 = "m32 fpu-soft ppce300c3"
> >>>>> +TUNE_PKGARCH_tune-ppce300c3 = "ppce300c3"
> >>>>> +PACKAGE_EXTRA_ARCHS_tune-ppce300c3 = "${PACKAGE_EXTRA_ARCHS_tune-powerpc-nf} ppce300c3"
> >>>>> diff --git a/meta/recipes-multimedia/flac/flac_1.2.1.bb b/meta/recipes-multimedia/flac/flac_1.2.1.bb
> >>>>> index 3c5b73c..25db1c4 100644
> >>>>> --- a/meta/recipes-multimedia/flac/flac_1.2.1.bb
> >>>>> +++ b/meta/recipes-multimedia/flac/flac_1.2.1.bb
> >>>>> @@ -36,9 +36,14 @@ EXTRA_OECONF = "--disable-oggtest --disable-id3libtest \
> >>>>>                     --without-xmms-exec-prefix \
> >>>>>                     --without-libiconv-prefix \
> >>>>>                     --without-id3lib"
> >>>>> +
> >>>>> +FLACOVERRIDE = ":${DEFAULTTUNE}"
> >>>>> +OVERRIDES .= "${FLACOVERRIDE}"
> >>>>> +
> >>>>>     EXTRA_OECONF_prepend_e500mc = "--disable-altivec "
> >>>>>     EXTRA_OECONF_prepend_e5500 = "--disable-altivec "
> >>>>>     EXTRA_OECONF_prepend_e5500-64b = "--disable-altivec "
> >>>>> +EXTRA_OECONF_prepend_ppce300c3 = "--disable-altivec "
> >>>>>
> >>>>
> >>>> This is getting ugly and is kind of unsafe. Perhaps the architecture
> >>>> should be doing something like:
> >>>>
> >>>> MACHINEOVERRIDES .= "${@bb.utils.contains("TUNE_FEATURES", "ppce300c3", ":noaltivec", "" ,d)}
> >>>>
> >>>> or even in this recipe just do:
> >>>>
> >>>> EXTRA_OECONF += "${@bb.utils.contains("TUNE_FEATURES", "ppce300c3", " --disable-altivec", "" ,d)}
> >>>
> >>> I definitely considered this route. I can do that for the new arch, and the
> >>> old ones, but can't test the old ones at the moment.
> >>
> >> The problem is actually in the flac.  This recipe sees powerpc as the machine
> >> type, and immediately enabled altivec.  By default altivec support is disabled
> >> in OE-Core.
> >>
> >> Perhaps one way we could address this is add a tune flag that says if the tune
> >> has altivec support or not.. then in the flac binary, disable it unless it's
> >> enabled?
> >
> > I think having altivec in the tune_features would be ideal. I'd love to
> > get this cleaned up too but I lack much knowledge about powerpc...
> 
> My proposal then would be to accept Bruce's patch, with the understanding that 
> we need to add a tune flag of 'altivec' to the PowerPC tunings (where 
> appropriate) and then make the flac recipe respect that flag.
> 
> Unfortunately I don't have the time to do that right now or I would.  Would an 
> enhancement bug in the Yocto Project bugzilla be enough to be sure the work is done?

Please send something machine specific. Adding DEFAULTTUNE is a bad idea
for several reasons, not least it might not be the selected tune.

I took Matthew's original patch on the understanding this gets fixed and
it hasn't happened but please do file the request.

Cheers,

Richard