[PATCH 4/6] drm/xe/vf: Provide early access to GMDID register

[Matt Roper]

On Thu, May 23, 2024 at 02:08:16PM -0700, Matt Roper wrote:
> On Thu, May 23, 2024 at 09:22:38PM +0200, Michal Wajdeczko wrote:
> > VFs do not have direct access to the GMDID register and must obtain
> > its value from the GuC. Since we need GMDID value very early in the
> > driver probe flow, before we even start the full setup of GT and GuC
> > data structures, we must do some early initializations ourselves.
> > 
> > Additionally, since we also need GMDID for the media GT, which isn't
> > created yet, temporarly tweak the root GT type into MEDIA to allow
> > communication with the correct GuC, as only it can provide the value
> > of the media GMDID register.
> > 
> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_pci.c | 47 ++++++++++++++++++++++++++++++++++---
> >  1 file changed, 44 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> > index ece410289f6c..eefac633dbc1 100644
> > --- a/drivers/gpu/drm/xe/xe_pci.c
> > +++ b/drivers/gpu/drm/xe/xe_pci.c
> > @@ -20,6 +20,8 @@
> >  #include "xe_device.h"
> >  #include "xe_drv.h"
> >  #include "xe_gt.h"
> > +#include "xe_gt_sriov_vf.h"
> > +#include "xe_guc.h"
> >  #include "xe_macros.h"
> >  #include "xe_mmio.h"
> >  #include "xe_module.h"
> > @@ -471,10 +473,49 @@ static void read_gmdid(struct xe_device *xe, enum xe_gmdid_type type, u32 *ver,
> >  
> >  	KUNIT_STATIC_STUB_REDIRECT(read_gmdid, xe, type, ver, revid);
> >  
> > -	if (type == GMDID_MEDIA)
> > -		gmdid_reg.addr += MEDIA_GT_GSI_OFFSET;
> > +	if (IS_SRIOV_VF(xe)) {
> > +		/*
> > +		 * To get the value of the GMDID register, VFs must obtain it
> > +		 * from the GuC using MMIO communication.
> > +		 *
> > +		 * Note that at this point the xe_gt is not fully uninitialized
> > +		 * and only basic access to MMIO registers is possible. To use
> > +		 * our existing GuC communication functions we must perform at
> > +		 * least basic xe_gt and xe_guc initialization.
> > +		 *
> > +		 * Since to obtain the value of GMDID_MEDIA we need to use the
> > +		 * media GuC, temporarly tweak the gt type.
> > +		 */
> > +		xe_gt_assert(gt, gt->info.type == XE_GT_TYPE_UNINITIALIZED);
> > +
> > +		if (type == GMDID_MEDIA) {
> > +			gt->info.id = 1;
> > +			gt->info.type = XE_GT_TYPE_MEDIA;
> > +		} else {
> > +			gt->info.id = 0;
> > +			gt->info.type = XE_GT_TYPE_MAIN;
> > +		}
> > +
> > +		xe_guc_comm_init_early(&gt->uc.guc);
> > +
> > +		/* don't bother with GMDID if failed to negotiate the GuC ABI */
> > +		val = xe_gt_sriov_vf_bootstrap(gt) ? 0 : xe_gt_sriov_vf_gmdid(gt);
> > +
> > +		/* undo */
> > +		gt->info.id = 0;
> > +		gt->info.type = XE_GT_TYPE_UNINITIALIZED;
> 
> The xe_guc_comm_init_early() and xe_gt_sriov_vf_bootstrap() above also
> make changes to the various GuC0 structures (which might be "wrong" if
> we were reading the media's GMD_ID, but I think all of those changes
> wind up getting overwritten later with the correct GuC0 values again
> later when we initialize the GuC for real.  You might want to add some
> kind of comment about that, since otherwise it looks at first glance
> like we might be leaving GuC1 state inside the GuC0 structure here.
> 
> Otherwise,
> 
>         Reviewed-by: Matt Roper <matthew.d.roper at intel.com>

Oh, one more question --- for non-SRIOV cases we assume that it's always
safe to read the GMD_ID register offsets, even if the platform has
fused-off media (like PVC did in the past).  If media is fused off, that
register read comes back as 0x0 and we move forward recognizing that
media just isn't available.  I don't think any of the post-MTL platforms
we have at the moment (i.e., platforms where GMD_ID exists) ever have
fused off media yet, but I imagine that situation will likely come back
at some point in the future.

With the SRIOV-based flows here, are we going to get any GuC errors
while blindly attempting to read the media GT's GMD_ID if media (and
thus the media GuC) isn't actually present?


Matt

> 
> > +	} else {
> > +		/*
> > +		 * We need to apply the GSI offset explicitly here as at this
> > +		 * point the xe_gt is not fully uninitialized and only basic
> > +		 * access to MMIO registers is possible.
> > +		 */
> > +		if (type == GMDID_MEDIA)
> > +			gmdid_reg.addr += MEDIA_GT_GSI_OFFSET;
> > +
> > +		val = xe_mmio_read32(gt, gmdid_reg);
> > +	}
> >  
> > -	val = xe_mmio_read32(gt, gmdid_reg);
> >  	*ver = REG_FIELD_GET(GMD_ID_ARCH_MASK, val) * 100 + REG_FIELD_GET(GMD_ID_RELEASE_MASK, val);
> >  	*revid = REG_FIELD_GET(GMD_ID_REVID, val);
> >  }
> > -- 
> > 2.43.0
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation

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