[Intel-xe] [PATCH 04/19] drm/xe: Add helper to get dss per group

[Lucas De Marchi]

On Wed, Mar 08, 2023 at 02:43:55PM -0800, Matt Roper wrote:
>On Wed, Mar 08, 2023 at 02:20:17PM -0800, Lucas De Marchi wrote:
>> On Wed, Mar 08, 2023 at 02:16:48PM -0800, Lucas De Marchi wrote:
>> > On Wed, Mar 08, 2023 at 01:53:38PM -0800, Matt Roper wrote:
>> > > On Tue, Mar 07, 2023 at 01:24:31AM -0800, Lucas De Marchi wrote:
>> > > > This is already used in intel_gt_mcr and will be used in workarounds.
>> > > > Add helpers so the platform-dependency is in a single place.
>> > > >
>> > > > Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> > > > ---
>> > > > drivers/gpu/drm/xe/xe_gt_mcr.c      | 2 +-
>> > > > drivers/gpu/drm/xe/xe_gt_topology.c | 6 ++++++
>> > > > drivers/gpu/drm/xe/xe_gt_topology.h | 3 +++
>> > > > 3 files changed, 10 insertions(+), 1 deletion(-)
>> > > >
>> > > > diff --git a/drivers/gpu/drm/xe/xe_gt_mcr.c b/drivers/gpu/drm/xe/xe_gt_mcr.c
>> > > > index a42061c4b9d2..694a79b40ec0 100644
>> > > > --- a/drivers/gpu/drm/xe/xe_gt_mcr.c
>> > > > +++ b/drivers/gpu/drm/xe/xe_gt_mcr.c
>> > > > @@ -223,7 +223,7 @@ static void init_steering_dss(struct xe_gt *gt)
>> > > > {
>> > > > 	unsigned int dss = min(xe_dss_mask_group_ffs(gt->fuse_topo.g_dss_mask, 0, 0),
>> > > > 			       xe_dss_mask_group_ffs(gt->fuse_topo.c_dss_mask, 0, 0));
>> > > > -	unsigned int dss_per_grp = gt_to_xe(gt)->info.platform == XE_PVC ? 8 : 4;
>> > > > +	unsigned int dss_per_grp = xe_gt_topology_get_dss_per_group(gt);
>> > > >
>> > > > 	gt->steering[DSS].group_target = dss / dss_per_grp;
>> > > > 	gt->steering[DSS].instance_target = dss % dss_per_grp;
>> > > > diff --git a/drivers/gpu/drm/xe/xe_gt_topology.c b/drivers/gpu/drm/xe/xe_gt_topology.c
>> > > > index 967f2349c67a..9fe06a46a400 100644
>> > > > --- a/drivers/gpu/drm/xe/xe_gt_topology.c
>> > > > +++ b/drivers/gpu/drm/xe/xe_gt_topology.c
>> > > > @@ -103,6 +103,12 @@ xe_gt_topology_dump(struct xe_gt *gt, struct drm_printer *p)
>> > > >
>> > > > }
>> > > >
>> > > > +unsigned int
>> > > > +xe_gt_topology_get_dss_per_group(struct xe_gt *gt)
>> > >
>> > > When this shows up as a general interface, it leaves me wondering what a
>> > > "group" is.  The rules for DSS steering happen to use gslices on Xe_HP
>> > > platforms and cslices on Xe_HPC platforms, but that's just the
>> > > platform-specific rules of steering and doesn't automatically translate
>> > > into which type of slice would be appropriate to use as a grouping
>> > > elsewhere in the driver.
>> >
>> > humn... maybe I myself didn't understand the group meaning here.
>> > is group a generic term for cslice/gslice?
>>
>> btw the term "group" is already used in e.g. xe_dss_mask_group_ffs()
>> exported in drivers/gpu/drm/xe/xe_gt_topology.h
>
>Right, but that's just working on an arbitrary grouping of bits (of any
>size).  The definition of "group" in that case is explicitly provided by
>the caller via parameters; the function itself doesn't decide what a
>group is.


so.. do you think documentation for what this is should be provided and
be sufficient? Otherwise, if you prefer not to expose this, how should
we implement xe_rtp_match_first_gslice_fused_off()? Just hardcode it as
4?  I was trying to avoid the word "slice" so I used the word group as
it is used around this code. Maybe I shouldn't.

What about the following?

  /**
   * xe_gt_topology_get_dss_per_slice - Get number of dss per slice
   * @gt: ...
   *
   * Each slice in the GT contains up to a fixed number of DSS. This
   * can be used when the ``g_dss_mask`` / ``c_dss_mask`` saved needs
   * to be interpreted on a per-slice basis.
   *
   * Returns: Number of DSS per slice to use with [gc]_dss_mask
   */


thanks
Lucas De Marchi