From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915/vlv: modeset_global_* for VLV v6 Date: Mon, 4 Nov 2013 23:28:10 +0200 Message-ID: <20131104212809.GZ13047@intel.com> References: <1383594766-6042-3-git-send-email-jbarnes@virtuousgeek.org> <1383598464-6966-1-git-send-email-jbarnes@virtuousgeek.org> <20131104212203.GY13047@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id B52A6101C0B for ; Mon, 4 Nov 2013 13:28:13 -0800 (PST) Content-Disposition: inline In-Reply-To: <20131104212203.GY13047@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Jesse Barnes Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Nov 04, 2013 at 11:22:03PM +0200, Ville Syrj=E4l=E4 wrote: > On Mon, Nov 04, 2013 at 12:54:24PM -0800, Jesse Barnes wrote: > > On VLV/BYT, we can adjust the CDclk frequency up or down based on the > > max pixel clock we need to drive. Lowering it can save power, while > > raising it is necessary to support high resolution. > > = > > Add a new callback in modeset_affected_pipes and a > > modeset_global_resources function to perform this adjustment as > > necessary. > > = > > v2: use punit interface for 320 and 266 MHz CDclk adjustments (Ville) > > v3: reset GMBUS dividers too, since we changed CDclk (Ville) > > v4: jump to highest voltage when going to 400MHz CDclk (Jesse) > > v5: drop duplicate define (Ville) > > use shifts by 1 for fixed point (Ville) > > drop new callback (Daniel) > > v6: fixup adjusted_mode.clock -> adjusted_mode.crtc_clock again (Ville) > > document Bunit reg access better (Ville) > > = > > Signed-off-by: Jesse Barnes > > --- > > drivers/gpu/drm/i915/i915_reg.h | 9 ++ > > drivers/gpu/drm/i915/intel_display.c | 179 +++++++++++++++++++++++++++= ++++++++ > > drivers/gpu/drm/i915/intel_i2c.c | 4 - > > 3 files changed, 188 insertions(+), 4 deletions(-) > > = > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i91= 5_reg.h > > index 737d8a3..d644d75 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -360,9 +360,17 @@ > > #define VLV_IOSF_DATA (VLV_DISPLAY_BASE + 0x2104) > > #define VLV_IOSF_ADDR (VLV_DISPLAY_BASE + 0x2108) > > = > > +/* See configdb bunit SB addr map */ > > +#define BUNIT_REG_BISOC 0x11 > > + > > #define PUNIT_OPCODE_REG_READ 6 > > #define PUNIT_OPCODE_REG_WRITE 7 > > = > > +#define PUNIT_REG_DSPFREQ 0x36 > > +#define DSPFREQSTAT_SHIFT 30 > > +#define DSPFREQSTAT_MASK (0x3 << DSPFREQSTAT_SHIFT) > > +#define DSPFREQGUAR_SHIFT 14 > > +#define DSPFREQGUAR_MASK (0x3 << DSPFREQGUAR_SHIFT) > > #define PUNIT_REG_PWRGT_CTRL 0x60 > > #define PUNIT_REG_PWRGT_STATUS 0x61 > > #define PUNIT_CLK_GATE 1 > > @@ -425,6 +433,7 @@ > > #define DSI_PLL_N1_DIV_MASK (3 << 16) > > #define DSI_PLL_M1_DIV_SHIFT 0 > > #define DSI_PLL_M1_DIV_MASK (0x1ff << 0) > > +#define CCK_DISPLAY_CLOCK_CONTROL 0x6b > > = > > /* > > * DPIO - a special bus for various display related registers to hide = behind > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i91= 5/intel_display.c > > index 606a594..ae83f01 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -3894,6 +3894,175 @@ static void i9xx_pfit_enable(struct intel_crtc = *crtc) > > I915_WRITE(BCLRPAT(crtc->pipe), 0); > > } > > = > > +static int valleyview_get_vco(struct drm_i915_private *dev_priv) > > +{ > > + int vco; > > + > > + switch (dev_priv->mem_freq) { > > + default: > > + case 800: > > + vco =3D 800; > > + break; > > + case 1066: > > + vco =3D 1600; > > + break; > > + case 1333: > > + vco =3D 2000; > > + break; > > + } > > + > > + return vco; > > +} > > + > > +/* Adjust CDclk dividers to allow high res or save power if possible */ > > +static void valleyview_set_cdclk(struct drm_device *dev, int cdclk) > > +{ > > + struct drm_i915_private *dev_priv =3D dev->dev_private; > > + u32 val, cmd; > > + > > + if (cdclk >=3D 320) /* jump to highest voltage for 400MHz too */ > > + cmd =3D 2; > > + else if (cdclk =3D=3D 266) > > + cmd =3D 1; > > + else > > + cmd =3D 0; > > + > > + mutex_lock(&dev_priv->rps.hw_lock); > > + val =3D vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ); > > + val &=3D ~DSPFREQGUAR_MASK; > > + val |=3D (cmd << DSPFREQGUAR_SHIFT); > > + vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val); > > + if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) & > > + DSPFREQSTAT_MASK) =3D=3D (cmd << DSPFREQSTAT_SHIFT), > > + 50)) { > > + DRM_ERROR("timed out waiting for CDclk change\n"); > > + } > > + mutex_unlock(&dev_priv->rps.hw_lock); > > + > > + if (cdclk =3D=3D 400) { > > + u32 divider, vco; > > + > > + vco =3D valleyview_get_vco(dev_priv); > > + divider =3D ((vco << 1) / cdclk) - 1; > > + > > + mutex_lock(&dev_priv->dpio_lock); > > + /* adjust cdclk divider */ > > + val =3D vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL); > > + val &=3D ~0xf; > > + val |=3D divider; > > + vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL, val); > > + mutex_unlock(&dev_priv->dpio_lock); > > + } > > + > > + mutex_lock(&dev_priv->dpio_lock); > > + /* adjust self-refresh exit latency value */ > > + val =3D vlv_bunit_read(dev_priv, BUNIT_REG_BISOC); > > + val &=3D ~0x7f; > > + > > + /* > > + * For high bandwidth configs, we set a higher latency in the bunit > > + * so that the core display fetch happens in time to avoid underruns. > > + */ > > + if (cdclk =3D=3D 400) > > + val |=3D 4500 / 250; /* 4.5 usec */ > > + else > > + val |=3D 3000 / 250; /* 3.0 usec */ > > + vlv_bunit_write(dev_priv, BUNIT_REG_BISOC, val); > > + mutex_unlock(&dev_priv->dpio_lock); > > + > > + /* Since we changed the CDclk, we need to update the GMBUSFREQ too */ > > + intel_i2c_reset(dev); > > +} > > + > > +static int valleyview_cur_cdclk(struct drm_i915_private *dev_priv) > > +{ > > + int cur_cdclk, vco; > > + int divider; > > + > > + vco =3D valleyview_get_vco(dev_priv); > > + > > + mutex_lock(&dev_priv->dpio_lock); > > + divider =3D vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL); > > + mutex_unlock(&dev_priv->dpio_lock); > > + > > + divider &=3D 0xf; > > + > > + cur_cdclk =3D (vco << 1) / (divider + 1); > > + > > + return cur_cdclk; > > +} > > + > > +static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv, > > + int max_pixclk) > > +{ > > + int cur_cdclk; > > + > > + cur_cdclk =3D valleyview_cur_cdclk(dev_priv); > > + > > + /* > > + * Really only a few cases to deal with, as only 4 CDclks are support= ed: > > + * 200MHz > > + * 267MHz > > + * 320MHz > > + * 400MHz > > + * So we check to see whether we're above 90% of the lower bin and > > + * adjust if needed. > > + */ > > + if (max_pixclk > 288000) { > > + return 400; > > + } else if (max_pixclk > 240000) { > > + return 320; > > + } else > > + return 266; > > + /* Looks like the 200MHz CDclk freq doesn't work on some configs */ > > +} > > + > > +static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv) > > +{ > > + struct drm_device *dev =3D dev_priv->dev; > > + struct intel_crtc *intel_crtc; > > + int max_pixclk =3D 0; > > + > > + list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list, > > + base.head) { > > + if (!intel_crtc->base.enabled) > > + continue; > > + > > + max_pixclk =3D max(max_pixclk, > > + intel_crtc->config.adjusted_mode.crtc_clock); > > + } > > + > > + return max_pixclk; > > +} > > + > > +static void valleyview_modeset_global_pipes(struct drm_device *dev, > > + unsigned *prepare_pipes) > > +{ > > + struct drm_i915_private *dev_priv =3D dev->dev_private; > > + struct intel_crtc *intel_crtc; > > + int max_pixclk =3D intel_mode_max_pixclk(dev_priv); > > + int cur_cdclk =3D valleyview_cur_cdclk(dev_priv); > > + > > + if (valleyview_calc_cdclk(dev_priv, max_pixclk) =3D=3D cur_cdclk) > > + return; > > + > > + list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list, > > + base.head) > > + if (intel_crtc->base.enabled) > > + *prepare_pipes |=3D (1 << intel_crtc->pipe); > > +} > > + > > > = > = > So now the problem is now that we have to calculate max_pixclk before > intel_crtc->config has been update for modeset_pipes. > = > If we make intel_mode_max_pixclk() and valleyview_modeset_global_pipes() = like so: > = > static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv, > unsigned int modeset_pipes, > struct intel_crtc_config *pipe_config) > { > struct drm_device *dev =3D dev_priv->dev; > struct intel_crtc *intel_crtc; > int max_pixclk =3D 0; > = > list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list, base.head) { > if (!intel_crtc->base.enabled) > continue; Hmm. After a second though, this enabled check is still wrong. But we know that if the pipe is in modeset_pipes it will be enabled, so we could move the enabled check to the second branch like so: if (modeset_pipes & (1 << pipe)) max(pipe_config); else if (crtc->enabled) max(intel_crtc->config); > = > if (modeset_pipes & (1 << intel_crtc_pipe)) > max_pixclk =3D max(max_pixclk, pipe_config->adjusted_mode.crtc_clock); > else > max_pixclk =3D max(max_pixclk, intel_crtc->config.adjusted_mode.crtc_c= lock); > } > = > return max_pixclk; > } > = > static void valleyview_modeset_global_pipes(struct drm_device *dev, > unsigned *prepare_pipes, > unsigned int modeset_pipes, > struct intel_crtc_config *pipe_config) > { > struct drm_i915_private *dev_priv =3D dev->dev_private; > struct intel_crtc *intel_crtc; > int max_pixclk =3D intel_mode_max_pixclk(dev_priv, modeset_pipes, pipe_c= onfig); > int cur_cdclk =3D valleyview_cur_cdclk(dev_priv); > = > if (valleyview_calc_cdclk(dev_priv, max_pixclk) =3D=3D cur_cdclk) > return; > = > list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list, > base.head) > if (intel_crtc->base.enabled) > *prepare_pipes |=3D (1 << intel_crtc->pipe); > } > = > And then we calculate it after the new pipe config has been computed like= so: > = > ... > if (modeset_pipes) { > pipe_config =3D intel_modeset_pipe_config(crtc, fb, mode); > if (IS_ERR(pipe_config)) { > ret =3D PTR_ERR(pipe_config); > pipe_config =3D NULL; > = > goto out; > } > intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config, > "[modeset]"); > } > = > + if (IS_VALLEYVIEW(dev)) > + valleyview_modeset_global_pipes(dev, &prepare_pipes, modeset_pipes, pi= pe_config); > = > for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc) > intel_crtc_disable(&intel_crtc->base); > ... > = > = > Then things should just work (tm). > = > -- = > Ville Syrj=E4l=E4 > Intel OTC -- = Ville Syrj=E4l=E4 Intel OTC