From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 3/3] drm/i915/vlv: modeset_global_* for VLV v5 Date: Mon, 4 Nov 2013 22:37:57 +0200 Message-ID: <20131104203757.GX13047@intel.com> References: <1383594766-6042-1-git-send-email-jbarnes@virtuousgeek.org> <1383594766-6042-3-git-send-email-jbarnes@virtuousgeek.org> 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 178D5FB064 for ; Mon, 4 Nov 2013 12:38:01 -0800 (PST) Content-Disposition: inline In-Reply-To: <1383594766-6042-3-git-send-email-jbarnes@virtuousgeek.org> 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:52:46AM -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) > = > Signed-off-by: Jesse Barnes > --- > drivers/gpu/drm/i915/i915_reg.h | 6 ++ > drivers/gpu/drm/i915/intel_display.c | 179 +++++++++++++++++++++++++++++= ++++++ > drivers/gpu/drm/i915/intel_i2c.c | 4 - > 3 files changed, 185 insertions(+), 4 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_= reg.h > index 737d8a3..5708a6f 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -363,6 +363,11 @@ > #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 +430,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 be= hind > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c > index 606a594..8cf42a6 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 *c= rtc) > 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, 0x11); > + 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 0x12; > + else > + val |=3D 0xc; I just went trawling in configdb to figure out what this does. Maybe we should make this a bit less magic. For example: /* * ... set a higher self refresh exit latency ... */ 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, 0x11, 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 supported: > + * 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; > + > + if (max_pixclk < intel_crtc->config.adjusted_mode.clock) ^^^^^ Maybe just write it as: max_pixclk =3D max(max_pixclk, intel_crtc->config.adjusted_mode.crtc_clock); Otherwise it's looking pretty good to me. > + max_pixclk =3D 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); > +} > + > +static void valleyview_modeset_global_resources(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + int max_pixclk =3D intel_mode_max_pixclk(dev_priv); > + int cur_cdclk =3D valleyview_cur_cdclk(dev_priv); > + int req_cdclk =3D valleyview_calc_cdclk(dev_priv, max_pixclk); > + > + if (req_cdclk !=3D cur_cdclk) > + valleyview_set_cdclk(dev, req_cdclk); > +} > + > static void valleyview_crtc_enable(struct drm_crtc *crtc) > { > struct drm_device *dev =3D crtc->dev; > @@ -8844,6 +9013,13 @@ intel_modeset_affected_pipes(struct drm_crtc *crtc= , unsigned *modeset_pipes, > *modeset_pipes &=3D 1 << intel_crtc->pipe; > *prepare_pipes &=3D 1 << intel_crtc->pipe; > = > + /* > + * See if the config requires any additional preparation, e.g. > + * to adjust global state with pipes off. > + */ > + if (IS_VALLEYVIEW(dev)) > + valleyview_modeset_global_pipes(dev, prepare_pipes); > + > DRM_DEBUG_KMS("set mode pipe masks: modeset: %x, prepare: %x, disable: = %x\n", > *modeset_pipes, *prepare_pipes, *disable_pipes); > } > @@ -10329,6 +10505,9 @@ static void intel_init_display(struct drm_device = *dev) > } > } else if (IS_G4X(dev)) { > dev_priv->display.write_eld =3D g4x_write_eld; > + } else if (IS_VALLEYVIEW(dev)) { > + dev_priv->display.modeset_global_resources =3D > + valleyview_modeset_global_resources; > } > = > /* Default just returns -ENODEV to indicate unsupported */ > diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/inte= l_i2c.c > index 2ca17b1..1263409 100644 > --- a/drivers/gpu/drm/i915/intel_i2c.c > +++ b/drivers/gpu/drm/i915/intel_i2c.c > @@ -87,10 +87,6 @@ static void gmbus_set_freq(struct drm_i915_private *de= v_priv) > = > BUG_ON(!IS_VALLEYVIEW(dev_priv->dev)); > = > - /* Skip setting the gmbus freq if BIOS has already programmed it */ > - if (I915_READ(GMBUSFREQ_VLV) !=3D 0xA0) > - return; > - > /* Obtain SKU information */ > mutex_lock(&dev_priv->dpio_lock); > hpll_freq =3D > -- = > 1.8.3.1 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC