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

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

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

Cheers,

Richard