[Intel-gfx] [PATCH] drm/i915/display: Defer application of initial chv_phy_control

[Intel-gfx] [PATCH] drm/i915/display: Defer application of initial chv_phy_control

[Chris Wilson]

To write to the DISPLAY_PHY_CONTROL requires holding the powerwells,
which during early resume we have not yet acquired until later in
intel_display_power_init_hw(). So compute the initial chv_phy_control,
but leave the HW unset until we first acquire the powerwell.

<7> [120.055984] i915 0000:00:02.0: [drm:intel_power_domains_init_hw [i915]] rawclk rate: 200000 kHz
<4> [120.056381] ------------[ cut here ]------------
<4> [120.056621] i915 0000:00:02.0: Unclaimed write to register 0x1e0100
<4> [120.056924] WARNING: CPU: 1 PID: 164 at drivers/gpu/drm/i915/intel_uncore.c:1166 __unclaimed_reg_debug+0x69/0x80 [i915]
<4> [120.056935] Modules linked in: vgem snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic btusb btrtl btbcm btintel i915 bluetooth coretemp crct10dif_pclmul crc32_pclmul snd_hda_intel snd_intel_dspcfg snd_hda_codec ghash_clmulni_intel snd_hwdep ecdh_generic ecc snd_hda_core r8169 snd_pcm lpc_ich realtek pinctrl_cherryview i2c_designware_pci prime_numbers
<4> [120.057027] CPU: 1 PID: 164 Comm: kworker/u4:3 Tainted: G     U            5.5.0-CI-CI_DRM_7854+ #1
<4> [120.057038] Hardware name:  /NUC5CPYB, BIOS PYBSWCEL.86A.0055.2016.0812.1130 08/12/2016
<4> [120.057058] Workqueue: events_unbound async_run_entry_fn
<4> [120.057275] RIP: 0010:__unclaimed_reg_debug+0x69/0x80 [i915]
<4> [120.057289] Code: 48 8b 78 18 48 8b 5f 50 48 85 db 74 2d e8 1f a0 3f e1 45 89 e8 48 89 e9 48 89 da 48 89 c6 48 c7 c7 00 8c 48 a0 e8 67 82 df e0 <0f> 0b 83 2d ce e2 2b 00 01 5b 5d 41 5c 41 5d c3 48 8b 1f eb ce 66
<4> [120.057301] RSP: 0018:ffffc90000bcfd08 EFLAGS: 00010082
<4> [120.057315] RAX: 0000000000000000 RBX: ffff888079919b60 RCX: 0000000000000003
<4> [120.057326] RDX: 0000000080000003 RSI: 0000000000000000 RDI: 00000000ffffffff
<4> [120.057336] RBP: ffffffffa04c9f4e R08: 0000000000000000 R09: 0000000000000001
<4> [120.057348] R10: 0000000025c3d560 R11: 000000006815f798 R12: 0000000000000000
<4> [120.057359] R13: 00000000001e0100 R14: 0000000000000286 R15: ffffffff8234a76b
<4> [120.057371] FS:  0000000000000000(0000) GS:ffff888074b00000(0000) knlGS:0000000000000000
<4> [120.057382] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4> [120.057393] CR2: 000055f4197df0d8 CR3: 000000006f326000 CR4: 00000000001006e0
<4> [120.057404] Call Trace:
<4> [120.057635]  fwtable_write32+0x114/0x1d0 [i915]
<4> [120.057892]  intel_power_domains_init_hw+0x4ff/0x650 [i915]
<4> [120.058150]  intel_power_domains_resume+0x3d/0x70 [i915]
<4> [120.058363]  i915_drm_resume_early+0x97/0xd0 [i915]
<4> [120.058575]  ? i915_resume_switcheroo+0x30/0x30 [i915]
<4> [120.058594]  dpm_run_callback+0x64/0x280
<4> [120.058626]  device_resume_early+0xa7/0xe0
<4> [120.058652]  async_resume_early+0x14/0x40

