| Message ID | 1397069046-50906-1-git-send-email-Tom.O'Rourke@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: > Higher RC6 residency is observed using timeout mode > instead of EI mode. This applies to Broadwell only. > The difference is particularly noticeable with video > playback. > > Issue: VIZ-3778 > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> How recent a nightly branch have you used to obtain these results? Chris just fixed some serious bugs in the gpu booster logic which would have affected all intermediate workloads. -Daniel > --- > drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 33b2592..0d63abf 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3335,15 +3335,23 @@ static void gen8_enable_rps(struct drm_device *dev) > for_each_ring(ring, dev_priv, unused) > I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10); > I915_WRITE(GEN6_RC_SLEEP, 0); > - I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */ > + if (IS_BROADWELL(dev)) > + I915_WRITE(GEN6_RC6_THRESHOLD, 625); /* 800us/1.28 for TO */ > + else > + I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */ > > /* 3: Enable RC6 */ > if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE) > rc6_mask = GEN6_RC_CTL_RC6_ENABLE; > intel_print_rc6_info(dev, rc6_mask); > - I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE | > - GEN6_RC_CTL_EI_MODE(1) | > - rc6_mask); > + if (IS_BROADWELL(dev)) > + I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE | > + GEN7_RC_CTL_TO_MODE | > + rc6_mask); > + else > + I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE | > + GEN6_RC_CTL_EI_MODE(1) | > + rc6_mask); > > /* 4 Program defaults and thresholds for RPS*/ > I915_WRITE(GEN6_RPNSWREQ, > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Apr 09, 2014 at 10:02:39PM +0200, Daniel Vetter wrote: > On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: > > Higher RC6 residency is observed using timeout mode > > instead of EI mode. This applies to Broadwell only. > > The difference is particularly noticeable with video > > playback. > > > > Issue: VIZ-3778 > > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d > > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> > > How recent a nightly branch have you used to obtain these results? > Chris just fixed some serious bugs in the gpu booster logic which would > have affected all intermediate workloads. > -Daniel He must not be using nightly if he has any BDW RC6 residency at all. > > > --- > > drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++++++---- > > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index 33b2592..0d63abf 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -3335,15 +3335,23 @@ static void gen8_enable_rps(struct drm_device *dev) > > for_each_ring(ring, dev_priv, unused) > > I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10); > > I915_WRITE(GEN6_RC_SLEEP, 0); > > - I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */ > > + if (IS_BROADWELL(dev)) > > + I915_WRITE(GEN6_RC6_THRESHOLD, 625); /* 800us/1.28 for TO */ > > + else > > + I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */ > > > > /* 3: Enable RC6 */ > > if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE) > > rc6_mask = GEN6_RC_CTL_RC6_ENABLE; > > intel_print_rc6_info(dev, rc6_mask); > > - I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE | > > - GEN6_RC_CTL_EI_MODE(1) | > > - rc6_mask); > > + if (IS_BROADWELL(dev)) > > + I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE | > > + GEN7_RC_CTL_TO_MODE | > > + rc6_mask); > > + else > > + I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE | > > + GEN6_RC_CTL_EI_MODE(1) | > > + rc6_mask); > > > > /* 4 Program defaults and thresholds for RPS*/ > > I915_WRITE(GEN6_RPNSWREQ, > > -- > > 1.7.9.5 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> > Higher RC6 residency is observed using timeout mode instead of EI >> > mode. This applies to Broadwell only. >> > The difference is particularly noticeable with video playback. >> > >> > Issue: VIZ-3778 >> > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d >> > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> >> >> How recent a nightly branch have you used to obtain these results? >> Chris just fixed some serious bugs in the gpu booster logic which >> would have affected all intermediate workloads. >> -Daniel > >He must not be using nightly if he has any BDW RC6 residency at all. > [TOR:] Ben is correct. I was testing mostly with a kernel for Android. I also tested with Ben's broadwell branch and saw similar improvement.
On Thu, Apr 10, 2014 at 08:29:55PM +0000, O'Rourke, Tom wrote: > >> > Higher RC6 residency is observed using timeout mode instead of EI > >> > mode. This applies to Broadwell only. > >> > The difference is particularly noticeable with video playback. > >> > > >> > Issue: VIZ-3778 > >> > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d > >> > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> > >> > >> How recent a nightly branch have you used to obtain these results? > >> Chris just fixed some serious bugs in the gpu booster logic which > >> would have affected all intermediate workloads. > >> -Daniel > > > >He must not be using nightly if he has any BDW RC6 residency at all. > > > [TOR:] Ben is correct. I was testing mostly with a kernel for Android. > I also tested with Ben's broadwell branch and saw similar improvement. Ok I think it'd be good to have this when we actually merge/enable the final pieces of the bdw rc6 code. Can you please work together with Ben to make sure this patch isn't lost? Or who is shepherding bdw rc6 nowadays? -Daniel
On Fri, Apr 11, 2014 at 11:00:38AM +0200, Daniel Vetter wrote: > On Thu, Apr 10, 2014 at 08:29:55PM +0000, O'Rourke, Tom wrote: > > >> > Higher RC6 residency is observed using timeout mode instead of EI > > >> > mode. This applies to Broadwell only. > > >> > The difference is particularly noticeable with video playback. > > >> > > > >> > Issue: VIZ-3778 > > >> > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d > > >> > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> > > >> > > >> How recent a nightly branch have you used to obtain these results? > > >> Chris just fixed some serious bugs in the gpu booster logic which > > >> would have affected all intermediate workloads. > > >> -Daniel > > > > > >He must not be using nightly if he has any BDW RC6 residency at all. > > > > > [TOR:] Ben is correct. I was testing mostly with a kernel for Android. > > I also tested with Ben's broadwell branch and saw similar improvement. > > Ok I think it'd be good to have this when we actually merge/enable the > final pieces of the bdw rc6 code. Can you please work together with Ben to > make sure this patch isn't lost? > > Or who is shepherding bdw rc6 nowadays? > -Daniel Locally, people are still using my RC6 "workaround." We're waiting on Mika at the moment. I am going to merge this patch to my bdw-rc6/broadwell branches. I think it would be great to get a little more detail under what circumstances this improves residency. We can discuss that internally.
On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: > Higher RC6 residency is observed using timeout mode > instead of EI mode. This applies to Broadwell only. > The difference is particularly noticeable with video > playback. > > Issue: VIZ-3778 > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> I've merged this one to my bdw-rc6 branch, and therefore my broadwell branch. Hopefully Kristen will see some improvement. > --- > drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 33b2592..0d63abf 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3335,15 +3335,23 @@ static void gen8_enable_rps(struct drm_device *dev) > for_each_ring(ring, dev_priv, unused) > I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10); > I915_WRITE(GEN6_RC_SLEEP, 0); > - I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */ > + if (IS_BROADWELL(dev)) > + I915_WRITE(GEN6_RC6_THRESHOLD, 625); /* 800us/1.28 for TO */ > + else > + I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */ > > /* 3: Enable RC6 */ > if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE) > rc6_mask = GEN6_RC_CTL_RC6_ENABLE; > intel_print_rc6_info(dev, rc6_mask); > - I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE | > - GEN6_RC_CTL_EI_MODE(1) | > - rc6_mask); > + if (IS_BROADWELL(dev)) > + I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE | > + GEN7_RC_CTL_TO_MODE | > + rc6_mask); > + else > + I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE | > + GEN6_RC_CTL_EI_MODE(1) | > + rc6_mask); > > /* 4 Program defaults and thresholds for RPS*/ > I915_WRITE(GEN6_RPNSWREQ, > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 29 Apr 2014 22:31:49 -0700 Ben Widawsky <ben@bwidawsk.net> wrote: > On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: > > Higher RC6 residency is observed using timeout mode > > instead of EI mode. This applies to Broadwell only. > > The difference is particularly noticeable with video > > playback. > > > > Issue: VIZ-3778 > > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d > > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> > > I've merged this one to my bdw-rc6 branch, and therefore my broadwell > branch. Hopefully Kristen will see some improvement. Unfortunately, I built your bdw-rc6 branch along with the revert I need to get my panel to work, and I get zero rc6 residency. Do I have to explicitly enable it?
On Wed, Apr 30, 2014 at 01:34:36PM -0700, Kristen Carlson Accardi wrote: > On Tue, 29 Apr 2014 22:31:49 -0700 > Ben Widawsky <ben@bwidawsk.net> wrote: > > > On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: > > > Higher RC6 residency is observed using timeout mode > > > instead of EI mode. This applies to Broadwell only. > > > The difference is particularly noticeable with video > > > playback. > > > > > > Issue: VIZ-3778 > > > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d > > > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> > > > > I've merged this one to my bdw-rc6 branch, and therefore my broadwell > > branch. Hopefully Kristen will see some improvement. > > Unfortunately, I built your bdw-rc6 branch along with the revert > I need to get my panel to work, and I get zero rc6 residency. Do I > have to explicitly enable it? I'm not actually sure. You can try it and let me know. I haven't had any time to verify the rebase. We can check my hack.
On Wed, 2014-04-30 at 13:41 -0700, Ben Widawsky wrote: > On Wed, Apr 30, 2014 at 01:34:36PM -0700, Kristen Carlson Accardi wrote: > > On Tue, 29 Apr 2014 22:31:49 -0700 > > Ben Widawsky <ben@bwidawsk.net> wrote: > > > > > On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: > > > > Higher RC6 residency is observed using timeout mode > > > > instead of EI mode. This applies to Broadwell only. > > > > The difference is particularly noticeable with video > > > > playback. > > > > > > > > Issue: VIZ-3778 > > > > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d > > > > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> > > > > > > I've merged this one to my bdw-rc6 branch, and therefore my broadwell > > > branch. Hopefully Kristen will see some improvement. > > > > Unfortunately, I built your bdw-rc6 branch along with the revert > > I need to get my panel to work, and I get zero rc6 residency. Do I > > have to explicitly enable it? > > I'm not actually sure. You can try it and let me know. I haven't had any > time to verify the rebase. We can check my hack. Note that in -nightly you also have to update sanitize_rc6_option() along with intel_enable_gt_powersave() and intel_disable_gt_powersave() since atm these keep RC6 disabled on BDW. --Imre
On Thu, 01 May 2014 00:03:15 +0300 Imre Deak <imre.deak@intel.com> wrote: > On Wed, 2014-04-30 at 13:41 -0700, Ben Widawsky wrote: > > On Wed, Apr 30, 2014 at 01:34:36PM -0700, Kristen Carlson Accardi wrote: > > > On Tue, 29 Apr 2014 22:31:49 -0700 > > > Ben Widawsky <ben@bwidawsk.net> wrote: > > > > > > > On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: > > > > > Higher RC6 residency is observed using timeout mode > > > > > instead of EI mode. This applies to Broadwell only. > > > > > The difference is particularly noticeable with video > > > > > playback. > > > > > > > > > > Issue: VIZ-3778 > > > > > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d > > > > > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> > > > > > > > > I've merged this one to my bdw-rc6 branch, and therefore my broadwell > > > > branch. Hopefully Kristen will see some improvement. > > > > > > Unfortunately, I built your bdw-rc6 branch along with the revert > > > I need to get my panel to work, and I get zero rc6 residency. Do I > > > have to explicitly enable it? > > > > I'm not actually sure. You can try it and let me know. I haven't had any > > time to verify the rebase. We can check my hack. > > Note that in -nightly you also have to update sanitize_rc6_option() > along with intel_enable_gt_powersave() and intel_disable_gt_powersave() > since atm these keep RC6 disabled on BDW. > > --Imre > Yes, I reverted fb5ed3b201fe5670c9ffeec3b5f6ff044d543c9e and was able to see some rc6 residency. With the idle workload, residency appears to be similar to before, so no regression.
On Wed, Apr 30, 2014 at 02:14:02PM -0700, Kristen Carlson Accardi wrote: > On Thu, 01 May 2014 00:03:15 +0300 > Imre Deak <imre.deak@intel.com> wrote: > > > On Wed, 2014-04-30 at 13:41 -0700, Ben Widawsky wrote: > > > On Wed, Apr 30, 2014 at 01:34:36PM -0700, Kristen Carlson Accardi wrote: > > > > On Tue, 29 Apr 2014 22:31:49 -0700 > > > > Ben Widawsky <ben@bwidawsk.net> wrote: > > > > > > > > > On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: > > > > > > Higher RC6 residency is observed using timeout mode > > > > > > instead of EI mode. This applies to Broadwell only. > > > > > > The difference is particularly noticeable with video > > > > > > playback. > > > > > > > > > > > > Issue: VIZ-3778 > > > > > > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d > > > > > > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> > > > > > > > > > > I've merged this one to my bdw-rc6 branch, and therefore my broadwell > > > > > branch. Hopefully Kristen will see some improvement. > > > > > > > > Unfortunately, I built your bdw-rc6 branch along with the revert > > > > I need to get my panel to work, and I get zero rc6 residency. Do I > > > > have to explicitly enable it? > > > > > > I'm not actually sure. You can try it and let me know. I haven't had any > > > time to verify the rebase. We can check my hack. > > > > Note that in -nightly you also have to update sanitize_rc6_option() > > along with intel_enable_gt_powersave() and intel_disable_gt_powersave() > > since atm these keep RC6 disabled on BDW. > > > > --Imre > > > > > Yes, I reverted fb5ed3b201fe5670c9ffeec3b5f6ff044d543c9e and was able > to see some rc6 residency. With the idle workload, residency appears > to be similar to before, so no regression. Thanks. I'll squash this in where appropriate.
>On Wed, Apr 30, 2014 at 02:14:02PM -0700, Kristen Carlson Accardi wrote: >> On Thu, 01 May 2014 00:03:15 +0300 >> Imre Deak <imre.deak@intel.com> wrote: >> >> > On Wed, 2014-04-30 at 13:41 -0700, Ben Widawsky wrote: >> > > On Wed, Apr 30, 2014 at 01:34:36PM -0700, Kristen Carlson Accardi wrote: >> > > > On Tue, 29 Apr 2014 22:31:49 -0700 Ben Widawsky >> > > > <ben@bwidawsk.net> wrote: >> > > > >> > > > > On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: >> > > > > > Higher RC6 residency is observed using timeout mode instead >> > > > > > of EI mode. This applies to Broadwell only. >> > > > > > The difference is particularly noticeable with video >> > > > > > playback. >> > > > > > >> > > > > > Issue: VIZ-3778 >> > > > > > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d >> > > > > > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> >> > > > > >> > > > > I've merged this one to my bdw-rc6 branch, and therefore my >> > > > > broadwell branch. Hopefully Kristen will see some improvement. >> > > > >> > > > Unfortunately, I built your bdw-rc6 branch along with the revert >> > > > I need to get my panel to work, and I get zero rc6 residency. >> > > > Do I have to explicitly enable it? >> > > >> > > I'm not actually sure. You can try it and let me know. I haven't >> > > had any time to verify the rebase. We can check my hack. >> > >> > Note that in -nightly you also have to update sanitize_rc6_option() >> > along with intel_enable_gt_powersave() and >> > intel_disable_gt_powersave() since atm these keep RC6 disabled on BDW. >> > >> > --Imre >> > >> >> >> Yes, I reverted fb5ed3b201fe5670c9ffeec3b5f6ff044d543c9e and was able >> to see some rc6 residency. With the idle workload, residency appears >> to be similar to before, so no regression. > >Thanks. I'll squash this in where appropriate. > >-- >Ben Widawsky, Intel Open Source Technology Center [TOR:] Can we get this patch merged now that RC6 is working on drm-intel-nightly? Thanks, Tom
On Fri, May 30, 2014 at 11:30:18PM +0000, O'Rourke, Tom wrote: > >On Wed, Apr 30, 2014 at 02:14:02PM -0700, Kristen Carlson Accardi wrote: > >> On Thu, 01 May 2014 00:03:15 +0300 > >> Imre Deak <imre.deak@intel.com> wrote: > >> > >> > On Wed, 2014-04-30 at 13:41 -0700, Ben Widawsky wrote: > >> > > On Wed, Apr 30, 2014 at 01:34:36PM -0700, Kristen Carlson Accardi wrote: > >> > > > On Tue, 29 Apr 2014 22:31:49 -0700 Ben Widawsky > >> > > > <ben@bwidawsk.net> wrote: > >> > > > > >> > > > > On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: > >> > > > > > Higher RC6 residency is observed using timeout mode instead > >> > > > > > of EI mode. This applies to Broadwell only. > >> > > > > > The difference is particularly noticeable with video > >> > > > > > playback. > >> > > > > > > >> > > > > > Issue: VIZ-3778 > >> > > > > > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d > >> > > > > > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> > >> > > > > > >> > > > > I've merged this one to my bdw-rc6 branch, and therefore my > >> > > > > broadwell branch. Hopefully Kristen will see some improvement. > >> > > > > >> > > > Unfortunately, I built your bdw-rc6 branch along with the revert > >> > > > I need to get my panel to work, and I get zero rc6 residency. > >> > > > Do I have to explicitly enable it? > >> > > > >> > > I'm not actually sure. You can try it and let me know. I haven't > >> > > had any time to verify the rebase. We can check my hack. > >> > > >> > Note that in -nightly you also have to update sanitize_rc6_option() > >> > along with intel_enable_gt_powersave() and > >> > intel_disable_gt_powersave() since atm these keep RC6 disabled on BDW. > >> > > >> > --Imre > >> > > >> > >> > >> Yes, I reverted fb5ed3b201fe5670c9ffeec3b5f6ff044d543c9e and was able > >> to see some rc6 residency. With the idle workload, residency appears > >> to be similar to before, so no regression. > > > >Thanks. I'll squash this in where appropriate. > > > >-- > >Ben Widawsky, Intel Open Source Technology Center > > [TOR:] Can we get this patch merged now that RC6 is working on drm-intel-nightly? Needs some review from bdw people. Also some relative residency improvement date should be added to the commit message (yes, we're allowed to do that now officially). -Daniel
>From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter >Sent: Monday, June 02, 2014 1:26 AM >To: O'Rourke, Tom >Cc: intel-gfx@lists.freedesktop.org; Ben Widawsky; Kristen Carlson Accardi >Subject: Re: [Intel-gfx] [PATCH] drm/i915/bdw: Use timeout mode for RC6 on >bdw > >On Fri, May 30, 2014 at 11:30:18PM +0000, O'Rourke, Tom wrote: >> >On Wed, Apr 30, 2014 at 02:14:02PM -0700, Kristen Carlson Accardi wrote: >> >> On Thu, 01 May 2014 00:03:15 +0300 >> >> Imre Deak <imre.deak@intel.com> wrote: >> >> >> >> > On Wed, 2014-04-30 at 13:41 -0700, Ben Widawsky wrote: >> >> > > On Wed, Apr 30, 2014 at 01:34:36PM -0700, Kristen Carlson Accardi >wrote: >> >> > > > On Tue, 29 Apr 2014 22:31:49 -0700 Ben Widawsky >> >> > > > <ben@bwidawsk.net> wrote: >> >> > > > >> >> > > > > On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: >> >> > > > > > Higher RC6 residency is observed using timeout mode >> >> > > > > > instead of EI mode. This applies to Broadwell only. >> >> > > > > > The difference is particularly noticeable with video >> >> > > > > > playback. >> >> > > > > > >> >> > > > > > Issue: VIZ-3778 >> >> > > > > > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d >> >> > > > > > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> >> >> > > > > >> >> > > > > I've merged this one to my bdw-rc6 branch, and therefore my >> >> > > > > broadwell branch. Hopefully Kristen will see some improvement. >> >> > > > >> >> > > > Unfortunately, I built your bdw-rc6 branch along with the >> >> > > > revert I need to get my panel to work, and I get zero rc6 residency. >> >> > > > Do I have to explicitly enable it? >> >> > > >> >> > > I'm not actually sure. You can try it and let me know. I >> >> > > haven't had any time to verify the rebase. We can check my hack. >> >> > >> >> > Note that in -nightly you also have to update >> >> > sanitize_rc6_option() along with intel_enable_gt_powersave() and >> >> > intel_disable_gt_powersave() since atm these keep RC6 disabled on BDW. >> >> > >> >> > --Imre >> >> > >> >> >> >> >> >> Yes, I reverted fb5ed3b201fe5670c9ffeec3b5f6ff044d543c9e and was >> >> able to see some rc6 residency. With the idle workload, residency >> >> appears to be similar to before, so no regression. >> > >> >Thanks. I'll squash this in where appropriate. >> > >> >-- >> >Ben Widawsky, Intel Open Source Technology Center >> >> [TOR:] Can we get this patch merged now that RC6 is working on drm-intel- >nightly? > >Needs some review from bdw people. Also some relative residency >improvement date should be added to the commit message (yes, we're allowed >to do that now officially). >-Daniel >-- [TOR:] Hello bdw people, please review this patch. Is relative performance data now required in the commit message? A week ago this would have been prohibited. Thanks, Tom
On Mon, Jun 02, 2014 at 09:51:27PM +0000, O'Rourke, Tom wrote: > >From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter > >Sent: Monday, June 02, 2014 1:26 AM > >To: O'Rourke, Tom > >Cc: intel-gfx@lists.freedesktop.org; Ben Widawsky; Kristen Carlson Accardi > >Subject: Re: [Intel-gfx] [PATCH] drm/i915/bdw: Use timeout mode for RC6 on > >bdw > > > >On Fri, May 30, 2014 at 11:30:18PM +0000, O'Rourke, Tom wrote: > >> >On Wed, Apr 30, 2014 at 02:14:02PM -0700, Kristen Carlson Accardi wrote: > >> >> On Thu, 01 May 2014 00:03:15 +0300 > >> >> Imre Deak <imre.deak@intel.com> wrote: > >> >> > >> >> > On Wed, 2014-04-30 at 13:41 -0700, Ben Widawsky wrote: > >> >> > > On Wed, Apr 30, 2014 at 01:34:36PM -0700, Kristen Carlson Accardi > >wrote: > >> >> > > > On Tue, 29 Apr 2014 22:31:49 -0700 Ben Widawsky > >> >> > > > <ben@bwidawsk.net> wrote: > >> >> > > > > >> >> > > > > On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: > >> >> > > > > > Higher RC6 residency is observed using timeout mode > >> >> > > > > > instead of EI mode. This applies to Broadwell only. > >> >> > > > > > The difference is particularly noticeable with video > >> >> > > > > > playback. > >> >> > > > > > > >> >> > > > > > Issue: VIZ-3778 > >> >> > > > > > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d > >> >> > > > > > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> > >> >> > > > > > >> >> > > > > I've merged this one to my bdw-rc6 branch, and therefore my > >> >> > > > > broadwell branch. Hopefully Kristen will see some improvement. > >> >> > > > > >> >> > > > Unfortunately, I built your bdw-rc6 branch along with the > >> >> > > > revert I need to get my panel to work, and I get zero rc6 residency. > >> >> > > > Do I have to explicitly enable it? > >> >> > > > >> >> > > I'm not actually sure. You can try it and let me know. I > >> >> > > haven't had any time to verify the rebase. We can check my hack. > >> >> > > >> >> > Note that in -nightly you also have to update > >> >> > sanitize_rc6_option() along with intel_enable_gt_powersave() and > >> >> > intel_disable_gt_powersave() since atm these keep RC6 disabled on BDW. > >> >> > > >> >> > --Imre > >> >> > > >> >> > >> >> > >> >> Yes, I reverted fb5ed3b201fe5670c9ffeec3b5f6ff044d543c9e and was > >> >> able to see some rc6 residency. With the idle workload, residency > >> >> appears to be similar to before, so no regression. > >> > > >> >Thanks. I'll squash this in where appropriate. > >> > > >> >-- > >> >Ben Widawsky, Intel Open Source Technology Center > >> > >> [TOR:] Can we get this patch merged now that RC6 is working on drm-intel- > >nightly? > > > >Needs some review from bdw people. Also some relative residency > >improvement date should be added to the commit message (yes, we're allowed > >to do that now officially). > >-Daniel > >-- > > [TOR:] Hello bdw people, please review this patch. > > Is relative performance data now required in the commit message? A week > ago this would have been prohibited. You might need to double-check with your own manager, but we're now again allowed to officially add it to the commit message. It was kinda always required, just had to be washed down more. -Daniel
>From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter >Sent: Tuesday, June 03, 2014 12:38 AM >To: O'Rourke, Tom >Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org; Ben Widawsky; Kristen >Carlson Accardi >Subject: Re: [Intel-gfx] [PATCH] drm/i915/bdw: Use timeout mode for RC6 on >bdw > [TOR:]...snip >> >> >> >> [TOR:] Can we get this patch merged now that RC6 is working on >> >> drm-intel- >> >nightly? >> > >> >Needs some review from bdw people. Also some relative residency >> >improvement date should be added to the commit message (yes, we're >> >allowed to do that now officially). >> >-Daniel >> >-- >> >> [TOR:] Hello bdw people, please review this patch. >> >> Is relative performance data now required in the commit message? A >> week ago this would have been prohibited. > >You might need to double-check with your own manager, but we're now again >allowed to officially add it to the commit message. It was kinda always required, >just had to be washed down more. >-Daniel [TOR:] Sorry, I am not allowed to add the performance improvement claims to the commit message. Tom
On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: > Higher RC6 residency is observed using timeout mode > instead of EI mode. This applies to Broadwell only. > The difference is particularly noticeable with video > playback. > > Issue: VIZ-3778 > Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> Now that CHV is out, does it apply there too? Reviewed-by: Ben Widawsky <ben@bwidawsk.net> [snip]
>From: Ben Widawsky [mailto:ben@bwidawsk.net] >Sent: Friday, June 20, 2014 9:43 AM >On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: >> Higher RC6 residency is observed using timeout mode instead of EI >> mode. This applies to Broadwell only. >> The difference is particularly noticeable with video playback. >> >> Issue: VIZ-3778 >> Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d >> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> > > >Now that CHV is out, does it apply there too? >Reviewed-by: Ben Widawsky <ben@bwidawsk.net> > >[snip] > > >-- >Ben Widawsky, Intel Open Source Technology Center [TOR:] For CHV, we expect timeout mode will provide benefit on some pre-production steppings and no benefit on the production stepping.
>>From: Ben Widawsky [mailto:ben@bwidawsk.net] >>Sent: Friday, June 20, 2014 9:43 AM >>On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: >>> Higher RC6 residency is observed using timeout mode instead of EI >>> mode. This applies to Broadwell only. >>> The difference is particularly noticeable with video playback. >>> >>> Issue: VIZ-3778 >>> Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d >>> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> >> >> >>Now that CHV is out, does it apply there too? >>Reviewed-by: Ben Widawsky <ben@bwidawsk.net> >> >>[snip] >> >> >>-- >>Ben Widawsky, Intel Open Source Technology Center >[TOR:] For CHV, we expect timeout mode will provide benefit on some pre- >production steppings and no benefit on the production stepping. [TOR:] Can we get this patch merged now that Ben has reviewed?
On Tue, 01 Jul 2014, "O'Rourke, Tom" <Tom.O'Rourke@intel.com> wrote: >>>From: Ben Widawsky [mailto:ben@bwidawsk.net] >>>Sent: Friday, June 20, 2014 9:43 AM >>>On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: >>>> Higher RC6 residency is observed using timeout mode instead of EI >>>> mode. This applies to Broadwell only. >>>> The difference is particularly noticeable with video playback. >>>> >>>> Issue: VIZ-3778 >>>> Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d >>>> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> >>> >>> >>>Now that CHV is out, does it apply there too? >>>Reviewed-by: Ben Widawsky <ben@bwidawsk.net> >>> >>>[snip] >>> >>> >>>-- >>>Ben Widawsky, Intel Open Source Technology Center >>[TOR:] For CHV, we expect timeout mode will provide benefit on some pre- >>production steppings and no benefit on the production stepping. > > [TOR:] Can we get this patch merged now that Ben has reviewed? Pushed to dinq, thanks for the patch and review. BR, Jani.
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 33b2592..0d63abf 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3335,15 +3335,23 @@ static void gen8_enable_rps(struct drm_device *dev) for_each_ring(ring, dev_priv, unused) I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10); I915_WRITE(GEN6_RC_SLEEP, 0); - I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */ + if (IS_BROADWELL(dev)) + I915_WRITE(GEN6_RC6_THRESHOLD, 625); /* 800us/1.28 for TO */ + else + I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */ /* 3: Enable RC6 */ if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE) rc6_mask = GEN6_RC_CTL_RC6_ENABLE; intel_print_rc6_info(dev, rc6_mask); - I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE | - GEN6_RC_CTL_EI_MODE(1) | - rc6_mask); + if (IS_BROADWELL(dev)) + I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE | + GEN7_RC_CTL_TO_MODE | + rc6_mask); + else + I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE | + GEN6_RC_CTL_EI_MODE(1) | + rc6_mask); /* 4 Program defaults and thresholds for RPS*/ I915_WRITE(GEN6_RPNSWREQ,
Higher RC6 residency is observed using timeout mode instead of EI mode. This applies to Broadwell only. The difference is particularly noticeable with video playback. Issue: VIZ-3778 Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> --- drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)