Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for x86/topology: fix erroneous smp_num_siblings on Intel Hybrid platform
On Tue, 2023-03-28 at 09:14 +0800, Zhang Rui wrote:
> Hi, Imre,
>
> Thanks for raising this.
>
> On Tue, 2023-03-28 at 00:26 +0300, Imre Deak wrote:
> > Hi Rui,
> >
> > after applying your
> > "x86/topology: fix erroneous smp_num_siblings on Intel Hybrid
> > platform"
> >
> > fix on the drm-tip tree (see the patchwork URL below) the CI tests
> > show
> > some regression on a HSW and a KBL machine (see [2] and [4] below)
> > in
> > the i915 driver. However I think they can't be related to your
> > changes,
> > since on these machines all cores will report the same number of
> > CPU
> > siblings.
>
> Right.
>
> > Could you confirm this and that in general the reported
> > siblings can only vary on platforms with both E and P cores (ADL-P
> > being
> > the first such platform)?
>
> Right.
>
> I don't think the patch could bring any change related.
> It only affects hybrid platforms.
Is this topology fix patch the only patch applied?
or together with some other patches?
I can hardly imagine that the fix patch can trigger such issues, so I
suspect they are intermittent issues. say
is the regression 100% reproducible?
does the warning/failure ever show without the patch?
BTW, I just happened to see this thread
https://lore.kernel.org/all/dm8pr11mb565580bcf44661b6a392f0cee0...@dm8pr11mb5655.namprd11.prod.outlook.com/
If the problem on hand has been verified to be not related with the
topology fix, can we update in this thread as well?
https://lore.kernel.org/all/20230323015640.27906-1-rui.zh...@intel.com/
This is another issue that the patch fixes. And it's better to have a
Buglink/Tested-by tag in the commit.
thanks,
rui
>
> Thanks,
> rui
> > Thanks,
> > Imre
> >
> > On Mon, Mar 27, 2023 at 07:02:25PM +, Patchwork wrote:
> > > == Series Details ==
> > >
> > > Series: x86/topology: fix erroneous smp_num_siblings on Intel
> > > Hybrid platform
> > > URL : https://patchwork.freedesktop.org/series/115661/
> > > State : failure
> > >
> > > == Summary ==
> > >
> > > CI Bug Log - changes from CI_DRM_12921 -> Patchwork_115661v1
> > >
> > >
> > > Summary
> > > ---
> > >
> > > **FAILURE**
> > >
> > > Serious unknown changes coming with Patchwork_115661v1
> > > absolutely
> > > need to be
> > > verified manually.
> > >
> > > If you think the reported changes have nothing to do with the
> > > changes
> > > introduced in Patchwork_115661v1, please notify your bug team
> > > to
> > > allow them
> > > to document this new failure mode, which will reduce false
> > > positives in CI.
> > >
> > > External URL:
> > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115661v1/index.html
> > >
> > > Participating hosts (37 -> 37)
> > > --
> > >
> > > No changes in participating hosts
> > >
> > > Possible new issues
> > > ---
> > >
> > > Here are the unknown changes that may have been introduced in
> > > Patchwork_115661v1:
> > >
> > > ### IGT changes ###
> > >
> > > Possible regressions
> > >
> > > * igt@i915_selftest@live@hangcheck:
> > > - fi-hsw-4770:[PASS][1] -> [DMESG-WARN][2]
> > >[1]:
> > > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/fi-hsw-4770/igt@i915_selftest@l...@hangcheck.html
> > >[2]:
> > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115661v1/fi-hsw-4770/igt@i915_selftest@l...@hangcheck.html
> > >
> > >
> > > Suppressed
> > >
> > > The following results come from untrusted machines, tests, or
> > > statuses.
> > > They do not affect the overall result.
> > >
> > > * igt@fbdev@info:
> > > - {bat-kbl-2}:[SKIP][3] ([fdo#109271]) -> [ABORT][4]
> > >[3]:
> > > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/bat-kbl-2/igt@fb...@info.html
> > >[4]:
> > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115661v1/bat-kbl-2/igt@fb...@info.html
> > >
> > >
> > > Known issues
> > >
> > >
> > > Here are the changes found in Patchwork_115661v1 that come from
> > > known i
Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for x86/topology: fix erroneous smp_num_siblings on Intel Hybrid platform
Hi, Imre,
Thanks for raising this.
On Tue, 2023-03-28 at 00:26 +0300, Imre Deak wrote:
> Hi Rui,
>
> after applying your
> "x86/topology: fix erroneous smp_num_siblings on Intel Hybrid
> platform"
>
> fix on the drm-tip tree (see the patchwork URL below) the CI tests
> show
> some regression on a HSW and a KBL machine (see [2] and [4] below) in
> the i915 driver. However I think they can't be related to your
> changes,
> since on these machines all cores will report the same number of CPU
> siblings.
Right.
> Could you confirm this and that in general the reported
> siblings can only vary on platforms with both E and P cores (ADL-P
> being
> the first such platform)?
Right.
I don't think the patch could bring any change related.
It only affects hybrid platforms.
Thanks,
rui
>
> Thanks,
> Imre
>
> On Mon, Mar 27, 2023 at 07:02:25PM +, Patchwork wrote:
> > == Series Details ==
> >
> > Series: x86/topology: fix erroneous smp_num_siblings on Intel
> > Hybrid platform
> > URL : https://patchwork.freedesktop.org/series/115661/
> > State : failure
> >
> > == Summary ==
> >
> > CI Bug Log - changes from CI_DRM_12921 -> Patchwork_115661v1
> >
> >
> > Summary
> > ---
> >
> > **FAILURE**
> >
> > Serious unknown changes coming with Patchwork_115661v1 absolutely
> > need to be
> > verified manually.
> >
> > If you think the reported changes have nothing to do with the
> > changes
> > introduced in Patchwork_115661v1, please notify your bug team to
> > allow them
> > to document this new failure mode, which will reduce false
> > positives in CI.
> >
> > External URL:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115661v1/index.html
> >
> > Participating hosts (37 -> 37)
> > --
> >
> > No changes in participating hosts
> >
> > Possible new issues
> > ---
> >
> > Here are the unknown changes that may have been introduced in
> > Patchwork_115661v1:
> >
> > ### IGT changes ###
> >
> > Possible regressions
> >
> > * igt@i915_selftest@live@hangcheck:
> > - fi-hsw-4770:[PASS][1] -> [DMESG-WARN][2]
> >[1]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/fi-hsw-4770/igt@i915_selftest@l...@hangcheck.html
> >[2]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115661v1/fi-hsw-4770/igt@i915_selftest@l...@hangcheck.html
> >
> >
> > Suppressed
> >
> > The following results come from untrusted machines, tests, or
> > statuses.
> > They do not affect the overall result.
> >
> > * igt@fbdev@info:
> > - {bat-kbl-2}:[SKIP][3] ([fdo#109271]) -> [ABORT][4]
> >[3]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/bat-kbl-2/igt@fb...@info.html
> >[4]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115661v1/bat-kbl-2/igt@fb...@info.html
> >
> >
> > Known issues
> >
> >
> > Here are the changes found in Patchwork_115661v1 that come from
> > known issues:
> >
> > ### IGT changes ###
> >
> > Issues hit
> >
> > * igt@gem_exec_suspend@basic-s3@lmem0:
> > - bat-dg2-11: [PASS][5] -> [INCOMPLETE][6]
> > ([i915#6311])
> >[5]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/bat-dg2-11/igt@gem_exec_suspend@basic...@lmem0.html
> >[6]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115661v1/bat-dg2-11/igt@gem_exec_suspend@basic...@lmem0.html
> >
> > * igt@i915_selftest@live@reset:
> > - bat-rpls-1: [PASS][7] -> [ABORT][8] ([i915#4983])
> >[7]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/bat-rpls-1/igt@i915_selftest@l...@reset.html
> >[8]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115661v1/bat-rpls-1/igt@i915_selftest@l...@reset.html
> >
> > * igt@i915_selftest@live@slpc:
> > - bat-rpls-2: NOTRUN -> [DMESG-FAIL][9] ([i915#6367] /
> > [i915#7913] / [i915#7996])
> >[9]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115661v1/bat-rpls-2/igt@i915_selftest@l...@slpc.html
> >
> > * igt@i915_suspend@basic-s3-without-i915:
> > - bat-rpls-2: NOTRUN -> [ABORT][10] ([i915#6687] /
> > [i915#7978])
> >[10]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115661v1/bat-rpls-2/igt@i915_susp...@basic-s3-without-i915.html
> >
> > * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-d-
> > dp-1:
> > - bat-dg2-8: [PASS][11] -> [FAIL][12] ([i915#7932])
> >[11]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc-frame-seque...@pipe-d-dp-1.html
> >[12]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115661v1/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc-frame-seque...@pipe-d-dp-1.html
> >
> >
> > Possible fixes
> >
> > * igt@i915_selftest@live@gt_heartbeat:
> > - fi-kbl-soraka: [DMESG-FAIL][13] ([i915#5334
Re: [Intel-gfx] [-next PATCH 3/4] treewide: Use DEVICE_ATTR_RO
On Tue, 2017-12-19 at 10:15 -0800, Joe Perches wrote:
> Convert DEVICE_ATTR uses to DEVICE_ATTR_RO where possible.
>
> Done with perl script:
>
> $ git grep -w --name-only DEVICE_ATTR | \
> xargs perl -i -e 'local $/; while (<>) {
> s/\bDEVICE_ATTR\s*\(\s*(\w+)\s*,\s*\(?(?:\s*S_IRUGO\s*|\s*0444\s*)\)?
> \s*,\s*\1_show\s*,\s*NULL\s*\)/DEVICE_ATTR_RO(\1)/g; print;}'
>
> Signed-off-by: Joe Perches
> ---
> arch/arm/mach-pxa/sharpsl_pm.c | 4 ++--
> arch/sh/drivers/push-switch.c| 2 +-
> arch/tile/kernel/sysfs.c | 10 +-
> drivers/acpi/device_sysfs.c | 6 +++---
> drivers/char/ipmi/ipmi_msghandler.c | 17
> -
> drivers/gpu/drm/i915/i915_sysfs.c| 6 +++---
> drivers/nvme/host/core.c | 10 +-
> drivers/s390/cio/css.c | 8
> drivers/s390/cio/device.c| 8
> drivers/s390/crypto/ap_card.c| 2 +-
> drivers/scsi/hpsa.c | 10 +-
> drivers/scsi/lpfc/lpfc_attr.c| 18
> --
> drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c | 8
> drivers/thermal/thermal_sysfs.c | 6 +++---
For the thermal part,
ACK-by: Zhang Rui
thanks,
rui
> sound/soc/soc-core.c | 2 +-
> sound/soc/soc-dapm.c | 2 +-
> 16 files changed, 58 insertions(+), 61 deletions(-)
>
> diff --git a/arch/arm/mach-pxa/sharpsl_pm.c b/arch/arm/mach-
> pxa/sharpsl_pm.c
> index 398ba9ba2632..ef9fd9b759cb 100644
> --- a/arch/arm/mach-pxa/sharpsl_pm.c
> +++ b/arch/arm/mach-pxa/sharpsl_pm.c
> @@ -802,8 +802,8 @@ static ssize_t battery_voltage_show(struct device
> *dev, struct device_attribute
> return sprintf(buf, "%d\n",
> sharpsl_pm.battstat.mainbat_voltage);
> }
>
> -static DEVICE_ATTR(battery_percentage, 0444,
> battery_percentage_show, NULL);
> -static DEVICE_ATTR(battery_voltage, 0444, battery_voltage_show,
> NULL);
> +static DEVICE_ATTR_RO(battery_percentage);
> +static DEVICE_ATTR_RO(battery_voltage);
>
> extern void (*apm_get_power_status)(struct apm_power_info *);
>
> diff --git a/arch/sh/drivers/push-switch.c b/arch/sh/drivers/push-
> switch.c
> index a17181160233..762bc5619910 100644
> --- a/arch/sh/drivers/push-switch.c
> +++ b/arch/sh/drivers/push-switch.c
> @@ -24,7 +24,7 @@ static ssize_t switch_show(struct device *dev,
> struct push_switch_platform_info *psw_info = dev-
> >platform_data;
> return sprintf(buf, "%s\n", psw_info->name);
> }
> -static DEVICE_ATTR(switch, S_IRUGO, switch_show, NULL);
> +static DEVICE_ATTR_RO(switch);
>
> static void switch_timer(struct timer_list *t)
> {
> diff --git a/arch/tile/kernel/sysfs.c b/arch/tile/kernel/sysfs.c
> index af5024f0fb5a..b09456a3d77a 100644
> --- a/arch/tile/kernel/sysfs.c
> +++ b/arch/tile/kernel/sysfs.c
> @@ -38,7 +38,7 @@ static ssize_t chip_width_show(struct device *dev,
> {
> return sprintf(page, "%u\n", smp_width);
> }
> -static DEVICE_ATTR(chip_width, 0444, chip_width_show, NULL);
> +static DEVICE_ATTR_RO(chip_width);
>
> static ssize_t chip_height_show(struct device *dev,
> struct device_attribute *attr,
> @@ -46,7 +46,7 @@ static ssize_t chip_height_show(struct device *dev,
> {
> return sprintf(page, "%u\n", smp_height);
> }
> -static DEVICE_ATTR(chip_height, 0444, chip_height_show, NULL);
> +static DEVICE_ATTR_RO(chip_height);
>
> static ssize_t chip_serial_show(struct device *dev,
> struct device_attribute *attr,
> @@ -54,7 +54,7 @@ static ssize_t chip_serial_show(struct device *dev,
> {
> return get_hv_confstr(page, HV_CONFSTR_CHIP_SERIAL_NUM);
> }
> -static DEVICE_ATTR(chip_serial, 0444, chip_serial_show, NULL);
> +static DEVICE_ATTR_RO(chip_serial);
>
> static ssize_t chip_revision_show(struct device *dev,
> struct device_attribute *attr,
> @@ -62,7 +62,7 @@ static ssize_t chip_revision_show(struct device
> *dev,
> {
> return get_hv_confstr(page, HV_CONFSTR_CHIP_REV);
> }
> -static DEVICE_ATTR(chip_revision, 0444, chip_revision_show, NULL);
> +static DEVICE_ATTR_RO(chip_revision);
>
>
> static ssize_t type_show(struct device *dev,
> @@ -71,7 +71,7 @@ static ssize_t type_show(struct device *dev,
> {
> return sprintf(page, "tilera\n");
>
Re: [Intel-gfx] [-next PATCH 2/4] treewide: Use DEVICE_ATTR_RW
On Tue, 2017-12-19 at 10:15 -0800, Joe Perches wrote:
> Convert DEVICE_ATTR uses to DEVICE_ATTR_RW where possible.
>
> Done with perl script:
>
> $ git grep -w --name-only DEVICE_ATTR | \
> xargs perl -i -e 'local $/; while (<>) {
> s/\bDEVICE_ATTR\s*\(\s*(\w+)\s*,\s*\(?(\s*S_IRUGO\s*\|\s*S_IWUSR|\s*S
> _IWUSR\s*\|\s*S_IRUGO\s*|\s*0644\s*)\)?\s*,\s*\1_show\s*,\s*\1_store\
> s*\)/DEVICE_ATTR_RW(\1)/g; print;}'
>
> Signed-off-by: Joe Perches
> ---
> arch/s390/kernel/topology.c | 3 +--
> arch/tile/kernel/sysfs.c | 2 +-
> drivers/gpu/drm/i915/i915_sysfs.c| 6 ++---
> drivers/platform/x86/compal-laptop.c | 18 +--
> drivers/s390/cio/device.c| 2 +-
> drivers/scsi/lpfc/lpfc_attr.c| 43
>
> drivers/thermal/thermal_sysfs.c | 9
For the thermal part,
ACK-by: Zhang Rui
thanks,
rui
> drivers/tty/serial/sh-sci.c | 2 +-
> drivers/usb/host/xhci-dbgcap.c | 2 +-
> drivers/usb/phy/phy-tahvo.c | 2 +-
> drivers/video/fbdev/auo_k190x.c | 4 ++--
> drivers/video/fbdev/w100fb.c | 4 ++--
> lib/test_firmware.c | 14 +---
> lib/test_kmod.c | 14 +---
> sound/soc/omap/mcbsp.c | 4 ++--
> 15 files changed, 49 insertions(+), 80 deletions(-)
>
> diff --git a/arch/s390/kernel/topology.c
> b/arch/s390/kernel/topology.c
> index 4d5b65e527b5..4b6e0397f66d 100644
> --- a/arch/s390/kernel/topology.c
> +++ b/arch/s390/kernel/topology.c
> @@ -404,8 +404,7 @@ static ssize_t dispatching_store(struct device
> *dev,
> put_online_cpus();
> return rc ? rc : count;
> }
> -static DEVICE_ATTR(dispatching, 0644, dispatching_show,
> - dispatching_store);
> +static DEVICE_ATTR_RW(dispatching);
>
> static ssize_t cpu_polarization_show(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> diff --git a/arch/tile/kernel/sysfs.c b/arch/tile/kernel/sysfs.c
> index 825867c53853..af5024f0fb5a 100644
> --- a/arch/tile/kernel/sysfs.c
> +++ b/arch/tile/kernel/sysfs.c
> @@ -184,7 +184,7 @@ static ssize_t hv_stats_store(struct device *dev,
> return n < 0 ? n : count;
> }
>
> -static DEVICE_ATTR(hv_stats, 0644, hv_stats_show, hv_stats_store);
> +static DEVICE_ATTR_RW(hv_stats);
>
> static int hv_stats_device_add(struct device *dev, struct
> subsys_interface *sif)
> {
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c
> b/drivers/gpu/drm/i915/i915_sysfs.c
> index c74a20b80182..1d0ab8ff5915 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -447,9 +447,9 @@ static ssize_t gt_min_freq_mhz_store(struct
> device *kdev,
>
> static DEVICE_ATTR(gt_act_freq_mhz, S_IRUGO, gt_act_freq_mhz_show,
> NULL);
> static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_cur_freq_mhz_show,
> NULL);
> -static DEVICE_ATTR(gt_boost_freq_mhz, S_IRUGO | S_IWUSR,
> gt_boost_freq_mhz_show, gt_boost_freq_mhz_store);
> -static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR,
> gt_max_freq_mhz_show, gt_max_freq_mhz_store);
> -static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR,
> gt_min_freq_mhz_show, gt_min_freq_mhz_store);
> +static DEVICE_ATTR_RW(gt_boost_freq_mhz);
> +static DEVICE_ATTR_RW(gt_max_freq_mhz);
> +static DEVICE_ATTR_RW(gt_min_freq_mhz);
>
> static DEVICE_ATTR(vlv_rpe_freq_mhz, S_IRUGO, vlv_rpe_freq_mhz_show,
> NULL);
>
> diff --git a/drivers/platform/x86/compal-laptop.c
> b/drivers/platform/x86/compal-laptop.c
> index 6bcb750e1865..4f9bc72f0584 100644
> --- a/drivers/platform/x86/compal-laptop.c
> +++ b/drivers/platform/x86/compal-laptop.c
> @@ -679,18 +679,12 @@ static int bat_writeable_property(struct
> power_supply *psy,
> /* == */
> /* Driver Globals */
> /* == */
> -static DEVICE_ATTR(wake_up_pme,
> - 0644, wake_up_pme_show, wake_up_pme_s
> tore);
> -static DEVICE_ATTR(wake_up_modem,
> - 0644, wake_up_modem_show, wake_up_modem_store
> );
> -static DEVICE_ATTR(wake_up_lan,
> - 0644, wake_up_lan_show, wake_up_lan_store);
> -static DEVICE_ATTR(wake_up_wlan,
> - 0644, wake_up_wlan_show,wake_up_wlan_store);
> -static DEVICE_ATTR(wake_up_key,
> - 0644, wake_up_key_show, wake_up_key_store);
> -static DEVICE_ATTR(wake_up_mouse,
> - 0644, wake_up_mouse_show, wake_up_mouse_store
> );
> +static DEVICE_ATTR_RW(wake_up_pme);
> +static DEVICE_ATTR_RW(wake_up_modem);
> +static DEVICE_ATTR_RW(wake_up_lan);
> +static DEVICE_A
Re: [Intel-gfx] [PATCH v1] ACPI: Switch to use generic UUID API
On Thu, 2017-05-04 at 12:21 +0300, Andy Shevchenko wrote:
> acpi_evaluate_dsm() and friends take a pointer to a raw buffer of 16
> bytes. Instead we convert them to use uuid_le type. At the same time
> we
> convert current users.
>
> acpi_str_to_uuid() becomes useless after the conversion and it's safe
> to
> get rid of it.
>
> The conversion fixes a potential bug in int340x_thermal as well since
> we have to use memcmp() on binary data.
>
> Cc: Rafael J. Wysocki
> Cc: Mika Westerberg
> Cc: Borislav Petkov
> Cc: Dan Williams
> Cc: Amir Goldstein
> Cc: Jarkko Sakkinen
> Cc: Jani Nikula
> Cc: Ben Skeggs
> Cc: Benjamin Tissoires
> Cc: Joerg Roedel
> Cc: Adrian Hunter
> Cc: Yisen Zhuang
> Cc: Bjorn Helgaas
> Cc: Zhang Rui
> Cc: Felipe Balbi
> Cc: Mathias Nyman
> Cc: Heikki Krogerus
> Cc: Liam Girdwood
> Cc: Mark Brown
> Signed-off-by: Andy Shevchenko
> ---
>
> diff --git a/drivers/thermal/int340x_thermal/int3400_thermal.c
> b/drivers/thermal/int340x_thermal/int3400_thermal.c
> index 9413c4abf0b9..c0eb3bb19b23 100644
> --- a/drivers/thermal/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/int340x_thermal/int3400_thermal.c
> @@ -23,7 +23,7 @@ enum int3400_thermal_uuid {
> INT3400_THERMAL_MAXIMUM_UUID,
> };
>
> -static u8 *int3400_thermal_uuids[INT3400_THERMAL_MAXIMUM_UUID] = {
> +static const char
> *int3400_thermal_uuids[INT3400_THERMAL_MAXIMUM_UUID] = {
> "42A441D6-AE6A-462b-A84B-4A8CE79027D3",
> "3A95C389-E4B8-4629-A526-C52C88626BAE",
> "97C68AE7-15FA-499c-B8C9-5DA81D606E0A",
> @@ -141,10 +141,10 @@ static int int3400_thermal_get_uuids(struct
> int3400_thermal_priv *priv)
> }
>
> for (j = 0; j < INT3400_THERMAL_MAXIMUM_UUID; j++) {
> - u8 uuid[16];
> + uuid_le u;
>
> - acpi_str_to_uuid(int3400_thermal_uuids[j],
> uuid);
> - if (!strncmp(uuid, objb->buffer.pointer,
> 16)) {
> + uuid_le_to_bin(int3400_thermal_uuids[j],
> &u);
> + if (!uuid_le_cmp(*(uuid_le *)objb-
> >buffer.pointer), u) {
> priv->uuid_bitmap |= (1 << j);
> break;
> }
thanks for the fix.
Acked-by: Zhang Rui
-rui
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V2 1/3] PM: Introduce suspend state PM_SUSPEND_FREEZE
On Mon, 2013-02-04 at 15:10 +0800, Zhang Rui wrote: > PM_SUSPEND_FREEZE state is a general state that > does not need any platform specific support, it equals > frozen processes + suspended devices + idle processors. > > Compared with PM_SUSPEND_MEMORY, > PM_SUSPEND_FREEZE saves less power > because the system is still in a running state. > PM_SUSPEND_FREEZE has less resume latency because it does not > touch BIOS, and the processors are in idle state. > > Compared with RTPM/idle, > PM_SUSPEND_FREEZE saves more power as > 1. the processor has longer sleep time because processes are frozen. >The deeper c-state the processor supports, more power saving we can get. > 2. PM_SUSPEND_FREEZE uses system suspend code path, thus we can get >more power saving from the devices that does not have good RTPM support. > > This state is useful for > 1) platforms that do not have STR, or have a broken STR. > 2) platforms that have an extremely low power idle state, >which can be used to replace STR. > > The following describes how PM_SUSPEND_FREEZE state works. > 1. echo freeze > /sys/power/state > 2. the processes are frozen. > 3. all the devices are suspended. > 4. all the processors are blocked by a wait queue > 5. all the processors idles and enters (Deep) c-state. > 6. an interrupt fires. > 7. a processor is woken up and handles the irq. > 8. if it is a general event, >a) the irq handler runs and quites. >b) goto step 4. > 9. if it is a real wake event, say, power button pressing, keyboard touch, > mouse moving, >a) the irq handler runs and activate the wakeup source >b) wakeup_source_activate() notifies the wait queue. >c) system starts resuming from PM_SUSPEND_FREEZE > 10. all the devices are resumed. > 11. all the processes are unfrozen. > 12. system is back to working. > > Known Issue: > The wakeup of this new PM_SUSPEND_FREEZE state may behave differently > from the previous suspend state. > Take ACPI platform for example, there are some GPEs that only enabled > when the system is in sleep state, to wake the system backk from S3/S4. > But we are not touching these GPEs during transition to PM_SUSPEND_FREEZE. > This means we may lose some wake event. > But on the other hand, as we do not disable all the Interrupts during > PM_SUSPEND_FREEZE, we may get some extra "wakeup" Interrupts, that are > not available for S3/S4. > > The patches has been tested on an old Sony laptop, and here are the results: > > Average Power: > 1. RPTM/idle for half an hour: >14.8W, 12.6W, 14.1W, 12.5W, 14.4W, 13.2W, 12.9W > 2. Freeze for half an hour: >11W, 10.4W, 9.4W, 11.3W 10.5W > 3. RTPM/idle for three hours: >11.6W > 4. Freeze for three hours: >10W > 5. Suspend to Memory: >0.5~0.9W > > Average Resume Latency: > 1. RTPM/idle with a black screen: (From pressing keyboard to screen back) >Less than 0.2s > 2. Freeze: (From pressing power button to screen back) >2.50s > 3. Suspend to Memory: (From pressing power button to screen back) >4.33s > > From the results, we can see that all the platforms should benefit from > this patch, even if it does not have Low Power S0. > fixed a build error when CONFIG_SUSPEND not set. refreshed patch attached. >From e784933534b8fc00b4e7b52f3c34ea9e614e513e Mon Sep 17 00:00:00 2001 From: Zhang Rui Date: Mon, 14 Jan 2013 11:12:53 +0800 Subject: [PATCH 1/3] PM: Introduce suspend state PM_SUSPEND_FREEZE PM_SUSPEND_FREEZE state is a general state that does not need any platform specific support, it equals frozen processes + suspended devices + idle processors. Compared with PM_SUSPEND_MEMORY, PM_SUSPEND_FREEZE saves less power because the system is still in a running state. PM_SUSPEND_FREEZE has less resume latency because it does not touch BIOS, and the processors are in idle state. Compared with RTPM/idle, PM_SUSPEND_FREEZE saves more power as 1. the processor has longer sleep time because processes are frozen. The deeper c-state the processor supports, more power saving we can get. 2. PM_SUSPEND_FREEZE uses system suspend code path, thus we can get more power saving from the devices that does not have good RTPM support. This state is useful for 1) platforms that do not have STR, or have a broken STR. 2) platforms that have an extremely low power idle state, which can be used to replace STR. The following describes how PM_SUSPEND_FREEZE state works. 1. echo freeze > /sys/power/state 2. the processes are frozen. 3. all the devices are suspended. 4. all the processors are blocked by a wait queue 5. all the processors idles and enters (Deep) c-state. 6. an interrupt fires. 7. a processor is woken up and handles the irq. 8. if it is a general event, a) the irq handler
Re: [Intel-gfx] [PATCH V2 3/3] i915: ignore lid open event when resuming
On Tue, 2013-02-05 at 11:14 +0100, Rafael J. Wysocki wrote: > On Tuesday, February 05, 2013 11:07:11 AM Daniel Vetter wrote: > > On Tue, Feb 05, 2013 at 03:41:53PM +0800, Zhang Rui wrote: > > > oops, forgot to update the changelog and comments. > > > refreshed patch attached. > > > > > > From b584fcebb6d715a317f192c88606b875ee88ce78 Mon Sep 17 00:00:00 2001 > > > From: Zhang Rui > > > Date: Thu, 24 Jan 2013 13:27:22 +0800 > > > Subject: [PATCH V3 3/3] i915: ignore lid open event when resuming > > > > > > i915 driver needs to do modeset when > > > 1. system resumes from sleep > > > 2. lid is opened > > > > Patch applied, thanks. There's been a bit of a merge conflict and one tiny > > checkpatch error, both fixed while applying. I plan to push this patch to > > drm-next for 3.9. > > Thanks Daniel! > > I will take the other patches from the series, then. > great, thank you, Rafael. -rui > Rafael > > > > > In PM_SUSPEND_MEM state, all the GPEs are cleared when system resumes, > > > thus it is the i915_resume code does the modeset rather than > > > intel_lid_notify(). > > > > > > But in PM_SUSPEND_FREEZE state, this will be broken because > > > system is still responsive to the lid events. > > > 1. When we close the lid in Freeze state, intel_lid_notify() sets > > > modeset_on_lid. > > > 2. When we reopen the lid, intel_lid_notify() will do a modeset, > > >before the system is resumed. > > > here is the error log, > > > > > > [92146.548074] WARNING: at drivers/gpu/drm/i915/intel_display.c:1028 > > > intel_wait_for_pipe_off+0x184/0x190 [i915]() > > > [92146.548076] Hardware name: VGN-Z540N > > > [92146.548078] pipe_off wait timed out > > > [92146.548167] Modules linked in: hid_generic usbhid hid > > > snd_hda_codec_realtek snd_hda_intel snd_hda_codec parport_pc snd_hwdep > > > ppdev snd_pcm_oss i915 snd_mixer_oss snd_pcm arc4 iwldvm snd_seq_dummy > > > mac80211 snd_seq_oss snd_seq_midi fbcon tileblit font bitblit softcursor > > > drm_kms_helper snd_rawmidi snd_seq_midi_event coretemp drm snd_seq kvm > > > btusb bluetooth snd_timer iwlwifi pcmcia tpm_infineon i2c_algo_bit joydev > > > snd_seq_device intel_agp cfg80211 snd intel_gtt yenta_socket pcmcia_rsrc > > > sony_laptop agpgart microcode psmouse tpm_tis serio_raw mxm_wmi soundcore > > > snd_page_alloc tpm acpi_cpufreq lpc_ich pcmcia_core tpm_bios mperf > > > processor lp parport firewire_ohci firewire_core crc_itu_t sdhci_pci > > > sdhci thermal e1000e > > > [92146.548173] Pid: 4304, comm: kworker/0:0 Tainted: GW > > > 3.8.0-rc3-s0i3-v3-test+ #9 > > > [92146.548175] Call Trace: > > > [92146.548189] [] warn_slowpath_common+0x72/0xa0 > > > [92146.548227] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915] > > > [92146.548263] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915] > > > [92146.548270] [] warn_slowpath_fmt+0x33/0x40 > > > [92146.548307] [] intel_wait_for_pipe_off+0x184/0x190 [i915] > > > [92146.548344] [] intel_disable_pipe+0x102/0x190 [i915] > > > [92146.548380] [] ? intel_disable_plane+0x64/0x80 [i915] > > > [92146.548417] [] i9xx_crtc_disable+0xbc/0x150 [i915] > > > [92146.548456] [] intel_crtc_update_dpms+0x5e/0x90 [i915] > > > [92146.548493] [] intel_modeset_setup_hw_state+0x42f/0x8f0 > > > [i915] > > > [92146.548535] [] intel_lid_notify+0x9b/0xc0 [i915] > > > [92146.548543] [] notifier_call_chain+0x43/0x60 > > > [92146.548550] [] __blocking_notifier_call_chain+0x41/0x80 > > > [92146.548556] [] blocking_notifier_call_chain+0x1f/0x30 > > > [92146.548563] [] acpi_lid_send_state+0x78/0xa4 > > > [92146.548569] [] acpi_button_notify+0x3b/0xf1 > > > [92146.548577] [] ? acpi_os_execute+0x17/0x19 > > > [92146.548582] [] ? acpi_ec_sync_query+0xa5/0xbc > > > [92146.548589] [] acpi_device_notify+0x16/0x18 > > > [92146.548595] [] acpi_ev_notify_dispatch+0x38/0x4f > > > [92146.548600] [] acpi_os_execute_deferred+0x20/0x2b > > > [92146.548607] [] process_one_work+0x128/0x3f0 > > > [92146.548613] [] ? common_interrupt+0x33/0x38 > > > [92146.548618] [] ? wake_up_worker+0x30/0x30 > > > [92146.548624] [] ? acpi_os_wait_events_complete+0x1e/0x1e > > > [92146.548629] [] worker_thread+0x119/0x3b0 > > > [92146.548634] [] ? manage_workers+0x240/0x240 > > > [92146.548640] [] kthread+0x94/0xa0 > > > [92146.54864
Re: [Intel-gfx] [PATCH V2 3/3] i915: ignore lid open event when resuming
On Tue, 2013-02-05 at 11:07 +0100, Daniel Vetter wrote: > On Tue, Feb 05, 2013 at 03:41:53PM +0800, Zhang Rui wrote: > > oops, forgot to update the changelog and comments. > > refreshed patch attached. > > > > From b584fcebb6d715a317f192c88606b875ee88ce78 Mon Sep 17 00:00:00 2001 > > From: Zhang Rui > > Date: Thu, 24 Jan 2013 13:27:22 +0800 > > Subject: [PATCH V3 3/3] i915: ignore lid open event when resuming > > > > i915 driver needs to do modeset when > > 1. system resumes from sleep > > 2. lid is opened > > Patch applied, thanks. There's been a bit of a merge conflict and one tiny > checkpatch error, both fixed while applying. I plan to push this patch to > drm-next for 3.9. > -Daniel > great, thanks! -rui > > > > In PM_SUSPEND_MEM state, all the GPEs are cleared when system resumes, > > thus it is the i915_resume code does the modeset rather than > > intel_lid_notify(). > > > > But in PM_SUSPEND_FREEZE state, this will be broken because > > system is still responsive to the lid events. > > 1. When we close the lid in Freeze state, intel_lid_notify() sets > > modeset_on_lid. > > 2. When we reopen the lid, intel_lid_notify() will do a modeset, > >before the system is resumed. > > here is the error log, > > > > [92146.548074] WARNING: at drivers/gpu/drm/i915/intel_display.c:1028 > > intel_wait_for_pipe_off+0x184/0x190 [i915]() > > [92146.548076] Hardware name: VGN-Z540N > > [92146.548078] pipe_off wait timed out > > [92146.548167] Modules linked in: hid_generic usbhid hid > > snd_hda_codec_realtek snd_hda_intel snd_hda_codec parport_pc snd_hwdep > > ppdev snd_pcm_oss i915 snd_mixer_oss snd_pcm arc4 iwldvm snd_seq_dummy > > mac80211 snd_seq_oss snd_seq_midi fbcon tileblit font bitblit softcursor > > drm_kms_helper snd_rawmidi snd_seq_midi_event coretemp drm snd_seq kvm > > btusb bluetooth snd_timer iwlwifi pcmcia tpm_infineon i2c_algo_bit joydev > > snd_seq_device intel_agp cfg80211 snd intel_gtt yenta_socket pcmcia_rsrc > > sony_laptop agpgart microcode psmouse tpm_tis serio_raw mxm_wmi soundcore > > snd_page_alloc tpm acpi_cpufreq lpc_ich pcmcia_core tpm_bios mperf > > processor lp parport firewire_ohci firewire_core crc_itu_t sdhci_pci sdhci > > thermal e1000e > > [92146.548173] Pid: 4304, comm: kworker/0:0 Tainted: GW > > 3.8.0-rc3-s0i3-v3-test+ #9 > > [92146.548175] Call Trace: > > [92146.548189] [] warn_slowpath_common+0x72/0xa0 > > [92146.548227] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915] > > [92146.548263] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915] > > [92146.548270] [] warn_slowpath_fmt+0x33/0x40 > > [92146.548307] [] intel_wait_for_pipe_off+0x184/0x190 [i915] > > [92146.548344] [] intel_disable_pipe+0x102/0x190 [i915] > > [92146.548380] [] ? intel_disable_plane+0x64/0x80 [i915] > > [92146.548417] [] i9xx_crtc_disable+0xbc/0x150 [i915] > > [92146.548456] [] intel_crtc_update_dpms+0x5e/0x90 [i915] > > [92146.548493] [] intel_modeset_setup_hw_state+0x42f/0x8f0 [i915] > > [92146.548535] [] intel_lid_notify+0x9b/0xc0 [i915] > > [92146.548543] [] notifier_call_chain+0x43/0x60 > > [92146.548550] [] __blocking_notifier_call_chain+0x41/0x80 > > [92146.548556] [] blocking_notifier_call_chain+0x1f/0x30 > > [92146.548563] [] acpi_lid_send_state+0x78/0xa4 > > [92146.548569] [] acpi_button_notify+0x3b/0xf1 > > [92146.548577] [] ? acpi_os_execute+0x17/0x19 > > [92146.548582] [] ? acpi_ec_sync_query+0xa5/0xbc > > [92146.548589] [] acpi_device_notify+0x16/0x18 > > [92146.548595] [] acpi_ev_notify_dispatch+0x38/0x4f > > [92146.548600] [] acpi_os_execute_deferred+0x20/0x2b > > [92146.548607] [] process_one_work+0x128/0x3f0 > > [92146.548613] [] ? common_interrupt+0x33/0x38 > > [92146.548618] [] ? wake_up_worker+0x30/0x30 > > [92146.548624] [] ? acpi_os_wait_events_complete+0x1e/0x1e > > [92146.548629] [] worker_thread+0x119/0x3b0 > > [92146.548634] [] ? manage_workers+0x240/0x240 > > [92146.548640] [] kthread+0x94/0xa0 > > [92146.548647] [] ? > > ftrace_raw_output_sched_stat_runtime+0x70/0xf0 > > [92146.548652] [] ret_from_kernel_thread+0x1b/0x28 > > [92146.548658] [] ? kthread_create_on_node+0xc0/0xc0 > > > > three different modeset flags are introduced in this patch > > MODESET_ON_LID_OPEN: do modeset on next lid open event > > MODESET_DONE: modeset already done > > MODESET_SUSPENDED: suspended, only do modeset when system is resumed > > > > In this way, > > 1. when lid is closed, MODESET_ON_LID_OPEN is set so that
Re: [Intel-gfx] [PATCH V2 3/3] i915: ignore lid open event when resuming
On Tue, 2013-02-05 at 07:58 +0800, Zhang Rui wrote: > On Mon, 2013-02-04 at 16:25 +0100, Daniel Vetter wrote: > > On Mon, Feb 04, 2013 at 03:10:11PM +0800, Zhang Rui wrote: > > > i915 driver needs to do modeset when > > > 1. system resumes from sleep > > > 2. lid is opened > > > > > > In PM_SUSPEND_MEM state, all the GPEs are cleared when system resumes, > > > thus it is the i915_resume code does the modeset rather than > > > intel_lid_notify(). > > > > > > But in PM_SUSPEND_FREEZE state, this will be broken because > > > system is still responsive to the lid events. > > > 1. When we close the lid in Freeze state, intel_lid_notify() sets > > > modeset_on_lid. > > > 2. When we reopen the lid, intel_lid_notify() will do a modeset, > > >before the system is resumed. > > > here is the error log, > > > > > > [92146.548074] WARNING: at drivers/gpu/drm/i915/intel_display.c:1028 > > > intel_wait_for_pipe_off+0x184/0x190 [i915]() > > > [92146.548076] Hardware name: VGN-Z540N > > > [92146.548078] pipe_off wait timed out > > > [92146.548167] Modules linked in: hid_generic usbhid hid > > > snd_hda_codec_realtek snd_hda_intel snd_hda_codec parport_pc snd_hwdep > > > ppdev snd_pcm_oss i915 snd_mixer_oss snd_pcm arc4 iwldvm snd_seq_dummy > > > mac80211 snd_seq_oss snd_seq_midi fbcon tileblit font bitblit softcursor > > > drm_kms_helper snd_rawmidi snd_seq_midi_event coretemp drm snd_seq kvm > > > btusb bluetooth snd_timer iwlwifi pcmcia tpm_infineon i2c_algo_bit joydev > > > snd_seq_device intel_agp cfg80211 snd intel_gtt yenta_socket pcmcia_rsrc > > > sony_laptop agpgart microcode psmouse tpm_tis serio_raw mxm_wmi soundcore > > > snd_page_alloc tpm acpi_cpufreq lpc_ich pcmcia_core tpm_bios mperf > > > processor lp parport firewire_ohci firewire_core crc_itu_t sdhci_pci > > > sdhci thermal e1000e > > > [92146.548173] Pid: 4304, comm: kworker/0:0 Tainted: GW > > > 3.8.0-rc3-s0i3-v3-test+ #9 > > > [92146.548175] Call Trace: > > > [92146.548189] [] warn_slowpath_common+0x72/0xa0 > > > [92146.548227] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915] > > > [92146.548263] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915] > > > [92146.548270] [] warn_slowpath_fmt+0x33/0x40 > > > [92146.548307] [] intel_wait_for_pipe_off+0x184/0x190 [i915] > > > [92146.548344] [] intel_disable_pipe+0x102/0x190 [i915] > > > [92146.548380] [] ? intel_disable_plane+0x64/0x80 [i915] > > > [92146.548417] [] i9xx_crtc_disable+0xbc/0x150 [i915] > > > [92146.548456] [] intel_crtc_update_dpms+0x5e/0x90 [i915] > > > [92146.548493] [] intel_modeset_setup_hw_state+0x42f/0x8f0 > > > [i915] > > > [92146.548535] [] intel_lid_notify+0x9b/0xc0 [i915] > > > [92146.548543] [] notifier_call_chain+0x43/0x60 > > > [92146.548550] [] __blocking_notifier_call_chain+0x41/0x80 > > > [92146.548556] [] blocking_notifier_call_chain+0x1f/0x30 > > > [92146.548563] [] acpi_lid_send_state+0x78/0xa4 > > > [92146.548569] [] acpi_button_notify+0x3b/0xf1 > > > [92146.548577] [] ? acpi_os_execute+0x17/0x19 > > > [92146.548582] [] ? acpi_ec_sync_query+0xa5/0xbc > > > [92146.548589] [] acpi_device_notify+0x16/0x18 > > > [92146.548595] [] acpi_ev_notify_dispatch+0x38/0x4f > > > [92146.548600] [] acpi_os_execute_deferred+0x20/0x2b > > > [92146.548607] [] process_one_work+0x128/0x3f0 > > > [92146.548613] [] ? common_interrupt+0x33/0x38 > > > [92146.548618] [] ? wake_up_worker+0x30/0x30 > > > [92146.548624] [] ? acpi_os_wait_events_complete+0x1e/0x1e > > > [92146.548629] [] worker_thread+0x119/0x3b0 > > > [92146.548634] [] ? manage_workers+0x240/0x240 > > > [92146.548640] [] kthread+0x94/0xa0 > > > [92146.548647] [] ? > > > ftrace_raw_output_sched_stat_runtime+0x70/0xf0 > > > [92146.548652] [] ret_from_kernel_thread+0x1b/0x28 > > > [92146.548658] [] ? kthread_create_on_node+0xc0/0xc0 > > > > > > three different modeset flags are introduced in this patch > > > MODESET_ON_LID: do modeset on next lid open event > > > MODESET_DONE: modeset already done > > > MODESET_ON_RESUME: do modeset when system is resumed > > > > > > In this way, > > > 1. when lid is closed, MODESET_ON_LID is set so that > > >we'll do modeset on next lid open event. > > > 2. when lid is opened, MODESET_DONE is set > > >so that duplicate lid open
Re: [Intel-gfx] [PATCH V2 3/3] i915: ignore lid open event when resuming
On Mon, 2013-02-04 at 16:25 +0100, Daniel Vetter wrote: > On Mon, Feb 04, 2013 at 03:10:11PM +0800, Zhang Rui wrote: > > i915 driver needs to do modeset when > > 1. system resumes from sleep > > 2. lid is opened > > > > In PM_SUSPEND_MEM state, all the GPEs are cleared when system resumes, > > thus it is the i915_resume code does the modeset rather than > > intel_lid_notify(). > > > > But in PM_SUSPEND_FREEZE state, this will be broken because > > system is still responsive to the lid events. > > 1. When we close the lid in Freeze state, intel_lid_notify() sets > > modeset_on_lid. > > 2. When we reopen the lid, intel_lid_notify() will do a modeset, > >before the system is resumed. > > here is the error log, > > > > [92146.548074] WARNING: at drivers/gpu/drm/i915/intel_display.c:1028 > > intel_wait_for_pipe_off+0x184/0x190 [i915]() > > [92146.548076] Hardware name: VGN-Z540N > > [92146.548078] pipe_off wait timed out > > [92146.548167] Modules linked in: hid_generic usbhid hid > > snd_hda_codec_realtek snd_hda_intel snd_hda_codec parport_pc snd_hwdep > > ppdev snd_pcm_oss i915 snd_mixer_oss snd_pcm arc4 iwldvm snd_seq_dummy > > mac80211 snd_seq_oss snd_seq_midi fbcon tileblit font bitblit softcursor > > drm_kms_helper snd_rawmidi snd_seq_midi_event coretemp drm snd_seq kvm > > btusb bluetooth snd_timer iwlwifi pcmcia tpm_infineon i2c_algo_bit joydev > > snd_seq_device intel_agp cfg80211 snd intel_gtt yenta_socket pcmcia_rsrc > > sony_laptop agpgart microcode psmouse tpm_tis serio_raw mxm_wmi soundcore > > snd_page_alloc tpm acpi_cpufreq lpc_ich pcmcia_core tpm_bios mperf > > processor lp parport firewire_ohci firewire_core crc_itu_t sdhci_pci sdhci > > thermal e1000e > > [92146.548173] Pid: 4304, comm: kworker/0:0 Tainted: GW > > 3.8.0-rc3-s0i3-v3-test+ #9 > > [92146.548175] Call Trace: > > [92146.548189] [] warn_slowpath_common+0x72/0xa0 > > [92146.548227] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915] > > [92146.548263] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915] > > [92146.548270] [] warn_slowpath_fmt+0x33/0x40 > > [92146.548307] [] intel_wait_for_pipe_off+0x184/0x190 [i915] > > [92146.548344] [] intel_disable_pipe+0x102/0x190 [i915] > > [92146.548380] [] ? intel_disable_plane+0x64/0x80 [i915] > > [92146.548417] [] i9xx_crtc_disable+0xbc/0x150 [i915] > > [92146.548456] [] intel_crtc_update_dpms+0x5e/0x90 [i915] > > [92146.548493] [] intel_modeset_setup_hw_state+0x42f/0x8f0 [i915] > > [92146.548535] [] intel_lid_notify+0x9b/0xc0 [i915] > > [92146.548543] [] notifier_call_chain+0x43/0x60 > > [92146.548550] [] __blocking_notifier_call_chain+0x41/0x80 > > [92146.548556] [] blocking_notifier_call_chain+0x1f/0x30 > > [92146.548563] [] acpi_lid_send_state+0x78/0xa4 > > [92146.548569] [] acpi_button_notify+0x3b/0xf1 > > [92146.548577] [] ? acpi_os_execute+0x17/0x19 > > [92146.548582] [] ? acpi_ec_sync_query+0xa5/0xbc > > [92146.548589] [] acpi_device_notify+0x16/0x18 > > [92146.548595] [] acpi_ev_notify_dispatch+0x38/0x4f > > [92146.548600] [] acpi_os_execute_deferred+0x20/0x2b > > [92146.548607] [] process_one_work+0x128/0x3f0 > > [92146.548613] [] ? common_interrupt+0x33/0x38 > > [92146.548618] [] ? wake_up_worker+0x30/0x30 > > [92146.548624] [] ? acpi_os_wait_events_complete+0x1e/0x1e > > [92146.548629] [] worker_thread+0x119/0x3b0 > > [92146.548634] [] ? manage_workers+0x240/0x240 > > [92146.548640] [] kthread+0x94/0xa0 > > [92146.548647] [] ? > > ftrace_raw_output_sched_stat_runtime+0x70/0xf0 > > [92146.548652] [] ret_from_kernel_thread+0x1b/0x28 > > [92146.548658] [] ? kthread_create_on_node+0xc0/0xc0 > > > > three different modeset flags are introduced in this patch > > MODESET_ON_LID: do modeset on next lid open event > > MODESET_DONE: modeset already done > > MODESET_ON_RESUME: do modeset when system is resumed > > > > In this way, > > 1. when lid is closed, MODESET_ON_LID is set so that > >we'll do modeset on next lid open event. > > 2. when lid is opened, MODESET_DONE is set > >so that duplicate lid open events will be ignored. > > 3. when system suspends, MODESET_ON_RESUME is set. > >In this case, we will not do modeset on any lid events. > > > > Plus, locking mechanism is also introduced to avoid racing. > > > > Signed-off-by: Zhang Rui > > Looks nice, two tiny bikesheds below. > Great, thanks for reviewing. Refreshed patch attached. >From 0d597b4859535c79ed545160cf4af9e6b5970e3c Mon Sep 17 0
[Intel-gfx] [PATCH V2 3/3] i915: ignore lid open event when resuming
i915 driver needs to do modeset when
1. system resumes from sleep
2. lid is opened
In PM_SUSPEND_MEM state, all the GPEs are cleared when system resumes,
thus it is the i915_resume code does the modeset rather than intel_lid_notify().
But in PM_SUSPEND_FREEZE state, this will be broken because
system is still responsive to the lid events.
1. When we close the lid in Freeze state, intel_lid_notify() sets
modeset_on_lid.
2. When we reopen the lid, intel_lid_notify() will do a modeset,
before the system is resumed.
here is the error log,
[92146.548074] WARNING: at drivers/gpu/drm/i915/intel_display.c:1028
intel_wait_for_pipe_off+0x184/0x190 [i915]()
[92146.548076] Hardware name: VGN-Z540N
[92146.548078] pipe_off wait timed out
[92146.548167] Modules linked in: hid_generic usbhid hid snd_hda_codec_realtek
snd_hda_intel snd_hda_codec parport_pc snd_hwdep ppdev snd_pcm_oss i915
snd_mixer_oss snd_pcm arc4 iwldvm snd_seq_dummy mac80211 snd_seq_oss
snd_seq_midi fbcon tileblit font bitblit softcursor drm_kms_helper snd_rawmidi
snd_seq_midi_event coretemp drm snd_seq kvm btusb bluetooth snd_timer iwlwifi
pcmcia tpm_infineon i2c_algo_bit joydev snd_seq_device intel_agp cfg80211 snd
intel_gtt yenta_socket pcmcia_rsrc sony_laptop agpgart microcode psmouse
tpm_tis serio_raw mxm_wmi soundcore snd_page_alloc tpm acpi_cpufreq lpc_ich
pcmcia_core tpm_bios mperf processor lp parport firewire_ohci firewire_core
crc_itu_t sdhci_pci sdhci thermal e1000e
[92146.548173] Pid: 4304, comm: kworker/0:0 Tainted: GW
3.8.0-rc3-s0i3-v3-test+ #9
[92146.548175] Call Trace:
[92146.548189] [] warn_slowpath_common+0x72/0xa0
[92146.548227] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915]
[92146.548263] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915]
[92146.548270] [] warn_slowpath_fmt+0x33/0x40
[92146.548307] [] intel_wait_for_pipe_off+0x184/0x190 [i915]
[92146.548344] [] intel_disable_pipe+0x102/0x190 [i915]
[92146.548380] [] ? intel_disable_plane+0x64/0x80 [i915]
[92146.548417] [] i9xx_crtc_disable+0xbc/0x150 [i915]
[92146.548456] [] intel_crtc_update_dpms+0x5e/0x90 [i915]
[92146.548493] [] intel_modeset_setup_hw_state+0x42f/0x8f0 [i915]
[92146.548535] [] intel_lid_notify+0x9b/0xc0 [i915]
[92146.548543] [] notifier_call_chain+0x43/0x60
[92146.548550] [] __blocking_notifier_call_chain+0x41/0x80
[92146.548556] [] blocking_notifier_call_chain+0x1f/0x30
[92146.548563] [] acpi_lid_send_state+0x78/0xa4
[92146.548569] [] acpi_button_notify+0x3b/0xf1
[92146.548577] [] ? acpi_os_execute+0x17/0x19
[92146.548582] [] ? acpi_ec_sync_query+0xa5/0xbc
[92146.548589] [] acpi_device_notify+0x16/0x18
[92146.548595] [] acpi_ev_notify_dispatch+0x38/0x4f
[92146.548600] [] acpi_os_execute_deferred+0x20/0x2b
[92146.548607] [] process_one_work+0x128/0x3f0
[92146.548613] [] ? common_interrupt+0x33/0x38
[92146.548618] [] ? wake_up_worker+0x30/0x30
[92146.548624] [] ? acpi_os_wait_events_complete+0x1e/0x1e
[92146.548629] [] worker_thread+0x119/0x3b0
[92146.548634] [] ? manage_workers+0x240/0x240
[92146.548640] [] kthread+0x94/0xa0
[92146.548647] [] ? ftrace_raw_output_sched_stat_runtime+0x70/0xf0
[92146.548652] [] ret_from_kernel_thread+0x1b/0x28
[92146.548658] [] ? kthread_create_on_node+0xc0/0xc0
three different modeset flags are introduced in this patch
MODESET_ON_LID: do modeset on next lid open event
MODESET_DONE: modeset already done
MODESET_ON_RESUME: do modeset when system is resumed
In this way,
1. when lid is closed, MODESET_ON_LID is set so that
we'll do modeset on next lid open event.
2. when lid is opened, MODESET_DONE is set
so that duplicate lid open events will be ignored.
3. when system suspends, MODESET_ON_RESUME is set.
In this case, we will not do modeset on any lid events.
Plus, locking mechanism is also introduced to avoid racing.
Signed-off-by: Zhang Rui
---
drivers/gpu/drm/i915/i915_dma.c |1 +
drivers/gpu/drm/i915/i915_drv.c | 14 +-
drivers/gpu/drm/i915/i915_drv.h | 11 +--
drivers/gpu/drm/i915/intel_lvds.c | 33 -
4 files changed, 39 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 99daa89..c7cb546 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1585,6 +1585,7 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
spin_lock_init(&dev_priv->dpio_lock);
mutex_init(&dev_priv->rps.hw_lock);
+ mutex_init(&dev_priv->modeset_lock);
if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
dev_priv->num_pipe = 3;
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1172658..bd7ab5b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -468,6 +468,12 @@ static int i915_drm_freeze(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_
[Intel-gfx] [PATCH V2 2/3] ACPI: enable ACPI SCI during suspend
Enable ACPI SCI during suspend so that SCI can be used
as wake events for PM_SUSPEND_FREEZE.
For S3/S4 transition,
We disable all GPEs in suspend_ops->prepare_late() to
fix a problem that GPEs may trigger SCI before
arch_suspend_disable_irqs() is run.
So it is safe to leave the SCI enabled until
arch_suspend_irq_disable() is run.
Signed-off-by: Zhang Rui
---
drivers/acpi/osl.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 3ff2678..3adeb10 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -787,7 +787,7 @@ acpi_os_install_interrupt_handler(u32 gsi, acpi_osd_handler
handler,
acpi_irq_handler = handler;
acpi_irq_context = context;
- if (request_irq(irq, acpi_irq, IRQF_SHARED, "acpi", acpi_irq)) {
+ if (request_irq(irq, acpi_irq, IRQF_SHARED | IRQF_NO_SUSPEND, "acpi",
acpi_irq)) {
printk(KERN_ERR PREFIX "SCI (IRQ%d) allocation failed\n", irq);
acpi_irq_handler = NULL;
return AE_NOT_ACQUIRED;
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V2 1/3] PM: Introduce suspend state PM_SUSPEND_FREEZE
PM_SUSPEND_FREEZE state is a general state that
does not need any platform specific support, it equals
frozen processes + suspended devices + idle processors.
Compared with PM_SUSPEND_MEMORY,
PM_SUSPEND_FREEZE saves less power
because the system is still in a running state.
PM_SUSPEND_FREEZE has less resume latency because it does not
touch BIOS, and the processors are in idle state.
Compared with RTPM/idle,
PM_SUSPEND_FREEZE saves more power as
1. the processor has longer sleep time because processes are frozen.
The deeper c-state the processor supports, more power saving we can get.
2. PM_SUSPEND_FREEZE uses system suspend code path, thus we can get
more power saving from the devices that does not have good RTPM support.
This state is useful for
1) platforms that do not have STR, or have a broken STR.
2) platforms that have an extremely low power idle state,
which can be used to replace STR.
The following describes how PM_SUSPEND_FREEZE state works.
1. echo freeze > /sys/power/state
2. the processes are frozen.
3. all the devices are suspended.
4. all the processors are blocked by a wait queue
5. all the processors idles and enters (Deep) c-state.
6. an interrupt fires.
7. a processor is woken up and handles the irq.
8. if it is a general event,
a) the irq handler runs and quites.
b) goto step 4.
9. if it is a real wake event, say, power button pressing, keyboard touch,
mouse moving,
a) the irq handler runs and activate the wakeup source
b) wakeup_source_activate() notifies the wait queue.
c) system starts resuming from PM_SUSPEND_FREEZE
10. all the devices are resumed.
11. all the processes are unfrozen.
12. system is back to working.
Known Issue:
The wakeup of this new PM_SUSPEND_FREEZE state may behave differently
from the previous suspend state.
Take ACPI platform for example, there are some GPEs that only enabled
when the system is in sleep state, to wake the system backk from S3/S4.
But we are not touching these GPEs during transition to PM_SUSPEND_FREEZE.
This means we may lose some wake event.
But on the other hand, as we do not disable all the Interrupts during
PM_SUSPEND_FREEZE, we may get some extra "wakeup" Interrupts, that are
not available for S3/S4.
The patches has been tested on an old Sony laptop, and here are the results:
Average Power:
1. RPTM/idle for half an hour:
14.8W, 12.6W, 14.1W, 12.5W, 14.4W, 13.2W, 12.9W
2. Freeze for half an hour:
11W, 10.4W, 9.4W, 11.3W 10.5W
3. RTPM/idle for three hours:
11.6W
4. Freeze for three hours:
10W
5. Suspend to Memory:
0.5~0.9W
Average Resume Latency:
1. RTPM/idle with a black screen: (From pressing keyboard to screen back)
Less than 0.2s
2. Freeze: (From pressing power button to screen back)
2.50s
3. Suspend to Memory: (From pressing power button to screen back)
4.33s
>From the results, we can see that all the platforms should benefit from
this patch, even if it does not have Low Power S0.
Signed-off-by: Zhang Rui
---
drivers/base/power/wakeup.c |6
include/linux/suspend.h |5 +++-
kernel/power/main.c |2 +-
kernel/power/suspend.c | 69 +++
4 files changed, 67 insertions(+), 15 deletions(-)
diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index e6ee5e8..79715e7 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -382,6 +382,12 @@ static void wakeup_source_activate(struct wakeup_source
*ws)
{
unsigned int cec;
+ /*
+* active wakeup source should bring the system
+* out of PM_SUSPEND_FREEZE state
+*/
+ freeze_wake();
+
ws->active = true;
ws->active_count++;
ws->last_time = ktime_get();
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 0c808d7..7420ab5 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -34,8 +34,10 @@ static inline void pm_restore_console(void)
typedef int __bitwise suspend_state_t;
#define PM_SUSPEND_ON ((__force suspend_state_t) 0)
-#define PM_SUSPEND_STANDBY ((__force suspend_state_t) 1)
+#define PM_SUSPEND_FREEZE ((__force suspend_state_t) 1)
+#define PM_SUSPEND_STANDBY ((__force suspend_state_t) 2)
#define PM_SUSPEND_MEM ((__force suspend_state_t) 3)
+#define PM_SUSPEND_MIN PM_SUSPEND_FREEZE
#define PM_SUSPEND_MAX ((__force suspend_state_t) 4)
enum suspend_stat_step {
@@ -192,6 +194,7 @@ struct platform_suspend_ops {
*/
extern void suspend_set_ops(const struct platform_suspend_ops *ops);
extern int suspend_valid_only_mem(suspend_state_t state);
+extern void freeze_wake(void);
/**
* arch_suspend_disable_irqs - disable IRQs for suspend
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 1c16f91..b1c26a9 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -313,7 +313,7 @@ static ssize_t state_show(struct kobject *kobj, struct
kobj
Re: [Intel-gfx] [RFC PATCH 3/3] i915: ignore lid open event when resuming
On Tue, 2013-01-29 at 11:10 +0100, Daniel Vetter wrote: > On Mon, Jan 28, 2013 at 06:06:38PM +0800, Zhang Rui wrote: > > On Mon, 2013-01-28 at 09:31 +0100, Daniel Vetter wrote: > > > On Mon, Jan 28, 2013 at 3:36 AM, Zhang Rui wrote: > > > >> Given that this essentially requires users to manually set this module > > > >> option to make stuff work I don't like this. > > > >> > > > > sorry, I do not understand. > > > > this patch just disables modeset_on_lid during suspend. > > > > > > Pardon me, the misunderstanding is on my side - I've mixed up > > > dev_priv->modeset_on_lid with the corresponding module option. > > > > > > Is my understanding correct that only with the new SCI pm state this > > > can happen, since that still allows acpi events to be processed (and > > > so our lid_notifier being called? > > > > > yep. > > > > > >> I see a few possible options: > > > >> - plug the races between all the different parts - I've never really > > > >> understood why acpi sends us events before the resume code has > > > >> completed ... If that's indeed intentional, we could delay the > > > >> handling a bit until at least the i915 resume code completed. > > > > > > > > IMO, the real question is that, during a suspend/resume cycle, if we > > > > need to take care of the lid open event or not. > > > > To me, the answer is no, because when resuming from STR, i915 is not > > > > aware of such an event, and the i915 resume code works well, right? > > > > so I do not think it is a problem to ignore the lid event for another > > > > lightweight suspend state, which is introduced in Patch 1/3 in this > > > > series. > > > > > > > >> - Judging from the diff context you're not on the latest 3.8-rc > > > >> codebase - we've applied a few fixes to this lid hack recently. Please > > > >> retest on a kernel with > > > >> > > > > the patch is based on 3.8.0-rc3, which already includes the commit > > > > below. > > > > And yes, the problem still exists. > > > > > > Ok, I think I see the issue now. But I suspect that we need some > > > additional locking to make this solid, since currently > > > dev_priv->modeset_on_lid updates can race with our lid notifier > > > handler. Let me think about this a bit more. > > > > hmm, > > with this patch, intel_lid_notify() will return immediately if > > modeset_on_lid is set to -1. So the only problem in my mind is that we > > got a lid open event during i915_suspend(). > > > > Say, > > lid is opened -> i915_lid_notify() is invoked (modeset_on_lid is 1 at > > this time) -> i915_suspend() set modeset_on_lid to -1, just before > > intel_modeset_setup_hw_state() being invoked in i915_lid_notify() -> > > intel_modeset_setup_hw_state() breaks the system. > > > > but my first question would be is this (lid open on suspend) possible in > > real world? > > If the answer is yes, then my second question is that, this problem > > exists for STR as well because SCI is still valid at this time when > > suspending to memory, have we seen such issues for STR before? > > > > Anyway, if the current code does not break STR, this patch should be > > enough for the new suspend state. > > what do you think? > > I think I have two wishlist changes for your patch ;-) > > - Convert dev_priv->modeset_on_lid to an enum, so that we have descriptive > names instead of magic numbers. > sure, will update in next version. > - I think we can close all races by adding a new lid_notifier mutex (I > prefer a new lock instead of overloading an existing one with). If we > hold that in the suspend/resume paths just for changing modeset_on_lid > and also for the entire lid notifier callback (i.e. grab the lock before > first looking at modest_on_lid, only drop it once the modeset fixup has > been completed at the end of the function) we should be race-free in all > cases. > > Also, I think we should move the modeset_on_lid updates in the > suspend/resume paths to the very beginning/end. > > Can you pls update your patch with these changes? Or do you see an > additional race we need to plug somewhere? > yep. will send out V2 soon. thanks for your comments. -rui ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 3/3] i915: ignore lid open event when resuming
On Mon, 2013-01-28 at 09:31 +0100, Daniel Vetter wrote: > On Mon, Jan 28, 2013 at 3:36 AM, Zhang Rui wrote: > >> Given that this essentially requires users to manually set this module > >> option to make stuff work I don't like this. > >> > > sorry, I do not understand. > > this patch just disables modeset_on_lid during suspend. > > Pardon me, the misunderstanding is on my side - I've mixed up > dev_priv->modeset_on_lid with the corresponding module option. > > Is my understanding correct that only with the new SCI pm state this > can happen, since that still allows acpi events to be processed (and > so our lid_notifier being called? > yep. > >> I see a few possible options: > >> - plug the races between all the different parts - I've never really > >> understood why acpi sends us events before the resume code has > >> completed ... If that's indeed intentional, we could delay the > >> handling a bit until at least the i915 resume code completed. > > > > IMO, the real question is that, during a suspend/resume cycle, if we > > need to take care of the lid open event or not. > > To me, the answer is no, because when resuming from STR, i915 is not > > aware of such an event, and the i915 resume code works well, right? > > so I do not think it is a problem to ignore the lid event for another > > lightweight suspend state, which is introduced in Patch 1/3 in this > > series. > > > >> - Judging from the diff context you're not on the latest 3.8-rc > >> codebase - we've applied a few fixes to this lid hack recently. Please > >> retest on a kernel with > >> > > the patch is based on 3.8.0-rc3, which already includes the commit > > below. > > And yes, the problem still exists. > > Ok, I think I see the issue now. But I suspect that we need some > additional locking to make this solid, since currently > dev_priv->modeset_on_lid updates can race with our lid notifier > handler. Let me think about this a bit more. hmm, with this patch, intel_lid_notify() will return immediately if modeset_on_lid is set to -1. So the only problem in my mind is that we got a lid open event during i915_suspend(). Say, lid is opened -> i915_lid_notify() is invoked (modeset_on_lid is 1 at this time) -> i915_suspend() set modeset_on_lid to -1, just before intel_modeset_setup_hw_state() being invoked in i915_lid_notify() -> intel_modeset_setup_hw_state() breaks the system. but my first question would be is this (lid open on suspend) possible in real world? If the answer is yes, then my second question is that, this problem exists for STR as well because SCI is still valid at this time when suspending to memory, have we seen such issues for STR before? Anyway, if the current code does not break STR, this patch should be enough for the new suspend state. what do you think? thanks, rui ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 3/3] i915: ignore lid open event when resuming
On Sun, 2013-01-27 at 16:45 +0100, Daniel Vetter wrote: > On Sun, Jan 27, 2013 at 4:21 PM, Zhang Rui wrote: > > i915 driver needs to do modeset when > > 1. system resumes from sleep > > 2. lid is opened > > > > In PM_SUSPEND_MEM state, all the GPEs are cleared when system resumes, > > thus it is the i915_resume code does the modeset rather than > > intel_lid_notify(). > > > > In PM_SUSPEND_FREEZE state, system is still resposive to the lid events. > > 1. When we close the lid in Freeze state, intel_lid_notify() sets > > modeset_on_lid. > > 2. When we reopen the lid, intel_lid_notify() will do a modeset, > >before the system is resumed. > > > > here is the error log, > > > > [92146.548074] WARNING: at drivers/gpu/drm/i915/intel_display.c:1028 > > intel_wait_for_pipe_off+0x184/0x190 [i915]() > > [92146.548076] Hardware name: VGN-Z540N > > [92146.548078] pipe_off wait timed out > > [92146.548167] Modules linked in: hid_generic usbhid hid > > snd_hda_codec_realtek snd_hda_intel snd_hda_codec parport_pc snd_hwdep > > ppdev snd_pcm_oss i915 snd_mixer_oss snd_pcm arc4 iwldvm snd_seq_dummy > > mac80211 snd_seq_oss snd_seq_midi fbcon tileblit font bitblit softcursor > > drm_kms_helper snd_rawmidi snd_seq_midi_event coretemp drm snd_seq kvm > > btusb bluetooth snd_timer iwlwifi pcmcia tpm_infineon i2c_algo_bit joydev > > snd_seq_device intel_agp cfg80211 snd intel_gtt yenta_socket pcmcia_rsrc > > sony_laptop agpgart microcode psmouse tpm_tis serio_raw mxm_wmi soundcore > > snd_page_alloc tpm acpi_cpufreq lpc_ich pcmcia_core tpm_bios mperf > > processor lp parport firewire_ohci firewire_core crc_itu_t sdhci_pci sdhci > > thermal e1000e > > [92146.548173] Pid: 4304, comm: kworker/0:0 Tainted: GW > > 3.8.0-rc3-s0i3-v3-test+ #9 > > [92146.548175] Call Trace: > > [92146.548189] [] warn_slowpath_common+0x72/0xa0 > > [92146.548227] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915] > > [92146.548263] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915] > > [92146.548270] [] warn_slowpath_fmt+0x33/0x40 > > [92146.548307] [] intel_wait_for_pipe_off+0x184/0x190 [i915] > > [92146.548344] [] intel_disable_pipe+0x102/0x190 [i915] > > [92146.548380] [] ? intel_disable_plane+0x64/0x80 [i915] > > [92146.548417] [] i9xx_crtc_disable+0xbc/0x150 [i915] > > [92146.548456] [] intel_crtc_update_dpms+0x5e/0x90 [i915] > > [92146.548493] [] intel_modeset_setup_hw_state+0x42f/0x8f0 [i915] > > [92146.548535] [] intel_lid_notify+0x9b/0xc0 [i915] > > [92146.548543] [] notifier_call_chain+0x43/0x60 > > [92146.548550] [] __blocking_notifier_call_chain+0x41/0x80 > > [92146.548556] [] blocking_notifier_call_chain+0x1f/0x30 > > [92146.548563] [] acpi_lid_send_state+0x78/0xa4 > > [92146.548569] [] acpi_button_notify+0x3b/0xf1 > > [92146.548577] [] ? acpi_os_execute+0x17/0x19 > > [92146.548582] [] ? acpi_ec_sync_query+0xa5/0xbc > > [92146.548589] [] acpi_device_notify+0x16/0x18 > > [92146.548595] [] acpi_ev_notify_dispatch+0x38/0x4f > > [92146.548600] [] acpi_os_execute_deferred+0x20/0x2b > > [92146.548607] [] process_one_work+0x128/0x3f0 > > [92146.548613] [] ? common_interrupt+0x33/0x38 > > [92146.548618] [] ? wake_up_worker+0x30/0x30 > > [92146.548624] [] ? acpi_os_wait_events_complete+0x1e/0x1e > > [92146.548629] [] worker_thread+0x119/0x3b0 > > [92146.548634] [] ? manage_workers+0x240/0x240 > > [92146.548640] [] kthread+0x94/0xa0 > > [92146.548647] [] ? > > ftrace_raw_output_sched_stat_runtime+0x70/0xf0 > > [92146.548652] [] ret_from_kernel_thread+0x1b/0x28 > > [92146.548658] [] ? kthread_create_on_node+0xc0/0xc0 > > > > so I'd like to use tristate for modeset_on_lid instead of bool. > > -1: ingore all the lid events. > > 0: do not do modeset when there is a lid-open event. > > 1: do modeset when there is a lid-open event. > > In this way, only device resume code will do modeset in a suspend/resume > > cycle. > > > > Signed-off-by: Zhang Rui > > Given that this essentially requires users to manually set this module > option to make stuff work I don't like this. > sorry, I do not understand. this patch just disables modeset_on_lid during suspend. > I see a few possible options: > - plug the races between all the different parts - I've never really > understood why acpi sends us events before the resume code has > completed ... If that's indeed intentional, we could delay the > handling a bit until at least the i915 resume code completed. IMO, the real question is that, during a suspend/resu
[Intel-gfx] [RFC PATCH 3/3] i915: ignore lid open event when resuming
i915 driver needs to do modeset when
1. system resumes from sleep
2. lid is opened
In PM_SUSPEND_MEM state, all the GPEs are cleared when system resumes,
thus it is the i915_resume code does the modeset rather than intel_lid_notify().
In PM_SUSPEND_FREEZE state, system is still resposive to the lid events.
1. When we close the lid in Freeze state, intel_lid_notify() sets
modeset_on_lid.
2. When we reopen the lid, intel_lid_notify() will do a modeset,
before the system is resumed.
here is the error log,
[92146.548074] WARNING: at drivers/gpu/drm/i915/intel_display.c:1028
intel_wait_for_pipe_off+0x184/0x190 [i915]()
[92146.548076] Hardware name: VGN-Z540N
[92146.548078] pipe_off wait timed out
[92146.548167] Modules linked in: hid_generic usbhid hid snd_hda_codec_realtek
snd_hda_intel snd_hda_codec parport_pc snd_hwdep ppdev snd_pcm_oss i915
snd_mixer_oss snd_pcm arc4 iwldvm snd_seq_dummy mac80211 snd_seq_oss
snd_seq_midi fbcon tileblit font bitblit softcursor drm_kms_helper snd_rawmidi
snd_seq_midi_event coretemp drm snd_seq kvm btusb bluetooth snd_timer iwlwifi
pcmcia tpm_infineon i2c_algo_bit joydev snd_seq_device intel_agp cfg80211 snd
intel_gtt yenta_socket pcmcia_rsrc sony_laptop agpgart microcode psmouse
tpm_tis serio_raw mxm_wmi soundcore snd_page_alloc tpm acpi_cpufreq lpc_ich
pcmcia_core tpm_bios mperf processor lp parport firewire_ohci firewire_core
crc_itu_t sdhci_pci sdhci thermal e1000e
[92146.548173] Pid: 4304, comm: kworker/0:0 Tainted: GW
3.8.0-rc3-s0i3-v3-test+ #9
[92146.548175] Call Trace:
[92146.548189] [] warn_slowpath_common+0x72/0xa0
[92146.548227] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915]
[92146.548263] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915]
[92146.548270] [] warn_slowpath_fmt+0x33/0x40
[92146.548307] [] intel_wait_for_pipe_off+0x184/0x190 [i915]
[92146.548344] [] intel_disable_pipe+0x102/0x190 [i915]
[92146.548380] [] ? intel_disable_plane+0x64/0x80 [i915]
[92146.548417] [] i9xx_crtc_disable+0xbc/0x150 [i915]
[92146.548456] [] intel_crtc_update_dpms+0x5e/0x90 [i915]
[92146.548493] [] intel_modeset_setup_hw_state+0x42f/0x8f0 [i915]
[92146.548535] [] intel_lid_notify+0x9b/0xc0 [i915]
[92146.548543] [] notifier_call_chain+0x43/0x60
[92146.548550] [] __blocking_notifier_call_chain+0x41/0x80
[92146.548556] [] blocking_notifier_call_chain+0x1f/0x30
[92146.548563] [] acpi_lid_send_state+0x78/0xa4
[92146.548569] [] acpi_button_notify+0x3b/0xf1
[92146.548577] [] ? acpi_os_execute+0x17/0x19
[92146.548582] [] ? acpi_ec_sync_query+0xa5/0xbc
[92146.548589] [] acpi_device_notify+0x16/0x18
[92146.548595] [] acpi_ev_notify_dispatch+0x38/0x4f
[92146.548600] [] acpi_os_execute_deferred+0x20/0x2b
[92146.548607] [] process_one_work+0x128/0x3f0
[92146.548613] [] ? common_interrupt+0x33/0x38
[92146.548618] [] ? wake_up_worker+0x30/0x30
[92146.548624] [] ? acpi_os_wait_events_complete+0x1e/0x1e
[92146.548629] [] worker_thread+0x119/0x3b0
[92146.548634] [] ? manage_workers+0x240/0x240
[92146.548640] [] kthread+0x94/0xa0
[92146.548647] [] ? ftrace_raw_output_sched_stat_runtime+0x70/0xf0
[92146.548652] [] ret_from_kernel_thread+0x1b/0x28
[92146.548658] [] ? kthread_create_on_node+0xc0/0xc0
so I'd like to use tristate for modeset_on_lid instead of bool.
-1: ingore all the lid events.
0: do not do modeset when there is a lid-open event.
1: do modeset when there is a lid-open event.
In this way, only device resume code will do modeset in a suspend/resume cycle.
Signed-off-by: Zhang Rui
---
drivers/gpu/drm/i915/i915_drv.c |4 ++--
drivers/gpu/drm/i915/i915_drv.h |2 +-
drivers/gpu/drm/i915/intel_lvds.c |2 ++
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1172658..d7613cb 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -492,8 +492,8 @@ static int i915_drm_freeze(struct drm_device *dev)
intel_opregion_fini(dev);
- /* Modeset on resume, not lid events */
- dev_priv->modeset_on_lid = 0;
+ /* Modeset on resume, ignore lid events */
+ dev_priv->modeset_on_lid = -1;
console_lock();
intel_fbdev_set_suspend(dev, 1);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ed30595..3fec498 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -748,7 +748,7 @@ typedef struct drm_i915_private {
unsigned long quirks;
/* Register state */
- bool modeset_on_lid;
+ int modeset_on_lid; /* -1: invalid, 0: no modeset, 1: do modeset */
struct {
/** Bridge to intel-gtt-ko */
diff --git a/drivers/gpu/drm/i915/intel_lvds.c
b/drivers/gpu/drm/i915/intel_lvds.c
index 17aee74..4ddae6d 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -512,6 +512,8 @@ static int intel_lid_notify(struct notifie
[Intel-gfx] [RFC PATCH 2/3] ACPI: enable ACPI SCI during suspend
Enable ACPI SCI during suspend so that SCI can be used
as wake events for PM_SUSPEND_FREEZE.
For S3/S4 transition,
We disable all GPEs in suspend_ops->prepare_late() to
fix a problem that GPEs may trigger SCI before
arch_suspend_disable_irqs() is run.
So it is safe to leave the SCI enabled until
arch_suspend_irq_disable() is run.
Signed-off-by: Zhang Rui
---
drivers/acpi/osl.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 3ff2678..3adeb10 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -787,7 +787,7 @@ acpi_os_install_interrupt_handler(u32 gsi, acpi_osd_handler
handler,
acpi_irq_handler = handler;
acpi_irq_context = context;
- if (request_irq(irq, acpi_irq, IRQF_SHARED, "acpi", acpi_irq)) {
+ if (request_irq(irq, acpi_irq, IRQF_SHARED | IRQF_NO_SUSPEND, "acpi",
acpi_irq)) {
printk(KERN_ERR PREFIX "SCI (IRQ%d) allocation failed\n", irq);
acpi_irq_handler = NULL;
return AE_NOT_ACQUIRED;
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 1/3] PM: Introduce suspend state PM_SUSPEND_FREEZE
PM_SUSPEND_FREEZE state is a general state that
does not need any platform specific support, it equals
frozen processes + suspended devices + idle processors.
Compared with PM_SUSPEND_MEMORY,
PM_SUSPEND_FREEZE saves less power
because the system is still in a running state.
PM_SUSPEND_FREEZE has less resume latency because it does not
touch BIOS, and the processors are in idle state.
Compared with RTPM/idle,
PM_SUSPEND_FREEZE saves more power as
1. the processor has longer sleep time because processes are frozen.
The deeper c-state the processor supports, more power saving we can get.
2. PM_SUSPEND_FREEZE uses system suspend code path, thus we can get
more power saving from the devices that does not have good RTPM support.
This state is useful for
1) platforms that do not have STR, or have a broken STR.
2) platforms that have an extremely low power idle state,
which can be used to replace STR.
The following describes how PM_SUSPEND_FREEZE state works.
1. echo freeze > /sys/power/state
2. the processes are frozen.
3. all the devices are suspended.
4. all the processors are blocked by a wait queue
5. all the processors idles and enters (Deep) c-state.
6. an interrupt fires.
7. a processor is woken up and handles the irq.
8. if it is a general event,
a) the irq handler runs and quites.
b) goto step 4.
9. if it is a real wake event, say, power button pressing, keyboard touch,
mouse moving,
a) the irq handler runs and activate the wakeup source
b) wakeup_source_activate() notifies the wait queue.
c) system starts resuming from PM_SUSPEND_FREEZE
10. all the devices are resumed.
11. all the processes are unfrozen.
12. system is back to working.
Known Issue:
The wakeup of this new PM_SUSPEND_FREEZE state may behave differently
from the previous suspend state.
Take ACPI platform for example, there are some GPEs that only enabled
when the system is in sleep state, to wake the system backk from S3/S4.
But we are not touching these GPEs during transition to PM_SUSPEND_FREEZE.
This means we may lose some wake event.
But on the other hand, as we do not disable all the Interrupts during
PM_SUSPEND_FREEZE, we may get some extra "wakeup" Interrupts, that are
not available for S3/S4.
The patches has been tested on an old Sony laptop, and here are the results:
Average Power:
1. RPTM/idle for half an hour:
14.8W, 12.6W, 14.1W, 12.5W, 14.4W, 13.2W, 12.9W
2. Freeze for half an hour:
11W, 10.4W, 9.4W, 11.3W 10.5W
3. RTPM/idle for three hours:
11.6W
4. Freeze for three hours:
10W
5. Suspend to Memory:
0.5~0.9W
Average Resume Latency:
1. RTPM/idle with a black screen: (From pressing keyboard to screen back)
Less than 0.2s
2. Freeze: (From pressing power button to screen back)
2.50s
3. Suspend to Memory: (From pressing power button to screen back)
4.33s
>From the results, we can see that all the platforms should benefit from
this patch, even if it does not have Low Power S0.
Signed-off-by: Zhang Rui
---
drivers/base/power/wakeup.c |6
include/linux/suspend.h |5 +++-
kernel/power/main.c |2 +-
kernel/power/suspend.c | 69 +++
4 files changed, 67 insertions(+), 15 deletions(-)
diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index e6ee5e8..79715e7 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -382,6 +382,12 @@ static void wakeup_source_activate(struct wakeup_source
*ws)
{
unsigned int cec;
+ /*
+* active wakeup source should bring the system
+* out of PM_SUSPEND_FREEZE state
+*/
+ freeze_wake();
+
ws->active = true;
ws->active_count++;
ws->last_time = ktime_get();
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 0c808d7..7420ab5 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -34,8 +34,10 @@ static inline void pm_restore_console(void)
typedef int __bitwise suspend_state_t;
#define PM_SUSPEND_ON ((__force suspend_state_t) 0)
-#define PM_SUSPEND_STANDBY ((__force suspend_state_t) 1)
+#define PM_SUSPEND_FREEZE ((__force suspend_state_t) 1)
+#define PM_SUSPEND_STANDBY ((__force suspend_state_t) 2)
#define PM_SUSPEND_MEM ((__force suspend_state_t) 3)
+#define PM_SUSPEND_MIN PM_SUSPEND_FREEZE
#define PM_SUSPEND_MAX ((__force suspend_state_t) 4)
enum suspend_stat_step {
@@ -192,6 +194,7 @@ struct platform_suspend_ops {
*/
extern void suspend_set_ops(const struct platform_suspend_ops *ops);
extern int suspend_valid_only_mem(suspend_state_t state);
+extern void freeze_wake(void);
/**
* arch_suspend_disable_irqs - disable IRQs for suspend
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 1c16f91..b1c26a9 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -313,7 +313,7 @@ static ssize_t state_show(struct kobject *kobj, struct
kobj
Re: [Intel-gfx] [PATCH V3 3/3] i915: introduce modeset_on_resume
On Fri, 2013-01-25 at 09:59 +0800, Zhenyu Wang wrote: > On 2013.01.24 19:49:40 +0800, Zhang Rui wrote: > > I need some graphics experts' comments before sending it out. > > > > Please send to intel-gfx@lists.freedesktop.org for i915 specific issue. > I want to get some internal feedback before sending it out. thanks, rui > > On Thu, 2013-01-24 at 19:43 +0800, Zhang Rui wrote: > > > i915 driver needs to do modeset when > > > 1. system resumes from sleep > > > 2. lid is opened > > > > > > When resuming from PM_SUSPEND_MEM state, all the GPEs are cleared, > > > thus it is the i915_resume code does the modeset rather than > > > intel_lid_notify(). > > > > > > But when in FREEZE state, system is still resposive to the lid events. > > > 1. i915_suspend() clears modeset_on_lid. > > > 2. When we close the lid, intel_lid_notify() sets modeset_on_lid. > > > 3. When we reopen the lid, intel_lid_notify() will do a modeset, > > >before the system is resumed. > > > > > > here is the error log I got, > > > > > > [92146.548074] WARNING: at drivers/gpu/drm/i915/intel_display.c:1028 > > > intel_wait_for_pipe_off+0x184/0x190 [i915]() > > > [92146.548076] Hardware name: VGN-Z540N > > > [92146.548078] pipe_off wait timed out > > > [92146.548167] Modules linked in: hid_generic usbhid hid > > > snd_hda_codec_realtek snd_hda_intel snd_hda_codec parport_pc snd_hwdep > > > ppdev snd_pcm_oss i915 snd_mixer_oss snd_pcm arc4 iwldvm snd_seq_dummy > > > mac80211 snd_seq_oss snd_seq_midi fbcon tileblit font bitblit softcursor > > > drm_kms_helper snd_rawmidi snd_seq_midi_event coretemp drm snd_seq kvm > > > btusb bluetooth snd_timer iwlwifi pcmcia tpm_infineon i2c_algo_bit joydev > > > snd_seq_device intel_agp cfg80211 snd intel_gtt yenta_socket pcmcia_rsrc > > > sony_laptop agpgart microcode psmouse tpm_tis serio_raw mxm_wmi soundcore > > > snd_page_alloc tpm acpi_cpufreq lpc_ich pcmcia_core tpm_bios mperf > > > processor lp parport firewire_ohci firewire_core crc_itu_t sdhci_pci > > > sdhci thermal e1000e > > > [92146.548173] Pid: 4304, comm: kworker/0:0 Tainted: GW > > > 3.8.0-rc3-s0i3-v3-test+ #9 > > > [92146.548175] Call Trace: > > > [92146.548189] [] warn_slowpath_common+0x72/0xa0 > > > [92146.548227] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915] > > > [92146.548263] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915] > > > [92146.548270] [] warn_slowpath_fmt+0x33/0x40 > > > [92146.548307] [] intel_wait_for_pipe_off+0x184/0x190 [i915] > > > [92146.548344] [] intel_disable_pipe+0x102/0x190 [i915] > > > [92146.548380] [] ? intel_disable_plane+0x64/0x80 [i915] > > > [92146.548417] [] i9xx_crtc_disable+0xbc/0x150 [i915] > > > [92146.548456] [] intel_crtc_update_dpms+0x5e/0x90 [i915] > > > [92146.548493] [] intel_modeset_setup_hw_state+0x42f/0x8f0 > > > [i915] > > > [92146.548535] [] intel_lid_notify+0x9b/0xc0 [i915] > > > [92146.548543] [] notifier_call_chain+0x43/0x60 > > > [92146.548550] [] __blocking_notifier_call_chain+0x41/0x80 > > > [92146.548556] [] blocking_notifier_call_chain+0x1f/0x30 > > > [92146.548563] [] acpi_lid_send_state+0x78/0xa4 > > > [92146.548569] [] acpi_button_notify+0x3b/0xf1 > > > [92146.548577] [] ? acpi_os_execute+0x17/0x19 > > > [92146.548582] [] ? acpi_ec_sync_query+0xa5/0xbc > > > [92146.548589] [] acpi_device_notify+0x16/0x18 > > > [92146.548595] [] acpi_ev_notify_dispatch+0x38/0x4f > > > [92146.548600] [] acpi_os_execute_deferred+0x20/0x2b > > > [92146.548607] [] process_one_work+0x128/0x3f0 > > > [92146.548613] [] ? common_interrupt+0x33/0x38 > > > [92146.548618] [] ? wake_up_worker+0x30/0x30 > > > [92146.548624] [] ? acpi_os_wait_events_complete+0x1e/0x1e > > > [92146.548629] [] worker_thread+0x119/0x3b0 > > > [92146.548634] [] ? manage_workers+0x240/0x240 > > > [92146.548640] [] kthread+0x94/0xa0 > > > [92146.548647] [] ? > > > ftrace_raw_output_sched_stat_runtime+0x70/0xf0 > > > [92146.548652] [] ret_from_kernel_thread+0x1b/0x28 > > > [92146.548658] [] ? kthread_create_on_node+0xc0/0xc0 > > > > > > Introduce a new flag modeset_on_resume to ignore the lid events during > > > suspend. > > > > > > Signed-off-by: Zhang Rui > > > --- > > > drivers/gpu/drm/i915/i915_drv.c |2 ++ > > > drivers/gpu/drm/i915/i915_drv.h |1