Closes: https://gitlab.freedesktop.org/drm/intel/issues/1089
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_power.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 64943179c05e..492668d5a193 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -5163,11 +5163,10 @@ static void chv_phy_control_init(struct drm_i915_private *dev_priv)
 		dev_priv->chv_phy_assert[DPIO_PHY1] = true;
 	}
 
-	intel_de_write(dev_priv, DISPLAY_PHY_CONTROL,
-		       dev_priv->chv_phy_control);
-
 	drm_dbg_kms(&dev_priv->drm, "Initial PHY_CONTROL=0x%08x\n",
 		    dev_priv->chv_phy_control);
+
+	/* Defer application of initial phy_control to enabling the powerwell */
 }
 
 static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv)
-- 
2.25.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/display: Defer application of initial chv_phy_control

[Patchwork]

== Series Details ==

Series: drm/i915/display: Defer application of initial chv_phy_control
URL   : https://patchwork.freedesktop.org/series/72865/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
8990b6f1d100 drm/i915/display: Defer application of initial chv_phy_control
-:12: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#12: 
<7> [120.055984] i915 0000:00:02.0: [drm:intel_power_domains_init_hw [i915]] rawclk rate: 200000 kHz

total: 0 errors, 1 warnings, 0 checks, 13 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/display: Defer application of initial chv_phy_control

[Patchwork]

== Series Details ==

