[RFC] drm/i915: Add a new modparam for customized ring multiplier

[RFC] drm/i915: Add a new modparam for customized ring multiplier

[Jackie Li]

From: Zhipeng Gong <zhipeng.gong@intel.com>

SKL platforms requires a higher ring multiplier when there's massive
GPU load. Current driver doesn't provide a way to override the ring
multiplier.

This patch adds a new module parameter to allow the overriding of
ring multiplier for Gen9 platforms.

Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
Cc: Zhipeng Gong <zhipeng.gong@intel.com>
Signed-off-by: Jackie Li <yaodong.li@intel.com>
---
 drivers/gpu/drm/i915/i915_params.c |  4 +++
 drivers/gpu/drm/i915/i915_params.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c    | 57 ++++++++++++++++++++++++++++++++------
 3 files changed, 53 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 8dfea03..f3dd179 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -101,6 +101,10 @@ i915_param_named_unsafe(disable_power_well, int, 0400,
 	"Disable display power wells when possible "
 	"(-1=auto [default], 0=power wells always on, 1=power wells disabled when possible)");
 
+i915_param_named_unsafe(ring_multiplier, uint, 0400,
+	"Override Ring/GT frequency multiplier."
+	"(2=2x ring multiplier [defaut], 3=3x ring multiplier)");
+
 i915_param_named_unsafe(enable_ips, int, 0600, "Enable IPS (default: true)");
 
 i915_param_named(fastboot, bool, 0600,
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 792ce26..6073d1c 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -43,6 +43,7 @@
 	param(int, enable_ppgtt, -1) \
 	param(int, enable_psr, -1) \
 	param(int, disable_power_well, -1) \
+	param(unsigned int, ring_multiplier, 2) \
 	param(int, enable_ips, 1) \
 	param(int, invert_brightness, 0) \
 	param(int, enable_guc, 0) \
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a349c4f..080051b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6818,6 +6818,34 @@ static void gen6_enable_rps(struct drm_i915_private *dev_priv)
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 }
 
+static inline void sanitize_ring_multiplier(struct drm_i915_private *i915)
+{
+	unsigned int m = i915_modparams.ring_multiplier;
+
+	/* Currently, only support 2x or 3x multipliers */
+	if (m != 2 && m != 3)
+		i915_modparams.ring_multiplier = 2;
+}
+
+static inline void get_ring_multiplier(struct drm_i915_private *i915,
+				      unsigned int *numer,
+				      unsigned int *denom)
+{
+	sanitize_ring_multiplier(i915);
+
+	if (IS_GEN9(i915)) {
+		*numer = i915_modparams.ring_multiplier;
+		*denom = 1;
+	} else if IS_HASWELL(i915) {
+		*numer = 5;
+		*denom = 2;
+	} else {
+		/* Use a 2x ring multiplier by default */
+		*numer = 2;
+		*denom = 1;
+	}
+}
+
 static void gen6_update_ring_freq(struct drm_i915_private *dev_priv)
 {
 	struct intel_rps *rps = &dev_priv->gt_pm.rps;
@@ -6825,6 +6853,8 @@ static void gen6_update_ring_freq(struct drm_i915_private *dev_priv)
 	unsigned int gpu_freq;
 	unsigned int max_ia_freq, min_ring_freq;
 	unsigned int max_gpu_freq, min_gpu_freq;
+	unsigned int ring_mul_numer, ring_mul_denom;
+	unsigned int ring_request_freq;
 	int scaling_factor = 180;
 	struct cpufreq_policy *policy;
 
@@ -6858,6 +6888,9 @@ static void gen6_update_ring_freq(struct drm_i915_private *dev_priv)
 		max_gpu_freq = rps->max_freq;
 	}
 
