Re: [Intel-gfx] [PATCH v5 6/7] drm/i915/gt: Create per-tile RPS sysfs interfaces

Andrzej Hajda
2022-03-03T10:55:36Z

On 17.02.2022 15:41, Andi Shyti wrote:
> Now tiles have their own sysfs interfaces under the gt/
> directory. Because RPS is a property that can be configured on a
> tile basis, then each tile should have its own interface
>
> The new sysfs structure will have a similar layout for the 4 tile
> case:
>
> /sys/.../card0
> ├── gt
> │   ├── gt0
> │   │   ├── id
> │   │   ├── rc6_enable
> │   │   ├── rc6_residency_ms
> │   │   ├── rps_act_freq_mhz
> │   │   ├── rps_boost_freq_mhz
> │   │   ├── rps_cur_freq_mhz
> │   │   ├── rps_max_freq_mhz
> │   │   ├── rps_min_freq_mhz
> │   │   ├── rps_RP0_freq_mhz
> │   │   ├── rps_RP1_freq_mhz
> │   │  └── rps_RPn_freq_mhz
> . .
> . .
> . .
> │   └── gtN
> │   ├── id
> │   ├── rc6_enable
> │   ├── rc6_residency_ms
> │   ├── rps_act_freq_mhz
> │   ├── rps_boost_freq_mhz
> │   ├── rps_cur_freq_mhz
> │   ├── rps_max_freq_mhz
> │   ├── rps_min_freq_mhz
> │   ├── rps_RP0_freq_mhz
> │   ├── rps_RP1_freq_mhz
> │   └── rps_RPn_freq_mhz
> ├── gt_act_freq_mhz -+
> ├── gt_boost_freq_mhz |
> ├── gt_cur_freq_mhz | Original interface
> ├── gt_max_freq_mhz +─-> kept as existing ABI;
> ├── gt_min_freq_mhz | it points to gt0/
> ├── gt_RP0_freq_mhz |
> ├── gt_RP1_freq_mhz |
> └── gt_RPn_freq_mhz -+
>
> The existing interfaces have been kept in their original location
> to preserve the existing ABI. They act on all the GTs: when
> writing they loop through all the GTs and write the information
> on each interface. When reading they provide the average value
> from all the GTs.
>
> This patch is not really adding exposing new interfaces (new
> ABI) other than adapting the existing one to more tiles. In any
> case this new set of interfaces will be a basic tool for system
> managers and administrators when using i915.
>
> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 276 ++++++++++++++++++++
> drivers/gpu/drm/i915/i915_sysfs.c | 177 -------------
> 2 files changed, 276 insertions(+), 177 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> index 27d93a36894a..8e86b8f675f1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> @@ -14,8 +14,33 @@
> #include "intel_gt_sysfs.h"
> #include "intel_gt_sysfs_pm.h"
> #include "intel_rc6.h"
> +#include "intel_rps.h"
>
> #ifdef CONFIG_PM
> +static int
> +sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr,
> + int (func)(struct intel_gt *gt, u32 val), u32 val)
> +{
> + struct intel_gt *gt;
> + int ret;
> +
> + if (!is_object_gt(&dev->kobj)) {
> + int i;
> + struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
> +
> + for_each_gt(gt, i915, i) {
> + ret = func(gt, val);
> + if (ret)
> + break;
> + }
> + } else {
> + gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + ret = func(gt, val);
> + }
> +
> + return ret;
> +}
> +
> static s64
> sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
> s64 (func)(struct intel_gt *gt))
> @@ -214,7 +239,258 @@ static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj)
> }
> #endif /* CONFIG_PM */
>
> +static s64 __act_freq_mhz_show(struct intel_gt *gt)
> +{
> + return intel_rps_read_actual_frequency(&gt->rps);
> +}
> +
> +static ssize_t act_freq_mhz_show(struct device *dev,
> + struct device_attribute *attr, char *buff)
> +{
> + s64 actual_freq = sysfs_gt_attribute_r_func(dev, attr,
> + __act_freq_mhz_show);
> +
> + return sysfs_emit(buff, "%u\n", (u32) actual_freq);

Again, variable can be just u32.

> +}
> +
> +static s64 __cur_freq_mhz_show(struct intel_gt *gt)
> +{
> + return intel_rps_get_requested_frequency(&gt->rps);
> +}
> +
> +static ssize_t cur_freq_mhz_show(struct device *dev,
> + struct device_attribute *attr, char *buff)
> +{
> + s64 cur_freq = sysfs_gt_attribute_r_func(dev, attr,
> + __cur_freq_mhz_show);
> +
> + return sysfs_emit(buff, "%u\n", (u32) cur_freq);
> +}
> +
> +static s64 __boost_freq_mhz_show(struct intel_gt *gt)
> +{
> + return intel_rps_get_boost_frequency(&gt->rps);
> +}
> +
> +static ssize_t boost_freq_mhz_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buff)
> +{
> + s64 boost_freq = sysfs_gt_attribute_r_func(dev, attr,
> + __boost_freq_mhz_show);
> +
> + return sysfs_emit(buff, "%u\n", (u32) boost_freq);
> +}
> +
> +static int __boost_freq_mhz_store(struct intel_gt *gt, u32 val)
> +{
> + struct intel_rps *rps = &gt->rps;
> + bool boost = false;
> +
> + /* Validate against (static) hardware limits */
> + val = intel_freq_opcode(rps, val);
> + if (val < rps->min_freq || val > rps->max_freq)
> + return -EINVAL;
> +
> + mutex_lock(&rps->lock);
> + if (val != rps->boost_freq) {
> + rps->boost_freq = val;
> + boost = atomic_read(&rps->num_waiters);
> + }
> + mutex_unlock(&rps->lock);
> + if (boost)
> + schedule_work(&rps->work);
> +
> + return 0;

Why not intel_rps_set_boost_frequency?
Why copy/paste from rps_set_boost_freq?

> +}
> +
> +static ssize_t boost_freq_mhz_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buff, size_t count)
> +{
> + ssize_t ret;
> + u32 val;
> +
> + ret = kstrtou32(buff, 0, &val);
> + if (ret)
> + return ret;
> +
> + return sysfs_gt_attribute_w_func(dev, attr,
> + __boost_freq_mhz_store, val) ?: count;
> +}
> +
> +static s64 __vlv_rpe_freq_mhz_show(struct intel_gt *gt)
> +{
> + struct intel_rps *rps = &gt->rps;
> +
> + return intel_gpu_freq(rps, rps->efficient_freq);
> +}
> +
> +static ssize_t vlv_rpe_freq_mhz_show(struct device *dev,
> + struct device_attribute *attr, char *buff)
> +{
> + s64 rpe_freq = sysfs_gt_attribute_r_func(dev, attr,
> + __vlv_rpe_freq_mhz_show);
> +
> + return sysfs_emit(buff, "%d\n", (int) rpe_freq);
> +}
> +
> +static s64 __max_freq_mhz_show(struct intel_gt *gt)
> +{
> + return intel_rps_get_max_frequency(&gt->rps);
> +}
> +
> +static int __set_max_freq(struct intel_gt *gt, u32 val)
> +{
> + return intel_rps_set_max_frequency(&gt->rps, val);
> +}
> +
> +static s64 __min_freq_mhz_show(struct intel_gt *gt)
> +{
> + return intel_rps_get_min_frequency(&gt->rps);
> +}
> +
> +static int __set_min_freq(struct intel_gt *gt, u32 val)
> +{
> + return intel_rps_set_min_frequency(&gt->rps, val);
> +}
> +
> +static ssize_t min_max_freq_mhz_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buff, size_t count);
> +static ssize_t min_max_freq_mhz_show(struct device *dev,
> + struct device_attribute *attr, char *buff);
> +
> +#define INTEL_GT_RPS_SYSFS_ATTR(_name, _mode, _show, _store) \
> + struct device_attribute dev_attr_gt_##_name = __ATTR(gt_##_name, _mode, _show, _store); \
> + struct device_attribute dev_attr_rps_##_name = __ATTR(rps_##_name, _mode, _show, _store)
> +
> +#define INTEL_GT_RPS_SYSFS_ATTR_RO(_name) \
> + INTEL_GT_RPS_SYSFS_ATTR(_name, 0444, _name##_show, NULL)
> +#define INTEL_GT_RPS_SYSFS_ATTR_RW(_name) \
> + INTEL_GT_RPS_SYSFS_ATTR(_name, 0644, _name##_show, _name##_store)
> +
> +static INTEL_GT_RPS_SYSFS_ATTR_RO(act_freq_mhz);
> +static INTEL_GT_RPS_SYSFS_ATTR_RO(cur_freq_mhz);
> +static INTEL_GT_RPS_SYSFS_ATTR_RW(boost_freq_mhz);
> +
> +static DEVICE_ATTR_RO(vlv_rpe_freq_mhz);
> +
> +static ssize_t rps_rp_mhz_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buff);
> +
> +static INTEL_GT_RPS_SYSFS_ATTR(RP0_freq_mhz, 0444, rps_rp_mhz_show, NULL);
> +static INTEL_GT_RPS_SYSFS_ATTR(RP1_freq_mhz, 0444, rps_rp_mhz_show, NULL);
> +static INTEL_GT_RPS_SYSFS_ATTR(RPn_freq_mhz, 0444, rps_rp_mhz_show, NULL);
> +
> +INTEL_GT_RPS_SYSFS_ATTR(max_freq_mhz, 0644, min_max_freq_mhz_show, min_max_freq_mhz_store);
> +INTEL_GT_RPS_SYSFS_ATTR(min_freq_mhz, 0644, min_max_freq_mhz_show, min_max_freq_mhz_store);
> +
> +#define GEN6_ATTR(s) { \
> + &dev_attr_##s##_act_freq_mhz.attr, \
> + &dev_attr_##s##_cur_freq_mhz.attr, \
> + &dev_attr_##s##_boost_freq_mhz.attr, \
> + &dev_attr_##s##_max_freq_mhz.attr, \
> + &dev_attr_##s##_min_freq_mhz.attr, \
> + &dev_attr_##s##_RP0_freq_mhz.attr, \
> + &dev_attr_##s##_RP1_freq_mhz.attr, \
> + &dev_attr_##s##_RPn_freq_mhz.attr, \
> + NULL, \
> + }
> +
> +#define GEN6_RPS_ATTR GEN6_ATTR(rps)
> +#define GEN6_GT_ATTR GEN6_ATTR(gt)
> +
> +static ssize_t min_max_freq_mhz_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buff, size_t count)
> +{
> + long ret;
> + u32 val;
> +
> + ret = kstrtou32(buff, 0, &val);
> + if (ret)
> + return ret;
> +
> + ret = (attr == &dev_attr_gt_min_freq_mhz ||
> + attr == &dev_attr_rps_min_freq_mhz) ?
> + sysfs_gt_attribute_w_func(dev, attr, __set_min_freq, val) :
> + sysfs_gt_attribute_w_func(dev, attr, __set_max_freq, val);
> +
> + return ret ?: count;
> +}
> +
> +static ssize_t min_max_freq_mhz_show(struct device *dev,
> + struct device_attribute *attr, char *buff)
> +{
> + s64 val;
> +
> + val = (attr == &dev_attr_gt_min_freq_mhz ||
> + attr == &dev_attr_rps_min_freq_mhz) ?
> + sysfs_gt_attribute_r_func(dev, attr, __min_freq_mhz_show) :
> + sysfs_gt_attribute_r_func(dev, attr, __max_freq_mhz_show);
> +
> + return sysfs_emit(buff, "%u\n", (u32) val);
> +}
> +
> +/* For now we have a static number of RP states */
> +static ssize_t rps_rp_mhz_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buff)
> +{
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + struct intel_rps *rps = &gt->rps;
> + u32 val;
> +
> + if (attr == &dev_attr_gt_RP0_freq_mhz ||
> + attr == &dev_attr_rps_RP0_freq_mhz) {
> + val = intel_rps_get_rp0_frequency(rps);
> + } else if (attr == &dev_attr_gt_RP1_freq_mhz ||
> + attr == &dev_attr_rps_RP1_freq_mhz) {
> + val = intel_rps_get_rp1_frequency(rps);
> + } else if (attr == &dev_attr_gt_RPn_freq_mhz ||
> + attr == &dev_attr_rps_RPn_freq_mhz) {
> + val = intel_rps_get_rpn_frequency(rps);
> + } else {
> + GEM_WARN_ON(1);
> + return -ENODEV;

I guess this is not necessary, otherwise similar path should be in other
helpers.

Regards
Andrzej

> + }
> +
> + return sysfs_emit(buff, "%d\n", val);
> +}
> +
> +static const struct attribute * const gen6_rps_attrs[] = GEN6_RPS_ATTR;
> +static const struct attribute * const gen6_gt_attrs[] = GEN6_GT_ATTR;
> +
> +static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj,
> + const struct attribute * const *attrs)
> +{
> + int ret;
> +
> + if (GRAPHICS_VER(gt->i915) < 6)
> + return 0;
> +
> + ret = sysfs_create_files(kobj, attrs);
> + if (ret)
> + return ret;
> +
> + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915))
> + ret = sysfs_create_file(kobj, &dev_attr_vlv_rpe_freq_mhz.attr);
> +
> + return ret;
> +}
> +
> void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
> {
> + int ret;
> +
> intel_sysfs_rc6_init(gt, kobj);
> +
> + ret = is_object_gt(kobj) ?
> + intel_sysfs_rps_init(gt, kobj, gen6_rps_attrs) :
> + intel_sysfs_rps_init(gt, kobj, gen6_gt_attrs);
> + if (ret)
> + drm_warn(&gt->i915->drm,
> + "failed to create gt%u RPS sysfs files", gt->info.id);
> }
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index b08a959e5ccc..0a0057940718 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -156,171 +156,6 @@ static const struct bin_attribute dpf_attrs_1 = {
> .private = (void *)1
> };
>
> -static ssize_t gt_act_freq_mhz_show(struct device *kdev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct drm_i915_private *i915 = kdev_minor_to_i915(kdev);
> - struct intel_rps *rps = &to_gt(i915)->rps;
> -
> - return sysfs_emit(buf, "%d\n", intel_rps_read_actual_frequency(rps));
> -}
> -
> -static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct drm_i915_private *i915 = kdev_minor_to_i915(kdev);
> - struct intel_rps *rps = &to_gt(i915)->rps;
> -
> - return sysfs_emit(buf, "%d\n", intel_rps_get_requested_frequency(rps));
> -}
> -
> -static ssize_t gt_boost_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
> -{
> - struct drm_i915_private *i915 = kdev_minor_to_i915(kdev);
> - struct intel_rps *rps = &to_gt(i915)->rps;
> -
> - return sysfs_emit(buf, "%d\n", intel_rps_get_boost_frequency(rps));
> -}
> -
> -static ssize_t gt_boost_freq_mhz_store(struct device *kdev,
> - struct device_attribute *attr,
> - const char *buf, size_t count)
> -{
> - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
> - struct intel_rps *rps = &to_gt(dev_priv)->rps;
> - ssize_t ret;
> - u32 val;
> -
> - ret = kstrtou32(buf, 0, &val);
> - if (ret)
> - return ret;
> -
> - ret = intel_rps_set_boost_frequency(rps, val);
> -
> - return ret ?: count;
> -}
> -
> -static ssize_t vlv_rpe_freq_mhz_show(struct device *kdev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
> - struct intel_rps *rps = &to_gt(dev_priv)->rps;
> -
> - return sysfs_emit(buf, "%d\n", intel_gpu_freq(rps, rps->efficient_freq));
> -}
> -
> -static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
> -{
> - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
> - struct intel_gt *gt = to_gt(dev_priv);
> - struct intel_rps *rps = &gt->rps;
> -
> - return sysfs_emit(buf, "%d\n", intel_rps_get_max_frequency(rps));
> -}
> -
> -static ssize_t gt_max_freq_mhz_store(struct device *kdev,
> - struct device_attribute *attr,
> - const char *buf, size_t count)
> -{
> - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
> - struct intel_gt *gt = to_gt(dev_priv);
> - struct intel_rps *rps = &gt->rps;
> - ssize_t ret;
> - u32 val;
> -
> - ret = kstrtou32(buf, 0, &val);
> - if (ret)
> - return ret;
> -
> - ret = intel_rps_set_max_frequency(rps, val);
> -
> - return ret ?: count;
> -}
> -
> -static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
> -{
> - struct drm_i915_private *i915 = kdev_minor_to_i915(kdev);
> - struct intel_gt *gt = to_gt(i915);
> - struct intel_rps *rps = &gt->rps;
> -
> - return sysfs_emit(buf, "%d\n", intel_rps_get_min_frequency(rps));
> -}
> -
> -static ssize_t gt_min_freq_mhz_store(struct device *kdev,
> - struct device_attribute *attr,
> - const char *buf, size_t count)
> -{
> - struct drm_i915_private *i915 = kdev_minor_to_i915(kdev);
> - struct intel_rps *rps = &to_gt(i915)->rps;
> - ssize_t ret;
> - u32 val;
> -
> - ret = kstrtou32(buf, 0, &val);
> - if (ret)
> - return ret;
> -
> - ret = intel_rps_set_min_frequency(rps, val);
> -
> - return ret ?: count;
> -}
> -
> -static DEVICE_ATTR_RO(gt_act_freq_mhz);
> -static DEVICE_ATTR_RO(gt_cur_freq_mhz);
> -static DEVICE_ATTR_RW(gt_boost_freq_mhz);
> -static DEVICE_ATTR_RW(gt_max_freq_mhz);
> -static DEVICE_ATTR_RW(gt_min_freq_mhz);
> -
> -static DEVICE_ATTR_RO(vlv_rpe_freq_mhz);
> -
> -static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf);
> -static DEVICE_ATTR(gt_RP0_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
> -static DEVICE_ATTR(gt_RP1_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
> -static DEVICE_ATTR(gt_RPn_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
> -
> -/* For now we have a static number of RP states */
> -static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
> -{
> - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
> - struct intel_rps *rps = &to_gt(dev_priv)->rps;
> - u32 val;
> -
> - if (attr == &dev_attr_gt_RP0_freq_mhz)
> - val = intel_rps_get_rp0_frequency(rps);
> - else if (attr == &dev_attr_gt_RP1_freq_mhz)
> - val = intel_rps_get_rp1_frequency(rps);
> - else if (attr == &dev_attr_gt_RPn_freq_mhz)
> - val = intel_rps_get_rpn_frequency(rps);
> - else
> - BUG();
> -
> - return sysfs_emit(buf, "%d\n", val);
> -}
> -
> -static const struct attribute * const gen6_attrs[] = {
> - &dev_attr_gt_act_freq_mhz.attr,
> - &dev_attr_gt_cur_freq_mhz.attr,
> - &dev_attr_gt_boost_freq_mhz.attr,
> - &dev_attr_gt_max_freq_mhz.attr,
> - &dev_attr_gt_min_freq_mhz.attr,
> - &dev_attr_gt_RP0_freq_mhz.attr,
> - &dev_attr_gt_RP1_freq_mhz.attr,
> - &dev_attr_gt_RPn_freq_mhz.attr,
> - NULL,
> -};
> -
> -static const struct attribute * const vlv_attrs[] = {
> - &dev_attr_gt_act_freq_mhz.attr,
> - &dev_attr_gt_cur_freq_mhz.attr,
> - &dev_attr_gt_boost_freq_mhz.attr,
> - &dev_attr_gt_max_freq_mhz.attr,
> - &dev_attr_gt_min_freq_mhz.attr,
> - &dev_attr_gt_RP0_freq_mhz.attr,
> - &dev_attr_gt_RP1_freq_mhz.attr,
> - &dev_attr_gt_RPn_freq_mhz.attr,
> - &dev_attr_vlv_rpe_freq_mhz.attr,
> - NULL,
> -};
> -
> #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR)
>
> static ssize_t error_state_read(struct file *filp, struct kobject *kobj,
> @@ -411,14 +246,6 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv)
> }
> }
>
> - ret = 0;
> - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> - ret = sysfs_create_files(&kdev->kobj, vlv_attrs);
> - else if (GRAPHICS_VER(dev_priv) >= 6)
> - ret = sysfs_create_files(&kdev->kobj, gen6_attrs);
> - if (ret)
> - drm_err(&dev_priv->drm, "RPS sysfs setup failed\n");
> -
> dev_priv->sysfs_gt = i915_setup_gt_sysfs(&kdev->kobj);
> if (!dev_priv->sysfs_gt)
> drm_warn(&dev_priv->drm,
> @@ -435,10 +262,6 @@ void i915_teardown_sysfs(struct drm_i915_private *dev_priv)
>
> i915_teardown_error_capture(kdev);
>
> - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> - sysfs_remove_files(&kdev->kobj, vlv_attrs);
> - else
> - sysfs_remove_files(&kdev->kobj, gen6_attrs);
> device_remove_bin_file(kdev, &dpf_attrs_1);
> device_remove_bin_file(kdev, &dpf_attrs);
> }