Series: drm/i915/display: Defer application of initial chv_phy_control
URL   : https://patchwork.freedesktop.org/series/72865/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7854 -> Patchwork_16375
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s0:
    - fi-cml-s:           [PASS][1] -> [FAIL][2] ([fdo#103375])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-cml-s/igt@gem_exec_suspend@basic-s0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16375/fi-cml-s/igt@gem_exec_suspend@basic-s0.html

  * igt@i915_module_load@reload:
    - fi-icl-u2:          [PASS][3] -> [DMESG-WARN][4] ([i915#289]) +2 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-icl-u2/igt@i915_module_load@reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16375/fi-icl-u2/igt@i915_module_load@reload.html

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770r:       [PASS][5] -> [DMESG-FAIL][6] ([i915#553] / [i915#725])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-hsw-4770r/igt@i915_selftest@live_blt.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16375/fi-hsw-4770r/igt@i915_selftest@live_blt.html
    - fi-hsw-4770:        [PASS][7] -> [DMESG-FAIL][8] ([i915#725])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-hsw-4770/igt@i915_selftest@live_blt.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16375/fi-hsw-4770/igt@i915_selftest@live_blt.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s0:
    - fi-bsw-kefka:       [TIMEOUT][9] ([i915#1089]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-bsw-kefka/igt@gem_exec_suspend@basic-s0.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16375/fi-bsw-kefka/igt@gem_exec_suspend@basic-s0.html
    - fi-bsw-n3050:       [TIMEOUT][11] ([i915#1089]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-bsw-n3050/igt@gem_exec_suspend@basic-s0.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16375/fi-bsw-n3050/igt@gem_exec_suspend@basic-s0.html

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [FAIL][13] ([i915#178]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16375/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-atomic:
    - fi-icl-u2:          [DMESG-WARN][15] ([i915#263]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16375/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html

  
#### Warnings ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-cml-s:           [TIMEOUT][17] ([i915#1078]) -> [FAIL][18] ([fdo#103375])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-cml-s/igt@gem_exec_suspend@basic-s3.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16375/fi-cml-s/igt@gem_exec_suspend@basic-s3.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-icl-u2:          [FAIL][19] ([i915#323]) -> [DMESG-WARN][20] ([IGT#4] / [i915#263])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-icl-u2/igt@kms_chamelium@common-hpd-after-suspend.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16375/fi-icl-u2/igt@kms_chamelium@common-hpd-after-suspend.html

  
  [IGT#4]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/4
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [i915#1078]: https://gitlab.freedesktop.org/drm/intel/issues/1078
  [i915#1089]: https://gitlab.freedesktop.org/drm/intel/issues/1089
  [i915#178]: https://gitlab.freedesktop.org/drm/intel/issues/178
  [i915#263]: https://gitlab.freedesktop.org/drm/intel/issues/263
  [i915#289]: https://gitlab.freedesktop.org/drm/intel/issues/289
  [i915#323]: https://gitlab.freedesktop.org/drm/intel/issues/323
  [i915#553]: https://gitlab.freedesktop.org/drm/intel/issues/553
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725


Participating hosts (48 -> 41)
------------------------------

  Additional (2): fi-hsw-peppy fi-skl-lmem 
  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-elk-e7500 fi-byt-clapper fi-bsw-nick fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7854 -> Patchwork_16375

  CI-20190529: 20190529
  CI_DRM_7854: 727605cdef77d1e7eafb7e4c05b0ee74132a0930 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5410: 9d3872ede14307ef4adb0866f8474f5c41e6b1c1 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16375: 8990b6f1d100b80edb99f22be0faf5347e9ba515 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

8990b6f1d100 drm/i915/display: Defer application of initial chv_phy_control

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16375/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/display: Defer application of initial chv_phy_control

[Ville Syrjälä]

On Sat, Feb 01, 2020 at 10:31:59AM +0000, Chris Wilson wrote:
> To write to the DISPLAY_PHY_CONTROL requires holding the powerwells,
> which during early resume we have not yet acquired until later in
> intel_display_power_init_hw(). So compute the initial chv_phy_control,
> but leave the HW unset until we first acquire the powerwell.
> 
> <7> [120.055984] i915 0000:00:02.0: [drm:intel_power_domains_init_hw [i915]] rawclk rate: 200000 kHz
> <4> [120.056381] ------------[ cut here ]------------
> <4> [120.056621] i915 0000:00:02.0: Unclaimed write to register 0x1e0100
> <4> [120.056924] WARNING: CPU: 1 PID: 164 at drivers/gpu/drm/i915/intel_uncore.c:1166 __unclaimed_reg_debug+0x69/0x80 [i915]
> <4> [120.056935] Modules linked in: vgem snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic btusb btrtl btbcm btintel i915 bluetooth coretemp crct10dif_pclmul crc32_pclmul snd_hda_intel snd_intel_dspcfg snd_hda_codec ghash_clmulni_intel snd_hwdep ecdh_generic ecc snd_hda_core r8169 snd_pcm lpc_ich realtek pinctrl_cherryview i2c_designware_pci prime_numbers
> <4> [120.057027] CPU: 1 PID: 164 Comm: kworker/u4:3 Tainted: G     U            5.5.0-CI-CI_DRM_7854+ #1
> <4> [120.057038] Hardware name:  /NUC5CPYB, BIOS PYBSWCEL.86A.0055.2016.0812.1130 08/12/2016
> <4> [120.057058] Workqueue: events_unbound async_run_entry_fn
> <4> [120.057275] RIP: 0010:__unclaimed_reg_debug+0x69/0x80 [i915]
> <4> [120.057289] Code: 48 8b 78 18 48 8b 5f 50 48 85 db 74 2d e8 1f a0 3f e1 45 89 e8 48 89 e9 48 89 da 48 89 c6 48 c7 c7 00 8c 48 a0 e8 67 82 df e0 <0f> 0b 83 2d ce e2 2b 00 01 5b 5d 41 5c 41 5d c3 48 8b 1f eb ce 66
> <4> [120.057301] RSP: 0018:ffffc90000bcfd08 EFLAGS: 00010082
> <4> [120.057315] RAX: 0000000000000000 RBX: ffff888079919b60 RCX: 0000000000000003
> <4> [120.057326] RDX: 0000000080000003 RSI: 0000000000000000 RDI: 00000000ffffffff
> <4> [120.057336] RBP: ffffffffa04c9f4e R08: 0000000000000000 R09: 0000000000000001
> <4> [120.057348] R10: 0000000025c3d560 R11: 000000006815f798 R12: 0000000000000000
> <4> [120.057359] R13: 00000000001e0100 R14: 0000000000000286 R15: ffffffff8234a76b
> <4> [120.057371] FS:  0000000000000000(0000) GS:ffff888074b00000(0000) knlGS:0000000000000000
> <4> [120.057382] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> <4> [120.057393] CR2: 000055f4197df0d8 CR3: 000000006f326000 CR4: 00000000001006e0
> <4> [120.057404] Call Trace:
> <4> [120.057635]  fwtable_write32+0x114/0x1d0 [i915]
> <4> [120.057892]  intel_power_domains_init_hw+0x4ff/0x650 [i915]
> <4> [120.058150]  intel_power_domains_resume+0x3d/0x70 [i915]
> <4> [120.058363]  i915_drm_resume_early+0x97/0xd0 [i915]
> <4> [120.058575]  ? i915_resume_switcheroo+0x30/0x30 [i915]
> <4> [120.058594]  dpm_run_callback+0x64/0x280
> <4> [120.058626]  device_resume_early+0xa7/0xe0
> <4> [120.058652]  async_resume_early+0x14/0x40
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/issues/1089
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_power.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 64943179c05e..492668d5a193 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -5163,11 +5163,10 @@ static void chv_phy_control_init(struct drm_i915_private *dev_priv)
>  		dev_priv->chv_phy_assert[DPIO_PHY1] = true;
>  	}
>  
> -	intel_de_write(dev_priv, DISPLAY_PHY_CONTROL,
> -		       dev_priv->chv_phy_control);
> -
>  	drm_dbg_kms(&dev_priv->drm, "Initial PHY_CONTROL=0x%08x\n",
>  		    dev_priv->chv_phy_control);
> +
> +	/* Defer application of initial phy_control to enabling the powerwell */

Can't recall if there was a specific reason for wanting to write this
immediately. Maybe not. At least all the asserts are after we write
the register elsewhere so should trip that stuff. I suppose the other
option would be to check that the display power well is enabled before
we write this. But this is probably OK.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

What I don't undestand is what actually changed to cause this? Did we
reorganize something in the init/resume sequence that previously forced
the display power well on before this point, or did we simply not check
for the unclaimed reg access?


>  }
>  
>  static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv)
> -- 
> 2.25.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/display: Defer application of initial chv_phy_control

[Chris Wilson]

Quoting Ville Syrjälä (2020-02-03 13:12:03)
> On Sat, Feb 01, 2020 at 10:31:59AM +0000, Chris Wilson wrote:
> What I don't undestand is what actually changed to cause this? Did we
> reorganize something in the init/resume sequence that previously forced
> the display power well on before this point, or did we simply not check
> for the unclaimed reg access?

I was expecting it have lain undisturbed for a long time. But it's not
in CI_DRM_7837 and then appears in CI_DRM_7850. We miss the intervening
log files, so can't pinpoint the commit exactly.

$ git diff --stat intel/CI_DRM_7837..intel/CI_DRM_7850
 80 files changed, 1304 insertions(+), 449 deletions(-)

Could be worse. Nothing stands out though.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2] drm/i915/display: Defer application of initial chv_phy_control

[Chris Wilson]

To write to the DISPLAY_PHY_CONTROL requires holding the powerwells,
which during early resume we have not yet acquired until later in
intel_display_power_init_hw(). So compute the initial chv_phy_control,
but leave the HW unset until we first acquire the powerwell.

<7> [120.055984] i915 0000:00:02.0: [drm:intel_power_domains_init_hw [i915]] rawclk rate: 200000 kHz
<4> [120.056381] ------------[ cut here ]------------
<4> [120.056621] i915 0000:00:02.0: Unclaimed write to register 0x1e0100
<4> [120.056924] WARNING: CPU: 1 PID: 164 at drivers/gpu/drm/i915/intel_uncore.c:1166 __unclaimed_reg_debug+0x69/0x80 [i915]
<4> [120.056935] Modules linked in: vgem snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic btusb btrtl btbcm btintel i915 bluetooth coretemp crct10dif_pclmul crc32_pclmul snd_hda_intel snd_intel_dspcfg snd_hda_codec ghash_clmulni_intel snd_hwdep ecdh_generic ecc snd_hda_core r8169 snd_pcm lpc_ich realtek pinctrl_cherryview i2c_designware_pci prime_numbers
<4> [120.057027] CPU: 1 PID: 164 Comm: kworker/u4:3 Tainted: G     U            5.5.0-CI-CI_DRM_7854+ #1
<4> [120.057038] Hardware name:  /NUC5CPYB, BIOS PYBSWCEL.86A.0055.2016.0812.1130 08/12/2016
<4> [120.057058] Workqueue: events_unbound async_run_entry_fn
<4> [120.057275] RIP: 0010:__unclaimed_reg_debug+0x69/0x80 [i915]
<4> [120.057289] Code: 48 8b 78 18 48 8b 5f 50 48 85 db 74 2d e8 1f a0 3f e1 45 89 e8 48 89 e9 48 89 da 48 89 c6 48 c7 c7 00 8c 48 a0 e8 67 82 df e0 <0f> 0b 83 2d ce e2 2b 00 01 5b 5d 41 5c 41 5d c3 48 8b 1f eb ce 66
<4> [120.057301] RSP: 0018:ffffc90000bcfd08 EFLAGS: 00010082
<4> [120.057315] RAX: 0000000000000000 RBX: ffff888079919b60 RCX: 0000000000000003
<4> [120.057326] RDX: 0000000080000003 RSI: 0000000000000000 RDI: 00000000ffffffff
<4> [120.057336] RBP: ffffffffa04c9f4e R08: 0000000000000000 R09: 0000000000000001
<4> [120.057348] R10: 0000000025c3d560 R11: 000000006815f798 R12: 0000000000000000
<4> [120.057359] R13: 00000000001e0100 R14: 0000000000000286 R15: ffffffff8234a76b
<4> [120.057371] FS:  0000000000000000(0000) GS:ffff888074b00000(0000) knlGS:0000000000000000
<4> [120.057382] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4> [120.057393] CR2: 000055f4197df0d8 CR3: 000000006f326000 CR4: 00000000001006e0
<4> [120.057404] Call Trace:
<4> [120.057635]  fwtable_write32+0x114/0x1d0 [i915]
<4> [120.057892]  intel_power_domains_init_hw+0x4ff/0x650 [i915]
<4> [120.058150]  intel_power_domains_resume+0x3d/0x70 [i915]
<4> [120.058363]  i915_drm_resume_early+0x97/0xd0 [i915]
<4> [120.058575]  ? i915_resume_switcheroo+0x30/0x30 [i915]
<4> [120.058594]  dpm_run_callback+0x64/0x280
<4> [120.058626]  device_resume_early+0xa7/0xe0
<4> [120.058652]  async_resume_early+0x14/0x40

v2: Write our expected value of DISPLAY_PHY_CONTROL during sync_hw, so
that it should always match the driver state after resume.

Closes: https://gitlab.freedesktop.org/drm/intel/issues/1089
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_power.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 64943179c05e..b01e9c51da7a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -1780,6 +1780,13 @@ static void chv_set_pipe_power_well(struct drm_i915_private *dev_priv,
 	vlv_punit_put(dev_priv);
 }
 
+static void chv_pipe_power_well_sync_hw(struct drm_i915_private *dev_priv,
+				       struct i915_power_well *power_well)
+{
+	intel_de_write(dev_priv, DISPLAY_PHY_CONTROL,
+		       dev_priv->chv_phy_control);
+}
+
 static void chv_pipe_power_well_enable(struct drm_i915_private *dev_priv,
 				       struct i915_power_well *power_well)
 {
@@ -2762,7 +2769,7 @@ static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
 };
 
 static const struct i915_power_well_ops chv_pipe_power_well_ops = {
-	.sync_hw = i9xx_power_well_sync_hw_noop,
+	.sync_hw = chv_pipe_power_well_sync_hw,
 	.enable = chv_pipe_power_well_enable,
 	.disable = chv_pipe_power_well_disable,
 	.is_enabled = chv_pipe_power_well_enabled,
@@ -5163,11 +5170,10 @@ static void chv_phy_control_init(struct drm_i915_private *dev_priv)
 		dev_priv->chv_phy_assert[DPIO_PHY1] = true;
 	}
 
-	intel_de_write(dev_priv, DISPLAY_PHY_CONTROL,
-		       dev_priv->chv_phy_control);
-
 	drm_dbg_kms(&dev_priv->drm, "Initial PHY_CONTROL=0x%08x\n",
 		    dev_priv->chv_phy_control);
+
+	/* Defer application of initial phy_control to enabling the powerwell */
 }
 
 static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv)
-- 
2.25.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915/display: Defer application of initial chv_phy_control

[Ville Syrjälä]

On Mon, Feb 03, 2020 at 02:50:16PM +0000, Chris Wilson wrote:
> To write to the DISPLAY_PHY_CONTROL requires holding the powerwells,
> which during early resume we have not yet acquired until later in
> intel_display_power_init_hw(). So compute the initial chv_phy_control,
> but leave the HW unset until we first acquire the powerwell.
> 
> <7> [120.055984] i915 0000:00:02.0: [drm:intel_power_domains_init_hw [i915]] rawclk rate: 200000 kHz
> <4> [120.056381] ------------[ cut here ]------------
> <4> [120.056621] i915 0000:00:02.0: Unclaimed write to register 0x1e0100
> <4> [120.056924] WARNING: CPU: 1 PID: 164 at drivers/gpu/drm/i915/intel_uncore.c:1166 __unclaimed_reg_debug+0x69/0x80 [i915]
> <4> [120.056935] Modules linked in: vgem snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic btusb btrtl btbcm btintel i915 bluetooth coretemp crct10dif_pclmul crc32_pclmul snd_hda_intel snd_intel_dspcfg snd_hda_codec ghash_clmulni_intel snd_hwdep ecdh_generic ecc snd_hda_core r8169 snd_pcm lpc_ich realtek pinctrl_cherryview i2c_designware_pci prime_numbers
> <4> [120.057027] CPU: 1 PID: 164 Comm: kworker/u4:3 Tainted: G     U            5.5.0-CI-CI_DRM_7854+ #1
> <4> [120.057038] Hardware name:  /NUC5CPYB, BIOS PYBSWCEL.86A.0055.2016.0812.1130 08/12/2016
> <4> [120.057058] Workqueue: events_unbound async_run_entry_fn
> <4> [120.057275] RIP: 0010:__unclaimed_reg_debug+0x69/0x80 [i915]
> <4> [120.057289] Code: 48 8b 78 18 48 8b 5f 50 48 85 db 74 2d e8 1f a0 3f e1 45 89 e8 48 89 e9 48 89 da 48 89 c6 48 c7 c7 00 8c 48 a0 e8 67 82 df e0 <0f> 0b 83 2d ce e2 2b 00 01 5b 5d 41 5c 41 5d c3 48 8b 1f eb ce 66
> <4> [120.057301] RSP: 0018:ffffc90000bcfd08 EFLAGS: 00010082
> <4> [120.057315] RAX: 0000000000000000 RBX: ffff888079919b60 RCX: 0000000000000003
> <4> [120.057326] RDX: 0000000080000003 RSI: 0000000000000000 RDI: 00000000ffffffff
> <4> [120.057336] RBP: ffffffffa04c9f4e R08: 0000000000000000 R09: 0000000000000001
> <4> [120.057348] R10: 0000000025c3d560 R11: 000000006815f798 R12: 0000000000000000
> <4> [120.057359] R13: 00000000001e0100 R14: 0000000000000286 R15: ffffffff8234a76b
> <4> [120.057371] FS:  0000000000000000(0000) GS:ffff888074b00000(0000) knlGS:0000000000000000
> <4> [120.057382] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> <4> [120.057393] CR2: 000055f4197df0d8 CR3: 000000006f326000 CR4: 00000000001006e0
> <4> [120.057404] Call Trace:
> <4> [120.057635]  fwtable_write32+0x114/0x1d0 [i915]
> <4> [120.057892]  intel_power_domains_init_hw+0x4ff/0x650 [i915]
> <4> [120.058150]  intel_power_domains_resume+0x3d/0x70 [i915]
> <4> [120.058363]  i915_drm_resume_early+0x97/0xd0 [i915]
> <4> [120.058575]  ? i915_resume_switcheroo+0x30/0x30 [i915]
> <4> [120.058594]  dpm_run_callback+0x64/0x280
> <4> [120.058626]  device_resume_early+0xa7/0xe0
> <4> [120.058652]  async_resume_early+0x14/0x40
> 
> v2: Write our expected value of DISPLAY_PHY_CONTROL during sync_hw, so
> that it should always match the driver state after resume.
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/issues/1089
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_power.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 64943179c05e..b01e9c51da7a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -1780,6 +1780,13 @@ static void chv_set_pipe_power_well(struct drm_i915_private *dev_priv,
>  	vlv_punit_put(dev_priv);
>  }
>  
> +static void chv_pipe_power_well_sync_hw(struct drm_i915_private *dev_priv,
> +				       struct i915_power_well *power_well)
> +{
> +	intel_de_write(dev_priv, DISPLAY_PHY_CONTROL,
> +		       dev_priv->chv_phy_control);

Done after the we grab the init domain, so should work now

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +}
> +
>  static void chv_pipe_power_well_enable(struct drm_i915_private *dev_priv,
>  				       struct i915_power_well *power_well)
>  {
> @@ -2762,7 +2769,7 @@ static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
>  };
>  
>  static const struct i915_power_well_ops chv_pipe_power_well_ops = {
> -	.sync_hw = i9xx_power_well_sync_hw_noop,
> +	.sync_hw = chv_pipe_power_well_sync_hw,
>  	.enable = chv_pipe_power_well_enable,
>  	.disable = chv_pipe_power_well_disable,
>  	.is_enabled = chv_pipe_power_well_enabled,
> @@ -5163,11 +5170,10 @@ static void chv_phy_control_init(struct drm_i915_private *dev_priv)
>  		dev_priv->chv_phy_assert[DPIO_PHY1] = true;
>  	}
>  
> -	intel_de_write(dev_priv, DISPLAY_PHY_CONTROL,
> -		       dev_priv->chv_phy_control);
> -
>  	drm_dbg_kms(&dev_priv->drm, "Initial PHY_CONTROL=0x%08x\n",
>  		    dev_priv->chv_phy_control);
> +
> +	/* Defer application of initial phy_control to enabling the powerwell */
>  }
>  
>  static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv)
> -- 
> 2.25.0

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/display: Defer application of initial chv_phy_control (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/display: Defer application of initial chv_phy_control (rev2)
URL   : https://patchwork.freedesktop.org/series/72865/
State : failure

== Summary ==

Applying: drm/i915/display: Defer application of initial chv_phy_control
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/display/intel_display_power.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/display/intel_display_power.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/display/intel_display_power.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 drm/i915/display: Defer application of initial chv_phy_control
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/display: Defer application of initial chv_phy_control

[Hans de Goede]

Hi,

On 2/3/20 2:12 PM, Ville Syrjälä wrote:
> On Sat, Feb 01, 2020 at 10:31:59AM +0000, Chris Wilson wrote:
>> To write to the DISPLAY_PHY_CONTROL requires holding the powerwells,
>> which during early resume we have not yet acquired until later in
>> intel_display_power_init_hw(). So compute the initial chv_phy_control,
>> but leave the HW unset until we first acquire the powerwell.
>>
>> <7> [120.055984] i915 0000:00:02.0: [drm:intel_power_domains_init_hw [i915]] rawclk rate: 200000 kHz
>> <4> [120.056381] ------------[ cut here ]------------
>> <4> [120.056621] i915 0000:00:02.0: Unclaimed write to register 0x1e0100
>> <4> [120.056924] WARNING: CPU: 1 PID: 164 at drivers/gpu/drm/i915/intel_uncore.c:1166 __unclaimed_reg_debug+0x69/0x80 [i915]
>> <4> [120.056935] Modules linked in: vgem snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic btusb btrtl btbcm btintel i915 bluetooth coretemp crct10dif_pclmul crc32_pclmul snd_hda_intel snd_intel_dspcfg snd_hda_codec ghash_clmulni_intel snd_hwdep ecdh_generic ecc snd_hda_core r8169 snd_pcm lpc_ich realtek pinctrl_cherryview i2c_designware_pci prime_numbers
>> <4> [120.057027] CPU: 1 PID: 164 Comm: kworker/u4:3 Tainted: G     U            5.5.0-CI-CI_DRM_7854+ #1
>> <4> [120.057038] Hardware name:  /NUC5CPYB, BIOS PYBSWCEL.86A.0055.2016.0812.1130 08/12/2016
>> <4> [120.057058] Workqueue: events_unbound async_run_entry_fn
>> <4> [120.057275] RIP: 0010:__unclaimed_reg_debug+0x69/0x80 [i915]
>> <4> [120.057289] Code: 48 8b 78 18 48 8b 5f 50 48 85 db 74 2d e8 1f a0 3f e1 45 89 e8 48 89 e9 48 89 da 48 89 c6 48 c7 c7 00 8c 48 a0 e8 67 82 df e0 <0f> 0b 83 2d ce e2 2b 00 01 5b 5d 41 5c 41 5d c3 48 8b 1f eb ce 66
>> <4> [120.057301] RSP: 0018:ffffc90000bcfd08 EFLAGS: 00010082
>> <4> [120.057315] RAX: 0000000000000000 RBX: ffff888079919b60 RCX: 0000000000000003
>> <4> [120.057326] RDX: 0000000080000003 RSI: 0000000000000000 RDI: 00000000ffffffff
>> <4> [120.057336] RBP: ffffffffa04c9f4e R08: 0000000000000000 R09: 0000000000000001
>> <4> [120.057348] R10: 0000000025c3d560 R11: 000000006815f798 R12: 0000000000000000
>> <4> [120.057359] R13: 00000000001e0100 R14: 0000000000000286 R15: ffffffff8234a76b
>> <4> [120.057371] FS:  0000000000000000(0000) GS:ffff888074b00000(0000) knlGS:0000000000000000
>> <4> [120.057382] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> <4> [120.057393] CR2: 000055f4197df0d8 CR3: 000000006f326000 CR4: 00000000001006e0
>> <4> [120.057404] Call Trace:
>> <4> [120.057635]  fwtable_write32+0x114/0x1d0 [i915]
>> <4> [120.057892]  intel_power_domains_init_hw+0x4ff/0x650 [i915]
>> <4> [120.058150]  intel_power_domains_resume+0x3d/0x70 [i915]
>> <4> [120.058363]  i915_drm_resume_early+0x97/0xd0 [i915]
>> <4> [120.058575]  ? i915_resume_switcheroo+0x30/0x30 [i915]
>> <4> [120.058594]  dpm_run_callback+0x64/0x280
>> <4> [120.058626]  device_resume_early+0xa7/0xe0
>> <4> [120.058652]  async_resume_early+0x14/0x40
>>
>> Closes: https://gitlab.freedesktop.org/drm/intel/issues/1089
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Imre Deak <imre.deak@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display_power.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
>> index 64943179c05e..492668d5a193 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
>> @@ -5163,11 +5163,10 @@ static void chv_phy_control_init(struct drm_i915_private *dev_priv)
>>   		dev_priv->chv_phy_assert[DPIO_PHY1] = true;
>>   	}
>>   
>> -	intel_de_write(dev_priv, DISPLAY_PHY_CONTROL,
>> -		       dev_priv->chv_phy_control);
>> -
>>   	drm_dbg_kms(&dev_priv->drm, "Initial PHY_CONTROL=0x%08x\n",
>>   		    dev_priv->chv_phy_control);
>> +
>> +	/* Defer application of initial phy_control to enabling the powerwell */
> 
> Can't recall if there was a specific reason for wanting to write this
> immediately. Maybe not. At least all the asserts are after we write
> the register elsewhere so should trip that stuff. I suppose the other
> option would be to check that the display power well is enabled before
> we write this. But this is probably OK.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> What I don't undestand is what actually changed to cause this? Did we
> reorganize something in the init/resume sequence that previously forced
> the display power well on before this point, or did we simply not check
> for the unclaimed reg access?

I have been seeing this happen occasionally for quite a while now, but it
was not always reproducible (IIRC), so I guess that we were racing with some
other code-path which did grab the power-well ?   I might be completely
wrong here, but the WARN triggered by this has been on my radar for quite
a while now.

Anyways, thank you for fixing this Chris.

Regards,

Hans

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx