On Mon, Sep 23, 2013 at 05:33:30PM -0300, Rodrigo Vivi wrote:
> Slice shutdown is a power savings feature whereby parts of HW i.e. slice is
> shut off on boot or dynamically to save power.
>
> This patch introduces a sysfs interface to easily allow dynamically switch
> between full and half GT3 slices.
>
> v2: remove unused variables and fix identation
gt3_policy is a misnomer, what you have is more of a "slice config".
I'm not convinced by the boot parameter and want to see how this
interacts with the automatic configuration before declaring any
interface sign.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_sysfs.c | 48 +++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 4 +++-
> drivers/gpu/drm/i915/intel_pm.c | 31 +++++++++++++++++++++++++
> 3 files changed, 82 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index 7659dd4..40f23a8 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -97,6 +97,48 @@ static struct attribute_group rc6_attr_group = {
> .name = power_group_name,
> .attrs = rc6_attrs
> };
> +
> +static ssize_t gt3_policy_show(struct device *kdev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
> + struct drm_device *dev = minor->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + return sprintf(buf, "%s\n", I915_READ(MI_PREDICATE_RESULT_2) ==
> + LOWER_SLICE_ENABLED ? "full" : "half");
> +}
> +
> +static ssize_t gt3_policy_store(struct device *kdev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
> + struct drm_device *dev = minor->dev;
> +
> + if (!strncmp(buf, "full", sizeof("full") - 1))
> + intel_set_gt3_full(dev);
Be strict and use strcmp(buf, "full") == 0
> + else if (!strncmp(buf, "half", sizeof("half") - 1))
> + intel_set_gt3_half(dev);
> + else
> + return -EINVAL;
This probably wants to be synchronous and not return before the change
has taken effect.
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(gt3_policy, S_IRUGO | S_IWUSR,
> + gt3_policy_show, gt3_policy_store);
> +
> +static struct attribute *gt3_policy_attrs[] = {
> + &dev_attr_gt3_policy.attr,
> + NULL
> +};
> +
> +static struct attribute_group gt3_policy_attr_group = {
> + .name = power_group_name,
> + .attrs = gt3_policy_attrs
> +};
> +
> #endif
>
> static int l3_access_valid(struct drm_device *dev, loff_t offset)
> @@ -528,6 +570,12 @@ void i915_setup_sysfs(struct drm_device *dev)
> if (ret)
> DRM_ERROR("RC6 residency sysfs setup failed\n");
> }
> + if (IS_HSW_GT3(dev)) {
> + ret = sysfs_merge_group(&dev->primary->kdev.kobj,
> + >3_policy_attr_group);
> + if (ret)
> + DRM_ERROR("GT3 policy sysfs setup failed\n");
Shouldn't there be a remove as well?
> + }
> #endif
> if (HAS_L3_DPF(dev)) {
> ret = device_create_bin_file(&dev->primary->kdev, &dpf_attrs);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f2ef5cb..2d28289 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -756,7 +756,9 @@ extern void intel_update_fbc(struct drm_device *dev);
> /* IPS */
> extern void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
> extern void intel_gpu_ips_teardown(void);
> -
> +/* Slice Shutdown */
> +extern void intel_set_gt3_full(struct drm_device *dev);
> +extern void intel_set_gt3_half(struct drm_device *dev);
> /* Power well */
> extern int i915_init_power_well(struct drm_device *dev);
> extern void i915_remove_power_well(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index c03566e..e7a987f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3712,6 +3712,37 @@ static void gen6_enable_rps(struct drm_device *dev)
> gen6_gt_force_wake_put(dev_priv);
> }
>
> +void intel_set_gt3_full(struct drm_device *dev)
Just one function will do,
intel_set_slice_config(dev, enum slice_config config);
(and if the boot parameter remains it should call into it)
I don't think you should be changing MI_PREDICATE whilst the (a) the
slice configuration has not taken effect, (b) whilst the GPU is active,
(c) those are indeed the same.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
