| Message ID | 1442585820-18623-1-git-send-email-thuth@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, 2015-09-18 at 16:17 +0200, Thomas Huth wrote: > It looks somewhat weird that you can enable TUNE_CELL on little > endian systems, so let's disable this option with CPU_LITTLE_ENDIAN. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > I first thought that it might be better to make this option depend > on PPC_CELL instead ... but I guess it's a bad idea to depend a > CPU option on a platform option? Alternatively, would it make > sense to make it depend on (GENERIC_CPU || CELL_CPU) instead? Hmm, it's a little backward, but I think it would be fine, and less confusing for users. Both PS3 and Cell select PPC_CELL, so it would work in both those cases. cheers
On 21/09/15 09:18, Michael Ellerman wrote: > On Fri, 2015-09-18 at 16:17 +0200, Thomas Huth wrote: >> It looks somewhat weird that you can enable TUNE_CELL on little >> endian systems, so let's disable this option with CPU_LITTLE_ENDIAN. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> I first thought that it might be better to make this option depend >> on PPC_CELL instead ... but I guess it's a bad idea to depend a >> CPU option on a platform option? Alternatively, would it make >> sense to make it depend on (GENERIC_CPU || CELL_CPU) instead? > > Hmm, it's a little backward, but I think it would be fine, and less confusing > for users. Both PS3 and Cell select PPC_CELL, so it would work in both those > cases. It's just that when you step through the kernel config (e.g. with "make menuconfig"), you normally step through the "Processor support" first, and then later do the "Platform support". I think most users won't look back into "Processor support" again once they already reached the "Platform support" section, so this TUNE_CELL option then might appear unnoticed when you enable a Cell platform under "Platform support". That's why thought that CPU_LITTLE_ENDIAN or (GENERIC_CPU || CELL_CPU) might be the better option here... but if you think PPC_CELL is the better way to go, I'm also fine to change my patch to use this instead (in the end, TUNE_CELL is likely hardly used anymore nowadays, so it should not be that bad if it does not appear "immediately" to the user). Thomas
On Mon, 2015-09-21 at 12:07 +0200, Thomas Huth wrote: > On 21/09/15 09:18, Michael Ellerman wrote: > > On Fri, 2015-09-18 at 16:17 +0200, Thomas Huth wrote: > >> It looks somewhat weird that you can enable TUNE_CELL on little > >> endian systems, so let's disable this option with CPU_LITTLE_ENDIAN. > >> > >> Signed-off-by: Thomas Huth <thuth@redhat.com> > >> --- > >> I first thought that it might be better to make this option depend > >> on PPC_CELL instead ... but I guess it's a bad idea to depend a > >> CPU option on a platform option? Alternatively, would it make > >> sense to make it depend on (GENERIC_CPU || CELL_CPU) instead? > > > > Hmm, it's a little backward, but I think it would be fine, and less confusing > > for users. Both PS3 and Cell select PPC_CELL, so it would work in both those > > cases. > > It's just that when you step through the kernel config (e.g. with "make > menuconfig"), you normally step through the "Processor support" first, > and then later do the "Platform support". I think most users won't look > back into "Processor support" again once they already reached the > "Platform support" section, so this TUNE_CELL option then might appear > unnoticed when you enable a Cell platform under "Platform support". Ah OK. Personally I almost never use menuconfig, but I guess some folks do. That actually seems like we should reorder those sections, ie. put platform support first, and then processor support. After all there's not much point agonising over whether to tune for CELL cpus if you then don't enable a Cell platform. I'm not sure if it's that simple in practice ... :) cheers
On 06/10/15 12:05, Michael Ellerman wrote: > On Mon, 2015-09-21 at 12:07 +0200, Thomas Huth wrote: >> On 21/09/15 09:18, Michael Ellerman wrote: >>> On Fri, 2015-09-18 at 16:17 +0200, Thomas Huth wrote: >>>> It looks somewhat weird that you can enable TUNE_CELL on little >>>> endian systems, so let's disable this option with CPU_LITTLE_ENDIAN. >>>> >>>> Signed-off-by: Thomas Huth <thuth@redhat.com> >>>> --- >>>> I first thought that it might be better to make this option depend >>>> on PPC_CELL instead ... but I guess it's a bad idea to depend a >>>> CPU option on a platform option? Alternatively, would it make >>>> sense to make it depend on (GENERIC_CPU || CELL_CPU) instead? >>> >>> Hmm, it's a little backward, but I think it would be fine, and less confusing >>> for users. Both PS3 and Cell select PPC_CELL, so it would work in both those >>> cases. >> >> It's just that when you step through the kernel config (e.g. with "make >> menuconfig"), you normally step through the "Processor support" first, >> and then later do the "Platform support". I think most users won't look >> back into "Processor support" again once they already reached the >> "Platform support" section, so this TUNE_CELL option then might appear >> unnoticed when you enable a Cell platform under "Platform support". > > Ah OK. Personally I almost never use menuconfig, but I guess some folks do. > > That actually seems like we should reorder those sections, ie. put platform > support first, and then processor support. After all there's not much point > agonising over whether to tune for CELL cpus if you then don't enable a Cell > platform. Not sure whether reordering the sections make much sense - others might think "I want to support Cell chips with my distro, so let's enable that first, then let's see which platforms I can select next..." - so I'd rather not do that. > I'm not sure if it's that simple in practice ... :) Maybe we could also simply remove the TUNE_CELL option nowadays? I think this was used for building generic Linux distros, which are just optimized for Cell ... but who is still doing that nowadays? Alternatively, if that is not an option and if you don't like my patch with CPU_LITTLE_ENDIAN, what about changing it to check "depends on (GENERIC_CPU || CELL_CPU)" instead? Thomas
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype index c140e94..d93e131 100644 --- a/arch/powerpc/platforms/Kconfig.cputype +++ b/arch/powerpc/platforms/Kconfig.cputype @@ -149,7 +149,7 @@ config 6xx config TUNE_CELL bool "Optimize for Cell Broadband Engine" - depends on PPC64 && PPC_BOOK3S + depends on PPC64 && PPC_BOOK3S && !CPU_LITTLE_ENDIAN help Cause the compiler to optimize for the PPE of the Cell Broadband Engine. This will make the code run considerably faster on Cell
It looks somewhat weird that you can enable TUNE_CELL on little endian systems, so let's disable this option with CPU_LITTLE_ENDIAN. Signed-off-by: Thomas Huth <thuth@redhat.com> --- I first thought that it might be better to make this option depend on PPC_CELL instead ... but I guess it's a bad idea to depend a CPU option on a platform option? Alternatively, would it make sense to make it depend on (GENERIC_CPU || CELL_CPU) instead? arch/powerpc/platforms/Kconfig.cputype | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)