Re: [Intel-gfx] [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Tue, 16 Jul 2013 22:43:49 +0200 Daniel Vetter wrote: > On Tue, Jul 16, 2013 at 01:19:25PM -0700, Jesse Barnes wrote: > > On Tue, 16 Jul 2013 10:06:54 -0700 > > Jesse Barnes wrote: > > > > > On Tue, 16 Jul 2013 11:34:25 +0400 > > > Konstantin Khlebnikov wrote: > > > > I've tested that patch and it really works for me. If you want change > > > > something for other hardware or > > > > extend range where forcewake is held prease do it in a separate patch. > > > > This will be good for bisecting new bugs in the future. > > > > > > Thanks a ton for finding this Konstantin, it puts us on the right > > > track. > > > > > > Can I ask you to test this patch? The theory is that having RC6 > > > enabled messes with the initial programming sequence, so it's probably > > > best to just shut it off at init until we're done, rather than trying > > > to forcewake around everywhere we need it. > > > > Oops, last one triggers a warn about IRQs. This one doesn't and still > > works for me. > > > > Testing welcome. > > > > Thanks, > > -- > > Jesse Barnes, Intel Open Source Technology Center > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > > b/drivers/gpu/drm/i915/i915_gem.c > > index c9d9d20..d962ec0 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -4112,6 +4112,9 @@ i915_gem_init_hw(struct drm_device *dev) > > drm_i915_private_t *dev_priv = dev->dev_private; > > int ret; > > > > + /* BIOS often leaves RC6 enabled, but disable it for hw init */ > > + intel_disable_gt_powersave(dev); > > I think it'd be better to have an explicit gen >= 6 check here and a > disable_rps call. disable_gt_powersave also calls the ironlake version, > which restores probably bogus values (since we haven't read them out yet > in the enable code) into the hw. Yeah the ilk "restore to initial freq" needs fixing, but I thought we wanted this on all gens? It shouldn't hurt anything, and may help with other issues as well. -- Jesse Barnes, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Tue, Jul 16, 2013 at 01:19:25PM -0700, Jesse Barnes wrote: > On Tue, 16 Jul 2013 10:06:54 -0700 > Jesse Barnes wrote: > > > On Tue, 16 Jul 2013 11:34:25 +0400 > > Konstantin Khlebnikov wrote: > > > I've tested that patch and it really works for me. If you want change > > > something for other hardware or > > > extend range where forcewake is held prease do it in a separate patch. > > > This will be good for bisecting new bugs in the future. > > > > Thanks a ton for finding this Konstantin, it puts us on the right > > track. > > > > Can I ask you to test this patch? The theory is that having RC6 > > enabled messes with the initial programming sequence, so it's probably > > best to just shut it off at init until we're done, rather than trying > > to forcewake around everywhere we need it. > > Oops, last one triggers a warn about IRQs. This one doesn't and still > works for me. > > Testing welcome. > > Thanks, > -- > Jesse Barnes, Intel Open Source Technology Center > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index c9d9d20..d962ec0 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4112,6 +4112,9 @@ i915_gem_init_hw(struct drm_device *dev) > drm_i915_private_t *dev_priv = dev->dev_private; > int ret; > > + /* BIOS often leaves RC6 enabled, but disable it for hw init */ > + intel_disable_gt_powersave(dev); I think it'd be better to have an explicit gen >= 6 check here and a disable_rps call. disable_gt_powersave also calls the ironlake version, which restores probably bogus values (since we haven't read them out yet in the enable code) into the hw. -Daniel > + > if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt()) > return -EIO; > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Tue, 16 Jul 2013 10:06:54 -0700 Jesse Barnes wrote: > On Tue, 16 Jul 2013 11:34:25 +0400 > Konstantin Khlebnikov wrote: > > I've tested that patch and it really works for me. If you want change > > something for other hardware or > > extend range where forcewake is held prease do it in a separate patch. > > This will be good for bisecting new bugs in the future. > > Thanks a ton for finding this Konstantin, it puts us on the right > track. > > Can I ask you to test this patch? The theory is that having RC6 > enabled messes with the initial programming sequence, so it's probably > best to just shut it off at init until we're done, rather than trying > to forcewake around everywhere we need it. Oops, last one triggers a warn about IRQs. This one doesn't and still works for me. Testing welcome. Thanks, -- Jesse Barnes, Intel Open Source Technology Center diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c9d9d20..d962ec0 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4112,6 +4112,9 @@ i915_gem_init_hw(struct drm_device *dev) drm_i915_private_t *dev_priv = dev->dev_private; int ret; + /* BIOS often leaves RC6 enabled, but disable it for hw init */ + intel_disable_gt_powersave(dev); + if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt()) return -EIO; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Tue, 16 Jul 2013 11:34:25 +0400
Konstantin Khlebnikov wrote:
> I've tested that patch and it really works for me. If you want change
> something for other hardware or
> extend range where forcewake is held prease do it in a separate patch.
> This will be good for bisecting new bugs in the future.
Thanks a ton for finding this Konstantin, it puts us on the right
track.
Can I ask you to test this patch? The theory is that having RC6
enabled messes with the initial programming sequence, so it's probably
best to just shut it off at init until we're done, rather than trying
to forcewake around everywhere we need it.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_d
index 12ea1a9..9152cba 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9648,6 +9648,9 @@ static void i915_disable_vga(struct drm_device *dev)
void intel_modeset_init_hw(struct drm_device *dev)
{
+ /* BIOS often leaves RC6 enabled, but disable it for hw init */
+ intel_disable_gt_powersave(dev);
+
intel_init_power_well(dev);
intel_prepare_ddi(dev);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Tue, 16 Jul 2013 11:34:25 +0400
Konstantin Khlebnikov khlebni...@openvz.org wrote:
I've tested that patch and it really works for me. If you want change
something for other hardware or
extend range where forcewake is held prease do it in a separate patch.
This will be good for bisecting new bugs in the future.
Thanks a ton for finding this Konstantin, it puts us on the right
track.
Can I ask you to test this patch? The theory is that having RC6
enabled messes with the initial programming sequence, so it's probably
best to just shut it off at init until we're done, rather than trying
to forcewake around everywhere we need it.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_d
index 12ea1a9..9152cba 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9648,6 +9648,9 @@ static void i915_disable_vga(struct drm_device *dev)
void intel_modeset_init_hw(struct drm_device *dev)
{
+ /* BIOS often leaves RC6 enabled, but disable it for hw init */
+ intel_disable_gt_powersave(dev);
+
intel_init_power_well(dev);
intel_prepare_ddi(dev);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Tue, 16 Jul 2013 10:06:54 -0700 Jesse Barnes jbar...@virtuousgeek.org wrote: On Tue, 16 Jul 2013 11:34:25 +0400 Konstantin Khlebnikov khlebni...@openvz.org wrote: I've tested that patch and it really works for me. If you want change something for other hardware or extend range where forcewake is held prease do it in a separate patch. This will be good for bisecting new bugs in the future. Thanks a ton for finding this Konstantin, it puts us on the right track. Can I ask you to test this patch? The theory is that having RC6 enabled messes with the initial programming sequence, so it's probably best to just shut it off at init until we're done, rather than trying to forcewake around everywhere we need it. Oops, last one triggers a warn about IRQs. This one doesn't and still works for me. Testing welcome. Thanks, -- Jesse Barnes, Intel Open Source Technology Center diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c9d9d20..d962ec0 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4112,6 +4112,9 @@ i915_gem_init_hw(struct drm_device *dev) drm_i915_private_t *dev_priv = dev-dev_private; int ret; + /* BIOS often leaves RC6 enabled, but disable it for hw init */ + intel_disable_gt_powersave(dev); + if (INTEL_INFO(dev)-gen 6 !intel_enable_gtt()) return -EIO; -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Tue, Jul 16, 2013 at 01:19:25PM -0700, Jesse Barnes wrote: On Tue, 16 Jul 2013 10:06:54 -0700 Jesse Barnes jbar...@virtuousgeek.org wrote: On Tue, 16 Jul 2013 11:34:25 +0400 Konstantin Khlebnikov khlebni...@openvz.org wrote: I've tested that patch and it really works for me. If you want change something for other hardware or extend range where forcewake is held prease do it in a separate patch. This will be good for bisecting new bugs in the future. Thanks a ton for finding this Konstantin, it puts us on the right track. Can I ask you to test this patch? The theory is that having RC6 enabled messes with the initial programming sequence, so it's probably best to just shut it off at init until we're done, rather than trying to forcewake around everywhere we need it. Oops, last one triggers a warn about IRQs. This one doesn't and still works for me. Testing welcome. Thanks, -- Jesse Barnes, Intel Open Source Technology Center diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c9d9d20..d962ec0 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4112,6 +4112,9 @@ i915_gem_init_hw(struct drm_device *dev) drm_i915_private_t *dev_priv = dev-dev_private; int ret; + /* BIOS often leaves RC6 enabled, but disable it for hw init */ + intel_disable_gt_powersave(dev); I think it'd be better to have an explicit gen = 6 check here and a disable_rps call. disable_gt_powersave also calls the ironlake version, which restores probably bogus values (since we haven't read them out yet in the enable code) into the hw. -Daniel + if (INTEL_INFO(dev)-gen 6 !intel_enable_gtt()) return -EIO; ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Tue, 16 Jul 2013 22:43:49 +0200 Daniel Vetter dan...@ffwll.ch wrote: On Tue, Jul 16, 2013 at 01:19:25PM -0700, Jesse Barnes wrote: On Tue, 16 Jul 2013 10:06:54 -0700 Jesse Barnes jbar...@virtuousgeek.org wrote: On Tue, 16 Jul 2013 11:34:25 +0400 Konstantin Khlebnikov khlebni...@openvz.org wrote: I've tested that patch and it really works for me. If you want change something for other hardware or extend range where forcewake is held prease do it in a separate patch. This will be good for bisecting new bugs in the future. Thanks a ton for finding this Konstantin, it puts us on the right track. Can I ask you to test this patch? The theory is that having RC6 enabled messes with the initial programming sequence, so it's probably best to just shut it off at init until we're done, rather than trying to forcewake around everywhere we need it. Oops, last one triggers a warn about IRQs. This one doesn't and still works for me. Testing welcome. Thanks, -- Jesse Barnes, Intel Open Source Technology Center diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c9d9d20..d962ec0 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4112,6 +4112,9 @@ i915_gem_init_hw(struct drm_device *dev) drm_i915_private_t *dev_priv = dev-dev_private; int ret; + /* BIOS often leaves RC6 enabled, but disable it for hw init */ + intel_disable_gt_powersave(dev); I think it'd be better to have an explicit gen = 6 check here and a disable_rps call. disable_gt_powersave also calls the ironlake version, which restores probably bogus values (since we haven't read them out yet in the enable code) into the hw. Yeah the ilk restore to initial freq needs fixing, but I thought we wanted this on all gens? It shouldn't hurt anything, and may help with other issues as well. -- Jesse Barnes, Intel Open Source Technology Center -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
