[Intel-gfx] [PATCH] drm/i915: Do not export RC6p and RC6pp if they don't exist

[Paulo Zanoni]

2014-10-07 10:36 GMT-03:00 Rodrigo Vivi <rodrigo.vivi at intel.com>:
> Avoid to expose RC6 and RC6pp to the platforms that doesn't support it.
> So powertop can be changed to show RC6p and RC6pp only on the platforms
> they are available.
>
> v2: Simplify by merging RC6p and RC6pp groups and respect the spec that
> mentions deep and deepest RC6 until BDW although they keep disabled
> by default.
>
> v3: Remove unecessary space.
>
> v4: RC6p and RC6pp is only for SNB and IVB; unify debug msg and use
>     has_rc6p() on sanitize options instead of is gen 6 and ivb.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=84524
> Cc: Josh Triplett <josh.triplett at intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  2 ++
>  drivers/gpu/drm/i915/i915_sysfs.c | 22 +++++++++++++++++++---
>  drivers/gpu/drm/i915/intel_pm.c   | 15 ++++++++++-----
>  3 files changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5ba5524..d5412af 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2185,6 +2185,8 @@ 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)          (HAS_RC6(dev) && INTEL_INFO(dev)->gen < 7)

I know I'm the one who asked you to use "< 7", but this is wrong since
IVB is gen7... I'm sorry for inducing the mistake here :(

>
>  #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..4a5af69 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,17 @@ 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,
> +       &dev_attr_rc6pp_residency_ms.attr,
> +       NULL
> +};
> +
> +static struct attribute_group rc6p_attr_group = {
> +       .name = power_group_name,
> +       .attrs =  rc6p_attrs
> +};
>  #endif
>
>  static int l3_access_valid(struct drm_device *dev, loff_t offset)
> @@ -595,12 +604,18 @@ 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");
> +       }
>  #endif
>         if (HAS_L3_DPF(dev)) {
>                 ret = device_create_bin_file(dev->primary->kdev, &dpf_attrs);
> @@ -640,5 +655,6 @@ 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);
>  #endif
>  }
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6a670ad..90eafa7 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3629,10 +3629,15 @@ 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");
> +       if (HAS_RC6p(dev))
> +               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");
> +
> +       else
> +               DRM_DEBUG_KMS("Enabling RC6 states: RC6 %s\n",
> +                             (mode & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off");
>  }
>
>  static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
> @@ -3649,7 +3654,7 @@ static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
>         if (enable_rc6 >= 0) {
>                 int mask;
>
> -               if (INTEL_INFO(dev)->gen == 6 || IS_IVYBRIDGE(dev))
> +               if (HAS_RC6p(dev))
>                         mask = INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE |
>                                INTEL_RC6pp_ENABLE;
>                 else
> --
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni