On Fri, 23 Aug 2013 13:44:17 -0300
Paulo Zanoni <przanoni@xxxxxxxxx> wrote:
> 2013/8/23 Jani Nikula <jani.nikula@xxxxxxxxx>:
>
> /* Please insert explanation on why we need this and what changes if
> we do this. */
>
> I applied your patches and booted them. I got into PC8, did the PC8
> test suite and nothing changed. I really don't know what to expect
> from this series and/or how to check what's improving. Also, see
> below:
>
So this is one of these things that will have no visible impact on
i915, but will impact other parts of the system. So I think the only
way to test it is by throwing it on the SIP board and checking the
power level of the components this impacts (Audio, thermal, KBC/EC,
LPT). And without the code which does the actual PCI D3 request from
i915, nothing will happen. Is it possible to get a patch which finds
some very obvious place to put the controller into D3 so we can check
to see if the opregion notifies are doing what they are supposed to?
>
> > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index a6df68e..7ed2248 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6093,6 +6093,8 @@ void hsw_enable_pc8_work(struct work_struct *__work)
> > lpt_disable_clkout_dp(dev);
> > hsw_pc8_disable_interrupts(dev);
> > hsw_disable_lcpll(dev_priv, true, true);
> > +
> > + intel_opregion_notify_adapter(dev, PCI_D1);
>
> Why D1? Shouldn't this be D3? I think that's what people having been
> asking us to implement.
>
> On the doc that explains "adapter power state notification", my
> understanding is that it suggests that we should call this _before_ we
> go into the lower states and the other chunk should be called _after_
> we're at the higher power states. So perhaps we should call
> intel_opregion_notify_adapter before hsw_disable_lcpll, and, on the
> chunk below, after hsw_restore_lcpll? But this is not 100% clear, I
> may be wrong.
>
> By the way, I modified your patch to implement the suggestions above,
> and got the same results: no noticeable difference, everything still
> works. No news is good news?
>
>
> > }
> >
> > static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)
> > @@ -6126,6 +6128,8 @@ static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
> > if (!dev_priv->pc8.enabled)
> > return;
> >
> > + intel_opregion_notify_adapter(dev, PCI_D0);
> > +
> > DRM_DEBUG_KMS("Disabling package C8+\n");
> >
> > hsw_restore_lcpll(dev_priv);
> > --
> > 1.7.9.5
> >
>
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
