On Mon, Oct 03, 2022 at 09:56:18AM +0100, Tvrtko Ursulin wrote:
>
> Hi Matt,
>
> On 01/10/2022 01:45, Matt Roper wrote:
> > MTL's media GT only has a single type of steering ("OAADDRM") which
> > selects between media slice 0 and media slice 1. We'll always steer to
> > media slice 0 unless it is fused off (which is the case when VD0, VE0,
> > and SFC0 are all reported as unavailable).
> >
> > Bspec: 67789
> > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/gt/intel_gt_mcr.c | 19 +++++++++++++++++--
> > drivers/gpu/drm/i915/gt/intel_gt_types.h | 1 +
> > drivers/gpu/drm/i915/gt/intel_workarounds.c | 18 +++++++++++++++++-
> > 3 files changed, 35 insertions(+), 3 deletions(-)
>
> [snip]
>
> > +static void
> > +mtl_media_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
> > +{
> > + /*
> > + * Unlike older platforms, we no longer setup implicit steering here;
> > + * all MCR accesses are explicitly steered.
> > + */
> > + if (drm_debug_enabled(DRM_UT_DRIVER)) {
> > + struct drm_printer p = drm_debug_printer("MCR Steering:");
> > +
> > + intel_gt_mcr_report_steering(&p, gt, false);
> > + }
> > +}
> > +
> > static void
> > gt_init_workarounds(struct intel_gt *gt, struct i915_wa_list *wal)
> > {
> > struct drm_i915_private *i915 = gt->i915;
> > - if (IS_METEORLAKE(i915) && gt->type == GT_PRIMARY)
> > + if (IS_METEORLAKE(i915) && gt->type == GT_MEDIA)
> > + mtl_media_gt_workarounds_init(gt, wal);
> > + else if (IS_METEORLAKE(i915) && gt->type == GT_PRIMARY)
> > mtl_3d_gt_workarounds_init(gt, wal);
> > else if (IS_PONTEVECCHIO(i915))
> > pvc_gt_workarounds_init(gt, wal);
>
> Casually reading only - wouldn't it be nicer if the if-ladder in here
> (gt_init_workarounds) would have a single case per platform, and then you'd
> fork further (3d vs media) in MTL specific function?
Actually, that reminds me that we probably need to change this in a
different direction --- starting with MTL, we should stop tying
workarounds to the platform (IS_METEORLAKE) but rather tie them to the
IP version (i.e., GRAPHICS_VER or MEDIA_VER) since in the future the
same chiplets can potentially be re-used on multiple platforms.
Conversely, you could also potentially have variants of the same
"platform" (e.g., MTL) that incorporate chiplets with different IP
versions (and thus need distinct lists of workarounds and such).
At the moment MTL is the only platform we have with the standalone media
design so there's no potential for mix-and-match of chiplets yet, and
IS_METEORLAKE works fine in the short term, but we do need to start
planning ahead and moving off of platform checks in areas of the driver
like this.
>
> Also, series ends up with mtl_media_gt_workarounds_init and
> mtl_3d_gt_workarounds_init apparently 100% identical. You will need two
> copies in the future?
Yes, the two GTs are expected to end up with completely different sets
of workarounds once the platform is enabled. We've just been delaying
on actually sending the new MTL workarounds upstream to give the
workaround database a bit more time to settle.
Matt
>
> Regards,
>
> Tvrtko
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
