On Sun, May 29, 2022 at 09:44:38PM -0700, Gupta, Anshuman wrote: > > > > -----Original Message----- > > From: Nikula, Jani <jani.nikula@xxxxxxxxx> > > Sent: Thursday, May 19, 2022 2:57 PM > > To: Gupta, Anshuman <anshuman.gupta@xxxxxxxxx>; intel- > > gfx@xxxxxxxxxxxxxxxxxxxxx > > Cc: Nilawar, Badal <badal.nilawar@xxxxxxxxx>; Ewins, Jon > > <jon.ewins@xxxxxxxxx>; Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>; Deak, Imre > > <imre.deak@xxxxxxxxx>; Gupta, Anshuman <anshuman.gupta@xxxxxxxxx> > > Subject: Re: [PATCH 3/7] drm/i915/dg2: DG2 MBD config > > > > On Wed, 18 May 2022, Anshuman Gupta <anshuman.gupta@xxxxxxxxx> wrote: > > > Add DG2 Motherboard Down Config check support. > > > > > > BSpec: 44477 > > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > > Signed-off-by: Anshuman Gupta <anshuman.gupta@xxxxxxxxx> > > > --- > > > drivers/gpu/drm/i915/display/intel_opregion.c | 2 ++ > > > drivers/gpu/drm/i915/i915_drv.h | 9 +++++++++ > > > 2 files changed, 11 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c > > > b/drivers/gpu/drm/i915/display/intel_opregion.c > > > index 3dcd54517b89..dec7628522c5 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_opregion.c > > > +++ b/drivers/gpu/drm/i915/display/intel_opregion.c > > > @@ -1271,6 +1271,8 @@ intel_opregion_vram_sr_required(struct > > > drm_i915_private *i915) > > > > > > if (IS_DG1(i915)) > > > return intel_opregion_dg1_mbd_config(i915); > > > + else if (IS_DG2_MBD(i915)) > > > + return true; > > > > > > return false; > > > } > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > b/drivers/gpu/drm/i915/i915_drv.h index 10f273800645..c5ecc490dced > > > 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -1071,6 +1071,15 @@ IS_SUBPLATFORM(const struct drm_i915_private > > *i915, > > > IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G11) > > #define > > > IS_DG2_G12(dev_priv) \ > > > IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G12) > > > +/* > > > + * FIXME: Need to define a new SUBPLATFORM > > INTEL_SUBPLATFORM_DG2_MBD > > > + * for PCI id range 5690..5695, but G10, G11 SUBPLATFROM conflicts > > > + * with those pci id range. > > > + */ > > > +#define DG2_MBD_CONFIG_MASK GENMASK(7, 4) > > > +#define DG2_MBD_CONFIG_VAL > > FIELD_PREP(DG2_MBD_CONFIG_MASK, 9) > > > +#define IS_DG2_MBD(dev_priv) (IS_DG2(dev_priv) && \ > > > + (INTEL_DEVID(dev_priv) & > > DG2_MBD_CONFIG_MASK) == > > > +DG2_MBD_CONFIG_VAL) > > > > No. Please don't do *any* magic masking or comparison of PCI ID bits or > > bitfields. > Hi Matt, > We need to distinguish between DG2 NB MBD and rest such that i915 can > figure out opregion vram_sr is requires for the DG2 platform. Could > you please suggestion on that. If you truly need to distinguish a specific sub-type of DG2, then creating a proper subplatform (with its own list of PCI IDs, matched in intel_device_info_subplatform_init() is the way to go. As far as I recall, it should be okay to have multiple subplatform bits set for a platform, so for example you can have both the G11 and a MBD subplatform bits set on a device and they can each be tested independently without interfering with each other. I'm not too familiar with the feature you're working on here. Is there a way we can detect whether it's supported by querying the pcode? Or what happens if you send your pcode request on a platform that doesn't support it? Do you just get a regular error back so that the driver would know to give up and move on, or would it actually cause some kind of behavioral problem? Matt > Thanks, > Anshuman Gupta. > > > > BR, > > Jani. > > > > > #define IS_ADLS_RPLS(dev_priv) \ > > > IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_S, > > INTEL_SUBPLATFORM_RPL) > > > #define IS_ADLP_N(dev_priv) \ > > > > -- > > Jani Nikula, Intel Open Source Graphics Center -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation
