Re: [Intel-gfx] [PATCH v2] drm/i915/DG{1, 2}: FIXME Temporary hammer to disable rpm

From: "Gupta, Anshuman" <anshuman.gupta@intel.com>
To: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Cc: "Auld, Matthew" <matthew.auld@intel.com>,
	"Vivi, Rodrigo" <rodrigo.vivi@intel.com>
Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/DG{1, 2}: FIXME Temporary hammer to disable rpm
Date: Thu, 15 Sep 2022 07:19:29 +0000	[thread overview]
Message-ID: <CY5PR11MB6211A81AB8DBECB13CEED9CF95499@CY5PR11MB6211.namprd11.prod.outlook.com> (raw)
In-Reply-To: <166322444054.4263.583762501856107655@jlahtine-mobl.ger.corp.intel.com>


> -----Original Message-----
> From: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Sent: Thursday, September 15, 2022 12:17 PM
> To: Gupta, Anshuman <anshuman.gupta@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Nilawar, Badal
> <badal.nilawar@intel.com>; Ewins, Jon <jon.ewins@intel.com>;
> andi.shyti@linux.intel.com; Gupta, Anshuman <anshuman.gupta@intel.com>;
> Auld, Matthew <matthew.auld@intel.com>
> Subject: Re: [PATCH v2] drm/i915/DG{1,2}: FIXME Temporary hammer to disable
> rpm
> 
> On the patch subject, could we aim to be a bit more readable and concise.
> Something like:
> 
> "drm/i915: Temporarily disable RPM on DG1/DG2"
> 
> The patch title definitely should not have a FIXME in it if we're going to merge it
> in that form.
> 
> There's nothing to be fixed about the patch itself, we are applying a workaround
> until we've fixed the rootcause, which is business as usual.
Thanks Joonas for feedback, will remove the FIXME from the subject before pushing.
Br,
Anshuman.
> 
> Regards, Joonas
> 
> Quoting Anshuman Gupta (2022-09-14 19:13:29)
> > DG1 and DG2 has lmem, and cpu can access the lmem objects via mmap and
> > i915 internal i915_gem_object_pin_map() for
> > i915 own usages. Both of these methods has pre-requisite requirement
> > to keep GFX PCI endpoint in D0 for a supported iomem transaction over
> > PCI link. (Refer PCIe specs 5.3.1.4.1)
> >
> > Both DG1/DG2 have a hardware bug that violates the PCIe specs and
> > support the iomem read write transaction over PCIe bus despite
> > endpoint is D3 state.
> > Due to above H/W bug, we had never observed any issue with i915
> > runtime PM versus lmem access.
> > But this issue becomes visible when PCIe gfx endpoint's upstream
> > bridge enters to D3, at this point any lmem read/write access will be
> > returned as unsupported request. But again this issue is not observed
> > on every platform because it has been observed on few host machines
> > DG1/DG2 endpoint's upstream bridge does not bind with pcieport driver.
> > which really disables the PCIe poer power savings and leaves the
> > bridge at D0 state.
> >
> > TODO:
> > With respect to i915_gem_object_pin_map(), every caller has to grab a
> > wakeref if gem object lies in lmem.
> >
> > Till we fix all issues related to runtime PM, we need to keep runtime
> > PM disable on both DG1 and DG2.
> >
> > V2:
> > - Keep a smaller FIXME code comment for both DG1/DG2.
> >
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_pci.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> > b/drivers/gpu/drm/i915/i915_pci.c index 77e7df21f539..4a7d226b074f
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -931,6 +931,14 @@ static const struct intel_device_info dg1_info = {
> >                 BIT(VCS0) | BIT(VCS2),
> >         /* Wa_16011227922 */
> >         .__runtime.ppgtt_size = 47,
> > +
> > +       /*
> > +        *  FIXME: Temporary hammer to disable rpm.
> > +        *  As per PCIe specs 5.3.1.4.1, all iomem read write request over a PCIe
> > +        *  function will be unsupported in case PCIe endpoint function is in D3.
> > +        *  Let's disable i915 rpm till we fix all known issue with lmem access in
> D3.
> > +        */
> > +       .has_runtime_pm = 0,
> >  };
> >
> >  static const struct intel_device_info adl_s_info = { @@ -1076,6
> > +1084,13 @@ static const struct intel_device_info dg2_info = {
> >         XE_LPD_FEATURES,
> >         .__runtime.cpu_transcoder_mask = BIT(TRANSCODER_A) |
> BIT(TRANSCODER_B) |
> >                                BIT(TRANSCODER_C) | BIT(TRANSCODER_D),
> > +       /*
> > +        *  FIXME: Temporary hammer to disable rpm.
> > +        *  As per PCIe specs 5.3.1.4.1, all iomem read write request over a PCIe
> > +        *  function will be unsupported in case PCIe endpoint function is in D3.
> > +        *  Let's disable i915 rpm till we fix all known issue with lmem access in
> D3.
> > +        */
> > +       .has_runtime_pm = 0,
> >         .require_force_probe = 1,
> >  };
> >
> > --
> > 2.26.2
> >