| Message ID | 1430825455-28382-2-git-send-email-akash.goel@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6320
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 276/276 276/276
ILK -1 302/302 301/302
SNB 316/316 316/316
IVB 342/342 342/342
BYT 286/286 286/286
BDW 318/318 318/318
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*ILK igt@kms_flip@flip-vs-dpms-interruptible PASS(2) DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:intel_pch_fifo_underrun_irq_handler[i915]]*ERROR*PCH_transcoder_A_FIFO_underrun@PCH transcoder A FIFO underrun
Note: You need to pay more attention to line start with '*'
On Tue, May 5, 2015 at 4:30 AM, <akash.goel@intel.com> wrote: > From: Akash Goel <akash.goel@intel.com> > > Ring frequency table programming changes for SKL. No need for a > floor on ring frequency, as the issue of performance impact with > ring running below DDR frequency, is believed to be fixed on SKL > > Issue: VIZ-5144 > Signed-off-by: Akash Goel <akash.goel@intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 26 +++++++++++++++++++++----- > 1 file changed, 21 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 421b78d..d1bdea7 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4582,6 +4582,7 @@ static void __gen6_update_ring_freq(struct drm_device *dev) > int min_freq = 15; > unsigned int gpu_freq; > unsigned int max_ia_freq, min_ring_freq; > + unsigned int max_gpu_freq, min_gpu_freq; > int scaling_factor = 180; > struct cpufreq_policy *policy; > > @@ -4606,17 +4607,31 @@ static void __gen6_update_ring_freq(struct drm_device *dev) > /* convert DDR frequency from units of 266.6MHz to bandwidth */ > min_ring_freq = mult_frac(min_ring_freq, 8, 3); > > + if (IS_SKYLAKE(dev)) { > + /* Convert GT frequency to 50 HZ units */ > + min_gpu_freq = dev_priv->rps.min_freq / GEN9_FREQ_SCALER; > + max_gpu_freq = dev_priv->rps.max_freq / GEN9_FREQ_SCALER; I got even more confused here with the scale differences... Doesn't look so consistent.. > + } else { > + min_gpu_freq = dev_priv->rps.min_freq; > + max_gpu_freq = dev_priv->rps.max_freq; > + } > + > /* > * For each potential GPU frequency, load a ring frequency we'd like > * to use for memory access. We do this by specifying the IA frequency > * the PCU should use as a reference to determine the ring frequency. > */ > - for (gpu_freq = dev_priv->rps.max_freq; gpu_freq >= dev_priv->rps.min_freq; > - gpu_freq--) { > - int diff = dev_priv->rps.max_freq - gpu_freq; > + for (gpu_freq = max_gpu_freq; gpu_freq >= min_gpu_freq; gpu_freq--) { > + int diff = max_gpu_freq - gpu_freq; > unsigned int ia_freq = 0, ring_freq = 0; > > - if (INTEL_INFO(dev)->gen >= 8) { > + if (IS_SKYLAKE(dev)) { > + /* > + * ring_freq = 2 * GT. ring_freq is in 100MHz units > + * No floor required for ring frequency on SKL. > + */ > + ring_freq = gpu_freq; Aren't we using the 16.6 now? Shouldn't it be converted anyway? > + } else if (INTEL_INFO(dev)->gen >= 8) { > /* max(2 * GT, DDR). NB: GT is 50MHz units */ > ring_freq = max(min_ring_freq, gpu_freq); > } else if (IS_HASWELL(dev)) { > @@ -5770,7 +5785,8 @@ static void intel_gen6_powersave_work(struct work_struct *work) > } else if (INTEL_INFO(dev)->gen >= 9) { > gen9_enable_rc6(dev); > gen9_enable_rps(dev); > - __gen6_update_ring_freq(dev); > + if (IS_SKYLAKE(dev)) why this if here? If it is a bxt restriction shouldn't it be in a separated patch? > + __gen6_update_ring_freq(dev); > } else if (IS_BROADWELL(dev)) { > gen8_enable_rps(dev); > __gen6_update_ring_freq(dev); > -- > 1.9.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, 2015-06-03 at 14:24 -0700, Rodrigo Vivi wrote: > On Tue, May 5, 2015 at 4:30 AM, <akash.goel@intel.com> wrote: > > From: Akash Goel <akash.goel@intel.com> > > > > Ring frequency table programming changes for SKL. No need for a > > floor on ring frequency, as the issue of performance impact with > > ring running below DDR frequency, is believed to be fixed on SKL > > > > Issue: VIZ-5144 > > Signed-off-by: Akash Goel <akash.goel@intel.com> > > --- > > drivers/gpu/drm/i915/intel_pm.c | 26 +++++++++++++++++++++----- > > 1 file changed, 21 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index 421b78d..d1bdea7 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -4582,6 +4582,7 @@ static void __gen6_update_ring_freq(struct drm_device *dev) > > int min_freq = 15; > > unsigned int gpu_freq; > > unsigned int max_ia_freq, min_ring_freq; > > + unsigned int max_gpu_freq, min_gpu_freq; > > int scaling_factor = 180; > > struct cpufreq_policy *policy; > > > > @@ -4606,17 +4607,31 @@ static void __gen6_update_ring_freq(struct drm_device *dev) > > /* convert DDR frequency from units of 266.6MHz to bandwidth */ > > min_ring_freq = mult_frac(min_ring_freq, 8, 3); > > > > + if (IS_SKYLAKE(dev)) { > > + /* Convert GT frequency to 50 HZ units */ > > + min_gpu_freq = dev_priv->rps.min_freq / GEN9_FREQ_SCALER; > > + max_gpu_freq = dev_priv->rps.max_freq / GEN9_FREQ_SCALER; > > I got even more confused here with the scale differences... Doesn't > look so consistent.. For Ring frequency table programming, as an input GT frequency has to be specified in units of 50 MHZ only. For SKL natural hardware unit is 16.667 MHZ and Driver is internally storing in the same unit only, hence an extra conversion required here. > > > + } else { > > + min_gpu_freq = dev_priv->rps.min_freq; > > + max_gpu_freq = dev_priv->rps.max_freq; > > + } > > + > > /* > > * For each potential GPU frequency, load a ring frequency we'd like > > * to use for memory access. We do this by specifying the IA frequency > > * the PCU should use as a reference to determine the ring frequency. > > */ > > - for (gpu_freq = dev_priv->rps.max_freq; gpu_freq >= dev_priv->rps.min_freq; > > - gpu_freq--) { > > - int diff = dev_priv->rps.max_freq - gpu_freq; > > + for (gpu_freq = max_gpu_freq; gpu_freq >= min_gpu_freq; gpu_freq--) { > > + int diff = max_gpu_freq - gpu_freq; > > unsigned int ia_freq = 0, ring_freq = 0; > > > > - if (INTEL_INFO(dev)->gen >= 8) { > > + if (IS_SKYLAKE(dev)) { > > + /* > > + * ring_freq = 2 * GT. ring_freq is in 100MHz units > > + * No floor required for ring frequency on SKL. > > + */ > > + ring_freq = gpu_freq; > > Aren't we using the 16.6 now? Shouldn't it be converted anyway? > > > + } else if (INTEL_INFO(dev)->gen >= 8) { > > /* max(2 * GT, DDR). NB: GT is 50MHz units */ > > ring_freq = max(min_ring_freq, gpu_freq); > > } else if (IS_HASWELL(dev)) { > > @@ -5770,7 +5785,8 @@ static void intel_gen6_powersave_work(struct work_struct *work) > > } else if (INTEL_INFO(dev)->gen >= 9) { > > gen9_enable_rc6(dev); > > gen9_enable_rps(dev); > > - __gen6_update_ring_freq(dev); > > + if (IS_SKYLAKE(dev)) > why this if here? > > If it is a bxt restriction shouldn't it be in a separated patch? Yes for BXT, its not supported. Would move it to separate patch. > > > + __gen6_update_ring_freq(dev); > > } else if (IS_BROADWELL(dev)) { > > gen8_enable_rps(dev); > > __gen6_update_ring_freq(dev); > > -- > > 1.9.2 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > >
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 421b78d..d1bdea7 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4582,6 +4582,7 @@ static void __gen6_update_ring_freq(struct drm_device *dev) int min_freq = 15; unsigned int gpu_freq; unsigned int max_ia_freq, min_ring_freq; + unsigned int max_gpu_freq, min_gpu_freq; int scaling_factor = 180; struct cpufreq_policy *policy; @@ -4606,17 +4607,31 @@ static void __gen6_update_ring_freq(struct drm_device *dev) /* convert DDR frequency from units of 266.6MHz to bandwidth */ min_ring_freq = mult_frac(min_ring_freq, 8, 3); + if (IS_SKYLAKE(dev)) { + /* Convert GT frequency to 50 HZ units */ + min_gpu_freq = dev_priv->rps.min_freq / GEN9_FREQ_SCALER; + max_gpu_freq = dev_priv->rps.max_freq / GEN9_FREQ_SCALER; + } else { + min_gpu_freq = dev_priv->rps.min_freq; + max_gpu_freq = dev_priv->rps.max_freq; + } + /* * For each potential GPU frequency, load a ring frequency we'd like * to use for memory access. We do this by specifying the IA frequency * the PCU should use as a reference to determine the ring frequency. */ - for (gpu_freq = dev_priv->rps.max_freq; gpu_freq >= dev_priv->rps.min_freq; - gpu_freq--) { - int diff = dev_priv->rps.max_freq - gpu_freq; + for (gpu_freq = max_gpu_freq; gpu_freq >= min_gpu_freq; gpu_freq--) { + int diff = max_gpu_freq - gpu_freq; unsigned int ia_freq = 0, ring_freq = 0; - if (INTEL_INFO(dev)->gen >= 8) { + if (IS_SKYLAKE(dev)) { + /* + * ring_freq = 2 * GT. ring_freq is in 100MHz units + * No floor required for ring frequency on SKL. + */ + ring_freq = gpu_freq; + } else if (INTEL_INFO(dev)->gen >= 8) { /* max(2 * GT, DDR). NB: GT is 50MHz units */ ring_freq = max(min_ring_freq, gpu_freq); } else if (IS_HASWELL(dev)) { @@ -5770,7 +5785,8 @@ static void intel_gen6_powersave_work(struct work_struct *work) } else if (INTEL_INFO(dev)->gen >= 9) { gen9_enable_rc6(dev); gen9_enable_rps(dev); - __gen6_update_ring_freq(dev); + if (IS_SKYLAKE(dev)) + __gen6_update_ring_freq(dev); } else if (IS_BROADWELL(dev)) { gen8_enable_rps(dev); __gen6_update_ring_freq(dev);