[Intel-gfx] [PATCH] drm/i915: Do not export RC6p and RC6pp if they don't exist
Rodrigo Vivi
rodrigo.vivi at gmail.com
Wed Oct 1 22:54:26 CEST 2014
On Wed, Oct 1, 2014 at 1:08 AM, Daniel Vetter <daniel at 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 at intel.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at 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 at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
More information about the Intel-gfx
mailing list