| Message ID | 20210903221036.34770-1-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/3] drm/i915/display: Some code improvements and code style fixes for DRRS | expand |
Looks good to me. Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> On 9/4/21 1:10 AM, José Roberto de Souza wrote: > It started as a code style fix for the lines above 100 col but it > turned out to simplifications to intel_drrs_set_state(). > Now it receives the desired refresh rate type, high or low. > > v3: > - Fixed the mode refesh rate debug message > > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/display/intel_drrs.c | 58 ++++++++--------------- > 1 file changed, 20 insertions(+), 38 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c b/drivers/gpu/drm/i915/display/intel_drrs.c > index a2b65eca14418..fa0411341a0da 100644 > --- a/drivers/gpu/drm/i915/display/intel_drrs.c > +++ b/drivers/gpu/drm/i915/display/intel_drrs.c > @@ -89,19 +89,13 @@ intel_drrs_compute_config(struct intel_dp *intel_dp, > > static void intel_drrs_set_state(struct drm_i915_private *dev_priv, > const struct intel_crtc_state *crtc_state, > - int refresh_rate) > + enum drrs_refresh_rate_type refresh_type) > { > struct intel_dp *intel_dp = dev_priv->drrs.dp; > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > - enum drrs_refresh_rate_type index = DRRS_HIGH_RR; > + struct drm_display_mode *mode; > > - if (refresh_rate <= 0) { > - drm_dbg_kms(&dev_priv->drm, > - "Refresh rate should be positive non-zero.\n"); > - return; > - } > - > - if (intel_dp == NULL) { > + if (!intel_dp) { > drm_dbg_kms(&dev_priv->drm, "DRRS not supported.\n"); > return; > } > @@ -117,15 +111,8 @@ static void intel_drrs_set_state(struct drm_i915_private *dev_priv, > return; > } > > - if (drm_mode_vrefresh(intel_dp->attached_connector->panel.downclock_mode) == > - refresh_rate) > - index = DRRS_LOW_RR; > - > - if (index == dev_priv->drrs.refresh_rate_type) { > - drm_dbg_kms(&dev_priv->drm, > - "DRRS requested for previously set RR...ignoring\n"); > + if (refresh_type == dev_priv->drrs.refresh_rate_type) > return; > - } > > if (!crtc_state->hw.active) { > drm_dbg_kms(&dev_priv->drm, > @@ -134,7 +121,7 @@ static void intel_drrs_set_state(struct drm_i915_private *dev_priv, > } > > if (DISPLAY_VER(dev_priv) >= 8 && !IS_CHERRYVIEW(dev_priv)) { > - switch (index) { > + switch (refresh_type) { > case DRRS_HIGH_RR: > intel_dp_set_m_n(crtc_state, M1_N1); > break; > @@ -151,7 +138,7 @@ static void intel_drrs_set_state(struct drm_i915_private *dev_priv, > u32 val; > > val = intel_de_read(dev_priv, reg); > - if (index > DRRS_HIGH_RR) { > + if (refresh_type == DRRS_LOW_RR) { > if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > val |= PIPECONF_EDP_RR_MODE_SWITCH_VLV; > else > @@ -165,10 +152,14 @@ static void intel_drrs_set_state(struct drm_i915_private *dev_priv, > intel_de_write(dev_priv, reg, val); > } > > - dev_priv->drrs.refresh_rate_type = index; > + dev_priv->drrs.refresh_rate_type = refresh_type; > > + if (refresh_type == DRRS_LOW_RR) > + mode = intel_dp->attached_connector->panel.downclock_mode; > + else > + mode = intel_dp->attached_connector->panel.fixed_mode; > drm_dbg_kms(&dev_priv->drm, "eDP Refresh Rate set to : %dHz\n", > - refresh_rate); > + drm_mode_vrefresh(mode)); > } > > static void > @@ -216,13 +207,7 @@ intel_drrs_disable_locked(struct intel_dp *intel_dp, > { > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > - if (dev_priv->drrs.refresh_rate_type == DRRS_LOW_RR) { > - int refresh; > - > - refresh = drm_mode_vrefresh(intel_dp->attached_connector->panel.fixed_mode); > - intel_drrs_set_state(dev_priv, crtc_state, refresh); > - } > - > + intel_drrs_set_state(dev_priv, crtc_state, DRRS_HIGH_RR); > dev_priv->drrs.dp = NULL; > } > > @@ -290,6 +275,7 @@ static void intel_drrs_downclock_work(struct work_struct *work) > struct drm_i915_private *dev_priv = > container_of(work, typeof(*dev_priv), drrs.work.work); > struct intel_dp *intel_dp; > + struct drm_crtc *crtc; > > mutex_lock(&dev_priv->drrs.mutex); > > @@ -306,12 +292,8 @@ static void intel_drrs_downclock_work(struct work_struct *work) > if (dev_priv->drrs.busy_frontbuffer_bits) > goto unlock; > > - if (dev_priv->drrs.refresh_rate_type != DRRS_LOW_RR) { > - struct drm_crtc *crtc = dp_to_dig_port(intel_dp)->base.base.crtc; > - > - intel_drrs_set_state(dev_priv, to_intel_crtc(crtc)->config, > - drm_mode_vrefresh(intel_dp->attached_connector->panel.downclock_mode)); > - } > + crtc = dp_to_dig_port(intel_dp)->base.base.crtc; > + intel_drrs_set_state(dev_priv, to_intel_crtc(crtc)->config, DRRS_LOW_RR); > > unlock: > mutex_unlock(&dev_priv->drrs.mutex); > @@ -354,9 +336,9 @@ void intel_drrs_invalidate(struct drm_i915_private *dev_priv, > dev_priv->drrs.busy_frontbuffer_bits |= frontbuffer_bits; > > /* invalidate means busy screen hence upclock */ > - if (frontbuffer_bits && dev_priv->drrs.refresh_rate_type == DRRS_LOW_RR) > + if (frontbuffer_bits) > intel_drrs_set_state(dev_priv, to_intel_crtc(crtc)->config, > - drm_mode_vrefresh(intel_dp->attached_connector->panel.fixed_mode)); > + DRRS_HIGH_RR); > > mutex_unlock(&dev_priv->drrs.mutex); > } > @@ -400,9 +382,9 @@ void intel_drrs_flush(struct drm_i915_private *dev_priv, > dev_priv->drrs.busy_frontbuffer_bits &= ~frontbuffer_bits; > > /* flush means busy screen hence upclock */ > - if (frontbuffer_bits && dev_priv->drrs.refresh_rate_type == DRRS_LOW_RR) > + if (frontbuffer_bits) > intel_drrs_set_state(dev_priv, to_intel_crtc(crtc)->config, > - drm_mode_vrefresh(intel_dp->attached_connector->panel.fixed_mode)); > + DRRS_HIGH_RR); > > /* > * flush also means no more activity hence schedule downclock, if all >
On Sat, 2021-09-04 at 00:11 +0000, Patchwork wrote:
Patch Details
Series: series starting with [v3,1/3] drm/i915/display: Some code improvements and code style fixes for DRRS (rev2)
URL: https://patchwork.freedesktop.org/series/94342/
State: failure
Details: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20958/index.html
CI Bug Log - changes from CI_DRM_10550 -> Patchwork_20958
Summary
FAILURE
Serious unknown changes coming with Patchwork_20958 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_20958, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20958/index.html
Possible new issues
Here are the unknown changes that may have been introduced in Patchwork_20958:
IGT changes
Possible regressions
* igt@kms_flip@basic-flip-vs-dpms:
* fi-rkl-11600: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20958/fi-rkl-11600/igt@kms_flip@basic-flip-vs-dpms.html> +1 similar issue
* igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c:
* fi-rkl-11600: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10550/fi-rkl-11600/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html> -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20958/fi-rkl-11600/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html>
As expected those failures are not related to this changes, CI now has passed BAT and shards.
So pushing it.
Thanks for the reviews GG.
Known issues
Here are the changes found in Patchwork_20958 that come from known issues:
IGT changes
Issues hit
* igt@amdgpu/amd_cs_nop@sync-fork-compute0:
* fi-snb-2600: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20958/fi-snb-2600/igt@amdgpu/amd_cs_nop@sync-fork-compute0.html> (fdo#109271<https://bugs.freedesktop.org/show_bug.cgi?id=109271>) +17 similar issues
* igt@fbdev@write:
* fi-rkl-11600: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10550/fi-rkl-11600/igt@fbdev@write.html> -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20958/fi-rkl-11600/igt@fbdev@write.html> (i915#2582<https://gitlab.freedesktop.org/drm/intel/issues/2582>) +4 similar issues
* igt@i915_pm_rpm@basic-rte:
* fi-rkl-11600: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10550/fi-rkl-11600/igt@i915_pm_rpm@basic-rte.html> -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20958/fi-rkl-11600/igt@i915_pm_rpm@basic-rte.html> (fdo#109308<https://bugs.freedesktop.org/show_bug.cgi?id=109308>) +1 similar issue
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
* fi-rkl-11600: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10550/fi-rkl-11600/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html> -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20958/fi-rkl-11600/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html> (fdo#111825<https://bugs.freedesktop.org/show_bug.cgi?id=111825>) +7 similar issues
* igt@kms_flip@basic-flip-vs-modeset:
* fi-rkl-11600: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20958/fi-rkl-11600/igt@kms_flip@basic-flip-vs-modeset.html> (i915#3669<https://gitlab.freedesktop.org/drm/intel/issues/3669>) +2 similar issues
* igt@kms_frontbuffer_tracking@basic:
* fi-rkl-11600: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10550/fi-rkl-11600/igt@kms_frontbuffer_tracking@basic.html> -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20958/fi-rkl-11600/igt@kms_frontbuffer_tracking@basic.html> (i915#3180<https://gitlab.freedesktop.org/drm/intel/issues/3180>)
* igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-a:
* fi-rkl-11600: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10550/fi-rkl-11600/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-a.html> -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20958/fi-rkl-11600/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-a.html> (i915#3919<https://gitlab.freedesktop.org/drm/intel/issues/3919>) +9 similar issues
* igt@prime_vgem@basic-fence-flip:
* fi-rkl-11600: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10550/fi-rkl-11600/igt@prime_vgem@basic-fence-flip.html> -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20958/fi-rkl-11600/igt@prime_vgem@basic-fence-flip.html> (i915#1845<https://gitlab.freedesktop.org/drm/intel/issues/1845>)
Possible fixes
* igt@i915_selftest@live@execlists:
* fi-icl-y: DMESG-FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10550/fi-icl-y/igt@i915_selftest@live@execlists.html> (i915#1993<https://gitlab.freedesktop.org/drm/intel/issues/1993>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20958/fi-icl-y/igt@i915_selftest@live@execlists.html>
* igt@i915_selftest@live@hangcheck:
* fi-snb-2600: INCOMPLETE<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10550/fi-snb-2600/igt@i915_selftest@live@hangcheck.html> (i915#3921<https://gitlab.freedesktop.org/drm/intel/issues/3921>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20958/fi-snb-2600/igt@i915_selftest@live@hangcheck.html>
* igt@kms_chamelium@hdmi-edid-read:
* fi-kbl-7500u: FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10550/fi-kbl-7500u/igt@kms_chamelium@hdmi-edid-read.html> (i915#3449<https://gitlab.freedesktop.org/drm/intel/issues/3449>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20958/fi-kbl-7500u/igt@kms_chamelium@hdmi-edid-read.html>
Participating hosts (45 -> 39)
Missing (6): bat-adls-5 bat-dg1-5 fi-bsw-cyan bat-adlp-4 fi-bdw-samus bat-jsl-1
Build changes
* Linux: CI_DRM_10550 -> Patchwork_20958
CI-20190529: 20190529
CI_DRM_10550: 07f6ce3dba287a2aa8a62cdd3b7d46ea0676007f @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_6197: 40888f97a6ad219f4ed48a1830d0ef3c9617d006 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_20958: 39fd08b7518ea95ce78926369cd9fcc5e1bc3c77 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
39fd08b7518e drm/i915/display: Prepare DRRS for frontbuffer rendering drop
243f8471da93 drm/i915/display: Share code between intel_drrs_flush and intel_drrs_invalidate
95963620f428 drm/i915/display: Some code improvements and code style fixes for DRRS
diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c b/drivers/gpu/drm/i915/display/intel_drrs.c index a2b65eca14418..fa0411341a0da 100644 --- a/drivers/gpu/drm/i915/display/intel_drrs.c +++ b/drivers/gpu/drm/i915/display/intel_drrs.c @@ -89,19 +89,13 @@ intel_drrs_compute_config(struct intel_dp *intel_dp, static void intel_drrs_set_state(struct drm_i915_private *dev_priv, const struct intel_crtc_state *crtc_state, - int refresh_rate) + enum drrs_refresh_rate_type refresh_type) { struct intel_dp *intel_dp = dev_priv->drrs.dp; struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); - enum drrs_refresh_rate_type index = DRRS_HIGH_RR; + struct drm_display_mode *mode; - if (refresh_rate <= 0) { - drm_dbg_kms(&dev_priv->drm, - "Refresh rate should be positive non-zero.\n"); - return; - } - - if (intel_dp == NULL) { + if (!intel_dp) { drm_dbg_kms(&dev_priv->drm, "DRRS not supported.\n"); return; } @@ -117,15 +111,8 @@ static void intel_drrs_set_state(struct drm_i915_private *dev_priv, return; } - if (drm_mode_vrefresh(intel_dp->attached_connector->panel.downclock_mode) == - refresh_rate) - index = DRRS_LOW_RR; - - if (index == dev_priv->drrs.refresh_rate_type) { - drm_dbg_kms(&dev_priv->drm, - "DRRS requested for previously set RR...ignoring\n"); + if (refresh_type == dev_priv->drrs.refresh_rate_type) return; - } if (!crtc_state->hw.active) { drm_dbg_kms(&dev_priv->drm, @@ -134,7 +121,7 @@ static void intel_drrs_set_state(struct drm_i915_private *dev_priv, } if (DISPLAY_VER(dev_priv) >= 8 && !IS_CHERRYVIEW(dev_priv)) { - switch (index) { + switch (refresh_type) { case DRRS_HIGH_RR: intel_dp_set_m_n(crtc_state, M1_N1); break; @@ -151,7 +138,7 @@ static void intel_drrs_set_state(struct drm_i915_private *dev_priv, u32 val; val = intel_de_read(dev_priv, reg); - if (index > DRRS_HIGH_RR) { + if (refresh_type == DRRS_LOW_RR) { if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) val |= PIPECONF_EDP_RR_MODE_SWITCH_VLV; else @@ -165,10 +152,14 @@ static void intel_drrs_set_state(struct drm_i915_private *dev_priv, intel_de_write(dev_priv, reg, val); } - dev_priv->drrs.refresh_rate_type = index; + dev_priv->drrs.refresh_rate_type = refresh_type; + if (refresh_type == DRRS_LOW_RR) + mode = intel_dp->attached_connector->panel.downclock_mode; + else + mode = intel_dp->attached_connector->panel.fixed_mode; drm_dbg_kms(&dev_priv->drm, "eDP Refresh Rate set to : %dHz\n", - refresh_rate); + drm_mode_vrefresh(mode)); } static void @@ -216,13 +207,7 @@ intel_drrs_disable_locked(struct intel_dp *intel_dp, { struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); - if (dev_priv->drrs.refresh_rate_type == DRRS_LOW_RR) { - int refresh; - - refresh = drm_mode_vrefresh(intel_dp->attached_connector->panel.fixed_mode); - intel_drrs_set_state(dev_priv, crtc_state, refresh); - } - + intel_drrs_set_state(dev_priv, crtc_state, DRRS_HIGH_RR); dev_priv->drrs.dp = NULL; } @@ -290,6 +275,7 @@ static void intel_drrs_downclock_work(struct work_struct *work) struct drm_i915_private *dev_priv = container_of(work, typeof(*dev_priv), drrs.work.work); struct intel_dp *intel_dp; + struct drm_crtc *crtc; mutex_lock(&dev_priv->drrs.mutex); @@ -306,12 +292,8 @@ static void intel_drrs_downclock_work(struct work_struct *work) if (dev_priv->drrs.busy_frontbuffer_bits) goto unlock; - if (dev_priv->drrs.refresh_rate_type != DRRS_LOW_RR) { - struct drm_crtc *crtc = dp_to_dig_port(intel_dp)->base.base.crtc; - - intel_drrs_set_state(dev_priv, to_intel_crtc(crtc)->config, - drm_mode_vrefresh(intel_dp->attached_connector->panel.downclock_mode)); - } + crtc = dp_to_dig_port(intel_dp)->base.base.crtc; + intel_drrs_set_state(dev_priv, to_intel_crtc(crtc)->config, DRRS_LOW_RR); unlock: mutex_unlock(&dev_priv->drrs.mutex); @@ -354,9 +336,9 @@ void intel_drrs_invalidate(struct drm_i915_private *dev_priv, dev_priv->drrs.busy_frontbuffer_bits |= frontbuffer_bits; /* invalidate means busy screen hence upclock */ - if (frontbuffer_bits && dev_priv->drrs.refresh_rate_type == DRRS_LOW_RR) + if (frontbuffer_bits) intel_drrs_set_state(dev_priv, to_intel_crtc(crtc)->config, - drm_mode_vrefresh(intel_dp->attached_connector->panel.fixed_mode)); + DRRS_HIGH_RR); mutex_unlock(&dev_priv->drrs.mutex); } @@ -400,9 +382,9 @@ void intel_drrs_flush(struct drm_i915_private *dev_priv, dev_priv->drrs.busy_frontbuffer_bits &= ~frontbuffer_bits; /* flush means busy screen hence upclock */ - if (frontbuffer_bits && dev_priv->drrs.refresh_rate_type == DRRS_LOW_RR) + if (frontbuffer_bits) intel_drrs_set_state(dev_priv, to_intel_crtc(crtc)->config, - drm_mode_vrefresh(intel_dp->attached_connector->panel.fixed_mode)); + DRRS_HIGH_RR); /* * flush also means no more activity hence schedule downclock, if all
It started as a code style fix for the lines above 100 col but it turned out to simplifications to intel_drrs_set_state(). Now it receives the desired refresh rate type, high or low. v3: - Fixed the mode refesh rate debug message Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/display/intel_drrs.c | 58 ++++++++--------------- 1 file changed, 20 insertions(+), 38 deletions(-)