On Tue, Jan 15, 2013 at 06:17:37PM +0000, Mark Hambleton wrote:
> Hi Lorenzo,
>
> > > +obj-$(CONFIG_BIG_LITTLE) += arm_big_little.o
> > There is nothing big.LITTLE specific in all of this, so arm_idle.c would
> >be better.
>
> I figured that because the current version calls into the big.little platform power framework (bL_entry.c) and makes calls into that framework that this wasn't totally generic and is dependant upon that code. The version of the cpuidle driver won't build unless that code is built in, so I still think this is more appropriate naming, I could call it bL_* but I suspect someone will object to that upstream because of the mixed case.
Well, calling it big.LITTLE, bL or whatever else describing a big and a
LITTLE is wrong IMHO because it makes people think this a driver for
big.LITTLE ARM platforms. And it is not, if we ever manage to make it
generic.
>
> > > +static int bl_enter_powerdown(struct cpuidle_device *dev,
> > > + struct cpuidle_driver *drv, int idx);
> > > +
> > > +static struct cpuidle_state bl_cpuidle_set[] __initdata = {
> > > + [0] = {
> > > + .enter = bl_cpuidle_simple_enter,
> > > + .exit_latency = 1,
> > > + .target_residency = 1,
> > > + .power_usage = UINT_MAX,
> > > + .flags = CPUIDLE_FLAG_TIME_VALID,
> > > + .name = "WFI",
> > > + .desc = "ARM WFI",
> > > + },
> > > + [1] = {
> > > + .enter = bl_enter_powerdown,
> > > + .exit_latency = 300,
> > > + .target_residency = 1000,
> > > + .flags = CPUIDLE_FLAG_TIME_VALID,
> > > + .name = "C1",
> > > + .desc = "ARM power down",
> > > + },
> > > +};
>
> > First problem, these states are not standard, in particular
> > target_residency, exit_latency and the enter function. We could
> > have a stab at initializing them from DT (or provide a way to probe
> > them from HW like Intel does), that's the only way to make this stuff
> > generic.
>
> Ok, I will extend with a DT node to retrieve the states, it means I no longer need the generic compatibility node.
Extending the states require a standard and we need to adapt kernel code
to comply and carry out required actions, it is not just a matter of
adding some latencies in DT bindings.
I fail to understand why we want to make this code generic NOW, ARM
kernel is not ready for that and to be honest I do not see why it has
to be done now.
>
> > > +struct cpuidle_driver bl_idle_driver = {
> > > + .name = "bl_idle",
> > > + .owner = THIS_MODULE,
> > > + .safe_state_index = 0
> > > +};
>
> > Different clusters have different drivers. This can be solved with
> > DT and a couple of bindings.
>
> Not sure I understand this one, I am looking at the latest 3.8-rc3 tree and there is only 1 driver registered. Do I need to look at another version?
A cpuidle driver basically is an array of C-states. I am merging code
that defines different states so different drivers for A15 and A7
clusters. cpuidle framework now supports multiple drivers, that's what
we want for big.LITTLE platforms (but the generic idle driver should
support as many drivers as required by DT bindings, which is 1 for
symmetric systems.
>
> > > + bL_set_entry_vector(cpu, cluster, cpu_resume);
>
> > This might not be what we want. We might want to resume from an
> > address different from cpu_resume (although it might well be
> > standardized since the power API allows to carry out platform
> > specific operations before jumping back to the kernel so I reckon
> > this can be and will be generic).
>
> Ok, this was a patch to the linaro tree, it wasn't intended as a re-write of the driver.
I still do not understand why this file move has to be done now.
>
> > > + ret = cpu_suspend((unsigned long) dev, bl_powerdown_finisher);
> > This is where things start getting a little more complicated. If we do
> > not know what the power state implies (e.g. L2 retained or not, global
> > timer or emulated local timers, GIC state lost in low-power or not) we
> > end up saving/restoring state or executing actions that may be useless.
> >
> > In Intel world executing "mwait" does the trick for all C-states. Here
> > we need to define what a state is and what it implies to carry out the
> > required actions in a generic way, provided we can pull it off.
> >
> > PSCI and the power API are a MAJOR step in this direction.
>
> Same again, this was based on what was already there... if you are re-working for PSCI please just treat this as a request to be more generic where possible.
To be generic we need multiple platforms and understand what are they
doing with the respective C-states. I will update the code for PSCI.
>
> > This should be improved to cater for multiple drivers but that's not my
> > major concern.
> Ok, as I said this was just a move of tc2-cpuidle.c from mach-vexpress into drivers/cpuidle, if that driver is deficient then I suspect someone else is looking to fix it?
The driver is not deficient it is just a perfectly working cpuidle driver,
TC2 specific, as tens of other cpuidle ARM drivers. It should be improved and
made generic in due course, when we have a standard for C-states and the
kernel save/restore and cache flushing mechanism is coded to follow suit.
Lorenzo
Hi there,
We're hoping to use the foundation model and its filesystem for our LLVM
AArch64 testing, but I'm having some difficulties getting reliable ssh access.
I'm running the suggested kernel image and filesystem with options:
$ Foundation_v8pkg/Foundation_v8 --image img-foundation.axf --block-device
vexpress64-openembedded_sdk-armv8_20121213-134.img --network=nat --network-
nat-ports=8022=22
However, when I login to the system:
$ ssh -p 8022 root@localhost
I can execute commands reasonably, but the session appears to hang when I
exit. I can, of course, exit with "<Ret>~.", but that leaves the process
running on the model.
Similar problems occur with scp, and even switching to bridged networking
doesn't resolve the issue.
Does anyone have any experience with this or a solution?
Cheers.
Tim.
-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On Tue, 15 Jan 2013, Mark Hambleton wrote:
> Hi Lorenzo,
>
> > > +obj-$(CONFIG_BIG_LITTLE) += arm_big_little.o
> > There is nothing big.LITTLE specific in all of this, so arm_idle.c would
> >be better.
>
> I figured that because the current version calls into the big.little
> platform power framework (bL_entry.c) and makes calls into that
> framework that this wasn't totally generic and is dependant upon that
> code. The version of the cpuidle driver won't build unless that code
> is built in, so I still think this is more appropriate naming, I could
> call it bL_* but I suspect someone will object to that upstream
> because of the mixed case.
I'll wait that someone with a cluebat. Semantically, "bL_" is the most
efficient prefix you could find to refer to b"ig.LITTLE". And no one
eported any issue with that from the initial public review so far.
Nicolas
(Forward to the right address for linaro-dev)
-------- Original Message --------
Subject: [PATCH] vexpress: Setup IRQ affinity to A7s for TC2
Date: Wed, 9 Jan 2013 14:20:19 +0000
From: Punit Agrawal <punit.agrawal(a)arm.com><mailto:punit.agrawal@arm.com>
To: tixy(a)linaro.org<mailto:tixy@linaro.org> <tixy(a)linaro.org><mailto:tixy@linaro.org>
CC: linaro-dev(a)linaro.org<mailto:linaro-dev@linaro.org> <linaro-dev(a)linaro.org><mailto:linaro-dev@linaro.org>, Punit Agrawal <Punit.Agrawal(a)arm.com><mailto:Punit.Agrawal@arm.com>
On TC2, to maximise power savings, it helps to affine the IRQs to the A7s. Now
that the cpu to parts mapping is available via /proc/cpuinfo, this patch
introduces a script to parse this file and build a mask of particular CPU type
and change the IRQ affinity to this mask.
The patch also makes the necessary changes to platform specific init script to
call above script and affine the IRQs to the A7s.
Lastly, it changes the makefile to include the new script in the target.
Change-Id: I2fd2840acd4602d9145c7c2f2e839e5dda09ff99
Signed-off-by: Punit Agrawal <punit.agrawal(a)arm.com><mailto:punit.agrawal@arm.com>
---
Hi Tixy,
I am not quite sure where to send this patch to get it included in Linaro. But
you seem to have a few patches in this repo, so maybe you can apply this one.
Let me know if you have any feedback on this as well. I would be happy to
incorporate that as long as the intent of the patches carries through.
Thanks,
Punit
device.mk | 3 ++-
init.arm-versatileexpress.rc | 6 ++++++
set_irq_affinity.sh | 45 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 53 insertions(+), 1 deletion(-)
create mode 100755 set_irq_affinity.sh
diff --git a/device.mk b/device.mk
index 6e6c6dd..9ca73e3 100644
--- a/device.mk
+++ b/device.mk
@@ -9,7 +9,8 @@ PRODUCT_COPY_FILES += \
device/linaro/vexpress/init.v2p-aarch64.rc:root/init.v2p-aarch64.rc \
device/linaro/vexpress/ueventd.v2p-aarch64.rc:root/ueventd.v2p-aarch64.rc \
device/linaro/vexpress/init.vexpress.sh:system/etc/init.vexpress.sh \
- device/linaro/vexpress/initlogo.rle:root/initlogo.rle
+ device/linaro/vexpress/initlogo.rle:root/initlogo.rle \
+ device/linaro/vexpress/set_irq_affinity.sh:root/set_irq_affinity.sh
PRODUCT_CHARACTERISTICS := tablet,nosdcard
diff --git a/init.arm-versatileexpress.rc b/init.arm-versatileexpress.rc
index a71e491..d710f36 100644
--- a/init.arm-versatileexpress.rc
+++ b/init.arm-versatileexpress.rc
@@ -33,6 +33,12 @@ on boot
chown system system /sys/class/graphics/fb0/fit_to_screen
chown system system /sys/class/graphics/fb1/overlays
+# setup IRQ affinity to the A7s
+service setirqaffinity /set_irq_affinity 0xc07
+ class main
+ user root
+ oneshot
+
service faketsd /system/bin/faketsd
class main
user bluetooth
diff --git a/set_irq_affinity.sh b/set_irq_affinity.sh
new file mode 100755
index 0000000..4a09d61
--- /dev/null
+++ b/set_irq_affinity.sh
@@ -0,0 +1,45 @@
+#!/system/bin/sh
+
+# This script sets the default affinity to the processors with the given part id.
+# - part id is in hex (as seen in /proc/cpuinfo)
+
+function build_mask_from_part_id {
+ local IFS
+ local mask
+ local ref_part_id
+
+ ref_part_id=$1
+ IFS=$'\n'
+
+ for line in `cat /proc/cpuinfo`
+ do
+ IFS=':'
+ set -A tokens $line
+
+ if [ "${line#'processor'}" != "$line" ]
+ then
+ cpu="${tokens[1]##' '}"
+ elif [ "${line#'CPU part'}" != "$line" ]
+ then
+ part_id="${tokens[1]##' '}"
+
+ if [ "$part_id" == "$ref_part_id" ]
+ then
+ (( mask |= 1 << $cpu ))
+ fi
+ fi
+ done
+ echo $(printf "%x" $mask)
+}
+
+ref_part_id=$(echo $1 | tr '[A-Z]' '[a-z]')
+mask=$(build_mask_from_part_id $ref_part_id)
+[ -z "$mask" ] && exit
+
+echo $mask > /proc/irq/default_smp_affinity
+
+for i in `ls /proc/irq`
+do
+ affinity_file="/proc/irq/$i/smp_affinity"
+ [ -e $affinity_file ] && echo $mask > $affinity_file
+done
--
1.7.9.5
-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
cpufreq core doesn't manage offline cpus and if driver->init() has returned
mask including offline cpus, it may result in unwanted behavior by cpufreq core
or governors.
We need to get only online cpus in this mask. There are two places to fix this
mask, cpufreq core and cpufreq driver. It makes sense to do this at common place
and hence is done in core.
Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org>
---
drivers/cpufreq/cpufreq.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 1f93dbd..de99517 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -970,6 +970,13 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
pr_debug("initialization failed\n");
goto err_unlock_policy;
}
+
+ /*
+ * affected cpus must always be the one, which are online. We aren't
+ * managing offline cpus here.
+ */
+ cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
+
policy->user_policy.min = policy->min;
policy->user_policy.max = policy->max;
--
1.7.12.rc2.18.g61b472e
The states could be sorted in the power backward order, we get rid of
some lines of code and by this way that fixes also a bug in the dynamic
c-states.
Changelog:
V2:
* added the optimization in the select menu governor
Daniel Lezcano (2):
cpuidle - remove the power_specified field in the driver
cpuidle - optimize the select function for the 'menu' governor
drivers/cpuidle/cpuidle.c | 17 ++++-------------
drivers/cpuidle/driver.c | 25 -------------------------
drivers/cpuidle/governors/menu.c | 20 ++++++++------------
include/linux/cpuidle.h | 2 +-
4 files changed, 13 insertions(+), 51 deletions(-)
--
1.7.5.4
Hi,
I tried booting linux-linaro-2.6.37 kernel on my beagle board C4. I executed
following:
1. Installed linaro on a 4 GB SD card using linaro-image-tools 0.4.1 with
hwpack daily snapshot hwpack_linaro-omap3_20110125-0_armel_supported.tar.gz
and linaro-natty-headless-tar-20101202-1.tar.gz. It was booting properly on
my BB.
2. Cloned linux-linaro-2.6.37. Changed to source directory
3. make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- omap2plus_defconfig
4. make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- menuconfig (enabled
EARLY_PRINTK)
5. make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- uImage
6. make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- modules
7. make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- modules_install
INSTALL_MOD_PATH=/media/rootfs
8. cp arch/arm/boot/uImage /media/boot; sync
Everything went on smoothly. Then I put the SD card on BB and powered it on.
I got a kernel panic: http://paste.ubuntu.com/560562
Please help me figuring out the problem. Is it because I didn't create
uInitrd? If so, then how to create it for ARM?
Regards,
Avik