On Wed, Sep 25, 2013 at 07:14:34AM +0800, Lee, Chon Ming wrote:
> On 09/24 18:18, Ville Syrjälä wrote:
> > On Tue, Sep 24, 2013 at 03:14:27PM +0800, Chon Ming Lee wrote:
> > > Without the DPIO cmnreset, the PLL fail to lock. This should have
> > > done by BIOS.
> > >
> > > v2: Move this to intel_uncore_sanitize to allow it to get call during
> > > resume path. (Daniel)
> > > v3: Remove redundant write 0 to DPIO_CTL, and use DPIO_RESET instead of
> > > just 0x1 (Ville)
> > > Without BIOS, DPIO/render well/media well may still power gated.
> > > Turn it off.
> > >
> > > Signed-off-by: Chon Ming Lee <chon.ming.lee@xxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/i915_reg.h | 9 +++++++++
> > > drivers/gpu/drm/i915/intel_uncore.c | 23 +++++++++++++++++++++++
> > > 2 files changed, 32 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index c4f9bef..c02f893 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -361,6 +361,15 @@
> > > #define PUNIT_OPCODE_REG_READ 6
> > > #define PUNIT_OPCODE_REG_WRITE 7
> > >
> > > +#define PUNIT_REG_PWRGT_CTRL 0x60
> > > +#define PUNIT_REG_PWRGT_STATUS 0x61
> > > +#define PUNIT_CLK_GATE 1
> > > +#define PUNIT_PWR_RESET 2
> > > +#define PUNIT_PWR_GATE 3
> > > +#define RENDER_PWRGT (PUNIT_PWR_GATE << 0)
> > > +#define MEDIA_PWRGT (PUNIT_PWR_GATE << 2)
> > > +#define DPIO_PWRGT (PUNIT_PWR_GATE << 6)
> >
> > Subsys 6 seems to be one of four TX lanes, and there's also the common
> > lane subsys, and the disp2d is one as well. RX supposedly is not relevant
> > for display PHY, not sure why it has subsys allocated too.
> >
> > Anyways my point would be that shouldn't we check all subsys ie render + media +
> > disp2d + common lane + all tx lanes?
> >
> By default, the common lane + all tx lanes are not power gated during cold boot
> or system resume. Unless S0ix entry actually power gate it by driver, then,
> this will need to add to turn off it.
OK. And as Imre pointed out to me the '<< 6' isn't a TX lane as I
claimed but the display subsystems (3). I assume that's the same thing
as the disp2d block, ie. the pipes. So the DPIO in the name is
wrong. It should be called display or disp2d I think.
If you say the PHY side isn't power gated during cold boot, I think we
can ignore it for now.
So if you rename the DPIO thing, this patch should be OK.
>
> > And should we maybe power gate the RX lanes always as those shouldn't be needed
> > for display?
>
> Yes, you are correct. I believe there should be another patch to do it, to
> enable power gate the VLV correctly for SOix entry or exit.
My plan is to power gate everything we can during runtime, not just s0ix.
But that's a biger topic we can discuss once Imre gets some relevant
patches ready.
> >
> > > +
> > > #define PUNIT_REG_GPU_LFM 0xd3
> > > #define PUNIT_REG_GPU_FREQ_REQ 0xd4
> > > #define PUNIT_REG_GPU_FREQ_STS 0xd8
> > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > > index 8649f1c..6923b4d 100644
> > > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > > @@ -276,10 +276,33 @@ static void intel_uncore_forcewake_reset(struct drm_device *dev)
> > >
> > > void intel_uncore_sanitize(struct drm_device *dev)
> > > {
> > > + struct drm_i915_private *dev_priv = dev->dev_private;
> > > + u32 reg_val;
> > > +
> > > intel_uncore_forcewake_reset(dev);
> > >
> > > /* BIOS often leaves RC6 enabled, but disable it for hw init */
> > > intel_disable_gt_powersave(dev);
> > > +
> > > + /* Trigger DPIO CMN RESET and turn off power gate, require
> > > + * especially in BIOS less system
> > > + */
> > > + if (IS_VALLEYVIEW(dev)) {
> > > +
> > > + mutex_lock(&dev_priv->rps.hw_lock);
> > > + reg_val = vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_STATUS);
> > > +
> > > + if (reg_val & (RENDER_PWRGT | MEDIA_PWRGT | DPIO_PWRGT))
> > > + vlv_punit_write(dev_priv, PUNIT_REG_PWRGT_CTRL, 0x0);
> > > +
> > > + mutex_unlock(&dev_priv->rps.hw_lock);
> > > +
> > > + reg_val = I915_READ(DPIO_CTL);
> > > + if (!(reg_val & DPIO_RESET)) {
> > > + I915_WRITE(DPIO_CTL, DPIO_RESET);
> > > + POSTING_READ(DPIO_CTL);
> > > + }
> > > + }
> > > }
> > >
> > > /*
> > > --
> > > 1.7.7.6
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Ville Syrjälä
> > Intel OTC
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
