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

[Matt Roper]

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.


Matt

> 
> Lucas De Marchi
> 
> > 
> > > 
> > > I don't see this new function being used yet in the other patches of
> > > this series, so I'm not sure exactly which workaround(s) plan to re-use
> > 
> > it was supposed to be
> > "[PATCH 07/19] drm/xe/rtp: Add match helper for gslice fused off", but
> > then I forgot to replace the hardcoded
> > `gt_to_xe(gt)->info.platform == XE_PVC ? 8 : 4;` by a call to this
> > function.
> > 
> > 
> > Lucas De Marchi
> > 
> > > it.  But likely we at least need some kind of clarifying rename for this
> > > function, otherwise it's probably going to get re-used incorrectly
> > > elsewhere in the driver.
> > > 
> > > 
> > > Matt
> > > 
> > > > +{
> > > > +	return gt_to_xe(gt)->info.platform == XE_PVC ? 8 : 4;
> > > > +}
> > > > +
> > > > /*
> > > >  * Used to obtain the index of the first DSS.  Can start searching from the
> > > >  * beginning of a specific dss group (e.g., gslice, cslice, etc.) if
> > > > diff --git a/drivers/gpu/drm/xe/xe_gt_topology.h b/drivers/gpu/drm/xe/xe_gt_topology.h
> > > > index 2375f005e0c8..cf8f75a4c7dd 100644
> > > > --- a/drivers/gpu/drm/xe/xe_gt_topology.h
> > > > +++ b/drivers/gpu/drm/xe/xe_gt_topology.h
> > > > @@ -14,6 +14,9 @@ void xe_gt_topology_init(struct xe_gt *gt);
> > > > 
> > > > void 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);
> > > > +
> > > > unsigned int
> > > > xe_dss_mask_group_ffs(const xe_dss_mask_t mask, int groupsize, int groupnum);
> > > > 
> > > > --
> > > > 2.39.0
> > > > 
> > > 
> > > -- 
> > > Matt Roper
> > > Graphics Software Engineer
> > > Linux GPU Platform Enablement
> > > Intel Corporation

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation