[Intel-gfx] [PATCH v2] drm/i915: fix long-standing SNB regression in power consumption after resume
This patch fixes regression in power consumtion of sandy bridge gpu, which
exists since v3.6 Sometimes after resuming from s2ram gpu starts thinking that
it's extremely busy. After that it never reaches rc6 state.
Bug exists since kernel v3.6, commit b4ae3f22d238617ca11610b29fde16cf8c0bc6e0
(drm/i915: load boot context at driver init time).
For some reason RC6 is already enabled at the beginning of resuming process.
Following initliaztion breaks some internal state and confuses RPS engine.
This patch disables RC6 at the beginnig of resume and initialization.
I've rearranged initialization sequence, because intel_disable_gt_powersave()
needs initialized force_wake_get/put and some locks from the dev_priv.
References: https://bugs.freedesktop.org/show_bug.cgi?id=54089
References: https://bugzilla.kernel.org/show_bug.cgi?id=58971
Signed-off-by: Konstantin Khlebnikov khlebni...@openvz.org
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/i915_dma.c | 16
drivers/gpu/drm/i915/intel_pm.c |5 +++--
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 3b315ba..d1ee611 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1511,6 +1511,13 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
dev_priv-dev = dev;
dev_priv-info = info;
+ spin_lock_init(dev_priv-irq_lock);
+ spin_lock_init(dev_priv-gpu_error.lock);
+ spin_lock_init(dev_priv-rps.lock);
+ mutex_init(dev_priv-dpio_lock);
+ mutex_init(dev_priv-rps.hw_lock);
+ mutex_init(dev_priv-modeset_restore_lock);
+
i915_dump_device_info(dev_priv);
if (i915_get_bridge_dev(dev)) {
@@ -1602,6 +1609,7 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
intel_irq_init(dev);
intel_gt_init(dev);
+ intel_gt_reset(dev);
/* Try to make sure MCHBAR is enabled before poking at it */
intel_setup_mchbar(dev);
@@ -1626,14 +1634,6 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
if (!IS_I945G(dev) !IS_I945GM(dev))
pci_enable_msi(dev-pdev);
- spin_lock_init(dev_priv-irq_lock);
- spin_lock_init(dev_priv-gpu_error.lock);
- spin_lock_init(dev_priv-rps.lock);
- mutex_init(dev_priv-dpio_lock);
-
- mutex_init(dev_priv-rps.hw_lock);
- mutex_init(dev_priv-modeset_restore_lock);
-
dev_priv-num_plane = 1;
if (IS_VALLEYVIEW(dev))
dev_priv-num_plane = 2;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index aa01128..b86db1e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4497,6 +4497,9 @@ void intel_gt_reset(struct drm_device *dev)
if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
__gen6_gt_force_wake_mt_reset(dev_priv);
}
+
+ /* BIOS often leaves RC6 enabled, but disable it for hw init */
+ intel_disable_gt_powersave(dev);
}
void intel_gt_init(struct drm_device *dev)
@@ -4505,8 +4508,6 @@ void intel_gt_init(struct drm_device *dev)
spin_lock_init(dev_priv-gt_lock);
- intel_gt_reset(dev);
-
if (IS_VALLEYVIEW(dev)) {
dev_priv-gt.force_wake_get = vlv_force_wake_get;
dev_priv-gt.force_wake_put = vlv_force_wake_put;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: fix long-standing SNB regression in power consumption after resume
On Wed, Jul 17, 2013 at 10:22:58AM +0400, Konstantin Khlebnikov wrote:
This patch fixes regression in power consumtion of sandy bridge gpu, which
exists since v3.6 Sometimes after resuming from s2ram gpu starts thinking that
it's extremely busy. After that it never reaches rc6 state.
Bug exists since kernel v3.6, commit b4ae3f22d238617ca11610b29fde16cf8c0bc6e0
(drm/i915: load boot context at driver init time).
For some reason RC6 is already enabled at the beginning of resuming process.
Following initliaztion breaks some internal state and confuses RPS engine.
This patch disables RC6 at the beginnig of resume and initialization.
I've rearranged initialization sequence, because intel_disable_gt_powersave()
needs initialized force_wake_get/put and some locks from the dev_priv.
References: https://bugs.freedesktop.org/show_bug.cgi?id=54089
References: https://bugzilla.kernel.org/show_bug.cgi?id=58971
Signed-off-by: Konstantin Khlebnikov khlebni...@openvz.org
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Jesse Barnes jbar...@virtuousgeek.org
lgtm, thanks a lot for digging through this giant mess and figuring out
what's actually broken here. Patch is merged to my -fixes queue with a
slightly pimped commit message and cc: stable added.
Note that there's a small issue with the drps code on ilk, we now write a
bogus fstart value as the requested frequency. I'll follow-up with a patch
to fix this, but I've figured that merging this one here first for wider
testing is more important.
Cheers, Daniel
---
drivers/gpu/drm/i915/i915_dma.c | 16
drivers/gpu/drm/i915/intel_pm.c |5 +++--
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 3b315ba..d1ee611 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1511,6 +1511,13 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
dev_priv-dev = dev;
dev_priv-info = info;
+ spin_lock_init(dev_priv-irq_lock);
+ spin_lock_init(dev_priv-gpu_error.lock);
+ spin_lock_init(dev_priv-rps.lock);
+ mutex_init(dev_priv-dpio_lock);
+ mutex_init(dev_priv-rps.hw_lock);
+ mutex_init(dev_priv-modeset_restore_lock);
+
i915_dump_device_info(dev_priv);
if (i915_get_bridge_dev(dev)) {
@@ -1602,6 +1609,7 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
intel_irq_init(dev);
intel_gt_init(dev);
+ intel_gt_reset(dev);
/* Try to make sure MCHBAR is enabled before poking at it */
intel_setup_mchbar(dev);
@@ -1626,14 +1634,6 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
if (!IS_I945G(dev) !IS_I945GM(dev))
pci_enable_msi(dev-pdev);
- spin_lock_init(dev_priv-irq_lock);
- spin_lock_init(dev_priv-gpu_error.lock);
- spin_lock_init(dev_priv-rps.lock);
- mutex_init(dev_priv-dpio_lock);
-
- mutex_init(dev_priv-rps.hw_lock);
- mutex_init(dev_priv-modeset_restore_lock);
-
dev_priv-num_plane = 1;
if (IS_VALLEYVIEW(dev))
dev_priv-num_plane = 2;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index aa01128..b86db1e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4497,6 +4497,9 @@ void intel_gt_reset(struct drm_device *dev)
if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
__gen6_gt_force_wake_mt_reset(dev_priv);
}
+
+ /* BIOS often leaves RC6 enabled, but disable it for hw init */
+ intel_disable_gt_powersave(dev);
}
void intel_gt_init(struct drm_device *dev)
@@ -4505,8 +4508,6 @@ void intel_gt_init(struct drm_device *dev)
spin_lock_init(dev_priv-gt_lock);
- intel_gt_reset(dev);
-
if (IS_VALLEYVIEW(dev)) {
dev_priv-gt.force_wake_get = vlv_force_wake_get;
dev_priv-gt.force_wake_put = vlv_force_wake_put;
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH 0/4 V7] Power-well API implementation for Haswell
At Wed, 17 Jul 2013 02:52:41 +,
Wang, Xingchao wrote:
Hi Takashi/Paulo,
would you change it to auto and test again.
Runtime power save should be enabled with auto.
Doesn't solve the problem. Should I open a bug report somewhere?
Having the power well enabled prevents some power saving features from
the graphics driver.
Is the HD-audio power-saving itself working? You can check it via watching
/sys/class/hwC*/power_{on|off}_acct files.
power_save option has to be adjusted appropriately. Note that many DEs
change this value dynamically per AC-cable plug/unplug depending on the
configuration, and often it's set to 0 (= no power save) when AC-cable is
plugged.
[Wang, Xingchao] Paulo used a new Ultrabook board with charger connected, and
see the default parameter auto=on.
In such scenario, power-well is always occupied by Display audio controller.
Moreover, in this board, if no external monitors connected,
It's using internal eDP and totally no audio support. Power-well usage in
such case also blocks some eDP features as Paulo told me.
So I think it's not a good idea to break the rule of power policy when
charger connected but it's necessary to add support in this particular case.
Takashi, do you think it's acceptable to let Display audio controller/codec
suspend in such case?
Do you mean the driver enables the powersave forcibly?
Then, no, not in general.
If the default parameter of autopm is the problem, this should be
changed, instead of forcing the policy in the driver.
OTOH, the audio codec's powersave policy is governed by the power_save
option and it's set up dynamically by the desktop system. We
shouldn't override it in the driver.
If the power well *must* be off when only an eDP is used
(e.g. otherwise the hardware doesn't work properly), then it's a
different story. Is it the case? And what exactly would be the
problem?
If it's the case, controlling the power well based on the runtime PM
is likely a bad design, as it relies on the parameter user sets.
(And remember that the power-saving of the audio can be disabled
completely via Kconfig, too.)
Takashi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH 0/4 V7] Power-well API implementation for Haswell
-Original Message-
From: Takashi Iwai [mailto:ti...@suse.de]
Sent: Wednesday, July 17, 2013 3:34 PM
To: Wang, Xingchao
Cc: Paulo Zanoni; alsa-de...@alsa-project.org; Daniel Vetter;
daniel.vet...@ffwll.ch; intel-gfx@lists.freedesktop.org; Wang xingchao;
Girdwood, Liam R; david.hennings...@canonical.com
Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well API
implementation for Haswell
At Wed, 17 Jul 2013 02:52:41 +,
Wang, Xingchao wrote:
Hi Takashi/Paulo,
would you change it to auto and test again.
Runtime power save should be enabled with auto.
Doesn't solve the problem. Should I open a bug report somewhere?
Having the power well enabled prevents some power saving features
from the graphics driver.
Is the HD-audio power-saving itself working? You can check it via
watching /sys/class/hwC*/power_{on|off}_acct files.
power_save option has to be adjusted appropriately. Note that many
DEs change this value dynamically per AC-cable plug/unplug depending
on the configuration, and often it's set to 0 (= no power save) when
AC-cable is plugged.
[Wang, Xingchao] Paulo used a new Ultrabook board with charger connected,
and see the default parameter auto=on.
In such scenario, power-well is always occupied by Display audio
controller. Moreover, in this board, if no external monitors connected, It's
using internal eDP and totally no audio support. Power-well usage in such case
also blocks some eDP features as Paulo told me.
So I think it's not a good idea to break the rule of power policy when
charger
connected but it's necessary to add support in this particular case.
Takashi, do you think it's acceptable to let Display audio controller/codec
suspend in such case?
Do you mean the driver enables the powersave forcibly?
Yes. I mean call pm_runtime_allow() for the power-well HD-A controller.
Then, no, not in general.
If the default parameter of autopm is the problem, this should be changed,
instead of forcing the policy in the driver.
OTOH, the audio codec's powersave policy is governed by the power_save
option and it's set up dynamically by the desktop system. We shouldn't
override it in the driver.
If the power well *must* be off when only an eDP is used (e.g. otherwise the
hardware doesn't work properly), then it's a
different story. Is it the case? And what exactly would be the
problem?
In the eDP only case, no audio is needed for the HD-A controller, so it's
wasting power in current design.
I think Paulo or Daniel could explain more details on the impact.
If it's the case, controlling the power well based on the runtime PM is
likely a
bad design, as it relies on the parameter user sets.
(And remember that the power-saving of the audio can be disabled
completely via Kconfig, too.)
From audio controller's point of view, if it's asked be active, it needs power
and should request power-well from gfx side.
In above case, audio controller should not be active but user set it be
active.
Thanks
--xingchao
Takashi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH 0/4 V7] Power-well API implementation for Haswell
At Wed, 17 Jul 2013 08:03:38 +,
Wang, Xingchao wrote:
-Original Message-
From: Takashi Iwai [mailto:ti...@suse.de]
Sent: Wednesday, July 17, 2013 3:34 PM
To: Wang, Xingchao
Cc: Paulo Zanoni; alsa-de...@alsa-project.org; Daniel Vetter;
daniel.vet...@ffwll.ch; intel-gfx@lists.freedesktop.org; Wang xingchao;
Girdwood, Liam R; david.hennings...@canonical.com
Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well API
implementation for Haswell
At Wed, 17 Jul 2013 02:52:41 +,
Wang, Xingchao wrote:
Hi Takashi/Paulo,
would you change it to auto and test again.
Runtime power save should be enabled with auto.
Doesn't solve the problem. Should I open a bug report somewhere?
Having the power well enabled prevents some power saving features
from the graphics driver.
Is the HD-audio power-saving itself working? You can check it via
watching /sys/class/hwC*/power_{on|off}_acct files.
power_save option has to be adjusted appropriately. Note that many
DEs change this value dynamically per AC-cable plug/unplug depending
on the configuration, and often it's set to 0 (= no power save) when
AC-cable is plugged.
[Wang, Xingchao] Paulo used a new Ultrabook board with charger connected,
and see the default parameter auto=on.
In such scenario, power-well is always occupied by Display audio
controller. Moreover, in this board, if no external monitors connected,
It's
using internal eDP and totally no audio support. Power-well usage in such
case
also blocks some eDP features as Paulo told me.
So I think it's not a good idea to break the rule of power policy when
charger
connected but it's necessary to add support in this particular case.
Takashi, do you think it's acceptable to let Display audio
controller/codec
suspend in such case?
Do you mean the driver enables the powersave forcibly?
Yes. I mean call pm_runtime_allow() for the power-well HD-A controller.
Then, no, not in general.
If the default parameter of autopm is the problem, this should be changed,
instead of forcing the policy in the driver.
OTOH, the audio codec's powersave policy is governed by the power_save
option and it's set up dynamically by the desktop system. We shouldn't
override it in the driver.
If the power well *must* be off when only an eDP is used (e.g. otherwise the
hardware doesn't work properly), then it's a
different story. Is it the case? And what exactly would be the
problem?
In the eDP only case, no audio is needed for the HD-A controller, so it's
wasting power in current design.
I think Paulo or Daniel could explain more details on the impact.
Consuming more power is the expected behavior. What else?
If it's the case, controlling the power well based on the runtime PM is
likely a
bad design, as it relies on the parameter user sets.
(And remember that the power-saving of the audio can be disabled
completely via Kconfig, too.)
From audio controller's point of view, if it's asked be active, it needs
power and should request power-well from gfx side.
In above case, audio controller should not be active but user set it be
active.
By setting the autopm on, user expects that no runtime PM happens.
In other words, the audio controller must be kept active as long as
this parameter is set. And this is the parameter user controls, and
not what the driver forcibly sets.
Takashi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Sanitize shared dpll state
On Wed, Jul 17, 2013 at 06:55:04AM +0200, Daniel Vetter wrote: There seems to be no limit to the amount of gunk the firmware can leave behind. Some platforms leave pch dplls on which are not in active use at all. The example in the bug report is a Apple Macbook Pro. Note that this escape scrunity of the hw state checker until we've tried to use this enabled, but unused pll since we did only check for the inverse case of a in-used, but disabled pll. v2: Add a WARN in the pll state checker which would have caught this case. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66952 Reported-and-tested-by: shui yangwei yangweix.s...@intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Plonk it in intel_sanitize_plls(), and preferably move all the sanitze encoder/crtc/pll into intel_sanitize_display() (in a later patch), so that intel_modeset_setup_hw_state() is not quite so broken up. Other than that minor request, Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH 0/4 V7] Power-well API implementation for Haswell
-Original Message-
From: Takashi Iwai [mailto:ti...@suse.de]
Sent: Wednesday, July 17, 2013 4:18 PM
To: Wang, Xingchao
Cc: Paulo Zanoni; alsa-de...@alsa-project.org; Daniel Vetter;
daniel.vet...@ffwll.ch; intel-gfx@lists.freedesktop.org; Wang xingchao;
Girdwood, Liam R; david.hennings...@canonical.com
Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well API
implementation for Haswell
At Wed, 17 Jul 2013 08:03:38 +,
Wang, Xingchao wrote:
-Original Message-
From: Takashi Iwai [mailto:ti...@suse.de]
Sent: Wednesday, July 17, 2013 3:34 PM
To: Wang, Xingchao
Cc: Paulo Zanoni; alsa-de...@alsa-project.org; Daniel Vetter;
daniel.vet...@ffwll.ch; intel-gfx@lists.freedesktop.org; Wang
xingchao; Girdwood, Liam R; david.hennings...@canonical.com
Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well API
implementation for Haswell
At Wed, 17 Jul 2013 02:52:41 +,
Wang, Xingchao wrote:
Hi Takashi/Paulo,
would you change it to auto and test again.
Runtime power save should be enabled with auto.
Doesn't solve the problem. Should I open a bug report somewhere?
Having the power well enabled prevents some power saving
features from the graphics driver.
Is the HD-audio power-saving itself working? You can check it
via watching /sys/class/hwC*/power_{on|off}_acct files.
power_save option has to be adjusted appropriately. Note that
many DEs change this value dynamically per AC-cable plug/unplug
depending on the configuration, and often it's set to 0 (= no
power save) when AC-cable is plugged.
[Wang, Xingchao] Paulo used a new Ultrabook board with charger
connected,
and see the default parameter auto=on.
In such scenario, power-well is always occupied by Display audio
controller. Moreover, in this board, if no external monitors
connected, It's
using internal eDP and totally no audio support. Power-well usage in
such case also blocks some eDP features as Paulo told me.
So I think it's not a good idea to break the rule of power policy
when charger
connected but it's necessary to add support in this particular case.
Takashi, do you think it's acceptable to let Display audio
controller/codec
suspend in such case?
Do you mean the driver enables the powersave forcibly?
Yes. I mean call pm_runtime_allow() for the power-well HD-A controller.
Then, no, not in general.
If the default parameter of autopm is the problem, this should be
changed, instead of forcing the policy in the driver.
OTOH, the audio codec's powersave policy is governed by the
power_save option and it's set up dynamically by the desktop system.
We shouldn't override it in the driver.
If the power well *must* be off when only an eDP is used (e.g.
otherwise the hardware doesn't work properly), then it's a
different story. Is it the case? And what exactly would be the
problem?
In the eDP only case, no audio is needed for the HD-A controller, so it's
wasting power in current design.
I think Paulo or Daniel could explain more details on the impact.
Consuming more power is the expected behavior. What else?
If it's the case, controlling the power well based on the runtime PM
is likely a bad design, as it relies on the parameter user sets.
(And remember that the power-saving of the audio can be disabled
completely via Kconfig, too.)
From audio controller's point of view, if it's asked be active, it needs
power
and should request power-well from gfx side.
In above case, audio controller should not be active but user set it be
active.
By setting the autopm on, user expects that no runtime PM happens.
In other words, the audio controller must be kept active as long as this
parameter is set. And this is the parameter user controls, and not what the
driver forcibly sets.
Okay, become clear now. :)
So I think the conflict for Paulo becomes, in eDP caes, if audio is active and
requested power-well, some eDP feature was under impact?
Paulo, would you clarify this in more details?
thanks
--xingchao
Takashi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [drm-intel:drm-intel-fixes 52/52] ERROR: Unrecognized email address: 'stable.]'
On Wed, Jul 17, 2013 at 11:10 AM, Fengguang Wu fengguang...@intel.com wrote: Hi Konstantin, FYI, there are new warnings show up in tree: git://people.freedesktop.org/~danvet/drm-intel.git drm-intel-fixes head: 71e4092e52499ec74bc1dec0f883b15f2c424ec5 commit: 71e4092e52499ec74bc1dec0f883b15f2c424ec5 [52/52] drm/i915: fix long-standing SNB regression in power consumption after resume v2 scripts/checkpatch.pl 0001-drm-i915-fix-long-standing-SNB-regression-in-power-c.patch ERROR: Unrecognized email address: 'stable.]' #41: cc: stable.] Well, I've added that while applying the patch - I tend to smash maintainer notes into the sob section and word-wraping caused the cc: stable remark to be parsed. Is there an officially sanctioned way for such notes that appeases checkpatch? Adding lkml and checkpatch maintainer. -Daniel --- 0-DAY kernel build testing backend Open Source Technology Center http://lists.01.org/mailman/listinfo/kbuild Intel Corporation -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Intel Crash
On Tue, Jul 16, 2013 at 09:51:43PM -0600, Oscar Dario Trujillo Tejada wrote: I send what you want(I guess), online and as attach [1]Xorg Log Found it, we were requesting a upload buffer of size 0, due to using a bad alignment value as a result of setting a bad default value for unknown cpu cache size. Fixed by commit 7f76a2bf319f59d463a1f96974b03d7c651847dd Author: Chris Wilson ch...@chris-wilson.co.uk Date: Wed Jul 17 10:22:17 2013 +0100 sna: Fall back to /proc/cpuinfo parsing if cpuid cache size probe fails -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Sanitize shared dpll state
On Wed, Jul 17, 2013 at 09:19:03AM +0100, Chris Wilson wrote: On Wed, Jul 17, 2013 at 06:55:04AM +0200, Daniel Vetter wrote: There seems to be no limit to the amount of gunk the firmware can leave behind. Some platforms leave pch dplls on which are not in active use at all. The example in the bug report is a Apple Macbook Pro. Note that this escape scrunity of the hw state checker until we've tried to use this enabled, but unused pll since we did only check for the inverse case of a in-used, but disabled pll. v2: Add a WARN in the pll state checker which would have caught this case. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66952 Reported-and-tested-by: shui yangwei yangweix.s...@intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Plonk it in intel_sanitize_plls(), and preferably move all the sanitze encoder/crtc/pll into intel_sanitize_display() (in a later patch), so that intel_modeset_setup_hw_state() is not quite so broken up. Agreed, but soemthing for -next. Other than that minor request, Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk Picked up for -fixes, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix dereferencing invalid connectors in is_crtc_connector_off()
In commit e3de42b68478a8c95dd27520e9adead2af9477a5
Author: Imre Deak imre.d...@intel.com
Date: Fri May 3 19:44:07 2013 +0200
drm/i915: force full modeset if the connector is in DPMS OFF mode
a new function was added that walked over the set of connectors to see
if any of the currently associated CRTC was switched off. This function
walked an array of connectors, rather than the array of pointers to
connectors contained in the drm_mode_set - i.e. it was dereferencing far
past the end of the first connector. This only becomes an issue if we
attempt to use a clone mode (i.e. more than one connector per CRTC) such
that set-num_connectors 1.
Reported-by: Timo Aaltonen tjaal...@ubuntu.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65927
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Imre Deak imre.d...@intel.com
Cc: Egbert Eich e...@suse.de
Cc: Jesse Barnes jbar...@virtuousgeek.org
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/i915/intel_display.c | 23 +--
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 9a10e71..6790600 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8658,15 +8658,20 @@ static void intel_set_config_restore_state(struct
drm_device *dev,
}
static bool
-is_crtc_connector_off(struct drm_crtc *crtc, struct drm_connector *connectors,
- int num_connectors)
+is_crtc_connector_off(struct drm_mode_set *set)
{
int i;
- for (i = 0; i num_connectors; i++)
- if (connectors[i].encoder
- connectors[i].encoder-crtc == crtc
- connectors[i].dpms != DRM_MODE_DPMS_ON)
+ if (set-num_connectors == 0)
+ return false;
+
+ if (WARN_ON(set-connectors == NULL))
+ return false;
+
+ for (i = 0; i set-num_connectors; i++)
+ if (set-connectors[i]-encoder
+ set-connectors[i]-encoder-crtc == set-crtc
+ set-connectors[i]-dpms != DRM_MODE_DPMS_ON)
return true;
return false;
@@ -8679,10 +8684,8 @@ intel_set_config_compute_mode_changes(struct
drm_mode_set *set,
/* We should be able to check here if the fb has the same properties
* and then just flip_or_move it */
- if (set-connectors != NULL
- is_crtc_connector_off(set-crtc, *set-connectors,
- set-num_connectors)) {
- config-mode_changed = true;
+ if (is_crtc_connector_off(set)) {
+ config-mode_changed = true;
} else if (set-crtc-fb != set-fb) {
/* If we have no fb then treat it as a full mode set */
if (set-crtc-fb == NULL) {
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix dereferencing invalid connectors in is_crtc_connector_off()
On Wed, Jul 17, 2013 at 12:14:40PM +0100, Chris Wilson wrote:
In commit e3de42b68478a8c95dd27520e9adead2af9477a5
Author: Imre Deak imre.d...@intel.com
Date: Fri May 3 19:44:07 2013 +0200
drm/i915: force full modeset if the connector is in DPMS OFF mode
a new function was added that walked over the set of connectors to see
if any of the currently associated CRTC was switched off. This function
walked an array of connectors, rather than the array of pointers to
connectors contained in the drm_mode_set - i.e. it was dereferencing far
past the end of the first connector. This only becomes an issue if we
attempt to use a clone mode (i.e. more than one connector per CRTC) such
that set-num_connectors 1.
Reported-by: Timo Aaltonen tjaal...@ubuntu.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65927
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Imre Deak imre.d...@intel.com
Cc: Egbert Eich e...@suse.de
Cc: Jesse Barnes jbar...@virtuousgeek.org
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: sta...@vger.kernel.org
Oh dear ...
Picked up for -fixes, thanks for the patch.
-Daniel
---
drivers/gpu/drm/i915/intel_display.c | 23 +--
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 9a10e71..6790600 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8658,15 +8658,20 @@ static void intel_set_config_restore_state(struct
drm_device *dev,
}
static bool
-is_crtc_connector_off(struct drm_crtc *crtc, struct drm_connector
*connectors,
- int num_connectors)
+is_crtc_connector_off(struct drm_mode_set *set)
{
int i;
- for (i = 0; i num_connectors; i++)
- if (connectors[i].encoder
- connectors[i].encoder-crtc == crtc
- connectors[i].dpms != DRM_MODE_DPMS_ON)
+ if (set-num_connectors == 0)
+ return false;
+
+ if (WARN_ON(set-connectors == NULL))
+ return false;
+
+ for (i = 0; i set-num_connectors; i++)
+ if (set-connectors[i]-encoder
+ set-connectors[i]-encoder-crtc == set-crtc
+ set-connectors[i]-dpms != DRM_MODE_DPMS_ON)
return true;
return false;
@@ -8679,10 +8684,8 @@ intel_set_config_compute_mode_changes(struct
drm_mode_set *set,
/* We should be able to check here if the fb has the same properties
* and then just flip_or_move it */
- if (set-connectors != NULL
- is_crtc_connector_off(set-crtc, *set-connectors,
- set-num_connectors)) {
- config-mode_changed = true;
+ if (is_crtc_connector_off(set)) {
+ config-mode_changed = true;
} else if (set-crtc-fb != set-fb) {
/* If we have no fb then treat it as a full mode set */
if (set-crtc-fb == NULL) {
--
1.8.3.2
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wednesday, July 17, 2013 09:16:38 AM Igor Gnatenko wrote:
On Wed, 2013-07-17 at 00:01 +0200, Rafael J. Wysocki wrote:
On Tuesday, July 16, 2013 05:08:16 PM Matthew Garrett wrote:
On Tue, 2013-07-16 at 17:32 +0400, Igor Gnatenko wrote:
Hmm. I found regression in user-space. In GNOME (maybe and other DEs)
we no longer see switch status of backlight.
Yeah, I can duplicate that. Rafael, we have to call
acpi_video_init_brightness() even if we're not going to initialise the
backlight - Thinkpads seem to use this as the trigger for enabling ACPI
notifications rather than handling it in firmware. This seems to do the
job:
Igor, does this additional patch from Matthew help?
Yes. With this patch I have backlight switch indicator on my ThinkPad X230.
OK, thanks for the confirmation.
Can you please also check if applying the appended patch on top of the Matthew's
one changes anything (ie. things still work)?
Rafael
---
drivers/acpi/video.c |5 +
1 file changed, 5 insertions(+)
Index: linux-pm/drivers/acpi/video.c
===
--- linux-pm.orig/drivers/acpi/video.c
+++ linux-pm/drivers/acpi/video.c
@@ -957,6 +957,11 @@ static void acpi_video_device_find_cap(s
if (result)
printk(KERN_ERR PREFIX Create sysfs link\n);
+ } else {
+ /* Remove the brightness object. */
+ kfree(device-brightness-levels);
+ kfree(device-brightness);
+ device-brightness = NULL;
}
}
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, 2013-07-17 at 13:38 +0200, Rafael J. Wysocki wrote:
On Wednesday, July 17, 2013 09:16:38 AM Igor Gnatenko wrote:
On Wed, 2013-07-17 at 00:01 +0200, Rafael J. Wysocki wrote:
On Tuesday, July 16, 2013 05:08:16 PM Matthew Garrett wrote:
On Tue, 2013-07-16 at 17:32 +0400, Igor Gnatenko wrote:
Hmm. I found regression in user-space. In GNOME (maybe and other DEs)
we no longer see switch status of backlight.
Yeah, I can duplicate that. Rafael, we have to call
acpi_video_init_brightness() even if we're not going to initialise the
backlight - Thinkpads seem to use this as the trigger for enabling ACPI
notifications rather than handling it in firmware. This seems to do the
job:
Igor, does this additional patch from Matthew help?
Yes. With this patch I have backlight switch indicator on my ThinkPad X230.
OK, thanks for the confirmation.
Can you please also check if applying the appended patch on top of the
Matthew's
one changes anything (ie. things still work)?
Yes. I've tested and not found regressions in indicator or in switcher. Good
work.
Rafael
---
Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com
drivers/acpi/video.c |5 +
1 file changed, 5 insertions(+)
Index: linux-pm/drivers/acpi/video.c
===
--- linux-pm.orig/drivers/acpi/video.c
+++ linux-pm/drivers/acpi/video.c
@@ -957,6 +957,11 @@ static void acpi_video_device_find_cap(s
if (result)
printk(KERN_ERR PREFIX Create sysfs link\n);
+ } else {
+ /* Remove the brightness object. */
+ kfree(device-brightness-levels);
+ kfree(device-brightness);
+ device-brightness = NULL;
}
}
--
Igor Gnatenko
Fedora release 19 (Schrödinger’s Cat)
Linux 3.11.0-0.rc0.git7.1.fc20.x86_64
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH 0/4 V7] Power-well API implementation for Haswell
2013/7/17 Wang, Xingchao xingchao.w...@intel.com:
-Original Message-
From: Takashi Iwai [mailto:ti...@suse.de]
Sent: Wednesday, July 17, 2013 4:18 PM
To: Wang, Xingchao
Cc: Paulo Zanoni; alsa-de...@alsa-project.org; Daniel Vetter;
daniel.vet...@ffwll.ch; intel-gfx@lists.freedesktop.org; Wang xingchao;
Girdwood, Liam R; david.hennings...@canonical.com
Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well API
implementation for Haswell
At Wed, 17 Jul 2013 08:03:38 +,
Wang, Xingchao wrote:
-Original Message-
From: Takashi Iwai [mailto:ti...@suse.de]
Sent: Wednesday, July 17, 2013 3:34 PM
To: Wang, Xingchao
Cc: Paulo Zanoni; alsa-de...@alsa-project.org; Daniel Vetter;
daniel.vet...@ffwll.ch; intel-gfx@lists.freedesktop.org; Wang
xingchao; Girdwood, Liam R; david.hennings...@canonical.com
Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well API
implementation for Haswell
At Wed, 17 Jul 2013 02:52:41 +,
Wang, Xingchao wrote:
Hi Takashi/Paulo,
would you change it to auto and test again.
Runtime power save should be enabled with auto.
Doesn't solve the problem. Should I open a bug report somewhere?
Having the power well enabled prevents some power saving
features from the graphics driver.
Is the HD-audio power-saving itself working? You can check it
via watching /sys/class/hwC*/power_{on|off}_acct files.
power_save option has to be adjusted appropriately. Note that
many DEs change this value dynamically per AC-cable plug/unplug
depending on the configuration, and often it's set to 0 (= no
power save) when AC-cable is plugged.
[Wang, Xingchao] Paulo used a new Ultrabook board with charger
connected,
and see the default parameter auto=on.
In such scenario, power-well is always occupied by Display audio
controller. Moreover, in this board, if no external monitors
connected, It's
using internal eDP and totally no audio support. Power-well usage in
such case also blocks some eDP features as Paulo told me.
So I think it's not a good idea to break the rule of power policy
when charger
connected but it's necessary to add support in this particular case.
Takashi, do you think it's acceptable to let Display audio
controller/codec
suspend in such case?
Do you mean the driver enables the powersave forcibly?
Yes. I mean call pm_runtime_allow() for the power-well HD-A controller.
Then, no, not in general.
If the default parameter of autopm is the problem, this should be
changed, instead of forcing the policy in the driver.
OTOH, the audio codec's powersave policy is governed by the
power_save option and it's set up dynamically by the desktop system.
We shouldn't override it in the driver.
If the power well *must* be off when only an eDP is used (e.g.
otherwise the hardware doesn't work properly), then it's a
different story. Is it the case? And what exactly would be the
problem?
In the eDP only case, no audio is needed for the HD-A controller, so it's
wasting power in current design.
I think Paulo or Daniel could explain more details on the impact.
Consuming more power is the expected behavior. What else?
If it's the case, controlling the power well based on the runtime PM
is likely a bad design, as it relies on the parameter user sets.
(And remember that the power-saving of the audio can be disabled
completely via Kconfig, too.)
From audio controller's point of view, if it's asked be active, it needs
power
and should request power-well from gfx side.
In above case, audio controller should not be active but user set it be
active.
By setting the autopm on, user expects that no runtime PM happens.
In other words, the audio controller must be kept active as long as this
parameter is set. And this is the parameter user controls, and not what the
driver forcibly sets.
Okay, become clear now. :)
So I think the conflict for Paulo becomes, in eDP caes, if audio is active
and requested power-well, some eDP feature was under impact?
Paulo, would you clarify this in more details?
On our driver we try to disable the power well whenever possible, as
soon as possible. We don't change our behavior based on power AC or
other user-space modifiable behavior (except the
i915.disable_power_well Kernel option). If the power well is not
disabled we can't enable some features, like PSR (panel self refresh,
and eDP feature) or PC8, which is another power-saving feature. This
will also make our QA procedures a lot more complex since when we want
to test specific features (e.g., PSR, PC8) we'll have to disconnect
the AC adapter or run scripts. So the behavior/predictability of our
driver will be based on the Audio driver power management policies.
I am not so experienced with general Linux Power Management
[Intel-gfx] [PATCH] drm/i915: correctly restore fences with objects attached
To avoid stalls we delay tiling changes and especially hold of
committing the new fence state for as long as possible.
Synchronization points are in the execbuf code and in our gtt fault
handler.
Unfortunately we've missed that tricky detail when adding proper fence
restore code in
commit 19b2dbde5732170a03bd82cc8bd442cf88d856f7
Author: Chris Wilson ch...@chris-wilson.co.uk
Date: Wed Jun 12 10:15:12 2013 +0100
drm/i915: Restore fences after resume and GPU resets
The result was that we've restored fences for objects with no tiling,
since the object-fence link still existed after resume. Now that
wouldn't have been too bad since any subsequent access would have
fixed things up, but if we've changed from tiled to untiled real havoc
happened:
The tiling stride is stored -1 in the fence register, so a stride of 0
resulted in all 1s in the top 32bits, and so a completely bogus fence
spanning everything from the start of the object to the top of the
GTT. The tell-tale in the register dumps looks like:
FENCE START 2: 0x0214d001
FENCE END 2: 0xf3ff
Bit 11 isn't set since the hw doesn't store it, even when writing all
1s (at least on my snb here).
To prevent such a gaffle in the future add a sanity check for fences
with an untiled object attached in i915_gem_write_fence.
v2: Fix the WARN, spotted by Chris.
v3: Trying to reuse get_fences looked ugly and obfuscated the code.
Instead reuse update_fence and to make it really dtrt also move the
fence dirty state clearing into update_fence.
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Stéphane Marchesin marc...@chromium.org
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=60530
Cc: sta...@vger.kernel.org (for 3.10 only)
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c9d9d20..e2370a2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2257,7 +2257,17 @@ void i915_gem_restore_fences(struct drm_device *dev)
for (i = 0; i dev_priv-num_fence_regs; i++) {
struct drm_i915_fence_reg *reg = dev_priv-fence_regs[i];
- i915_gem_write_fence(dev, i, reg-obj);
+
+ /*
+* Commit delayed tiling changes if we have an object still
+* attached to the fence, otherwise just clear the fence.
+*/
+ if (reg-obj) {
+ i915_gem_object_update_fence(reg-obj, reg,
+reg-obj-tiling_mode);
+ } else {
+ i915_gem_write_fence(dev, i, NULL);
+ }
}
}
@@ -2792,6 +2802,10 @@ static void i915_gem_write_fence(struct drm_device *dev,
int reg,
if (i915_gem_object_needs_mb(dev_priv-fence_regs[reg].obj))
mb();
+ WARN(obj (!obj-stride || !obj-tiling_mode),
+bogus fence setup with stride: 0x%x, tiling mode: %i\n,
+obj-stride, obj-tiling_mode);
+
switch (INTEL_INFO(dev)-gen) {
case 7:
case 6:
@@ -2833,6 +2847,7 @@ static void i915_gem_object_update_fence(struct
drm_i915_gem_object *obj,
fence-obj = NULL;
list_del_init(fence-lru_list);
}
+ obj-fence_dirty = false;
}
static int
@@ -2962,7 +2977,6 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj)
return 0;
i915_gem_object_update_fence(obj, reg, enable);
- obj-fence_dirty = false;
return 0;
}
--
1.8.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH 0/4 V7] Power-well API implementation for Haswell
At Wed, 17 Jul 2013 10:31:26 -0300,
Paulo Zanoni wrote:
2013/7/17 Wang, Xingchao xingchao.w...@intel.com:
-Original Message-
From: Takashi Iwai [mailto:ti...@suse.de]
Sent: Wednesday, July 17, 2013 4:18 PM
To: Wang, Xingchao
Cc: Paulo Zanoni; alsa-de...@alsa-project.org; Daniel Vetter;
daniel.vet...@ffwll.ch; intel-gfx@lists.freedesktop.org; Wang xingchao;
Girdwood, Liam R; david.hennings...@canonical.com
Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well API
implementation for Haswell
At Wed, 17 Jul 2013 08:03:38 +,
Wang, Xingchao wrote:
-Original Message-
From: Takashi Iwai [mailto:ti...@suse.de]
Sent: Wednesday, July 17, 2013 3:34 PM
To: Wang, Xingchao
Cc: Paulo Zanoni; alsa-de...@alsa-project.org; Daniel Vetter;
daniel.vet...@ffwll.ch; intel-gfx@lists.freedesktop.org; Wang
xingchao; Girdwood, Liam R; david.hennings...@canonical.com
Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well API
implementation for Haswell
At Wed, 17 Jul 2013 02:52:41 +,
Wang, Xingchao wrote:
Hi Takashi/Paulo,
would you change it to auto and test again.
Runtime power save should be enabled with auto.
Doesn't solve the problem. Should I open a bug report somewhere?
Having the power well enabled prevents some power saving
features from the graphics driver.
Is the HD-audio power-saving itself working? You can check it
via watching /sys/class/hwC*/power_{on|off}_acct files.
power_save option has to be adjusted appropriately. Note that
many DEs change this value dynamically per AC-cable plug/unplug
depending on the configuration, and often it's set to 0 (= no
power save) when AC-cable is plugged.
[Wang, Xingchao] Paulo used a new Ultrabook board with charger
connected,
and see the default parameter auto=on.
In such scenario, power-well is always occupied by Display audio
controller. Moreover, in this board, if no external monitors
connected, It's
using internal eDP and totally no audio support. Power-well usage in
such case also blocks some eDP features as Paulo told me.
So I think it's not a good idea to break the rule of power policy
when charger
connected but it's necessary to add support in this particular case.
Takashi, do you think it's acceptable to let Display audio
controller/codec
suspend in such case?
Do you mean the driver enables the powersave forcibly?
Yes. I mean call pm_runtime_allow() for the power-well HD-A controller.
Then, no, not in general.
If the default parameter of autopm is the problem, this should be
changed, instead of forcing the policy in the driver.
OTOH, the audio codec's powersave policy is governed by the
power_save option and it's set up dynamically by the desktop system.
We shouldn't override it in the driver.
If the power well *must* be off when only an eDP is used (e.g.
otherwise the hardware doesn't work properly), then it's a
different story. Is it the case? And what exactly would be the
problem?
In the eDP only case, no audio is needed for the HD-A controller, so it's
wasting power in current design.
I think Paulo or Daniel could explain more details on the impact.
Consuming more power is the expected behavior. What else?
If it's the case, controlling the power well based on the runtime PM
is likely a bad design, as it relies on the parameter user sets.
(And remember that the power-saving of the audio can be disabled
completely via Kconfig, too.)
From audio controller's point of view, if it's asked be active, it needs
power
and should request power-well from gfx side.
In above case, audio controller should not be active but user set it be
active.
By setting the autopm on, user expects that no runtime PM happens.
In other words, the audio controller must be kept active as long as this
parameter is set. And this is the parameter user controls, and not what
the
driver forcibly sets.
Okay, become clear now. :)
So I think the conflict for Paulo becomes, in eDP caes, if audio is active
and requested power-well, some eDP feature was under impact?
Paulo, would you clarify this in more details?
On our driver we try to disable the power well whenever possible, as
soon as possible. We don't change our behavior based on power AC or
other user-space modifiable behavior (except the
i915.disable_power_well Kernel option). If the power well is not
disabled we can't enable some features, like PSR (panel self refresh,
and eDP feature) or PC8, which is another power-saving feature. This
will also make our QA procedures a lot more complex since when we want
to test specific features (e.g., PSR, PC8) we'll have to disconnect
the AC adapter or
Re: [Intel-gfx] [alsa-devel] [PATCH 0/4 V7] Power-well API implementation for Haswell
On 07/17/2013 04:00 PM, Takashi Iwai wrote:
At Wed, 17 Jul 2013 10:31:26 -0300,
Paulo Zanoni wrote:
2013/7/17 Wang, Xingchao xingchao.w...@intel.com:
-Original Message-
From: Takashi Iwai [mailto:ti...@suse.de]
Sent: Wednesday, July 17, 2013 4:18 PM
To: Wang, Xingchao
Cc: Paulo Zanoni; alsa-de...@alsa-project.org; Daniel Vetter;
daniel.vet...@ffwll.ch; intel-gfx@lists.freedesktop.org; Wang xingchao;
Girdwood, Liam R; david.hennings...@canonical.com
Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well API
implementation for Haswell
At Wed, 17 Jul 2013 08:03:38 +,
Wang, Xingchao wrote:
-Original Message-
From: Takashi Iwai [mailto:ti...@suse.de]
Sent: Wednesday, July 17, 2013 3:34 PM
To: Wang, Xingchao
Cc: Paulo Zanoni; alsa-de...@alsa-project.org; Daniel Vetter;
daniel.vet...@ffwll.ch; intel-gfx@lists.freedesktop.org; Wang
xingchao; Girdwood, Liam R; david.hennings...@canonical.com
Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well API
implementation for Haswell
At Wed, 17 Jul 2013 02:52:41 +,
Wang, Xingchao wrote:
Hi Takashi/Paulo,
would you change it to auto and test again.
Runtime power save should be enabled with auto.
Doesn't solve the problem. Should I open a bug report somewhere?
Having the power well enabled prevents some power saving
features from the graphics driver.
Is the HD-audio power-saving itself working? You can check it
via watching /sys/class/hwC*/power_{on|off}_acct files.
power_save option has to be adjusted appropriately. Note that
many DEs change this value dynamically per AC-cable plug/unplug
depending on the configuration, and often it's set to 0 (= no
power save) when AC-cable is plugged.
[Wang, Xingchao] Paulo used a new Ultrabook board with charger
connected,
and see the default parameter auto=on.
In such scenario, power-well is always occupied by Display audio
controller. Moreover, in this board, if no external monitors
connected, It's
using internal eDP and totally no audio support. Power-well usage in
such case also blocks some eDP features as Paulo told me.
So I think it's not a good idea to break the rule of power policy
when charger
connected but it's necessary to add support in this particular case.
Takashi, do you think it's acceptable to let Display audio
controller/codec
suspend in such case?
Do you mean the driver enables the powersave forcibly?
Yes. I mean call pm_runtime_allow() for the power-well HD-A controller.
Then, no, not in general.
If the default parameter of autopm is the problem, this should be
changed, instead of forcing the policy in the driver.
OTOH, the audio codec's powersave policy is governed by the
power_save option and it's set up dynamically by the desktop system.
We shouldn't override it in the driver.
If the power well *must* be off when only an eDP is used (e.g.
otherwise the hardware doesn't work properly), then it's a
different story. Is it the case? And what exactly would be the
problem?
In the eDP only case, no audio is needed for the HD-A controller, so it's
wasting power in current design.
I think Paulo or Daniel could explain more details on the impact.
Consuming more power is the expected behavior. What else?
If it's the case, controlling the power well based on the runtime PM
is likely a bad design, as it relies on the parameter user sets.
(And remember that the power-saving of the audio can be disabled
completely via Kconfig, too.)
From audio controller's point of view, if it's asked be active, it needs power
and should request power-well from gfx side.
In above case, audio controller should not be active but user set it be
active.
By setting the autopm on, user expects that no runtime PM happens.
In other words, the audio controller must be kept active as long as this
parameter is set. And this is the parameter user controls, and not what the
driver forcibly sets.
Okay, become clear now. :)
So I think the conflict for Paulo becomes, in eDP caes, if audio is active and
requested power-well, some eDP feature was under impact?
Paulo, would you clarify this in more details?
On our driver we try to disable the power well whenever possible, as
soon as possible. We don't change our behavior based on power AC or
other user-space modifiable behavior (except the
i915.disable_power_well Kernel option). If the power well is not
disabled we can't enable some features, like PSR (panel self refresh,
and eDP feature) or PC8, which is another power-saving feature. This
will also make our QA procedures a lot more complex since when we want
to test specific features (e.g., PSR, PC8) we'll have to disconnect
the AC adapter or run scripts. So the behavior/predictability of our
driver will be based on the Audio driver power management policies.
So all missing feature are about the power saving?
I am not so experienced with general Linux Power Management code, so
maybe the way the Audio driver is behaving is just the usual way, but
I
Re: [Intel-gfx] [PATCH] drm/i915: correctly restore fences with objects attached
On Wed, Jul 17, 2013 at 02:51:28PM +0200, Daniel Vetter wrote: To avoid stalls we delay tiling changes and especially hold of committing the new fence state for as long as possible. Synchronization points are in the execbuf code and in our gtt fault handler. Unfortunately we've missed that tricky detail when adding proper fence restore code in commit 19b2dbde5732170a03bd82cc8bd442cf88d856f7 Author: Chris Wilson ch...@chris-wilson.co.uk Date: Wed Jun 12 10:15:12 2013 +0100 drm/i915: Restore fences after resume and GPU resets The result was that we've restored fences for objects with no tiling, since the object-fence link still existed after resume. Now that wouldn't have been too bad since any subsequent access would have fixed things up, but if we've changed from tiled to untiled real havoc happened: The tiling stride is stored -1 in the fence register, so a stride of 0 resulted in all 1s in the top 32bits, and so a completely bogus fence spanning everything from the start of the object to the top of the GTT. The tell-tale in the register dumps looks like: FENCE START 2: 0x0214d001 FENCE END 2: 0xf3ff Bit 11 isn't set since the hw doesn't store it, even when writing all 1s (at least on my snb here). To prevent such a gaffle in the future add a sanity check for fences with an untiled object attached in i915_gem_write_fence. v2: Fix the WARN, spotted by Chris. v3: Trying to reuse get_fences looked ugly and obfuscated the code. Instead reuse update_fence and to make it really dtrt also move the fence dirty state clearing into update_fence. Cc: Chris Wilson ch...@chris-wilson.co.uk Cc: Stéphane Marchesin marc...@chromium.org Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=60530 Cc: sta...@vger.kernel.org (for 3.10 only) Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Sigh. I thought we were covered because before anything accessed this dirty object, the fence would have been rewritten. However, Daniel correctly points out that the stride==0 fence clobbers the entire GTT and so may be used by the hardware in preference to any other fence. Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] [v5] drm/i915: Create VMAs
On Tue, 2013-07-16 at 16:50 -0700, Ben Widawsky wrote:
Formerly: drm/i915: Create VMAs (part 1)
In a previous patch, the notion of a VM was introduced. A VMA describes
an area of part of the VM address space. A VMA is similar to the concept
in the linux mm. However, instead of representing regular memory, a VMA
is backed by a GEM BO. There may be many VMAs for a given object, one
for each VM the object is to be used in. This may occur through flink,
dma-buf, or a number of other transient states.
Currently the code depends on only 1 VMA per object, for the global GTT
(and aliasing PPGTT). The following patches will address this and make
the rest of the infrastructure more suited
v2: s/i915_obj/i915_gem_obj (Chris)
v3: Only move an object to the now global unbound list if there are no
more VMAs for the object which are bound into a VM (ie. the list is
empty).
v4: killed obj-gtt_space
some reworks due to rebase
v5: Free vma on error path (Imre)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_drv.h| 48 ++-
drivers/gpu/drm/i915/i915_gem.c| 71
+++---
drivers/gpu/drm/i915/i915_gem_evict.c | 12 --
drivers/gpu/drm/i915/i915_gem_gtt.c| 5 ++-
drivers/gpu/drm/i915/i915_gem_stolen.c | 14 +--
5 files changed, 118 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b3ba428..1a32412 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -533,6 +533,17 @@ struct i915_hw_ppgtt {
int (*enable)(struct drm_device *dev);
};
+/* To make things as simple as possible (ie. no refcounting), a VMA's
lifetime
+ * will always be = an objects lifetime. So object refcounting should cover
us.
+ */
+struct i915_vma {
+ struct drm_mm_node node;
+ struct drm_i915_gem_object *obj;
+ struct i915_address_space *vm;
+
+ struct list_head vma_link; /* Link in the object's VMA list */
+};
+
struct i915_ctx_hang_stats {
/* This context had batch pending when hang was declared */
unsigned batch_pending;
@@ -1229,8 +1240,9 @@ struct drm_i915_gem_object {
const struct drm_i915_gem_object_ops *ops;
- /** Current space allocated to this object in the GTT, if any. */
- struct drm_mm_node gtt_space;
+ /** List of VMAs backed by this object */
+ struct list_head vma_list;
+
/** Stolen memory for this object, instead of being backed by shmem. */
struct drm_mm_node *stolen;
struct list_head global_list;
@@ -1356,18 +1368,32 @@ struct drm_i915_gem_object {
#define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
-/* Offset of the first PTE pointing to this object */
-static inline unsigned long
-i915_gem_obj_ggtt_offset(struct drm_i915_gem_object *o)
+/* This is a temporary define to help transition us to real VMAs. If you see
+ * this, you're either reviewing code, or bisecting it. */
+static inline struct i915_vma *
+__i915_gem_obj_to_vma(struct drm_i915_gem_object *obj)
{
- return o-gtt_space.start;
+ if (list_empty(obj-vma_list))
+ return NULL;
+ return list_first_entry(obj-vma_list, struct i915_vma, vma_link);
}
/* Whether or not this object is currently mapped by the translation tables
*/
static inline bool
i915_gem_obj_ggtt_bound(struct drm_i915_gem_object *o)
{
- return drm_mm_node_allocated(o-gtt_space);
+ struct i915_vma *vma = __i915_gem_obj_to_vma(o);
+ if (vma == NULL)
+ return false;
+ return drm_mm_node_allocated(vma-node);
+}
+
+/* Offset of the first PTE pointing to this object */
+static inline unsigned long
+i915_gem_obj_ggtt_offset(struct drm_i915_gem_object *o)
+{
+ BUG_ON(list_empty(o-vma_list));
+ return __i915_gem_obj_to_vma(o)-node.start;
}
/* The size used in the translation tables may be larger than the actual
size of
@@ -1377,14 +1403,15 @@ i915_gem_obj_ggtt_bound(struct drm_i915_gem_object *o)
static inline unsigned long
i915_gem_obj_ggtt_size(struct drm_i915_gem_object *o)
{
- return o-gtt_space.size;
+ BUG_ON(list_empty(o-vma_list));
+ return __i915_gem_obj_to_vma(o)-node.size;
}
static inline void
i915_gem_obj_ggtt_set_color(struct drm_i915_gem_object *o,
enum i915_cache_level color)
{
- o-gtt_space.color = color;
+ __i915_gem_obj_to_vma(o)-node.color = color;
}
/**
@@ -1691,6 +1718,9 @@ void i915_gem_object_init(struct drm_i915_gem_object
*obj,
struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
size_t size);
void i915_gem_free_object(struct drm_gem_object *obj);
+struct i915_vma *i915_gem_vma_create(struct drm_i915_gem_object *obj,
+ struct
Re: [Intel-gfx] [PATCH v2] drm/i915: fix long-standing SNB regression in power consumption after resume
On Wed, 17 Jul 2013 10:22:58 +0400 Konstantin Khlebnikov khlebni...@openvz.org wrote: This patch fixes regression in power consumtion of sandy bridge gpu, which exists since v3.6 Sometimes after resuming from s2ram gpu starts thinking that it's extremely busy. After that it never reaches rc6 state. Bug exists since kernel v3.6, commit b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 (drm/i915: load boot context at driver init time). For some reason RC6 is already enabled at the beginning of resuming process. Following initliaztion breaks some internal state and confuses RPS engine. This patch disables RC6 at the beginnig of resume and initialization. I've rearranged initialization sequence, because intel_disable_gt_powersave() needs initialized force_wake_get/put and some locks from the dev_priv. References: https://bugs.freedesktop.org/show_bug.cgi?id=54089 References: https://bugzilla.kernel.org/show_bug.cgi?id=58971 Signed-off-by: Konstantin Khlebnikov khlebni...@openvz.org Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Chris Wilson ch...@chris-wilson.co.uk Cc: Jesse Barnes jbar...@virtuousgeek.org --- My hero! So the later init change didn't work? Either way, great to have this fix in the tree... thanks again. -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] build: Depend on cairo 1.12.0 for CAIRO_FORMAT_RGB30
We've somewhat recently added RGB30 support to testdisplay, but we need cairo 1.12.0 for that. Barf early. Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index f65942f..2eba12a 100644 --- a/configure.ac +++ b/configure.ac @@ -74,7 +74,7 @@ PKG_CHECK_MODULES(DRM, [libdrm_intel = 2.4.38 libdrm]) PKG_CHECK_MODULES(PCIACCESS, [pciaccess = 0.10]) # for testdisplay -PKG_CHECK_MODULES(CAIRO, cairo) +PKG_CHECK_MODULES(CAIRO, [cairo = 1.12.0]) PKG_CHECK_MODULES(LIBUDEV, [libudev], [udev=yes], [udev=no]) if test x$udev = xyes; then AC_DEFINE(HAVE_UDEV,1,[Enable udev-based monitor hotplug detection]) -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: correctly restore fences with objects attached
On Wed, 17 Jul 2013 14:51:28 +0200
Daniel Vetter daniel.vet...@ffwll.ch wrote:
+ if (reg-obj) {
+ i915_gem_object_update_fence(reg-obj, reg,
+ reg-obj-tiling_mode);
+ } else {
+ i915_gem_write_fence(dev, i, NULL);
+ }
Why do we look at reg-obj here? Can it be NULL? Or would
reg-obj-tiling_mode != I915_TILING_NONE do the same thing?
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Extend i915_powersave parameter.
when playing with fbc and psr I noticed that this flag was underused
so I decided to extend it a bit...
I know that the force enable made it more difficult to implement along
with individual parameters, but I sent this patch as a startup for the
discussion...
please let me know if you have better ideas...
On Wed, Jul 17, 2013 at 12:36 PM, Rodrigo Vivi rodrigo.v...@gmail.com wrote:
i915_powersave was already an umbrella for disabling downclocking and fbc.
Now on it is extended to also force enabling them.
Also more powersavings features has been added under it: RC6 and IPS.
In the future it can cover more powersavings features like
PSR, Slice Shutdown, etc. So this will be the easiest path to disable or
enable all of them together.
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
drivers/gpu/drm/i915/i915_drv.c | 9 +++--
drivers/gpu/drm/i915/intel_display.c | 9 +
drivers/gpu/drm/i915/intel_pm.c | 13 ++---
3 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b07362f..e4fb431 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -53,10 +53,15 @@ MODULE_PARM_DESC(panel_ignore_lid,
Override lid status (0=autodetect, 1=autodetect disabled
[default],
-1=force lid closed, -2=force lid open));
-unsigned int i915_powersave __read_mostly = 1;
+unsigned int i915_powersave __read_mostly = -1;
module_param_named(powersave, i915_powersave, int, 0600);
MODULE_PARM_DESC(powersave,
- Enable powersavings, fbc, downclocking, etc. (default:
true));
+ Force Enable/Disable powersavings,
+ ignoring individual parameters when available.
+ Features covered: downclocking, fbc, rc6 and ips
+ 0 = forcibly disables all powersavings features
+ 1 = forcibly enables all powersavings features
+ default: -1 (use per-feature default/parameter));
int i915_semaphores __read_mostly = -1;
module_param_named(semaphores, i915_semaphores, int, 0600);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index f53359b..e84a7d9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4089,7 +4089,8 @@ retry:
static void hsw_compute_ips_config(struct intel_crtc *crtc,
struct intel_crtc_config *pipe_config)
{
- pipe_config-ips_enabled = i915_enable_ips
+ pipe_config-ips_enabled = i915_powersave != 0
+ (i915_powersave == 1 || i915_enable_ips)
hsw_crtc_supports_ips(crtc)
pipe_config-pipe_bpp == 24;
}
@@ -4329,7 +4330,7 @@ static void i9xx_update_pll_dividers(struct intel_crtc
*crtc,
crtc-lowfreq_avail = false;
if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_LVDS)
- reduced_clock i915_powersave) {
+ reduced_clock i915_powersave == 0) {
I915_WRITE(FP1(pipe), fp2);
crtc-config.dpll_hw_state.fp1 = fp2;
crtc-lowfreq_avail = true;
@@ -7156,7 +7157,7 @@ void intel_mark_idle(struct drm_device *dev)
{
struct drm_crtc *crtc;
- if (!i915_powersave)
+ if (i915_powersave == 0)
return;
list_for_each_entry(crtc, dev-mode_config.crtc_list, head) {
@@ -7173,7 +7174,7 @@ void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
struct drm_device *dev = obj-base.dev;
struct drm_crtc *crtc;
- if (!i915_powersave)
+ if (i915_powersave == 0)
return;
list_for_each_entry(crtc, dev-mode_config.crtc_list, head) {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 43031ec..405b475 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -452,9 +452,6 @@ void intel_update_fbc(struct drm_device *dev)
struct drm_i915_gem_object *obj;
unsigned int max_hdisplay, max_vdisplay;
- if (!i915_powersave)
- return;
-
if (!I915_HAS_FBC(dev))
return;
@@ -491,13 +488,13 @@ void intel_update_fbc(struct drm_device *dev)
intel_fb = to_intel_framebuffer(fb);
obj = intel_fb-obj;
- if (i915_enable_fbc 0
+ if (i915_enable_fbc 0 i915_powersave 0
INTEL_INFO(dev)-gen = 7 !IS_HASWELL(dev)) {
DRM_DEBUG_KMS(disabled per chip default\n);
dev_priv-fbc.no_fbc_reason = FBC_CHIP_DEFAULT;
goto out_disable;
}
- if (!i915_enable_fbc) {
+ if (!i915_enable_fbc || i915_powersave == 0) {
DRM_DEBUG_KMS(fbc disabled per module param\n);
Re: [Intel-gfx] [PATCH] drm/i915: Extend i915_powersave parameter.
On Wed, Jul 17, 2013 at 12:36:12PM -0300, Rodrigo Vivi wrote:
i915_powersave was already an umbrella for disabling downclocking and fbc.
Now on it is extended to also force enabling them.
Also more powersavings features has been added under it: RC6 and IPS.
In the future it can cover more powersavings features like
PSR, Slice Shutdown, etc. So this will be the easiest path to disable or
enable all of them together.
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
drivers/gpu/drm/i915/i915_drv.c | 9 +++--
drivers/gpu/drm/i915/intel_display.c | 9 +
drivers/gpu/drm/i915/intel_pm.c | 13 ++---
3 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b07362f..e4fb431 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -53,10 +53,15 @@ MODULE_PARM_DESC(panel_ignore_lid,
Override lid status (0=autodetect, 1=autodetect disabled
[default],
-1=force lid closed, -2=force lid open));
-unsigned int i915_powersave __read_mostly = 1;
+unsigned int i915_powersave __read_mostly = -1;
module_param_named(powersave, i915_powersave, int, 0600);
MODULE_PARM_DESC(powersave,
- Enable powersavings, fbc, downclocking, etc. (default: true));
+ Force Enable/Disable powersavings,
+ ignoring individual parameters when available.
+ Features covered: downclocking, fbc, rc6 and ips
+ 0 = forcibly disables all powersavings features
+ 1 = forcibly enables all powersavings features
+ default: -1 (use per-feature default/parameter));
int i915_semaphores __read_mostly = -1;
module_param_named(semaphores, i915_semaphores, int, 0600);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index f53359b..e84a7d9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4089,7 +4089,8 @@ retry:
static void hsw_compute_ips_config(struct intel_crtc *crtc,
struct intel_crtc_config *pipe_config)
{
- pipe_config-ips_enabled = i915_enable_ips
+ pipe_config-ips_enabled = i915_powersave != 0
+(i915_powersave == 1 || i915_enable_ips)
hsw_crtc_supports_ips(crtc)
pipe_config-pipe_bpp == 24;
}
@@ -4329,7 +4330,7 @@ static void i9xx_update_pll_dividers(struct intel_crtc
*crtc,
crtc-lowfreq_avail = false;
if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_LVDS)
- reduced_clock i915_powersave) {
+ reduced_clock i915_powersave == 0) {
Sense inverted.
@@ -452,9 +452,6 @@ void intel_update_fbc(struct drm_device *dev)
struct drm_i915_gem_object *obj;
unsigned int max_hdisplay, max_vdisplay;
- if (!i915_powersave)
- return;
-
if (!I915_HAS_FBC(dev))
return;
@@ -491,13 +488,13 @@ void intel_update_fbc(struct drm_device *dev)
intel_fb = to_intel_framebuffer(fb);
obj = intel_fb-obj;
- if (i915_enable_fbc 0
+ if (i915_enable_fbc 0 i915_powersave 0
INTEL_INFO(dev)-gen = 7 !IS_HASWELL(dev)) {
DRM_DEBUG_KMS(disabled per chip default\n);
dev_priv-fbc.no_fbc_reason = FBC_CHIP_DEFAULT;
goto out_disable;
}
- if (!i915_enable_fbc) {
+ if (!i915_enable_fbc || i915_powersave == 0) {
This is unreadable. Split it up like intel_enable_rc6() and make it
verbose.
DRM_DEBUG_KMS(fbc disabled per module param\n);
dev_priv-fbc.no_fbc_reason = FBC_MODULE_PARAM;
goto out_disable;
@@ -3171,12 +3168,14 @@ int intel_enable_rc6(const struct drm_device *dev)
if (INTEL_INFO(dev)-gen 5)
return 0;
- /* Respect the kernel parameter if it is set */
+ /* Respect the kernel parameters if they are set */
+ if (i915_powersave == 0)
+ return 0;
Semantics broken here. Force enabling a specific tunable such as rc6
should override a default powersave.
if (i915_enable_rc6 = 0)
return i915_enable_rc6;
/* Disable RC6 on Ironlake */
- if (INTEL_INFO(dev)-gen == 5)
+ if (INTEL_INFO(dev)-gen == 5 i915_powersave 0)
return 0;
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: correctly restore fences with objects attached
On Wed, Jul 17, 2013 at 08:34:48AM -0700, Jesse Barnes wrote:
On Wed, 17 Jul 2013 14:51:28 +0200
Daniel Vetter daniel.vet...@ffwll.ch wrote:
+ if (reg-obj) {
+ i915_gem_object_update_fence(reg-obj, reg,
+reg-obj-tiling_mode);
+ } else {
+ i915_gem_write_fence(dev, i, NULL);
+ }
Why do we look at reg-obj here? Can it be NULL? Or would
reg-obj-tiling_mode != I915_TILING_NONE do the same thing?
Yes. This is the array of fence registers, irrespective of whether an
object is using one.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] Fix backlight issues on some Windows 8 systems
On Sat, Jun 22, 2013 at 4:46 PM, Yves-Alexis Perez cor...@debian.org wrote: On dim., 2013-06-09 at 19:01 -0400, Matthew Garrett wrote: The first two patches in this series are picked from other patchesets aimed at solving similar problems. The last simply unregisters ACPI backlight control on Windows 8 systems when using an Intel GPU. Similar code could be added to other drivers, but I'm reluctant to do so without further investigation as to the behaviour of the vendor drivers under Windows. Hi, I've read this thread coming from https://bugzilla.kernel.org/show_bug.cgi?id=51231 and tried the patches on a Lenovo ThinkPad X230 with intel graphics. The problem with thoses fixes is that they still introduce a regression in how the brightness is handled, at least for me. For me too. Before Linux support for acpi_osi(Windows 2012) (and when booting with acpi_osi=!Windows 2012), brightness keys were handled by the kernel just fine, whether in console, in the display manager or in my desktop environment (Xfce). xfce4-power-manager just needs to be told that the brightness keys are already handled and it doesn't need to do anything. How do you tell xfce4-power-manager that? For me everything works fine when acpi_osi=!Windows 2012, which is why I wrote a patch for this particular laptop. http://article.gmane.org/gmane.linux.acpi.devel/60969 So can the previous behavior be actually restored? It *should*. The #1 rule of the Linux kernel is to never ever break user-space, isn't it? Please keep me on CC: on replies, I'm not subscribed to the various lists. You don't need to ask that in mailing lists that don't have reply-to munged (sane ones), and vger ones don't. Cheers. -- Felipe Contreras ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [drm-intel:drm-intel-fixes 52/52] ERROR: Unrecognized email address: 'stable.]'
On Wed, 2013-07-17 at 11:29 +0200, Daniel Vetter wrote: On Wed, Jul 17, 2013 at 11:10 AM, Fengguang Wu fengguang...@intel.com wrote: FYI, there are new warnings show up in tree: git://people.freedesktop.org/~danvet/drm-intel.git drm-intel-fixes head: 71e4092e52499ec74bc1dec0f883b15f2c424ec5 commit: 71e4092e52499ec74bc1dec0f883b15f2c424ec5 [52/52] drm/i915: fix long-standing SNB regression in power consumption after resume v2 scripts/checkpatch.pl 0001-drm-i915-fix-long-standing-SNB-regression-in-power-c.patch ERROR: Unrecognized email address: 'stable.]' #41: cc: stable.] Well, I've added that while applying the patch - I tend to smash maintainer notes into the sob section and word-wraping caused the cc: stable remark to be parsed. Is there an officially sanctioned way for such notes that appeases checkpatch? Adding lkml and checkpatch maintainer. -Daniel (It would have been nice to get the content that failes instead of having to pull the tree) Don't wrap text to start a line with cc: Cc: Chris Wilson ch...@chris-wilson.co.uk Cc: Jesse Barnes jbar...@virtuousgeek.org [danvet: Add note about v1 vs. v2 of this patch and use standard layout for the commit citation. Also add the tested-bys from v1 and a cc: stable.] Cc: sta...@vger.kernel.org (Note: tiny conflict due to the addition of You could have added something like: [danvet: Add note about v1 vs. v2 of this patch and use standard layout for the commit citation. Also add the tested-bys from v1 and a cc: stable.] ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] build: Depend on cairo 1.12.0 for CAIRO_FORMAT_RGB30
On Wed, Jul 17, 2013 at 04:25:29PM +0100, Damien Lespiau wrote: We've somewhat recently added RGB30 support to testdisplay, but we need cairo 1.12.0 for that. Barf early. Signed-off-by: Damien Lespiau damien.lesp...@intel.com Applied, thanks for the patch. -Daniel --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index f65942f..2eba12a 100644 --- a/configure.ac +++ b/configure.ac @@ -74,7 +74,7 @@ PKG_CHECK_MODULES(DRM, [libdrm_intel = 2.4.38 libdrm]) PKG_CHECK_MODULES(PCIACCESS, [pciaccess = 0.10]) # for testdisplay -PKG_CHECK_MODULES(CAIRO, cairo) +PKG_CHECK_MODULES(CAIRO, [cairo = 1.12.0]) PKG_CHECK_MODULES(LIBUDEV, [libudev], [udev=yes], [udev=no]) if test x$udev = xyes; then AC_DEFINE(HAVE_UDEV,1,[Enable udev-based monitor hotplug detection]) -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: allow root to submit secure buffers even if !master
This should allow userland tools running under X to submit secure
batches for various things. This gives master DRM clients slightly more
permissions, but doesn't give regular processes any more, since a root
process can already map the registers directly and poke at hw.
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1b58694..377aa1f 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -858,10 +858,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
flags = 0;
if (args-flags I915_EXEC_SECURE) {
- if (!file-is_master || !capable(CAP_SYS_ADMIN))
- return -EPERM;
-
- flags |= I915_DISPATCH_SECURE;
+ if (file-is_master || capable(CAP_SYS_ADMIN))
+ flags |= I915_DISPATCH_SECURE;
+ else
+ return -EPERM;
}
if (args-flags I915_EXEC_IS_PINNED)
flags |= I915_DISPATCH_PINNED;
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/11] drm/i915: Enable/Disable PSR
2013/7/11 Rodrigo Vivi rodrigo.v...@gmail.com:
Adding Enable and Disable PSR functionalities. This includes setting the
PSR configuration over AUX, sending SDP VSC DIP over the eDP PIPE config,
enabling PSR in the sink via DPCD register and finally enabling PSR on
the host.
This patch is based on initial PSR code by Sateesh Kavuri and Kumar Shobhit
but in a different implementation.
v2: * moved functions around and changed its names.
* removed VSC DIP unset from disable.
* remove FBC wa.
* don't mask LSPS anymore.
* incorporate new crtc usage after a rebase.
v3: Make a clear separation between Sink (Panel) and Source (HW) enabling.
v4: Fix identation and other style issues raised by checkpatch (by Paulo).
v5: Changes according to Paulo's review:
static on write_vsc;
avoid using dp_to_dev when already calling dp_to_dig_port;
remove unecessary TP default time setting;
remove unecessary interrupts disabling;
remove unecessary wait_for_vblank when disabling psr;
v6: remove unecessary wait_for_vblank when writing vsc;
v7: adding setup once function to avoid unnecessarily write to vsc
and set debug_ctl every time we enable or disable psr.
Cc: Paulo Zanoni paulo.r.zan...@intel.com
Credits-by: Sateesh Kavuri sateesh.kav...@intel.com
Credits-by: Shobhit Kumar shobhit.ku...@intel.com
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
drivers/gpu/drm/i915/i915_reg.h | 42 +++
drivers/gpu/drm/i915/intel_dp.c | 149
+++
drivers/gpu/drm/i915/intel_drv.h | 4 ++
3 files changed, 195 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dc3d6a7..31e4dbb 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1779,6 +1779,47 @@
#define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B)
#define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B)
+/* HSW eDP PSR registers */
+#define EDP_PSR_CTL0x64800
+#define EDP_PSR_ENABLE (131)
+#define EDP_PSR_LINK_DISABLE (027)
+#define EDP_PSR_LINK_STANDBY (127)
+#define EDP_PSR_MIN_LINK_ENTRY_TIME_MASK (325)
+#define EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES (025)
+#define EDP_PSR_MIN_LINK_ENTRY_TIME_4_LINES (125)
+#define EDP_PSR_MIN_LINK_ENTRY_TIME_2_LINES (225)
+#define EDP_PSR_MIN_LINK_ENTRY_TIME_0_LINES (325)
+#define EDP_PSR_MAX_SLEEP_TIME_SHIFT 20
+#define EDP_PSR_SKIP_AUX_EXIT(112)
+#define EDP_PSR_TP1_TP2_SEL (011)
+#define EDP_PSR_TP1_TP3_SEL (111)
+#define EDP_PSR_TP2_TP3_TIME_500us (08)
+#define EDP_PSR_TP2_TP3_TIME_100us (18)
+#define EDP_PSR_TP2_TP3_TIME_2500us (28)
+#define EDP_PSR_TP2_TP3_TIME_0us (38)
+#define EDP_PSR_TP1_TIME_500us (04)
+#define EDP_PSR_TP1_TIME_100us (14)
+#define EDP_PSR_TP1_TIME_2500us (24)
+#define EDP_PSR_TP1_TIME_0us (34)
+#define EDP_PSR_IDLE_FRAME_SHIFT 0
+
+#define EDP_PSR_AUX_CTL0x64810
+#define EDP_PSR_AUX_DATA1 0x64814
+#define EDP_PSR_DPCD_COMMAND 0x8006
+#define EDP_PSR_AUX_DATA2 0x64818
+#define EDP_PSR_DPCD_NORMAL_OPERATION(124)
+#define EDP_PSR_AUX_DATA3 0x6481c
+#define EDP_PSR_AUX_DATA4 0x64820
+#define EDP_PSR_AUX_DATA5 0x64824
+
+#define EDP_PSR_STATUS_CTL 0x64840
+#define EDP_PSR_STATUS_STATE_MASK(729)
+
+#define EDP_PSR_DEBUG_CTL 0x64860
+#define EDP_PSR_DEBUG_MASK_LPSP (127)
+#define EDP_PSR_DEBUG_MASK_MEMUP (126)
+#define EDP_PSR_DEBUG_MASK_HPD (125)
+
/* VGA port control */
#define ADPA 0x61100
#define PCH_ADPA0xe1100
@@ -2048,6 +2089,7 @@
* (Haswell and newer) to see which VIDEO_DIP_DATA byte corresponds to each
byte
* of the infoframe structure specified by CEA-861. */
#define VIDEO_DIP_DATA_SIZE 32
+#define VIDEO_DIP_VSC_DATA_SIZE 36
#define VIDEO_DIP_CTL 0x61170
/* Pre HSW: */
#define VIDEO_DIP_ENABLE (1 31)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d273e36..d4b52a9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1383,6 +1383,153 @@ static bool is_edp_psr(struct intel_dp *intel_dp)
intel_dp-psr_dpcd[0] DP_PSR_IS_SUPPORTED;
}
+static bool intel_edp_is_psr_enabled(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (!IS_HASWELL(dev))
+ return false;
+
+ return I915_READ(EDP_PSR_CTL) EDP_PSR_ENABLE;
+}
+
Re: [Intel-gfx] [PATCH 06/11] drm/i915: Match all PSR mode entry conditions before enabling it.
2013/7/11 Rodrigo Vivi rodrigo.v...@gmail.com:
v2: Prefer seq_puts to seq_printf by Paulo Zanoni.
v3: small changes like avoiding calling dp_to_dig_port twice as noticed by
Paulo Zanoni.
v4: Avoiding reading non-existent registers - noticed by Paulo
on first psr debugfs patch.
v5: Accepting more suggestions from Paulo:
* check sw interlace flag instead of i915_read
* introduce PSR_S3D_ENABLED to avoid forgeting it whenever added.
Cc: Paulo Zanoni paulo.r.zan...@intel.com
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 44 ++
drivers/gpu/drm/i915/i915_drv.h | 13 +++
drivers/gpu/drm/i915/i915_reg.h | 7
drivers/gpu/drm/i915/intel_dp.c | 74
-
4 files changed, 130 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index fe3cd5a..e679968 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1948,17 +1948,47 @@ static int i915_edp_psr_status(struct seq_file *m,
void *data)
struct drm_info_node *node = m-private;
struct drm_device *dev = node-minor-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
- u32 psrctl, psrstat, psrperf;
+ u32 psrstat, psrperf;
- if (!IS_HASWELL(dev)) {
- seq_puts(m, PSR not supported on this platform\n);
+ if (IS_HASWELL(dev) I915_READ(EDP_PSR_CTL) EDP_PSR_ENABLE) {
+ seq_puts(m, PSR enabled\n);
+ } else {
If we're not Haswell we fall on this else case, then we print PSR
disabled and look at no_psr_reason.
With that fixed: Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
+ seq_puts(m, PSR disabled: );
+ switch (dev_priv-no_psr_reason) {
+ case PSR_NO_SOURCE:
+ seq_puts(m, not supported on this platform);
+ break;
+ case PSR_NO_SINK:
+ seq_puts(m, not supported by panel);
+ break;
+ case PSR_CRTC_NOT_ACTIVE:
+ seq_puts(m, crtc not active);
+ break;
+ case PSR_PWR_WELL_ENABLED:
+ seq_puts(m, power well enabled);
+ break;
+ case PSR_NOT_TILED:
+ seq_puts(m, not tiled);
+ break;
+ case PSR_SPRITE_ENABLED:
+ seq_puts(m, sprite enabled);
+ break;
+ case PSR_S3D_ENABLED:
+ seq_puts(m, stereo 3d enabled);
+ break;
+ case PSR_INTERLACED_ENABLED:
+ seq_puts(m, interlaced enabled);
+ break;
+ case PSR_HSW_NOT_DDIA:
+ seq_puts(m, HSW ties PSR to DDI A (eDP));
+ break;
+ default:
+ seq_puts(m, unknown reason);
+ }
+ seq_puts(m, \n);
return 0;
}
- psrctl = I915_READ(EDP_PSR_CTL);
- seq_printf(m, PSR Enabled: %s\n,
- yesno(psrctl EDP_PSR_ENABLE));
-
psrstat = I915_READ(EDP_PSR_STATUS_CTL);
seq_puts(m, PSR Current State: );
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 842aada..d0b9483 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -560,6 +560,17 @@ struct i915_fbc {
} no_fbc_reason;
};
+enum no_psr_reason {
+ PSR_NO_SOURCE, /* Not supported on platform */
+ PSR_NO_SINK, /* Not supported by panel */
+ PSR_CRTC_NOT_ACTIVE,
+ PSR_PWR_WELL_ENABLED,
+ PSR_NOT_TILED,
+ PSR_SPRITE_ENABLED,
+ PSR_S3D_ENABLED,
+ PSR_INTERLACED_ENABLED,
+ PSR_HSW_NOT_DDIA,
+};
enum intel_pch {
PCH_NONE = 0, /* No PCH present */
@@ -1161,6 +1172,8 @@ typedef struct drm_i915_private {
/* Haswell power well */
struct i915_power_well power_well;
+ enum no_psr_reason no_psr_reason;
+
struct i915_gpu_error gpu_error;
struct drm_i915_gem_object *vlv_pctx;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b328ec6..3bca337 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4150,6 +4150,13 @@
#define HSW_TVIDEO_DIP_VSC_DATA(trans) \
_TRANSCODER(trans, HSW_VIDEO_DIP_VSC_DATA_A,
HSW_VIDEO_DIP_VSC_DATA_B)
+#define HSW_STEREO_3D_CTL_A0x70020
+#define S3D_ENABLE (131)
+#define HSW_STEREO_3D_CTL_B0x71020
+
+#define HSW_STEREO_3D_CTL(trans) \
+ _TRANSCODER(trans, HSW_STEREO_3D_CTL_A, HSW_STEREO_3D_CTL_A)
+
Re: [Intel-gfx] [PATCH 07/11] drm/i915: add update function to disable/enable-back PSR
2013/7/11 Rodrigo Vivi rodrigo.v...@gmail.com:
Required function to disable PSR when going to console mode.
But also can be used whenever PSR mode entry conditions changed.
v2: Add it before PSR Hook. Update function not really been called yet.
v3: Fix coding style detected by checkpatch by Paulo Zanoni.
v4: do_enable must be static as Paulo noticed.
Cc: Paulo Zanoni paulo.r.zan...@intel.com
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
Even though we can improve the loops and other things with
pipe_config, this patch seems to work, so:
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
We can still do the reworks later.
---
drivers/gpu/drm/i915/intel_dp.c | 31 ++-
drivers/gpu/drm/i915/intel_drv.h | 1 +
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c0bd887..c0870a69 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1568,7 +1568,7 @@ static bool intel_edp_psr_match_conditions(struct
intel_dp *intel_dp)
return true;
}
-void intel_edp_psr_enable(struct intel_dp *intel_dp)
+static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -1586,6 +1586,15 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp)
intel_edp_psr_enable_source(intel_dp);
}
+void intel_edp_psr_enable(struct intel_dp *intel_dp)
+{
+ struct drm_device *dev = intel_dp_to_dev(intel_dp);
+
+ if (intel_edp_psr_match_conditions(intel_dp)
+ !intel_edp_is_psr_enabled(dev))
+ intel_edp_psr_do_enable(intel_dp);
+}
+
void intel_edp_psr_disable(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -1602,6 +1611,26 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp)
DRM_ERROR(Timed out waiting for PSR Idle State\n);
}
+void intel_edp_psr_update(struct drm_device *dev)
+{
+ struct intel_encoder *encoder;
+ struct intel_dp *intel_dp = NULL;
+
+ list_for_each_entry(encoder, dev-mode_config.encoder_list,
base.head)
+ if (encoder-type == INTEL_OUTPUT_EDP) {
+ intel_dp = enc_to_intel_dp(encoder-base);
+
+ if (!is_edp_psr(intel_dp))
+ return;
+
+ if (!intel_edp_psr_match_conditions(intel_dp))
+ intel_edp_psr_disable(intel_dp);
+ else
+ if (!intel_edp_is_psr_enabled(dev))
+ intel_edp_psr_do_enable(intel_dp);
+ }
+}
+
static void intel_disable_dp(struct intel_encoder *encoder)
{
struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base);
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index ff36a40..40e955d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -837,5 +837,6 @@ extern bool intel_set_pch_fifo_underrun_reporting(struct
drm_device *dev,
extern void intel_edp_psr_enable(struct intel_dp *intel_dp);
extern void intel_edp_psr_disable(struct intel_dp *intel_dp);
+extern void intel_edp_psr_update(struct drm_device *dev);
#endif /* __INTEL_DRV_H__ */
--
1.7.11.7
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Extend i915_powersave parameter.
On Wed, Jul 17, 2013 at 12:46 PM, Chris Wilson ch...@chris-wilson.co.uk wrote:
On Wed, Jul 17, 2013 at 12:36:12PM -0300, Rodrigo Vivi wrote:
i915_powersave was already an umbrella for disabling downclocking and fbc.
Now on it is extended to also force enabling them.
Also more powersavings features has been added under it: RC6 and IPS.
In the future it can cover more powersavings features like
PSR, Slice Shutdown, etc. So this will be the easiest path to disable or
enable all of them together.
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
drivers/gpu/drm/i915/i915_drv.c | 9 +++--
drivers/gpu/drm/i915/intel_display.c | 9 +
drivers/gpu/drm/i915/intel_pm.c | 13 ++---
3 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
index b07362f..e4fb431 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -53,10 +53,15 @@ MODULE_PARM_DESC(panel_ignore_lid,
Override lid status (0=autodetect, 1=autodetect disabled
[default],
-1=force lid closed, -2=force lid open));
-unsigned int i915_powersave __read_mostly = 1;
+unsigned int i915_powersave __read_mostly = -1;
module_param_named(powersave, i915_powersave, int, 0600);
MODULE_PARM_DESC(powersave,
- Enable powersavings, fbc, downclocking, etc. (default:
true));
+ Force Enable/Disable powersavings,
+ ignoring individual parameters when available.
+ Features covered: downclocking, fbc, rc6 and ips
+ 0 = forcibly disables all powersavings features
+ 1 = forcibly enables all powersavings features
+ default: -1 (use per-feature default/parameter));
int i915_semaphores __read_mostly = -1;
module_param_named(semaphores, i915_semaphores, int, 0600);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index f53359b..e84a7d9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4089,7 +4089,8 @@ retry:
static void hsw_compute_ips_config(struct intel_crtc *crtc,
struct intel_crtc_config *pipe_config)
{
- pipe_config-ips_enabled = i915_enable_ips
+ pipe_config-ips_enabled = i915_powersave != 0
+(i915_powersave == 1 || i915_enable_ips)
hsw_crtc_supports_ips(crtc)
pipe_config-pipe_bpp == 24;
}
@@ -4329,7 +4330,7 @@ static void i9xx_update_pll_dividers(struct intel_crtc
*crtc,
crtc-lowfreq_avail = false;
if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_LVDS)
- reduced_clock i915_powersave) {
+ reduced_clock i915_powersave == 0) {
Sense inverted.
!= I meant! stupid me! :P
@@ -452,9 +452,6 @@ void intel_update_fbc(struct drm_device *dev)
struct drm_i915_gem_object *obj;
unsigned int max_hdisplay, max_vdisplay;
- if (!i915_powersave)
- return;
-
if (!I915_HAS_FBC(dev))
return;
@@ -491,13 +488,13 @@ void intel_update_fbc(struct drm_device *dev)
intel_fb = to_intel_framebuffer(fb);
obj = intel_fb-obj;
- if (i915_enable_fbc 0
+ if (i915_enable_fbc 0 i915_powersave 0
INTEL_INFO(dev)-gen = 7 !IS_HASWELL(dev)) {
DRM_DEBUG_KMS(disabled per chip default\n);
dev_priv-fbc.no_fbc_reason = FBC_CHIP_DEFAULT;
goto out_disable;
}
- if (!i915_enable_fbc) {
+ if (!i915_enable_fbc || i915_powersave == 0) {
This is unreadable. Split it up like intel_enable_rc6() and make it
verbose.
Yes, I know.. even worse on ips case :(
One idea Paulo had is to change i915_enable_fbc based on
i915_powersave and put a debug message.
DRM_DEBUG_KMS(fbc disabled per module param\n);
dev_priv-fbc.no_fbc_reason = FBC_MODULE_PARAM;
goto out_disable;
@@ -3171,12 +3168,14 @@ int intel_enable_rc6(const struct drm_device *dev)
if (INTEL_INFO(dev)-gen 5)
return 0;
- /* Respect the kernel parameter if it is set */
+ /* Respect the kernel parameters if they are set */
+ if (i915_powersave == 0)
+ return 0;
Semantics broken here. Force enabling a specific tunable such as rc6
should override a default powersave.
As I said this is just a start point for discussion.
So if I understood correctly you suggest that if we have
i915_powersave=0 and i915_enable_rc6=1 we should enable rc6?
This is not how it is implemented nowadays on fbc right now... And
this lead me to use pwoesave as an umbrella for force disable/enable
ignoring individual parameters. But I'm open to do the other way
around as long as we have a standardized behavior.
So, we can either respect individual parameters when they
Re: [Intel-gfx] Intel Crash
I updated and all seems to be fine, thanks 2013/7/17 Chris Wilson ch...@chris-wilson.co.uk On Tue, Jul 16, 2013 at 09:51:43PM -0600, Oscar Dario Trujillo Tejada wrote: I send what you want(I guess), online and as attach [1]Xorg Log Found it, we were requesting a upload buffer of size 0, due to using a bad alignment value as a result of setting a bad default value for unknown cpu cache size. Fixed by commit 7f76a2bf319f59d463a1f96974b03d7c651847dd Author: Chris Wilson ch...@chris-wilson.co.uk Date: Wed Jul 17 10:22:17 2013 +0100 sna: Fall back to /proc/cpuinfo parsing if cpuid cache size probe fails -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/6] drm/i915: Free stolen node on failed preallocation
The odds of this happening are *extremely* unlikely.
Reported-by: Imre Deak imre.d...@intel.com
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_gem_stolen.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 46a9715..a893834 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -405,7 +405,7 @@ i915_gem_object_create_stolen_for_preallocated(struct
drm_device *dev,
obj-gtt_space);
if (ret) {
DRM_DEBUG_KMS(failed to allocate stolen GTT space\n);
- goto unref_out;
+ goto err_out;
}
}
@@ -416,7 +416,8 @@ i915_gem_object_create_stolen_for_preallocated(struct
drm_device *dev,
return obj;
-unref_out:
+err_out:
+ drm_mm_put_block(stolen);
drm_gem_object_unreference(obj-base);
return NULL;
}
--
1.8.3.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/6] drm/i915: Create VMAs
Formerly: drm/i915: Create VMAs (part 1)
In a previous patch, the notion of a VM was introduced. A VMA describes
an area of part of the VM address space. A VMA is similar to the concept
in the linux mm. However, instead of representing regular memory, a VMA
is backed by a GEM BO. There may be many VMAs for a given object, one
for each VM the object is to be used in. This may occur through flink,
dma-buf, or a number of other transient states.
Currently the code depends on only 1 VMA per object, for the global GTT
(and aliasing PPGTT). The following patches will address this and make
the rest of the infrastructure more suited
v2: s/i915_obj/i915_gem_obj (Chris)
v3: Only move an object to the now global unbound list if there are no
more VMAs for the object which are bound into a VM (ie. the list is
empty).
v4: killed obj-gtt_space
some reworks due to rebase
v5: Free vma on error path (Imre)
v6: Another missed vma free in i915_gem_object_bind_to_gtt error path
(Imre)
Fixed vma freeing in stolen preallocation (Imre)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_drv.h| 48 +-
drivers/gpu/drm/i915/i915_gem.c| 74 +++---
drivers/gpu/drm/i915/i915_gem_evict.c | 12 --
drivers/gpu/drm/i915/i915_gem_gtt.c| 5 ++-
drivers/gpu/drm/i915/i915_gem_stolen.c | 15 +--
5 files changed, 120 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b3ba428..1a32412 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -533,6 +533,17 @@ struct i915_hw_ppgtt {
int (*enable)(struct drm_device *dev);
};
+/* To make things as simple as possible (ie. no refcounting), a VMA's lifetime
+ * will always be = an objects lifetime. So object refcounting should cover
us.
+ */
+struct i915_vma {
+ struct drm_mm_node node;
+ struct drm_i915_gem_object *obj;
+ struct i915_address_space *vm;
+
+ struct list_head vma_link; /* Link in the object's VMA list */
+};
+
struct i915_ctx_hang_stats {
/* This context had batch pending when hang was declared */
unsigned batch_pending;
@@ -1229,8 +1240,9 @@ struct drm_i915_gem_object {
const struct drm_i915_gem_object_ops *ops;
- /** Current space allocated to this object in the GTT, if any. */
- struct drm_mm_node gtt_space;
+ /** List of VMAs backed by this object */
+ struct list_head vma_list;
+
/** Stolen memory for this object, instead of being backed by shmem. */
struct drm_mm_node *stolen;
struct list_head global_list;
@@ -1356,18 +1368,32 @@ struct drm_i915_gem_object {
#define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
-/* Offset of the first PTE pointing to this object */
-static inline unsigned long
-i915_gem_obj_ggtt_offset(struct drm_i915_gem_object *o)
+/* This is a temporary define to help transition us to real VMAs. If you see
+ * this, you're either reviewing code, or bisecting it. */
+static inline struct i915_vma *
+__i915_gem_obj_to_vma(struct drm_i915_gem_object *obj)
{
- return o-gtt_space.start;
+ if (list_empty(obj-vma_list))
+ return NULL;
+ return list_first_entry(obj-vma_list, struct i915_vma, vma_link);
}
/* Whether or not this object is currently mapped by the translation tables */
static inline bool
i915_gem_obj_ggtt_bound(struct drm_i915_gem_object *o)
{
- return drm_mm_node_allocated(o-gtt_space);
+ struct i915_vma *vma = __i915_gem_obj_to_vma(o);
+ if (vma == NULL)
+ return false;
+ return drm_mm_node_allocated(vma-node);
+}
+
+/* Offset of the first PTE pointing to this object */
+static inline unsigned long
+i915_gem_obj_ggtt_offset(struct drm_i915_gem_object *o)
+{
+ BUG_ON(list_empty(o-vma_list));
+ return __i915_gem_obj_to_vma(o)-node.start;
}
/* The size used in the translation tables may be larger than the actual size
of
@@ -1377,14 +1403,15 @@ i915_gem_obj_ggtt_bound(struct drm_i915_gem_object *o)
static inline unsigned long
i915_gem_obj_ggtt_size(struct drm_i915_gem_object *o)
{
- return o-gtt_space.size;
+ BUG_ON(list_empty(o-vma_list));
+ return __i915_gem_obj_to_vma(o)-node.size;
}
static inline void
i915_gem_obj_ggtt_set_color(struct drm_i915_gem_object *o,
enum i915_cache_level color)
{
- o-gtt_space.color = color;
+ __i915_gem_obj_to_vma(o)-node.color = color;
}
/**
@@ -1691,6 +1718,9 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
size_t size);
void i915_gem_free_object(struct drm_gem_object *obj);
+struct i915_vma *i915_gem_vma_create(struct drm_i915_gem_object *obj,
+struct
Re: [Intel-gfx] [PATCH 0/3] Fix backlight issues on some Windows 8 systems
On mer., 2013-07-17 at 10:51 -0500, Felipe Contreras wrote: On Sat, Jun 22, 2013 at 4:46 PM, Yves-Alexis Perez cor...@debian.org wrote: Before Linux support for acpi_osi(Windows 2012) (and when booting with acpi_osi=!Windows 2012), brightness keys were handled by the kernel just fine, whether in console, in the display manager or in my desktop environment (Xfce). xfce4-power-manager just needs to be told that the brightness keys are already handled and it doesn't need to do anything. How do you tell xfce4-power-manager that? xfconf-query -c xfce4-power-manager -p /xfce4-power-manager/change-brightness-on-key-events -s false You might have to create the key before. See https://bugzilla.xfce.org/show_bug.cgi?id=7541 for more information. Regards, -- Yves-Alexis signature.asc Description: This is a digitally signed message part ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/11] drm/i915: Adding global I915_PARAM for PSR ENABLED.
On Wed, Jul 17, 2013 at 02:46:52PM -0300, Rodrigo Vivi wrote: Hi Chris, could you please review this specific one or give you ack here? Didn't see anything wrong with it. The only caveat I have is that the GETPARAM must be accurate immediately following a setcrtc. If you can guarrantee that is true, you can have my Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk and if Daniel delays, ask him to reserve the PARAM number. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: allow root to submit secure buffers even if !master
On Wed, Jul 17, 2013 at 09:45:30AM -0700, Jesse Barnes wrote:
This should allow userland tools running under X to submit secure
batches for various things. This gives master DRM clients slightly more
permissions, but doesn't give regular processes any more, since a root
process can already map the registers directly and poke at hw.
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1b58694..377aa1f 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -858,10 +858,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
flags = 0;
if (args-flags I915_EXEC_SECURE) {
- if (!file-is_master || !capable(CAP_SYS_ADMIN))
+ if (!f(ile-is_master || capable(CAP_SYS_ADMIN)))
return -EPERM;
Would have made for a much smaller patch.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: allow root to submit secure buffers even if !master
On Wed, 17 Jul 2013 21:20:07 +0100
Chris Wilson ch...@chris-wilson.co.uk wrote:
On Wed, Jul 17, 2013 at 09:45:30AM -0700, Jesse Barnes wrote:
This should allow userland tools running under X to submit secure
batches for various things. This gives master DRM clients slightly more
permissions, but doesn't give regular processes any more, since a root
process can already map the registers directly and poke at hw.
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1b58694..377aa1f 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -858,10 +858,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
flags = 0;
if (args-flags I915_EXEC_SECURE) {
- if (!file-is_master || !capable(CAP_SYS_ADMIN))
+ if (!f(ile-is_master || capable(CAP_SYS_ADMIN)))
return -EPERM;
Would have made for a much smaller patch.
And maybe without the typo? :)
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915: Create VMAs
On Wed, Jul 17, 2013 at 12:19:03PM -0700, Ben Widawsky wrote:
Formerly: drm/i915: Create VMAs (part 1)
In a previous patch, the notion of a VM was introduced. A VMA describes
an area of part of the VM address space. A VMA is similar to the concept
in the linux mm. However, instead of representing regular memory, a VMA
is backed by a GEM BO. There may be many VMAs for a given object, one
for each VM the object is to be used in. This may occur through flink,
dma-buf, or a number of other transient states.
Currently the code depends on only 1 VMA per object, for the global GTT
(and aliasing PPGTT). The following patches will address this and make
the rest of the infrastructure more suited
v2: s/i915_obj/i915_gem_obj (Chris)
v3: Only move an object to the now global unbound list if there are no
more VMAs for the object which are bound into a VM (ie. the list is
empty).
v4: killed obj-gtt_space
some reworks due to rebase
v5: Free vma on error path (Imre)
v6: Another missed vma free in i915_gem_object_bind_to_gtt error path
(Imre)
Fixed vma freeing in stolen preallocation (Imre)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Entire series merged to dinq, thanks a lot for the patches and review. On
to the next step in this journey then!
Cheers, Daniel
---
drivers/gpu/drm/i915/i915_drv.h| 48 +-
drivers/gpu/drm/i915/i915_gem.c| 74
+++---
drivers/gpu/drm/i915/i915_gem_evict.c | 12 --
drivers/gpu/drm/i915/i915_gem_gtt.c| 5 ++-
drivers/gpu/drm/i915/i915_gem_stolen.c | 15 +--
5 files changed, 120 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b3ba428..1a32412 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -533,6 +533,17 @@ struct i915_hw_ppgtt {
int (*enable)(struct drm_device *dev);
};
+/* To make things as simple as possible (ie. no refcounting), a VMA's
lifetime
+ * will always be = an objects lifetime. So object refcounting should cover
us.
+ */
+struct i915_vma {
+ struct drm_mm_node node;
+ struct drm_i915_gem_object *obj;
+ struct i915_address_space *vm;
+
+ struct list_head vma_link; /* Link in the object's VMA list */
+};
+
struct i915_ctx_hang_stats {
/* This context had batch pending when hang was declared */
unsigned batch_pending;
@@ -1229,8 +1240,9 @@ struct drm_i915_gem_object {
const struct drm_i915_gem_object_ops *ops;
- /** Current space allocated to this object in the GTT, if any. */
- struct drm_mm_node gtt_space;
+ /** List of VMAs backed by this object */
+ struct list_head vma_list;
+
/** Stolen memory for this object, instead of being backed by shmem. */
struct drm_mm_node *stolen;
struct list_head global_list;
@@ -1356,18 +1368,32 @@ struct drm_i915_gem_object {
#define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
-/* Offset of the first PTE pointing to this object */
-static inline unsigned long
-i915_gem_obj_ggtt_offset(struct drm_i915_gem_object *o)
+/* This is a temporary define to help transition us to real VMAs. If you see
+ * this, you're either reviewing code, or bisecting it. */
+static inline struct i915_vma *
+__i915_gem_obj_to_vma(struct drm_i915_gem_object *obj)
{
- return o-gtt_space.start;
+ if (list_empty(obj-vma_list))
+ return NULL;
+ return list_first_entry(obj-vma_list, struct i915_vma, vma_link);
}
/* Whether or not this object is currently mapped by the translation tables
*/
static inline bool
i915_gem_obj_ggtt_bound(struct drm_i915_gem_object *o)
{
- return drm_mm_node_allocated(o-gtt_space);
+ struct i915_vma *vma = __i915_gem_obj_to_vma(o);
+ if (vma == NULL)
+ return false;
+ return drm_mm_node_allocated(vma-node);
+}
+
+/* Offset of the first PTE pointing to this object */
+static inline unsigned long
+i915_gem_obj_ggtt_offset(struct drm_i915_gem_object *o)
+{
+ BUG_ON(list_empty(o-vma_list));
+ return __i915_gem_obj_to_vma(o)-node.start;
}
/* The size used in the translation tables may be larger than the actual
size of
@@ -1377,14 +1403,15 @@ i915_gem_obj_ggtt_bound(struct drm_i915_gem_object *o)
static inline unsigned long
i915_gem_obj_ggtt_size(struct drm_i915_gem_object *o)
{
- return o-gtt_space.size;
+ BUG_ON(list_empty(o-vma_list));
+ return __i915_gem_obj_to_vma(o)-node.size;
}
static inline void
i915_gem_obj_ggtt_set_color(struct drm_i915_gem_object *o,
enum i915_cache_level color)
{
- o-gtt_space.color = color;
+ __i915_gem_obj_to_vma(o)-node.color = color;
}
/**
@@ -1691,6 +1718,9 @@ void i915_gem_object_init(struct drm_i915_gem_object
*obj,
struct drm_i915_gem_object
Re: [Intel-gfx] [PATCH] drm/i915: Extend i915_powersave parameter.
On Wed, Jul 17, 2013 at 10:13 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Jul 17, 2013 at 02:45:29PM -0300, Rodrigo Vivi wrote: So if I understood correctly you suggest that if we have i915_powersave=0 and i915_enable_rc6=1 we should enable rc6? This is not how it is implemented nowadays on fbc right now... And this lead me to use pwoesave as an umbrella for force disable/enable ignoring individual parameters. But I'm open to do the other way around as long as we have a standardized behavior. Just depends on whether powersave=0 is default and powersave=-1 is force off, with powersave=1 as force on. So, we can either respect individual parameters when they are set or completely ignore. In the way it is now it doesn't respect individual fbc parameter for powersave=0. My opinion is that we respect the specific module parameters, and if they are left to default values, then apply the global powersave parameter. If that too is default, then we apply the module default. Jumping in a bit late, but: I've honestly never understood why we have two levels of module options. Imo having individual knobs for each delicate feature makes more sense, strange dependencies in module option will only confuse dim-witted developers like me when looking at a bug ;-) So could we just reduce powersave to the few things that we haven't touched yet (iirc only DRRS)? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/11] drm/i915: Adding global I915_PARAM for PSR ENABLED.
On Wed, Jul 17, 2013 at 5:18 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Jul 17, 2013 at 02:46:52PM -0300, Rodrigo Vivi wrote: Hi Chris, could you please review this specific one or give you ack here? Didn't see anything wrong with it. The only caveat I have is that the GETPARAM must be accurate immediately following a setcrtc. To be truly honest I have no idea, mainly when we alternate with fbcon updating psr state at set_base. Could you please also review subsequent patches in this series... 10 and 11. I think 11 answer this question... Another alternative would be using i915_enable_psr + i915_powersave check instead of reading the register for current enabled status. If you can guarrantee that is true, you can have my Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk and if Daniel delays, ask him to reserve the PARAM number. -Chris -- Chris Wilson, Intel Open Source Technology Centre -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/11] drm/i915: Adding global I915_PARAM for PSR ENABLED.
On Wed, Jul 17, 2013 at 06:01:03PM -0300, Rodrigo Vivi wrote: On Wed, Jul 17, 2013 at 5:18 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Jul 17, 2013 at 02:46:52PM -0300, Rodrigo Vivi wrote: Hi Chris, could you please review this specific one or give you ack here? Didn't see anything wrong with it. The only caveat I have is that the GETPARAM must be accurate immediately following a setcrtc. To be truly honest I have no idea, mainly when we alternate with fbcon updating psr state at set_base. Could you please also review subsequent patches in this series... 10 and 11. I think 11 answer this question... If it is not clear by this point, and the changelog doesn't make it clear, then something is missing from this patch. Hint ;-) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Make i915 events part of uapi
On 07/16/2013 10:07 PM, Daniel Vetter wrote: On Tue, Jul 16, 2013 at 04:41:13PM -0700, Ben Widawsky wrote: Make the uevent strings part of the user API for people who wish to write their own listeners. CC: Chad Versace chad.vers...@linux.intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net One thing I've toyed around with a bit is that we should add kerneldoc to our uapi headers and create a DocBook out of it (maybe as a subsection in the drm userspace api chapter). I guess the DocBook integration needs an overall approach, but we should start to add comments to each piece of userspace api to clearly spec them. See below for what I have in mind ... Yes, docs please. I don't have the kernel-fu of a kernel-dev, so without docs I don't know what these events mean and what to expect from them. - parity_event[0] = L3_PARITY_ERROR=1; + parity_event[0] = I915_L3_PARITY_EVENT=1; Small nitpick. I usually find string concatenation more readable like this, with a space: parity_event[0] = I915_L3_PARITY_EVENT =1; But, that's just my preference, so feel free to ignore me. +#define I915_L3_PARITY_EVENT L3_PARITY_ERROR +#define I915_ERROR_EVENT ERROR +#define I915_RESET_EVENT RESET Maybe this is a dumb question... since these are uevents, do you think the names would be improved by given them a UEVENT suffix? Like I915_ERROR_UEVENT? Or is that dumb, because these tokens are intended to serve more purposes than uevents? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Extend i915_powersave parameter.
On Wed, Jul 17, 2013 at 6:00 PM, Daniel Vetter dan...@ffwll.ch wrote: On Wed, Jul 17, 2013 at 10:13 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Jul 17, 2013 at 02:45:29PM -0300, Rodrigo Vivi wrote: So if I understood correctly you suggest that if we have i915_powersave=0 and i915_enable_rc6=1 we should enable rc6? This is not how it is implemented nowadays on fbc right now... And this lead me to use pwoesave as an umbrella for force disable/enable ignoring individual parameters. But I'm open to do the other way around as long as we have a standardized behavior. While you were writing my git send-email on v2 was failling because of dam proxy... But It's never late. And I think I agree with you... I just started this discussion exactly because I got confused about this flag and didn't like the way it was at fbc. The only advantage that I see on this two levels is an easy way to disable all powersaving features at once, mainly now it is increasing the numbers... instead of boot a kernel and manually put at grub: i915.powersave=0 i915.i915_enable_fbc=0 i915.i915_enable_rc6=0 i915.enable_ips=0 i915.enable_psr=0 i915.enable_slice_shutdown=0 you just have to write i915.powersave=0 Just depends on whether powersave=0 is default and powersave=-1 is force off, with powersave=1 as force on. So, we can either respect individual parameters when they are set or completely ignore. In the way it is now it doesn't respect individual fbc parameter for powersave=0. My opinion is that we respect the specific module parameters, and if they are left to default values, then apply the global powersave parameter. If that too is default, then we apply the module default. Jumping in a bit late, but: I've honestly never understood why we have two levels of module options. Imo having individual knobs for each delicate feature makes more sense, strange dependencies in module option will only confuse dim-witted developers like me when looking at a bug ;-) So could we just reduce powersave to the few things that we haven't touched yet (iirc only DRRS)? That is fine for me too... either add all features under this umbrella or make it be only one feature like drrs that doesn't have its own parameter... the only cons I see in this case is the name of parameter that is too generic... But honestly I don't have a stronger position I just wanted to start the discussion because I don't like the way it is today... So it is up to you... I can either send v2 or a new simple patch that removes fbc from this i915_powersave. Just let me know what is better... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Extend i915_powersave parameter.
On Wed, Jul 17, 2013 at 11:23 PM, Rodrigo Vivi rodrigo.v...@gmail.com wrote: My opinion is that we respect the specific module parameters, and if they are left to default values, then apply the global powersave parameter. If that too is default, then we apply the module default. Jumping in a bit late, but: I've honestly never understood why we have two levels of module options. Imo having individual knobs for each delicate feature makes more sense, strange dependencies in module option will only confuse dim-witted developers like me when looking at a bug ;-) So could we just reduce powersave to the few things that we haven't touched yet (iirc only DRRS)? That is fine for me too... either add all features under this umbrella or make it be only one feature like drrs that doesn't have its own parameter... the only cons I see in this case is the name of parameter that is too generic... We're allowed to kill module options now, so if we fix up drrs and make powersave completely useless, we can just remove. But honestly I don't have a stronger position I just wanted to start the discussion because I don't like the way it is today... So it is up to you... I can either send v2 or a new simple patch that removes fbc from this i915_powersave. Just let me know what is better... I vote for moving fbc out of powersave as a separate option. Worst case we need to educate a users and tell him that frobbing around with random module options isn't a good idea ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH 0/4 V7] Power-well API implementation for Haswell
-Original Message-
From: Takashi Iwai [mailto:ti...@suse.de]
Sent: Wednesday, July 17, 2013 10:22 PM
To: David Henningsson
Cc: Paulo Zanoni; Wang, Xingchao; alsa-de...@alsa-project.org; Daniel Vetter;
daniel.vet...@ffwll.ch; intel-gfx@lists.freedesktop.org; Wang xingchao;
Girdwood, Liam R; Jin, Gordon
Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well API
implementation for Haswell
At Wed, 17 Jul 2013 16:05:43 +0200,
David Henningsson wrote:
On 07/17/2013 04:00 PM, Takashi Iwai wrote:
At Wed, 17 Jul 2013 10:31:26 -0300,
Paulo Zanoni wrote:
2013/7/17 Wang, Xingchao xingchao.w...@intel.com:
-Original Message-
From: Takashi Iwai [mailto:ti...@suse.de]
Sent: Wednesday, July 17, 2013 4:18 PM
To: Wang, Xingchao
Cc: Paulo Zanoni; alsa-de...@alsa-project.org; Daniel Vetter;
daniel.vet...@ffwll.ch; intel-gfx@lists.freedesktop.org; Wang
xingchao; Girdwood, Liam R; david.hennings...@canonical.com
Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well
API implementation for Haswell
At Wed, 17 Jul 2013 08:03:38 +, Wang, Xingchao wrote:
-Original Message-
From: Takashi Iwai [mailto:ti...@suse.de]
Sent: Wednesday, July 17, 2013 3:34 PM
To: Wang, Xingchao
Cc: Paulo Zanoni; alsa-de...@alsa-project.org; Daniel Vetter;
daniel.vet...@ffwll.ch; intel-gfx@lists.freedesktop.org; Wang
xingchao; Girdwood, Liam R; david.hennings...@canonical.com
Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well
API implementation for Haswell
At Wed, 17 Jul 2013 02:52:41 +, Wang, Xingchao wrote:
Hi Takashi/Paulo,
would you change it to auto and test again.
Runtime power save should be enabled with auto.
Doesn't solve the problem. Should I open a bug report
somewhere?
Having the power well enabled prevents some power saving
features from the graphics driver.
Is the HD-audio power-saving itself working? You can check
it via watching /sys/class/hwC*/power_{on|off}_acct files.
power_save option has to be adjusted appropriately. Note
that many DEs change this value dynamically per AC-cable
plug/unplug depending on the configuration, and often it's
set to 0 (= no power save) when AC-cable is plugged.
[Wang, Xingchao] Paulo used a new Ultrabook board with charger
connected,
and see the default parameter auto=on.
In such scenario, power-well is always occupied by Display
audio controller. Moreover, in this board, if no external
monitors connected, It's
using internal eDP and totally no audio support. Power-well
usage in such case also blocks some eDP features as Paulo told me.
So I think it's not a good idea to break the rule of power
policy when charger
connected but it's necessary to add support in this particular case.
Takashi, do you think it's acceptable to let Display audio
controller/codec
suspend in such case?
Do you mean the driver enables the powersave forcibly?
Yes. I mean call pm_runtime_allow() for the power-well HD-A
controller.
Then, no, not in general.
If the default parameter of autopm is the problem, this should
be changed, instead of forcing the policy in the driver.
OTOH, the audio codec's powersave policy is governed by the
power_save option and it's set up dynamically by the desktop system.
We shouldn't override it in the driver.
If the power well *must* be off when only an eDP is used (e.g.
otherwise the hardware doesn't work properly), then it's a
different story. Is it the case? And what exactly would be the
problem?
In the eDP only case, no audio is needed for the HD-A
controller, so it's
wasting power in current design.
I think Paulo or Daniel could explain more details on the impact.
Consuming more power is the expected behavior. What else?
If it's the case, controlling the power well based on the
runtime PM is likely a bad design, as it relies on the parameter user
sets.
(And remember that the power-saving of the audio can be
disabled completely via Kconfig, too.)
From audio controller's point of view, if it's asked be active,
it needs power
and should request power-well from gfx side.
In above case, audio controller should not be active but user
set it be
active.
By setting the autopm on, user expects that no runtime PM happens.
In other words, the audio controller must be kept active as long
as this parameter is set. And this is the parameter user
controls, and not what the driver forcibly sets.
Okay, become clear now. :)
So I think the conflict for Paulo becomes, in eDP caes, if audio is
active
and requested power-well, some eDP feature was under impact?
Paulo, would you clarify this in more details?
On our driver we try to disable the power well whenever possible,
as soon as possible. We don't
[Intel-gfx] [Update][PATCH 0/3] Fix backlight issues on some Windows 8 systems
On Sunday, June 09, 2013 07:01:36 PM Matthew Garrett wrote: Windows 8 introduced new policy for backlight control by pushing it out to graphics drivers. This appears to have coincided with a range of vendors adding Windows 8 checks to their backlight control code which trigger either awkward behaviour (Lenovo) or complete brokenness (some Dells). The simplest thing to do would be to just disable ACPI backlight control entirely if the firmware indicates Windows 8 support, but it's entirely possible that individual graphics drivers might still make use of the ACPI functionality in preference to native control. The first two patches in this series are picked from other patchesets aimed at solving similar problems. The last simply unregisters ACPI backlight control on Windows 8 systems when using an Intel GPU. Similar code could be added to other drivers, but I'm reluctant to do so without further investigation as to the behaviour of the vendor drivers under Windows. Well, after some more time spent on that, we now have a series of 3 patches (different from the $subject one) that we think may be used to address this issue. As far as I can say, it has been tested by multiple people whose systems have those problems and they generally saw improvement. It is not my ideal approach, but it seems to be the least intrusive and/or with the least amount of possible side effects that we can do right now as a general measure (alternatively, we could create a possibly long blacklist table of affected systems with different workarounds for them, but let's just say that is not overwhelmingly attractive). [1/3] Make ACPICA export things that we need for checking OSI(Win8). [2/3] Make acpi_video_device_find_cap() call acpi_video_init_brightness() even if it is not going to register the backlight interface (needed for Thinkpads). [3/3] Avoid using ACPI backlight if i915 is in use and the firmware believes we are Windows 8. Many thanks to everyone involved! Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] ACPI / video: Always call acpi_video_init_brightness() on init
From: Matthew Garrett matthew.garr...@nebula.com
We have to call acpi_video_init_brightness() even if we're not going
to initialise the backlight - Thinkpads seem to use this as the
trigger for enabling ACPI notifications rather than handling it in
firmware.
[rjw: Drop the brightness object created by
acpi_video_init_brightness() if we are not going to use it.]
Signed-off-by: Matthew Garrett matthew.garr...@nebula.com
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/video.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
Index: linux-pm/drivers/acpi/video.c
===
--- linux-pm.orig/drivers/acpi/video.c
+++ linux-pm/drivers/acpi/video.c
@@ -884,6 +884,9 @@ static void acpi_video_device_find_cap(s
if (acpi_has_method(device-dev-handle, _DDC))
device-cap._DDC = 1;
+ if (acpi_video_init_brightness(device))
+ return;
+
if (acpi_video_backlight_support()) {
struct backlight_properties props;
struct pci_dev *pdev;
@@ -893,9 +896,6 @@ static void acpi_video_device_find_cap(s
static int count = 0;
char *name;
- result = acpi_video_init_brightness(device);
- if (result)
- return;
name = kasprintf(GFP_KERNEL, acpi_video%d, count);
if (!name)
return;
@@ -955,6 +955,11 @@ static void acpi_video_device_find_cap(s
if (result)
printk(KERN_ERR PREFIX Create sysfs link\n);
+ } else {
+ /* Remove the brightness object. */
+ kfree(device-brightness-levels);
+ kfree(device-brightness);
+ device-brightness = NULL;
}
}
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] ACPICA: expose OSI version
From: Aaron Lu aaron...@intel.com
Expose acpi_gbl_osi_data so that code outside of ACPICA can check
the value of the last successfull _OSI call. The definitions for
OSI versions are moved to actypes.h so that other components can
access them too.
Based on a patch from Matthew Garrett which in turn was based on
an earlier patch from Seth Forshee.
[rjw: Changelog]
Signed-off-by: Aaron Lu aaron...@intel.com
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/acpica/aclocal.h | 13 -
include/acpi/acpixf.h | 1 +
include/acpi/actypes.h| 15 +++
3 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/acpi/acpica/aclocal.h b/drivers/acpi/acpica/aclocal.h
index dfed265..d4a49016 100644
--- a/drivers/acpi/acpica/aclocal.h
+++ b/drivers/acpi/acpica/aclocal.h
@@ -931,19 +931,6 @@ struct acpi_bit_register_info {
/* Structs and definitions for _OSI support and I/O port validation */
-#define ACPI_OSI_WIN_2000 0x01
-#define ACPI_OSI_WIN_XP 0x02
-#define ACPI_OSI_WIN_XP_SP1 0x03
-#define ACPI_OSI_WINSRV_20030x04
-#define ACPI_OSI_WIN_XP_SP2 0x05
-#define ACPI_OSI_WINSRV_2003_SP10x06
-#define ACPI_OSI_WIN_VISTA 0x07
-#define ACPI_OSI_WINSRV_20080x08
-#define ACPI_OSI_WIN_VISTA_SP1 0x09
-#define ACPI_OSI_WIN_VISTA_SP2 0x0A
-#define ACPI_OSI_WIN_7 0x0B
-#define ACPI_OSI_WIN_8 0x0C
-
#define ACPI_ALWAYS_ILLEGAL 0x00
struct acpi_interface_info {
diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index 1b09300..22d497e 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -62,6 +62,7 @@ extern u32 acpi_current_gpe_count;
extern struct acpi_table_fadt acpi_gbl_FADT;
extern u8 acpi_gbl_system_awake_and_running;
extern u8 acpi_gbl_reduced_hardware; /* ACPI 5.0 */
+extern u8 acpi_gbl_osi_data;
/* Runtime configuration of debug print levels */
diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index a64adcc..22b03c9 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -1144,4 +1144,19 @@ struct acpi_memory_list {
#endif
};
+/* Definitions for _OSI support */
+
+#define ACPI_OSI_WIN_2000 0x01
+#define ACPI_OSI_WIN_XP 0x02
+#define ACPI_OSI_WIN_XP_SP1 0x03
+#define ACPI_OSI_WINSRV_20030x04
+#define ACPI_OSI_WIN_XP_SP2 0x05
+#define ACPI_OSI_WINSRV_2003_SP10x06
+#define ACPI_OSI_WIN_VISTA 0x07
+#define ACPI_OSI_WINSRV_20080x08
+#define ACPI_OSI_WIN_VISTA_SP1 0x09
+#define ACPI_OSI_WIN_VISTA_SP2 0x0A
+#define ACPI_OSI_WIN_7 0x0B
+#define ACPI_OSI_WIN_8 0x0C
+
#endif /* __ACTYPES_H__ */
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH 0/4 V7] Power-well API implementation for Haswell
Hi Paulo/Daniel,
Do you agree to export an API in gfx side for eDP case?
Basically the api will let audio drver know which pipe in use. i.e. in the eDP
only caes, audio driver
Will know gfx is not using HDMI/DP and would like to let power-well off.
As there's the conflict when user expect display audio driver always active but
gfx need audio driver off.
Audio driver could make decision to release power-well if it knows the eDP only
case through the API.
OTOH, I think audio driver could also export an API for gfx side, if gfx driver
need audio driver release power-well but it's in usage,
It will call this API and audio drvier will release power-well accordingly.
This change make HDMI/DP hotplug handling complicated in audio driver side, if
audio driver release power-well, it would enter suspend mode.
Meanwhile the user may expect it's in active mode, this may cause some confuse.
Thanks
--xingchao
-Original Message-
From: Wang, Xingchao
Sent: Thursday, July 18, 2013 7:18 AM
To: 'Takashi Iwai'; David Henningsson; Paulo Zanoni
Cc: alsa-de...@alsa-project.org; Daniel Vetter; daniel.vet...@ffwll.ch;
intel-gfx@lists.freedesktop.org; Wang xingchao; Girdwood, Liam R; Jin, Gordon
Subject: RE: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well API
implementation for Haswell
-Original Message-
From: Takashi Iwai [mailto:ti...@suse.de]
Sent: Wednesday, July 17, 2013 10:22 PM
To: David Henningsson
Cc: Paulo Zanoni; Wang, Xingchao; alsa-de...@alsa-project.org; Daniel
Vetter; daniel.vet...@ffwll.ch; intel-gfx@lists.freedesktop.org; Wang
xingchao; Girdwood, Liam R; Jin, Gordon
Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well API
implementation for Haswell
At Wed, 17 Jul 2013 16:05:43 +0200,
David Henningsson wrote:
On 07/17/2013 04:00 PM, Takashi Iwai wrote:
At Wed, 17 Jul 2013 10:31:26 -0300, Paulo Zanoni wrote:
2013/7/17 Wang, Xingchao xingchao.w...@intel.com:
-Original Message-
From: Takashi Iwai [mailto:ti...@suse.de]
Sent: Wednesday, July 17, 2013 4:18 PM
To: Wang, Xingchao
Cc: Paulo Zanoni; alsa-de...@alsa-project.org; Daniel Vetter;
daniel.vet...@ffwll.ch; intel-gfx@lists.freedesktop.org; Wang
xingchao; Girdwood, Liam R; david.hennings...@canonical.com
Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well
API implementation for Haswell
At Wed, 17 Jul 2013 08:03:38 +, Wang, Xingchao wrote:
-Original Message-
From: Takashi Iwai [mailto:ti...@suse.de]
Sent: Wednesday, July 17, 2013 3:34 PM
To: Wang, Xingchao
Cc: Paulo Zanoni; alsa-de...@alsa-project.org; Daniel Vetter;
daniel.vet...@ffwll.ch; intel-gfx@lists.freedesktop.org; Wang
xingchao; Girdwood, Liam R; david.hennings...@canonical.com
Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7]
Power-well API implementation for Haswell
At Wed, 17 Jul 2013 02:52:41 +, Wang, Xingchao wrote:
Hi Takashi/Paulo,
would you change it to auto and test again.
Runtime power save should be enabled with auto.
Doesn't solve the problem. Should I open a bug report
somewhere?
Having the power well enabled prevents some power saving
features from the graphics driver.
Is the HD-audio power-saving itself working? You can check
it via watching /sys/class/hwC*/power_{on|off}_acct files.
power_save option has to be adjusted appropriately. Note
that many DEs change this value dynamically per AC-cable
plug/unplug depending on the configuration, and often it's
set to 0 (= no power save) when AC-cable is plugged.
[Wang, Xingchao] Paulo used a new Ultrabook board with
charger connected,
and see the default parameter auto=on.
In such scenario, power-well is always occupied by Display
audio controller. Moreover, in this board, if no external
monitors connected, It's
using internal eDP and totally no audio support. Power-well
usage in such case also blocks some eDP features as Paulo told me.
So I think it's not a good idea to break the rule of power
policy when charger
connected but it's necessary to add support in this particular
case.
Takashi, do you think it's acceptable to let Display audio
controller/codec
suspend in such case?
Do you mean the driver enables the powersave forcibly?
Yes. I mean call pm_runtime_allow() for the power-well HD-A
controller.
Then, no, not in general.
If the default parameter of autopm is the problem, this
should be changed, instead of forcing the policy in the driver.
OTOH, the audio codec's powersave policy is governed by the
power_save option and it's set up dynamically by the desktop
system.
We shouldn't override it in the driver.
If the power well *must* be off when only an eDP is used (e.g.
otherwise the hardware doesn't
[Intel-gfx] linux-next: manual merge of the drm-intel tree with the drm-intel-fixes tree
Hi all,
Today's linux-next merge of the drm-intel tree got a conflict in
drivers/gpu/drm/i915/i915_dma.c between commit 7dcd2677ea91 (drm/i915:
fix long-standing SNB regression in power consumption after resume v2)
from the drm-intel-fixes tree and commit 59cdb63d529c (drm/i915: kill
dev_priv-rps.lock) from the drm-intel tree.
I fixed it up (see below) and can carry the fix as necessary (no action
is required).
--
Cheers,
Stephen Rothwells...@canb.auug.org.au
diff --cc drivers/gpu/drm/i915/i915_dma.c
index 095e3db,fd52de7..000
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@@ -1495,16 -1490,12 +1490,19 @@@ int i915_driver_load(struct drm_device
dev_priv-dev = dev;
dev_priv-info = info;
+ spin_lock_init(dev_priv-irq_lock);
+ spin_lock_init(dev_priv-gpu_error.lock);
- spin_lock_init(dev_priv-rps.lock);
+ spin_lock_init(dev_priv-backlight.lock);
+ mutex_init(dev_priv-dpio_lock);
+ mutex_init(dev_priv-rps.hw_lock);
+ mutex_init(dev_priv-modeset_restore_lock);
+
i915_dump_device_info(dev_priv);
+ INIT_LIST_HEAD(dev_priv-vm_list);
+ INIT_LIST_HEAD(dev_priv-gtt.base.global_link);
+ list_add(dev_priv-gtt.base.global_link, dev_priv-vm_list);
+
if (i915_get_bridge_dev(dev)) {
ret = -EIO;
goto free_priv;
pgpI9ifLtbrLL.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] linux-next: manual merge of the drm-intel tree with the drm-intel-fixes tree
Hi all,
Today's linux-next merge of the drm-intel tree got a conflict in
drivers/gpu/drm/i915/i915_gem.c between commit 067556084a0e (drm/i915:
Correct obj-mm_list link to dev_priv-dev_priv-mm.inactive_list) from
the drm-intel-fixes tree and commit 5cef07e16283 (drm/i915: Move
active/inactive lists to new mm) from the drm-intel tree.
I fixed it up (see below) and can carry the fix as necessary (no action
is required).
--
Cheers,
Stephen Rothwells...@canb.auug.org.au
diff --cc drivers/gpu/drm/i915/i915_gem.c
index 97afd26,9a523df..000
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@@ -2275,14 -2282,11 +2276,10 @@@ void i915_gem_reset(struct drm_device *
/* Move everything out of the GPU domains to ensure we do any
* necessary invalidation upon reuse.
*/
- list_for_each_entry(obj,
- dev_priv-mm.inactive_list,
- mm_list)
- {
+ list_for_each_entry(obj, vm-inactive_list, mm_list)
obj-base.read_domains = ~I915_GEM_GPU_DOMAINS;
- }
- /* The fence registers are invalidated so clear them out */
- i915_gem_reset_fences(dev);
+ i915_gem_restore_fences(dev);
}
/**
@@@ -2666,27 -2679,12 +2671,27 @@@ static void i965_write_fence_reg(struc
fence_pitch_shift = I965_FENCE_PITCH_SHIFT;
}
+ fence_reg += reg * 8;
+
+ /* To w/a incoherency with non-atomic 64-bit register updates,
+ * we split the 64-bit update into two 32-bit writes. In order
+ * for a partial fence not to be evaluated between writes, we
+ * precede the update with write to turn off the fence register,
+ * and only enable the fence as the last step.
+ *
+ * For extra levels of paranoia, we make sure each step lands
+ * before applying the next step.
+ */
+ I915_WRITE(fence_reg, 0);
+ POSTING_READ(fence_reg);
+
if (obj) {
- u32 size = obj-gtt_space-size;
+ u32 size = i915_gem_obj_ggtt_size(obj);
+ uint64_t val;
- val = (uint64_t)((obj-gtt_offset + size - 4096)
+ val = (uint64_t)((i915_gem_obj_ggtt_offset(obj) + size - 4096)
0xf000) 32;
- val |= obj-gtt_offset 0xf000;
+ val |= i915_gem_obj_ggtt_offset(obj) 0xf000;
val |= (uint64_t)((obj-stride / 128) - 1) fence_pitch_shift;
if (obj-tiling_mode == I915_TILING_Y)
val |= 1 I965_FENCE_TILING_Y_SHIFT;
@@@ -3992,11 -4050,8 +4022,6 @@@ i915_gem_idle(struct drm_device *dev
if (!drm_core_check_feature(dev, DRIVER_MODESET))
i915_gem_evict_everything(dev);
- /* Hack! Don't let anybody do execbuf while we don't control the chip.
-* We need to replace this with a semaphore, or something.
-* And not confound mm.suspended!
-*/
- dev_priv-mm.suspended = 1;
- i915_gem_reset_fences(dev);
-
del_timer_sync(dev_priv-gpu_error.hangcheck_timer);
i915_kernel_lost_context(dev);
@@@ -4594,7 -4664,7 +4635,7 @@@ i915_gem_inactive_shrink(struct shrinke
list_for_each_entry(obj, dev_priv-mm.unbound_list, global_list)
if (obj-pages_pin_count == 0)
cnt += obj-base.size PAGE_SHIFT;
- list_for_each_entry(obj, dev_priv-mm.inactive_list, mm_list)
- list_for_each_entry(obj, vm-inactive_list, global_list)
++ list_for_each_entry(obj, vm-inactive_list, mm_list)
if (obj-pin_count == 0 obj-pages_pin_count == 0)
cnt += obj-base.size PAGE_SHIFT;
pgpFR3Q_w9WHC.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915: Create VMAs
On Thu, Jul 18, 2013 at 01:12:17AM +0100, Chris Wilson wrote: On Wed, Jul 17, 2013 at 12:19:03PM -0700, Ben Widawsky wrote: Formerly: drm/i915: Create VMAs (part 1) In a previous patch, the notion of a VM was introduced. A VMA describes an area of part of the VM address space. A VMA is similar to the concept in the linux mm. However, instead of representing regular memory, a VMA is backed by a GEM BO. There may be many VMAs for a given object, one for each VM the object is to be used in. This may occur through flink, dma-buf, or a number of other transient states. Currently the code depends on only 1 VMA per object, for the global GTT (and aliasing PPGTT). The following patches will address this and make the rest of the infrastructure more suited v2: s/i915_obj/i915_gem_obj (Chris) v3: Only move an object to the now global unbound list if there are no more VMAs for the object which are bound into a VM (ie. the list is empty). v4: killed obj-gtt_space some reworks due to rebase v5: Free vma on error path (Imre) v6: Another missed vma free in i915_gem_object_bind_to_gtt error path (Imre) Fixed vma freeing in stolen preallocation (Imre) Big-bada-boom; Just from looking at the code I think I see a bug. A bug which didn't exist in the original version of the code, and doesn't exist after the very next patch in the overall series. Now I am terribly curious - why in the world (if that's indeed the bug) can I not seem to hit this locally on my machine? I'll send the patch for the fix now, but I'd really like to know what's different in our setup. I've tried, UXA, SNA, and the igt test suite... set-cache-level needs to iterate over vma, and in particular should not dereference a non-existent one. Or if we decided that set-cache-level was a ggtt only property, just not explode if there is no global vma. -Chris The current state is that cache level is still per object, but this function itself will iterate over all VMAs. As I said, it happens in the very next patch in the series. In the original patch series cover letter, I made cache levels per VMA an optional TODO. For the time being, I don't see much benefit. -- Chris Wilson, Intel Open Source Technology Centre -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Don't try to deref an unbound VMA
This was actually correct in the original series, and is also fixed
later in the patch series, but was broken in this middle state.
I'm not really certain how I didn't hit it sooner.
This patch should be squashed into:
commit 8f588cfc349bbbd8ae62a13679b9efba41645064
Author: Ben Widawsky b...@bwidawsk.net
Date: Wed Jul 17 12:19:03 2013 -0700
drm/i915: Create VMAs
CC: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_gem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a9fa2bb..c9487bb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3314,7 +3314,7 @@ int i915_gem_object_set_cache_level(struct
drm_i915_gem_object *obj,
return -EBUSY;
}
- if (!i915_gem_valid_gtt_space(dev, vma-node, cache_level)) {
+ if (vma !i915_gem_valid_gtt_space(dev, vma-node, cache_level)) {
ret = i915_gem_object_unbind(obj);
if (ret)
return ret;
--
1.8.3.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] linux-next: manual merge of the akpm tree with the drm-intel tree
Hi Andrew,
Today's linux-next merge of the akpm tree got a conflict in
drivers/gpu/drm/i915/i915_gem.c between commit 5cef07e16283 (drm/i915:
Move active/inactive lists to new mm) from the drm-intel tree and commit
drivers-convert-shrinkers-to-new-count-scan-api-fix from the akpm tree.
I fixed it up (see below) and can carry the fix as necessary (no action
is required).
--
Cheers,
Stephen Rothwells...@canb.auug.org.au
diff --cc drivers/gpu/drm/i915/i915_gem.c
index 50e26d3,a56feaa..000
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@@ -4614,10 -4570,9 +4614,10 @@@ i915_gem_inactive_count(struct shrinke
struct drm_i915_private,
mm.inactive_shrinker);
struct drm_device *dev = dev_priv-dev;
+ struct i915_address_space *vm = dev_priv-gtt.base;
struct drm_i915_gem_object *obj;
bool unlock = true;
- long cnt;
+ unsigned long count;
if (!mutex_trylock(dev-struct_mutex)) {
if (!mutex_is_locked_by(dev-struct_mutex, current))
@@@ -4629,13 -4584,13 +4629,13 @@@
unlock = false;
}
- cnt = 0;
+ count = 0;
list_for_each_entry(obj, dev_priv-mm.unbound_list, global_list)
if (obj-pages_pin_count == 0)
- cnt += obj-base.size PAGE_SHIFT;
+ count += obj-base.size PAGE_SHIFT;
- list_for_each_entry(obj, dev_priv-mm.inactive_list, mm_list)
+ list_for_each_entry(obj, vm-inactive_list, mm_list)
if (obj-pin_count == 0 obj-pages_pin_count == 0)
- cnt += obj-base.size PAGE_SHIFT;
+ count += obj-base.size PAGE_SHIFT;
if (unlock)
mutex_unlock(dev-struct_mutex);
pgpF3DnWYfYos.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [QA] Testing report for `drm-intel-testing` (was: Updated -next) on ww29
Summary We covered the platform: Haswell mobile, HSW desktop, HSW ULT, IvyBridge, SandyBridge, IronLake. In this circle, 28 bugs are still opened, 5 bugs are closed. Test Environment commit bdd5c7d4e63d5167e78410ccf914e10de28e9a86 Merge: 83e7cd6 18097b9 Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Fri Jul 12 18:56:14 2013 +0200 Merge remote-tracking branch 'drm-upstream/drm-fixes' into drm-intel-nightly Finding Opened Bugs: Bug 66951https://bugs.freedesktop.org/show_bug.cgi?id=66951 - [Calpella]Resume from S4 causes system reboot sporadically Bug 66844https://bugs.freedesktop.org/show_bug.cgi?id=66844 - [SNB Regression]*ERROR* conflict detected with stolen region: [0xcba0 - 0xcfa0] Bug 66808https://bugs.freedesktop.org/show_bug.cgi?id=66808 - [HSW Bisected]igt/kms_flip/blocking-absolute-wf_vblank due to power well enabled by default Bug 66726https://bugs.freedesktop.org/show_bug.cgi?id=66726 - [PNV Regression]*ERROR* conflict detected with stolen region: [0x7f80 - 0x8000] Bug 66482https://bugs.freedesktop.org/show_bug.cgi?id=66482 - igt/kms_flip/absolute-wf_vblank randomly causes 3[ 1.211971] [drm:intel_pipe_config_compare] *ERROR* mismatch in adjusted_mode.flags (expected 0, found 1) Bug 66480https://bugs.freedesktop.org/show_bug.cgi?id=66480 - [SNB regression]*ERROR* mismatch in adjusted_mode.flags Bug 66301https://bugs.freedesktop.org/show_bug.cgi?id=66301 - [HSW mobile] resume from s4 sporadically causes call trace and machine is reachable Bug 65995https://bugs.freedesktop.org/show_bug.cgi?id=65995 - [HSW mobile] eDP can't light up when boot up Bug 65927https://bugs.freedesktop.org/show_bug.cgi?id=65927 - [HSW] crash when vga output set to mirror mode Bug 65880https://bugs.freedesktop.org/show_bug.cgi?id=65880 - [HSW bisected] igt/module_reload randomly causes system hang Bug 65700https://bugs.freedesktop.org/show_bug.cgi?id=65700 - [SNB]3[drm:ironlake_disable_pch_transcoder] *ERROR* failed to disable transcoder 0 Bug 65496https://bugs.freedesktop.org/show_bug.cgi?id=65496 - [HSW] Probabilistic resume from s4 causes call trace and system hang, with warm boot Bug 65442https://bugs.freedesktop.org/show_bug.cgi?id=65442 - [ALL bisected]I-G-T/testdisplay causes call trace while setting up customized timing Bug 65387https://bugs.freedesktop.org/show_bug.cgi?id=65387 - [HSW] vebox semaphores ignored Bug 64415https://bugs.freedesktop.org/show_bug.cgi?id=64415 - [ILK] Some modes unable to light up on DP display Bug 64379https://bugs.freedesktop.org/show_bug.cgi?id=64379 - [HSW desktop bisected] ddi pll tracking botch-up due to vt-switchless resume Bug 63981https://bugs.freedesktop.org/show_bug.cgi?id=63981 - MacBook Pro retina 13 inch early 2013 jittery display Bug 61041https://bugs.freedesktop.org/show_bug.cgi?id=61041 - [Pineview]I-G-T/testdisplay VGA 1600x1200 65Hz messing the screen Bug 60002https://bugs.freedesktop.org/show_bug.cgi?id=60002 - [PNV]I-G-T/kms_flip subtest：'blocking-absolute-wf_vblank' fail Bug 5https://bugs.freedesktop.org/show_bug.cgi?id=5 - [ILK,IVB]kms_flip error: inter-flip ts jitter Bug 59836https://bugs.freedesktop.org/show_bug.cgi?id=59836 - [PNV]igt/kms_flip/absolute-wf_vblank fail Bug 59339https://bugs.freedesktop.org/show_bug.cgi?id=59339 - [ILK] I-G-T/kms_flip subtest: 'flip-vs-panning' fail Bug 59321https://bugzilla.kernel.org/show_bug.cgi?id=59321 - S4 broken with Haswell Bug 58701https://bugs.freedesktop.org/show_bug.cgi?id=58701 - [SNB DP]I-G-T/testdisplay 1920x1080i showing not full Bug 57441https://bugs.freedesktop.org/show_bug.cgi?id=57441 - [HSW]I-G-T/sysfs_l3_parity fail Bug 54687https://bugs.freedesktop.org/show_bug.cgi?id=54687 - [ilk] pipe off timeout Bug 51975https://bugs.freedesktop.org/show_bug.cgi?id=51975 - [IVB]can't find the HDMI audio device Bug 42194https://bugs.freedesktop.org/show_bug.cgi?id=42194 - [IVB/SNB] coldplug new monitors for fbcon on lastclose() Closed Bugs: Bug 66952https://bugs.freedesktop.org/show_bug.cgi?id=66952 - [IVB Regression: Apple MacBook Pro] shared pll warning, reusing active PLL (eDP + two any pipes will cause call trace in dmesg) Bug 66838https://bugs.freedesktop.org/show_bug.cgi?id=66838 - [ILK]igt/gem_write_read_ring_switch/blt2bsd causes *ERROR* bsd ring hung inside bo (0x952000 ctx 0) at 0x952004 Bug 66610https://bugs.freedesktop.org/show_bug.cgi?id=66610 - [PNV/ILK] igt/module_reload fails and causes many cases fail: modules.dep: no such fileBug 66533https://bugs.freedesktop.org/show_bug.cgi?id=66533 - [Bisected]igt/debugfs_reader i915 sysfs path not found Bug 66445https://bugs.freedesktop.org/show_bug.cgi?id=66445 - [GM45/ILK Regression] *ERROR* mismatch in clock (expected 66285, found 0 Bug 66306https://bugs.freedesktop.org/show_bug.cgi?id=66306 - [HSW ULT] display port can't turn off by using xrandr --output xxx �Coff Best Regards~~ Open Source Technology Center (OTC) Terence Yang（杨光） Tel:
Re: [Intel-gfx] [PATCH 1/3] ACPICA: expose OSI version
From: Aaron Lu aaron...@intel.com
Expose acpi_gbl_osi_data so that code outside of ACPICA can check
the value of the last successfull _OSI call. The definitions for
OSI versions are moved to actypes.h so that other components can
access them too.
Based on a patch from Matthew Garrett which in turn was based on
an earlier patch from Seth Forshee.
[rjw: Changelog]
Signed-off-by: Aaron Lu aaron...@intel.com
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com
---
drivers/acpi/acpica/aclocal.h | 13 -
include/acpi/acpixf.h | 1 +
include/acpi/actypes.h| 15 +++
3 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/acpi/acpica/aclocal.h b/drivers/acpi/acpica/aclocal.h
index dfed265..d4a49016 100644
--- a/drivers/acpi/acpica/aclocal.h
+++ b/drivers/acpi/acpica/aclocal.h
@@ -931,19 +931,6 @@ struct acpi_bit_register_info {
/* Structs and definitions for _OSI support and I/O port validation */
-#define ACPI_OSI_WIN_2000 0x01
-#define ACPI_OSI_WIN_XP 0x02
-#define ACPI_OSI_WIN_XP_SP1 0x03
-#define ACPI_OSI_WINSRV_20030x04
-#define ACPI_OSI_WIN_XP_SP2 0x05
-#define ACPI_OSI_WINSRV_2003_SP10x06
-#define ACPI_OSI_WIN_VISTA 0x07
-#define ACPI_OSI_WINSRV_20080x08
-#define ACPI_OSI_WIN_VISTA_SP1 0x09
-#define ACPI_OSI_WIN_VISTA_SP2 0x0A
-#define ACPI_OSI_WIN_7 0x0B
-#define ACPI_OSI_WIN_8 0x0C
-
#define ACPI_ALWAYS_ILLEGAL 0x00
struct acpi_interface_info {
diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index 1b09300..22d497e 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -62,6 +62,7 @@ extern u32 acpi_current_gpe_count;
extern struct acpi_table_fadt acpi_gbl_FADT;
extern u8 acpi_gbl_system_awake_and_running;
extern u8 acpi_gbl_reduced_hardware; /* ACPI 5.0 */
+extern u8 acpi_gbl_osi_data;
/* Runtime configuration of debug print levels */
diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index a64adcc..22b03c9 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -1144,4 +1144,19 @@ struct acpi_memory_list {
#endif
};
+/* Definitions for _OSI support */
+
+#define ACPI_OSI_WIN_2000 0x01
+#define ACPI_OSI_WIN_XP 0x02
+#define ACPI_OSI_WIN_XP_SP1 0x03
+#define ACPI_OSI_WINSRV_20030x04
+#define ACPI_OSI_WIN_XP_SP2 0x05
+#define ACPI_OSI_WINSRV_2003_SP10x06
+#define ACPI_OSI_WIN_VISTA 0x07
+#define ACPI_OSI_WINSRV_20080x08
+#define ACPI_OSI_WIN_VISTA_SP1 0x09
+#define ACPI_OSI_WIN_VISTA_SP2 0x0A
+#define ACPI_OSI_WIN_7 0x0B
+#define ACPI_OSI_WIN_8 0x0C
+
#endif /* __ACTYPES_H__ */
--
Igor Gnatenko
Fedora release 19 (Schrödinger’s Cat)
Linux 3.9.9-302.fc19.x86_64
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] ACPI / video: Always call acpi_video_init_brightness() on init
From: Matthew Garrett matthew.garr...@nebula.com
We have to call acpi_video_init_brightness() even if we're not going
to initialise the backlight - Thinkpads seem to use this as the
trigger for enabling ACPI notifications rather than handling it in
firmware.
[rjw: Drop the brightness object created by
acpi_video_init_brightness() if we are not going to use it.]
Signed-off-by: Matthew Garrett matthew.garr...@nebula.com
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com
---
drivers/acpi/video.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
Index: linux-pm/drivers/acpi/video.c
===
--- linux-pm.orig/drivers/acpi/video.c
+++ linux-pm/drivers/acpi/video.c
@@ -884,6 +884,9 @@ static void acpi_video_device_find_cap(s
if (acpi_has_method(device-dev-handle, _DDC))
device-cap._DDC = 1;
+ if (acpi_video_init_brightness(device))
+ return;
+
if (acpi_video_backlight_support()) {
struct backlight_properties props;
struct pci_dev *pdev;
@@ -893,9 +896,6 @@ static void acpi_video_device_find_cap(s
static int count = 0;
char *name;
- result = acpi_video_init_brightness(device);
- if (result)
- return;
name = kasprintf(GFP_KERNEL, acpi_video%d, count);
if (!name)
return;
@@ -955,6 +955,11 @@ static void acpi_video_device_find_cap(s
if (result)
printk(KERN_ERR PREFIX Create sysfs link\n);
+ } else {
+ /* Remove the brightness object. */
+ kfree(device-brightness-levels);
+ kfree(device-brightness);
+ device-brightness = NULL;
}
}
--
Igor Gnatenko
Fedora release 19 (Schrödinger’s Cat)
Linux 3.9.9-302.fc19.x86_64
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
