[Intel-xe] [RFC v3 1/2] drm/xe: Enable Fixed CCS mode setting
Niranjana Vishwanathapura
niranjana.vishwanathapura at intel.com
Thu Nov 16 03:27:57 UTC 2023
On Tue, Nov 14, 2023 at 04:53:51PM -0800, Matt Roper wrote:
>On Mon, Nov 13, 2023 at 04:46:03PM -0800, Niranjana Vishwanathapura wrote:
>> Disable dynamic HW load balancing of compute resource assignment
>> to engines and instead enabled fixed mode of mapping compute
>> resources to engines.
>
>Since the code below is only doing this for PVC and not for other
>platforms like DG2/ATS-M, we should clarify that in the commit message.
>And also, this setting is mandatory on PVC due to Wa_16016805146 so we
>probably want to state that as well so that nobody gets confused down
>the road and thinks this was an optional / arbitrary decision.
>
Ok, will add
>>
>> By default enable only one CCS engine with all compute slices
>> assigned to it. This seems to be the desired configuration for
>> common workloads.
>>
>> Allow user to configure this CCS mode setting through a 'ccs_mode'
>> sysfs entry.
>
>I'm wondering if we should break out the sysfs stuff to a separate
>commit. Do the UMDs already have their support ready that's necessary
>for us to land this new uapi in the KMD?
>
Ok, will move it a separate patch here
>>
>> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura at intel.com>
>> ---
>> drivers/gpu/drm/xe/Makefile | 1 +
>> drivers/gpu/drm/xe/regs/xe_gt_regs.h | 14 ++++++
>> drivers/gpu/drm/xe/xe_gt.c | 10 ++++
>> drivers/gpu/drm/xe/xe_gt.h | 2 +
>> drivers/gpu/drm/xe/xe_gt_ccs_mode.c | 73 ++++++++++++++++++++++++++++
>> drivers/gpu/drm/xe/xe_gt_ccs_mode.h | 22 +++++++++
>> drivers/gpu/drm/xe/xe_gt_sysfs.c | 53 ++++++++++++++++++++
>> drivers/gpu/drm/xe/xe_gt_types.h | 14 ++++++
>> drivers/gpu/drm/xe/xe_guc_ads.c | 3 ++
>> drivers/gpu/drm/xe/xe_hw_engine.c | 20 ++++++++
>> 10 files changed, 212 insertions(+)
>> create mode 100644 drivers/gpu/drm/xe/xe_gt_ccs_mode.c
>> create mode 100644 drivers/gpu/drm/xe/xe_gt_ccs_mode.h
>>
>> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>> index a1a8847e2ba3..dab5c9a8ad44 100644
>> --- a/drivers/gpu/drm/xe/Makefile
>> +++ b/drivers/gpu/drm/xe/Makefile
>> @@ -58,6 +58,7 @@ xe-y += xe_bb.o \
>> xe_force_wake.o \
>> xe_ggtt.o \
>> xe_gt.o \
>> + xe_gt_ccs_mode.o \
>> xe_gt_clock.o \
>> xe_gt_debugfs.o \
>> xe_gt_idle_sysfs.o \
>> diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> index cc27fe8fc363..576b72000ae9 100644
>> --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> @@ -395,8 +395,22 @@
>> #define COMP_CKN_IN REG_GENMASK(30, 29)
>>
>> #define RCU_MODE XE_REG(0x14800, XE_REG_OPTION_MASKED)
>> +#define RCU_MODE_FIXED_SLICE_CCS_MODE REG_BIT(1)
>> #define RCU_MODE_CCS_ENABLE REG_BIT(0)
>>
>> +/*
>> + * Total of 4 cslices, where each cslice is in the form:
>> + * [0-3] CCS ID
>> + * [4-6] RSVD
>> + * [7] Disabled
>> + */
>> +#define CCS_MODE XE_REG(0x14804)
>> +#define CCS_MODE_CSLICE_0_3_MASK REG_GENMASK(11, 0) /* 3 bits per cslice */
>> +#define CCS_MODE_CSLICE_MASK 0x7 /* CCS0-3 + rsvd */
>> +#define CCS_MODE_CSLICE_WIDTH ilog2(CCS_MODE_CSLICE_MASK + 1)
>> +#define CCS_MODE_CSLICE(cslice, ccs) \
>> + (ccs << (cslice * CCS_MODE_CSLICE_WIDTH))
>> +
>> #define FORCEWAKE_ACK_GT XE_REG(0x130044)
>> #define FORCEWAKE_KERNEL BIT(0)
>> #define FORCEWAKE_USER BIT(1)
>> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
>> index 6c885dde5d59..1f9b678fc530 100644
>> --- a/drivers/gpu/drm/xe/xe_gt.c
>> +++ b/drivers/gpu/drm/xe/xe_gt.c
>> @@ -21,6 +21,7 @@
>> #include "xe_execlist.h"
>> #include "xe_force_wake.h"
>> #include "xe_ggtt.h"
>> +#include "xe_gt_ccs_mode.h"
>> #include "xe_gt_clock.h"
>> #include "xe_gt_idle_sysfs.h"
>> #include "xe_gt_mcr.h"
>> @@ -451,6 +452,12 @@ static int all_fw_domain_init(struct xe_gt *gt)
>> if (err)
>> goto err_force_wake;
>>
>> + /* Configure default CCS mode of 1 engine with all resources */
>> + if (xe_gt_ccs_mode_enabled(gt)) {
>> + gt->ccs.num_engines = 1;
>> + xe_gt_apply_ccs_mode(gt);
>> + }
>> +
>> err = xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>> XE_WARN_ON(err);
>> xe_device_mem_access_put(gt_to_xe(gt));
>> @@ -553,6 +560,9 @@ static int do_gt_restart(struct xe_gt *gt)
>> xe_reg_sr_apply_whitelist(hwe);
>> }
>>
>> + /* Get CCS mode in sync between sw/hw */
>> + xe_gt_apply_ccs_mode(gt);
>> +
>> return 0;
>> }
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
>> index caded203a8a0..1c2a1d35a2c1 100644
>> --- a/drivers/gpu/drm/xe/xe_gt.h
>> +++ b/drivers/gpu/drm/xe/xe_gt.h
>> @@ -17,6 +17,8 @@
>> for_each_if(((hwe__) = (gt__)->hw_engines + (id__)) && \
>> xe_hw_engine_is_valid((hwe__)))
>>
>> +#define CCS_MASK(gt) ((gt->info.engine_mask & XE_HW_ENGINE_CCS_MASK) >> XE_HW_ENGINE_CCS0)
>> +
>> #ifdef CONFIG_FAULT_INJECTION
>> extern struct fault_attr gt_reset_failure;
>> static inline bool xe_fault_inject_gt_reset(void)
>> diff --git a/drivers/gpu/drm/xe/xe_gt_ccs_mode.c b/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
>> new file mode 100644
>> index 000000000000..fcbdc13df46e
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
>> @@ -0,0 +1,73 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2023 Intel Corporation
>> + */
>> +
>> +#include "regs/xe_gt_regs.h"
>> +#include "xe_assert.h"
>> +#include "xe_gt.h"
>> +#include "xe_gt_ccs_mode.h"
>> +#include "xe_mmio.h"
>> +
>> +static void __xe_gt_apply_ccs_mode(struct xe_gt *gt, u32 num_engines)
>> +{
>> + u32 mode = CCS_MODE_CSLICE_0_3_MASK; /* disable all by default */
>> + struct xe_device *xe = gt_to_xe(gt);
>> + struct xe_hw_engine *hwe;
>> + int num_slices = hweight32(CCS_MASK(gt));
>> + int width, cslice;
>> + enum xe_hw_engine_id id;
>> + u32 config = 0;
>> +
>> + xe_assert(xe, xe_gt_ccs_mode_enabled(gt));
>> +
>> + xe_assert(xe, num_engines && num_engines <= num_slices);
>> + xe_assert(xe, !(num_slices % num_engines));
>> +
>> + /*
>> + * Loop over all available slices and assign each a user engine.
>> + *
>> + * With 1 engine (ccs0):
>> + * slice 0, 1, 2, 3: ccs0
>> + *
>> + * With 2 engines (ccs0, ccs1):
>> + * slice 0, 2: ccs0
>> + * slice 1, 3: ccs1
>> + *
>> + * With 4 engines (ccs0, ccs1, ccs2, ccs3):
>> + * slice 0: ccs0
>> + * slice 1: ccs1
>> + * slice 2: ccs2
>> + * slice 3: ccs3
>> + */
>> + for (width = num_slices / num_engines, cslice = 0; width--;) {
>> + for_each_hw_engine(hwe, gt, id) {
>> + if (hwe->class != XE_ENGINE_CLASS_COMPUTE)
>> + continue;
>> +
>> + if (hwe->logical_instance >= num_engines)
>> + break;
>> +
>> + config |= BIT(hwe->instance) << XE_HW_ENGINE_CCS0;
>> +
>> + /* If a slice is fused off, leave disabled */
>> + while ((CCS_MASK(gt) & BIT(cslice)) == 0)
>> + cslice++;
>> +
>> + mode &= ~CCS_MODE_CSLICE(cslice, CCS_MODE_CSLICE_MASK);
>> + mode |= CCS_MODE_CSLICE(cslice, hwe->instance);
>> + cslice++;
>> + }
>> + }
>
>Maybe it's just me, but the above seems somewhat hard to follow (even
>though I think it's correct). I think an alternate implementation could
>be done with bitmap operations (untested):
>
> unsigned long cslice_mask = CCS_MASK(gt);
> unsigned long logical_usable = GENMASK(num_engines - 1, 0);
> unsigned long physical_usable = 0;
>
> xe_assert(num_engines && num_engines <= hweight32(cslice_mask));
>
> bitmap_onto(&physical_usable, &logical_usable, &cslice_mask, 4);
> for_each_set_bit(cslice, &cslice_mask, 4) {
> int ccs = bitmap_bitremap(cslice, &cslice_mask, &physical_usable, 4)
>
> mode &= ~CCS_MODE_CSLICE(cslice, CCS_MODE_CSLICE_MASK);
> mode |= CCS_MODE_CSLICE(cslice, ccs);
> }
>
>I'm not sure whether that winds up being more or less readable in the
>end.
>
Yah, it looks pretty neat once those bitmap functions are understood.
But unfortunately, those bitmap functions are only defined under CONFIG_NUMA
(not sure why). I don't want to bring that dependency here for now. May be
we can ask maintainers to move it out of CONFIG_NUMA?
>> +
>> + xe_mmio_write32(gt, CCS_MODE, mode);
>> +
>> + xe_gt_info(gt, "CCS_MODE=%x config:%08x, num_engines:%d, num_slices:%d\n",
>> + mode, config, num_engines, num_slices);
>> +}
>> +
>> +void xe_gt_apply_ccs_mode(struct xe_gt *gt)
>> +{
>> + if (gt->ccs.num_engines)
>> + __xe_gt_apply_ccs_mode(gt, gt->ccs.num_engines);
>> +}
>> diff --git a/drivers/gpu/drm/xe/xe_gt_ccs_mode.h b/drivers/gpu/drm/xe/xe_gt_ccs_mode.h
>> new file mode 100644
>> index 000000000000..2f3352173e7b
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_gt_ccs_mode.h
>> @@ -0,0 +1,22 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2023 Intel Corporation
>> + */
>> +
>> +#ifndef _XE_GT_CCS_MODE_H_
>> +#define _XE_GT_CCS_MODE_H_
>> +
>> +#include "xe_device_types.h"
>> +#include "xe_gt.h"
>> +#include "xe_gt_types.h"
>> +#include "xe_platform_types.h"
>> +
>> +void xe_gt_apply_ccs_mode(struct xe_gt *gt);
>> +
>> +static inline bool xe_gt_ccs_mode_enabled(const struct xe_gt *gt)
>> +{
>> + return GRAPHICS_VERx100(gt_to_xe(gt)) == 1260 && CCS_MASK(gt);
>> +}
>> +
>> +#endif
>> +
>> diff --git a/drivers/gpu/drm/xe/xe_gt_sysfs.c b/drivers/gpu/drm/xe/xe_gt_sysfs.c
>> index c69d2e8a0fe1..e53221c2894f 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_sysfs.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_sysfs.c
>> @@ -10,7 +10,11 @@
>>
>> #include <drm/drm_managed.h>
>>
>> +#include "regs/xe_gt_regs.h"
>> #include "xe_gt.h"
>> +#include "xe_gt_ccs_mode.h"
>> +#include "xe_gt_printk.h"
>> +#include "xe_mmio.h"
>>
>> static void xe_gt_sysfs_kobj_release(struct kobject *kobj)
>> {
>> @@ -29,6 +33,50 @@ static void gt_sysfs_fini(struct drm_device *drm, void *arg)
>> kobject_put(gt->sysfs);
>> }
>>
>> +static ssize_t
>> +ccs_mode_show(struct device *kdev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct xe_gt *gt = kobj_to_gt(&kdev->kobj);
>> +
>> + return sysfs_emit(buf, "Enabled compute engines %d; Number of compute slices %d\n",
>> + gt->ccs.num_engines, hweight32(CCS_MASK(gt)));
>> +}
>> +
>> +static ssize_t
>> +ccs_mode_store(struct device *kdev, struct device_attribute *attr,
>> + const char *buff, size_t count)
>> +{
>> + struct xe_gt *gt = kobj_to_gt(&kdev->kobj);
>> + u32 num_engines, num_slices;
>> + int ret;
>> +
>> + ret = kstrtou32(buff, 0, &num_engines);
>> + if (ret)
>> + return ret;
>> +
>> + /*
>> + * Ensure number of engines specified is valid and there is an
>> + * exact multiple of engines for slices.
>> + */
>> + num_slices = hweight32(CCS_MASK(gt));
>> + if (!num_engines || num_engines > num_slices || num_slices % num_engines) {
>> + xe_gt_dbg(gt, "Invalid compute config, %d engines %d slices\n",
>> + num_engines, num_slices);
>> + return -EINVAL;
>> + }
>> +
>> + if (gt->ccs.num_engines != num_engines) {
>> + xe_gt_info(gt, "Setting compute mode to %d\n", num_engines);
>> + gt->ccs.num_engines = num_engines;
>> + xe_gt_reset_async(gt);
>> + }
>> +
>> + return count;
>> +}
>> +
>> +static DEVICE_ATTR_RW(ccs_mode);
>> +
>> void xe_gt_sysfs_init(struct xe_gt *gt)
>> {
>> struct xe_tile *tile = gt_to_tile(gt);
>> @@ -52,6 +100,11 @@ void xe_gt_sysfs_init(struct xe_gt *gt)
>>
>> gt->sysfs = &kg->base;
>>
>> + if (GRAPHICS_VERx100(xe) == 1260 &&
>> + sysfs_create_file(gt->sysfs, &dev_attr_ccs_mode.attr))
>> + drm_warn(&xe->drm,
>> + "Sysfs creation for ccs_mode setting failed\n");
>> +
>> err = drmm_add_action_or_reset(&xe->drm, gt_sysfs_fini, gt);
>> if (err) {
>> drm_warn(&xe->drm, "%s: drmm_add_action_or_reset failed, err: %d\n",
>> diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
>> index d3f2793684e2..ab1740005600 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_types.h
>> +++ b/drivers/gpu/drm/xe/xe_gt_types.h
>> @@ -185,6 +185,20 @@ struct xe_gt {
>> spinlock_t lock;
>> } tlb_invalidation;
>>
>> + /**
>> + * @ccs: Fixed mapping between CCS engines and compute slices.
>> + * Through the per-tile 'ccs_mode' sysfs interface, the user can specify a
>> + * fixed number of compute hardware engines to which the available compute
>> + * slices are to be allocated. By default all compute slices are allocated
>> + * to the first available compute engine instance. This user configuration
>> + * change triggers a tile reset and it is expected that user will ensure
>> + * the tile is idle while doing so.
>> + */
>> + struct {
>> + /** @num_engines: Number of CCS engines enabled */
>> + u32 num_engines;
>> + } ccs;
>
>Just because the term "ccs" at the GT is a bit overloaded, I wonder if
>we should pick some name that makes it clear that this is referring to
>CCS engines and not to CCS compression. E.g., maybe make the structure
>"ccs_engines" and the field "num_active" or something like that?
>
Ok, will change it to ccs_mode.
Niranjana
>
>Matt
>
>> +
>> /** @usm: unified shared memory state */
>> struct {
>> /**
>> diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c
>> index 88789826e781..b6be7a7ad76a 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_ads.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_ads.c
>> @@ -12,6 +12,7 @@
>> #include "regs/xe_guc_regs.h"
>> #include "xe_bo.h"
>> #include "xe_gt.h"
>> +#include "xe_gt_ccs_mode.h"
>> #include "xe_guc.h"
>> #include "xe_hw_engine.h"
>> #include "xe_lrc.h"
>> @@ -454,6 +455,8 @@ static unsigned int guc_mmio_regset_write(struct xe_guc_ads *ads,
>> { .reg = RING_HWS_PGA(hwe->mmio_base), },
>> { .reg = RING_IMR(hwe->mmio_base), },
>> { .reg = RCU_MODE, .skip = hwe != hwe_rcs_reset_domain },
>> + { .reg = CCS_MODE,
>> + .skip = hwe != hwe_rcs_reset_domain || !xe_gt_ccs_mode_enabled(hwe->gt) },
>> };
>> u32 i;
>>
>> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c b/drivers/gpu/drm/xe/xe_hw_engine.c
>> index e831e63c5e48..4f41699f95ec 100644
>> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
>> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
>> @@ -16,6 +16,7 @@
>> #include "xe_execlist.h"
>> #include "xe_force_wake.h"
>> #include "xe_gt.h"
>> +#include "xe_gt_ccs_mode.h"
>> #include "xe_gt_topology.h"
>> #include "xe_hw_fence.h"
>> #include "xe_irq.h"
>> @@ -283,6 +284,13 @@ void xe_hw_engine_enable_ring(struct xe_hw_engine *hwe)
>> hw_engine_mmio_read32(hwe, RING_MI_MODE(0));
>> }
>>
>> +static bool xe_hw_engine_match_fixed_cslice_mode(const struct xe_gt *gt,
>> + const struct xe_hw_engine *hwe)
>> +{
>> + return xe_gt_ccs_mode_enabled(gt) &&
>> + xe_rtp_match_first_render_or_compute(gt, hwe);
>> +}
>> +
>> void
>> xe_hw_engine_setup_default_lrc_state(struct xe_hw_engine *hwe)
>> {
>> @@ -307,6 +315,12 @@ xe_hw_engine_setup_default_lrc_state(struct xe_hw_engine *hwe)
>> blit_cctl_val,
>> XE_RTP_ACTION_FLAG(ENGINE_BASE)))
>> },
>> + /* Use Fixed slice CCS mode */
>> + { XE_RTP_NAME("RCU_MODE_FIXED_SLICE_CCS_MODE"),
>> + XE_RTP_RULES(FUNC(xe_hw_engine_match_fixed_cslice_mode)),
>> + XE_RTP_ACTIONS(FIELD_SET(RCU_MODE, RCU_MODE_FIXED_SLICE_CCS_MODE,
>> + RCU_MODE_FIXED_SLICE_CCS_MODE))
>> + },
>> {}
>> };
>>
>> @@ -841,6 +855,12 @@ bool xe_hw_engine_is_reserved(struct xe_hw_engine *hwe)
>> if (hwe->class == XE_ENGINE_CLASS_OTHER)
>> return true;
>>
>> + /* Check for engines disabled by ccs_mode setting */
>> + if (xe_gt_ccs_mode_enabled(gt) &&
>> + hwe->class == XE_ENGINE_CLASS_COMPUTE &&
>> + hwe->logical_instance >= gt->ccs.num_engines)
>> + return true;
>> +
>> return xe->info.supports_usm && hwe->class == XE_ENGINE_CLASS_COPY &&
>> hwe->instance == gt->usm.reserved_bcs_instance;
>> }
>> --
>> 2.21.0.rc0.32.g243a4c7e27
>>
>
>--
>Matt Roper
>Graphics Software Engineer
>Linux GPU Platform Enablement
>Intel Corporation
More information about the Intel-xe
mailing list