All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
@ 2020-09-14 17:57 Anusha Srivatsa
  2020-09-14 18:43 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/pll: Centralize PLL_ENABLE register lookup (rev5) Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Anusha Srivatsa @ 2020-09-14 17:57 UTC (permalink / raw)
  To: intel-gfx

We currenty check for platform at multiple parts in the driver
to grab the correct PLL. Let us begin to centralize it through a
helper function.

v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg() (Ville)

v3: Clean up combo_pll_disable() (Rodrigo)

v4: s/dev_priv/i915 (Jani)
Move static and return type to the same line( Ville, Jani)

Suggested-by: Matt Roper <matthew.d.roper@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 35 ++++++++++---------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index c9013f8f766f..e08684e34078 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -147,6 +147,18 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
 			pll->info->name, onoff(state), onoff(cur_state));
 }
 
+static i915_reg_t
+intel_combo_pll_enable_reg(struct drm_i915_private *i915,
+			   struct intel_shared_dpll *pll)
+{
+
+	if (IS_ELKHARTLAKE(i915) && (pll->info->id == DPLL_ID_EHL_DPLL4))
+		return MG_PLL_ENABLE(0);
+
+	return CNL_DPLL_ENABLE(pll->info->id);
+
+
+}
 /**
  * intel_prepare_shared_dpll - call a dpll's prepare hook
  * @crtc_state: CRTC, and its state, which has a shared dpll
@@ -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv,
 				   struct intel_shared_dpll *pll,
 				   struct intel_dpll_hw_state *hw_state)
 {
-	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
-
-	if (IS_ELKHARTLAKE(dev_priv) &&
-	    pll->info->id == DPLL_ID_EHL_DPLL4) {
-		enable_reg = MG_PLL_ENABLE(0);
-	}
+	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
 
 	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
 }
@@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
 static void combo_pll_enable(struct drm_i915_private *dev_priv,
 			     struct intel_shared_dpll *pll)
 {
-	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
+	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
 
 	if (IS_ELKHARTLAKE(dev_priv) &&
 	    pll->info->id == DPLL_ID_EHL_DPLL4) {
-		enable_reg = MG_PLL_ENABLE(0);
 
 		/*
 		 * We need to disable DC states when this DPLL is enabled.
@@ -4157,19 +4163,14 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv,
 static void combo_pll_disable(struct drm_i915_private *dev_priv,
 			      struct intel_shared_dpll *pll)
 {
-	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
+	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
 
-	if (IS_ELKHARTLAKE(dev_priv) &&
-	    pll->info->id == DPLL_ID_EHL_DPLL4) {
-		enable_reg = MG_PLL_ENABLE(0);
-		icl_pll_disable(dev_priv, pll, enable_reg);
+	icl_pll_disable(dev_priv, pll, enable_reg);
 
+	if (IS_ELKHARTLAKE(dev_priv) &&
+	    pll->info->id == DPLL_ID_EHL_DPLL4)
 		intel_display_power_put(dev_priv, POWER_DOMAIN_DPLL_DC_OFF,
 					pll->wakeref);
-		return;
-	}
-
-	icl_pll_disable(dev_priv, pll, enable_reg);
 }
 
 static void tbt_pll_disable(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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/pll: Centralize PLL_ENABLE register lookup (rev5)
  2020-09-14 17:57 [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup Anusha Srivatsa
@ 2020-09-14 18:43 ` Patchwork
  2020-09-14 19:07 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
  2020-09-15  0:54 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2020-09-14 18:43 UTC (permalink / raw)
  To: Anusha Srivatsa; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/pll: Centralize PLL_ENABLE register lookup (rev5)
URL   : https://patchwork.freedesktop.org/series/81150/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
f5ec8439a17d drm/i915/pll: Centralize PLL_ENABLE register lookup
-:38: CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#38: FILE: drivers/gpu/drm/i915/display/intel_dpll_mgr.c:154:
+{
+

-:39: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'pll->info->id == DPLL_ID_EHL_DPLL4'
#39: FILE: drivers/gpu/drm/i915/display/intel_dpll_mgr.c:155:
+	if (IS_ELKHARTLAKE(i915) && (pll->info->id == DPLL_ID_EHL_DPLL4))

-:44: CHECK:LINE_SPACING: Please don't use multiple blank lines
#44: FILE: drivers/gpu/drm/i915/display/intel_dpll_mgr.c:160:
+
+

-:45: CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#45: FILE: drivers/gpu/drm/i915/display/intel_dpll_mgr.c:161:
+
+}

total: 0 errors, 0 warnings, 4 checks, 66 lines checked


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/pll: Centralize PLL_ENABLE register lookup (rev5)
  2020-09-14 17:57 [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup Anusha Srivatsa
  2020-09-14 18:43 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/pll: Centralize PLL_ENABLE register lookup (rev5) Patchwork
@ 2020-09-14 19:07 ` Patchwork
  2020-09-15  0:54 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2020-09-14 19:07 UTC (permalink / raw)
  To: Anusha Srivatsa; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 6410 bytes --]

== Series Details ==

Series: drm/i915/pll: Centralize PLL_ENABLE register lookup (rev5)
URL   : https://patchwork.freedesktop.org/series/81150/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9007 -> Patchwork_18494
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_flink_basic@basic:
    - fi-tgl-y:           [PASS][1] -> [DMESG-WARN][2] ([i915#402]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/fi-tgl-y/igt@gem_flink_basic@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/fi-tgl-y/igt@gem_flink_basic@basic.html

  * igt@i915_module_load@reload:
    - fi-byt-j1900:       [PASS][3] -> [DMESG-WARN][4] ([i915#1982])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/fi-byt-j1900/igt@i915_module_load@reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/fi-byt-j1900/igt@i915_module_load@reload.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1:
    - fi-icl-u2:          [PASS][5] -> [DMESG-WARN][6] ([i915#1982]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html

  
#### Possible fixes ####

  * igt@gem_flink_basic@bad-flink:
    - fi-tgl-y:           [DMESG-WARN][7] ([i915#402]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/fi-tgl-y/igt@gem_flink_basic@bad-flink.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/fi-tgl-y/igt@gem_flink_basic@bad-flink.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-bsw-kefka:       [DMESG-WARN][9] ([i915#1982]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@i915_selftest@live@execlists:
    - fi-icl-y:           [INCOMPLETE][11] ([i915#2276]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/fi-icl-y/igt@i915_selftest@live@execlists.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/fi-icl-y/igt@i915_selftest@live@execlists.html

  * igt@kms_busy@basic@flip:
    - fi-tgl-y:           [DMESG-WARN][13] ([i915#1982]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/fi-tgl-y/igt@kms_busy@basic@flip.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/fi-tgl-y/igt@kms_busy@basic@flip.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7500u:       [DMESG-WARN][15] ([i915#2203]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/fi-kbl-7500u/igt@kms_chamelium@common-hpd-after-suspend.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/fi-kbl-7500u/igt@kms_chamelium@common-hpd-after-suspend.html

  
#### Warnings ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-tgl-y:           [DMESG-WARN][17] ([i915#1982] / [i915#2411]) -> [DMESG-WARN][18] ([i915#2411])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/fi-tgl-y/igt@i915_pm_rpm@basic-pci-d3-state.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/fi-tgl-y/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-x1275:       [DMESG-FAIL][19] ([i915#62]) -> [DMESG-FAIL][20] ([i915#62] / [i915#95])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
    - fi-kbl-x1275:       [DMESG-WARN][21] ([i915#62] / [i915#92]) -> [DMESG-WARN][22] ([i915#62] / [i915#92] / [i915#95]) +6 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html

  * igt@kms_force_connector_basic@force-edid:
    - fi-kbl-x1275:       [DMESG-WARN][23] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][24] ([i915#62] / [i915#92]) +5 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html

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

  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203
  [i915#2276]: https://gitlab.freedesktop.org/drm/intel/issues/2276
  [i915#2411]: https://gitlab.freedesktop.org/drm/intel/issues/2411
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (45 -> 39)
------------------------------

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


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

  * Linux: CI_DRM_9007 -> Patchwork_18494

  CI-20190529: 20190529
  CI_DRM_9007: 30b3e38bd6d569451279af2767b620d0ef88665d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5782: ccfb27dab0f06c009d332053548920cb740e4258 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18494: f5ec8439a17d4de83b182b92dc0e28ab7749de25 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

f5ec8439a17d drm/i915/pll: Centralize PLL_ENABLE register lookup

== Logs ==

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

[-- Attachment #1.2: Type: text/html, Size: 8360 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/pll: Centralize PLL_ENABLE register lookup (rev5)
  2020-09-14 17:57 [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup Anusha Srivatsa
  2020-09-14 18:43 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/pll: Centralize PLL_ENABLE register lookup (rev5) Patchwork
  2020-09-14 19:07 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-09-15  0:54 ` Patchwork
  2 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2020-09-15  0:54 UTC (permalink / raw)
  To: Anusha Srivatsa; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 14328 bytes --]

== Series Details ==

Series: drm/i915/pll: Centralize PLL_ENABLE register lookup (rev5)
URL   : https://patchwork.freedesktop.org/series/81150/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9007_full -> Patchwork_18494_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_whisper@basic-fds:
    - shard-glk:          [PASS][1] -> [DMESG-WARN][2] ([i915#118] / [i915#95])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-glk7/igt@gem_exec_whisper@basic-fds.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-glk9/igt@gem_exec_whisper@basic-fds.html

  * igt@gem_userptr_blits@unsync-unmap-cycles:
    - shard-skl:          [PASS][3] -> [TIMEOUT][4] ([i915#1958])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-skl6/igt@gem_userptr_blits@unsync-unmap-cycles.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-skl1/igt@gem_userptr_blits@unsync-unmap-cycles.html

  * igt@gen9_exec_parse@allowed-single:
    - shard-apl:          [PASS][5] -> [DMESG-WARN][6] ([i915#1436] / [i915#1635] / [i915#716])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-apl8/igt@gen9_exec_parse@allowed-single.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-apl2/igt@gen9_exec_parse@allowed-single.html

  * igt@i915_selftest@mock@contexts:
    - shard-apl:          [PASS][7] -> [INCOMPLETE][8] ([i915#1635] / [i915#2278])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-apl6/igt@i915_selftest@mock@contexts.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-apl4/igt@i915_selftest@mock@contexts.html

  * igt@kms_big_fb@x-tiled-64bpp-rotate-180:
    - shard-glk:          [PASS][9] -> [DMESG-WARN][10] ([i915#1982])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-glk5/igt@kms_big_fb@x-tiled-64bpp-rotate-180.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-glk2/igt@kms_big_fb@x-tiled-64bpp-rotate-180.html

  * igt@kms_cursor_legacy@basic-flip-before-cursor-atomic:
    - shard-tglb:         [PASS][11] -> [DMESG-WARN][12] ([i915#1982])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-tglb6/igt@kms_cursor_legacy@basic-flip-before-cursor-atomic.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-tglb1/igt@kms_cursor_legacy@basic-flip-before-cursor-atomic.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-skl:          [PASS][13] -> [FAIL][14] ([i915#2346])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-skl10/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-skl7/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible@a-dp1:
    - shard-apl:          [PASS][15] -> [DMESG-WARN][16] ([i915#1635] / [i915#1982])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-apl3/igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible@a-dp1.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-apl7/igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible@a-dp1.html
    - shard-kbl:          [PASS][17] -> [DMESG-WARN][18] ([i915#1982]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-kbl7/igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible@a-dp1.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-kbl6/igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible@a-dp1.html

  * igt@kms_flip@flip-vs-suspend@c-dp1:
    - shard-kbl:          [PASS][19] -> [DMESG-WARN][20] ([i915#180]) +7 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-kbl4/igt@kms_flip@flip-vs-suspend@c-dp1.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-kbl1/igt@kms_flip@flip-vs-suspend@c-dp1.html

  * igt@kms_flip_tiling@flip-y-tiled:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([i915#699])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-skl5/igt@kms_flip_tiling@flip-y-tiled.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-skl5/igt@kms_flip_tiling@flip-y-tiled.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-msflip-blt:
    - shard-iclb:         [PASS][23] -> [DMESG-WARN][24] ([i915#1982]) +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-msflip-blt.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-msflip-blt.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([i915#1188]) +2 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-skl7/igt@kms_hdr@bpc-switch-dpms.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-skl8/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [PASS][27] -> [FAIL][28] ([fdo#108145] / [i915#265]) +1 similar issue
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [PASS][29] -> [SKIP][30] ([fdo#109441]) +2 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-iclb1/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@perf@polling-parameterized:
    - shard-iclb:         [PASS][31] -> [FAIL][32] ([i915#1542])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-iclb7/igt@perf@polling-parameterized.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-iclb8/igt@perf@polling-parameterized.html

  * igt@perf@polling-small-buf:
    - shard-skl:          [PASS][33] -> [DMESG-WARN][34] ([i915#1982]) +11 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-skl7/igt@perf@polling-small-buf.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-skl7/igt@perf@polling-small-buf.html

  
#### Possible fixes ####

  * igt@gem_ctx_persistence@legacy-engines-mixed-process@blt:
    - shard-apl:          [FAIL][35] ([i915#1635] / [i915#2374]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-apl1/igt@gem_ctx_persistence@legacy-engines-mixed-process@blt.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-apl8/igt@gem_ctx_persistence@legacy-engines-mixed-process@blt.html

  * igt@gem_exec_parallel@fds@vecs0:
    - shard-skl:          [DMESG-WARN][37] ([i915#1982]) -> [PASS][38] +2 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-skl9/igt@gem_exec_parallel@fds@vecs0.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-skl3/igt@gem_exec_parallel@fds@vecs0.html

  * igt@gem_exec_whisper@basic-fds-forked:
    - shard-glk:          [DMESG-WARN][39] ([i915#118] / [i915#95]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-glk1/igt@gem_exec_whisper@basic-fds-forked.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-glk9/igt@gem_exec_whisper@basic-fds-forked.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-apl:          [FAIL][41] ([i915#1635] / [i915#644]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-apl7/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-apl4/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-skl:          [INCOMPLETE][43] ([i915#300]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-skl7/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-skl8/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_cursor_legacy@short-flip-after-cursor-atomic-transitions:
    - shard-tglb:         [DMESG-WARN][45] ([i915#1982]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-tglb5/igt@kms_cursor_legacy@short-flip-after-cursor-atomic-transitions.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-tglb6/igt@kms_cursor_legacy@short-flip-after-cursor-atomic-transitions.html

  * igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-untiled:
    - shard-apl:          [DMESG-WARN][47] ([i915#1635] / [i915#1982]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-apl4/igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-untiled.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-apl3/igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-untiled.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1:
    - shard-skl:          [FAIL][49] ([i915#2122]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-skl9/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-skl4/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1:
    - shard-skl:          [FAIL][51] ([i915#79]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-skl9/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-skl4/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@a-dp1:
    - shard-kbl:          [DMESG-WARN][53] ([i915#180]) -> [PASS][54] +2 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-kbl1/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-kbl2/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-move:
    - shard-iclb:         [DMESG-WARN][55] ([i915#1982]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-iclb3/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-move.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-iclb3/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-move.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          [FAIL][57] ([fdo#108145] / [i915#265]) -> [PASS][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-skl10/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-skl1/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-iclb:         [SKIP][59] ([fdo#109441]) -> [PASS][60] +2 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-iclb8/igt@kms_psr@psr2_cursor_plane_onoff.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html

  * igt@perf@polling-parameterized:
    - shard-skl:          [FAIL][61] ([i915#1542]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9007/shard-skl8/igt@perf@polling-parameterized.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18494/shard-skl5/igt@perf@polling-parameterized.html

  
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1958]: https://gitlab.freedesktop.org/drm/intel/issues/1958
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2278]: https://gitlab.freedesktop.org/drm/intel/issues/2278
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2374]: https://gitlab.freedesktop.org/drm/intel/issues/2374
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#300]: https://gitlab.freedesktop.org/drm/intel/issues/300
  [i915#644]: https://gitlab.freedesktop.org/drm/intel/issues/644
  [i915#699]: https://gitlab.freedesktop.org/drm/intel/issues/699
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (11 -> 11)
------------------------------

  No changes in participating hosts


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

  * Linux: CI_DRM_9007 -> Patchwork_18494

  CI-20190529: 20190529
  CI_DRM_9007: 30b3e38bd6d569451279af2767b620d0ef88665d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5782: ccfb27dab0f06c009d332053548920cb740e4258 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18494: f5ec8439a17d4de83b182b92dc0e28ab7749de25 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

[-- Attachment #1.2: Type: text/html, Size: 17077 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
@ 2020-09-10 23:48 Anusha Srivatsa
  0 siblings, 0 replies; 19+ messages in thread
From: Anusha Srivatsa @ 2020-09-10 23:48 UTC (permalink / raw)
  To: intel-gfx

We currenty check for platform at multiple parts in the driver
to grab the correct PLL. Let us begin to centralize it through a
helper function.

v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg() (Ville)

v3: Clean up combo_pll_disable() (Rodrigo)

v4: s/dev_priv/i915 (Jani)
Move static and return type to the same line( Ville, Jxani)

Suggested-by: Matt Roper <matthew.d.roper@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 35 ++++++++++---------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index c9013f8f766f..e08684e34078 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -147,6 +147,18 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
 			pll->info->name, onoff(state), onoff(cur_state));
 }
 
+static i915_reg_t
+intel_combo_pll_enable_reg(struct drm_i915_private *i915,
+			   struct intel_shared_dpll *pll)
+{
+
+	if (IS_ELKHARTLAKE(i915) && (pll->info->id == DPLL_ID_EHL_DPLL4))
+		return MG_PLL_ENABLE(0);
+
+	return CNL_DPLL_ENABLE(pll->info->id);
+
+
+}
 /**
  * intel_prepare_shared_dpll - call a dpll's prepare hook
  * @crtc_state: CRTC, and its state, which has a shared dpll
@@ -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv,
 				   struct intel_shared_dpll *pll,
 				   struct intel_dpll_hw_state *hw_state)
 {
-	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
-
-	if (IS_ELKHARTLAKE(dev_priv) &&
-	    pll->info->id == DPLL_ID_EHL_DPLL4) {
-		enable_reg = MG_PLL_ENABLE(0);
-	}
+	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
 
 	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
 }
@@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
 static void combo_pll_enable(struct drm_i915_private *dev_priv,
 			     struct intel_shared_dpll *pll)
 {
-	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
+	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
 
 	if (IS_ELKHARTLAKE(dev_priv) &&
 	    pll->info->id == DPLL_ID_EHL_DPLL4) {
-		enable_reg = MG_PLL_ENABLE(0);
 
 		/*
 		 * We need to disable DC states when this DPLL is enabled.
@@ -4157,19 +4163,14 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv,
 static void combo_pll_disable(struct drm_i915_private *dev_priv,
 			      struct intel_shared_dpll *pll)
 {
-	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
+	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
 
-	if (IS_ELKHARTLAKE(dev_priv) &&
-	    pll->info->id == DPLL_ID_EHL_DPLL4) {
-		enable_reg = MG_PLL_ENABLE(0);
-		icl_pll_disable(dev_priv, pll, enable_reg);
+	icl_pll_disable(dev_priv, pll, enable_reg);
 
+	if (IS_ELKHARTLAKE(dev_priv) &&
+	    pll->info->id == DPLL_ID_EHL_DPLL4)
 		intel_display_power_put(dev_priv, POWER_DOMAIN_DPLL_DC_OFF,
 					pll->wakeref);
-		return;
-	}
-
-	icl_pll_disable(dev_priv, pll, enable_reg);
 }
 
 static void tbt_pll_disable(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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
  2020-09-10 13:30 ` Jani Nikula
@ 2020-09-10 17:05   ` Srivatsa, Anusha
  0 siblings, 0 replies; 19+ messages in thread
From: Srivatsa, Anusha @ 2020-09-10 17:05 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx



> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Thursday, September 10, 2020 6:31 AM
> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel-
> gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register
> lookup
> 
> On Tue, 08 Sep 2020, Anusha Srivatsa <anusha.srivatsa@intel.com> wrote:
> > We currenty check for platform at multiple parts in the driver to grab
> > the correct PLL. Let us begin to centralize it through a helper
> > function.
> >
> > v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg() (Ville)
> >
> > v3: Clean up combo_pll_disable() (Rodrigo)
> >
> > Suggested-by: Matt Roper <matthew.d.roper@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 29
> > +++++++++++--------
> >  1 file changed, 17 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index c9013f8f766f..441b6f52e808 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -147,6 +147,18 @@ void assert_shared_dpll(struct drm_i915_private
> *dev_priv,
> >  			pll->info->name, onoff(state), onoff(cur_state));  }
> >
> > +static
> > +i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private
> > +*dev_priv,
> 
> Please keep the static keyword and the return type on the same line with
> each other.
> 
> And since you're touching this, please rename dev_priv to i915 in all new
> code adding it.

Sure. Thanks for the feedback Jani.

Anusha
> BR,
> Jani.
> 
> 
> > +				    struct intel_shared_dpll *pll) {
> > +
> > +	if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id ==
> DPLL_ID_EHL_DPLL4))
> > +			return MG_PLL_ENABLE(0);
> > +
> > +	return CNL_DPLL_ENABLE(pll->info->id);
> > +
> > +
> > +}
> >  /**
> >   * intel_prepare_shared_dpll - call a dpll's prepare hook
> >   * @crtc_state: CRTC, and its state, which has a shared dpll @@
> > -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct
> drm_i915_private *dev_priv,
> >  				   struct intel_shared_dpll *pll,
> >  				   struct intel_dpll_hw_state *hw_state)  {
> > -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > -
> > -	if (IS_ELKHARTLAKE(dev_priv) &&
> > -	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> > -		enable_reg = MG_PLL_ENABLE(0);
> > -	}
> > +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> >
> >  	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);  }
> > @@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct
> > drm_i915_private *dev_priv,  static void combo_pll_enable(struct
> drm_i915_private *dev_priv,
> >  			     struct intel_shared_dpll *pll)  {
> > -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> >
> >  	if (IS_ELKHARTLAKE(dev_priv) &&
> >  	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> > -		enable_reg = MG_PLL_ENABLE(0);
> >
> >  		/*
> >  		 * We need to disable DC states when this DPLL is enabled.
> > @@ -4157,19 +4163,18 @@ static void icl_pll_disable(struct
> > drm_i915_private *dev_priv,  static void combo_pll_disable(struct
> drm_i915_private *dev_priv,
> >  			      struct intel_shared_dpll *pll)  {
> > -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> > +
> > +	icl_pll_disable(dev_priv, pll, enable_reg);
> >
> >  	if (IS_ELKHARTLAKE(dev_priv) &&
> >  	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> > -		enable_reg = MG_PLL_ENABLE(0);
> > -		icl_pll_disable(dev_priv, pll, enable_reg);
> >
> >  		intel_display_power_put(dev_priv,
> POWER_DOMAIN_DPLL_DC_OFF,
> >  					pll->wakeref);
> >  		return;
> >  	}
> >
> > -	icl_pll_disable(dev_priv, pll, enable_reg);
> >  }
> >
> >  static void tbt_pll_disable(struct drm_i915_private *dev_priv,
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
  2020-09-08 23:39 Anusha Srivatsa
  2020-09-09 13:06 ` Vivi, Rodrigo
@ 2020-09-10 13:30 ` Jani Nikula
  2020-09-10 17:05   ` Srivatsa, Anusha
  1 sibling, 1 reply; 19+ messages in thread
From: Jani Nikula @ 2020-09-10 13:30 UTC (permalink / raw)
  To: Anusha Srivatsa, intel-gfx

On Tue, 08 Sep 2020, Anusha Srivatsa <anusha.srivatsa@intel.com> wrote:
> We currenty check for platform at multiple parts in the driver
> to grab the correct PLL. Let us begin to centralize it through a
> helper function.
>
> v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg() (Ville)
>
> v3: Clean up combo_pll_disable() (Rodrigo)
>
> Suggested-by: Matt Roper <matthew.d.roper@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 29 +++++++++++--------
>  1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index c9013f8f766f..441b6f52e808 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -147,6 +147,18 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
>  			pll->info->name, onoff(state), onoff(cur_state));
>  }
>  
> +static
> +i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private *dev_priv,

Please keep the static keyword and the return type on the same line with
each other.

And since you're touching this, please rename dev_priv to i915 in all
new code adding it.

BR,
Jani.


> +				    struct intel_shared_dpll *pll)
> +{
> +
> +	if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id == DPLL_ID_EHL_DPLL4))
> +			return MG_PLL_ENABLE(0);
> +
> +	return CNL_DPLL_ENABLE(pll->info->id);
> +
> +
> +}
>  /**
>   * intel_prepare_shared_dpll - call a dpll's prepare hook
>   * @crtc_state: CRTC, and its state, which has a shared dpll
> @@ -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  				   struct intel_shared_dpll *pll,
>  				   struct intel_dpll_hw_state *hw_state)
>  {
> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> -
> -	if (IS_ELKHARTLAKE(dev_priv) &&
> -	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> -		enable_reg = MG_PLL_ENABLE(0);
> -	}
> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
>  
>  	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
>  }
> @@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
>  static void combo_pll_enable(struct drm_i915_private *dev_priv,
>  			     struct intel_shared_dpll *pll)
>  {
> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
>  
>  	if (IS_ELKHARTLAKE(dev_priv) &&
>  	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> -		enable_reg = MG_PLL_ENABLE(0);
>  
>  		/*
>  		 * We need to disable DC states when this DPLL is enabled.
> @@ -4157,19 +4163,18 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv,
>  static void combo_pll_disable(struct drm_i915_private *dev_priv,
>  			      struct intel_shared_dpll *pll)
>  {
> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> +
> +	icl_pll_disable(dev_priv, pll, enable_reg);
>  
>  	if (IS_ELKHARTLAKE(dev_priv) &&
>  	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> -		enable_reg = MG_PLL_ENABLE(0);
> -		icl_pll_disable(dev_priv, pll, enable_reg);
>  
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_DPLL_DC_OFF,
>  					pll->wakeref);
>  		return;
>  	}
>  
> -	icl_pll_disable(dev_priv, pll, enable_reg);
>  }
>  
>  static void tbt_pll_disable(struct drm_i915_private *dev_priv,

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
  2020-09-08 23:39 Anusha Srivatsa
@ 2020-09-09 13:06 ` Vivi, Rodrigo
  2020-09-10 13:30 ` Jani Nikula
  1 sibling, 0 replies; 19+ messages in thread
From: Vivi, Rodrigo @ 2020-09-09 13:06 UTC (permalink / raw)
  To: Srivatsa, Anusha; +Cc: intel-gfx



> On Sep 8, 2020, at 4:39 PM, Srivatsa, Anusha <anusha.srivatsa@intel.com> wrote:
> 
> We currenty check for platform at multiple parts in the driver
> to grab the correct PLL. Let us begin to centralize it through a
> helper function.
> 
> v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg() (Ville)
> 
> v3: Clean up combo_pll_disable() (Rodrigo)
> 
> Suggested-by: Matt Roper <matthew.d.roper@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 29 +++++++++++--------
> 1 file changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index c9013f8f766f..441b6f52e808 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -147,6 +147,18 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
> 			pll->info->name, onoff(state), onoff(cur_state));
> }
> 
> +static
> +i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private *dev_priv,
> +				    struct intel_shared_dpll *pll)
> +{
> +
> +	if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id == DPLL_ID_EHL_DPLL4))
> +			return MG_PLL_ENABLE(0);
> +
> +	return CNL_DPLL_ENABLE(pll->info->id);
> +
> +
> +}
> /**
>  * intel_prepare_shared_dpll - call a dpll's prepare hook
>  * @crtc_state: CRTC, and its state, which has a shared dpll
> @@ -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv,
> 				   struct intel_shared_dpll *pll,
> 				   struct intel_dpll_hw_state *hw_state)
> {
> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> -
> -	if (IS_ELKHARTLAKE(dev_priv) &&
> -	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> -		enable_reg = MG_PLL_ENABLE(0);
> -	}
> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> 
> 	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
> }
> @@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
> static void combo_pll_enable(struct drm_i915_private *dev_priv,
> 			     struct intel_shared_dpll *pll)
> {
> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> 
> 	if (IS_ELKHARTLAKE(dev_priv) &&
> 	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> -		enable_reg = MG_PLL_ENABLE(0);
> 
> 		/*
> 		 * We need to disable DC states when this DPLL is enabled.
> @@ -4157,19 +4163,18 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv,
> static void combo_pll_disable(struct drm_i915_private *dev_priv,
> 			      struct intel_shared_dpll *pll)
> {
> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> +
> +	icl_pll_disable(dev_priv, pll, enable_reg);
> 
> 	if (IS_ELKHARTLAKE(dev_priv) &&
> 	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> -		enable_reg = MG_PLL_ENABLE(0);
> -		icl_pll_disable(dev_priv, pll, enable_reg);
> 
> 		intel_display_power_put(dev_priv, POWER_DOMAIN_DPLL_DC_OFF,
> 					pll->wakeref);
> 		return;

this return can also be removed

> 	}
> 
> -	icl_pll_disable(dev_priv, pll, enable_reg);
> }
> 
> static void tbt_pll_disable(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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
@ 2020-09-08 23:39 Anusha Srivatsa
  2020-09-09 13:06 ` Vivi, Rodrigo
  2020-09-10 13:30 ` Jani Nikula
  0 siblings, 2 replies; 19+ messages in thread
From: Anusha Srivatsa @ 2020-09-08 23:39 UTC (permalink / raw)
  To: intel-gfx

We currenty check for platform at multiple parts in the driver
to grab the correct PLL. Let us begin to centralize it through a
helper function.

v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg() (Ville)

v3: Clean up combo_pll_disable() (Rodrigo)

Suggested-by: Matt Roper <matthew.d.roper@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 29 +++++++++++--------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index c9013f8f766f..441b6f52e808 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -147,6 +147,18 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
 			pll->info->name, onoff(state), onoff(cur_state));
 }
 
+static
+i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private *dev_priv,
+				    struct intel_shared_dpll *pll)
+{
+
+	if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id == DPLL_ID_EHL_DPLL4))
+			return MG_PLL_ENABLE(0);
+
+	return CNL_DPLL_ENABLE(pll->info->id);
+
+
+}
 /**
  * intel_prepare_shared_dpll - call a dpll's prepare hook
  * @crtc_state: CRTC, and its state, which has a shared dpll
@@ -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv,
 				   struct intel_shared_dpll *pll,
 				   struct intel_dpll_hw_state *hw_state)
 {
-	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
-
-	if (IS_ELKHARTLAKE(dev_priv) &&
-	    pll->info->id == DPLL_ID_EHL_DPLL4) {
-		enable_reg = MG_PLL_ENABLE(0);
-	}
+	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
 
 	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
 }
@@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
 static void combo_pll_enable(struct drm_i915_private *dev_priv,
 			     struct intel_shared_dpll *pll)
 {
-	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
+	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
 
 	if (IS_ELKHARTLAKE(dev_priv) &&
 	    pll->info->id == DPLL_ID_EHL_DPLL4) {
-		enable_reg = MG_PLL_ENABLE(0);
 
 		/*
 		 * We need to disable DC states when this DPLL is enabled.
@@ -4157,19 +4163,18 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv,
 static void combo_pll_disable(struct drm_i915_private *dev_priv,
 			      struct intel_shared_dpll *pll)
 {
-	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
+	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
+
+	icl_pll_disable(dev_priv, pll, enable_reg);
 
 	if (IS_ELKHARTLAKE(dev_priv) &&
 	    pll->info->id == DPLL_ID_EHL_DPLL4) {
-		enable_reg = MG_PLL_ENABLE(0);
-		icl_pll_disable(dev_priv, pll, enable_reg);
 
 		intel_display_power_put(dev_priv, POWER_DOMAIN_DPLL_DC_OFF,
 					pll->wakeref);
 		return;
 	}
 
-	icl_pll_disable(dev_priv, pll, enable_reg);
 }
 
 static void tbt_pll_disable(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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
  2020-09-03 17:04       ` Srivatsa, Anusha
@ 2020-09-08 21:00         ` Vivi, Rodrigo
  0 siblings, 0 replies; 19+ messages in thread
From: Vivi, Rodrigo @ 2020-09-08 21:00 UTC (permalink / raw)
  To: Srivatsa, Anusha; +Cc: intel-gfx



> On Sep 3, 2020, at 10:04 AM, Srivatsa, Anusha <anusha.srivatsa@intel.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
>> Sent: Wednesday, September 2, 2020 2:32 PM
>> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register
>> lookup
>> 
>> 
>> 
>>> On Sep 2, 2020, at 12:30 PM, Srivatsa, Anusha
>> <anusha.srivatsa@intel.com> wrote:
>>> 
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>> Sent: Tuesday, September 1, 2020 12:30 PM
>>>> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>>>> Cc: intel-gfx@lists.freedesktop.org
>>>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE
>>>> register lookup
>>>> 
>>>> On Tue, Sep 01, 2020 at 11:27:58AM -0700, Anusha Srivatsa wrote:
>>>>> We currenty check for platform at multiple parts in the driver to
>>>>> grab the correct PLL. Let us begin to centralize it through a helper
>>>>> function.
>>>>> 
>>>>> v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg()
>>>>> (Ville)
>>>>> 
>>>>> Suggested-by: Matt Roper <matthew.d.roper@intel.com>
>>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>> Cc: Matt Roper <matthew.d.roper@intel.com>
>>>>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>>>>> ---
>>>>> drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 25
>>>>> +++++++++++--------
>>>>> 1 file changed, 15 insertions(+), 10 deletions(-)
>>>>> 
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>>>>> b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>>>>> index c9013f8f766f..7440836c5e44 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>>>>> @@ -147,6 +147,18 @@ void assert_shared_dpll(struct
>> drm_i915_private
>>>> *dev_priv,
>>>>> 			pll->info->name, onoff(state), onoff(cur_state));  }
>>>>> 
>>>>> +static
>>>>> +i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private
>>>> *dev_priv,
>>>>> +				    struct intel_shared_dpll *pll) {
>>>>> +
>>>>> +	if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id ==
>>>> DPLL_ID_EHL_DPLL4))
>>>>> +			return MG_PLL_ENABLE(0);
>>>>> +
>>>>> +	return CNL_DPLL_ENABLE(pll->info->id);
>>>>> +
>>>>> +
>>>>> +}
>>>>> /**
>>>>> * intel_prepare_shared_dpll - call a dpll's prepare hook
>>>>> * @crtc_state: CRTC, and its state, which has a shared dpll @@
>>>>> -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct
>>>> drm_i915_private *dev_priv,
>>>>> 				   struct intel_shared_dpll *pll,
>>>>> 				   struct intel_dpll_hw_state *hw_state)  {
>>>>> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
>>>>> -
>>>>> -	if (IS_ELKHARTLAKE(dev_priv) &&
>>>>> -	    pll->info->id == DPLL_ID_EHL_DPLL4) {
>>>>> -		enable_reg = MG_PLL_ENABLE(0);
>>>>> -	}
>>>>> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
>>>>> 
>>>>> 	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
>>>>> } @@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct
>>>>> drm_i915_private *dev_priv,  static void combo_pll_enable(struct
>>>> drm_i915_private *dev_priv,
>>>>> 			     struct intel_shared_dpll *pll)  {
>>>>> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
>>>>> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
>>>>> 
>>>>> 	if (IS_ELKHARTLAKE(dev_priv) &&
>>>>> 	    pll->info->id == DPLL_ID_EHL_DPLL4) {
>>>> 
>>>> there's probably something else that we can do now with the
>>>> power_{put,get} to get rid of the, now, doubled if checks...
>>> 
>>> Don't follow you here Rodrigo.
>> 
>> me neither ;)
>> I'm just brainstorming... thinking out lout.
>> 
>>> Are you suggesting using power_{put/get} to somehow get rid of doubled
>> if?
>> 
>> after this patch, on this path we will do this if check twice.
>> not a big deal, but we can probably do something better.
>> 
>> However I don't understand why we had this get/put here at first place.
>> Only for this platform and only for this pll4. So, what I am wondering is that
>> we have something better to do with the power_well infrastructure in
>> general that would allow us to avoid the if (platform && pll4) in favor of
>> something more generic.
>> 
>> but definitely not a blocker for this patch itself.
> Ok. So maybe the power well infrastructure change can be part a later patch?

sure

> 
>> 
>>> 
>>>>> -		enable_reg = MG_PLL_ENABLE(0);
>>>>> 
>>>>> 		/*
>>>>> 		 * We need to disable DC states when this DPLL is enabled.
>>>>> @@ -4157,11 +4163,10 @@ static void icl_pll_disable(struct
>>>>> drm_i915_private *dev_priv,  static void combo_pll_disable(struct
>>>> drm_i915_private *dev_priv,
>>>>> 			      struct intel_shared_dpll *pll)  {
>>>>> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
>>>>> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
>>>>> 
>>>>> 	if (IS_ELKHARTLAKE(dev_priv) &&
>>>>> 	    pll->info->id == DPLL_ID_EHL_DPLL4) {
>>>>> -		enable_reg = MG_PLL_ENABLE(0);
>>>>> 		icl_pll_disable(dev_priv, pll, enable_reg);
>>>> 
>>>> but here, at least, let's clean this function now...
>>>> move this call above and out of the if and delete the one below and
>>>> keep just the power_put inside the if...
>>> 
>>> Good change. Thanks!
>>> Will change that.
> With the above code movement, do I have your reviewed-by?

yes

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

> 
> Anusha 
>>> Anusha
>>> 
>>>>> 
>>>>> 		intel_display_power_put(dev_priv,
>>>> POWER_DOMAIN_DPLL_DC_OFF,
>>>>> --
>>>>> 2.25.0
>>>>> 
>>>>> _______________________________________________
>>>>> Intel-gfx mailing list
>>>>> Intel-gfx@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
  2020-09-02 21:31     ` Vivi, Rodrigo
@ 2020-09-03 17:04       ` Srivatsa, Anusha
  2020-09-08 21:00         ` Vivi, Rodrigo
  0 siblings, 1 reply; 19+ messages in thread
From: Srivatsa, Anusha @ 2020-09-03 17:04 UTC (permalink / raw)
  To: Vivi, Rodrigo; +Cc: intel-gfx



> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Sent: Wednesday, September 2, 2020 2:32 PM
> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register
> lookup
> 
> 
> 
> > On Sep 2, 2020, at 12:30 PM, Srivatsa, Anusha
> <anusha.srivatsa@intel.com> wrote:
> >
> >
> >
> >> -----Original Message-----
> >> From: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >> Sent: Tuesday, September 1, 2020 12:30 PM
> >> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> >> Cc: intel-gfx@lists.freedesktop.org
> >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE
> >> register lookup
> >>
> >> On Tue, Sep 01, 2020 at 11:27:58AM -0700, Anusha Srivatsa wrote:
> >>> We currenty check for platform at multiple parts in the driver to
> >>> grab the correct PLL. Let us begin to centralize it through a helper
> >>> function.
> >>>
> >>> v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg()
> >>> (Ville)
> >>>
> >>> Suggested-by: Matt Roper <matthew.d.roper@intel.com>
> >>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> Cc: Matt Roper <matthew.d.roper@intel.com>
> >>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >>> ---
> >>> drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 25
> >>> +++++++++++--------
> >>> 1 file changed, 15 insertions(+), 10 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> >>> b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> >>> index c9013f8f766f..7440836c5e44 100644
> >>> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> >>> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> >>> @@ -147,6 +147,18 @@ void assert_shared_dpll(struct
> drm_i915_private
> >> *dev_priv,
> >>> 			pll->info->name, onoff(state), onoff(cur_state));  }
> >>>
> >>> +static
> >>> +i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private
> >> *dev_priv,
> >>> +				    struct intel_shared_dpll *pll) {
> >>> +
> >>> +	if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id ==
> >> DPLL_ID_EHL_DPLL4))
> >>> +			return MG_PLL_ENABLE(0);
> >>> +
> >>> +	return CNL_DPLL_ENABLE(pll->info->id);
> >>> +
> >>> +
> >>> +}
> >>> /**
> >>>  * intel_prepare_shared_dpll - call a dpll's prepare hook
> >>>  * @crtc_state: CRTC, and its state, which has a shared dpll @@
> >>> -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct
> >> drm_i915_private *dev_priv,
> >>> 				   struct intel_shared_dpll *pll,
> >>> 				   struct intel_dpll_hw_state *hw_state)  {
> >>> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> >>> -
> >>> -	if (IS_ELKHARTLAKE(dev_priv) &&
> >>> -	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> >>> -		enable_reg = MG_PLL_ENABLE(0);
> >>> -	}
> >>> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> >>>
> >>> 	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
> >>> } @@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct
> >>> drm_i915_private *dev_priv,  static void combo_pll_enable(struct
> >> drm_i915_private *dev_priv,
> >>> 			     struct intel_shared_dpll *pll)  {
> >>> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> >>> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> >>>
> >>> 	if (IS_ELKHARTLAKE(dev_priv) &&
> >>> 	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> >>
> >> there's probably something else that we can do now with the
> >> power_{put,get} to get rid of the, now, doubled if checks...
> >
> > Don't follow you here Rodrigo.
> 
> me neither ;)
> I'm just brainstorming... thinking out lout.
> 
> > Are you suggesting using power_{put/get} to somehow get rid of doubled
> if?
> 
> after this patch, on this path we will do this if check twice.
> not a big deal, but we can probably do something better.
> 
> However I don't understand why we had this get/put here at first place.
> Only for this platform and only for this pll4. So, what I am wondering is that
> we have something better to do with the power_well infrastructure in
> general that would allow us to avoid the if (platform && pll4) in favor of
> something more generic.
> 
> but definitely not a blocker for this patch itself.
Ok. So maybe the power well infrastructure change can be part a later patch?

> 
> >
> >>> -		enable_reg = MG_PLL_ENABLE(0);
> >>>
> >>> 		/*
> >>> 		 * We need to disable DC states when this DPLL is enabled.
> >>> @@ -4157,11 +4163,10 @@ static void icl_pll_disable(struct
> >>> drm_i915_private *dev_priv,  static void combo_pll_disable(struct
> >> drm_i915_private *dev_priv,
> >>> 			      struct intel_shared_dpll *pll)  {
> >>> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> >>> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> >>>
> >>> 	if (IS_ELKHARTLAKE(dev_priv) &&
> >>> 	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> >>> -		enable_reg = MG_PLL_ENABLE(0);
> >>> 		icl_pll_disable(dev_priv, pll, enable_reg);
> >>
> >> but here, at least, let's clean this function now...
> >> move this call above and out of the if and delete the one below and
> >> keep just the power_put inside the if...
> >
> > Good change. Thanks!
> > Will change that.
 With the above code movement, do I have your reviewed-by?

Anusha 
> > Anusha
> >
> >>>
> >>> 		intel_display_power_put(dev_priv,
> >> POWER_DOMAIN_DPLL_DC_OFF,
> >>> --
> >>> 2.25.0
> >>>
> >>> _______________________________________________
> >>> Intel-gfx mailing list
> >>> Intel-gfx@lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
  2020-09-02 19:30   ` Srivatsa, Anusha
@ 2020-09-02 21:31     ` Vivi, Rodrigo
  2020-09-03 17:04       ` Srivatsa, Anusha
  0 siblings, 1 reply; 19+ messages in thread
From: Vivi, Rodrigo @ 2020-09-02 21:31 UTC (permalink / raw)
  To: Srivatsa, Anusha; +Cc: intel-gfx



> On Sep 2, 2020, at 12:30 PM, Srivatsa, Anusha <anusha.srivatsa@intel.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Sent: Tuesday, September 1, 2020 12:30 PM
>> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register
>> lookup
>> 
>> On Tue, Sep 01, 2020 at 11:27:58AM -0700, Anusha Srivatsa wrote:
>>> We currenty check for platform at multiple parts in the driver to grab
>>> the correct PLL. Let us begin to centralize it through a helper
>>> function.
>>> 
>>> v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg() (Ville)
>>> 
>>> Suggested-by: Matt Roper <matthew.d.roper@intel.com>
>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Cc: Matt Roper <matthew.d.roper@intel.com>
>>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 25
>>> +++++++++++--------
>>> 1 file changed, 15 insertions(+), 10 deletions(-)
>>> 
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>>> b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>>> index c9013f8f766f..7440836c5e44 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>>> @@ -147,6 +147,18 @@ void assert_shared_dpll(struct drm_i915_private
>> *dev_priv,
>>> 			pll->info->name, onoff(state), onoff(cur_state));  }
>>> 
>>> +static
>>> +i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private
>> *dev_priv,
>>> +				    struct intel_shared_dpll *pll) {
>>> +
>>> +	if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id ==
>> DPLL_ID_EHL_DPLL4))
>>> +			return MG_PLL_ENABLE(0);
>>> +
>>> +	return CNL_DPLL_ENABLE(pll->info->id);
>>> +
>>> +
>>> +}
>>> /**
>>>  * intel_prepare_shared_dpll - call a dpll's prepare hook
>>>  * @crtc_state: CRTC, and its state, which has a shared dpll @@
>>> -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct
>> drm_i915_private *dev_priv,
>>> 				   struct intel_shared_dpll *pll,
>>> 				   struct intel_dpll_hw_state *hw_state)  {
>>> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
>>> -
>>> -	if (IS_ELKHARTLAKE(dev_priv) &&
>>> -	    pll->info->id == DPLL_ID_EHL_DPLL4) {
>>> -		enable_reg = MG_PLL_ENABLE(0);
>>> -	}
>>> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
>>> 
>>> 	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);  }
>>> @@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct
>>> drm_i915_private *dev_priv,  static void combo_pll_enable(struct
>> drm_i915_private *dev_priv,
>>> 			     struct intel_shared_dpll *pll)  {
>>> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
>>> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
>>> 
>>> 	if (IS_ELKHARTLAKE(dev_priv) &&
>>> 	    pll->info->id == DPLL_ID_EHL_DPLL4) {
>> 
>> there's probably something else that we can do now with the
>> power_{put,get} to get rid of the, now, doubled if checks...
> 
> Don't follow you here Rodrigo.

me neither ;)
I'm just brainstorming... thinking out lout. 

> Are you suggesting using power_{put/get} to somehow get rid of doubled if?

after this patch, on this path we will do this if check twice.
not a big deal, but we can probably do something better.

However I don't understand why we had this get/put here at first place.
Only for this platform and only for this pll4. So, what I am wondering is
that we have something better to do with the power_well infrastructure
in general that would allow us to avoid the if (platform && pll4) in favor
of something more generic.

but definitely not a blocker for this patch itself.

> 
>>> -		enable_reg = MG_PLL_ENABLE(0);
>>> 
>>> 		/*
>>> 		 * We need to disable DC states when this DPLL is enabled.
>>> @@ -4157,11 +4163,10 @@ static void icl_pll_disable(struct
>>> drm_i915_private *dev_priv,  static void combo_pll_disable(struct
>> drm_i915_private *dev_priv,
>>> 			      struct intel_shared_dpll *pll)  {
>>> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
>>> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
>>> 
>>> 	if (IS_ELKHARTLAKE(dev_priv) &&
>>> 	    pll->info->id == DPLL_ID_EHL_DPLL4) {
>>> -		enable_reg = MG_PLL_ENABLE(0);
>>> 		icl_pll_disable(dev_priv, pll, enable_reg);
>> 
>> but here, at least, let's clean this function now...
>> move this call above and out of the if and delete the one below and keep
>> just the power_put inside the if...
> 
> Good change. Thanks!
> Will change that.
> 
> Anusha
> 
>>> 
>>> 		intel_display_power_put(dev_priv,
>> POWER_DOMAIN_DPLL_DC_OFF,
>>> --
>>> 2.25.0
>>> 
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
  2020-09-01 19:29 ` Rodrigo Vivi
@ 2020-09-02 19:30   ` Srivatsa, Anusha
  2020-09-02 21:31     ` Vivi, Rodrigo
  0 siblings, 1 reply; 19+ messages in thread
From: Srivatsa, Anusha @ 2020-09-02 19:30 UTC (permalink / raw)
  To: Vivi, Rodrigo; +Cc: intel-gfx



> -----Original Message-----
> From: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Sent: Tuesday, September 1, 2020 12:30 PM
> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register
> lookup
> 
> On Tue, Sep 01, 2020 at 11:27:58AM -0700, Anusha Srivatsa wrote:
> > We currenty check for platform at multiple parts in the driver to grab
> > the correct PLL. Let us begin to centralize it through a helper
> > function.
> >
> > v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg() (Ville)
> >
> > Suggested-by: Matt Roper <matthew.d.roper@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 25
> > +++++++++++--------
> >  1 file changed, 15 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index c9013f8f766f..7440836c5e44 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -147,6 +147,18 @@ void assert_shared_dpll(struct drm_i915_private
> *dev_priv,
> >  			pll->info->name, onoff(state), onoff(cur_state));  }
> >
> > +static
> > +i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private
> *dev_priv,
> > +				    struct intel_shared_dpll *pll) {
> > +
> > +	if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id ==
> DPLL_ID_EHL_DPLL4))
> > +			return MG_PLL_ENABLE(0);
> > +
> > +	return CNL_DPLL_ENABLE(pll->info->id);
> > +
> > +
> > +}
> >  /**
> >   * intel_prepare_shared_dpll - call a dpll's prepare hook
> >   * @crtc_state: CRTC, and its state, which has a shared dpll @@
> > -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct
> drm_i915_private *dev_priv,
> >  				   struct intel_shared_dpll *pll,
> >  				   struct intel_dpll_hw_state *hw_state)  {
> > -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > -
> > -	if (IS_ELKHARTLAKE(dev_priv) &&
> > -	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> > -		enable_reg = MG_PLL_ENABLE(0);
> > -	}
> > +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> >
> >  	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);  }
> > @@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct
> > drm_i915_private *dev_priv,  static void combo_pll_enable(struct
> drm_i915_private *dev_priv,
> >  			     struct intel_shared_dpll *pll)  {
> > -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> >
> >  	if (IS_ELKHARTLAKE(dev_priv) &&
> >  	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> 
> there's probably something else that we can do now with the
> power_{put,get} to get rid of the, now, doubled if checks...

Don't follow you here Rodrigo. Are you suggesting using power_{put/get} to somehow get rid of doubled if?

> > -		enable_reg = MG_PLL_ENABLE(0);
> >
> >  		/*
> >  		 * We need to disable DC states when this DPLL is enabled.
> > @@ -4157,11 +4163,10 @@ static void icl_pll_disable(struct
> > drm_i915_private *dev_priv,  static void combo_pll_disable(struct
> drm_i915_private *dev_priv,
> >  			      struct intel_shared_dpll *pll)  {
> > -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> >
> >  	if (IS_ELKHARTLAKE(dev_priv) &&
> >  	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> > -		enable_reg = MG_PLL_ENABLE(0);
> >  		icl_pll_disable(dev_priv, pll, enable_reg);
> 
> but here, at least, let's clean this function now...
> move this call above and out of the if and delete the one below and keep
> just the power_put inside the if...

Good change. Thanks!
Will change that.

Anusha
 
> >
> >  		intel_display_power_put(dev_priv,
> POWER_DOMAIN_DPLL_DC_OFF,
> > --
> > 2.25.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
  2020-09-01 18:27 Anusha Srivatsa
@ 2020-09-01 19:29 ` Rodrigo Vivi
  2020-09-02 19:30   ` Srivatsa, Anusha
  0 siblings, 1 reply; 19+ messages in thread
From: Rodrigo Vivi @ 2020-09-01 19:29 UTC (permalink / raw)
  To: Anusha Srivatsa; +Cc: intel-gfx

On Tue, Sep 01, 2020 at 11:27:58AM -0700, Anusha Srivatsa wrote:
> We currenty check for platform at multiple parts in the driver
> to grab the correct PLL. Let us begin to centralize it through a
> helper function.
> 
> v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg() (Ville)
> 
> Suggested-by: Matt Roper <matthew.d.roper@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 25 +++++++++++--------
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index c9013f8f766f..7440836c5e44 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -147,6 +147,18 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
>  			pll->info->name, onoff(state), onoff(cur_state));
>  }
>  
> +static
> +i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private *dev_priv,
> +				    struct intel_shared_dpll *pll)
> +{
> +
> +	if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id == DPLL_ID_EHL_DPLL4))
> +			return MG_PLL_ENABLE(0);
> +
> +	return CNL_DPLL_ENABLE(pll->info->id);
> +
> +
> +}
>  /**
>   * intel_prepare_shared_dpll - call a dpll's prepare hook
>   * @crtc_state: CRTC, and its state, which has a shared dpll
> @@ -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  				   struct intel_shared_dpll *pll,
>  				   struct intel_dpll_hw_state *hw_state)
>  {
> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> -
> -	if (IS_ELKHARTLAKE(dev_priv) &&
> -	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> -		enable_reg = MG_PLL_ENABLE(0);
> -	}
> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
>  
>  	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
>  }
> @@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
>  static void combo_pll_enable(struct drm_i915_private *dev_priv,
>  			     struct intel_shared_dpll *pll)
>  {
> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
>  
>  	if (IS_ELKHARTLAKE(dev_priv) &&
>  	    pll->info->id == DPLL_ID_EHL_DPLL4) {

there's probably something else that we can do now with the power_{put,get}
to get rid of the, now, doubled if checks...

> -		enable_reg = MG_PLL_ENABLE(0);
>  
>  		/*
>  		 * We need to disable DC states when this DPLL is enabled.
> @@ -4157,11 +4163,10 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv,
>  static void combo_pll_disable(struct drm_i915_private *dev_priv,
>  			      struct intel_shared_dpll *pll)
>  {
> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> +	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
>  
>  	if (IS_ELKHARTLAKE(dev_priv) &&
>  	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> -		enable_reg = MG_PLL_ENABLE(0);
>  		icl_pll_disable(dev_priv, pll, enable_reg);

but here, at least, let's clean this function now...
move this call above and out of the if and delete the one below
and keep just the power_put inside the if...

>  
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_DPLL_DC_OFF,
> -- 
> 2.25.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
@ 2020-09-01 18:27 Anusha Srivatsa
  2020-09-01 19:29 ` Rodrigo Vivi
  0 siblings, 1 reply; 19+ messages in thread
From: Anusha Srivatsa @ 2020-09-01 18:27 UTC (permalink / raw)
  To: intel-gfx

We currenty check for platform at multiple parts in the driver
to grab the correct PLL. Let us begin to centralize it through a
helper function.

v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg() (Ville)

Suggested-by: Matt Roper <matthew.d.roper@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 25 +++++++++++--------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index c9013f8f766f..7440836c5e44 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -147,6 +147,18 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
 			pll->info->name, onoff(state), onoff(cur_state));
 }
 
+static
+i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private *dev_priv,
+				    struct intel_shared_dpll *pll)
+{
+
+	if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id == DPLL_ID_EHL_DPLL4))
+			return MG_PLL_ENABLE(0);
+
+	return CNL_DPLL_ENABLE(pll->info->id);
+
+
+}
 /**
  * intel_prepare_shared_dpll - call a dpll's prepare hook
  * @crtc_state: CRTC, and its state, which has a shared dpll
@@ -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv,
 				   struct intel_shared_dpll *pll,
 				   struct intel_dpll_hw_state *hw_state)
 {
-	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
-
-	if (IS_ELKHARTLAKE(dev_priv) &&
-	    pll->info->id == DPLL_ID_EHL_DPLL4) {
-		enable_reg = MG_PLL_ENABLE(0);
-	}
+	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
 
 	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
 }
@@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
 static void combo_pll_enable(struct drm_i915_private *dev_priv,
 			     struct intel_shared_dpll *pll)
 {
-	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
+	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
 
 	if (IS_ELKHARTLAKE(dev_priv) &&
 	    pll->info->id == DPLL_ID_EHL_DPLL4) {
-		enable_reg = MG_PLL_ENABLE(0);
 
 		/*
 		 * We need to disable DC states when this DPLL is enabled.
@@ -4157,11 +4163,10 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv,
 static void combo_pll_disable(struct drm_i915_private *dev_priv,
 			      struct intel_shared_dpll *pll)
 {
-	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
+	i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
 
 	if (IS_ELKHARTLAKE(dev_priv) &&
 	    pll->info->id == DPLL_ID_EHL_DPLL4) {
-		enable_reg = MG_PLL_ENABLE(0);
 		icl_pll_disable(dev_priv, pll, enable_reg);
 
 		intel_display_power_put(dev_priv, POWER_DOMAIN_DPLL_DC_OFF,
-- 
2.25.0

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
  2020-08-31 19:03   ` Srivatsa, Anusha
@ 2020-08-31 19:47     ` Ville Syrjälä
  0 siblings, 0 replies; 19+ messages in thread
From: Ville Syrjälä @ 2020-08-31 19:47 UTC (permalink / raw)
  To: Srivatsa, Anusha; +Cc: intel-gfx

On Mon, Aug 31, 2020 at 07:03:47PM +0000, Srivatsa, Anusha wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: Monday, August 31, 2020 6:42 AM
> > To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register
> > lookup
> > 
> > On Fri, Aug 28, 2020 at 02:58:32PM -0700, Anusha Srivatsa wrote:
> > > We currenty check for platform at multiple parts in the driver to grab
> > > the correct PLL. Let us begin to centralize it through a helper
> > > function.
> > >
> > > Suggested-by: Matt Roper <matthew.d.roper@intel.com>
> > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 27
> > > ++++++++++++-------
> > >  1 file changed, 17 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > index 81ab975fe4f0..388136618bb7 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > @@ -147,6 +147,20 @@ void assert_shared_dpll(struct drm_i915_private
> > *dev_priv,
> > >  			pll->info->name, onoff(state), onoff(cur_state));  }
> > >
> > > +static
> > > +i915_reg_t intel_get_pll_enable_reg(struct drm_i915_private *dev_priv,
> > > +				    struct intel_shared_dpll *pll)
> > 
> > combo_pll_enable_reg() ?
> Actually want to avoid mentioning combo in the name. We might have platforms that do not have combo phys. We still want this function to be one place where platforms gets the PLL_ENABLE register.

There's no point in mixing up different PHY types in a single function.

>  
> > 
> > > +{
> > > +
> > > +	if (IS_ELKHARTLAKE(dev_priv)) {
> > > +		if (pll->info->id == DPLL_ID_EHL_DPLL4)
> > > +			return MG_PLL_ENABLE(0);
> > > +	}
> > 
> > Ugly nested if.
>  Will change it.
> 
> Anusha 
> > > +
> > > +	return CNL_DPLL_ENABLE(pll->info->id);
> > > +
> > > +
> > > +}
> > >  /**
> > >   * intel_prepare_shared_dpll - call a dpll's prepare hook
> > >   * @crtc_state: CRTC, and its state, which has a shared dpll @@
> > > -3842,12 +3856,7 @@ static bool combo_pll_get_hw_state(struct
> > drm_i915_private *dev_priv,
> > >  				   struct intel_shared_dpll *pll,
> > >  				   struct intel_dpll_hw_state *hw_state)  {
> > > -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > > -
> > > -	if (IS_ELKHARTLAKE(dev_priv) &&
> > > -	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> > > -		enable_reg = MG_PLL_ENABLE(0);
> > > -	}
> > > +	i915_reg_t enable_reg = intel_get_pll_enable_reg(dev_priv, pll);
> > >
> > >  	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);  }
> > > @@ -4045,11 +4054,10 @@ static void icl_pll_enable(struct
> > > drm_i915_private *dev_priv,  static void combo_pll_enable(struct
> > drm_i915_private *dev_priv,
> > >  			     struct intel_shared_dpll *pll)  {
> > > -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > > +	i915_reg_t enable_reg = intel_get_pll_enable_reg(dev_priv, pll);
> > >
> > >  	if (IS_ELKHARTLAKE(dev_priv) &&
> > >  	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> > > -		enable_reg = MG_PLL_ENABLE(0);
> > >
> > >  		/*
> > >  		 * We need to disable DC states when this DPLL is enabled.
> > > @@ -4157,11 +4165,10 @@ static void icl_pll_disable(struct
> > > drm_i915_private *dev_priv,  static void combo_pll_disable(struct
> > drm_i915_private *dev_priv,
> > >  			      struct intel_shared_dpll *pll)  {
> > > -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > > +	i915_reg_t enable_reg = intel_get_pll_enable_reg(dev_priv, pll);
> > >
> > >  	if (IS_ELKHARTLAKE(dev_priv) &&
> > >  	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> > > -		enable_reg = MG_PLL_ENABLE(0);
> > >  		icl_pll_disable(dev_priv, pll, enable_reg);
> > >
> > >  		intel_display_power_put(dev_priv,
> > POWER_DOMAIN_DPLL_DC_OFF,
> > > --
> > > 2.25.0
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > --
> > Ville Syrjälä
> > Intel

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
  2020-08-31 13:42 ` Ville Syrjälä
@ 2020-08-31 19:03   ` Srivatsa, Anusha
  2020-08-31 19:47     ` Ville Syrjälä
  0 siblings, 1 reply; 19+ messages in thread
From: Srivatsa, Anusha @ 2020-08-31 19:03 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx



> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Monday, August 31, 2020 6:42 AM
> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register
> lookup
> 
> On Fri, Aug 28, 2020 at 02:58:32PM -0700, Anusha Srivatsa wrote:
> > We currenty check for platform at multiple parts in the driver to grab
> > the correct PLL. Let us begin to centralize it through a helper
> > function.
> >
> > Suggested-by: Matt Roper <matthew.d.roper@intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 27
> > ++++++++++++-------
> >  1 file changed, 17 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index 81ab975fe4f0..388136618bb7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -147,6 +147,20 @@ void assert_shared_dpll(struct drm_i915_private
> *dev_priv,
> >  			pll->info->name, onoff(state), onoff(cur_state));  }
> >
> > +static
> > +i915_reg_t intel_get_pll_enable_reg(struct drm_i915_private *dev_priv,
> > +				    struct intel_shared_dpll *pll)
> 
> combo_pll_enable_reg() ?
Actually want to avoid mentioning combo in the name. We might have platforms that do not have combo phys. We still want this function to be one place where platforms gets the PLL_ENABLE register.
 
> 
> > +{
> > +
> > +	if (IS_ELKHARTLAKE(dev_priv)) {
> > +		if (pll->info->id == DPLL_ID_EHL_DPLL4)
> > +			return MG_PLL_ENABLE(0);
> > +	}
> 
> Ugly nested if.
 Will change it.

Anusha 
> > +
> > +	return CNL_DPLL_ENABLE(pll->info->id);
> > +
> > +
> > +}
> >  /**
> >   * intel_prepare_shared_dpll - call a dpll's prepare hook
> >   * @crtc_state: CRTC, and its state, which has a shared dpll @@
> > -3842,12 +3856,7 @@ static bool combo_pll_get_hw_state(struct
> drm_i915_private *dev_priv,
> >  				   struct intel_shared_dpll *pll,
> >  				   struct intel_dpll_hw_state *hw_state)  {
> > -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > -
> > -	if (IS_ELKHARTLAKE(dev_priv) &&
> > -	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> > -		enable_reg = MG_PLL_ENABLE(0);
> > -	}
> > +	i915_reg_t enable_reg = intel_get_pll_enable_reg(dev_priv, pll);
> >
> >  	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);  }
> > @@ -4045,11 +4054,10 @@ static void icl_pll_enable(struct
> > drm_i915_private *dev_priv,  static void combo_pll_enable(struct
> drm_i915_private *dev_priv,
> >  			     struct intel_shared_dpll *pll)  {
> > -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > +	i915_reg_t enable_reg = intel_get_pll_enable_reg(dev_priv, pll);
> >
> >  	if (IS_ELKHARTLAKE(dev_priv) &&
> >  	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> > -		enable_reg = MG_PLL_ENABLE(0);
> >
> >  		/*
> >  		 * We need to disable DC states when this DPLL is enabled.
> > @@ -4157,11 +4165,10 @@ static void icl_pll_disable(struct
> > drm_i915_private *dev_priv,  static void combo_pll_disable(struct
> drm_i915_private *dev_priv,
> >  			      struct intel_shared_dpll *pll)  {
> > -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > +	i915_reg_t enable_reg = intel_get_pll_enable_reg(dev_priv, pll);
> >
> >  	if (IS_ELKHARTLAKE(dev_priv) &&
> >  	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> > -		enable_reg = MG_PLL_ENABLE(0);
> >  		icl_pll_disable(dev_priv, pll, enable_reg);
> >
> >  		intel_display_power_put(dev_priv,
> POWER_DOMAIN_DPLL_DC_OFF,
> > --
> > 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
  2020-08-28 21:58 Anusha Srivatsa
@ 2020-08-31 13:42 ` Ville Syrjälä
  2020-08-31 19:03   ` Srivatsa, Anusha
  0 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjälä @ 2020-08-31 13:42 UTC (permalink / raw)
  To: Anusha Srivatsa; +Cc: intel-gfx

On Fri, Aug 28, 2020 at 02:58:32PM -0700, Anusha Srivatsa wrote:
> We currenty check for platform at multiple parts in the driver
> to grab the correct PLL. Let us begin to centralize it through a
> helper function.
> 
> Suggested-by: Matt Roper <matthew.d.roper@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 27 ++++++++++++-------
>  1 file changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index 81ab975fe4f0..388136618bb7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -147,6 +147,20 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
>  			pll->info->name, onoff(state), onoff(cur_state));
>  }
>  
> +static
> +i915_reg_t intel_get_pll_enable_reg(struct drm_i915_private *dev_priv,
> +				    struct intel_shared_dpll *pll)

combo_pll_enable_reg() ?

> +{
> +
> +	if (IS_ELKHARTLAKE(dev_priv)) {
> +		if (pll->info->id == DPLL_ID_EHL_DPLL4)
> +			return MG_PLL_ENABLE(0);
> +	}

Ugly nested if.

> +
> +	return CNL_DPLL_ENABLE(pll->info->id);
> +
> +
> +}
>  /**
>   * intel_prepare_shared_dpll - call a dpll's prepare hook
>   * @crtc_state: CRTC, and its state, which has a shared dpll
> @@ -3842,12 +3856,7 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  				   struct intel_shared_dpll *pll,
>  				   struct intel_dpll_hw_state *hw_state)
>  {
> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> -
> -	if (IS_ELKHARTLAKE(dev_priv) &&
> -	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> -		enable_reg = MG_PLL_ENABLE(0);
> -	}
> +	i915_reg_t enable_reg = intel_get_pll_enable_reg(dev_priv, pll);
>  
>  	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
>  }
> @@ -4045,11 +4054,10 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
>  static void combo_pll_enable(struct drm_i915_private *dev_priv,
>  			     struct intel_shared_dpll *pll)
>  {
> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> +	i915_reg_t enable_reg = intel_get_pll_enable_reg(dev_priv, pll);
>  
>  	if (IS_ELKHARTLAKE(dev_priv) &&
>  	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> -		enable_reg = MG_PLL_ENABLE(0);
>  
>  		/*
>  		 * We need to disable DC states when this DPLL is enabled.
> @@ -4157,11 +4165,10 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv,
>  static void combo_pll_disable(struct drm_i915_private *dev_priv,
>  			      struct intel_shared_dpll *pll)
>  {
> -	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> +	i915_reg_t enable_reg = intel_get_pll_enable_reg(dev_priv, pll);
>  
>  	if (IS_ELKHARTLAKE(dev_priv) &&
>  	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> -		enable_reg = MG_PLL_ENABLE(0);
>  		icl_pll_disable(dev_priv, pll, enable_reg);
>  
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_DPLL_DC_OFF,
> -- 
> 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
@ 2020-08-28 21:58 Anusha Srivatsa
  2020-08-31 13:42 ` Ville Syrjälä
  0 siblings, 1 reply; 19+ messages in thread
From: Anusha Srivatsa @ 2020-08-28 21:58 UTC (permalink / raw)
  To: intel-gfx

We currenty check for platform at multiple parts in the driver
to grab the correct PLL. Let us begin to centralize it through a
helper function.

Suggested-by: Matt Roper <matthew.d.roper@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 27 ++++++++++++-------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 81ab975fe4f0..388136618bb7 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -147,6 +147,20 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
 			pll->info->name, onoff(state), onoff(cur_state));
 }
 
+static
+i915_reg_t intel_get_pll_enable_reg(struct drm_i915_private *dev_priv,
+				    struct intel_shared_dpll *pll)
+{
+
+	if (IS_ELKHARTLAKE(dev_priv)) {
+		if (pll->info->id == DPLL_ID_EHL_DPLL4)
+			return MG_PLL_ENABLE(0);
+	}
+
+	return CNL_DPLL_ENABLE(pll->info->id);
+
+
+}
 /**
  * intel_prepare_shared_dpll - call a dpll's prepare hook
  * @crtc_state: CRTC, and its state, which has a shared dpll
@@ -3842,12 +3856,7 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv,
 				   struct intel_shared_dpll *pll,
 				   struct intel_dpll_hw_state *hw_state)
 {
-	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
-
-	if (IS_ELKHARTLAKE(dev_priv) &&
-	    pll->info->id == DPLL_ID_EHL_DPLL4) {
-		enable_reg = MG_PLL_ENABLE(0);
-	}
+	i915_reg_t enable_reg = intel_get_pll_enable_reg(dev_priv, pll);
 
 	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
 }
@@ -4045,11 +4054,10 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
 static void combo_pll_enable(struct drm_i915_private *dev_priv,
 			     struct intel_shared_dpll *pll)
 {
-	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
+	i915_reg_t enable_reg = intel_get_pll_enable_reg(dev_priv, pll);
 
 	if (IS_ELKHARTLAKE(dev_priv) &&
 	    pll->info->id == DPLL_ID_EHL_DPLL4) {
-		enable_reg = MG_PLL_ENABLE(0);
 
 		/*
 		 * We need to disable DC states when this DPLL is enabled.
@@ -4157,11 +4165,10 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv,
 static void combo_pll_disable(struct drm_i915_private *dev_priv,
 			      struct intel_shared_dpll *pll)
 {
-	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
+	i915_reg_t enable_reg = intel_get_pll_enable_reg(dev_priv, pll);
 
 	if (IS_ELKHARTLAKE(dev_priv) &&
 	    pll->info->id == DPLL_ID_EHL_DPLL4) {
-		enable_reg = MG_PLL_ENABLE(0);
 		icl_pll_disable(dev_priv, pll, enable_reg);
 
 		intel_display_power_put(dev_priv, POWER_DOMAIN_DPLL_DC_OFF,
-- 
2.25.0

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2020-09-15  0:54 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-14 17:57 [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup Anusha Srivatsa
2020-09-14 18:43 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/pll: Centralize PLL_ENABLE register lookup (rev5) Patchwork
2020-09-14 19:07 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-09-15  0:54 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2020-09-10 23:48 [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup Anusha Srivatsa
2020-09-08 23:39 Anusha Srivatsa
2020-09-09 13:06 ` Vivi, Rodrigo
2020-09-10 13:30 ` Jani Nikula
2020-09-10 17:05   ` Srivatsa, Anusha
2020-09-01 18:27 Anusha Srivatsa
2020-09-01 19:29 ` Rodrigo Vivi
2020-09-02 19:30   ` Srivatsa, Anusha
2020-09-02 21:31     ` Vivi, Rodrigo
2020-09-03 17:04       ` Srivatsa, Anusha
2020-09-08 21:00         ` Vivi, Rodrigo
2020-08-28 21:58 Anusha Srivatsa
2020-08-31 13:42 ` Ville Syrjälä
2020-08-31 19:03   ` Srivatsa, Anusha
2020-08-31 19:47     ` Ville Syrjälä

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.