| Message ID | 1412089233-3632-1-git-send-email-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Sep 30, 2014 at 08:00:33AM -0700, Rodrigo Vivi wrote: > Avoid to expose RC6 and RC6pp to the platforms that doesn't support it. > Although this doesn't really change powertop behaviour as described on the > request. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=84524 > Cc: Josh Triplett <josh.triplett@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 3 +++ > drivers/gpu/drm/i915/i915_sysfs.c | 39 ++++++++++++++++++++++++++++++++++++--- > drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++---- > 3 files changed, 47 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index d10b417..54b2aac 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2185,6 +2185,9 @@ struct drm_i915_cmd_table { > #define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev)) > #define HAS_RUNTIME_PM(dev) (IS_GEN6(dev) || IS_HASWELL(dev) || \ > IS_BROADWELL(dev) || IS_VALLEYVIEW(dev)) > +#define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6) > +#define HAS_RC6p(dev) (IS_GEN6(dev) || IS_IVYBRIDGE(dev)) > +#define HAS_RC6pp(dev) IS_GEN6(dev) > > #define INTEL_PCH_DEVICE_ID_MASK 0xff00 > #define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00 > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c > index 503847f..879e889 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -139,8 +139,6 @@ static DEVICE_ATTR(rc6pp_residency_ms, S_IRUGO, show_rc6pp_ms, NULL); > static struct attribute *rc6_attrs[] = { > &dev_attr_rc6_enable.attr, > &dev_attr_rc6_residency_ms.attr, > - &dev_attr_rc6p_residency_ms.attr, > - &dev_attr_rc6pp_residency_ms.attr, > NULL > }; > > @@ -148,6 +146,26 @@ static struct attribute_group rc6_attr_group = { > .name = power_group_name, > .attrs = rc6_attrs > }; > + > +static struct attribute *rc6p_attrs[] = { > + &dev_attr_rc6p_residency_ms.attr, > + NULL > +}; > + > +static struct attribute_group rc6p_attr_group = { > + .name = power_group_name, > + .attrs = rc6p_attrs > +}; > + > +static struct attribute *rc6pp_attrs[] = { > + &dev_attr_rc6pp_residency_ms.attr, > + NULL > +}; > + > +static struct attribute_group rc6pp_attr_group = { > + .name = power_group_name, > + .attrs = rc6pp_attrs > +}; > #endif > > static int l3_access_valid(struct drm_device *dev, loff_t offset) > @@ -595,12 +613,25 @@ void i915_setup_sysfs(struct drm_device *dev) > int ret; > > #ifdef CONFIG_PM > - if (INTEL_INFO(dev)->gen >= 6) { > + if (HAS_RC6(dev)) { > ret = sysfs_merge_group(&dev->primary->kdev->kobj, > &rc6_attr_group); > if (ret) > DRM_ERROR("RC6 residency sysfs setup failed\n"); > } > + if (HAS_RC6p(dev)) { > + ret = sysfs_merge_group(&dev->primary->kdev->kobj, > + &rc6p_attr_group); > + if (ret) > + DRM_ERROR("RC6p residency sysfs setup failed\n"); > + } > + > + if (HAS_RC6pp(dev)) { > + ret = sysfs_merge_group(&dev->primary->kdev->kobj, > + &rc6pp_attr_group); > + if (ret) > + DRM_ERROR("RC6pp residency sysfs setup failed\n"); > + } > #endif > if (HAS_L3_DPF(dev)) { > ret = device_create_bin_file(dev->primary->kdev, &dpf_attrs); > @@ -640,5 +671,7 @@ void i915_teardown_sysfs(struct drm_device *dev) > device_remove_bin_file(dev->primary->kdev, &dpf_attrs); > #ifdef CONFIG_PM > sysfs_unmerge_group(&dev->primary->kdev->kobj, &rc6_attr_group); > + sysfs_unmerge_group(&dev->primary->kdev->kobj, &rc6p_attr_group); > + sysfs_unmerge_group(&dev->primary->kdev->kobj, &rc6pp_attr_group); I think we could do just one additional group here for both rc6p and rc6pp, simplifies the code a bit. -Daniel > #endif > } > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 7553e47..fa1227e 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3632,10 +3632,14 @@ static void intel_print_rc6_info(struct drm_device *dev, u32 mode) > else > mode = 0; > } > - DRM_DEBUG_KMS("Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n", > - (mode & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off", > - (mode & GEN6_RC_CTL_RC6p_ENABLE) ? "on" : "off", > - (mode & GEN6_RC_CTL_RC6pp_ENABLE) ? "on" : "off"); > + DRM_DEBUG_KMS("Enabling RC6 states: RC6 %s\n", > + (mode & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off"); > + if (HAS_RC6p(dev)) > + DRM_DEBUG_KMS("Enabling RC6 states: RC6p %s\n", > + (mode & GEN6_RC_CTL_RC6p_ENABLE) ? "on" : "off"); > + if (HAS_RC6pp(dev)) > + DRM_DEBUG_KMS("Enabling RC6 states: RC6pp %s\n", > + (mode & GEN6_RC_CTL_RC6pp_ENABLE) ? "on" : "off"); > } > > static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6) > -- > 1.9.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Oct 1, 2014 at 1:08 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Tue, Sep 30, 2014 at 08:00:33AM -0700, Rodrigo Vivi wrote: >> Avoid to expose RC6 and RC6pp to the platforms that doesn't support it. >> Although this doesn't really change powertop behaviour as described on the >> request. >> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=84524 >> Cc: Josh Triplett <josh.triplett@intel.com> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 3 +++ >> drivers/gpu/drm/i915/i915_sysfs.c | 39 ++++++++++++++++++++++++++++++++++++--- >> drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++---- >> 3 files changed, 47 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index d10b417..54b2aac 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -2185,6 +2185,9 @@ struct drm_i915_cmd_table { >> #define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev)) >> #define HAS_RUNTIME_PM(dev) (IS_GEN6(dev) || IS_HASWELL(dev) || \ >> IS_BROADWELL(dev) || IS_VALLEYVIEW(dev)) >> +#define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6) >> +#define HAS_RC6p(dev) (IS_GEN6(dev) || IS_IVYBRIDGE(dev)) >> +#define HAS_RC6pp(dev) IS_GEN6(dev) >> >> #define INTEL_PCH_DEVICE_ID_MASK 0xff00 >> #define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00 >> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c >> index 503847f..879e889 100644 >> --- a/drivers/gpu/drm/i915/i915_sysfs.c >> +++ b/drivers/gpu/drm/i915/i915_sysfs.c >> @@ -139,8 +139,6 @@ static DEVICE_ATTR(rc6pp_residency_ms, S_IRUGO, show_rc6pp_ms, NULL); >> static struct attribute *rc6_attrs[] = { >> &dev_attr_rc6_enable.attr, >> &dev_attr_rc6_residency_ms.attr, >> - &dev_attr_rc6p_residency_ms.attr, >> - &dev_attr_rc6pp_residency_ms.attr, >> NULL >> }; >> >> @@ -148,6 +146,26 @@ static struct attribute_group rc6_attr_group = { >> .name = power_group_name, >> .attrs = rc6_attrs >> }; >> + >> +static struct attribute *rc6p_attrs[] = { >> + &dev_attr_rc6p_residency_ms.attr, >> + NULL >> +}; >> + >> +static struct attribute_group rc6p_attr_group = { >> + .name = power_group_name, >> + .attrs = rc6p_attrs >> +}; >> + >> +static struct attribute *rc6pp_attrs[] = { >> + &dev_attr_rc6pp_residency_ms.attr, >> + NULL >> +}; >> + >> +static struct attribute_group rc6pp_attr_group = { >> + .name = power_group_name, >> + .attrs = rc6pp_attrs >> +}; >> #endif >> >> static int l3_access_valid(struct drm_device *dev, loff_t offset) >> @@ -595,12 +613,25 @@ void i915_setup_sysfs(struct drm_device *dev) >> int ret; >> >> #ifdef CONFIG_PM >> - if (INTEL_INFO(dev)->gen >= 6) { >> + if (HAS_RC6(dev)) { >> ret = sysfs_merge_group(&dev->primary->kdev->kobj, >> &rc6_attr_group); >> if (ret) >> DRM_ERROR("RC6 residency sysfs setup failed\n"); >> } >> + if (HAS_RC6p(dev)) { >> + ret = sysfs_merge_group(&dev->primary->kdev->kobj, >> + &rc6p_attr_group); >> + if (ret) >> + DRM_ERROR("RC6p residency sysfs setup failed\n"); >> + } >> + >> + if (HAS_RC6pp(dev)) { >> + ret = sysfs_merge_group(&dev->primary->kdev->kobj, >> + &rc6pp_attr_group); >> + if (ret) >> + DRM_ERROR("RC6pp residency sysfs setup failed\n"); >> + } >> #endif >> if (HAS_L3_DPF(dev)) { >> ret = device_create_bin_file(dev->primary->kdev, &dpf_attrs); >> @@ -640,5 +671,7 @@ void i915_teardown_sysfs(struct drm_device *dev) >> device_remove_bin_file(dev->primary->kdev, &dpf_attrs); >> #ifdef CONFIG_PM >> sysfs_unmerge_group(&dev->primary->kdev->kobj, &rc6_attr_group); >> + sysfs_unmerge_group(&dev->primary->kdev->kobj, &rc6p_attr_group); >> + sysfs_unmerge_group(&dev->primary->kdev->kobj, &rc6pp_attr_group); > > I think we could do just one additional group here for both rc6p and > rc6pp, simplifies the code a bit. if both snb and ivb have both deep and deepest I agree... But I defined above rc6pp only on snb and rc6p on snb and ivb... am I wrong? > -Daniel > >> #endif >> } >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index 7553e47..fa1227e 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -3632,10 +3632,14 @@ static void intel_print_rc6_info(struct drm_device *dev, u32 mode) >> else >> mode = 0; >> } >> - DRM_DEBUG_KMS("Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n", >> - (mode & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off", >> - (mode & GEN6_RC_CTL_RC6p_ENABLE) ? "on" : "off", >> - (mode & GEN6_RC_CTL_RC6pp_ENABLE) ? "on" : "off"); >> + DRM_DEBUG_KMS("Enabling RC6 states: RC6 %s\n", >> + (mode & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off"); >> + if (HAS_RC6p(dev)) >> + DRM_DEBUG_KMS("Enabling RC6 states: RC6p %s\n", >> + (mode & GEN6_RC_CTL_RC6p_ENABLE) ? "on" : "off"); >> + if (HAS_RC6pp(dev)) >> + DRM_DEBUG_KMS("Enabling RC6 states: RC6pp %s\n", >> + (mode & GEN6_RC_CTL_RC6pp_ENABLE) ? "on" : "off"); >> } >> >> static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6) >> -- >> 1.9.3 >> >> _______________________________________________ >> 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
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index d10b417..54b2aac 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2185,6 +2185,9 @@ struct drm_i915_cmd_table { #define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev)) #define HAS_RUNTIME_PM(dev) (IS_GEN6(dev) || IS_HASWELL(dev) || \ IS_BROADWELL(dev) || IS_VALLEYVIEW(dev)) +#define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6) +#define HAS_RC6p(dev) (IS_GEN6(dev) || IS_IVYBRIDGE(dev)) +#define HAS_RC6pp(dev) IS_GEN6(dev) #define INTEL_PCH_DEVICE_ID_MASK 0xff00 #define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00 diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c index 503847f..879e889 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.c +++ b/drivers/gpu/drm/i915/i915_sysfs.c @@ -139,8 +139,6 @@ static DEVICE_ATTR(rc6pp_residency_ms, S_IRUGO, show_rc6pp_ms, NULL); static struct attribute *rc6_attrs[] = { &dev_attr_rc6_enable.attr, &dev_attr_rc6_residency_ms.attr, - &dev_attr_rc6p_residency_ms.attr, - &dev_attr_rc6pp_residency_ms.attr, NULL }; @@ -148,6 +146,26 @@ static struct attribute_group rc6_attr_group = { .name = power_group_name, .attrs = rc6_attrs }; + +static struct attribute *rc6p_attrs[] = { + &dev_attr_rc6p_residency_ms.attr, + NULL +}; + +static struct attribute_group rc6p_attr_group = { + .name = power_group_name, + .attrs = rc6p_attrs +}; + +static struct attribute *rc6pp_attrs[] = { + &dev_attr_rc6pp_residency_ms.attr, + NULL +}; + +static struct attribute_group rc6pp_attr_group = { + .name = power_group_name, + .attrs = rc6pp_attrs +}; #endif static int l3_access_valid(struct drm_device *dev, loff_t offset) @@ -595,12 +613,25 @@ void i915_setup_sysfs(struct drm_device *dev) int ret; #ifdef CONFIG_PM - if (INTEL_INFO(dev)->gen >= 6) { + if (HAS_RC6(dev)) { ret = sysfs_merge_group(&dev->primary->kdev->kobj, &rc6_attr_group); if (ret) DRM_ERROR("RC6 residency sysfs setup failed\n"); } + if (HAS_RC6p(dev)) { + ret = sysfs_merge_group(&dev->primary->kdev->kobj, + &rc6p_attr_group); + if (ret) + DRM_ERROR("RC6p residency sysfs setup failed\n"); + } + + if (HAS_RC6pp(dev)) { + ret = sysfs_merge_group(&dev->primary->kdev->kobj, + &rc6pp_attr_group); + if (ret) + DRM_ERROR("RC6pp residency sysfs setup failed\n"); + } #endif if (HAS_L3_DPF(dev)) { ret = device_create_bin_file(dev->primary->kdev, &dpf_attrs); @@ -640,5 +671,7 @@ void i915_teardown_sysfs(struct drm_device *dev) device_remove_bin_file(dev->primary->kdev, &dpf_attrs); #ifdef CONFIG_PM sysfs_unmerge_group(&dev->primary->kdev->kobj, &rc6_attr_group); + sysfs_unmerge_group(&dev->primary->kdev->kobj, &rc6p_attr_group); + sysfs_unmerge_group(&dev->primary->kdev->kobj, &rc6pp_attr_group); #endif } diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 7553e47..fa1227e 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3632,10 +3632,14 @@ static void intel_print_rc6_info(struct drm_device *dev, u32 mode) else mode = 0; } - DRM_DEBUG_KMS("Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n", - (mode & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off", - (mode & GEN6_RC_CTL_RC6p_ENABLE) ? "on" : "off", - (mode & GEN6_RC_CTL_RC6pp_ENABLE) ? "on" : "off"); + DRM_DEBUG_KMS("Enabling RC6 states: RC6 %s\n", + (mode & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off"); + if (HAS_RC6p(dev)) + DRM_DEBUG_KMS("Enabling RC6 states: RC6p %s\n", + (mode & GEN6_RC_CTL_RC6p_ENABLE) ? "on" : "off"); + if (HAS_RC6pp(dev)) + DRM_DEBUG_KMS("Enabling RC6 states: RC6pp %s\n", + (mode & GEN6_RC_CTL_RC6pp_ENABLE) ? "on" : "off"); } static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
Avoid to expose RC6 and RC6pp to the platforms that doesn't support it. Although this doesn't really change powertop behaviour as described on the request. References: https://bugs.freedesktop.org/show_bug.cgi?id=84524 Cc: Josh Triplett <josh.triplett@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/i915_sysfs.c | 39 ++++++++++++++++++++++++++++++++++++--- drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++---- 3 files changed, 47 insertions(+), 7 deletions(-)