From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 1/2] drm/i915: Send a DPIO cmnreset during driver load or system resume. Date: Wed, 25 Sep 2013 10:25:37 +0300 Message-ID: <20130925072537.GH4531@intel.com> References: <1380006867-20771-1-git-send-email-chon.ming.lee@intel.com> <20130924151815.GF4531@intel.com> <20130924231434.GA24292@ubuntu.png.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 2FE3FE7A40 for ; Wed, 25 Sep 2013 00:25:41 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130924231434.GA24292@ubuntu.png.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: "Lee, Chon Ming" Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, Sep 25, 2013 at 07:14:34AM +0800, Lee, Chon Ming wrote: > On 09/24 18:18, Ville Syrj=E4l=E4 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 > > > --- > > > 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/i= 915_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 releva= nt > > 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 col= d boot > or system resume. Unless S0ix entry actually power gate it by driver, th= en, > 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 b= e 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/i9= 15/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 =3D 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 =3D 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 =3D 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@lists.freedesktop.org > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > = > > -- = > > Ville Syrj=E4l=E4 > > Intel OTC -- = Ville Syrj=E4l=E4 Intel OTC