* [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files @ 2020-02-14 11:03 Andi Shyti 2020-02-14 12:51 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gt: make a gt sysfs group and move power management files (rev3) Patchwork ` (5 more replies) 0 siblings, 6 replies; 13+ messages in thread From: Andi Shyti @ 2020-02-14 11:03 UTC (permalink / raw) To: Intel GFX The GT has its own properties and in sysfs they should be grouped in the 'gt/' directory. Create the 'gt/' directory in sysfs and move the power management related files. Signed-off-by: Andi Shyti <andi.shyti@intel.com> --- Hi, this version has some more substantial differences, nothing that changes the wanted behavior, though. Thanks Chris, Jani and Tvrtko for the reviews, Andi Changelog: ========== v2 -> v3: - fix some cleanups that I forgot in the previous patch - fix reference pointers to the gt structure - and many other small changes here and there. v1 -> v2: - keep the existing files as they are - use "intel_gt_*" as prefix instead of "sysfs_*" drivers/gpu/drm/i915/Makefile | 4 +- drivers/gpu/drm/i915/gt/intel_gt.c | 3 + drivers/gpu/drm/i915/gt/intel_gt_types.h | 1 + drivers/gpu/drm/i915/gt/sysfs_gt.c | 85 +++++ drivers/gpu/drm/i915/gt/sysfs_gt.h | 22 ++ drivers/gpu/drm/i915/gt/sysfs_gt_pm.c | 446 +++++++++++++++++++++++ drivers/gpu/drm/i915/gt/sysfs_gt_pm.h | 17 + drivers/gpu/drm/i915/i915_sysfs.c | 375 +------------------ drivers/gpu/drm/i915/i915_sysfs.h | 3 + 9 files changed, 581 insertions(+), 375 deletions(-) create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt.c create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt.h create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt_pm.c create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt_pm.h diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 49eed50ef0a4..3b81c8a96c06 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -106,7 +106,9 @@ gt-y += \ gt/intel_rps.o \ gt/intel_sseu.o \ gt/intel_timeline.o \ - gt/intel_workarounds.o + gt/intel_workarounds.o \ + gt/sysfs_gt.o \ + gt/sysfs_gt_pm.o # autogenerated null render state gt-y += \ gt/gen6_renderstate.o \ diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index f1f1b306e0af..e794d05d69a1 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -15,6 +15,7 @@ #include "intel_rps.h" #include "intel_uncore.h" #include "intel_pm.h" +#include "sysfs_gt.h" void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915) { @@ -321,6 +322,7 @@ void intel_gt_driver_register(struct intel_gt *gt) intel_rps_driver_register(>->rps); debugfs_gt_register(gt); + intel_gt_sysfs_register(gt); } static int intel_gt_init_scratch(struct intel_gt *gt, unsigned int size) @@ -641,6 +643,7 @@ void intel_gt_driver_remove(struct intel_gt *gt) void intel_gt_driver_unregister(struct intel_gt *gt) { + intel_gt_sysfs_unregister(gt); intel_rps_driver_unregister(>->rps); } diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h index 96890dd12b5f..552a27cc0622 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h @@ -32,6 +32,7 @@ struct intel_gt { struct drm_i915_private *i915; struct intel_uncore *uncore; struct i915_ggtt *ggtt; + struct kobject kobj; struct intel_uc uc; diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt.c b/drivers/gpu/drm/i915/gt/sysfs_gt.c new file mode 100644 index 000000000000..4ca140fc215f --- /dev/null +++ b/drivers/gpu/drm/i915/gt/sysfs_gt.c @@ -0,0 +1,85 @@ +// SPDX-License-Identifier: MIT + +/* + * Copyright © 2019 Intel Corporation + */ + +#include <linux/sysfs.h> +#include <drm/drm_device.h> +#include <linux/kobject.h> +#include <linux/printk.h> + +#include "../i915_drv.h" +#include "intel_gt.h" +#include "intel_gt_types.h" +#include "intel_rc6.h" + +#include "sysfs_gt.h" +#include "sysfs_gt_pm.h" + +static inline struct kobject *gt_to_parent_obj(struct intel_gt *gt) +{ + return kobject_get(>->i915->drm.primary->kdev->kobj); +} + +static ssize_t gt_info_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + return snprintf(buff, PAGE_SIZE, "0\n"); +} + +static DEVICE_ATTR_RO(gt_info); + +static void sysfs_gt_kobj_release(struct kobject *kobj) +{ + struct intel_gt *gt = kobj_to_gt(kobj); + + drm_info(>->i915->drm, "releasing interface\n"); +} + +static struct kobj_type sysfs_gt_ktype = { + .release = sysfs_gt_kobj_release, + .sysfs_ops = &kobj_sysfs_ops, +}; + +void intel_gt_sysfs_register(struct intel_gt *gt) +{ + struct kobject *kparent = gt_to_parent_obj(gt); + int ret; + + ret = kobject_init_and_add(>->kobj, &sysfs_gt_ktype, kparent, "gt"); + if (ret) { + drm_err(>->i915->drm, "failed to initialize sysfs file\n"); + kobject_put(>->kobj); + goto parent_files; + } + + ret = sysfs_create_file(>->kobj, &dev_attr_gt_info.attr); + if (ret) + drm_err(>->i915->drm, "failed to create sysfs gt info files\n"); + + intel_gt_sysfs_pm_init(gt, >->kobj); + +parent_files: + /* + * we need to make things right with the + * ABI compatibility. The files were originally + * generated under the parent directory. + */ + intel_gt_sysfs_pm_init(gt, kparent); +} + +void intel_gt_sysfs_unregister(struct intel_gt *gt) +{ + struct kobject *root = gt_to_parent_obj(gt); + + if (>->kobj) { + sysfs_remove_file(>->kobj, &dev_attr_gt_info.attr); + intel_gt_sysfs_pm_remove(gt, >->kobj); + kobject_put(>->kobj); + } + + intel_gt_sysfs_pm_remove(gt, root); + kobject_put(root); +} diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt.h b/drivers/gpu/drm/i915/gt/sysfs_gt.h new file mode 100644 index 000000000000..efff6b884b18 --- /dev/null +++ b/drivers/gpu/drm/i915/gt/sysfs_gt.h @@ -0,0 +1,22 @@ +/* SPDX-License-Identifier: MIT */ + +/* + * Copyright © 2019 Intel Corporation + */ + +#ifndef SYSFS_GT_H +#define SYSFS_GT_H + +#include "intel_gt_types.h" + +struct intel_gt; + +static inline struct intel_gt *kobj_to_gt(struct kobject *kobj) +{ + return container_of(kobj, struct intel_gt, kobj); +} + +void intel_gt_sysfs_register(struct intel_gt *gt); +void intel_gt_sysfs_unregister(struct intel_gt *gt); + +#endif /* SYSFS_GT_H */ diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c new file mode 100644 index 000000000000..a3746485569f --- /dev/null +++ b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c @@ -0,0 +1,446 @@ +// SPDX-License-Identifier: MIT + +/* + * Copyright © 2019 Intel Corporation + */ + +#include <drm/drm_device.h> +#include <linux/sysfs.h> +#include <linux/printk.h> + +#include "../i915_drv.h" +#include "../i915_sysfs.h" +#include "intel_gt.h" +#include "intel_rc6.h" +#include "intel_rps.h" +#include "sysfs_gt.h" +#include "sysfs_gt_pm.h" + +struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev) +{ + struct kobject *kobj = &dev->kobj; + /* + * We are interested at knowing from where the interface + * has been called, whether it's called from gt/ or from + * the parent directory. + * From the interface position it depends also the value of + * the private data. + * If the interface is called from gt/ then private data is + * of the "struct intel_gt *" type, otherwise it's * a + * "struct drm_i915_private *" type. + */ + if (strcmp(dev->kobj.name, "gt")) { + struct drm_i915_private *i915 = kdev_minor_to_i915(dev); + + drm_warn(&i915->drm, "the interface is obsolete, use gt/\n"); + return &i915->gt; + } + + return kobj_to_gt(kobj); +} + +#ifdef CONFIG_PM +static u32 get_residency(struct intel_gt *gt, i915_reg_t reg) +{ + intel_wakeref_t wakeref; + u64 res = 0; + + with_intel_runtime_pm(gt->uncore->rpm, wakeref) + res = intel_rc6_residency_us(>->rc6, reg); + + return DIV_ROUND_CLOSEST_ULL(res, 1000); +} + +static ssize_t rc6_enable_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + u8 mask = 0; + + if (HAS_RC6(gt->i915)) + mask |= BIT(0); + if (HAS_RC6p(gt->i915)) + mask |= BIT(1); + if (HAS_RC6pp(gt->i915)) + mask |= BIT(2); + + return snprintf(buff, PAGE_SIZE, "%x\n", mask); +} + +static ssize_t rc6_residency_ms_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + u32 rc6_residency = get_residency(gt, GEN6_GT_GFX_RC6); + + return snprintf(buff, PAGE_SIZE, "%u\n", rc6_residency); +} + +static ssize_t rc6p_residency_ms_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + u32 rc6p_residency = get_residency(gt, GEN6_GT_GFX_RC6p); + + return snprintf(buff, PAGE_SIZE, "%u\n", rc6p_residency); +} + +static ssize_t rc6pp_residency_ms_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + u32 rc6pp_residency = get_residency(gt, GEN6_GT_GFX_RC6pp); + + return snprintf(buff, PAGE_SIZE, "%u\n", rc6pp_residency); +} + +static ssize_t media_rc6_residency_ms_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + u32 rc6_residency = get_residency(gt, VLV_GT_MEDIA_RC6); + + return snprintf(buff, PAGE_SIZE, "%u\n", rc6_residency); +} + +static DEVICE_ATTR_RO(rc6_enable); +static DEVICE_ATTR_RO(rc6_residency_ms); +static DEVICE_ATTR_RO(rc6p_residency_ms); +static DEVICE_ATTR_RO(rc6pp_residency_ms); +static DEVICE_ATTR_RO(media_rc6_residency_ms); + +static const struct attribute *rc6_attrs[] = { + &dev_attr_rc6_enable.attr, + &dev_attr_rc6_residency_ms.attr, + NULL +}; + +static const struct attribute *rc6p_attrs[] = { + &dev_attr_rc6p_residency_ms.attr, + &dev_attr_rc6pp_residency_ms.attr, + NULL +}; + +static const struct attribute *media_rc6_attrs[] = { + &dev_attr_media_rc6_residency_ms.attr, + NULL +}; + +static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj) +{ + int ret = 0; + + if (HAS_RC6(gt->i915)) { + ret = sysfs_create_files(kobj, rc6_attrs); + if (ret) + drm_err(>->i915->drm, + "failed to create RC6 sysfs files\n"); + } + + if (HAS_RC6p(gt->i915)) { + ret = sysfs_create_files(kobj, rc6p_attrs); + if (ret) + drm_err(>->i915->drm, + "failed to create RC6p sysfs files\n"); + } + + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) { + ret = sysfs_create_files(kobj, media_rc6_attrs); + if (ret) + drm_err(>->i915->drm, + "failed to create media RC6 sysfs files\n"); + } +} +#else +static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj) +{ + return 0; +} +#endif /* CONFIG_PM */ + +static ssize_t gt_act_freq_mhz_show(struct device *dev, + struct device_attribute *attr, char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + + return snprintf(buff, PAGE_SIZE, "%d\n", + intel_rps_read_actual_frequency(>->rps)); +} + +static ssize_t gt_cur_freq_mhz_show(struct device *dev, + struct device_attribute *attr, char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + + return snprintf(buff, PAGE_SIZE, "%d\n", + intel_gpu_freq(rps, rps->cur_freq)); +} + +static ssize_t gt_boost_freq_mhz_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + + return snprintf(buff, PAGE_SIZE, "%d\n", + intel_gpu_freq(rps, rps->boost_freq)); +} + +static ssize_t gt_boost_freq_mhz_store(struct device *dev, + struct device_attribute *attr, + const char *buff, size_t count) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + bool boost = false; + ssize_t ret; + u32 val; + + ret = kstrtou32(buff, 0, &val); + if (ret) + return ret; + + /* 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 count; +} + +static ssize_t vlv_rpe_freq_mhz_show(struct device *dev, + struct device_attribute *attr, char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + + return snprintf(buff, PAGE_SIZE, "%d\n", + intel_gpu_freq(rps, rps->efficient_freq)); +} + +static ssize_t gt_max_freq_mhz_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + + return snprintf(buff, PAGE_SIZE, "%d\n", + intel_gpu_freq(rps, rps->max_freq_softlimit)); +} + +static ssize_t gt_max_freq_mhz_store(struct device *dev, + struct device_attribute *attr, + const char *buff, size_t count) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + ssize_t ret; + u32 val; + + ret = kstrtou32(buff, 0, &val); + if (ret) + return ret; + + mutex_lock(&rps->lock); + + val = intel_freq_opcode(rps, val); + if (val < rps->min_freq || + val > rps->max_freq || + val < rps->min_freq_softlimit) { + ret = -EINVAL; + goto unlock; + } + + if (val > rps->rp0_freq) + DRM_DEBUG("User requested overclocking to %d\n", + intel_gpu_freq(rps, val)); + + rps->max_freq_softlimit = val; + + val = clamp_t(int, rps->cur_freq, + rps->min_freq_softlimit, + rps->max_freq_softlimit); + + /* + * We still need *_set_rps to process the new max_delay and + * update the interrupt limits and PMINTRMSK even though + * frequency request may be unchanged. + */ + intel_rps_set(rps, val); + +unlock: + mutex_unlock(&rps->lock); + + return ret ?: count; +} + +static ssize_t gt_min_freq_mhz_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + + return snprintf(buff, PAGE_SIZE, "%d\n", + intel_gpu_freq(rps, rps->min_freq_softlimit)); +} + +static ssize_t gt_min_freq_mhz_store(struct device *dev, + struct device_attribute *attr, + const char *buff, size_t count) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + ssize_t ret; + u32 val; + + ret = kstrtou32(buff, 0, &val); + if (ret) + return ret; + + mutex_lock(&rps->lock); + + val = intel_freq_opcode(rps, val); + if (val < rps->min_freq || + val > rps->max_freq || + val > rps->max_freq_softlimit) { + ret = -EINVAL; + goto unlock; + } + + rps->min_freq_softlimit = val; + + val = clamp_t(int, rps->cur_freq, + rps->min_freq_softlimit, + rps->max_freq_softlimit); + + /* + * We still need *_set_rps to process the new min_delay and + * update the interrupt limits and PMINTRMSK even though + * frequency request may be unchanged. + */ + intel_rps_set(rps, val); + +unlock: + mutex_unlock(&rps->lock); + + 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 *dev, + struct device_attribute *attr, + char *buff); + +static DEVICE_ATTR(gt_RP0_freq_mhz, 0444, gt_rp_mhz_show, NULL); +static DEVICE_ATTR(gt_RP1_freq_mhz, 0444, gt_rp_mhz_show, NULL); +static DEVICE_ATTR(gt_RPn_freq_mhz, 0444, gt_rp_mhz_show, NULL); + +/* For now we have a static number of RP states */ +static ssize_t gt_rp_mhz_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + u32 val; + + if (attr == &dev_attr_gt_RP0_freq_mhz) + val = intel_gpu_freq(rps, rps->rp0_freq); + else if (attr == &dev_attr_gt_RP1_freq_mhz) + val = intel_gpu_freq(rps, rps->rp1_freq); + else if (attr == &dev_attr_gt_RPn_freq_mhz) + val = intel_gpu_freq(rps, rps->min_freq); + else + BUG(); + + return snprintf(buff, PAGE_SIZE, "%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, +}; + +static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj) +{ + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) + return sysfs_create_files(kobj, vlv_attrs); + + if (INTEL_GEN(gt->i915) >= 6) + return sysfs_create_files(kobj, gen6_attrs); + + return 0; +} + +void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj) +{ + int ret; + + intel_sysfs_rc6_init(gt, kobj); + + ret = intel_sysfs_rps_init(gt, kobj); + if (ret) + drm_err(>->i915->drm, "failed to create RPS sysfs files"); +} + +void intel_gt_sysfs_pm_remove(struct intel_gt *gt, struct kobject *kobj) +{ +#ifdef CONFIG_PM + if (HAS_RC6(gt->i915)) + sysfs_remove_files(kobj, rc6_attrs); + if (HAS_RC6p(gt->i915)) + sysfs_remove_files(kobj, rc6p_attrs); + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) + sysfs_remove_files(kobj, media_rc6_attrs); +#endif + + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) + sysfs_remove_files(kobj, vlv_attrs); + else + sysfs_remove_files(kobj, gen6_attrs); +} diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt_pm.h b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.h new file mode 100644 index 000000000000..758d0c3cb998 --- /dev/null +++ b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.h @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: MIT */ + +/* + * Copyright © 2019 Intel Corporation + */ + +#ifndef SYSFS_RC6_H +#define SYSFS_RC6_H + +#include <linux/kobject.h> + +#include "intel_gt_types.h" + +void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj); +void intel_gt_sysfs_pm_remove(struct intel_gt *gt, struct kobject *kobj); + +#endif /* SYSFS_RC6_H */ diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c index c14d762bd652..1298977fc08b 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.c +++ b/drivers/gpu/drm/i915/i915_sysfs.c @@ -38,113 +38,12 @@ #include "intel_pm.h" #include "intel_sideband.h" -static inline struct drm_i915_private *kdev_minor_to_i915(struct device *kdev) +struct drm_i915_private *kdev_minor_to_i915(struct device *kdev) { struct drm_minor *minor = dev_get_drvdata(kdev); return to_i915(minor->dev); } -#ifdef CONFIG_PM -static u32 calc_residency(struct drm_i915_private *dev_priv, - i915_reg_t reg) -{ - intel_wakeref_t wakeref; - u64 res = 0; - - with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref) - res = intel_rc6_residency_us(&dev_priv->gt.rc6, reg); - - return DIV_ROUND_CLOSEST_ULL(res, 1000); -} - -static ssize_t -show_rc6_mask(struct device *kdev, struct device_attribute *attr, char *buf) -{ - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); - unsigned int mask; - - mask = 0; - if (HAS_RC6(dev_priv)) - mask |= BIT(0); - if (HAS_RC6p(dev_priv)) - mask |= BIT(1); - if (HAS_RC6pp(dev_priv)) - mask |= BIT(2); - - return snprintf(buf, PAGE_SIZE, "%x\n", mask); -} - -static ssize_t -show_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf) -{ - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); - u32 rc6_residency = calc_residency(dev_priv, GEN6_GT_GFX_RC6); - return snprintf(buf, PAGE_SIZE, "%u\n", rc6_residency); -} - -static ssize_t -show_rc6p_ms(struct device *kdev, struct device_attribute *attr, char *buf) -{ - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); - u32 rc6p_residency = calc_residency(dev_priv, GEN6_GT_GFX_RC6p); - return snprintf(buf, PAGE_SIZE, "%u\n", rc6p_residency); -} - -static ssize_t -show_rc6pp_ms(struct device *kdev, struct device_attribute *attr, char *buf) -{ - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); - u32 rc6pp_residency = calc_residency(dev_priv, GEN6_GT_GFX_RC6pp); - return snprintf(buf, PAGE_SIZE, "%u\n", rc6pp_residency); -} - -static ssize_t -show_media_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf) -{ - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); - u32 rc6_residency = calc_residency(dev_priv, VLV_GT_MEDIA_RC6); - return snprintf(buf, PAGE_SIZE, "%u\n", rc6_residency); -} - -static DEVICE_ATTR(rc6_enable, S_IRUGO, show_rc6_mask, NULL); -static DEVICE_ATTR(rc6_residency_ms, S_IRUGO, show_rc6_ms, NULL); -static DEVICE_ATTR(rc6p_residency_ms, S_IRUGO, show_rc6p_ms, NULL); -static DEVICE_ATTR(rc6pp_residency_ms, S_IRUGO, show_rc6pp_ms, NULL); -static DEVICE_ATTR(media_rc6_residency_ms, S_IRUGO, show_media_rc6_ms, NULL); - -static struct attribute *rc6_attrs[] = { - &dev_attr_rc6_enable.attr, - &dev_attr_rc6_residency_ms.attr, - NULL -}; - -static const 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 const struct attribute_group rc6p_attr_group = { - .name = power_group_name, - .attrs = rc6p_attrs -}; - -static struct attribute *media_rc6_attrs[] = { - &dev_attr_media_rc6_residency_ms.attr, - NULL -}; - -static const struct attribute_group media_rc6_attr_group = { - .name = power_group_name, - .attrs = media_rc6_attrs -}; -#endif - static int l3_access_valid(struct drm_i915_private *i915, loff_t offset) { if (!HAS_L3_DPF(i915)) @@ -256,239 +155,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 = &i915->gt.rps; - - return snprintf(buf, PAGE_SIZE, "%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 = &i915->gt.rps; - - return snprintf(buf, PAGE_SIZE, "%d\n", - intel_gpu_freq(rps, rps->cur_freq)); -} - -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 = &i915->gt.rps; - - return snprintf(buf, PAGE_SIZE, "%d\n", - intel_gpu_freq(rps, rps->boost_freq)); -} - -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 = &dev_priv->gt.rps; - bool boost = false; - ssize_t ret; - u32 val; - - ret = kstrtou32(buf, 0, &val); - if (ret) - return ret; - - /* 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 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 = &dev_priv->gt.rps; - - return snprintf(buf, PAGE_SIZE, "%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_rps *rps = &dev_priv->gt.rps; - - return snprintf(buf, PAGE_SIZE, "%d\n", - intel_gpu_freq(rps, rps->max_freq_softlimit)); -} - -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_rps *rps = &dev_priv->gt.rps; - ssize_t ret; - u32 val; - - ret = kstrtou32(buf, 0, &val); - if (ret) - return ret; - - mutex_lock(&rps->lock); - - val = intel_freq_opcode(rps, val); - if (val < rps->min_freq || - val > rps->max_freq || - val < rps->min_freq_softlimit) { - ret = -EINVAL; - goto unlock; - } - - if (val > rps->rp0_freq) - DRM_DEBUG("User requested overclocking to %d\n", - intel_gpu_freq(rps, val)); - - rps->max_freq_softlimit = val; - - val = clamp_t(int, rps->cur_freq, - rps->min_freq_softlimit, - rps->max_freq_softlimit); - - /* - * We still need *_set_rps to process the new max_delay and - * update the interrupt limits and PMINTRMSK even though - * frequency request may be unchanged. - */ - intel_rps_set(rps, val); - -unlock: - mutex_unlock(&rps->lock); - - return ret ?: count; -} - -static ssize_t gt_min_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 = &dev_priv->gt.rps; - - return snprintf(buf, PAGE_SIZE, "%d\n", - intel_gpu_freq(rps, rps->min_freq_softlimit)); -} - -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 *dev_priv = kdev_minor_to_i915(kdev); - struct intel_rps *rps = &dev_priv->gt.rps; - ssize_t ret; - u32 val; - - ret = kstrtou32(buf, 0, &val); - if (ret) - return ret; - - mutex_lock(&rps->lock); - - val = intel_freq_opcode(rps, val); - if (val < rps->min_freq || - val > rps->max_freq || - val > rps->max_freq_softlimit) { - ret = -EINVAL; - goto unlock; - } - - rps->min_freq_softlimit = val; - - val = clamp_t(int, rps->cur_freq, - rps->min_freq_softlimit, - rps->max_freq_softlimit); - - /* - * We still need *_set_rps to process the new min_delay and - * update the interrupt limits and PMINTRMSK even though - * frequency request may be unchanged. - */ - intel_rps_set(rps, val); - -unlock: - mutex_unlock(&rps->lock); - - 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 = &dev_priv->gt.rps; - u32 val; - - if (attr == &dev_attr_gt_RP0_freq_mhz) - val = intel_gpu_freq(rps, rps->rp0_freq); - else if (attr == &dev_attr_gt_RP1_freq_mhz) - val = intel_gpu_freq(rps, rps->rp1_freq); - else if (attr == &dev_attr_gt_RPn_freq_mhz) - val = intel_gpu_freq(rps, rps->min_freq); - else - BUG(); - - return snprintf(buf, PAGE_SIZE, "%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, @@ -559,29 +225,6 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv) struct device *kdev = dev_priv->drm.primary->kdev; int ret; -#ifdef CONFIG_PM - if (HAS_RC6(dev_priv)) { - ret = sysfs_merge_group(&kdev->kobj, - &rc6_attr_group); - if (ret) - drm_err(&dev_priv->drm, - "RC6 residency sysfs setup failed\n"); - } - if (HAS_RC6p(dev_priv)) { - ret = sysfs_merge_group(&kdev->kobj, - &rc6p_attr_group); - if (ret) - drm_err(&dev_priv->drm, - "RC6p residency sysfs setup failed\n"); - } - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { - ret = sysfs_merge_group(&kdev->kobj, - &media_rc6_attr_group); - if (ret) - drm_err(&dev_priv->drm, - "Media RC6 residency sysfs setup failed\n"); - } -#endif if (HAS_L3_DPF(dev_priv)) { ret = device_create_bin_file(kdev, &dpf_attrs); if (ret) @@ -597,14 +240,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 (INTEL_GEN(dev_priv) >= 6) - ret = sysfs_create_files(&kdev->kobj, gen6_attrs); - if (ret) - drm_err(&dev_priv->drm, "RPS sysfs setup failed\n"); - i915_setup_error_capture(kdev); } @@ -614,14 +249,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); -#ifdef CONFIG_PM - sysfs_unmerge_group(&kdev->kobj, &rc6_attr_group); - sysfs_unmerge_group(&kdev->kobj, &rc6p_attr_group); -#endif } diff --git a/drivers/gpu/drm/i915/i915_sysfs.h b/drivers/gpu/drm/i915/i915_sysfs.h index 41afd4366416..ad6114de81c9 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.h +++ b/drivers/gpu/drm/i915/i915_sysfs.h @@ -6,8 +6,11 @@ #ifndef __I915_SYSFS_H__ #define __I915_SYSFS_H__ +#include <linux/device.h> + struct drm_i915_private; +struct drm_i915_private *kdev_minor_to_i915(struct device *kdev); void i915_setup_sysfs(struct drm_i915_private *i915); void i915_teardown_sysfs(struct drm_i915_private *i915); -- 2.25.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gt: make a gt sysfs group and move power management files (rev3) 2020-02-14 11:03 [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files Andi Shyti @ 2020-02-14 12:51 ` Patchwork 2020-02-14 12:52 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork ` (4 subsequent siblings) 5 siblings, 0 replies; 13+ messages in thread From: Patchwork @ 2020-02-14 12:51 UTC (permalink / raw) To: Andi Shyti; +Cc: intel-gfx == Series Details == Series: drm/i915/gt: make a gt sysfs group and move power management files (rev3) URL : https://patchwork.freedesktop.org/series/73190/ State : warning == Summary == $ dim checkpatch origin/drm-tip 2bb6ef6acc00 drm/i915/gt: make a gt sysfs group and move power management files -:71: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating? #71: new file mode 100644 -:553: WARNING:DEVICE_ATTR_FUNCTIONS: Consider renaming function(s) 'gt_rp_mhz_show' to 'gt_RP0_freq_mhz_show' #553: FILE: drivers/gpu/drm/i915/gt/sysfs_gt_pm.c:359: +static DEVICE_ATTR(gt_RP0_freq_mhz, 0444, gt_rp_mhz_show, NULL); -:554: WARNING:DEVICE_ATTR_FUNCTIONS: Consider renaming function(s) 'gt_rp_mhz_show' to 'gt_RP1_freq_mhz_show' #554: FILE: drivers/gpu/drm/i915/gt/sysfs_gt_pm.c:360: +static DEVICE_ATTR(gt_RP1_freq_mhz, 0444, gt_rp_mhz_show, NULL); -:555: CHECK:CAMELCASE: Avoid CamelCase: <gt_RPn_freq_mhz> #555: FILE: drivers/gpu/drm/i915/gt/sysfs_gt_pm.c:361: +static DEVICE_ATTR(gt_RPn_freq_mhz, 0444, gt_rp_mhz_show, NULL); -:555: WARNING:DEVICE_ATTR_FUNCTIONS: Consider renaming function(s) 'gt_rp_mhz_show' to 'gt_RPn_freq_mhz_show' #555: FILE: drivers/gpu/drm/i915/gt/sysfs_gt_pm.c:361: +static DEVICE_ATTR(gt_RPn_freq_mhz, 0444, gt_rp_mhz_show, NULL); -:570: CHECK:CAMELCASE: Avoid CamelCase: <dev_attr_gt_RPn_freq_mhz> #570: FILE: drivers/gpu/drm/i915/gt/sysfs_gt_pm.c:376: + else if (attr == &dev_attr_gt_RPn_freq_mhz) -:573: WARNING:AVOID_BUG: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON() #573: FILE: drivers/gpu/drm/i915/gt/sysfs_gt_pm.c:379: + BUG(); total: 0 errors, 5 warnings, 2 checks, 1029 lines checked _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915/gt: make a gt sysfs group and move power management files (rev3) 2020-02-14 11:03 [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files Andi Shyti 2020-02-14 12:51 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gt: make a gt sysfs group and move power management files (rev3) Patchwork @ 2020-02-14 12:52 ` Patchwork 2020-02-14 12:54 ` [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files Tvrtko Ursulin ` (3 subsequent siblings) 5 siblings, 0 replies; 13+ messages in thread From: Patchwork @ 2020-02-14 12:52 UTC (permalink / raw) To: Andi Shyti; +Cc: intel-gfx == Series Details == Series: drm/i915/gt: make a gt sysfs group and move power management files (rev3) URL : https://patchwork.freedesktop.org/series/73190/ State : warning == Summary == $ dim sparse origin/drm-tip Sparse version: v0.6.0 Commit: drm/i915/gt: make a gt sysfs group and move power management files +drivers/gpu/drm/i915/gt/sysfs_gt_pm.c:19:17: warning: symbol 'intel_gt_sysfs_get_drvdata' was not declared. Should it be static? _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files 2020-02-14 11:03 [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files Andi Shyti 2020-02-14 12:51 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gt: make a gt sysfs group and move power management files (rev3) Patchwork 2020-02-14 12:52 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork @ 2020-02-14 12:54 ` Tvrtko Ursulin 2020-02-14 13:16 ` Andi Shyti 2020-02-14 13:18 ` Chris Wilson 2020-02-14 13:13 ` [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: make a gt sysfs group and move power management files (rev3) Patchwork ` (2 subsequent siblings) 5 siblings, 2 replies; 13+ messages in thread From: Tvrtko Ursulin @ 2020-02-14 12:54 UTC (permalink / raw) To: Andi Shyti, Intel GFX On 14/02/2020 11:03, Andi Shyti wrote: > The GT has its own properties and in sysfs they should be grouped > in the 'gt/' directory. > > Create the 'gt/' directory in sysfs and move the power management > related files. Can you paste the new and legacy paths in the commit message? > > Signed-off-by: Andi Shyti <andi.shyti@intel.com> > --- > Hi, > > this version has some more substantial differences, nothing that > changes the wanted behavior, though. > > Thanks Chris, Jani and Tvrtko for the reviews, > Andi > > Changelog: > ========== > > v2 -> v3: > - fix some cleanups that I forgot in the previous patch > - fix reference pointers to the gt structure > - and many other small changes here and there. > v1 -> v2: > - keep the existing files as they are > - use "intel_gt_*" as prefix instead of "sysfs_*" > > drivers/gpu/drm/i915/Makefile | 4 +- > drivers/gpu/drm/i915/gt/intel_gt.c | 3 + > drivers/gpu/drm/i915/gt/intel_gt_types.h | 1 + > drivers/gpu/drm/i915/gt/sysfs_gt.c | 85 +++++ > drivers/gpu/drm/i915/gt/sysfs_gt.h | 22 ++ > drivers/gpu/drm/i915/gt/sysfs_gt_pm.c | 446 +++++++++++++++++++++++ > drivers/gpu/drm/i915/gt/sysfs_gt_pm.h | 17 + > drivers/gpu/drm/i915/i915_sysfs.c | 375 +------------------ > drivers/gpu/drm/i915/i915_sysfs.h | 3 + > 9 files changed, 581 insertions(+), 375 deletions(-) > create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt.c > create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt.h > create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt_pm.c > create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt_pm.h > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index 49eed50ef0a4..3b81c8a96c06 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -106,7 +106,9 @@ gt-y += \ > gt/intel_rps.o \ > gt/intel_sseu.o \ > gt/intel_timeline.o \ > - gt/intel_workarounds.o > + gt/intel_workarounds.o \ > + gt/sysfs_gt.o \ > + gt/sysfs_gt_pm.o > # autogenerated null render state > gt-y += \ > gt/gen6_renderstate.o \ > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c > index f1f1b306e0af..e794d05d69a1 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c > @@ -15,6 +15,7 @@ > #include "intel_rps.h" > #include "intel_uncore.h" > #include "intel_pm.h" > +#include "sysfs_gt.h" > > void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915) > { > @@ -321,6 +322,7 @@ void intel_gt_driver_register(struct intel_gt *gt) > intel_rps_driver_register(>->rps); > > debugfs_gt_register(gt); > + intel_gt_sysfs_register(gt); > } > > static int intel_gt_init_scratch(struct intel_gt *gt, unsigned int size) > @@ -641,6 +643,7 @@ void intel_gt_driver_remove(struct intel_gt *gt) > > void intel_gt_driver_unregister(struct intel_gt *gt) > { > + intel_gt_sysfs_unregister(gt); > intel_rps_driver_unregister(>->rps); > } > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h > index 96890dd12b5f..552a27cc0622 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h > +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h > @@ -32,6 +32,7 @@ struct intel_gt { > struct drm_i915_private *i915; > struct intel_uncore *uncore; > struct i915_ggtt *ggtt; > + struct kobject kobj; sysfs_root or something like would perhaps be more descriptive? > > struct intel_uc uc; > > diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt.c b/drivers/gpu/drm/i915/gt/sysfs_gt.c > new file mode 100644 > index 000000000000..4ca140fc215f > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/sysfs_gt.c > @@ -0,0 +1,85 @@ > +// SPDX-License-Identifier: MIT > + > +/* > + * Copyright © 2019 Intel Corporation > + */ > + > +#include <linux/sysfs.h> > +#include <drm/drm_device.h> > +#include <linux/kobject.h> > +#include <linux/printk.h> > + > +#include "../i915_drv.h" > +#include "intel_gt.h" > +#include "intel_gt_types.h" > +#include "intel_rc6.h" > + > +#include "sysfs_gt.h" > +#include "sysfs_gt_pm.h" > + > +static inline struct kobject *gt_to_parent_obj(struct intel_gt *gt) > +{ > + return kobject_get(>->i915->drm.primary->kdev->kobj); It's a bit surprising X_to_Y helper get a reference as well, no? gt_get_parent_obj perhaps? But where is this released? > +} > + > +static ssize_t gt_info_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + return snprintf(buff, PAGE_SIZE, "0\n"); > +} > + > +static DEVICE_ATTR_RO(gt_info); > + > +static void sysfs_gt_kobj_release(struct kobject *kobj) > +{ > + struct intel_gt *gt = kobj_to_gt(kobj); > + > + drm_info(>->i915->drm, "releasing interface\n"); Debugging remnants. > +} > + > +static struct kobj_type sysfs_gt_ktype = { > + .release = sysfs_gt_kobj_release, > + .sysfs_ops = &kobj_sysfs_ops, > +}; > + > +void intel_gt_sysfs_register(struct intel_gt *gt) > +{ > + struct kobject *kparent = gt_to_parent_obj(gt); > + int ret; > + > + ret = kobject_init_and_add(>->kobj, &sysfs_gt_ktype, kparent, "gt"); > + if (ret) { > + drm_err(>->i915->drm, "failed to initialize sysfs file\n"); > + kobject_put(>->kobj); So you want gt->kobj to be embedded struct and you want to then override the release vfunc so it is not freed, but what is the specific reason you want it embedded? > + goto parent_files; > + } > + > + ret = sysfs_create_file(>->kobj, &dev_attr_gt_info.attr); > + if (ret) > + drm_err(>->i915->drm, "failed to create sysfs gt info files\n"); > + > + intel_gt_sysfs_pm_init(gt, >->kobj); > + > +parent_files: > + /* > + * we need to make things right with the > + * ABI compatibility. The files were originally > + * generated under the parent directory. > + */ > + intel_gt_sysfs_pm_init(gt, kparent); > +} > + > +void intel_gt_sysfs_unregister(struct intel_gt *gt) > +{ > + struct kobject *root = gt_to_parent_obj(gt); > + > + if (>->kobj) { This is always true. > + sysfs_remove_file(>->kobj, &dev_attr_gt_info.attr); > + intel_gt_sysfs_pm_remove(gt, >->kobj); > + kobject_put(>->kobj); I think kobject_put is enough to tear down the whole hierarchy so you could simplify this. > + } > + > + intel_gt_sysfs_pm_remove(gt, root); > + kobject_put(root); Maybe stick to the same terminology regarding root and parent. Get/put on the parent looks unbalanced. Both register and unregister take a reference and only unregister releases it. But do you even need a reference? > +} > diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt.h b/drivers/gpu/drm/i915/gt/sysfs_gt.h > new file mode 100644 > index 000000000000..efff6b884b18 > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/sysfs_gt.h > @@ -0,0 +1,22 @@ > +/* SPDX-License-Identifier: MIT */ > + > +/* > + * Copyright © 2019 Intel Corporation > + */ > + > +#ifndef SYSFS_GT_H > +#define SYSFS_GT_H > + > +#include "intel_gt_types.h" > + > +struct intel_gt; > + > +static inline struct intel_gt *kobj_to_gt(struct kobject *kobj) > +{ > + return container_of(kobj, struct intel_gt, kobj); > +} > + > +void intel_gt_sysfs_register(struct intel_gt *gt); > +void intel_gt_sysfs_unregister(struct intel_gt *gt); > + > +#endif /* SYSFS_GT_H */ > diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c > new file mode 100644 > index 000000000000..a3746485569f > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c > @@ -0,0 +1,446 @@ > +// SPDX-License-Identifier: MIT > + > +/* > + * Copyright © 2019 Intel Corporation > + */ > + > +#include <drm/drm_device.h> > +#include <linux/sysfs.h> > +#include <linux/printk.h> > + > +#include "../i915_drv.h" > +#include "../i915_sysfs.h" > +#include "intel_gt.h" > +#include "intel_rc6.h" > +#include "intel_rps.h" > +#include "sysfs_gt.h" > +#include "sysfs_gt_pm.h" > + > +struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev) > +{ > + struct kobject *kobj = &dev->kobj; > + /* > + * We are interested at knowing from where the interface > + * has been called, whether it's called from gt/ or from > + * the parent directory. > + * From the interface position it depends also the value of > + * the private data. > + * If the interface is called from gt/ then private data is > + * of the "struct intel_gt *" type, otherwise it's * a > + * "struct drm_i915_private *" type. > + */ > + if (strcmp(dev->kobj.name, "gt")) { > + struct drm_i915_private *i915 = kdev_minor_to_i915(dev); > + > + drm_warn(&i915->drm, "the interface is obsolete, use gt/\n"); Can you log current->name & pid? I am also thinking is a level down from warn would be better. Notice sounds intuitively correct to me. I am also tempted by the _once alternative, but then it makes less sense to include name & pid. Regards, Tvrtko > + return &i915->gt; > + } > + > + return kobj_to_gt(kobj); > +} > + > +#ifdef CONFIG_PM > +static u32 get_residency(struct intel_gt *gt, i915_reg_t reg) > +{ > + intel_wakeref_t wakeref; > + u64 res = 0; > + > + with_intel_runtime_pm(gt->uncore->rpm, wakeref) > + res = intel_rc6_residency_us(>->rc6, reg); > + > + return DIV_ROUND_CLOSEST_ULL(res, 1000); > +} > + > +static ssize_t rc6_enable_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + u8 mask = 0; > + > + if (HAS_RC6(gt->i915)) > + mask |= BIT(0); > + if (HAS_RC6p(gt->i915)) > + mask |= BIT(1); > + if (HAS_RC6pp(gt->i915)) > + mask |= BIT(2); > + > + return snprintf(buff, PAGE_SIZE, "%x\n", mask); > +} > + > +static ssize_t rc6_residency_ms_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + u32 rc6_residency = get_residency(gt, GEN6_GT_GFX_RC6); > + > + return snprintf(buff, PAGE_SIZE, "%u\n", rc6_residency); > +} > + > +static ssize_t rc6p_residency_ms_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + u32 rc6p_residency = get_residency(gt, GEN6_GT_GFX_RC6p); > + > + return snprintf(buff, PAGE_SIZE, "%u\n", rc6p_residency); > +} > + > +static ssize_t rc6pp_residency_ms_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + u32 rc6pp_residency = get_residency(gt, GEN6_GT_GFX_RC6pp); > + > + return snprintf(buff, PAGE_SIZE, "%u\n", rc6pp_residency); > +} > + > +static ssize_t media_rc6_residency_ms_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + u32 rc6_residency = get_residency(gt, VLV_GT_MEDIA_RC6); > + > + return snprintf(buff, PAGE_SIZE, "%u\n", rc6_residency); > +} > + > +static DEVICE_ATTR_RO(rc6_enable); > +static DEVICE_ATTR_RO(rc6_residency_ms); > +static DEVICE_ATTR_RO(rc6p_residency_ms); > +static DEVICE_ATTR_RO(rc6pp_residency_ms); > +static DEVICE_ATTR_RO(media_rc6_residency_ms); > + > +static const struct attribute *rc6_attrs[] = { > + &dev_attr_rc6_enable.attr, > + &dev_attr_rc6_residency_ms.attr, > + NULL > +}; > + > +static const struct attribute *rc6p_attrs[] = { > + &dev_attr_rc6p_residency_ms.attr, > + &dev_attr_rc6pp_residency_ms.attr, > + NULL > +}; > + > +static const struct attribute *media_rc6_attrs[] = { > + &dev_attr_media_rc6_residency_ms.attr, > + NULL > +}; > + > +static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj) > +{ > + int ret = 0; > + > + if (HAS_RC6(gt->i915)) { > + ret = sysfs_create_files(kobj, rc6_attrs); > + if (ret) > + drm_err(>->i915->drm, > + "failed to create RC6 sysfs files\n"); > + } > + > + if (HAS_RC6p(gt->i915)) { > + ret = sysfs_create_files(kobj, rc6p_attrs); > + if (ret) > + drm_err(>->i915->drm, > + "failed to create RC6p sysfs files\n"); > + } > + > + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) { > + ret = sysfs_create_files(kobj, media_rc6_attrs); > + if (ret) > + drm_err(>->i915->drm, > + "failed to create media RC6 sysfs files\n"); > + } > +} > +#else > +static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj) > +{ > + return 0; > +} > +#endif /* CONFIG_PM */ > + > +static ssize_t gt_act_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_rps_read_actual_frequency(>->rps)); > +} > + > +static ssize_t gt_cur_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_gpu_freq(rps, rps->cur_freq)); > +} > + > +static ssize_t gt_boost_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_gpu_freq(rps, rps->boost_freq)); > +} > + > +static ssize_t gt_boost_freq_mhz_store(struct device *dev, > + struct device_attribute *attr, > + const char *buff, size_t count) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + bool boost = false; > + ssize_t ret; > + u32 val; > + > + ret = kstrtou32(buff, 0, &val); > + if (ret) > + return ret; > + > + /* 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 count; > +} > + > +static ssize_t vlv_rpe_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_gpu_freq(rps, rps->efficient_freq)); > +} > + > +static ssize_t gt_max_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_gpu_freq(rps, rps->max_freq_softlimit)); > +} > + > +static ssize_t gt_max_freq_mhz_store(struct device *dev, > + struct device_attribute *attr, > + const char *buff, size_t count) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + ssize_t ret; > + u32 val; > + > + ret = kstrtou32(buff, 0, &val); > + if (ret) > + return ret; > + > + mutex_lock(&rps->lock); > + > + val = intel_freq_opcode(rps, val); > + if (val < rps->min_freq || > + val > rps->max_freq || > + val < rps->min_freq_softlimit) { > + ret = -EINVAL; > + goto unlock; > + } > + > + if (val > rps->rp0_freq) > + DRM_DEBUG("User requested overclocking to %d\n", > + intel_gpu_freq(rps, val)); > + > + rps->max_freq_softlimit = val; > + > + val = clamp_t(int, rps->cur_freq, > + rps->min_freq_softlimit, > + rps->max_freq_softlimit); > + > + /* > + * We still need *_set_rps to process the new max_delay and > + * update the interrupt limits and PMINTRMSK even though > + * frequency request may be unchanged. > + */ > + intel_rps_set(rps, val); > + > +unlock: > + mutex_unlock(&rps->lock); > + > + return ret ?: count; > +} > + > +static ssize_t gt_min_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_gpu_freq(rps, rps->min_freq_softlimit)); > +} > + > +static ssize_t gt_min_freq_mhz_store(struct device *dev, > + struct device_attribute *attr, > + const char *buff, size_t count) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + ssize_t ret; > + u32 val; > + > + ret = kstrtou32(buff, 0, &val); > + if (ret) > + return ret; > + > + mutex_lock(&rps->lock); > + > + val = intel_freq_opcode(rps, val); > + if (val < rps->min_freq || > + val > rps->max_freq || > + val > rps->max_freq_softlimit) { > + ret = -EINVAL; > + goto unlock; > + } > + > + rps->min_freq_softlimit = val; > + > + val = clamp_t(int, rps->cur_freq, > + rps->min_freq_softlimit, > + rps->max_freq_softlimit); > + > + /* > + * We still need *_set_rps to process the new min_delay and > + * update the interrupt limits and PMINTRMSK even though > + * frequency request may be unchanged. > + */ > + intel_rps_set(rps, val); > + > +unlock: > + mutex_unlock(&rps->lock); > + > + 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 *dev, > + struct device_attribute *attr, > + char *buff); > + > +static DEVICE_ATTR(gt_RP0_freq_mhz, 0444, gt_rp_mhz_show, NULL); > +static DEVICE_ATTR(gt_RP1_freq_mhz, 0444, gt_rp_mhz_show, NULL); > +static DEVICE_ATTR(gt_RPn_freq_mhz, 0444, gt_rp_mhz_show, NULL); > + > +/* For now we have a static number of RP states */ > +static ssize_t gt_rp_mhz_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + u32 val; > + > + if (attr == &dev_attr_gt_RP0_freq_mhz) > + val = intel_gpu_freq(rps, rps->rp0_freq); > + else if (attr == &dev_attr_gt_RP1_freq_mhz) > + val = intel_gpu_freq(rps, rps->rp1_freq); > + else if (attr == &dev_attr_gt_RPn_freq_mhz) > + val = intel_gpu_freq(rps, rps->min_freq); > + else > + BUG(); > + > + return snprintf(buff, PAGE_SIZE, "%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, > +}; > + > +static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj) > +{ > + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) > + return sysfs_create_files(kobj, vlv_attrs); > + > + if (INTEL_GEN(gt->i915) >= 6) > + return sysfs_create_files(kobj, gen6_attrs); > + > + return 0; > +} > + > +void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj) > +{ > + int ret; > + > + intel_sysfs_rc6_init(gt, kobj); > + > + ret = intel_sysfs_rps_init(gt, kobj); > + if (ret) > + drm_err(>->i915->drm, "failed to create RPS sysfs files"); > +} > + > +void intel_gt_sysfs_pm_remove(struct intel_gt *gt, struct kobject *kobj) > +{ > +#ifdef CONFIG_PM > + if (HAS_RC6(gt->i915)) > + sysfs_remove_files(kobj, rc6_attrs); > + if (HAS_RC6p(gt->i915)) > + sysfs_remove_files(kobj, rc6p_attrs); > + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) > + sysfs_remove_files(kobj, media_rc6_attrs); > +#endif > + > + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) > + sysfs_remove_files(kobj, vlv_attrs); > + else > + sysfs_remove_files(kobj, gen6_attrs); > +} > diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt_pm.h b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.h > new file mode 100644 > index 000000000000..758d0c3cb998 > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.h > @@ -0,0 +1,17 @@ > +/* SPDX-License-Identifier: MIT */ > + > +/* > + * Copyright © 2019 Intel Corporation > + */ > + > +#ifndef SYSFS_RC6_H > +#define SYSFS_RC6_H > + > +#include <linux/kobject.h> > + > +#include "intel_gt_types.h" > + > +void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj); > +void intel_gt_sysfs_pm_remove(struct intel_gt *gt, struct kobject *kobj); > + > +#endif /* SYSFS_RC6_H */ > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c > index c14d762bd652..1298977fc08b 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -38,113 +38,12 @@ > #include "intel_pm.h" > #include "intel_sideband.h" > > -static inline struct drm_i915_private *kdev_minor_to_i915(struct device *kdev) > +struct drm_i915_private *kdev_minor_to_i915(struct device *kdev) > { > struct drm_minor *minor = dev_get_drvdata(kdev); > return to_i915(minor->dev); > } > > -#ifdef CONFIG_PM > -static u32 calc_residency(struct drm_i915_private *dev_priv, > - i915_reg_t reg) > -{ > - intel_wakeref_t wakeref; > - u64 res = 0; > - > - with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref) > - res = intel_rc6_residency_us(&dev_priv->gt.rc6, reg); > - > - return DIV_ROUND_CLOSEST_ULL(res, 1000); > -} > - > -static ssize_t > -show_rc6_mask(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - unsigned int mask; > - > - mask = 0; > - if (HAS_RC6(dev_priv)) > - mask |= BIT(0); > - if (HAS_RC6p(dev_priv)) > - mask |= BIT(1); > - if (HAS_RC6pp(dev_priv)) > - mask |= BIT(2); > - > - return snprintf(buf, PAGE_SIZE, "%x\n", mask); > -} > - > -static ssize_t > -show_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - u32 rc6_residency = calc_residency(dev_priv, GEN6_GT_GFX_RC6); > - return snprintf(buf, PAGE_SIZE, "%u\n", rc6_residency); > -} > - > -static ssize_t > -show_rc6p_ms(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - u32 rc6p_residency = calc_residency(dev_priv, GEN6_GT_GFX_RC6p); > - return snprintf(buf, PAGE_SIZE, "%u\n", rc6p_residency); > -} > - > -static ssize_t > -show_rc6pp_ms(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - u32 rc6pp_residency = calc_residency(dev_priv, GEN6_GT_GFX_RC6pp); > - return snprintf(buf, PAGE_SIZE, "%u\n", rc6pp_residency); > -} > - > -static ssize_t > -show_media_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - u32 rc6_residency = calc_residency(dev_priv, VLV_GT_MEDIA_RC6); > - return snprintf(buf, PAGE_SIZE, "%u\n", rc6_residency); > -} > - > -static DEVICE_ATTR(rc6_enable, S_IRUGO, show_rc6_mask, NULL); > -static DEVICE_ATTR(rc6_residency_ms, S_IRUGO, show_rc6_ms, NULL); > -static DEVICE_ATTR(rc6p_residency_ms, S_IRUGO, show_rc6p_ms, NULL); > -static DEVICE_ATTR(rc6pp_residency_ms, S_IRUGO, show_rc6pp_ms, NULL); > -static DEVICE_ATTR(media_rc6_residency_ms, S_IRUGO, show_media_rc6_ms, NULL); > - > -static struct attribute *rc6_attrs[] = { > - &dev_attr_rc6_enable.attr, > - &dev_attr_rc6_residency_ms.attr, > - NULL > -}; > - > -static const 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 const struct attribute_group rc6p_attr_group = { > - .name = power_group_name, > - .attrs = rc6p_attrs > -}; > - > -static struct attribute *media_rc6_attrs[] = { > - &dev_attr_media_rc6_residency_ms.attr, > - NULL > -}; > - > -static const struct attribute_group media_rc6_attr_group = { > - .name = power_group_name, > - .attrs = media_rc6_attrs > -}; > -#endif > - > static int l3_access_valid(struct drm_i915_private *i915, loff_t offset) > { > if (!HAS_L3_DPF(i915)) > @@ -256,239 +155,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 = &i915->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%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 = &i915->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%d\n", > - intel_gpu_freq(rps, rps->cur_freq)); > -} > - > -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 = &i915->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%d\n", > - intel_gpu_freq(rps, rps->boost_freq)); > -} > - > -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 = &dev_priv->gt.rps; > - bool boost = false; > - ssize_t ret; > - u32 val; > - > - ret = kstrtou32(buf, 0, &val); > - if (ret) > - return ret; > - > - /* 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 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 = &dev_priv->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%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_rps *rps = &dev_priv->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%d\n", > - intel_gpu_freq(rps, rps->max_freq_softlimit)); > -} > - > -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_rps *rps = &dev_priv->gt.rps; > - ssize_t ret; > - u32 val; > - > - ret = kstrtou32(buf, 0, &val); > - if (ret) > - return ret; > - > - mutex_lock(&rps->lock); > - > - val = intel_freq_opcode(rps, val); > - if (val < rps->min_freq || > - val > rps->max_freq || > - val < rps->min_freq_softlimit) { > - ret = -EINVAL; > - goto unlock; > - } > - > - if (val > rps->rp0_freq) > - DRM_DEBUG("User requested overclocking to %d\n", > - intel_gpu_freq(rps, val)); > - > - rps->max_freq_softlimit = val; > - > - val = clamp_t(int, rps->cur_freq, > - rps->min_freq_softlimit, > - rps->max_freq_softlimit); > - > - /* > - * We still need *_set_rps to process the new max_delay and > - * update the interrupt limits and PMINTRMSK even though > - * frequency request may be unchanged. > - */ > - intel_rps_set(rps, val); > - > -unlock: > - mutex_unlock(&rps->lock); > - > - return ret ?: count; > -} > - > -static ssize_t gt_min_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 = &dev_priv->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%d\n", > - intel_gpu_freq(rps, rps->min_freq_softlimit)); > -} > - > -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 *dev_priv = kdev_minor_to_i915(kdev); > - struct intel_rps *rps = &dev_priv->gt.rps; > - ssize_t ret; > - u32 val; > - > - ret = kstrtou32(buf, 0, &val); > - if (ret) > - return ret; > - > - mutex_lock(&rps->lock); > - > - val = intel_freq_opcode(rps, val); > - if (val < rps->min_freq || > - val > rps->max_freq || > - val > rps->max_freq_softlimit) { > - ret = -EINVAL; > - goto unlock; > - } > - > - rps->min_freq_softlimit = val; > - > - val = clamp_t(int, rps->cur_freq, > - rps->min_freq_softlimit, > - rps->max_freq_softlimit); > - > - /* > - * We still need *_set_rps to process the new min_delay and > - * update the interrupt limits and PMINTRMSK even though > - * frequency request may be unchanged. > - */ > - intel_rps_set(rps, val); > - > -unlock: > - mutex_unlock(&rps->lock); > - > - 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 = &dev_priv->gt.rps; > - u32 val; > - > - if (attr == &dev_attr_gt_RP0_freq_mhz) > - val = intel_gpu_freq(rps, rps->rp0_freq); > - else if (attr == &dev_attr_gt_RP1_freq_mhz) > - val = intel_gpu_freq(rps, rps->rp1_freq); > - else if (attr == &dev_attr_gt_RPn_freq_mhz) > - val = intel_gpu_freq(rps, rps->min_freq); > - else > - BUG(); > - > - return snprintf(buf, PAGE_SIZE, "%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, > @@ -559,29 +225,6 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv) > struct device *kdev = dev_priv->drm.primary->kdev; > int ret; > > -#ifdef CONFIG_PM > - if (HAS_RC6(dev_priv)) { > - ret = sysfs_merge_group(&kdev->kobj, > - &rc6_attr_group); > - if (ret) > - drm_err(&dev_priv->drm, > - "RC6 residency sysfs setup failed\n"); > - } > - if (HAS_RC6p(dev_priv)) { > - ret = sysfs_merge_group(&kdev->kobj, > - &rc6p_attr_group); > - if (ret) > - drm_err(&dev_priv->drm, > - "RC6p residency sysfs setup failed\n"); > - } > - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > - ret = sysfs_merge_group(&kdev->kobj, > - &media_rc6_attr_group); > - if (ret) > - drm_err(&dev_priv->drm, > - "Media RC6 residency sysfs setup failed\n"); > - } > -#endif > if (HAS_L3_DPF(dev_priv)) { > ret = device_create_bin_file(kdev, &dpf_attrs); > if (ret) > @@ -597,14 +240,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 (INTEL_GEN(dev_priv) >= 6) > - ret = sysfs_create_files(&kdev->kobj, gen6_attrs); > - if (ret) > - drm_err(&dev_priv->drm, "RPS sysfs setup failed\n"); > - > i915_setup_error_capture(kdev); > } > > @@ -614,14 +249,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); > -#ifdef CONFIG_PM > - sysfs_unmerge_group(&kdev->kobj, &rc6_attr_group); > - sysfs_unmerge_group(&kdev->kobj, &rc6p_attr_group); > -#endif > } > diff --git a/drivers/gpu/drm/i915/i915_sysfs.h b/drivers/gpu/drm/i915/i915_sysfs.h > index 41afd4366416..ad6114de81c9 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.h > +++ b/drivers/gpu/drm/i915/i915_sysfs.h > @@ -6,8 +6,11 @@ > #ifndef __I915_SYSFS_H__ > #define __I915_SYSFS_H__ > > +#include <linux/device.h> > + > struct drm_i915_private; > > +struct drm_i915_private *kdev_minor_to_i915(struct device *kdev); > void i915_setup_sysfs(struct drm_i915_private *i915); > void i915_teardown_sysfs(struct drm_i915_private *i915); > > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files 2020-02-14 12:54 ` [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files Tvrtko Ursulin @ 2020-02-14 13:16 ` Andi Shyti 2020-02-14 13:38 ` Tvrtko Ursulin 2020-02-14 13:18 ` Chris Wilson 1 sibling, 1 reply; 13+ messages in thread From: Andi Shyti @ 2020-02-14 13:16 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Intel GFX Hi Tvrtko, > > The GT has its own properties and in sysfs they should be grouped > > in the 'gt/' directory. > > > > Create the 'gt/' directory in sysfs and move the power management > > related files. > > Can you paste the new and legacy paths in the commit message? sure! > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h > > index 96890dd12b5f..552a27cc0622 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h > > @@ -32,6 +32,7 @@ struct intel_gt { > > struct drm_i915_private *i915; > > struct intel_uncore *uncore; > > struct i915_ggtt *ggtt; > > + struct kobject kobj; > > sysfs_root or something like would perhaps be more descriptive? it's a kobj, but yes, I can call it that. > > +static inline struct kobject *gt_to_parent_obj(struct intel_gt *gt) > > +{ > > + return kobject_get(>->i915->drm.primary->kdev->kobj); > > It's a bit surprising X_to_Y helper get a reference as well, no? > gt_get_parent_obj perhaps? But where is this released? sure! the kobject put is handled down, for all the cases, have I missed any? > > +} > > + > > +static ssize_t gt_info_show(struct device *dev, > > + struct device_attribute *attr, > > + char *buff) > > +{ > > + return snprintf(buff, PAGE_SIZE, "0\n"); > > +} > > + > > +static DEVICE_ATTR_RO(gt_info); > > + > > +static void sysfs_gt_kobj_release(struct kobject *kobj) > > +{ > > + struct intel_gt *gt = kobj_to_gt(kobj); > > + > > + drm_info(>->i915->drm, "releasing interface\n"); > > Debugging remnants. I wanted to fill this function with a goodbye message :) > > +void intel_gt_sysfs_register(struct intel_gt *gt) > > +{ > > + struct kobject *kparent = gt_to_parent_obj(gt); > > + int ret; > > + > > + ret = kobject_init_and_add(>->kobj, &sysfs_gt_ktype, kparent, "gt"); > > + if (ret) { > > + drm_err(>->i915->drm, "failed to initialize sysfs file\n"); > > + kobject_put(>->kobj); > > So you want gt->kobj to be embedded struct and you want to then override the > release vfunc so it is not freed, but what is the specific reason you want > it embedded? it looked to me like the cleanest way. There is no real "struct device" that is containing the object I am creating, sot that the set_drvdata() was producing some unwanted effects. Embedding it in the gt, I can always get easily to the gt structure containign the kobject. > > +void intel_gt_sysfs_unregister(struct intel_gt *gt) > > +{ > > + struct kobject *root = gt_to_parent_obj(gt); > > + > > + if (>->kobj) { > > This is always true. remannt from a vim replace command :) > > + sysfs_remove_file(>->kobj, &dev_attr_gt_info.attr); > > + intel_gt_sysfs_pm_remove(gt, >->kobj); > > + kobject_put(>->kobj); > > I think kobject_put is enough to tear down the whole hierarchy so you could > simplify this. Uh! forgot that kobject was cleaning up everythign. Thanks! > > + } > > + > > + intel_gt_sysfs_pm_remove(gt, root); > > + kobject_put(root); > > Maybe stick to the same terminology regarding root and parent. yes. > Get/put on the parent looks unbalanced. Both register and unregister take a > reference and only unregister releases it. But do you even need a reference? why? I take it here: static inline struct kobject *gt_to_parent_obj(struct intel_gt *gt) { return kobject_get(>->i915->drm.primary->kdev->kobj); } at the beginning (when the driver is loaded) and I release it at the end (when the driver is unloaded). Am I not seeing something? > > +struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev) > > +{ > > + struct kobject *kobj = &dev->kobj; > > + /* > > + * We are interested at knowing from where the interface > > + * has been called, whether it's called from gt/ or from > > + * the parent directory. > > + * From the interface position it depends also the value of > > + * the private data. > > + * If the interface is called from gt/ then private data is > > + * of the "struct intel_gt *" type, otherwise it's * a > > + * "struct drm_i915_private *" type. > > + */ > > + if (strcmp(dev->kobj.name, "gt")) { > > + struct drm_i915_private *i915 = kdev_minor_to_i915(dev); > > + > > + drm_warn(&i915->drm, "the interface is obsolete, use gt/\n"); > > Can you log current->name & pid? > > I am also thinking is a level down from warn would be better. Notice sounds > intuitively correct to me. I swear, I thought hard to come up with a meaningful message, but that's the best I came up with. > I am also tempted by the _once alternative, but then it makes less sense to > include name & pid. It's true, it can be an unrelenting message, and I thought of it, but if the user is resilient at reading out from the wrong directory, why shouldn't I :) Andi _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files 2020-02-14 13:16 ` Andi Shyti @ 2020-02-14 13:38 ` Tvrtko Ursulin 2020-02-14 13:57 ` Andi Shyti 0 siblings, 1 reply; 13+ messages in thread From: Tvrtko Ursulin @ 2020-02-14 13:38 UTC (permalink / raw) To: Andi Shyti; +Cc: Intel GFX On 14/02/2020 13:16, Andi Shyti wrote: > Hi Tvrtko, > >>> The GT has its own properties and in sysfs they should be grouped >>> in the 'gt/' directory. >>> >>> Create the 'gt/' directory in sysfs and move the power management >>> related files. >> >> Can you paste the new and legacy paths in the commit message? > > sure! > >>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h >>> index 96890dd12b5f..552a27cc0622 100644 >>> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h >>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h >>> @@ -32,6 +32,7 @@ struct intel_gt { >>> struct drm_i915_private *i915; >>> struct intel_uncore *uncore; >>> struct i915_ggtt *ggtt; >>> + struct kobject kobj; >> >> sysfs_root or something like would perhaps be more descriptive? > > it's a kobj, but yes, I can call it that. > >>> +static inline struct kobject *gt_to_parent_obj(struct intel_gt *gt) >>> +{ >>> + return kobject_get(>->i915->drm.primary->kdev->kobj); >> >> It's a bit surprising X_to_Y helper get a reference as well, no? >> gt_get_parent_obj perhaps? But where is this released? > > sure! > > the kobject put is handled down, for all the cases, have I missed > any? > >>> +} >>> + >>> +static ssize_t gt_info_show(struct device *dev, >>> + struct device_attribute *attr, >>> + char *buff) >>> +{ >>> + return snprintf(buff, PAGE_SIZE, "0\n"); >>> +} >>> + >>> +static DEVICE_ATTR_RO(gt_info); >>> + >>> +static void sysfs_gt_kobj_release(struct kobject *kobj) >>> +{ >>> + struct intel_gt *gt = kobj_to_gt(kobj); >>> + >>> + drm_info(>->i915->drm, "releasing interface\n"); >> >> Debugging remnants. > > I wanted to fill this function with a goodbye message :) > >>> +void intel_gt_sysfs_register(struct intel_gt *gt) >>> +{ >>> + struct kobject *kparent = gt_to_parent_obj(gt); >>> + int ret; >>> + >>> + ret = kobject_init_and_add(>->kobj, &sysfs_gt_ktype, kparent, "gt"); >>> + if (ret) { >>> + drm_err(>->i915->drm, "failed to initialize sysfs file\n"); >>> + kobject_put(>->kobj); >> >> So you want gt->kobj to be embedded struct and you want to then override the >> release vfunc so it is not freed, but what is the specific reason you want >> it embedded? > > it looked to me like the cleanest way. > > There is no real "struct device" that is containing the object I > am creating, sot that the set_drvdata() was producing some > unwanted effects. Embedding it in the gt, I can always get > easily to the gt structure containign the kobject. Got it. > >>> +void intel_gt_sysfs_unregister(struct intel_gt *gt) >>> +{ >>> + struct kobject *root = gt_to_parent_obj(gt); >>> + >>> + if (>->kobj) { >> >> This is always true. > > remannt from a vim replace command :) > >>> + sysfs_remove_file(>->kobj, &dev_attr_gt_info.attr); >>> + intel_gt_sysfs_pm_remove(gt, >->kobj); >>> + kobject_put(>->kobj); >> >> I think kobject_put is enough to tear down the whole hierarchy so you could >> simplify this. > > Uh! forgot that kobject was cleaning up everythign. Thanks! > >>> + } >>> + >>> + intel_gt_sysfs_pm_remove(gt, root); >>> + kobject_put(root); >> >> Maybe stick to the same terminology regarding root and parent. > > yes. > >> Get/put on the parent looks unbalanced. Both register and unregister take a >> reference and only unregister releases it. But do you even need a reference? > > why? I take it here: > > static inline struct kobject *gt_to_parent_obj(struct intel_gt *gt) > { > return kobject_get(>->i915->drm.primary->kdev->kobj); > } > > at the beginning (when the driver is loaded) and I release it at > the end (when the driver is unloaded). Am I not seeing something? Gt_to_parent_obj at the top of intel_gt_sysfs_register balances out with the put at the end of the same function. What balances out gt_to_parent_obj from intel_gt_sysfs_register? > >>> +struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev) >>> +{ >>> + struct kobject *kobj = &dev->kobj; >>> + /* >>> + * We are interested at knowing from where the interface >>> + * has been called, whether it's called from gt/ or from >>> + * the parent directory. >>> + * From the interface position it depends also the value of >>> + * the private data. >>> + * If the interface is called from gt/ then private data is >>> + * of the "struct intel_gt *" type, otherwise it's * a >>> + * "struct drm_i915_private *" type. >>> + */ >>> + if (strcmp(dev->kobj.name, "gt")) { >>> + struct drm_i915_private *i915 = kdev_minor_to_i915(dev); >>> + >>> + drm_warn(&i915->drm, "the interface is obsolete, use gt/\n"); >> >> Can you log current->name & pid? >> >> I am also thinking is a level down from warn would be better. Notice sounds >> intuitively correct to me. > > I swear, I thought hard to come up with a meaningful message, but > that's the best I came up with. At least we need to mention it is about sysfs, it needs to be helpful for the userspace developer/user to know what is being access and from where. I suggested to google for this. This is what I came up with as an example: [ 775.385966] batman_adv: [Deprecated]: batadv-vis (pid 3251) Use of sysfs file "iface_status". [ 775.385966] Use batadv genl family instead I am sure there are more examples, I remember many procfs and sysfs deprecated interfaces from the past. >> I am also tempted by the _once alternative, but then it makes less sense to >> include name & pid. > > It's true, it can be an unrelenting message, and I thought of it, > but if the user is resilient at reading out from the wrong > directory, why shouldn't I :) Because we always try to avoid emitting spammy logs when they can be easily triggered by userspace. Can we do rate limit? I think that could work well with logging the process name & pid. Also, we need an entry in Documentation/ABI/obsolete/. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files 2020-02-14 13:38 ` Tvrtko Ursulin @ 2020-02-14 13:57 ` Andi Shyti 0 siblings, 0 replies; 13+ messages in thread From: Andi Shyti @ 2020-02-14 13:57 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Intel GFX Hi Tvrtko, > > > > + } > > > > + > > > > + intel_gt_sysfs_pm_remove(gt, root); > > > > + kobject_put(root); > > > > > > Maybe stick to the same terminology regarding root and parent. > > > > yes. > > > > > Get/put on the parent looks unbalanced. Both register and unregister take a > > > reference and only unregister releases it. But do you even need a reference? > > > > why? I take it here: > > > > static inline struct kobject *gt_to_parent_obj(struct intel_gt *gt) > > { > > return kobject_get(>->i915->drm.primary->kdev->kobj); > > } > > > > at the beginning (when the driver is loaded) and I release it at > > the end (when the driver is unloaded). Am I not seeing something? > > Gt_to_parent_obj at the top of intel_gt_sysfs_register balances out with the > put at the end of the same function. What balances out gt_to_parent_obj from > intel_gt_sysfs_register? And... you are right! either nothing or too many :) > > > I am also tempted by the _once alternative, but then it makes less sense to > > > include name & pid. > > > > It's true, it can be an unrelenting message, and I thought of it, > > but if the user is resilient at reading out from the wrong > > directory, why shouldn't I :) > > Because we always try to avoid emitting spammy logs when they can be easily > triggered by userspace. Can we do rate limit? I think that could work well > with logging the process name & pid. yes, if two people suggested the same thing, most probably that's the right thing to do. > Also, we need an entry in Documentation/ABI/obsolete/. I was waiting this patch to get in shape before adding the interface to obsolete. I will include it in the next patch. Thanks a lot for the review, Andi _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files 2020-02-14 12:54 ` [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files Tvrtko Ursulin 2020-02-14 13:16 ` Andi Shyti @ 2020-02-14 13:18 ` Chris Wilson 2020-02-14 13:41 ` Tvrtko Ursulin 1 sibling, 1 reply; 13+ messages in thread From: Chris Wilson @ 2020-02-14 13:18 UTC (permalink / raw) To: Andi Shyti, Intel GFX, Tvrtko Ursulin Quoting Tvrtko Ursulin (2020-02-14 12:54:35) > > On 14/02/2020 11:03, Andi Shyti wrote: > > +struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev) > > +{ > > + struct kobject *kobj = &dev->kobj; > > + /* > > + * We are interested at knowing from where the interface > > + * has been called, whether it's called from gt/ or from > > + * the parent directory. > > + * From the interface position it depends also the value of > > + * the private data. > > + * If the interface is called from gt/ then private data is > > + * of the "struct intel_gt *" type, otherwise it's * a > > + * "struct drm_i915_private *" type. > > + */ > > + if (strcmp(dev->kobj.name, "gt")) { > > + struct drm_i915_private *i915 = kdev_minor_to_i915(dev); > > + > > + drm_warn(&i915->drm, "the interface is obsolete, use gt/\n"); > > Can you log current->name & pid? > > I am also thinking is a level down from warn would be better. Notice > sounds intuitively correct to me. git grep -e 'pr.*obsolete' | grep warn | wc -l 21 git grep -e 'pr.*obsolete' | grep notice | wc -l 1 git grep -e 'pr.*obsolete' | grep info | wc -l 4 Looks like warn's back on the menu, boys. > I am also tempted by the _once alternative, but then it makes less sense > to include name & pid. I'm more afraid that there are users out there that frequently poke these files. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files 2020-02-14 13:18 ` Chris Wilson @ 2020-02-14 13:41 ` Tvrtko Ursulin 0 siblings, 0 replies; 13+ messages in thread From: Tvrtko Ursulin @ 2020-02-14 13:41 UTC (permalink / raw) To: Chris Wilson, Andi Shyti, Intel GFX On 14/02/2020 13:18, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2020-02-14 12:54:35) >> >> On 14/02/2020 11:03, Andi Shyti wrote: >>> +struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev) >>> +{ >>> + struct kobject *kobj = &dev->kobj; >>> + /* >>> + * We are interested at knowing from where the interface >>> + * has been called, whether it's called from gt/ or from >>> + * the parent directory. >>> + * From the interface position it depends also the value of >>> + * the private data. >>> + * If the interface is called from gt/ then private data is >>> + * of the "struct intel_gt *" type, otherwise it's * a >>> + * "struct drm_i915_private *" type. >>> + */ >>> + if (strcmp(dev->kobj.name, "gt")) { >>> + struct drm_i915_private *i915 = kdev_minor_to_i915(dev); >>> + >>> + drm_warn(&i915->drm, "the interface is obsolete, use gt/\n"); >> >> Can you log current->name & pid? >> >> I am also thinking is a level down from warn would be better. Notice >> sounds intuitively correct to me. > > git grep -e 'pr.*obsolete' | grep warn | wc -l > 21 > git grep -e 'pr.*obsolete' | grep notice | wc -l > 1 > git grep -e 'pr.*obsolete' | grep info | wc -l > 4 > > Looks like warn's back on the menu, boys. Majority is just wrong. :P >> I am also tempted by the _once alternative, but then it makes less sense >> to include name & pid. > > I'm more afraid that there are users out there that frequently poke > these files. Agreed, I think best option is to go with ratelimited and name & pid logged. And more verbosity about what has been access and what should be accessed instead. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: make a gt sysfs group and move power management files (rev3) 2020-02-14 11:03 [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files Andi Shyti ` (2 preceding siblings ...) 2020-02-14 12:54 ` [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files Tvrtko Ursulin @ 2020-02-14 13:13 ` Patchwork 2020-02-14 13:24 ` Chris Wilson 2020-02-14 13:14 ` [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files Chris Wilson 2020-02-17 19:04 ` [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/gt: make a gt sysfs group and move power management files (rev3) Patchwork 5 siblings, 1 reply; 13+ messages in thread From: Patchwork @ 2020-02-14 13:13 UTC (permalink / raw) To: Andi Shyti; +Cc: intel-gfx == Series Details == Series: drm/i915/gt: make a gt sysfs group and move power management files (rev3) URL : https://patchwork.freedesktop.org/series/73190/ State : success == Summary == CI Bug Log - changes from CI_DRM_7939 -> Patchwork_16569 ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/index.html Known issues ------------ Here are the changes found in Patchwork_16569 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@i915_selftest@live_gem_contexts: - fi-cml-s: [PASS][1] -> [DMESG-FAIL][2] ([i915#877]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/fi-cml-s/igt@i915_selftest@live_gem_contexts.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/fi-cml-s/igt@i915_selftest@live_gem_contexts.html #### Possible fixes #### * igt@gem_exec_parallel@contexts: - {fi-ehl-1}: [INCOMPLETE][3] ([i915#937]) -> [PASS][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/fi-ehl-1/igt@gem_exec_parallel@contexts.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/fi-ehl-1/igt@gem_exec_parallel@contexts.html * igt@i915_selftest@live_execlists: - fi-icl-y: [DMESG-FAIL][5] ([fdo#108569]) -> [PASS][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/fi-icl-y/igt@i915_selftest@live_execlists.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/fi-icl-y/igt@i915_selftest@live_execlists.html #### Warnings #### * igt@gem_close_race@basic-threads: - fi-byt-j1900: [INCOMPLETE][7] ([i915#45]) -> [TIMEOUT][8] ([fdo#112271] / [i915#1084] / [i915#816]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/fi-byt-j1900/igt@gem_close_race@basic-threads.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/fi-byt-j1900/igt@gem_close_race@basic-threads.html * igt@i915_pm_rpm@basic-rte: - fi-kbl-guc: [FAIL][9] ([i915#579]) -> [SKIP][10] ([fdo#109271]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/fi-kbl-guc/igt@i915_pm_rpm@basic-rte.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/fi-kbl-guc/igt@i915_pm_rpm@basic-rte.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569 [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [fdo#112271]: https://bugs.freedesktop.org/show_bug.cgi?id=112271 [i915#1084]: https://gitlab.freedesktop.org/drm/intel/issues/1084 [i915#45]: https://gitlab.freedesktop.org/drm/intel/issues/45 [i915#579]: https://gitlab.freedesktop.org/drm/intel/issues/579 [i915#816]: https://gitlab.freedesktop.org/drm/intel/issues/816 [i915#877]: https://gitlab.freedesktop.org/drm/intel/issues/877 [i915#937]: https://gitlab.freedesktop.org/drm/intel/issues/937 Participating hosts (44 -> 43) ------------------------------ Additional (7): fi-hsw-peppy fi-glk-dsi fi-ivb-3770 fi-icl-u3 fi-bsw-kefka fi-skl-6600u fi-kbl-r Missing (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-ctg-p8600 fi-byt-n2820 fi-byt-clapper fi-bdw-samus fi-snb-2600 Build changes ------------- * CI: CI-20190529 -> None * Linux: CI_DRM_7939 -> Patchwork_16569 CI-20190529: 20190529 CI_DRM_7939: cceb0c30a34af6ca96e35211ecdc5ca198d44e7e @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5441: 534ca091fe4ffed916752165bc5becd7ff56cd84 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_16569: 2bb6ef6acc00a99d609b52064cf9baa76bb79004 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 2bb6ef6acc00 drm/i915/gt: make a gt sysfs group and move power management files == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/index.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: make a gt sysfs group and move power management files (rev3) 2020-02-14 13:13 ` [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: make a gt sysfs group and move power management files (rev3) Patchwork @ 2020-02-14 13:24 ` Chris Wilson 0 siblings, 0 replies; 13+ messages in thread From: Chris Wilson @ 2020-02-14 13:24 UTC (permalink / raw) To: Andi Shyti, Patchwork, intel-gfx; +Cc: intel-gfx Quoting Patchwork (2020-02-14 13:13:51) > == Series Details == > > Series: drm/i915/gt: make a gt sysfs group and move power management files (rev3) > URL : https://patchwork.freedesktop.org/series/73190/ > State : success > > == Summary == > > CI Bug Log - changes from CI_DRM_7939 -> Patchwork_16569 > ==================================================== > > Summary > ------- > > **SUCCESS** > > No regressions found. > > External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/index.html Hmm, do we not have the sysfs walker in BAT? Apparently not. This *should* produce a warning :) -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files 2020-02-14 11:03 [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files Andi Shyti ` (3 preceding siblings ...) 2020-02-14 13:13 ` [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: make a gt sysfs group and move power management files (rev3) Patchwork @ 2020-02-14 13:14 ` Chris Wilson 2020-02-17 19:04 ` [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/gt: make a gt sysfs group and move power management files (rev3) Patchwork 5 siblings, 0 replies; 13+ messages in thread From: Chris Wilson @ 2020-02-14 13:14 UTC (permalink / raw) To: Andi Shyti, Intel GFX Quoting Andi Shyti (2020-02-14 11:03:08) > +struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev) > +{ > + struct kobject *kobj = &dev->kobj; > + /* > + * We are interested at knowing from where the interface > + * has been called, whether it's called from gt/ or from > + * the parent directory. > + * From the interface position it depends also the value of > + * the private data. > + * If the interface is called from gt/ then private data is > + * of the "struct intel_gt *" type, otherwise it's * a > + * "struct drm_i915_private *" type. > + */ > + if (strcmp(dev->kobj.name, "gt")) { > + struct drm_i915_private *i915 = kdev_minor_to_i915(dev); > + > + drm_warn(&i915->drm, "the interface is obsolete, use gt/\n"); > + return &i915->gt; I guess it's not possible to flesh this out with path? And we do need it to be warn_once else the user will be flooded. One final request, can we also put the old entries under CONFIG_DRM_I915_SYSFS_OBSOLETE_GT (or somesuch) As far as the naming goes, the compromise isn't horrendous. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/gt: make a gt sysfs group and move power management files (rev3) 2020-02-14 11:03 [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files Andi Shyti ` (4 preceding siblings ...) 2020-02-14 13:14 ` [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files Chris Wilson @ 2020-02-17 19:04 ` Patchwork 5 siblings, 0 replies; 13+ messages in thread From: Patchwork @ 2020-02-17 19:04 UTC (permalink / raw) To: Andi Shyti; +Cc: intel-gfx == Series Details == Series: drm/i915/gt: make a gt sysfs group and move power management files (rev3) URL : https://patchwork.freedesktop.org/series/73190/ State : failure == Summary == CI Bug Log - changes from CI_DRM_7939_full -> Patchwork_16569_full ==================================================== Summary ------- **FAILURE** Serious unknown changes coming with Patchwork_16569_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_16569_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. Possible new issues ------------------- Here are the unknown changes that may have been introduced in Patchwork_16569_full: ### IGT changes ### #### Possible regressions #### * igt@i915_pm_rc6_residency@rc6-idle: - shard-tglb: [PASS][1] -> [SKIP][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-tglb3/igt@i915_pm_rc6_residency@rc6-idle.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-tglb3/igt@i915_pm_rc6_residency@rc6-idle.html * igt@perf@rc6-disable: - shard-iclb: [PASS][3] -> [SKIP][4] +2 similar issues [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-iclb1/igt@perf@rc6-disable.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-iclb4/igt@perf@rc6-disable.html #### Warnings #### * igt@i915_pm_rc6_residency@media-rc6-accuracy: - shard-iclb: [SKIP][5] ([fdo#109289]) -> [SKIP][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-iclb3/igt@i915_pm_rc6_residency@media-rc6-accuracy.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-iclb1/igt@i915_pm_rc6_residency@media-rc6-accuracy.html #### Suppressed #### The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * {igt@gem_ctx_persistence@close-replace-race}: - shard-iclb: [PASS][7] -> [FAIL][8] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-iclb5/igt@gem_ctx_persistence@close-replace-race.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-iclb5/igt@gem_ctx_persistence@close-replace-race.html Known issues ------------ Here are the changes found in Patchwork_16569_full that come from known issues: ### IGT changes ### #### Issues hit #### * igt@gem_ctx_persistence@processes: - shard-skl: [PASS][9] -> [FAIL][10] ([i915#570] / [i915#679]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-skl1/igt@gem_ctx_persistence@processes.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-skl6/igt@gem_ctx_persistence@processes.html * igt@gem_exec_await@wide-contexts: - shard-kbl: [PASS][11] -> [INCOMPLETE][12] ([fdo#103665]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-kbl6/igt@gem_exec_await@wide-contexts.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-kbl3/igt@gem_exec_await@wide-contexts.html * igt@gem_exec_schedule@in-order-bsd: - shard-iclb: [PASS][13] -> [SKIP][14] ([fdo#112146]) +6 similar issues [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-iclb8/igt@gem_exec_schedule@in-order-bsd.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-iclb4/igt@gem_exec_schedule@in-order-bsd.html * igt@gem_exec_schedule@pi-shared-iova-bsd: - shard-iclb: [PASS][15] -> [SKIP][16] ([i915#677]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-iclb3/igt@gem_exec_schedule@pi-shared-iova-bsd.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-iclb2/igt@gem_exec_schedule@pi-shared-iova-bsd.html * igt@gem_linear_blits@normal: - shard-hsw: [PASS][17] -> [FAIL][18] ([i915#694]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-hsw6/igt@gem_linear_blits@normal.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-hsw1/igt@gem_linear_blits@normal.html * igt@gen9_exec_parse@allowed-single: - shard-skl: [PASS][19] -> [DMESG-WARN][20] ([i915#716]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-skl1/igt@gen9_exec_parse@allowed-single.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-skl6/igt@gen9_exec_parse@allowed-single.html * igt@i915_pm_rc6_residency@rc6-accuracy: - shard-tglb: [PASS][21] -> [SKIP][22] ([fdo#111719]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-tglb3/igt@i915_pm_rc6_residency@rc6-accuracy.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-tglb6/igt@i915_pm_rc6_residency@rc6-accuracy.html * igt@i915_pm_rc6_residency@rc6-idle: - shard-snb: [PASS][23] -> [SKIP][24] ([fdo#109271]) +1 similar issue [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-snb2/igt@i915_pm_rc6_residency@rc6-idle.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-snb4/igt@i915_pm_rc6_residency@rc6-idle.html - shard-skl: [PASS][25] -> [SKIP][26] ([fdo#109271]) +2 similar issues [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-skl6/igt@i915_pm_rc6_residency@rc6-idle.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-skl9/igt@i915_pm_rc6_residency@rc6-idle.html - shard-apl: [PASS][27] -> [SKIP][28] ([fdo#109271]) +2 similar issues [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-apl7/igt@i915_pm_rc6_residency@rc6-idle.html [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-apl3/igt@i915_pm_rc6_residency@rc6-idle.html - shard-glk: [PASS][29] -> [SKIP][30] ([fdo#109271]) +2 similar issues [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-glk1/igt@i915_pm_rc6_residency@rc6-idle.html [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-glk2/igt@i915_pm_rc6_residency@rc6-idle.html * igt@i915_suspend@forcewake: - shard-kbl: [PASS][31] -> [DMESG-WARN][32] ([i915#180]) +1 similar issue [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-kbl4/igt@i915_suspend@forcewake.html [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-kbl2/igt@i915_suspend@forcewake.html * igt@kms_flip@flip-vs-expired-vblank-interruptible: - shard-skl: [PASS][33] -> [FAIL][34] ([i915#79]) [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-skl7/igt@kms_flip@flip-vs-expired-vblank-interruptible.html [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-skl3/igt@kms_flip@flip-vs-expired-vblank-interruptible.html * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-pwrite: - shard-tglb: [PASS][35] -> [SKIP][36] ([i915#668]) +5 similar issues [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-tglb8/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-pwrite.html [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-tglb8/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-pwrite.html * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes: - shard-apl: [PASS][37] -> [DMESG-WARN][38] ([i915#180]) +1 similar issue [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-apl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html - shard-iclb: [PASS][39] -> [INCOMPLETE][40] ([i915#250]) [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-iclb8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-iclb3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc: - shard-skl: [PASS][41] -> [FAIL][42] ([fdo#108145] / [i915#265]) [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html * igt@kms_psr@no_drrs: - shard-iclb: [PASS][43] -> [FAIL][44] ([i915#173]) [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-iclb6/igt@kms_psr@no_drrs.html [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-iclb1/igt@kms_psr@no_drrs.html * igt@kms_psr@psr2_primary_mmap_cpu: - shard-iclb: [PASS][45] -> [SKIP][46] ([fdo#109441]) +1 similar issue [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-iclb8/igt@kms_psr@psr2_primary_mmap_cpu.html * igt@kms_setmode@basic: - shard-kbl: [PASS][47] -> [FAIL][48] ([i915#31]) [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-kbl7/igt@kms_setmode@basic.html [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-kbl3/igt@kms_setmode@basic.html * igt@perf@rc6-disable: - shard-hsw: [PASS][49] -> [SKIP][50] ([fdo#109271]) +2 similar issues [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-hsw4/igt@perf@rc6-disable.html [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-hsw5/igt@perf@rc6-disable.html - shard-kbl: [PASS][51] -> [SKIP][52] ([fdo#109271]) +2 similar issues [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-kbl3/igt@perf@rc6-disable.html [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-kbl4/igt@perf@rc6-disable.html * igt@perf_pmu@busy-vcs1: - shard-iclb: [PASS][53] -> [SKIP][54] ([fdo#112080]) +12 similar issues [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-iclb1/igt@perf_pmu@busy-vcs1.html [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-iclb3/igt@perf_pmu@busy-vcs1.html * igt@prime_busy@hang-bsd2: - shard-iclb: [PASS][55] -> [SKIP][56] ([fdo#109276]) +19 similar issues [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-iclb2/igt@prime_busy@hang-bsd2.html [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-iclb7/igt@prime_busy@hang-bsd2.html #### Possible fixes #### * igt@gem_ctx_isolation@rcs0-s3: - shard-kbl: [DMESG-WARN][57] ([i915#180]) -> [PASS][58] +7 similar issues [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-kbl6/igt@gem_ctx_isolation@rcs0-s3.html [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-kbl1/igt@gem_ctx_isolation@rcs0-s3.html * {igt@gem_ctx_persistence@close-replace-race}: - shard-glk: [FAIL][59] -> [PASS][60] [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-glk8/igt@gem_ctx_persistence@close-replace-race.html [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-glk4/igt@gem_ctx_persistence@close-replace-race.html * igt@gem_exec_schedule@pi-common-bsd1: - shard-iclb: [SKIP][61] ([fdo#109276]) -> [PASS][62] +20 similar issues [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-iclb3/igt@gem_exec_schedule@pi-common-bsd1.html [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-iclb1/igt@gem_exec_schedule@pi-common-bsd1.html * igt@gem_exec_schedule@preempt-other-chain-bsd: - shard-iclb: [SKIP][63] ([fdo#112146]) -> [PASS][64] +8 similar issues [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-iclb2/igt@gem_exec_schedule@preempt-other-chain-bsd.html [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-iclb7/igt@gem_exec_schedule@preempt-other-chain-bsd.html * igt@gem_workarounds@suspend-resume-context: - shard-apl: [DMESG-WARN][65] ([i915#180]) -> [PASS][66] +2 similar issues [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-apl6/igt@gem_workarounds@suspend-resume-context.html [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-apl1/igt@gem_workarounds@suspend-resume-context.html * igt@kms_flip@plain-flip-fb-recreate: - shard-kbl: [FAIL][67] ([i915#34]) -> [PASS][68] [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-kbl4/igt@kms_flip@plain-flip-fb-recreate.html [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-kbl2/igt@kms_flip@plain-flip-fb-recreate.html * igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-mmap-cpu: - shard-tglb: [SKIP][69] ([i915#668]) -> [PASS][70] +9 similar issues [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-tglb6/igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-mmap-cpu.html [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-tglb6/igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-mmap-cpu.html * igt@kms_psr@psr2_suspend: - shard-iclb: [SKIP][71] ([fdo#109441]) -> [PASS][72] +1 similar issue [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-iclb3/igt@kms_psr@psr2_suspend.html [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-iclb2/igt@kms_psr@psr2_suspend.html * igt@kms_setmode@basic: - shard-apl: [FAIL][73] ([i915#31]) -> [PASS][74] [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-apl1/igt@kms_setmode@basic.html [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-apl1/igt@kms_setmode@basic.html * igt@perf_pmu@busy-check-all-vcs1: - shard-iclb: [SKIP][75] ([fdo#112080]) -> [PASS][76] +12 similar issues [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-iclb6/igt@perf_pmu@busy-check-all-vcs1.html [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-iclb2/igt@perf_pmu@busy-check-all-vcs1.html #### Warnings #### * igt@gem_ctx_isolation@vcs1-nonpriv: - shard-iclb: [SKIP][77] ([fdo#112080]) -> [FAIL][78] ([IGT#28]) +1 similar issue [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-iclb3/igt@gem_ctx_isolation@vcs1-nonpriv.html [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-iclb1/igt@gem_ctx_isolation@vcs1-nonpriv.html * igt@gem_tiled_blits@interruptible: - shard-hsw: [FAIL][79] ([i915#818]) -> [FAIL][80] ([i915#694]) [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-hsw8/igt@gem_tiled_blits@interruptible.html [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-hsw8/igt@gem_tiled_blits@interruptible.html * igt@gem_tiled_blits@normal: - shard-hsw: [FAIL][81] ([i915#694]) -> [FAIL][82] ([i915#818]) [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-hsw1/igt@gem_tiled_blits@normal.html [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-hsw7/igt@gem_tiled_blits@normal.html * igt@i915_pm_dc@dc6-psr: - shard-tglb: [SKIP][83] ([i915#468]) -> [FAIL][84] ([i915#454]) [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-tglb2/igt@i915_pm_dc@dc6-psr.html [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-tglb1/igt@i915_pm_dc@dc6-psr.html * igt@i915_pm_rc6_residency@media-rc6-accuracy: - shard-tglb: [SKIP][85] ([fdo#109289] / [fdo#111719]) -> [SKIP][86] ([fdo#111719]) [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7939/shard-tglb1/igt@i915_pm_rc6_residency@media-rc6-accuracy.html [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/shard-tglb7/igt@i915_pm_rc6_residency@media-rc6-accuracy.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [IGT#28]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/28 [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665 [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145 [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276 [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289 [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441 [fdo#111719]: https://bugs.freedesktop.org/show_bug.cgi?id=111719 [fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080 [fdo#112146]: https://bugs.freedesktop.org/show_bug.cgi?id=112146 [i915#173]: https://gitlab.freedesktop.org/drm/intel/issues/173 [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180 [i915#250]: https://gitlab.freedesktop.org/drm/intel/issues/250 [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265 [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31 [i915#34]: https://gitlab.freedesktop.org/drm/intel/issues/34 [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454 [i915#468]: https://gitlab.freedesktop.org/drm/intel/issues/468 [i915#570]: https://gitlab.freedesktop.org/drm/intel/issues/570 [i915#668]: https://gitlab.freedesktop.org/drm/intel/issues/668 [i915#677]: https://gitlab.freedesktop.org/drm/intel/issues/677 [i915#679]: https://gitlab.freedesktop.org/drm/intel/issues/679 [i915#694]: https://gitlab.freedesktop.org/drm/intel/issues/694 [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716 [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79 [i915#818]: https://gitlab.freedesktop.org/drm/intel/issues/818 Participating hosts (10 -> 10) ------------------------------ No changes in participating hosts Build changes ------------- * CI: CI-20190529 -> None * Linux: CI_DRM_7939 -> Patchwork_16569 CI-20190529: 20190529 CI_DRM_7939: cceb0c30a34af6ca96e35211ecdc5ca198d44e7e @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5441: 534ca091fe4ffed916752165bc5becd7ff56cd84 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_16569: 2bb6ef6acc00a99d609b52064cf9baa76bb79004 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16569/index.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-02-17 19:04 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-14 11:03 [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files Andi Shyti 2020-02-14 12:51 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gt: make a gt sysfs group and move power management files (rev3) Patchwork 2020-02-14 12:52 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork 2020-02-14 12:54 ` [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files Tvrtko Ursulin 2020-02-14 13:16 ` Andi Shyti 2020-02-14 13:38 ` Tvrtko Ursulin 2020-02-14 13:57 ` Andi Shyti 2020-02-14 13:18 ` Chris Wilson 2020-02-14 13:41 ` Tvrtko Ursulin 2020-02-14 13:13 ` [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: make a gt sysfs group and move power management files (rev3) Patchwork 2020-02-14 13:24 ` Chris Wilson 2020-02-14 13:14 ` [Intel-gfx] [PATCH] drm/i915/gt: make a gt sysfs group and move power management files Chris Wilson 2020-02-17 19:04 ` [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/gt: make a gt sysfs group and move power management files (rev3) Patchwork
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).