[PATCH 1/2] drm/xe: Always setup GT MMIO adjustment data

[Matt Roper]

On Tue, Nov 19, 2024 at 02:33:28PM -0600, Lucas De Marchi wrote:
> On Thu, Nov 14, 2024 at 08:36:46PM +0100, Michal Wajdeczko wrote:
> > 
> > 
> > On 14.11.2024 20:23, Lucas De Marchi wrote:
> > > On Thu, Nov 14, 2024 at 06:59:54PM +0100, Michal Wajdeczko wrote:
> > > > While we believed that xe_gt_mmio_init() will be called just once
> > > > per GT, this might not be a case due to some tweaks that need to
> > > > performed by the VF driver during early probe.  To avoid leaving
> > > 
> > > can you be explicit where this is being set on the VF case?
> > 
> > it will be used in read_gmdid(), see next patch
> > 
> > > Shouldn't VF
> > > itself deal with fixing up what it did?
> > 
> > in read_gmdid() we already said
> > 
> > 516      /*
> > 517       * Only undo xe_gt.info here, the remaining changes made above
> > 518       * will be overwritten as part of the regular initialization.
> > 519       */
> > 
> > and it looks that xe_gt_mmio_init() is not overwriting everything as we
> > would expect
> 
> We only need to read the gmdid and for that need the mmio struct to be
> filled out. We end up needing the gt as a context, but this all looks to
> be fixing things that shouldn't have been done.

I think the problem is that when you say we "only need to read the
gmdid" that isn't a simple register read when running on an SRIOV VF.
Rather it's a handshake and query sent to the GuC, which actually
involves reading/writing lots of GuC registers to eventually get back
that single version number we want.

SRIOV has a lot of chicken-and-egg problems; in this case the problem is
that our driver needs to know what IP version it's running on in order
to know how to do basic GT initialization.  However at the same time we
need to have already done basic GT + GuC initialization in order to even
find out the IP version since the VF doesn't have any direct access to
the GMD_ID registers.  So we wind up relying on some ugly hacks like
this that do a partial/fake initialization of the GT (under the
understanding that a lot of other GT functionality like MCR, forcewake,
etc. won't get used) to do things "out of order" compared to native
execution, and then we come back and fix things up later.

I think some of the headaches here are really oversights in the hardware
design (e.g., why doesn't the VF BAR expose GMD_ID values somewhere so
that we don't have to go through the GuC?).  But I don't have any good
ideas for handling things in a cleaner manner, aside from having a
completely independent .ko with different code flows to run on VFs,
which I don't think anyone really wants.


Matt

> 
> without looking deeper I don't have a short-term alternative, though.
> 
> Matt Roper, any better alternative that comes to mind? You did the mmio
> conversion and acked this "misbehavior" for VF earlier, so I'd like to
> hear your opinion.
> 
> Lucas De Marchi
> 
> > 
> > > 
> > > > any stale data in case of the re-run, reset the GT MMIO adjustment
> > > > data for the non-media GT case.
> > > 
> > > 
> > > Lucas De Marchi
> > > 
> > > > 
> > > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> > > > Cc: Matt Roper <matthew.d.roper at intel.com>
> > > > ---
> > > > drivers/gpu/drm/xe/xe_gt.c | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> > > > index d6744be01a68..d45d2cecc4dc 100644
> > > > --- a/drivers/gpu/drm/xe/xe_gt.c
> > > > +++ b/drivers/gpu/drm/xe/xe_gt.c
> > > > @@ -643,6 +643,9 @@ void xe_gt_mmio_init(struct xe_gt *gt)
> > > >     if (gt->info.type == XE_GT_TYPE_MEDIA) {
> > > >         gt->mmio.adj_offset = MEDIA_GT_GSI_OFFSET;
> > > >         gt->mmio.adj_limit = MEDIA_GT_GSI_LENGTH;
> > > > +    } else {
> > > > +        gt->mmio.adj_offset = 0;
> > > > +        gt->mmio.adj_limit = 0;
> > > >     }
> > > > 
> > > >     if (IS_SRIOV_VF(gt_to_xe(gt)))
> > > > -- 
> > > > 2.43.0
> > > > 
> > 

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