Hi Matt,
first of all thanks a lot for the observations you are raising.
On Wed, Feb 21, 2024 at 12:51:04PM -0800, Matt Roper wrote:
On Wed, Feb 21, 2024 at 01:12:18AM +0100, Andi Shyti wrote:
On Tue, Feb 20, 2024 at 03:39:18PM -0800, Matt Roper wrote:
On Tue, Feb 20, 2024 at 03:35:26PM +0100, Andi Shyti wrote:
...
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index a425db5ed3a2..e19df4ef47f6 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -168,6 +168,14 @@ static void init_unused_rings(struct intel_gt *gt) } } +static void intel_gt_apply_ccs_mode(struct intel_gt *gt) +{
- if (!IS_DG2(gt->i915))
return;- intel_uncore_write(gt->uncore, XEHP_CCS_MODE, 0);
This doesn't look right to me. A value of 0 means every cslice gets associated with CCS0.
Yes, that's what I'm trying to do. The behavior I'm looking for is this one:
/* ... * 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 ... */where the user can configure runtime the mode, making sure that no client is connected to i915.
But, this needs to be written
As we are now forcing mode '1', then all cslices are connected with ccs0.
Right --- and that's what I'm pointing out as illegal. I think that code comment above was taken out of context from a different RFC series; that's not an accurate description of the behavior we want here.
First, the above comment is using ccs# to refer to userspace engines, not hardware engines. As a simple example, DG2-G11 only ever has a single CCS which userspace sees as "instance 0" but which is actually CCS1 at the hardware level. If you try to follow the comment above when programming CCS_MODE, you've assigned all of the cslices to a non-existent engine and assigned no cslices to the CCS engine that actually exists. For DG2-G10 (and I think DG2-G12), there are different combinations of fused-off / not-fused-off engines that will always show up in userspace as CCS0-CCSn, even if those don't match the hardware IDs.
Second, the above comment is assuming that you have a part with a maximum fusing config (i.e., all cslices present). Using DG2-G11 again as an example, there's also only a single cslice (cslice1), so if you tell CCS1 that it's allowed to use EUs from non-existent cslice0, cslice2, and cslice3, you might not get the behavior you were hoping for.
if the hardware slices are fused off we wouldn't see them in a first place, right? And that's anyway a permanent configuration that wouldn't affect the patch.
BTW, is there any machine I can test this scenario?
On a DG2-G11 platform, that will flat out break compute since CCS0 is never present (G11 only has a single CCS and it's always the hardware's CCS1). Even on a G10 or G12 this could also break things depending on the fusing of your card if the hardware CCS0 happens to be missing.
Also, the register says that we need a field value of 0x7 for each cslice that's fused off. By passing 0, we're telling the CCS engine that it can use cslices that may not actually exist.
does it? Or do I need to write the id (0x0-0x3) of the user engine? That's how the mode is calculated in this algorithm.
0x0 - 0x3 are how you specify that a specific CCS engine can use the cslice. If the cslice can't be used at all (i.e., it's fused off), then you need to program a 0x7 to ensure no engines try to use the non-existent DSS/EUs.
I planned to limit this to the only DG2 (and ATSM, of course). Do you think it would it help?
Andi