| Message ID | 1390969547-1018-2-git-send-email-benjamin.widawsky@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> On Wed, Jan 29, 2014 at 2:25 AM, Ben Widawsky <benjamin.widawsky@intel.com> wrote: > At one time, we though all future platforms would have the deeper RC6 > states. As it turned out, they killed it after Ivybridge, and began > using other means to achieve the power savings (the stuff we need to get > to PC7+). > > The enable function was left in a weird state of odd corner cases as a > result. Since the future is now, and we also have some insight into > what's currently the future, we have an opportunity to simplify, and > future proof the function. > > NOTE: VLV will be addressed in a subsequent patch. This patch was trying > not to change functionality. > > NOTE2: All callers sanitize the return value anyway, so this patch is > simply to have the code make a bit more sense. > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/intel_pm.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 53d64bb..bcbdac2 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3161,14 +3161,10 @@ int intel_enable_rc6(const struct drm_device *dev) > if (INTEL_INFO(dev)->gen == 5) > return 0; > > - if (IS_HASWELL(dev)) > - return INTEL_RC6_ENABLE; > - > - /* snb/ivb have more than one rc6 state. */ > - if (INTEL_INFO(dev)->gen == 6) > + if (IS_IVYBRIDGE(dev) || IS_VALLEYVIEW(dev)) > + return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE); > + else > return INTEL_RC6_ENABLE; > - > - return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE); > } > > static void gen6_enable_rps_interrupts(struct drm_device *dev) > -- > 1.8.5.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky > <benjamin.widawsky@intel.com <mailto:benjamin.widawsky@intel.com>> wrote: > > At one time, we though all future platforms would have the deeper RC6 > states. As it turned out, they killed it after Ivybridge, and began > using other means to achieve the power savings (the stuff we need to get > to PC7+). > > The enable function was left in a weird state of odd corner cases as a > result. Since the future is now, and we also have some insight into > what's currently the future, we have an opportunity to simplify, and > future proof the function. > > NOTE: VLV will be addressed in a subsequent patch. This patch was trying > not to change functionality. > > NOTE2: All callers sanitize the return value anyway, so this patch is > simply to have the code make a bit more sense. > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net <mailto:ben@bwidawsk.net>> > --- > drivers/gpu/drm/i915/intel_pm.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c > index 53d64bb..bcbdac2 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3161,14 +3161,10 @@ int intel_enable_rc6(const struct drm_device > *dev) > if (INTEL_INFO(dev)->gen == 5) > return 0; > > - if (IS_HASWELL(dev)) > - return INTEL_RC6_ENABLE; > - > - /* snb/ivb have more than one rc6 state. */ > - if (INTEL_INFO(dev)->gen == 6) > + if (IS_IVYBRIDGE(dev) || IS_VALLEYVIEW(dev)) > + return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE); > + else > return INTEL_RC6_ENABLE; > - > - return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE); > } > > static void gen6_enable_rps_interrupts(struct drm_device *dev) > -- > 1.8.5.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org <mailto:Intel-gfx@lists.freedesktop.org> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > Reviewed-by: Deepak S <deepak.s@intel.com>
Here is the v2 of the RC6, and RPS enabling for Broadwell. With the last patch in the series RC6 works without hangs (in limited testing). Without the last patch, the first batch seems to always hang. I don't think there any actual fixes over the last version. I changed enough of the RPS code to not feel comfortable putting Rodrigo's r-b back on. I had a couple of small tweaks requested by Rodrigo which should be in there. If they aren't in there, it was a mistake. Summary: 1-5: All new cleanups new the RPS code. 6: Still in flux default nominal state 7: Pull Jeff's earlier fix around reset into gen8 with a cleanup 8-9: Enable RPS 10-11: Enable RC6 Thanks to Ken for giving this a quick test and allowing me to work from home :-) Ben Widawsky (11): drm/i915: Reorganize the overclock code drm/i915: Fix coding style for RPS drm/i915: Rename and comment all the RPS *stuff* drm/i915: Remove extraneous MMIO for RPS drm/i915: remove rps local variables drm/i915/bdw: Set initial rps freq to nominal drm/i915/bdw: Extract rp_state_caps logic drm/i915/bdw: RPS frequency bits are the same as HSW drm/i915/bdw: Implement a basic PM interrupt handler drm/i915/bdw: Enable RC6 drm/i915/bdw: Ensure a context is loaded before RC6 drivers/gpu/drm/i915/i915_debugfs.c | 26 ++--- drivers/gpu/drm/i915/i915_drv.h | 31 +++-- drivers/gpu/drm/i915/i915_gem.c | 12 ++ drivers/gpu/drm/i915/i915_irq.c | 108 +++++++++++++++--- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/i915_sysfs.c | 81 ++++++-------- drivers/gpu/drm/i915/intel_display.c | 5 + drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_pm.c | 211 +++++++++++++++++++++-------------- 9 files changed, 308 insertions(+), 169 deletions(-)
Ben Widawsky (12): drm/i915: Reorganize the overclock code drm/i915: Fix coding style for RPS drm/i915: Store the HW min frequency as min_freq drm/i915: Rename and comment all the RPS *stuff* drm/i915: Remove extraneous MMIO for RPS drm/i915: remove rps local variables drm/i915/bdw: Set initial rps freq to RP0 drm/i915/bdw: Extract rp_state_caps logic drm/i915/bdw: RPS frequency bits are the same as HSW drm/i915/bdw: Implement a basic PM interrupt handler drm/i915/bdw: Ensure a context is loaded before RC6 drm/i915/bdw: Enable RC6 drivers/gpu/drm/i915/i915_debugfs.c | 26 ++--- drivers/gpu/drm/i915/i915_drv.c | 4 +- drivers/gpu/drm/i915/i915_drv.h | 25 ++-- drivers/gpu/drm/i915/i915_gem.c | 10 ++ drivers/gpu/drm/i915/i915_irq.c | 109 +++++++++++++++--- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/i915_sysfs.c | 81 ++++++------- drivers/gpu/drm/i915/intel_display.c | 5 + drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_pm.c | 213 +++++++++++++++++++++-------------- 10 files changed, 306 insertions(+), 170 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 53d64bb..bcbdac2 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3161,14 +3161,10 @@ int intel_enable_rc6(const struct drm_device *dev) if (INTEL_INFO(dev)->gen == 5) return 0; - if (IS_HASWELL(dev)) - return INTEL_RC6_ENABLE; - - /* snb/ivb have more than one rc6 state. */ - if (INTEL_INFO(dev)->gen == 6) + if (IS_IVYBRIDGE(dev) || IS_VALLEYVIEW(dev)) + return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE); + else return INTEL_RC6_ENABLE; - - return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE); } static void gen6_enable_rps_interrupts(struct drm_device *dev)
At one time, we though all future platforms would have the deeper RC6 states. As it turned out, they killed it after Ivybridge, and began using other means to achieve the power savings (the stuff we need to get to PC7+). The enable function was left in a weird state of odd corner cases as a result. Since the future is now, and we also have some insight into what's currently the future, we have an opportunity to simplify, and future proof the function. NOTE: VLV will be addressed in a subsequent patch. This patch was trying not to change functionality. NOTE2: All callers sanitize the return value anyway, so this patch is simply to have the code make a bit more sense. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/intel_pm.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)