Re: [PATCH v3 15/15] drm/i915/gt: Allow the user to change the CCS mode through sysfs

[Daniel Vetter <daniel.vetter@ffwll.ch>]

On Fri, Aug 23, 2024 at 03:08:55PM +0200, Andi Shyti wrote:
> Create the 'ccs_mode' file under
> 
> /sys/class/drm/cardX/gt/gt0/ccs_mode
> 
> This file allows the user to read and set the current CCS mode.
> 
>  - Reading: The user can read the current CCS mode, which can be
>    1, 2, or 4. This value is derived from the current engine
>    mask.
> 
>  - Writing: The user can set the CCS mode to 1, 2, or 4,
>    depending on the desired number of exposed engines and the
>    required load balancing.
> 
> The interface will return -EBUSY if other clients are connected
> to i915, or -EINVAL if an invalid value is set.

This does not agree with the code. The code is a _lot_ more clever.
-Sima

> 
> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.c | 82 ++++++++++++++++++++-
>  1 file changed, 80 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.c b/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.c
> index cc46ee9dea3f..1ed6153ff8cf 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.c
> @@ -6,6 +6,7 @@
>  #include "i915_drv.h"
>  #include "intel_engine_user.h"
>  #include "intel_gt_ccs_mode.h"
> +#include "intel_gt_pm.h"
>  #include "intel_gt_print.h"
>  #include "intel_gt_regs.h"
>  #include "intel_gt_sysfs.h"
> @@ -172,7 +173,7 @@ static int rb_engine_cmp(struct rb_node *rb_new, const struct rb_node *rb_old)
>  	return new->uabi_class - old->uabi_class;
>  }
>  
> -static void __maybe_unused add_uabi_ccs_engines(struct intel_gt *gt, u32 ccs_mode)
> +static void add_uabi_ccs_engines(struct intel_gt *gt, u32 ccs_mode)
>  {
>  	struct drm_i915_private *i915 = gt->i915;
>  	intel_engine_mask_t new_ccs_mask, tmp;
> @@ -230,7 +231,7 @@ static void __maybe_unused add_uabi_ccs_engines(struct intel_gt *gt, u32 ccs_mod
>  	mutex_unlock(&i915->uabi_engines_mutex);
>  }
>  
> -static void __maybe_unused remove_uabi_ccs_engines(struct intel_gt *gt, u8 ccs_mode)
> +static void remove_uabi_ccs_engines(struct intel_gt *gt, u8 ccs_mode)
>  {
>  	struct drm_i915_private *i915 = gt->i915;
>  	intel_engine_mask_t new_ccs_mask, tmp;
> @@ -273,8 +274,85 @@ static ssize_t num_cslices_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(num_cslices);
>  
> +static ssize_t ccs_mode_show(struct device *dev,
> +			     struct device_attribute *attr, char *buff)
> +{
> +	struct intel_gt *gt = kobj_to_gt(&dev->kobj);
> +	u32 ccs_mode;
> +
> +	ccs_mode = hweight32(gt->ccs.id_mask);
> +
> +	return sysfs_emit(buff, "%u\n", ccs_mode);
> +}
> +
> +static ssize_t ccs_mode_store(struct device *dev,
> +			      struct device_attribute *attr,
> +			      const char *buff, size_t count)
> +{
> +	struct intel_gt *gt = kobj_to_gt(&dev->kobj);
> +	int num_cslices = hweight32(CCS_MASK(gt));
> +	int ccs_mode = hweight32(gt->ccs.id_mask);
> +	ssize_t ret;
> +	u32 val;
> +
> +	ret = kstrtou32(buff, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * As of now possible values to be set are 1, 2, 4,
> +	 * up to the maximum number of available slices
> +	 */
> +	if (!val || val > num_cslices || (num_cslices % val))
> +		return -EINVAL;
> +
> +	/* Let's wait until the GT is no longer in use */
> +	ret = intel_gt_pm_wait_for_idle(gt);
> +	if (ret)
> +		return ret;
> +
> +	mutex_lock(&gt->wakeref.mutex);
> +
> +	/*
> +	 * Let's check again that the GT is idle,
> +	 * we don't want to change the CCS mode
> +	 * while someone is using the GT
> +	 */
> +	if (intel_gt_pm_is_awake(gt)) {
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	/*
> +	 * Nothing to do if the requested setting
> +	 * is the same as the current one
> +	 */
> +	if (val == ccs_mode)
> +		goto out;
> +	else if (val > ccs_mode)
> +		add_uabi_ccs_engines(gt, val);
> +	else
> +		remove_uabi_ccs_engines(gt, val);
> +
> +out:
> +	mutex_unlock(&gt->wakeref.mutex);
> +
> +	return ret ?: count;
> +}
> +static DEVICE_ATTR_RW(ccs_mode);
> +
>  void intel_gt_sysfs_ccs_init(struct intel_gt *gt)
>  {
>  	if (sysfs_create_file(&gt->sysfs_gt, &dev_attr_num_cslices.attr))
>  		gt_warn(gt, "Failed to create sysfs num_cslices files\n");
> +
> +	/*
> +	 * Do not create the ccs_mode file for non DG2 platforms
> +	 * because they don't need it as they have only one CCS engine
> +	 */
> +	if (!IS_DG2(gt->i915))
> +		return;
> +
> +	if (sysfs_create_file(&gt->sysfs_gt, &dev_attr_ccs_mode.attr))
> +		gt_warn(gt, "Failed to create sysfs ccs_mode files\n");
>  }
> -- 
> 2.45.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch