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

[Mark Hatle <mark.hatle@windriver.com>]

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?

--Mark

> Cheers,
>
> Richard
>