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

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

[Anshuman Gupta]

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 pecs 5.3.1.4.1)

TODO:
A solution towards releasing mmap mappings in runtime suspend is
already work in progress.
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.

Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 77e7df21f539..28f38f1cc5cc 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -931,6 +931,7 @@ static const struct intel_device_info dg1_info = {
 		BIT(VCS0) | BIT(VCS2),
 	/* Wa_16011227922 */
 	.__runtime.ppgtt_size = 47,
+	.has_runtime_pm = 0,
 };
 
 static const struct intel_device_info adl_s_info = {
@@ -1076,6 +1077,7 @@ 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),
+	.has_runtime_pm = 0,
 	.require_force_probe = 1,
 };
 
-- 
2.26.2

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/DG{1, 2}: FIXME Temporary hammer to disable rpm

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 7266 bytes --]

== Series Details ==

Series: drm/i915/DG{1, 2}: FIXME Temporary hammer to disable rpm
URL   : https://patchwork.freedesktop.org/series/108477/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12128 -> Patchwork_108477v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_108477v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_108477v1, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108477v1/index.html

Participating hosts (42 -> 41)
------------------------------

  Additional (1): bat-dg2-11 
  Missing    (2): fi-ctg-p8600 fi-bdw-samus 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_108477v1:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_pm_rpm@module-reload:
    - bat-dg1-5:          [PASS][1] -> [SKIP][2] +2 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12128/bat-dg1-5/igt@i915_pm_rpm@module-reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108477v1/bat-dg1-5/igt@i915_pm_rpm@module-reload.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_exec_suspend@basic-s0@lmem0:
    - {bat-dg2-11}:       NOTRUN -> [DMESG-WARN][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108477v1/bat-dg2-11/igt@gem_exec_suspend@basic-s0@lmem0.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - {bat-dg2-8}:        [PASS][4] -> [SKIP][5] +2 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12128/bat-dg2-8/igt@i915_pm_rpm@basic-pci-d3-state.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108477v1/bat-dg2-8/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@i915_pm_rpm@basic-rte:
    - {bat-dg2-9}:        [PASS][6] -> [SKIP][7] +2 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12128/bat-dg2-9/igt@i915_pm_rpm@basic-rte.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108477v1/bat-dg2-9/igt@i915_pm_rpm@basic-rte.html

  * igt@i915_pm_rpm@module-reload:
    - {bat-dg2-11}:       NOTRUN -> [SKIP][8] +2 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108477v1/bat-dg2-11/igt@i915_pm_rpm@module-reload.html

  
Known issues
------------

  Here are the changes found in Patchwork_108477v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-ivb-3770:        NOTRUN -> [SKIP][9] ([fdo#109271] / [fdo#111827])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108477v1/fi-ivb-3770/igt@kms_chamelium@common-hpd-after-suspend.html
    - fi-pnv-d510:        NOTRUN -> [SKIP][10] ([fdo#109271])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108477v1/fi-pnv-d510/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions:
    - fi-bsw-kefka:       [PASS][11] -> [FAIL][12] ([i915#6298])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12128/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108477v1/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gem:
    - fi-pnv-d510:        [DMESG-FAIL][13] ([i915#4528]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12128/fi-pnv-d510/igt@i915_selftest@live@gem.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108477v1/fi-pnv-d510/igt@i915_selftest@live@gem.html

  * igt@i915_selftest@live@hangcheck:
    - fi-ivb-3770:        [INCOMPLETE][15] ([i915#3303] / [i915#5370]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12128/fi-ivb-3770/igt@i915_selftest@live@hangcheck.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108477v1/fi-ivb-3770/igt@i915_selftest@live@hangcheck.html
    - bat-dg1-5:          [DMESG-FAIL][17] ([i915#4957]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12128/bat-dg1-5/igt@i915_selftest@live@hangcheck.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108477v1/bat-dg1-5/igt@i915_selftest@live@hangcheck.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3595]: https://gitlab.freedesktop.org/drm/intel/issues/3595
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
  [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4873]: https://gitlab.freedesktop.org/drm/intel/issues/4873
  [i915#4957]: https://gitlab.freedesktop.org/drm/intel/issues/4957
  [i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
  [i915#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274
  [i915#5278]: https://gitlab.freedesktop.org/drm/intel/issues/5278
  [i915#5370]: https://gitlab.freedesktop.org/drm/intel/issues/5370
  [i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621


Build changes
-------------

  * Linux: CI_DRM_12128 -> Patchwork_108477v1

  CI-20190529: 20190529
  CI_DRM_12128: 9508a7418e4beed93db88d42235449bc097eaf97 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6653: 4f927248ebbf11f03f4c1ea2254f011e7575322f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_108477v1: 9508a7418e4beed93db88d42235449bc097eaf97 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

58ea318eed16 drm/i915/DG{1, 2}: FIXME Temporary hammer to disable rpm

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108477v1/index.html

[-- Attachment #2: Type: text/html, Size: 6887 bytes --]

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

[Anshuman Gupta]

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)

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.

Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 77e7df21f539..f31d7f5399cc 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -931,6 +931,26 @@ 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.
+	 *  But both DG1/DG2 has a hardware bug that violates the PCIe specs
+	 *  and supports 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 gets discover 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 binds with pcieport driver.
+	 *  which really disables the PCIe power savings and leaves the bridge to D0
+	 *  state.
+	 *  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 +1096,7 @@ 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),
+	.has_runtime_pm = 0,
 	.require_force_probe = 1,
 };
 
-- 
2.26.2

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

[Rodrigo Vivi]

On Wed, Sep 14, 2022 at 07:45:53PM +0530, Anshuman Gupta wrote:
> 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)
> 
> 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.
> 
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_pci.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 77e7df21f539..f31d7f5399cc 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -931,6 +931,26 @@ 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.
> +	 *  But both DG1/DG2 has a hardware bug that violates the PCIe specs
> +	 *  and supports 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 gets discover 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 binds with pcieport driver.
> +	 *  which really disables the PCIe power savings and leaves the bridge to D0
> +	 *  state.
> +	 *  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 +1096,7 @@ 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),
> +	.has_runtime_pm = 0,

The FIXME msg can be smaller, but it also needs to be here.

With this in place fell free to use:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Since the proper solution might take a while let's protect from this case,
regardless of any other on going discussion about the force_probe protection.


>  	.require_force_probe = 1,
>  };
>  
> -- 
> 2.26.2
> 

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

[Andi Shyti]

Hi Anshuman,

On Wed, Sep 14, 2022 at 10:33:15AM -0400, Rodrigo Vivi wrote:
> On Wed, Sep 14, 2022 at 07:45:53PM +0530, Anshuman Gupta wrote:
> > 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)
> > 
> > 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.
> > 
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_pci.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> > index 77e7df21f539..f31d7f5399cc 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -931,6 +931,26 @@ 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.
> > +	 *  But both DG1/DG2 has a hardware bug that violates the PCIe specs

/has/have/

> > +	 *  and supports the iomem read write transaction over PCIe bus despite

/supports/support/

> > +	 *  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 gets discover when PCIe gfx endpoint's upstream

/gets discover/becomes visible/

> > +	 *  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 binds with pcieport driver.

/binds/bind/

> > +	 *  which really disables the PCIe power savings and leaves the bridge to D0
> > +	 *  state.
> > +	 *  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 +1096,7 @@ 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),
> > +	.has_runtime_pm = 0,
> 
> The FIXME msg can be smaller, but it also needs to be here.

I actually like the comment, is very clear and helps
understanding the issue :)

Thanks again for addressing the issue and with the hope to see
the proper fix soon:

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Thanks,
Andi

> With this in place fell free to use:
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> Since the proper solution might take a while let's protect from this case,
> regardless of any other on going discussion about the force_probe protection.
> 
> 
> >  	.require_force_probe = 1,
> >  };
> >  
> > -- 
> > 2.26.2
> > 

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

[Gupta, Anshuman]

> -----Original Message-----
> From: Andi Shyti <andi.shyti@linux.intel.com>
> Sent: Wednesday, September 14, 2022 8:13 PM
> To: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; intel-
> gfx@lists.freedesktop.org; joonas.lahtinen@linux.intel.com; Ewins, Jon
> <jon.ewins@intel.com>; andi.shyti@linux.intel.com; Auld, Matthew
> <matthew.auld@intel.com>
> Subject: Re: [PATCH] drm/i915/DG{1,2}: FIXME Temporary hammer to disable
> rpm
> 
> Hi Anshuman,
> 
> On Wed, Sep 14, 2022 at 10:33:15AM -0400, Rodrigo Vivi wrote:
> > On Wed, Sep 14, 2022 at 07:45:53PM +0530, Anshuman Gupta wrote:
> > > 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)
> > >
> > > 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.
> > >
> > > Cc: Matthew Auld <matthew.auld@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_pci.c | 21 +++++++++++++++++++++
> > >  1 file changed, 21 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> > > b/drivers/gpu/drm/i915/i915_pci.c index 77e7df21f539..f31d7f5399cc
> > > 100644
> > > --- a/drivers/gpu/drm/i915/i915_pci.c
> > > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > > @@ -931,6 +931,26 @@ 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.
> > > +	 *  But both DG1/DG2 has a hardware bug that violates the PCIe
> > > +specs
> 
> /has/have/
> 
> > > +	 *  and supports the iomem read write transaction over PCIe bus
> > > +despite
> 
> /supports/support/
> 
> > > +	 *  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 gets discover when PCIe gfx endpoint's upstream
> 
> /gets discover/becomes visible/
> 
> > > +	 *  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 binds with pcieport
> driver.
> 
> /binds/bind/
> 
> > > +	 *  which really disables the PCIe power savings and leaves the bridge to
> D0
> > > +	 *  state.
> > > +	 *  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
> > > +1096,7 @@ 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),
> > > +	.has_runtime_pm = 0,
> >
> > The FIXME msg can be smaller, but it also needs to be here.
> 
> I actually like the comment, is very clear and helps understanding the issue :)
Shall I move the comment to commit log , and keep a smaller comment for both DG1 and DG2 ?
With that I can address your comment and Rodrigo comment as well.
Keeping such a big comment at two places will not make any sense.
Thanks,
Anshuman Gupta.
> 
> Thanks again for addressing the issue and with the hope to see the proper fix
> soon:
> 
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> 
> Thanks,
> Andi
> 
> > With this in place fell free to use:
> >
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >
> > Since the proper solution might take a while let's protect from this
> > case, regardless of any other on going discussion about the force_probe
> protection.
> >
> >
> > >  	.require_force_probe = 1,
> > >  };
> > >
> > > --
> > > 2.26.2
> > >

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

[Andi Shyti]

Hi Anshuman,

[...]

> > > > +	 *  which really disables the PCIe power savings and leaves the bridge to
> > D0
> > > > +	 *  state.
> > > > +	 *  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
> > > > +1096,7 @@ 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),
> > > > +	.has_runtime_pm = 0,
> > >
> > > The FIXME msg can be smaller, but it also needs to be here.
> > 
> > I actually like the comment, is very clear and helps understanding the issue :)
> Shall I move the comment to commit log , and keep a smaller comment for both DG1 and DG2 ?
> With that I can address your comment and Rodrigo comment as well.
> Keeping such a big comment at two places will not make any sense.

OK for me!

Thanks!
Andi