+
+	get_ring_multiplier(dev_priv, &ring_mul_numer, &ring_mul_denom);
+
 	/*
 	 * 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
@@ -6867,18 +6900,24 @@ static void gen6_update_ring_freq(struct drm_i915_private *dev_priv)
 		int diff = max_gpu_freq - gpu_freq;
 		unsigned int ia_freq = 0, ring_freq = 0;
 
+		/*
+		 * ring_request_freq = ring_multiplier * GPU frequency.
+		 * Ring freq is in 100MHz units while GPU frequency is in 50MHz
+		 * units.
+		 */
+		ring_request_freq = mult_frac(gpu_freq, ring_mul_numer,
+					      2 * ring_mul_denom);
+
 		if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv)) {
 			/*
-			 * ring_freq = 2 * GT. ring_freq is in 100MHz units
-			 * No floor required for ring frequency on SKL.
+			 * ring_freq = ring request freq. No floor required for
+			 * ring frequency on SKL.
 			 */
-			ring_freq = gpu_freq;
-		} else if (INTEL_INFO(dev_priv)->gen >= 8) {
-			/* max(2 * GT, DDR). NB: GT is 50MHz units */
-			ring_freq = max(min_ring_freq, gpu_freq);
-		} else if (IS_HASWELL(dev_priv)) {
-			ring_freq = mult_frac(gpu_freq, 5, 4);
-			ring_freq = max(min_ring_freq, ring_freq);
+			ring_freq = ring_request_freq;
+		} else if (INTEL_INFO(dev_priv)->gen >= 8 ||
+			IS_HASWELL(dev_priv)) {
+			/* max(ring request freq, DDR). NB: GT is 50MHz units */
+			ring_freq = max(min_ring_freq, ring_request_freq);
 			/* leave ia_freq as the default, chosen by cpufreq */
 		} else {
 			/* On older processors, there is no separate ring
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Add a new modparam for customized ring multiplier

[Patchwork]

== Series Details ==

Series: drm/i915: Add a new modparam for customized ring multiplier
URL   : https://patchwork.freedesktop.org/series/35535/
State : success

== Summary ==

Series 35535v1 drm/i915: Add a new modparam for customized ring multiplier
https://patchwork.freedesktop.org/api/1.0/series/35535/revisions/1/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-kbl-r) fdo#104172

fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#104172 https://bugs.freedesktop.org/show_bug.cgi?id=104172

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:433s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:439s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:377s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:498s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:276s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:493s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:493s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:484s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:471s
fi-elk-e7500     total:224  pass:163  dwarn:15  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:178  dwarn:1   dfail:0   fail:1   skip:108 time:261s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:529s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:406s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:414s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:387s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:475s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:425s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:481s
fi-kbl-7560u     total:288  pass:268  dwarn:1   dfail:0   fail:0   skip:19  time:527s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:462s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:526s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:575s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:445s
fi-skl-6600u     total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:532s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:552s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:505s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:501s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:446s
fi-snb-2520m     total:3    pass:2    dwarn:0   dfail:0   fail:0   skip:0  
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:405s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:585s
fi-cnl-y         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:25 
fi-glk-dsi       total:54   pass:46   dwarn:0   dfail:0   fail:0   skip:7  

dcb649c7cee0af1ed444d25c2230f18fa4bad49e drm-tip: 2017y-12m-18d-16h-15m-16s UTC integration manifest
fe7a55adb46a drm/i915: Add a new modparam for customized ring multiplier

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7530/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC] drm/i915: Add a new modparam for customized ring multiplier

[Chris Wilson]

Quoting Jackie Li (2017-12-18 21:22:08)
> From: Zhipeng Gong <zhipeng.gong@intel.com>
> 
> SKL platforms requires a higher ring multiplier when there's massive
> GPU load. Current driver doesn't provide a way to override the ring
> multiplier.
> 
> This patch adds a new module parameter to allow the overriding of
> ring multiplier for Gen9 platforms.

So the default ring-scaling is not good enough, the first thing we do is
to try and ensure the defaults work for nearly all use cases. My
impression is that you want a nonlinear scalefactor, low power workloads
don't try and ramp up the ring frequencies as aggressively, high power
workloads try hard for higher frequencies, and then get throttled back
harder as well. How well can we autotune it? What events tells us if the
ratio is too high or too low?

Adding an untested unsafe modparam is a big no. If you want us to
support such a parameter, we at least need it not to taint the kernel
and come with some tests that prove it functions as intended. Better
still might be a I915_SETPARAM (or hooking up the write func of the
modparam) to support changing the value on the fly.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC] drm/i915: Add a new modparam for customized ring multiplier

[Yaodong Li]

On 12/18/2017 01:47 PM, Chris Wilson wrote:
> Quoting Jackie Li (2017-12-18 21:22:08)
>> From: Zhipeng Gong <zhipeng.gong@intel.com>
>>
>> SKL platforms requires a higher ring multiplier when there's massive
>> GPU load. Current driver doesn't provide a way to override the ring
>> multiplier.
>>
>> This patch adds a new module parameter to allow the overriding of
>> ring multiplier for Gen9 platforms.
> So the default ring-scaling is not good enough, the first thing we do is
> to try and ensure the defaults work for nearly all use cases. My
> impression is that you want a nonlinear scalefactor, low power workloads
> don't try and ramp up the ring frequencies as aggressively, high power
> workloads try hard for higher frequencies, and then get throttled back
> harder as well. How well can we autotune it? What events tells us if the
> ratio is too high or too low?
Thanks Chris! the background was that we found that the performance
would drop on some SKL platforms with the default ring scaling factor.
and use of a 3x ring scaler can solve this problem.

One of the trigger of this issue is when there's massive GPU workloads
while having little/negligible CPU load - currently, we have no way to 
monitor
GPU/CPU workload in our driver. This is the reason why we use a modparam.
Do you have any suggestion about this?

Zhipeng and Dmitry may add more insights on this issue and related tuning.
> Adding an untested unsafe modparam is a big no. If you want us to
> support such a parameter, we at least need it not to taint the kernel
> and come with some tests that prove it functions as intended. Better
> still might be a I915_SETPARAM (or hooking up the write func of the
> modparam) to support changing the value on the fly.
the intention of this patch was to add a way to override the
default 2x ring scaling factor so that we would be able choose
a different ring scaler for SKL platforms which were known that would
have massive GPU workload. And related changes (using a 3x scaler)
had been tested.

It would be great if we can do this on the fly.  However, we're going
to need a way to monitor the trigger events in order to change the
value dynamically.

By saying use I915_SETPARAM (or different write func), are you suggesting
to request the new ring freq directly while setting the new ring scaling 
factor?

Regards,
-Jackie
> -Chris

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC] drm/i915: Add a new modparam for customized ring multiplier

[Chris Wilson]

Quoting Chris Wilson (2017-12-18 21:47:25)
> Quoting Jackie Li (2017-12-18 21:22:08)
> > From: Zhipeng Gong <zhipeng.gong@intel.com>
> > 
> > SKL platforms requires a higher ring multiplier when there's massive
> > GPU load. Current driver doesn't provide a way to override the ring
> > multiplier.
> > 
> > This patch adds a new module parameter to allow the overriding of
> > ring multiplier for Gen9 platforms.
> 
> So the default ring-scaling is not good enough, the first thing we do is
> to try and ensure the defaults work for nearly all use cases. My
> impression is that you want a nonlinear scalefactor, low power workloads
> don't try and ramp up the ring frequencies as aggressively, high power
> workloads try hard for higher frequencies, and then get throttled back
> harder as well. How well can we autotune it? What events tells us if the
> ratio is too high or too low?

One thing that came to mind is that we don't know the min/max ring
frequencies and just program them blindly. Is it the case that at max
gpu freq, there is still headroom on the ring freq, or do you require a
steeper ramp so that you hit the max ringfreq earlier for your workload
(which then presumably can run at less than max gpufreq, so pushing
power elsewhere).
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC] drm/i915: Add a new modparam for customized ring multiplier

[Rogozhkin, Dmitry V]

Clarification on the issue. Consider that you have a massive load on GT and just tiny one on IA. If GT will program the RING frequency to be lower than IA frequency, then you will fall into the situation when RING frequency constantly transits from GT to IA level and back. Each transition of a RING frequency is a full system stall. If you will have "good" transition rate with few transitions per few milliseconds you will lose ~10% of performance. That's the case for media workloads when you easily can step into this since 1) media utilizes few GPU engines and with few parallel workloads you can make sure that at least 1 engine is _always_ doing something, 2) media BB are relatively small, so you have regular wakeups of the IA to manage requests. This will affect Gen9 platforms due to HW design change (we've spot this in SKL). This will not happen in Gen8 (old HW design). This will be fixed in Gen10+ (CNL+).

On SKL we ran into this with the GPU frequency pinned to 700MHz, CPU to 2GHz. Multipliers were x2 for GT, x1 for IA.

So, effectively, what we need to do is to make sure that RING frequency request from GT is _not_ below the request from IA. If IA requests 2GHz, we can't request 1.4GHz, we need request at least 2GHz. Multiplier patch was intended to do exactly that, but manually. Can  we somehow automate that managing IA frequency requests to the RING?

Dmitry.

-----Original Message-----
From: Chris Wilson [mailto:chris@chris-wilson.co.uk] 
Sent: Tuesday, December 26, 2017 6:36 AM
To: Li, Yaodong <yaodong.li@intel.com>; intel-gfx@lists.freedesktop.org
Cc: Gong, Zhipeng <zhipeng.gong@intel.com>; Widawsky, Benjamin <benjamin.widawsky@intel.com>; Mateo Lozano, Oscar <oscar.mateo@intel.com>; Kamble, Sagar A <sagar.a.kamble@intel.com>; Rogozhkin, Dmitry V <dmitry.v.rogozhkin@intel.com>; Li, Yaodong <yaodong.li@intel.com>
Subject: Re: [RFC] drm/i915: Add a new modparam for customized ring multiplier

Quoting Chris Wilson (2017-12-18 21:47:25)
> Quoting Jackie Li (2017-12-18 21:22:08)
> > From: Zhipeng Gong <zhipeng.gong@intel.com>
> > 
> > SKL platforms requires a higher ring multiplier when there's massive 
> > GPU load. Current driver doesn't provide a way to override the ring 
> > multiplier.
> > 
> > This patch adds a new module parameter to allow the overriding of 
> > ring multiplier for Gen9 platforms.
> 
> So the default ring-scaling is not good enough, the first thing we do 
> is to try and ensure the defaults work for nearly all use cases. My 
> impression is that you want a nonlinear scalefactor, low power 
> workloads don't try and ramp up the ring frequencies as aggressively, 
> high power workloads try hard for higher frequencies, and then get 
> throttled back harder as well. How well can we autotune it? What 
> events tells us if the ratio is too high or too low?

One thing that came to mind is that we don't know the min/max ring frequencies and just program them blindly. Is it the case that at max gpu freq, there is still headroom on the ring freq, or do you require a steeper ramp so that you hit the max ringfreq earlier for your workload (which then presumably can run at less than max gpufreq, so pushing power elsewhere).
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC] drm/i915: Add a new modparam for customized ring multiplier

[Chris Wilson]

Quoting Rogozhkin, Dmitry V (2017-12-26 16:39:23)
> Clarification on the issue. Consider that you have a massive load on GT and just tiny one on IA. If GT will program the RING frequency to be lower than IA frequency, then you will fall into the situation when RING frequency constantly transits from GT to IA level and back. Each transition of a RING frequency is a full system stall. If you will have "good" transition rate with few transitions per few milliseconds you will lose ~10% of performance. That's the case for media workloads when you easily can step into this since 1) media utilizes few GPU engines and with few parallel workloads you can make sure that at least 1 engine is _always_ doing something, 2) media BB are relatively small, so you have regular wakeups of the IA to manage requests. This will affect Gen9 platforms due to HW design change (we've spot this in SKL). This will not happen in Gen8 (old HW design). This will be fixed in Gen10+ (CNL+).

To clarify, the HW will flip between the two GT/IA requests rather than
stick to the highest? Iirc, the expectation was that we were setting a
requested minimum frequency for the ring/ia based off the gpu freq.

> On SKL we ran into this with the GPU frequency pinned to 700MHz, CPU to 2GHz. Multipliers were x2 for GT, x1 for IA.

Basically, with the GPU clocked to mid frequency, memory throughput is
insufficient to keep the fixed functions occupied, and you need to
increase the ring frequency. Is there ever a case where we don't need
max ring frequency? (Perhaps we still need to set low frequency for GT
idle?) I guess media is more susceptible to this as that workload should
be sustainable at reduced clocks, GL et al are much more likely to keep
the clocks ramped all the way up.

Do you know anything about the opposite position. I heard a suggestion
that simply increasing the ringfreq universally caused thermal
throttling in some other workloads. Do you have any knowledge of those?
 
> So, effectively, what we need to do is to make sure that RING frequency request from GT is _not_ below the request from IA. If IA requests 2GHz, we can't request 1.4GHz, we need request at least 2GHz. Multiplier patch was intended to do exactly that, but manually. Can  we somehow automate that managing IA frequency requests to the RING?

You are thinking of plugging into intel_pstate to make it smarter for ia
freq transitions? That seems possible, certainly. I'm not sure if the
ring frequency is actually poked from anywhere else in the kernel, would
be interesting to find out.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC] drm/i915: Add a new modparam for customized ring multiplier

[Rogozhkin, Dmitry V]

>> To clarify, the HW will flip between the two GT/IA requests rather than stick to the highest?

Yes, it will flip on Gen9. On Gen8 there was some mechanism (HW) which flattened that. But it was removed/substituted in Gen9. In Gen10 it was tuned  to close the mentioned issue.

>> Do you know anything about the opposite position. I heard a suggestion that simply increasing the ringfreq universally caused thermal throttling in some other workloads. Do you have any knowledge of those?

Initially we tried to just increase GT multiplier to x3 and stepped into the throttling. Thus, we introduced parameter to be able to mitigate all that depending on the SKU and user needs. I definitely asked what will be if GT request will be bigger than IA request. But that was a year ago and I don't remember the answer. Let me ask again. I will mail back in few days.

>> You are thinking of plugging into intel_pstate to make it smarter for ia freq transitions?

Yep. This seems a correct step to give some automatic support instead of parameter/hardcoded multiplier.

Dmitry.

-----Original Message-----
From: Chris Wilson [mailto:chris@chris-wilson.co.uk] 
Sent: Tuesday, December 26, 2017 8:59 AM
To: Rogozhkin, Dmitry V <dmitry.v.rogozhkin@intel.com>; Li, Yaodong <yaodong.li@intel.com>; intel-gfx@lists.freedesktop.org
Cc: Gong, Zhipeng <zhipeng.gong@intel.com>; Widawsky, Benjamin <benjamin.widawsky@intel.com>; Mateo Lozano, Oscar <oscar.mateo@intel.com>; Kamble, Sagar A <sagar.a.kamble@intel.com>; Li, Yaodong <yaodong.li@intel.com>
Subject: RE: [RFC] drm/i915: Add a new modparam for customized ring multiplier

Quoting Rogozhkin, Dmitry V (2017-12-26 16:39:23)
> Clarification on the issue. Consider that you have a massive load on GT and just tiny one on IA. If GT will program the RING frequency to be lower than IA frequency, then you will fall into the situation when RING frequency constantly transits from GT to IA level and back. Each transition of a RING frequency is a full system stall. If you will have "good" transition rate with few transitions per few milliseconds you will lose ~10% of performance. That's the case for media workloads when you easily can step into this since 1) media utilizes few GPU engines and with few parallel workloads you can make sure that at least 1 engine is _always_ doing something, 2) media BB are relatively small, so you have regular wakeups of the IA to manage requests. This will affect Gen9 platforms due to HW design change (we've spot this in SKL). This will not happen in Gen8 (old HW design). This will be fixed in Gen10+ (CNL+).

To clarify, the HW will flip between the two GT/IA requests rather than stick to the highest? Iirc, the expectation was that we were setting a requested minimum frequency for the ring/ia based off the gpu freq.

> On SKL we ran into this with the GPU frequency pinned to 700MHz, CPU to 2GHz. Multipliers were x2 for GT, x1 for IA.

Basically, with the GPU clocked to mid frequency, memory throughput is insufficient to keep the fixed functions occupied, and you need to increase the ring frequency. Is there ever a case where we don't need max ring frequency? (Perhaps we still need to set low frequency for GT
idle?) I guess media is more susceptible to this as that workload should be sustainable at reduced clocks, GL et al are much more likely to keep the clocks ramped all the way up.

Do you know anything about the opposite position. I heard a suggestion that simply increasing the ringfreq universally caused thermal throttling in some other workloads. Do you have any knowledge of those?
 
> So, effectively, what we need to do is to make sure that RING frequency request from GT is _not_ below the request from IA. If IA requests 2GHz, we can't request 1.4GHz, we need request at least 2GHz. Multiplier patch was intended to do exactly that, but manually. Can  we somehow automate that managing IA frequency requests to the RING?

You are thinking of plugging into intel_pstate to make it smarter for ia freq transitions? That seems possible, certainly. I'm not sure if the ring frequency is actually poked from anywhere else in the kernel, would be interesting to find out.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC] drm/i915: Add a new modparam for customized ring multiplier

[Rogozhkin, Dmitry V]

>> I definitely asked what will be if GT request will be bigger than IA request. But that was a year ago and I don't remember the answer. Let me ask again. I will mail back in few days.

Hi Chris, here is the response.

Question was: "Whether we can meet with the RING transition penalty (at least theoretically) if we will have GT request higher than IA request with the dominant IA load and tiny GT load, i.e. reverted situation of what we have actually faced? For example, if we will try to pin IA frequency to 800MHz (x1 multiplier) and GT frequency to 700MHz (x2 multiplier): in that case we will have requests for ring 800 vs. 1400."

Answer is: "In this case, if the GT will toggle between RC0 and RC6, it will force ring frequency to toggle between 800 and 1400, which in the toggling time will stall IA execution. This will lead to performance loss." However, this is a case if we have really few toggle events within few milliseconds. It is quite probable that GT driver will not allow such behavior to happen if it simply doesn't often toggle between RC0 and RC6. Considering that GT driver probably handles much less interrupts than IA, this can be the case. So, I think Chris, that's now question to you: how often you toggle between RC0 and RC6 to see the reverted issue to happen? If you don't toggle much, then RING will simply remain on 1400 almost all the time and you will see no issue.

Again, I remind that's the talk about Gen9 only.

Dmitry.

-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Rogozhkin, Dmitry V
Sent: Tuesday, December 26, 2017 9:39 AM
To: Chris Wilson <chris@chris-wilson.co.uk>; Li, Yaodong <yaodong.li@intel.com>; intel-gfx@lists.freedesktop.org
Cc: Widawsky, Benjamin <benjamin.widawsky@intel.com>
Subject: Re: [Intel-gfx] [RFC] drm/i915: Add a new modparam for customized ring multiplier

>> To clarify, the HW will flip between the two GT/IA requests rather than stick to the highest?

Yes, it will flip on Gen9. On Gen8 there was some mechanism (HW) which flattened that. But it was removed/substituted in Gen9. In Gen10 it was tuned  to close the mentioned issue.

>> Do you know anything about the opposite position. I heard a suggestion that simply increasing the ringfreq universally caused thermal throttling in some other workloads. Do you have any knowledge of those?

Initially we tried to just increase GT multiplier to x3 and stepped into the throttling. Thus, we introduced parameter to be able to mitigate all that depending on the SKU and user needs. I definitely asked what will be if GT request will be bigger than IA request. But that was a year ago and I don't remember the answer. Let me ask again. I will mail back in few days.

>> You are thinking of plugging into intel_pstate to make it smarter for ia freq transitions?

Yep. This seems a correct step to give some automatic support instead of parameter/hardcoded multiplier.

Dmitry.

-----Original Message-----
From: Chris Wilson [mailto:chris@chris-wilson.co.uk] 
Sent: Tuesday, December 26, 2017 8:59 AM
To: Rogozhkin, Dmitry V <dmitry.v.rogozhkin@intel.com>; Li, Yaodong <yaodong.li@intel.com>; intel-gfx@lists.freedesktop.org
Cc: Gong, Zhipeng <zhipeng.gong@intel.com>; Widawsky, Benjamin <benjamin.widawsky@intel.com>; Mateo Lozano, Oscar <oscar.mateo@intel.com>; Kamble, Sagar A <sagar.a.kamble@intel.com>; Li, Yaodong <yaodong.li@intel.com>
Subject: RE: [RFC] drm/i915: Add a new modparam for customized ring multiplier

Quoting Rogozhkin, Dmitry V (2017-12-26 16:39:23)
> Clarification on the issue. Consider that you have a massive load on GT and just tiny one on IA. If GT will program the RING frequency to be lower than IA frequency, then you will fall into the situation when RING frequency constantly transits from GT to IA level and back. Each transition of a RING frequency is a full system stall. If you will have "good" transition rate with few transitions per few milliseconds you will lose ~10% of performance. That's the case for media workloads when you easily can step into this since 1) media utilizes few GPU engines and with few parallel workloads you can make sure that at least 1 engine is _always_ doing something, 2) media BB are relatively small, so you have regular wakeups of the IA to manage requests. This will affect Gen9 platforms due to HW design change (we've spot this in SKL). This will not happen in Gen8 (old HW design). This will be fixed in Gen10+ (CNL+).

To clarify, the HW will flip between the two GT/IA requests rather than stick to the highest? Iirc, the expectation was that we were setting a requested minimum frequency for the ring/ia based off the gpu freq.

> On SKL we ran into this with the GPU frequency pinned to 700MHz, CPU to 2GHz. Multipliers were x2 for GT, x1 for IA.

Basically, with the GPU clocked to mid frequency, memory throughput is insufficient to keep the fixed functions occupied, and you need to increase the ring frequency. Is there ever a case where we don't need max ring frequency? (Perhaps we still need to set low frequency for GT
idle?) I guess media is more susceptible to this as that workload should be sustainable at reduced clocks, GL et al are much more likely to keep the clocks ramped all the way up.

Do you know anything about the opposite position. I heard a suggestion that simply increasing the ringfreq universally caused thermal throttling in some other workloads. Do you have any knowledge of those?
 
> So, effectively, what we need to do is to make sure that RING frequency request from GT is _not_ below the request from IA. If IA requests 2GHz, we can't request 1.4GHz, we need request at least 2GHz. Multiplier patch was intended to do exactly that, but manually. Can  we somehow automate that managing IA frequency requests to the RING?

You are thinking of plugging into intel_pstate to make it smarter for ia freq transitions? That seems possible, certainly. I'm not sure if the ring frequency is actually poked from anywhere else in the kernel, would be interesting to find out.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC] drm/i915: Add a new modparam for customized ring multiplier

[Yaodong Li]

>>> You are thinking of plugging into intel_pstate to make it smarter for ia freq transitions?
> Yep. This seems a correct step to give some automatic support instead of parameter/hardcoded multiplier.
>
Does this mean we should use cpufreq/intel_pstate based approach instead 
of the current modparam solution for Gen9?

Some concerns and questions about intel_pstate approach:
a) Currently, we cannot get the accurate pstate/target freq value from 
cpufreq in intel_pstate active mode since
      these values won't be exported to cpufreq layer, so if we won't 
change intel_pstate code then we only can get
      the max cpu freq of a new policy.
b) intel_pstate policy is attached to each logic cpu, which means we 
will receive policy/freq transition notification
     for each logic cpu freq change. One question is how we are going to 
decide the freq of the ring? just use the max
     cpu freq reported?
c) With the intel_pstate approach we may still run into thermal 
throttling, in this case, can a certain cooling device
     be triggered to lower the cpu freq?

Thanks and Regards,
-Jackie

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC] drm/i915: Add a new modparam for customized ring multiplier

[Sagar Arun Kamble]

Since ring frequency programming needs consideration of both IA and GT 
frequency requests I think keeping the logic
to program the ring frequency table in driver that monitors both IA/GT 
busyness and power budgets like intel_ips
will be more appropriate. intel_ips is relying on global load derived 
from all CPUs.
I understand that power awareness and busyness based policy might be 
trickier but having that as tunable will give better flexibility.

On 1/3/2018 11:51 PM, Yaodong Li wrote:
>
>>>> You are thinking of plugging into intel_pstate to make it smarter 
>>>> for ia freq transitions?
>> Yep. This seems a correct step to give some automatic support instead 
>> of parameter/hardcoded multiplier.
>>
> Does this mean we should use cpufreq/intel_pstate based approach 
> instead of the current modparam solution for Gen9?
>
> Some concerns and questions about intel_pstate approach:
> a) Currently, we cannot get the accurate pstate/target freq value from 
> cpufreq in intel_pstate active mode since
>      these values won't be exported to cpufreq layer, so if we won't 
> change intel_pstate code then we only can get
>      the max cpu freq of a new policy.
> b) intel_pstate policy is attached to each logic cpu, which means we 
> will receive policy/freq transition notification
>     for each logic cpu freq change. One question is how we are going 
> to decide the freq of the ring? just use the max
>     cpu freq reported?
> c) With the intel_pstate approach we may still run into thermal 
> throttling, in this case, can a certain cooling device
>     be triggered to lower the cpu freq?
>
> Thanks and Regards,
> -Jackie
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC] drm/i915: Add a new modparam for customized ring multiplier

[Yaodong Li]

On 01/03/2018 10:10 PM, Sagar Arun Kamble wrote:
> Since ring frequency programming needs consideration of both IA and GT 
> frequency requests I think keeping the logic
> to program the ring frequency table in driver that monitors both IA/GT 
> busyness and power budgets like intel_ips
> will be more appropriate. intel_ips is relying on global load derived 
> from all CPUs.
> I understand that power awareness and busyness based policy might be 
> trickier but having that as tunable will give better flexibility.
>
By just looking into the current code, the way intel_ips checks gpu 
busyness cannot reflect the actual workload of GT
(e.g. gpu busy is true even if there's only one pending request), in 
this case, we shall not increase the ring freq if we
want to use a "workload monitoring" based solution. so we need a more 
accurate way to monitor the current GT workload
(e.g.  when the pending request count reaches a center tunable threshold??).

> On 1/3/2018 11:51 PM, Yaodong Li wrote:
>>
>>>>> You are thinking of plugging into intel_pstate to make it smarter 
>>>>> for ia freq transitions?
>>> Yep. This seems a correct step to give some automatic support 
>>> instead of parameter/hardcoded multiplier.
>>>
>> Does this mean we should use cpufreq/intel_pstate based approach 
>> instead of the current modparam solution for Gen9?
>>
>> Some concerns and questions about intel_pstate approach:
>> a) Currently, we cannot get the accurate pstate/target freq value 
>> from cpufreq in intel_pstate active mode since
>>      these values won't be exported to cpufreq layer, so if we won't 
>> change intel_pstate code then we only can get
>>      the max cpu freq of a new policy.
>> b) intel_pstate policy is attached to each logic cpu, which means we 
>> will receive policy/freq transition notification
>>     for each logic cpu freq change. One question is how we are going 
>> to decide the freq of the ring? just use the max
>>     cpu freq reported?
>> c) With the intel_pstate approach we may still run into thermal 
>> throttling, in this case, can a certain cooling device
>>     be triggered to lower the cpu freq?
>>
>> Thanks and Regards,
>> -Jackie
>>
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC] drm/i915: Add a new modparam for customized ring multiplier

[Sagar Arun Kamble]

On 1/5/2018 3:22 AM, Yaodong Li wrote:
>
> On 01/03/2018 10:10 PM, Sagar Arun Kamble wrote:
>> Since ring frequency programming needs consideration of both IA and 
>> GT frequency requests I think keeping the logic
>> to program the ring frequency table in driver that monitors both 
>> IA/GT busyness and power budgets like intel_ips
>> will be more appropriate. intel_ips is relying on global load derived 
>> from all CPUs.
>> I understand that power awareness and busyness based policy might be 
>> trickier but having that as tunable will give better flexibility.
>>
> By just looking into the current code, the way intel_ips checks gpu 
> busyness cannot reflect the actual workload of GT
> (e.g. gpu busy is true even if there's only one pending request), in 
> this case, we shall not increase the ring freq if we
> want to use a "workload monitoring" based solution. so we need a more 
> accurate way to monitor the current GT workload
> (e.g.  when the pending request count reaches a center tunable 
> threshold??).
Yes. May be we can share the PMU data about engine busyness with intel_ips.
>
>> On 1/3/2018 11:51 PM, Yaodong Li wrote:
>>>
>>>>>> You are thinking of plugging into intel_pstate to make it smarter 
>>>>>> for ia freq transitions?
>>>> Yep. This seems a correct step to give some automatic support 
>>>> instead of parameter/hardcoded multiplier.
>>>>
>>> Does this mean we should use cpufreq/intel_pstate based approach 
>>> instead of the current modparam solution for Gen9?
>>>
>>> Some concerns and questions about intel_pstate approach:
>>> a) Currently, we cannot get the accurate pstate/target freq value 
>>> from cpufreq in intel_pstate active mode since
>>>      these values won't be exported to cpufreq layer, so if we won't 
>>> change intel_pstate code then we only can get
>>>      the max cpu freq of a new policy.
>>> b) intel_pstate policy is attached to each logic cpu, which means we 
>>> will receive policy/freq transition notification
>>>     for each logic cpu freq change. One question is how we are going 
>>> to decide the freq of the ring? just use the max
>>>     cpu freq reported?
>>> c) With the intel_pstate approach we may still run into thermal 
>>> throttling, in this case, can a certain cooling device
>>>     be triggered to lower the cpu freq?
>>>
>>> Thanks and Regards,
>>> -Jackie
>>>
>>
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC] drm/i915: Add a new modparam for customized ring multiplier

[Yaodong Li]

On 01/05/2018 02:15 AM, Sagar Arun Kamble wrote:
>
>
> On 1/5/2018 3:22 AM, Yaodong Li wrote:
>>
>> On 01/03/2018 10:10 PM, Sagar Arun Kamble wrote:
>>> Since ring frequency programming needs consideration of both IA and 
>>> GT frequency requests I think keeping the logic
>>> to program the ring frequency table in driver that monitors both 
>>> IA/GT busyness and power budgets like intel_ips
>>> will be more appropriate. intel_ips is relying on global load 
>>> derived from all CPUs.
>>> I understand that power awareness and busyness based policy might be 
>>> trickier but having that as tunable will give better flexibility.
>>>
>> By just looking into the current code, the way intel_ips checks gpu 
>> busyness cannot reflect the actual workload of GT
>> (e.g. gpu busy is true even if there's only one pending request), in 
>> this case, we shall not increase the ring freq if we
>> want to use a "workload monitoring" based solution. so we need a more 
>> accurate way to monitor the current GT workload
>> (e.g.  when the pending request count reaches a center tunable 
>> threshold??).
> Yes. May be we can share the PMU data about engine busyness with 
> intel_ips.
Thank you Sagar! Can you tell more about how we can get the gpu busyness 
from PMU data?

I think the solution would be to set the ring freq table to use a ring 
freq >= current ia freq (for all possible gpu freq) once we found
gpu workload is high (need to tune the threshold), and we will decrease 
the ring freq (use a 2x multiplier?) once we found the GT
workload is low. The benefit to use the intel_ips is it can tell both 
the cpu & gpu busyness. However, we do need an accurate way
to check at least the busyness of gpu for this issue.

>>> On 1/3/2018 11:51 PM, Yaodong Li wrote:
>>>>
>>>>>>> You are thinking of plugging into intel_pstate to make it 
>>>>>>> smarter for ia freq transitions?
>>>>> Yep. This seems a correct step to give some automatic support 
>>>>> instead of parameter/hardcoded multiplier.
>>>>>
>>>> Does this mean we should use cpufreq/intel_pstate based approach 
>>>> instead of the current modparam solution for Gen9?
>>>>
>>>> Some concerns and questions about intel_pstate approach:
>>>> a) Currently, we cannot get the accurate pstate/target freq value 
>>>> from cpufreq in intel_pstate active mode since
>>>>      these values won't be exported to cpufreq layer, so if we 
>>>> won't change intel_pstate code then we only can get
>>>>      the max cpu freq of a new policy.
>>>> b) intel_pstate policy is attached to each logic cpu, which means 
>>>> we will receive policy/freq transition notification
>>>>     for each logic cpu freq change. One question is how we are 
>>>> going to decide the freq of the ring? just use the max
>>>>     cpu freq reported?
>>>> c) With the intel_pstate approach we may still run into thermal 
>>>> throttling, in this case, can a certain cooling device
>>>>     be triggered to lower the cpu freq?
>>>>
>>>> Thanks and Regards,
>>>> -Jackie
>>>>
>>>
>>
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC] drm/i915: Add a new modparam for customized ring multiplier

[Sagar Arun Kamble]

On 1/6/2018 5:53 AM, Yaodong Li wrote:
> On 01/05/2018 02:15 AM, Sagar Arun Kamble wrote:
>>
>>
>> On 1/5/2018 3:22 AM, Yaodong Li wrote:
>>>
>>> On 01/03/2018 10:10 PM, Sagar Arun Kamble wrote:
>>>> Since ring frequency programming needs consideration of both IA and 
>>>> GT frequency requests I think keeping the logic
>>>> to program the ring frequency table in driver that monitors both 
>>>> IA/GT busyness and power budgets like intel_ips
>>>> will be more appropriate. intel_ips is relying on global load 
>>>> derived from all CPUs.
>>>> I understand that power awareness and busyness based policy might 
>>>> be trickier but having that as tunable will give better flexibility.
>>>>
>>> By just looking into the current code, the way intel_ips checks gpu 
>>> busyness cannot reflect the actual workload of GT
>>> (e.g. gpu busy is true even if there's only one pending request), in 
>>> this case, we shall not increase the ring freq if we
>>> want to use a "workload monitoring" based solution. so we need a 
>>> more accurate way to monitor the current GT workload
>>> (e.g.  when the pending request count reaches a center tunable 
>>> threshold??).
>> Yes. May be we can share the PMU data about engine busyness with 
>> intel_ips.
> Thank you Sagar! Can you tell more about how we can get the gpu 
> busyness from PMU data?
>
It seems to be not possible as of now since pmu is reporting accumulated 
busy time per engine without info about time they became active/idle,
unless I am missing something. Also having pmu ON in release environment 
needs to be checked. Chris, could you please clarify on pmu usage here.

Also thinking about below two options to get the GPU busyness:
1. Sharing i915 rps power zone (LOW_POWER, BETWEEN, HIGH_POWER) to 
intel_ips (kind of indicators of GPU load)
2. Sampling GT C0 counter to derive full GPU busyness. This though may 
not have been tested much so far across platforms.

> I think the solution would be to set the ring freq table to use a ring 
> freq >= current ia freq (for all possible gpu freq) once we found
> gpu workload is high (need to tune the threshold), and we will 
> decrease the ring freq (use a 2x multiplier?) once we found the GT
> workload is low. The benefit to use the intel_ips is it can tell both 
> the cpu & gpu busyness. However, we do need an accurate way
> to check at least the busyness of gpu for this issue.
Agree.

Thanks
Sagar
>
>>>> On 1/3/2018 11:51 PM, Yaodong Li wrote:
>>>>>
>>>>>>>> You are thinking of plugging into intel_pstate to make it 
>>>>>>>> smarter for ia freq transitions?
>>>>>> Yep. This seems a correct step to give some automatic support 
>>>>>> instead of parameter/hardcoded multiplier.
>>>>>>
>>>>> Does this mean we should use cpufreq/intel_pstate based approach 
>>>>> instead of the current modparam solution for Gen9?
>>>>>
>>>>> Some concerns and questions about intel_pstate approach:
>>>>> a) Currently, we cannot get the accurate pstate/target freq value 
>>>>> from cpufreq in intel_pstate active mode since
>>>>>      these values won't be exported to cpufreq layer, so if we 
>>>>> won't change intel_pstate code then we only can get
>>>>>      the max cpu freq of a new policy.
>>>>> b) intel_pstate policy is attached to each logic cpu, which means 
>>>>> we will receive policy/freq transition notification
>>>>>     for each logic cpu freq change. One question is how we are 
>>>>> going to decide the freq of the ring? just use the max
>>>>>     cpu freq reported?
>>>>> c) With the intel_pstate approach we may still run into thermal 
>>>>> throttling, in this case, can a certain cooling device
>>>>>     be triggered to lower the cpu freq?
>>>>>
>>>>> Thanks and Regards,
>>>>> -Jackie
>>>>>
>>>>
>>>
>>
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC] drm/i915: cpufreq based dynamic ring freqency table on Gen9

[Jackie Li]

A known issue that ring frequency transition would lead to
a full system stall had impacted the media performance when
the GT frequency is lower than IA freqency and there's heavy
GT workload while having low IA workload. In this case, the
ring frequency will be toggled between GT and IA frequency.
The solution is to keep the ring frequency higher than or
equal to current IA frequency so that we can avoid the toggling
between different frequencies issue.

This patch monitors the cpufreq frequency notifications (freqency
transition notification when governors are used, or new frequency
policy when bypassing the governors such as the active mode of
intel_pstate) and updates the ring frequency table by calculating
the ring frequency for each possible gpu frequency with a new
formula ring_freq = max(current IA freq, 2 * gpu_freq). with this
we can guarantee that the ring frequency would depend on IA freq
when the 2x gpu freq is lower then the active IA freq, and it would
depend on 2x gpu freq once the 2x gpu freq is higher than current
IA freq.

Potential issues with this solution:
0) currently, this patch is now only monitoring the frequency change
   of CPU 0. it would fail if another logic CPU was using a higher
   frequency.
1) the latency of the ring freq table update was not considered.
   Need more understanding to the cost of each ring freq table update.
2) Not sure the impact for low GT workload while the ring frequency
   is still set to higher or equal to IA frequency.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
Signed-off-by: Jackie Li <yaodong.li@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |   4 ++
 drivers/gpu/drm/i915/intel_pm.c | 106 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 107 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a689396..39311cd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2044,6 +2044,10 @@ struct drm_i915_private {
 	/* gen6+ GT PM state */
 	struct intel_gen6_power_mgmt gt_pm;
 
+	/* cpufreq notifiers*/
+	struct notifier_block cpufreq_transition;
+	struct notifier_block cpufreq_policy;
+
 	/* ilk-only ips/rps state. Everything in here is protected by the global
 	 * mchdev_lock in intel_pm.c */
 	struct intel_ilk_power_mgmt ips;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1db79a8..6a7e439 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6849,7 +6849,7 @@ static void gen6_update_ring_freq(struct drm_i915_private *dev_priv)
 	/* convert DDR frequency from units of 266.6MHz to bandwidth */
 	min_ring_freq = mult_frac(min_ring_freq, 8, 3);
 
-	if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv)) {
+	if (IS_CANNONLAKE(dev_priv)) {
 		/* Convert GT frequency to 50 HZ units */
 		min_gpu_freq = rps->min_freq / GEN9_FREQ_SCALER;
 		max_gpu_freq = rps->max_freq / GEN9_FREQ_SCALER;
@@ -6867,7 +6867,7 @@ static void gen6_update_ring_freq(struct drm_i915_private *dev_priv)
 		int diff = max_gpu_freq - gpu_freq;
 		unsigned int ia_freq = 0, ring_freq = 0;
 
-		if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv)) {
+		if (IS_CANNONLAKE(dev_priv)) {
 			/*
 			 * ring_freq = 2 * GT. ring_freq is in 100MHz units
 			 * No floor required for ring frequency on SKL.
@@ -8024,6 +8024,103 @@ void intel_disable_gt_powersave(struct drm_i915_private *dev_priv)
 	mutex_unlock(&dev_priv->pcu_lock);
 }
 
+static void gen9_update_ring_freq(struct drm_i915_private *dev_priv,
+				  unsigned int cpu_freq_mhz)
+{
+	struct intel_rps *rps = &dev_priv->gt_pm.rps;
+	unsigned int max_gpu_freq, min_gpu_freq;
+	unsigned int gpu_freq, ring_freq;
+	unsigned int cpu_freq_50mhz = cpu_freq_mhz / 50;
+
+	GEM_BUG_ON(!IS_GEN9(dev_priv));
+
+	mutex_lock(&dev_priv->pcu_lock);
+
+	if (IS_GEN9_BC(dev_priv)) {
+		/* Convert GT frequency to 50MHz units */
+		min_gpu_freq = rps->min_freq / GEN9_FREQ_SCALER;
+		max_gpu_freq = rps->max_freq / GEN9_FREQ_SCALER;
+	} else {
+		min_gpu_freq = rps->min_freq;
+		max_gpu_freq = rps->max_freq;
+	}
+
+	DRM_INFO("%s: gpu freq in 50MHz unit (%d - %d), (%d - %d),"
+		 "cpu freq in 50MHz unit %d\n",
+		 __func__,
+		 min_gpu_freq, max_gpu_freq,
+		 min_gpu_freq * 50,
+		 max_gpu_freq * 50,
+		 cpu_freq_50mhz);
+
+	for (gpu_freq = max_gpu_freq; gpu_freq >= min_gpu_freq; gpu_freq--) {
+		/* Ring freq is 100MHz unit */
+		ring_freq = max(cpu_freq_50mhz / 2, gpu_freq);
+
+		sandybridge_pcode_write(
+			dev_priv,
+			GEN6_PCODE_WRITE_MIN_FREQ_TABLE,
+			ring_freq << GEN6_PCODE_FREQ_RING_RATIO_SHIFT |
+			gpu_freq);
+	}
+
+	mutex_unlock(&dev_priv->pcu_lock);
+}
+
+static int cpufreq_transition_cb(struct notifier_block *nb, unsigned long val,
+				 void *data)
+{
+	struct cpufreq_freqs *freq = data;
+	struct drm_i915_private *i915 = container_of(nb,
+						     struct drm_i915_private,
+						     cpufreq_transition);
+
+	if (val != CPUFREQ_POSTCHANGE)
+		return 0;
+
+	if (freq->cpu != 0)
+		return 0;
+
+	if (freq->new == freq->old)
+		return 0;
+
+	gen9_update_ring_freq(i915, freq->new / 1000);
+
+	return 0;
+}
+
+static int cpufreq_policy_cb(struct notifier_block *nb, unsigned long val,
+			     void *data)
+{
+	struct cpufreq_policy *policy = data;
+	struct drm_i915_private *i915 = container_of(nb,
+						     struct drm_i915_private,
+						     cpufreq_policy);
+
+	if (val != CPUFREQ_NOTIFY)
+		return 0;
+
+	if (policy->cpu != 0)
+		return 0;
+
+	gen9_update_ring_freq(i915, policy->max / 1000);
+
+	return 0;
+}
+
+
+static inline
+void intel_register_cpufreq_notifiers(struct drm_i915_private *i915)
+{
+	i915->cpufreq_transition.notifier_call = cpufreq_transition_cb;
+	i915->cpufreq_policy.notifier_call = cpufreq_policy_cb;
+
+	cpufreq_register_notifier(&i915->cpufreq_transition,
+				  CPUFREQ_TRANSITION_NOTIFIER);
+	cpufreq_register_notifier(&i915->cpufreq_policy,
+				  CPUFREQ_POLICY_NOTIFIER);
+}
+
 static inline void intel_enable_llc_pstate(struct drm_i915_private *i915)
 {
 	lockdep_assert_held(&i915->pcu_lock);
@@ -8031,7 +8128,10 @@ static inline void intel_enable_llc_pstate(struct drm_i915_private *i915)
 	if (i915->gt_pm.llc_pstate.enabled)
 		return;
 
-	gen6_update_ring_freq(i915);
+	if (IS_GEN9(i915))
+		intel_register_cpufreq_notifiers(i915);
+	else
+		gen6_update_ring_freq(i915);
 
 	i915->gt_pm.llc_pstate.enabled = true;
 }
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for drm/i915: Add a new modparam for customized ring multiplier

[Patchwork]

== Series Details ==

Series: drm/i915: Add a new modparam for customized ring multiplier
URL   : https://patchwork.freedesktop.org/series/35535/
State : failure

== Summary ==

Test kms_fbcon_fbt:
        Subgroup fbc-suspend:
                pass       -> SKIP       (shard-hsw)
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
                fail       -> PASS       (shard-snb) fdo#101623
Test drv_selftest:
        Subgroup live_hangcheck:
                incomplete -> PASS       (shard-snb) fdo#104313
Test kms_cursor_crc:
        Subgroup cursor-128x128-suspend:
                pass       -> SKIP       (shard-snb) fdo#103375
Test kms_plane:
        Subgroup plane-position-covered-pipe-c-planes:
                pass       -> INCOMPLETE (shard-hsw)
Test drv_suspend:
        Subgroup debugfs-reader:
                skip       -> PASS       (shard-hsw)

fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#104313 https://bugs.freedesktop.org/show_bug.cgi?id=104313
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375

shard-hsw        total:2637 pass:1490 dwarn:1   dfail:0   fail:10  skip:1135 time:9281s
shard-snb        total:2712 pass:1307 dwarn:1   dfail:0   fail:12  skip:1392 time:7951s
Blacklisted hosts:
shard-apl        total:2712 pass:1686 dwarn:2   dfail:1   fail:22  skip:1001 time:13786s
shard-kbl        total:2699 pass:1793 dwarn:1   dfail:1   fail:25  skip:878 time:10894s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7530/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915: Add a new modparam for customized ring multiplier (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Add a new modparam for customized ring multiplier (rev2)
URL   : https://patchwork.freedesktop.org/series/35535/
State : failure

== Summary ==

Series 35535v2 drm/i915: Add a new modparam for customized ring multiplier
https://patchwork.freedesktop.org/api/1.0/series/35535/revisions/2/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> PASS       (fi-elk-e7500) fdo#103989
Test gem_ctx_switch:
        Subgroup basic-default-heavy:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-kbl-7500u)
                pass       -> DMESG-WARN (fi-kbl-r)
Test gem_exec_basic:
        Subgroup gtt-blt:
                pass       -> DMESG-WARN (fi-skl-6770hq)
Test gem_exec_create:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-kbl-7560u)
Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> INCOMPLETE (fi-skl-6260u)
                pass       -> INCOMPLETE (fi-skl-6600u)
                pass       -> INCOMPLETE (fi-skl-6700hq)
                pass       -> INCOMPLETE (fi-skl-6700k2)
                pass       -> INCOMPLETE (fi-skl-6770hq)
                pass       -> INCOMPLETE (fi-skl-guc)
                pass       -> INCOMPLETE (fi-skl-gvtdvm)
                pass       -> INCOMPLETE (fi-kbl-7500u)
                pass       -> INCOMPLETE (fi-kbl-7560u)
                pass       -> INCOMPLETE (fi-kbl-7567u)
                pass       -> INCOMPLETE (fi-kbl-r)
                pass       -> INCOMPLETE (fi-cfl-s2)

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:419s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:429s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:373s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:479s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:281s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:487s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:482s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:468s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:451s
fi-cfl-s2        total:109  pass:97   dwarn:0   dfail:0   fail:0   skip:11 
fi-elk-e7500     total:224  pass:169  dwarn:9   dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:276s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:512s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:390s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:399s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:412s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:459s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:416s
fi-kbl-7500u     total:109  pass:96   dwarn:1   dfail:0   fail:0   skip:11 
fi-kbl-7560u     total:109  pass:104  dwarn:1   dfail:0   fail:0   skip:3  
fi-kbl-7567u     total:109  pass:105  dwarn:0   dfail:0   fail:0   skip:3  
fi-kbl-r         total:109  pass:96   dwarn:1   dfail:0   fail:0   skip:11 
fi-skl-6260u     total:109  pass:104  dwarn:1   dfail:0   fail:0   skip:3  
fi-skl-6600u     total:109  pass:97   dwarn:0   dfail:0   fail:0   skip:11 
fi-skl-6700hq    total:109  pass:97   dwarn:0   dfail:0   fail:0   skip:11 
fi-skl-6700k2    total:109  pass:97   dwarn:0   dfail:0   fail:0   skip:11 
fi-skl-6770hq    total:109  pass:104  dwarn:1   dfail:0   fail:0   skip:3  
fi-skl-guc       total:109  pass:97   dwarn:0   dfail:0   fail:0   skip:11 
fi-skl-gvtdvm    total:109  pass:104  dwarn:0   dfail:0   fail:0   skip:4  
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:527s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:403s
Blacklisted hosts:
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:472s

0edf4cc4e7c163fa61312f1708fc92284b855f8e drm-tip: 2018y-01m-30d-18h-33m-48s UTC integration manifest
5422cb80b935 drm/i915: cpufreq based dynamic ring freqency table on Gen9

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7832/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx