| Message ID | 1418374096-26561-3-git-send-email-deepak.s@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Dec 12, 2014 at 02:18:15PM +0530, deepak.s@linux.intel.com wrote: > From: Deepak S <deepak.s@linux.intel.com> > > Use new Sideband offset to read max/min/gaur freq based on the SKU it > is running on. Based on the Number of EU, we read different bits to > identify the max frequencies at which system can run. > > Signed-off-by: Deepak S <deepak.s@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 4 +-- > drivers/gpu/drm/i915/i915_reg.h | 12 ++++++++ > drivers/gpu/drm/i915/intel_pm.c | 52 ++++++++++++++++++++++++++++++----- > drivers/gpu/drm/i915/intel_sideband.c | 4 +-- > 4 files changed, 61 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index b58bad4..0690dff 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3016,8 +3016,8 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u32 mbox, u32 *val > int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u32 mbox, u32 val); > > /* intel_sideband.c */ > -u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr); > -void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val); > +u32 vlv_punit_read(struct drm_i915_private *dev_priv, u32 addr); > +void vlv_punit_write(struct drm_i915_private *dev_priv, u32 addr, u32 val); > u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr); > u32 vlv_gpio_nc_read(struct drm_i915_private *dev_priv, u32 reg); > void vlv_gpio_nc_write(struct drm_i915_private *dev_priv, u32 reg, u32 val); > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index b57cba3..f41290c 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -602,6 +602,18 @@ enum punit_power_well { > #define PUNIT_FUSE_BUS2 0xf6 /* bits 47:40 */ > #define PUNIT_FUSE_BUS1 0xf5 /* bits 55:48 */ > > +#define FB_GFX_FMAX_AT_VMAX_FUSE 0x136 > +#define FB_GFX_FMAX_AT_VMAX_FUSE_MASK 0xff > +#define FB_GFX_FMAX_AT_VMAX_2SS4EU_FUSE_SHIFT 24 > +#define FB_GFX_FMAX_AT_VMAX_2SS6EU_FUSE_SHIFT 16 > +#define FB_GFX_FMAX_AT_VMAX_2SS8EU_FUSE_SHIFT 8 > + This blank line makes me think FB_GFX_GUAR_FREQ_FUSE_MASK isn't part of this register. So best not leave such blank line here. I have 0x3c3c3c28 in this register, which matches what I get using the old method. > +#define FB_GFX_GUAR_FREQ_FUSE_MASK 0xff > + > +#define FB_GFX_FMIN_AT_VMIN_FUSE 0x137 > +#define FB_GFX_FMIN_AT_VMIN_FUSE_MASK 0xff > +#define FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT 8 I have 0x69841428 here. The low 8 bits look like another freq value. What is it? I have no docs for this stuff so can't really review apart from looking at what my hardware reports. Actually, since all the values are 8 bits maybe it would be neater to just #define FB_GFX_FREQ_FUSE_MASK 0xff and use that everywhere instead of having three different definitions for the same 0xff value. > + > #define PUNIT_GPU_STATUS_REG 0xdb > #define PUNIT_GPU_STATUS_MAX_FREQ_SHIFT 16 > #define PUNIT_GPU_STATUS_MAX_FREQ_MASK 0xff > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 2acb3de..71b8e2f 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4346,11 +4346,29 @@ void gen6_update_ring_freq(struct drm_device *dev) > > static int cherryview_rps_max_freq(struct drm_i915_private *dev_priv) > { > + struct drm_device *dev = dev_priv->dev; > + struct intel_device_info *info; > u32 val, rp0; > > - val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG); > - rp0 = (val >> PUNIT_GPU_STATUS_MAX_FREQ_SHIFT) & PUNIT_GPU_STATUS_MAX_FREQ_MASK; > - > + info = (struct intel_device_info *)&dev_priv->info; Pointless cast. Also the assignment could be done when declaring info, and we usually use INTEL_INFO() to get at it. > + > + if (dev->pdev->revision >= 0x20) { Do we really need this check? I would think it would be up to the Punit firmware version rather the stepping. My BSW has PCI rev 0x15, but with the latest BIOS both fuse registers already contain correct looking information. > + val = vlv_punit_read(dev_priv, FB_GFX_FMAX_AT_VMAX_FUSE); > + > + if (info->eu_total == 8) /* (2 * 4) config */ > + rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS4EU_FUSE_SHIFT); > + else if (info->eu_total == 12) /* (2 * 6) config */ > + rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS6EU_FUSE_SHIFT); > + else if (info->eu_total == 16) /* (2 * 8) config */ > + rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS8EU_FUSE_SHIFT); > + else /* Setting (2 * 8) Min RP0 for any other combination */ > + rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS8EU_FUSE_SHIFT); 'switch (INTEL_INFO(dev)->eu_total)' perhaps? > + rp0 = (rp0 & FB_GFX_FMAX_AT_VMAX_FUSE_MASK); > + } else { /* For pre-production hardware */ > + val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG); > + rp0 = (val >> PUNIT_GPU_STATUS_MAX_FREQ_SHIFT) & > + PUNIT_GPU_STATUS_MAX_FREQ_MASK; > + } > return rp0; > } > > @@ -4366,20 +4384,40 @@ static int cherryview_rps_rpe_freq(struct drm_i915_private *dev_priv) > > static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv) > { > + struct drm_device *dev = dev_priv->dev; > + struct intel_device_info *info; > u32 val, rp1; > > - val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS); > - rp1 = (val >> PUNIT_GPU_STATUS_MAX_FREQ_SHIFT) & PUNIT_GPU_STATUS_MAX_FREQ_MASK; > + info = (struct intel_device_info *)&dev_priv->info; Unused here. > > + if (dev->pdev->revision >= 0x20) { > + val = vlv_punit_read(dev_priv, FB_GFX_FMAX_AT_VMAX_FUSE); > + rp1 = (val & FB_GFX_GUAR_FREQ_FUSE_MASK); > + } else { /* For pre-production hardware */ > + val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS); > + rp1 = ((val >> PUNIT_GPU_STATUS_MAX_FREQ_SHIFT) & > + PUNIT_GPU_STATUS_MAX_FREQ_MASK); > + } > return rp1; > } > > static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv) > { > + struct drm_device *dev = dev_priv->dev; > + struct intel_device_info *info; > u32 val, rpn; > + info = (struct intel_device_info *)&dev_priv->info; Unused. > + > + if (dev->pdev->revision >= 0x20) { > + val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE); > + rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) & > + FB_GFX_FMIN_AT_VMIN_FUSE_MASK); > + } else { /* For pre-production hardware */ > + val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG); > + rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) & > + PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK); > + } > > - val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG); > - rpn = (val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) & PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK; > return rpn; > } > > diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c > index 01d841e..3c42eef 100644 > --- a/drivers/gpu/drm/i915/intel_sideband.c > +++ b/drivers/gpu/drm/i915/intel_sideband.c > @@ -75,7 +75,7 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn, > return 0; > } > > -u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr) > +u32 vlv_punit_read(struct drm_i915_private *dev_priv, u32 addr) Good thing you had to pass a constant. Otherwise this could have caused quite a bit of head scratching for you ;) > { > u32 val = 0; > > @@ -89,7 +89,7 @@ u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr) > return val; > } > > -void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val) > +void vlv_punit_write(struct drm_i915_private *dev_priv, u32 addr, u32 val) > { > WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock)); > > -- > 1.9.1
On Fri, 12 Dec 2014, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Fri, Dec 12, 2014 at 02:18:15PM +0530, deepak.s@linux.intel.com wrote: >> From: Deepak S <deepak.s@linux.intel.com> >> >> Use new Sideband offset to read max/min/gaur freq based on the SKU it >> is running on. Based on the Number of EU, we read different bits to >> identify the max frequencies at which system can run. >> >> Signed-off-by: Deepak S <deepak.s@linux.intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 4 +-- >> drivers/gpu/drm/i915/i915_reg.h | 12 ++++++++ >> drivers/gpu/drm/i915/intel_pm.c | 52 ++++++++++++++++++++++++++++++----- >> drivers/gpu/drm/i915/intel_sideband.c | 4 +-- >> 4 files changed, 61 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index b58bad4..0690dff 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -3016,8 +3016,8 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u32 mbox, u32 *val >> int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u32 mbox, u32 val); >> >> /* intel_sideband.c */ >> -u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr); >> -void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val); >> +u32 vlv_punit_read(struct drm_i915_private *dev_priv, u32 addr); >> +void vlv_punit_write(struct drm_i915_private *dev_priv, u32 addr, u32 val); >> u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr); >> u32 vlv_gpio_nc_read(struct drm_i915_private *dev_priv, u32 reg); >> void vlv_gpio_nc_write(struct drm_i915_private *dev_priv, u32 reg, u32 val); >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index b57cba3..f41290c 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -602,6 +602,18 @@ enum punit_power_well { >> #define PUNIT_FUSE_BUS2 0xf6 /* bits 47:40 */ >> #define PUNIT_FUSE_BUS1 0xf5 /* bits 55:48 */ >> >> +#define FB_GFX_FMAX_AT_VMAX_FUSE 0x136 >> +#define FB_GFX_FMAX_AT_VMAX_FUSE_MASK 0xff >> +#define FB_GFX_FMAX_AT_VMAX_2SS4EU_FUSE_SHIFT 24 >> +#define FB_GFX_FMAX_AT_VMAX_2SS6EU_FUSE_SHIFT 16 >> +#define FB_GFX_FMAX_AT_VMAX_2SS8EU_FUSE_SHIFT 8 >> + > > This blank line makes me think FB_GFX_GUAR_FREQ_FUSE_MASK isn't part of > this register. So best not leave such blank line here. > > I have 0x3c3c3c28 in this register, which matches what I get using the > old method. > >> +#define FB_GFX_GUAR_FREQ_FUSE_MASK 0xff >> + >> +#define FB_GFX_FMIN_AT_VMIN_FUSE 0x137 >> +#define FB_GFX_FMIN_AT_VMIN_FUSE_MASK 0xff >> +#define FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT 8 > > I have 0x69841428 here. The low 8 bits look like another freq value. > What is it? > > I have no docs for this stuff so can't really review apart from looking > at what my hardware reports. > > > Actually, since all the values are 8 bits maybe it would be neater to > just > #define FB_GFX_FREQ_FUSE_MASK 0xff > and use that everywhere instead of having three different definitions > for the same 0xff value. > >> + >> #define PUNIT_GPU_STATUS_REG 0xdb >> #define PUNIT_GPU_STATUS_MAX_FREQ_SHIFT 16 >> #define PUNIT_GPU_STATUS_MAX_FREQ_MASK 0xff >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index 2acb3de..71b8e2f 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -4346,11 +4346,29 @@ void gen6_update_ring_freq(struct drm_device *dev) >> >> static int cherryview_rps_max_freq(struct drm_i915_private *dev_priv) >> { >> + struct drm_device *dev = dev_priv->dev; >> + struct intel_device_info *info; >> u32 val, rp0; >> >> - val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG); >> - rp0 = (val >> PUNIT_GPU_STATUS_MAX_FREQ_SHIFT) & PUNIT_GPU_STATUS_MAX_FREQ_MASK; >> - >> + info = (struct intel_device_info *)&dev_priv->info; > > Pointless cast. Also the assignment could be done when declaring info, > and we usually use INTEL_INFO() to get at it. > >> + >> + if (dev->pdev->revision >= 0x20) { > > Do we really need this check? I would think it would be up to the > Punit firmware version rather the stepping. My BSW has PCI rev 0x15, > but with the latest BIOS both fuse registers already contain correct > looking information. > >> + val = vlv_punit_read(dev_priv, FB_GFX_FMAX_AT_VMAX_FUSE); >> + >> + if (info->eu_total == 8) /* (2 * 4) config */ >> + rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS4EU_FUSE_SHIFT); >> + else if (info->eu_total == 12) /* (2 * 6) config */ >> + rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS6EU_FUSE_SHIFT); >> + else if (info->eu_total == 16) /* (2 * 8) config */ >> + rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS8EU_FUSE_SHIFT); >> + else /* Setting (2 * 8) Min RP0 for any other combination */ >> + rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS8EU_FUSE_SHIFT); > > 'switch (INTEL_INFO(dev)->eu_total)' perhaps? > >> + rp0 = (rp0 & FB_GFX_FMAX_AT_VMAX_FUSE_MASK); >> + } else { /* For pre-production hardware */ >> + val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG); >> + rp0 = (val >> PUNIT_GPU_STATUS_MAX_FREQ_SHIFT) & >> + PUNIT_GPU_STATUS_MAX_FREQ_MASK; >> + } >> return rp0; >> } >> >> @@ -4366,20 +4384,40 @@ static int cherryview_rps_rpe_freq(struct drm_i915_private *dev_priv) >> >> static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv) >> { >> + struct drm_device *dev = dev_priv->dev; >> + struct intel_device_info *info; >> u32 val, rp1; >> >> - val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS); >> - rp1 = (val >> PUNIT_GPU_STATUS_MAX_FREQ_SHIFT) & PUNIT_GPU_STATUS_MAX_FREQ_MASK; >> + info = (struct intel_device_info *)&dev_priv->info; > > Unused here. > >> >> + if (dev->pdev->revision >= 0x20) { > >> + val = vlv_punit_read(dev_priv, FB_GFX_FMAX_AT_VMAX_FUSE); >> + rp1 = (val & FB_GFX_GUAR_FREQ_FUSE_MASK); >> + } else { /* For pre-production hardware */ >> + val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS); >> + rp1 = ((val >> PUNIT_GPU_STATUS_MAX_FREQ_SHIFT) & >> + PUNIT_GPU_STATUS_MAX_FREQ_MASK); >> + } >> return rp1; >> } >> >> static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv) >> { >> + struct drm_device *dev = dev_priv->dev; >> + struct intel_device_info *info; >> u32 val, rpn; >> + info = (struct intel_device_info *)&dev_priv->info; > > Unused. > >> + >> + if (dev->pdev->revision >= 0x20) { >> + val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE); >> + rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) & >> + FB_GFX_FMIN_AT_VMIN_FUSE_MASK); >> + } else { /* For pre-production hardware */ >> + val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG); >> + rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) & >> + PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK); >> + } >> >> - val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG); >> - rpn = (val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) & PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK; >> return rpn; >> } >> >> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c >> index 01d841e..3c42eef 100644 >> --- a/drivers/gpu/drm/i915/intel_sideband.c >> +++ b/drivers/gpu/drm/i915/intel_sideband.c >> @@ -75,7 +75,7 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn, >> return 0; >> } >> >> -u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr) >> +u32 vlv_punit_read(struct drm_i915_private *dev_priv, u32 addr) > > Good thing you had to pass a constant. Otherwise this could have caused > quite a bit of head scratching for you ;) This is the kind of thing that really should be a separate fix. If, for any reason, we need to revert the rest, we'll still want this. BR, Jani. > >> { >> u32 val = 0; >> >> @@ -89,7 +89,7 @@ u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr) >> return val; >> } >> >> -void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val) >> +void vlv_punit_write(struct drm_i915_private *dev_priv, u32 addr, u32 val) >> { >> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock)); >> >> -- >> 1.9.1 > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Monday 15 December 2014 12:21 PM, Jani Nikula wrote: > On Fri, 12 Dec 2014, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >> On Fri, Dec 12, 2014 at 02:18:15PM +0530, deepak.s@linux.intel.com wrote: >>> From: Deepak S <deepak.s@linux.intel.com> >>> >>> Use new Sideband offset to read max/min/gaur freq based on the SKU it >>> is running on. Based on the Number of EU, we read different bits to >>> identify the max frequencies at which system can run. >>> >>> Signed-off-by: Deepak S <deepak.s@linux.intel.com> >>> --- >>> drivers/gpu/drm/i915/i915_drv.h | 4 +-- >>> drivers/gpu/drm/i915/i915_reg.h | 12 ++++++++ >>> drivers/gpu/drm/i915/intel_pm.c | 52 ++++++++++++++++++++++++++++++----- >>> drivers/gpu/drm/i915/intel_sideband.c | 4 +-- >>> 4 files changed, 61 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >>> index b58bad4..0690dff 100644 >>> --- a/drivers/gpu/drm/i915/i915_drv.h >>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>> @@ -3016,8 +3016,8 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u32 mbox, u32 *val >>> int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u32 mbox, u32 val); >>> >>> /* intel_sideband.c */ >>> -u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr); >>> -void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val); >>> +u32 vlv_punit_read(struct drm_i915_private *dev_priv, u32 addr); >>> +void vlv_punit_write(struct drm_i915_private *dev_priv, u32 addr, u32 val); >>> u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr); >>> u32 vlv_gpio_nc_read(struct drm_i915_private *dev_priv, u32 reg); >>> void vlv_gpio_nc_write(struct drm_i915_private *dev_priv, u32 reg, u32 val); >>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >>> index b57cba3..f41290c 100644 >>> --- a/drivers/gpu/drm/i915/i915_reg.h >>> +++ b/drivers/gpu/drm/i915/i915_reg.h >>> @@ -602,6 +602,18 @@ enum punit_power_well { >>> #define PUNIT_FUSE_BUS2 0xf6 /* bits 47:40 */ >>> #define PUNIT_FUSE_BUS1 0xf5 /* bits 55:48 */ >>> >>> +#define FB_GFX_FMAX_AT_VMAX_FUSE 0x136 >>> +#define FB_GFX_FMAX_AT_VMAX_FUSE_MASK 0xff >>> +#define FB_GFX_FMAX_AT_VMAX_2SS4EU_FUSE_SHIFT 24 >>> +#define FB_GFX_FMAX_AT_VMAX_2SS6EU_FUSE_SHIFT 16 >>> +#define FB_GFX_FMAX_AT_VMAX_2SS8EU_FUSE_SHIFT 8 >>> + >> This blank line makes me think FB_GFX_GUAR_FREQ_FUSE_MASK isn't part of >> this register. So best not leave such blank line here. >> >> I have 0x3c3c3c28 in this register, which matches what I get using the >> old method. >> >>> +#define FB_GFX_GUAR_FREQ_FUSE_MASK 0xff >>> + >>> +#define FB_GFX_FMIN_AT_VMIN_FUSE 0x137 >>> +#define FB_GFX_FMIN_AT_VMIN_FUSE_MASK 0xff >>> +#define FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT 8 >> I have 0x69841428 here. The low 8 bits look like another freq value. >> What is it? >> >> I have no docs for this stuff so can't really review apart from looking >> at what my hardware reports. >> >> >> Actually, since all the values are 8 bits maybe it would be neater to >> just >> #define FB_GFX_FREQ_FUSE_MASK 0xff >> and use that everywhere instead of having three different definitions >> for the same 0xff value. >> >>> + >>> #define PUNIT_GPU_STATUS_REG 0xdb >>> #define PUNIT_GPU_STATUS_MAX_FREQ_SHIFT 16 >>> #define PUNIT_GPU_STATUS_MAX_FREQ_MASK 0xff >>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >>> index 2acb3de..71b8e2f 100644 >>> --- a/drivers/gpu/drm/i915/intel_pm.c >>> +++ b/drivers/gpu/drm/i915/intel_pm.c >>> @@ -4346,11 +4346,29 @@ void gen6_update_ring_freq(struct drm_device *dev) >>> >>> static int cherryview_rps_max_freq(struct drm_i915_private *dev_priv) >>> { >>> + struct drm_device *dev = dev_priv->dev; >>> + struct intel_device_info *info; >>> u32 val, rp0; >>> >>> - val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG); >>> - rp0 = (val >> PUNIT_GPU_STATUS_MAX_FREQ_SHIFT) & PUNIT_GPU_STATUS_MAX_FREQ_MASK; >>> - >>> + info = (struct intel_device_info *)&dev_priv->info; >> Pointless cast. Also the assignment could be done when declaring info, >> and we usually use INTEL_INFO() to get at it. >> >>> + >>> + if (dev->pdev->revision >= 0x20) { >> Do we really need this check? I would think it would be up to the >> Punit firmware version rather the stepping. My BSW has PCI rev 0x15, >> but with the latest BIOS both fuse registers already contain correct >> looking information. >> >>> + val = vlv_punit_read(dev_priv, FB_GFX_FMAX_AT_VMAX_FUSE); >>> + >>> + if (info->eu_total == 8) /* (2 * 4) config */ >>> + rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS4EU_FUSE_SHIFT); >>> + else if (info->eu_total == 12) /* (2 * 6) config */ >>> + rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS6EU_FUSE_SHIFT); >>> + else if (info->eu_total == 16) /* (2 * 8) config */ >>> + rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS8EU_FUSE_SHIFT); >>> + else /* Setting (2 * 8) Min RP0 for any other combination */ >>> + rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS8EU_FUSE_SHIFT); >> 'switch (INTEL_INFO(dev)->eu_total)' perhaps? >> >>> + rp0 = (rp0 & FB_GFX_FMAX_AT_VMAX_FUSE_MASK); >>> + } else { /* For pre-production hardware */ >>> + val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG); >>> + rp0 = (val >> PUNIT_GPU_STATUS_MAX_FREQ_SHIFT) & >>> + PUNIT_GPU_STATUS_MAX_FREQ_MASK; >>> + } >>> return rp0; >>> } >>> >>> @@ -4366,20 +4384,40 @@ static int cherryview_rps_rpe_freq(struct drm_i915_private *dev_priv) >>> >>> static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv) >>> { >>> + struct drm_device *dev = dev_priv->dev; >>> + struct intel_device_info *info; >>> u32 val, rp1; >>> >>> - val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS); >>> - rp1 = (val >> PUNIT_GPU_STATUS_MAX_FREQ_SHIFT) & PUNIT_GPU_STATUS_MAX_FREQ_MASK; >>> + info = (struct intel_device_info *)&dev_priv->info; >> Unused here. >> >>> >>> + if (dev->pdev->revision >= 0x20) { >>> + val = vlv_punit_read(dev_priv, FB_GFX_FMAX_AT_VMAX_FUSE); >>> + rp1 = (val & FB_GFX_GUAR_FREQ_FUSE_MASK); >>> + } else { /* For pre-production hardware */ >>> + val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS); >>> + rp1 = ((val >> PUNIT_GPU_STATUS_MAX_FREQ_SHIFT) & >>> + PUNIT_GPU_STATUS_MAX_FREQ_MASK); >>> + } >>> return rp1; >>> } >>> >>> static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv) >>> { >>> + struct drm_device *dev = dev_priv->dev; >>> + struct intel_device_info *info; >>> u32 val, rpn; >>> + info = (struct intel_device_info *)&dev_priv->info; >> Unused. >> >>> + >>> + if (dev->pdev->revision >= 0x20) { >>> + val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE); >>> + rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) & >>> + FB_GFX_FMIN_AT_VMIN_FUSE_MASK); >>> + } else { /* For pre-production hardware */ >>> + val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG); >>> + rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) & >>> + PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK); >>> + } >>> >>> - val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG); >>> - rpn = (val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) & PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK; >>> return rpn; >>> } >>> >>> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c >>> index 01d841e..3c42eef 100644 >>> --- a/drivers/gpu/drm/i915/intel_sideband.c >>> +++ b/drivers/gpu/drm/i915/intel_sideband.c >>> @@ -75,7 +75,7 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn, >>> return 0; >>> } >>> >>> -u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr) >>> +u32 vlv_punit_read(struct drm_i915_private *dev_priv, u32 addr) >> Good thing you had to pass a constant. Otherwise this could have caused >> quite a bit of head scratching for you ;) > This is the kind of thing that really should be a separate fix. If, for > any reason, we need to revert the rest, we'll still want this. > > BR, > Jani. Agreed, Better to have a separate patch :) >>> { >>> u32 val = 0; >>> >>> @@ -89,7 +89,7 @@ u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr) >>> return val; >>> } >>> >>> -void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val) >>> +void vlv_punit_write(struct drm_i915_private *dev_priv, u32 addr, u32 val) >>> { >>> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock)); >>> >>> -- >>> 1.9.1 >> -- >> Ville Syrjälä >> Intel OTC >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
From: Deepak S <deepak.s@linux.intel.com>
CHV/BSW production system has new turbo offset to read different freq.
This series adds the support.
Deepak S (3):
drm/i915/chv: Populate total EU count on Cherryview
drm/i915: Increase the range of sideband address.
drm/i915: New offset for reading frequencies on CHV.
drivers/gpu/drm/i915/i915_dma.c | 11 ++++++++
drivers/gpu/drm/i915/i915_drv.h | 5 ++--
drivers/gpu/drm/i915/i915_reg.h | 20 +++++++++++++
drivers/gpu/drm/i915/intel_pm.c | 53 +++++++++++++++++++++++++++++------
drivers/gpu/drm/i915/intel_sideband.c | 4 +--
5 files changed, 81 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b58bad4..0690dff 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3016,8 +3016,8 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u32 mbox, u32 *val int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u32 mbox, u32 val); /* intel_sideband.c */ -u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr); -void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val); +u32 vlv_punit_read(struct drm_i915_private *dev_priv, u32 addr); +void vlv_punit_write(struct drm_i915_private *dev_priv, u32 addr, u32 val); u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr); u32 vlv_gpio_nc_read(struct drm_i915_private *dev_priv, u32 reg); void vlv_gpio_nc_write(struct drm_i915_private *dev_priv, u32 reg, u32 val); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index b57cba3..f41290c 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -602,6 +602,18 @@ enum punit_power_well { #define PUNIT_FUSE_BUS2 0xf6 /* bits 47:40 */ #define PUNIT_FUSE_BUS1 0xf5 /* bits 55:48 */ +#define FB_GFX_FMAX_AT_VMAX_FUSE 0x136 +#define FB_GFX_FMAX_AT_VMAX_FUSE_MASK 0xff +#define FB_GFX_FMAX_AT_VMAX_2SS4EU_FUSE_SHIFT 24 +#define FB_GFX_FMAX_AT_VMAX_2SS6EU_FUSE_SHIFT 16 +#define FB_GFX_FMAX_AT_VMAX_2SS8EU_FUSE_SHIFT 8 + +#define FB_GFX_GUAR_FREQ_FUSE_MASK 0xff + +#define FB_GFX_FMIN_AT_VMIN_FUSE 0x137 +#define FB_GFX_FMIN_AT_VMIN_FUSE_MASK 0xff +#define FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT 8 + #define PUNIT_GPU_STATUS_REG 0xdb #define PUNIT_GPU_STATUS_MAX_FREQ_SHIFT 16 #define PUNIT_GPU_STATUS_MAX_FREQ_MASK 0xff diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 2acb3de..71b8e2f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4346,11 +4346,29 @@ void gen6_update_ring_freq(struct drm_device *dev) static int cherryview_rps_max_freq(struct drm_i915_private *dev_priv) { + struct drm_device *dev = dev_priv->dev; + struct intel_device_info *info; u32 val, rp0; - val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG); - rp0 = (val >> PUNIT_GPU_STATUS_MAX_FREQ_SHIFT) & PUNIT_GPU_STATUS_MAX_FREQ_MASK; - + info = (struct intel_device_info *)&dev_priv->info; + + if (dev->pdev->revision >= 0x20) { + val = vlv_punit_read(dev_priv, FB_GFX_FMAX_AT_VMAX_FUSE); + + if (info->eu_total == 8) /* (2 * 4) config */ + rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS4EU_FUSE_SHIFT); + else if (info->eu_total == 12) /* (2 * 6) config */ + rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS6EU_FUSE_SHIFT); + else if (info->eu_total == 16) /* (2 * 8) config */ + rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS8EU_FUSE_SHIFT); + else /* Setting (2 * 8) Min RP0 for any other combination */ + rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS8EU_FUSE_SHIFT); + rp0 = (rp0 & FB_GFX_FMAX_AT_VMAX_FUSE_MASK); + } else { /* For pre-production hardware */ + val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG); + rp0 = (val >> PUNIT_GPU_STATUS_MAX_FREQ_SHIFT) & + PUNIT_GPU_STATUS_MAX_FREQ_MASK; + } return rp0; } @@ -4366,20 +4384,40 @@ static int cherryview_rps_rpe_freq(struct drm_i915_private *dev_priv) static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv) { + struct drm_device *dev = dev_priv->dev; + struct intel_device_info *info; u32 val, rp1; - val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS); - rp1 = (val >> PUNIT_GPU_STATUS_MAX_FREQ_SHIFT) & PUNIT_GPU_STATUS_MAX_FREQ_MASK; + info = (struct intel_device_info *)&dev_priv->info; + if (dev->pdev->revision >= 0x20) { + val = vlv_punit_read(dev_priv, FB_GFX_FMAX_AT_VMAX_FUSE); + rp1 = (val & FB_GFX_GUAR_FREQ_FUSE_MASK); + } else { /* For pre-production hardware */ + val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS); + rp1 = ((val >> PUNIT_GPU_STATUS_MAX_FREQ_SHIFT) & + PUNIT_GPU_STATUS_MAX_FREQ_MASK); + } return rp1; } static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv) { + struct drm_device *dev = dev_priv->dev; + struct intel_device_info *info; u32 val, rpn; + info = (struct intel_device_info *)&dev_priv->info; + + if (dev->pdev->revision >= 0x20) { + val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE); + rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) & + FB_GFX_FMIN_AT_VMIN_FUSE_MASK); + } else { /* For pre-production hardware */ + val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG); + rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) & + PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK); + } - val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG); - rpn = (val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) & PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK; return rpn; } diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c index 01d841e..3c42eef 100644 --- a/drivers/gpu/drm/i915/intel_sideband.c +++ b/drivers/gpu/drm/i915/intel_sideband.c @@ -75,7 +75,7 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn, return 0; } -u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr) +u32 vlv_punit_read(struct drm_i915_private *dev_priv, u32 addr) { u32 val = 0; @@ -89,7 +89,7 @@ u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr) return val; } -void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val) +void vlv_punit_write(struct drm_i915_private *dev_priv, u32 addr, u32 val) { WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));