| Message ID | 20221013233223.103381-1-anusha.srivatsa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/display: Do both crawl and squash when changing cdclk | expand |
From: Patchwork <patchwork@emeril.freedesktop.org>
Sent: Thursday, October 13, 2022 9:57 PM
To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: ✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915/display: Do both crawl and squash when changing cdclk
Patch Details
Series:
series starting with [1/2] drm/i915/display: Do both crawl and squash when changing cdclk
URL:
https://patchwork.freedesktop.org/series/109694/
State:
failure
Details:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/index.html
CI Bug Log - changes from CI_DRM_12242_full -> Patchwork_109694v1_full
Summary
FAILURE
Serious unknown changes coming with Patchwork_109694v1_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_109694v1_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Participating hosts (9 -> 11)
Additional (2): shard-rkl shard-dg1
Possible new issues
Here are the unknown changes that may have been introduced in Patchwork_109694v1_full:
IGT changes
Possible regressions
* igt@syncobj_wait@wait-any-snapshot:
* shard-skl: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl9/igt@syncobj_wait@wait-any-snapshot.html> -> FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl4/igt@syncobj_wait@wait-any-snapshot.html>
Failure not related to the changes introduced in this series.
Anusha
Known issues
Here are the changes found in Patchwork_109694v1_full that come from known issues:
CI changes
Issues hit
* boot:
* shard-glk: (PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk1/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk1/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk1/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk2/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk2/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk2/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk2/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk3/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk3/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk3/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk5/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk5/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk5/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk6/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk6/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk6/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk7/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk7/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk8/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk8/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk8/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk8/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk9/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk9/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk9/boot.html>) -> (PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk9/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk9/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk9/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk8/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk8/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk8/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk7/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk7/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk7/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk6/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk6/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk6/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk5/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk5/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk5/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk3/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk3/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk3/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk3/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk2/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk2/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk1/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk1/boot.html>, FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk1/boot.html>, PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk1/boot.html>) (i915#4392<https://gitlab.freedesktop.org/drm/intel/issues/4392>)
IGT changes
Issues hit
* igt@gem_ctx_exec@basic-nohangcheck:
* shard-tglb: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-tglb2/igt@gem_ctx_exec@basic-nohangcheck.html> -> FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-tglb2/igt@gem_ctx_exec@basic-nohangcheck.html> (i915#6268<https://gitlab.freedesktop.org/drm/intel/issues/6268>)
* igt@gem_ctx_isolation@preservation-s3@vcs0:
* shard-apl: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl1/igt@gem_ctx_isolation@preservation-s3@vcs0.html> -> DMESG-WARN<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-apl3/igt@gem_ctx_isolation@preservation-s3@vcs0.html> (i915#180<https://gitlab.freedesktop.org/drm/intel/issues/180>)
* igt@gem_ctx_isolation@preservation-s3@vecs0:
* shard-tglb: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-tglb2/igt@gem_ctx_isolation@preservation-s3@vecs0.html> -> DMESG-WARN<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-tglb7/igt@gem_ctx_isolation@preservation-s3@vecs0.html> (i915#2411<https://gitlab.freedesktop.org/drm/intel/issues/2411> / i915#2867<https://gitlab.freedesktop.org/drm/intel/issues/2867>) +1 similar issue
* igt@gem_eio@reset-stress:
* shard-tglb: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-tglb5/igt@gem_eio@reset-stress.html> -> FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-tglb1/igt@gem_eio@reset-stress.html> (i915#5784<https://gitlab.freedesktop.org/drm/intel/issues/5784>)
* igt@gem_exec_balancer@parallel-keep-in-fence:
* shard-iclb: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb2/igt@gem_exec_balancer@parallel-keep-in-fence.html> -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-iclb8/igt@gem_exec_balancer@parallel-keep-in-fence.html> (i915#4525<https://gitlab.freedesktop.org/drm/intel/issues/4525>) +1 similar issue
* igt@gem_exec_fair@basic-none@vcs1:
* shard-iclb: NOTRUN -> FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-iclb1/igt@gem_exec_fair@basic-none@vcs1.html> (i915#2842<https://gitlab.freedesktop.org/drm/intel/issues/2842>)
* igt@gem_exec_fair@basic-pace-share@rcs0:
* shard-apl: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl8/igt@gem_exec_fair@basic-pace-share@rcs0.html> -> FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-apl1/igt@gem_exec_fair@basic-pace-share@rcs0.html> (i915#2842<https://gitlab.freedesktop.org/drm/intel/issues/2842>)
* igt@gem_exec_fair@basic-pace@vecs0:
* shard-iclb: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb8/igt@gem_exec_fair@basic-pace@vecs0.html> -> FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-iclb3/igt@gem_exec_fair@basic-pace@vecs0.html> (i915#2842<https://gitlab.freedesktop.org/drm/intel/issues/2842>)
* igt@gem_lmem_swapping@heavy-random:
* shard-skl: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl1/igt@gem_lmem_swapping@heavy-random.html> (fdo#109271<https://bugs.freedesktop.org/show_bug.cgi?id=109271> / i915#4613<https://gitlab.freedesktop.org/drm/intel/issues/4613>) +1 similar issue
* igt@gem_userptr_blits@dmabuf-sync:
* shard-skl: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl5/igt@gem_userptr_blits@dmabuf-sync.html> (fdo#109271<https://bugs.freedesktop.org/show_bug.cgi?id=109271> / i915#3323<https://gitlab.freedesktop.org/drm/intel/issues/3323>)
* igt@i915_pipe_stress@stress-xrgb8888-untiled:
* shard-apl: NOTRUN -> FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-apl7/igt@i915_pipe_stress@stress-xrgb8888-untiled.html> (i915#7036<https://gitlab.freedesktop.org/drm/intel/issues/7036>)
* shard-skl: NOTRUN -> FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl6/igt@i915_pipe_stress@stress-xrgb8888-untiled.html> (i915#7036<https://gitlab.freedesktop.org/drm/intel/issues/7036>)
* igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_mc_ccs:
* shard-skl: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl1/igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_mc_ccs.html> (fdo#109271<https://bugs.freedesktop.org/show_bug.cgi?id=109271> / i915#3886<https://gitlab.freedesktop.org/drm/intel/issues/3886>) +2 similar issues
* igt@kms_ccs@pipe-a-random-ccs-data-y_tiled_gen12_rc_ccs_cc:
* shard-apl: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-apl7/igt@kms_ccs@pipe-a-random-ccs-data-y_tiled_gen12_rc_ccs_cc.html> (fdo#109271<https://bugs.freedesktop.org/show_bug.cgi?id=109271> / i915#3886<https://gitlab.freedesktop.org/drm/intel/issues/3886>) +2 similar issues
* igt@kms_ccs@pipe-b-bad-rotation-90-4_tiled_dg2_rc_ccs_cc:
* shard-apl: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-apl7/igt@kms_ccs@pipe-b-bad-rotation-90-4_tiled_dg2_rc_ccs_cc.html> (fdo#109271<https://bugs.freedesktop.org/show_bug.cgi?id=109271>) +52 similar issues
* igt@kms_chamelium@hdmi-hpd-storm:
* shard-apl: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-apl7/igt@kms_chamelium@hdmi-hpd-storm.html> (fdo#109271<https://bugs.freedesktop.org/show_bug.cgi?id=109271> / fdo#111827<https://bugs.freedesktop.org/show_bug.cgi?id=111827>) +3 similar issues
* igt@kms_chamelium@hdmi-hpd-with-enabled-mode:
* shard-skl: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl6/igt@kms_chamelium@hdmi-hpd-with-enabled-mode.html> (fdo#109271<https://bugs.freedesktop.org/show_bug.cgi?id=109271> / fdo#111827<https://bugs.freedesktop.org/show_bug.cgi?id=111827>) +6 similar issues
* igt@kms_color@ctm-0-25:
* shard-skl: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl1/igt@kms_color@ctm-0-25.html> (fdo#109271<https://bugs.freedesktop.org/show_bug.cgi?id=109271> / i915#3546<https://gitlab.freedesktop.org/drm/intel/issues/3546>)
* igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions:
* shard-iclb: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb6/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions.html> -> FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-iclb7/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions.html> (i915#2346<https://gitlab.freedesktop.org/drm/intel/issues/2346>)
* igt@kms_fbcon_fbt@fbc-suspend:
* shard-apl: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl7/igt@kms_fbcon_fbt@fbc-suspend.html> -> INCOMPLETE<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-apl6/igt@kms_fbcon_fbt@fbc-suspend.html> (i915#180<https://gitlab.freedesktop.org/drm/intel/issues/180> / i915#1982<https://gitlab.freedesktop.org/drm/intel/issues/1982> / i915#4939<https://gitlab.freedesktop.org/drm/intel/issues/4939>)
* igt@kms_fbcon_fbt@psr-suspend:
* shard-skl: NOTRUN -> FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl5/igt@kms_fbcon_fbt@psr-suspend.html> (i915#4767<https://gitlab.freedesktop.org/drm/intel/issues/4767>)
* igt@kms_flip@2x-flip-vs-expired-vblank@bc-hdmi-a1-hdmi-a2:
* shard-glk: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk5/igt@kms_flip@2x-flip-vs-expired-vblank@bc-hdmi-a1-hdmi-a2.html> -> FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk5/igt@kms_flip@2x-flip-vs-expired-vblank@bc-hdmi-a1-hdmi-a2.html> (i915#79<https://gitlab.freedesktop.org/drm/intel/issues/79>) +1 similar issue
* igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1:
* shard-skl: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl6/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1.html> -> FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl7/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1.html> (i915#79<https://gitlab.freedesktop.org/drm/intel/issues/79>) +1 similar issue
* igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1:
* shard-skl: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl10/igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1.html> -> FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl6/igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1.html> (i915#2122<https://gitlab.freedesktop.org/drm/intel/issues/2122>) +7 similar issues
* igt@kms_flip_scaled_crc@flip-32bpp-4tile-to-32bpp-4tiledg2rcccs-downscaling@pipe-a-valid-mode:
* shard-iclb: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-iclb8/igt@kms_flip_scaled_crc@flip-32bpp-4tile-to-32bpp-4tiledg2rcccs-downscaling@pipe-a-valid-mode.html> (i915#2587<https://gitlab.freedesktop.org/drm/intel/issues/2587> / i915#2672<https://gitlab.freedesktop.org/drm/intel/issues/2672>) +2 similar issues
* igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-downscaling@pipe-a-default-mode:
* shard-iclb: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-iclb2/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-downscaling@pipe-a-default-mode.html> (i915#3555<https://gitlab.freedesktop.org/drm/intel/issues/3555>)
* igt@kms_flip_scaled_crc@flip-64bpp-yftile-to-32bpp-yftile-upscaling@pipe-a-default-mode:
* shard-iclb: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-iclb3/igt@kms_flip_scaled_crc@flip-64bpp-yftile-to-32bpp-yftile-upscaling@pipe-a-default-mode.html> (i915#2672<https://gitlab.freedesktop.org/drm/intel/issues/2672>) +8 similar issues
* igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-32bpp-ytilegen12rcccs-upscaling@pipe-a-default-mode:
* shard-iclb: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-iclb2/igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-32bpp-ytilegen12rcccs-upscaling@pipe-a-default-mode.html> (i915#2672<https://gitlab.freedesktop.org/drm/intel/issues/2672> / i915#3555<https://gitlab.freedesktop.org/drm/intel/issues/3555>)
* igt@kms_frontbuffer_tracking@fbcpsr-suspend:
* shard-skl: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl1/igt@kms_frontbuffer_tracking@fbcpsr-suspend.html> (fdo#109271<https://bugs.freedesktop.org/show_bug.cgi?id=109271>) +147 similar issues
* igt@kms_plane_alpha_blend@alpha-opaque-fb@pipe-a-dp-1:
* shard-apl: NOTRUN -> FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-apl7/igt@kms_plane_alpha_blend@alpha-opaque-fb@pipe-a-dp-1.html> (i915#4573<https://gitlab.freedesktop.org/drm/intel/issues/4573>) +2 similar issues
* igt@kms_plane_alpha_blend@alpha-opaque-fb@pipe-b-edp-1:
* shard-skl: NOTRUN -> FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl6/igt@kms_plane_alpha_blend@alpha-opaque-fb@pipe-b-edp-1.html> (i915#4573<https://gitlab.freedesktop.org/drm/intel/issues/4573>) +2 similar issues
* igt@kms_psr2_su@page_flip-xrgb8888:
* shard-skl: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl5/igt@kms_psr2_su@page_flip-xrgb8888.html> (fdo#109271<https://bugs.freedesktop.org/show_bug.cgi?id=109271> / i915#658<https://gitlab.freedesktop.org/drm/intel/issues/658>) +2 similar issues
* igt@kms_psr@psr2_primary_mmap_cpu:
* shard-iclb: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html> -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-iclb1/igt@kms_psr@psr2_primary_mmap_cpu.html> (fdo#109441<https://bugs.freedesktop.org/show_bug.cgi?id=109441>) +3 similar issues
* igt@kms_writeback@writeback-invalid-parameters:
* shard-skl: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl1/igt@kms_writeback@writeback-invalid-parameters.html> (fdo#109271<https://bugs.freedesktop.org/show_bug.cgi?id=109271> / i915#2437<https://gitlab.freedesktop.org/drm/intel/issues/2437>)
* igt@perf@short-reads:
* shard-skl: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl4/igt@perf@short-reads.html> -> FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl10/igt@perf@short-reads.html> (i915#51<https://gitlab.freedesktop.org/drm/intel/issues/51>)
* igt@syncobj_wait@wait-for-submit-delayed-submit:
* shard-skl: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl6/igt@syncobj_wait@wait-for-submit-delayed-submit.html> -> DMESG-WARN<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl7/igt@syncobj_wait@wait-for-submit-delayed-submit.html> (i915#1982<https://gitlab.freedesktop.org/drm/intel/issues/1982>)
* igt@sysfs_clients@create:
* shard-apl: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-apl7/igt@sysfs_clients@create.html> (fdo#109271<https://bugs.freedesktop.org/show_bug.cgi?id=109271> / i915#2994<https://gitlab.freedesktop.org/drm/intel/issues/2994>)
* shard-skl: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl6/igt@sysfs_clients@create.html> (fdo#109271<https://bugs.freedesktop.org/show_bug.cgi?id=109271> / i915#2994<https://gitlab.freedesktop.org/drm/intel/issues/2994>)
Possible fixes
* igt@feature_discovery@psr2:
* shard-iclb: SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb3/igt@feature_discovery@psr2.html> (i915#658<https://gitlab.freedesktop.org/drm/intel/issues/658>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-iclb2/igt@feature_discovery@psr2.html>
* igt@gem_exec_balancer@parallel-bb-first:
* shard-iclb: SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb3/igt@gem_exec_balancer@parallel-bb-first.html> (i915#4525<https://gitlab.freedesktop.org/drm/intel/issues/4525>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-iclb2/igt@gem_exec_balancer@parallel-bb-first.html>
* igt@gem_exec_fair@basic-flow@rcs0:
* shard-tglb: FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-tglb7/igt@gem_exec_fair@basic-flow@rcs0.html> (i915#2842<https://gitlab.freedesktop.org/drm/intel/issues/2842>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-tglb5/igt@gem_exec_fair@basic-flow@rcs0.html>
* igt@gen9_exec_parse@allowed-all:
* shard-skl: DMESG-WARN<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl10/igt@gen9_exec_parse@allowed-all.html> (i915#5566<https://gitlab.freedesktop.org/drm/intel/issues/5566> / i915#716<https://gitlab.freedesktop.org/drm/intel/issues/716>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl6/igt@gen9_exec_parse@allowed-all.html>
* shard-apl: DMESG-WARN<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl2/igt@gen9_exec_parse@allowed-all.html> (i915#5566<https://gitlab.freedesktop.org/drm/intel/issues/5566> / i915#716<https://gitlab.freedesktop.org/drm/intel/issues/716>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-apl7/igt@gen9_exec_parse@allowed-all.html>
* igt@i915_pm_dc@dc9-dpms:
* shard-iclb: SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb3/igt@i915_pm_dc@dc9-dpms.html> (i915#4281<https://gitlab.freedesktop.org/drm/intel/issues/4281>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-iclb5/igt@i915_pm_dc@dc9-dpms.html>
* igt@i915_pm_sseu@full-enable:
* shard-skl: FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl5/igt@i915_pm_sseu@full-enable.html> (i915#3524<https://gitlab.freedesktop.org/drm/intel/issues/3524>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl4/igt@i915_pm_sseu@full-enable.html>
* igt@kms_cursor_legacy@flip-vs-cursor-crc-atomic:
* shard-skl: FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl6/igt@kms_cursor_legacy@flip-vs-cursor-crc-atomic.html> (i915#2346<https://gitlab.freedesktop.org/drm/intel/issues/2346>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl9/igt@kms_cursor_legacy@flip-vs-cursor-crc-atomic.html>
* igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2:
* shard-glk: FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk8/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2.html> (i915#2122<https://gitlab.freedesktop.org/drm/intel/issues/2122>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-glk3/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2.html>
* igt@kms_flip@busy-flip@c-edp1:
* shard-skl: FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl5/igt@kms_flip@busy-flip@c-edp1.html> (i915#7200<https://gitlab.freedesktop.org/drm/intel/issues/7200>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl4/igt@kms_flip@busy-flip@c-edp1.html> +1 similar issue
* igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1:
* shard-skl: FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl6/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html> (i915#79<https://gitlab.freedesktop.org/drm/intel/issues/79>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl7/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html>
* igt@kms_hdr@bpc-switch-suspend@pipe-a-dp-1:
* shard-apl: DMESG-WARN<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl3/igt@kms_hdr@bpc-switch-suspend@pipe-a-dp-1.html> (i915#180<https://gitlab.freedesktop.org/drm/intel/issues/180>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-apl7/igt@kms_hdr@bpc-switch-suspend@pipe-a-dp-1.html> +1 similar issue
* igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-5@pipe-c-edp-1:
* shard-iclb: SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb2/igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-5@pipe-c-edp-1.html> (i915#5235<https://gitlab.freedesktop.org/drm/intel/issues/5235>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-iclb8/igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-5@pipe-c-edp-1.html> +2 similar issues
* igt@kms_psr@psr2_sprite_plane_move:
* shard-iclb: SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb6/igt@kms_psr@psr2_sprite_plane_move.html> (fdo#109441<https://bugs.freedesktop.org/show_bug.cgi?id=109441>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html> +2 similar issues
Warnings
* igt@dmabuf@all@dma_fence_chain:
* shard-skl: INCOMPLETE<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl6/igt@dmabuf@all@dma_fence_chain.html> (i915#6949<https://gitlab.freedesktop.org/drm/intel/issues/6949> / i915#7165<https://gitlab.freedesktop.org/drm/intel/issues/7165>) -> INCOMPLETE<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl9/igt@dmabuf@all@dma_fence_chain.html> (i915#6949<https://gitlab.freedesktop.org/drm/intel/issues/6949> / i915#7165<https://gitlab.freedesktop.org/drm/intel/issues/7165> / i915#7192<https://gitlab.freedesktop.org/drm/intel/issues/7192>)
* igt@i915_pm_dc@dc3co-vpb-simulation:
* shard-iclb: SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb6/igt@i915_pm_dc@dc3co-vpb-simulation.html> (i915#658<https://gitlab.freedesktop.org/drm/intel/issues/658>) -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html> (i915#588<https://gitlab.freedesktop.org/drm/intel/issues/588>)
* igt@i915_selftest@mock@vma:
* shard-skl: INCOMPLETE<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl6/igt@i915_selftest@mock@vma.html> (i915#6950<https://gitlab.freedesktop.org/drm/intel/issues/6950> / i915#7065<https://gitlab.freedesktop.org/drm/intel/issues/7065>) -> INCOMPLETE<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl7/igt@i915_selftest@mock@vma.html> (i915#6950<https://gitlab.freedesktop.org/drm/intel/issues/6950> / i915#7065<https://gitlab.freedesktop.org/drm/intel/issues/7065> / i915#7192<https://gitlab.freedesktop.org/drm/intel/issues/7192>)
* igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-sf:
* shard-iclb: SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb2/igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-sf.html> (i915#2920<https://gitlab.freedesktop.org/drm/intel/issues/2920>) -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-iclb8/igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-sf.html> (i915#658<https://gitlab.freedesktop.org/drm/intel/issues/658>) +1 similar issue
* igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area:
* shard-iclb: SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb2/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area.html> (i915#2920<https://gitlab.freedesktop.org/drm/intel/issues/2920>) -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-iclb8/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area.html> (fdo#111068<https://bugs.freedesktop.org/show_bug.cgi?id=111068> / i915#658<https://gitlab.freedesktop.org/drm/intel/issues/658>)
* igt@runner@aborted:
* shard-apl: (FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl2/igt@runner@aborted.html>, FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl6/igt@runner@aborted.html>, FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl8/igt@runner@aborted.html>, FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl3/igt@runner@aborted.html>, FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl3/igt@runner@aborted.html>) (fdo#109271<https://bugs.freedesktop.org/show_bug.cgi?id=109271> / i915#3002<https://gitlab.freedesktop.org/drm/intel/issues/3002> / i915#4312<https://gitlab.freedesktop.org/drm/intel/issues/4312>) -> (FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-apl3/igt@runner@aborted.html>, FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-apl2/igt@runner@aborted.html>, FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-apl3/igt@runner@aborted.html>, FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-apl6/igt@runner@aborted.html>) (i915#180<https://gitlab.freedesktop.org/drm/intel/issues/180> / i915#3002<https://gitlab.freedesktop.org/drm/intel/issues/3002> / i915#4312<https://gitlab.freedesktop.org/drm/intel/issues/4312>)
* shard-skl: (FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl6/igt@runner@aborted.html>, FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl6/igt@runner@aborted.html>, FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl4/igt@runner@aborted.html>, FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl10/igt@runner@aborted.html>) (i915#3002<https://gitlab.freedesktop.org/drm/intel/issues/3002> / i915#4312<https://gitlab.freedesktop.org/drm/intel/issues/4312>) -> (FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl9/igt@runner@aborted.html>, FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl1/igt@runner@aborted.html>, FAIL<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109694v1/shard-skl10/igt@runner@aborted.html>) (i915#3002<https://gitlab.freedesktop.org/drm/intel/issues/3002> / i915#4312<https://gitlab.freedesktop.org/drm/intel/issues/4312> / i915#6949<https://gitlab.freedesktop.org/drm/intel/issues/6949>)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
Build changes
* Linux: CI_DRM_12242 -> Patchwork_109694v1
CI-20190529: 20190529
CI_DRM_12242: 075a81b1efd29300194bdf7877e08b6dbe3079d9 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7012: ca6f5bdd537d26692c4b1ca011b8c4f227d95703 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_109694v1: 075a81b1efd29300194bdf7877e08b6dbe3079d9 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
On Thu, Oct 13, 2022 at 04:32:22PM -0700, Anusha Srivatsa wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > For MTL, changing cdclk from between certain frequencies has > both squash and crawl. Use the current cdclk config and > the new(desired) cdclk config to construtc a mid cdclk config. > Set the cdclk twice: > - Current cdclk -> mid cdclk > - mid cdclk -> desired cdclk > > v2: Add check in intel_modeset_calc_cdclk() to avoid cdclk > change via modeset for platforms that support squash_crawl sequences(Ville) > > v3: Add checks for: > - scenario where only slow clock is used and > cdclk is actually 0 (bringing up display). > - PLLs are on before looking up the waveform. > - Squash and crawl capability checks.(Ville) > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_cdclk.c | 157 +++++++++++++++++---- > 1 file changed, 128 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c > index ad401357ab66..430b4cb0a8ab 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -1675,7 +1675,7 @@ static u32 cdclk_squash_waveform(struct drm_i915_private *dev_priv, > const struct intel_cdclk_vals *table = dev_priv->display.cdclk.table; > int i; > > - if (cdclk == dev_priv->display.cdclk.hw.bypass) > + if (cdclk == dev_priv->display.cdclk.hw.bypass || cdclk == 0) cdclk should never be zero. Why is that needed? Hmm. Ah, we do set it to zero during sanitation. But that shouldn't matter since we shouldn't call this in that case... > return 0; > > for (i = 0; table[i].refclk; i++) > @@ -1689,37 +1689,72 @@ static u32 cdclk_squash_waveform(struct drm_i915_private *dev_priv, > return 0xffff; > } > > -static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > - const struct intel_cdclk_config *cdclk_config, > - enum pipe pipe) > +static int cdclk_squash_divider(u16 waveform) > +{ > + return hweight16(waveform ?: 0xffff); > +} > + > +static bool cdclk_crawl_and_squash(struct drm_i915_private *i915, > + const struct intel_cdclk_config *old_cdclk_config, > + const struct intel_cdclk_config *new_cdclk_config, > + struct intel_cdclk_config *mid_cdclk_config) > +{ > + u16 old_waveform = cdclk_squash_waveform(i915, old_cdclk_config->cdclk); > + u16 new_waveform = cdclk_squash_waveform(i915, new_cdclk_config->cdclk); ... but here we do call it. Just moving these to be called after the vco checks should take care of that issue. > + u16 mid_waveform; > + int size = 16; > + int div = 2; > + > + /* Return if both Squash and Crawl are not present */ > + if (!HAS_CDCLK_CRAWL(i915) || !has_cdclk_squasher(i915)) > + return false; > + > + /* Return if Squash only or Crawl only is the desired action */ > + if (old_cdclk_config->vco <= 0 || new_cdclk_config->vco <= 0 || > + old_cdclk_config->vco == new_cdclk_config->vco || > + old_waveform == new_waveform) > + return false; > + > + *mid_cdclk_config = *new_cdclk_config; > + > + /* If moving to a higher cdclk(squash) the mid cdclk config > + * should have the new (squash) waveform. > + * If moving to a lower cdclk (crawl) the mid cdclk config > + * should have the new vco. > + */ > + > + if (cdclk_squash_divider(new_waveform) > cdclk_squash_divider(old_waveform)) { > + mid_cdclk_config->vco = old_cdclk_config->vco; > + mid_waveform = new_waveform; > + } else { > + mid_cdclk_config->vco = new_cdclk_config->vco; > + mid_waveform = old_waveform; > + } > + > + mid_cdclk_config->cdclk = DIV_ROUND_CLOSEST(cdclk_squash_divider(mid_waveform) * > + mid_cdclk_config->vco, size * div); > + > + /* make sure the mid clock came out sane */ > + > + drm_WARN_ON(&i915->drm, mid_cdclk_config->cdclk < > + min(old_cdclk_config->cdclk, new_cdclk_config->cdclk)); > + drm_WARN_ON(&i915->drm, mid_cdclk_config->cdclk > > + i915->display.cdclk.max_cdclk_freq); > + drm_WARN_ON(&i915->drm, cdclk_squash_waveform(i915, mid_cdclk_config->cdclk) != > + mid_waveform); > + > + return true; > +} > + > +static void _bxt_set_cdclk(struct drm_i915_private *dev_priv, > + const struct intel_cdclk_config *cdclk_config, > + enum pipe pipe) > { > int cdclk = cdclk_config->cdclk; > int vco = cdclk_config->vco; > u32 val; > u16 waveform; > int clock; > - int ret; > - > - /* Inform power controller of upcoming frequency change. */ > - if (DISPLAY_VER(dev_priv) >= 11) > - ret = skl_pcode_request(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL, > - SKL_CDCLK_PREPARE_FOR_CHANGE, > - SKL_CDCLK_READY_FOR_CHANGE, > - SKL_CDCLK_READY_FOR_CHANGE, 3); > - else > - /* > - * BSpec requires us to wait up to 150usec, but that leads to > - * timeouts; the 2ms used here is based on experiment. > - */ > - ret = snb_pcode_write_timeout(&dev_priv->uncore, > - HSW_PCODE_DE_WRITE_FREQ_REQ, > - 0x80000000, 150, 2); > - if (ret) { > - drm_err(&dev_priv->drm, > - "Failed to inform PCU about cdclk change (err %d, freq %d)\n", > - ret, cdclk); > - return; > - } > > if (HAS_CDCLK_CRAWL(dev_priv) && dev_priv->display.cdclk.hw.vco > 0 && vco > 0) { > if (dev_priv->display.cdclk.hw.vco != vco) > @@ -1772,6 +1807,44 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > > if (pipe != INVALID_PIPE) > intel_crtc_wait_for_next_vblank(intel_crtc_for_pipe(dev_priv, pipe)); > +} > + > +static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > + const struct intel_cdclk_config *cdclk_config, > + enum pipe pipe) > +{ > + struct intel_cdclk_config mid_cdclk_config; > + int cdclk = cdclk_config->cdclk; > + int ret; > + > + /* Inform power controller of upcoming frequency change. */ > + if (DISPLAY_VER(dev_priv) >= 11) > + ret = skl_pcode_request(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL, > + SKL_CDCLK_PREPARE_FOR_CHANGE, > + SKL_CDCLK_READY_FOR_CHANGE, > + SKL_CDCLK_READY_FOR_CHANGE, 3); > + else > + /* > + * BSpec requires us to wait up to 150usec, but that leads to > + * timeouts; the 2ms used here is based on experiment. > + */ > + ret = snb_pcode_write_timeout(&dev_priv->uncore, > + HSW_PCODE_DE_WRITE_FREQ_REQ, > + 0x80000000, 150, 2); > + if (ret) { > + drm_err(&dev_priv->drm, > + "Failed to inform PCU about cdclk change (err %d, freq %d)\n", > + ret, cdclk); > + return; > + } > + > + if (cdclk_crawl_and_squash(dev_priv, &dev_priv->display.cdclk.hw, > + cdclk_config, &mid_cdclk_config)) { > + _bxt_set_cdclk(dev_priv, &mid_cdclk_config, pipe); > + _bxt_set_cdclk(dev_priv, cdclk_config, pipe); > + } else { > + _bxt_set_cdclk(dev_priv, cdclk_config, pipe); > + } > > if (DISPLAY_VER(dev_priv) >= 11) { > ret = snb_pcode_write(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL, > @@ -1944,6 +2017,27 @@ void intel_cdclk_uninit_hw(struct drm_i915_private *i915) > skl_cdclk_uninit_hw(i915); > } > > +static bool intel_cdclk_can_crawl_and_squash(struct drm_i915_private *i915, > + const struct intel_cdclk_config *a, > + const struct intel_cdclk_config *b) > +{ > + u16 old_waveform; > + u16 new_waveform; > + > + if (a->vco == 0 || b->vco == 0) > + return false; > + > + if (HAS_CDCLK_CRAWL(i915) && has_cdclk_squasher(i915)) { > + old_waveform = cdclk_squash_waveform(i915, a->cdclk); > + new_waveform = cdclk_squash_waveform(i915, b->cdclk); > + } else { > + return false; > + } A bit of weird construct this. I would make it if (!has_crawl || !has_squash) return false; ... just like you had in the other function. And doing that check before the vco checks would also be more consistent with the other function. > + > + return a->vco != b->vco && > + old_waveform != new_waveform; > +} > + > static bool intel_cdclk_can_crawl(struct drm_i915_private *dev_priv, > const struct intel_cdclk_config *a, > const struct intel_cdclk_config *b) > @@ -2750,9 +2844,14 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state) > pipe = INVALID_PIPE; > } > > - if (intel_cdclk_can_squash(dev_priv, > - &old_cdclk_state->actual, > - &new_cdclk_state->actual)) { > + if (intel_cdclk_can_crawl_and_squash(dev_priv, > + &old_cdclk_state->actual, > + &new_cdclk_state->actual)) { > + drm_dbg_kms(&dev_priv->drm, > + "Can change cdclk via crawler and squasher\n"); "crawler" is a bit weird term here. Maybe we should fix up all of thse to use the terms "crawling" and "squashing" or something along those lines. I'd make that a separate patch though. > + } else if (intel_cdclk_can_squash(dev_priv, > + &old_cdclk_state->actual, > + &new_cdclk_state->actual)) { > drm_dbg_kms(&dev_priv->drm, > "Can change cdclk via squasher\n"); > } else if (intel_cdclk_can_crawl(dev_priv, > -- > 2.25.1
On 13.10.2022 16:32, Anusha Srivatsa wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > For MTL, changing cdclk from between certain frequencies has > both squash and crawl. Use the current cdclk config and > the new(desired) cdclk config to construtc a mid cdclk config. > Set the cdclk twice: > - Current cdclk -> mid cdclk > - mid cdclk -> desired cdclk > > v2: Add check in intel_modeset_calc_cdclk() to avoid cdclk > change via modeset for platforms that support squash_crawl sequences(Ville) > > v3: Add checks for: > - scenario where only slow clock is used and > cdclk is actually 0 (bringing up display). > - PLLs are on before looking up the waveform. > - Squash and crawl capability checks.(Ville) > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Please add the Bspec number. > --- > drivers/gpu/drm/i915/display/intel_cdclk.c | 157 +++++++++++++++++---- > 1 file changed, 128 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c > index ad401357ab66..430b4cb0a8ab 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -1675,7 +1675,7 @@ static u32 cdclk_squash_waveform(struct drm_i915_private *dev_priv, > const struct intel_cdclk_vals *table = dev_priv->display.cdclk.table; > int i; > > - if (cdclk == dev_priv->display.cdclk.hw.bypass) > + if (cdclk == dev_priv->display.cdclk.hw.bypass || cdclk == 0) > return 0; > > for (i = 0; table[i].refclk; i++) > @@ -1689,37 +1689,72 @@ static u32 cdclk_squash_waveform(struct drm_i915_private *dev_priv, > return 0xffff; > } > > -static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > - const struct intel_cdclk_config *cdclk_config, > - enum pipe pipe) > +static int cdclk_squash_divider(u16 waveform) > +{ > + return hweight16(waveform ?: 0xffff); > +} > + > +static bool cdclk_crawl_and_squash(struct drm_i915_private *i915, > + const struct intel_cdclk_config *old_cdclk_config, > + const struct intel_cdclk_config *new_cdclk_config, > + struct intel_cdclk_config *mid_cdclk_config) > +{ I was thinking of asking to rename this function to a more descriptive one, but then I myself was not able to come up with one. For a fresh eyes, it is difficult to make out what this function is actually doing. Can you please add a summary as a comment above this function pointing out what is mid_cdclk and whats the meaning of its return value. > + u16 old_waveform = cdclk_squash_waveform(i915, old_cdclk_config->cdclk); > + u16 new_waveform = cdclk_squash_waveform(i915, new_cdclk_config->cdclk); > + u16 mid_waveform; > + int size = 16; > + int div = 2; > + > + /* Return if both Squash and Crawl are not present */ > + if (!HAS_CDCLK_CRAWL(i915) || !has_cdclk_squasher(i915)) > + return false; Can cdclk_squasher feature availability be also made a part of device_info structure like cdclk_crawl and create a macro similar to HAS_CDCLK_CRAWL? Like Ville said it looks bit weird. Also we would avoid adding platform checks inside has_cdclk_squasher() function like it is done now in your second patch. > + > + /* Return if Squash only or Crawl only is the desired action */ > + if (old_cdclk_config->vco <= 0 || new_cdclk_config->vco <= 0 || > + old_cdclk_config->vco == new_cdclk_config->vco || > + old_waveform == new_waveform) > + return false; > + > + *mid_cdclk_config = *new_cdclk_config; > + > + /* If moving to a higher cdclk(squash) the mid cdclk config > + * should have the new (squash) waveform. > + * If moving to a lower cdclk (crawl) the mid cdclk config > + * should have the new vco. > + */ > + > + if (cdclk_squash_divider(new_waveform) > cdclk_squash_divider(old_waveform)) { > + mid_cdclk_config->vco = old_cdclk_config->vco; > + mid_waveform = new_waveform; > + } else { > + mid_cdclk_config->vco = new_cdclk_config->vco; > + mid_waveform = old_waveform; > + } > + > + mid_cdclk_config->cdclk = DIV_ROUND_CLOSEST(cdclk_squash_divider(mid_waveform) * > + mid_cdclk_config->vco, size * div); > + > + /* make sure the mid clock came out sane */ > + > + drm_WARN_ON(&i915->drm, mid_cdclk_config->cdclk < > + min(old_cdclk_config->cdclk, new_cdclk_config->cdclk)); > + drm_WARN_ON(&i915->drm, mid_cdclk_config->cdclk > > + i915->display.cdclk.max_cdclk_freq); > + drm_WARN_ON(&i915->drm, cdclk_squash_waveform(i915, mid_cdclk_config->cdclk) != > + mid_waveform); > + > + return true; > +} > + > +static void _bxt_set_cdclk(struct drm_i915_private *dev_priv, > + const struct intel_cdclk_config *cdclk_config, > + enum pipe pipe) > { > int cdclk = cdclk_config->cdclk; > int vco = cdclk_config->vco; > u32 val; > u16 waveform; > int clock; > - int ret; > - > - /* Inform power controller of upcoming frequency change. */ > - if (DISPLAY_VER(dev_priv) >= 11) > - ret = skl_pcode_request(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL, > - SKL_CDCLK_PREPARE_FOR_CHANGE, > - SKL_CDCLK_READY_FOR_CHANGE, > - SKL_CDCLK_READY_FOR_CHANGE, 3); > - else > - /* > - * BSpec requires us to wait up to 150usec, but that leads to > - * timeouts; the 2ms used here is based on experiment. > - */ > - ret = snb_pcode_write_timeout(&dev_priv->uncore, > - HSW_PCODE_DE_WRITE_FREQ_REQ, > - 0x80000000, 150, 2); > - if (ret) { > - drm_err(&dev_priv->drm, > - "Failed to inform PCU about cdclk change (err %d, freq %d)\n", > - ret, cdclk); > - return; > - } > > if (HAS_CDCLK_CRAWL(dev_priv) && dev_priv->display.cdclk.hw.vco > 0 && vco > 0) { > if (dev_priv->display.cdclk.hw.vco != vco) > @@ -1772,6 +1807,44 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > > if (pipe != INVALID_PIPE) > intel_crtc_wait_for_next_vblank(intel_crtc_for_pipe(dev_priv, pipe)); > +} > + > +static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > + const struct intel_cdclk_config *cdclk_config, > + enum pipe pipe) > +{ bxt_set_cdclk() is now bloated as it is reused for new platforms with new features resulting in too many condition checks. I see it is now time to switch to a new set_cdclk() function. I would prefer a new function for platforms starting from which supports squash/crawl feature and add this new crawl_and_squash feature implementation to the same function. But definitely I dislike using bxt_set_cdclk for MTL. > + struct intel_cdclk_config mid_cdclk_config; > + int cdclk = cdclk_config->cdclk; > + int ret; > + > + /* Inform power controller of upcoming frequency change. */ > + if (DISPLAY_VER(dev_priv) >= 11) > + ret = skl_pcode_request(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL, > + SKL_CDCLK_PREPARE_FOR_CHANGE, > + SKL_CDCLK_READY_FOR_CHANGE, > + SKL_CDCLK_READY_FOR_CHANGE, 3); > + else > + /* > + * BSpec requires us to wait up to 150usec, but that leads to > + * timeouts; the 2ms used here is based on experiment. > + */ > + ret = snb_pcode_write_timeout(&dev_priv->uncore, > + HSW_PCODE_DE_WRITE_FREQ_REQ, > + 0x80000000, 150, 2); > + if (ret) { > + drm_err(&dev_priv->drm, > + "Failed to inform PCU about cdclk change (err %d, freq %d)\n", > + ret, cdclk); > + return; > + } > + > + if (cdclk_crawl_and_squash(dev_priv, &dev_priv->display.cdclk.hw, > + cdclk_config, &mid_cdclk_config)) { > + _bxt_set_cdclk(dev_priv, &mid_cdclk_config, pipe); > + _bxt_set_cdclk(dev_priv, cdclk_config, pipe); > + } else { > + _bxt_set_cdclk(dev_priv, cdclk_config, pipe); > + } > > if (DISPLAY_VER(dev_priv) >= 11) { > ret = snb_pcode_write(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL, > @@ -1944,6 +2017,27 @@ void intel_cdclk_uninit_hw(struct drm_i915_private *i915) > skl_cdclk_uninit_hw(i915); > } > > +static bool intel_cdclk_can_crawl_and_squash(struct drm_i915_private *i915, > + const struct intel_cdclk_config *a, > + const struct intel_cdclk_config *b) > +{ > + u16 old_waveform; > + u16 new_waveform; > + > + if (a->vco == 0 || b->vco == 0) > + return false; > + > + if (HAS_CDCLK_CRAWL(i915) && has_cdclk_squasher(i915)) { > + old_waveform = cdclk_squash_waveform(i915, a->cdclk); > + new_waveform = cdclk_squash_waveform(i915, b->cdclk); > + } else { > + return false; > + } > + > + return a->vco != b->vco && > + old_waveform != new_waveform; > +} > + > static bool intel_cdclk_can_crawl(struct drm_i915_private *dev_priv, > const struct intel_cdclk_config *a, > const struct intel_cdclk_config *b) > @@ -2750,9 +2844,14 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state) > pipe = INVALID_PIPE; > } > > - if (intel_cdclk_can_squash(dev_priv, > - &old_cdclk_state->actual, > - &new_cdclk_state->actual)) { > + if (intel_cdclk_can_crawl_and_squash(dev_priv, > + &old_cdclk_state->actual, > + &new_cdclk_state->actual)) { > + drm_dbg_kms(&dev_priv->drm, > + "Can change cdclk via crawler and squasher\n"); > + } else if (intel_cdclk_can_squash(dev_priv, > + &old_cdclk_state->actual, > + &new_cdclk_state->actual)) { In the bxt_set_cdclk(), we perform crawl_and_squash only if neither crawl and squash alone can't accomplish cdclk change. So move the intel_cdclk_can_crawl_and_squash() check to after the checks for crawl and squash individually. This would just make sure the logs reflect how actually the cdclk is changed. Regards, Bala > drm_dbg_kms(&dev_priv->drm, > "Can change cdclk via squasher\n"); > } else if (intel_cdclk_can_crawl(dev_priv, > -- > 2.25.1 >
On Thu, Oct 20, 2022 at 08:12:04PM +0530, Balasubramani Vivekanandan wrote: > On 13.10.2022 16:32, Anusha Srivatsa wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > For MTL, changing cdclk from between certain frequencies has > > both squash and crawl. Use the current cdclk config and > > the new(desired) cdclk config to construtc a mid cdclk config. > > Set the cdclk twice: > > - Current cdclk -> mid cdclk > > - mid cdclk -> desired cdclk > > > > v2: Add check in intel_modeset_calc_cdclk() to avoid cdclk > > change via modeset for platforms that support squash_crawl sequences(Ville) > > > > v3: Add checks for: > > - scenario where only slow clock is used and > > cdclk is actually 0 (bringing up display). > > - PLLs are on before looking up the waveform. > > - Squash and crawl capability checks.(Ville) > > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Please add the Bspec number. > > > --- > > drivers/gpu/drm/i915/display/intel_cdclk.c | 157 +++++++++++++++++---- > > 1 file changed, 128 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c > > index ad401357ab66..430b4cb0a8ab 100644 > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > > @@ -1675,7 +1675,7 @@ static u32 cdclk_squash_waveform(struct drm_i915_private *dev_priv, > > const struct intel_cdclk_vals *table = dev_priv->display.cdclk.table; > > int i; > > > > - if (cdclk == dev_priv->display.cdclk.hw.bypass) > > + if (cdclk == dev_priv->display.cdclk.hw.bypass || cdclk == 0) > > return 0; > > > > for (i = 0; table[i].refclk; i++) > > @@ -1689,37 +1689,72 @@ static u32 cdclk_squash_waveform(struct drm_i915_private *dev_priv, > > return 0xffff; > > } > > > > -static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > > - const struct intel_cdclk_config *cdclk_config, > > - enum pipe pipe) > > +static int cdclk_squash_divider(u16 waveform) > > +{ > > + return hweight16(waveform ?: 0xffff); > > +} > > + > > +static bool cdclk_crawl_and_squash(struct drm_i915_private *i915, > > + const struct intel_cdclk_config *old_cdclk_config, > > + const struct intel_cdclk_config *new_cdclk_config, > > + struct intel_cdclk_config *mid_cdclk_config) > > +{ > > I was thinking of asking to rename this function to a more descriptive > one, but then I myself was not able to come up with one. > For a fresh eyes, it is difficult to make out what this function is > actually doing. Can you please add a summary as a comment above this > function pointing out what is mid_cdclk and whats the meaning of its > return value. > > > + u16 old_waveform = cdclk_squash_waveform(i915, old_cdclk_config->cdclk); > > + u16 new_waveform = cdclk_squash_waveform(i915, new_cdclk_config->cdclk); > > + u16 mid_waveform; > > + int size = 16; > > + int div = 2; > > + > > + /* Return if both Squash and Crawl are not present */ > > + if (!HAS_CDCLK_CRAWL(i915) || !has_cdclk_squasher(i915)) > > + return false; > > Can cdclk_squasher feature availability be also made a part of > device_info structure like cdclk_crawl and create a macro similar to > HAS_CDCLK_CRAWL? > Like Ville said it looks bit weird. Also we would avoid adding platform > checks inside has_cdclk_squasher() function like it is done now in your > second patch. > > > + > > + /* Return if Squash only or Crawl only is the desired action */ > > + if (old_cdclk_config->vco <= 0 || new_cdclk_config->vco <= 0 || > > + old_cdclk_config->vco == new_cdclk_config->vco || > > + old_waveform == new_waveform) > > + return false; > > + > > + *mid_cdclk_config = *new_cdclk_config; > > + > > + /* If moving to a higher cdclk(squash) the mid cdclk config > > + * should have the new (squash) waveform. > > + * If moving to a lower cdclk (crawl) the mid cdclk config > > + * should have the new vco. > > + */ > > + > > + if (cdclk_squash_divider(new_waveform) > cdclk_squash_divider(old_waveform)) { > > + mid_cdclk_config->vco = old_cdclk_config->vco; > > + mid_waveform = new_waveform; > > + } else { > > + mid_cdclk_config->vco = new_cdclk_config->vco; > > + mid_waveform = old_waveform; > > + } > > + > > + mid_cdclk_config->cdclk = DIV_ROUND_CLOSEST(cdclk_squash_divider(mid_waveform) * > > + mid_cdclk_config->vco, size * div); > > + > > + /* make sure the mid clock came out sane */ > > + > > + drm_WARN_ON(&i915->drm, mid_cdclk_config->cdclk < > > + min(old_cdclk_config->cdclk, new_cdclk_config->cdclk)); > > + drm_WARN_ON(&i915->drm, mid_cdclk_config->cdclk > > > + i915->display.cdclk.max_cdclk_freq); > > + drm_WARN_ON(&i915->drm, cdclk_squash_waveform(i915, mid_cdclk_config->cdclk) != > > + mid_waveform); > > + > > + return true; > > +} > > + > > +static void _bxt_set_cdclk(struct drm_i915_private *dev_priv, > > + const struct intel_cdclk_config *cdclk_config, > > + enum pipe pipe) > > { > > int cdclk = cdclk_config->cdclk; > > int vco = cdclk_config->vco; > > u32 val; > > u16 waveform; > > int clock; > > - int ret; > > - > > - /* Inform power controller of upcoming frequency change. */ > > - if (DISPLAY_VER(dev_priv) >= 11) > > - ret = skl_pcode_request(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL, > > - SKL_CDCLK_PREPARE_FOR_CHANGE, > > - SKL_CDCLK_READY_FOR_CHANGE, > > - SKL_CDCLK_READY_FOR_CHANGE, 3); > > - else > > - /* > > - * BSpec requires us to wait up to 150usec, but that leads to > > - * timeouts; the 2ms used here is based on experiment. > > - */ > > - ret = snb_pcode_write_timeout(&dev_priv->uncore, > > - HSW_PCODE_DE_WRITE_FREQ_REQ, > > - 0x80000000, 150, 2); > > - if (ret) { > > - drm_err(&dev_priv->drm, > > - "Failed to inform PCU about cdclk change (err %d, freq %d)\n", > > - ret, cdclk); > > - return; > > - } > > > > if (HAS_CDCLK_CRAWL(dev_priv) && dev_priv->display.cdclk.hw.vco > 0 && vco > 0) { > > if (dev_priv->display.cdclk.hw.vco != vco) > > @@ -1772,6 +1807,44 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > > > > if (pipe != INVALID_PIPE) > > intel_crtc_wait_for_next_vblank(intel_crtc_for_pipe(dev_priv, pipe)); > > +} > > + > > +static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > > + const struct intel_cdclk_config *cdclk_config, > > + enum pipe pipe) > > +{ > > bxt_set_cdclk() is now bloated as it is reused for new platforms with > new features resulting in too many condition checks. I see it is now > time to switch to a new set_cdclk() function. I would prefer a new > function for platforms starting from which supports squash/crawl feature > and add this new crawl_and_squash feature implementation to the same > function. But definitely I dislike using bxt_set_cdclk for MTL. bxt vs. icl split might make sense since there are a bunch of if-else along those lines. Beyond that it all we'd achieve is code duplication I think. Well, we might be able to avoid some code duplication if we managed to chunk the different parts of bxt_set_cdclk() into lower level subfunctions, and just cobble together higher level variants (crawl+squash,just crawl,just squash, neither). But basically all of those are just subsets of the crawl+squash version, hence the duplication. Another approach I was musing about was to add vfuncs for lower level operations (pll enable, pll disable, etc.) to get rid of the if-else stuff. But dunno if enough of the platforms would fit that model to make it sensible. > > > + struct intel_cdclk_config mid_cdclk_config; > > + int cdclk = cdclk_config->cdclk; > > + int ret; > > + > > + /* Inform power controller of upcoming frequency change. */ > > + if (DISPLAY_VER(dev_priv) >= 11) > > + ret = skl_pcode_request(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL, > > + SKL_CDCLK_PREPARE_FOR_CHANGE, > > + SKL_CDCLK_READY_FOR_CHANGE, > > + SKL_CDCLK_READY_FOR_CHANGE, 3); > > + else > > + /* > > + * BSpec requires us to wait up to 150usec, but that leads to > > + * timeouts; the 2ms used here is based on experiment. > > + */ > > + ret = snb_pcode_write_timeout(&dev_priv->uncore, > > + HSW_PCODE_DE_WRITE_FREQ_REQ, > > + 0x80000000, 150, 2); > > + if (ret) { > > + drm_err(&dev_priv->drm, > > + "Failed to inform PCU about cdclk change (err %d, freq %d)\n", > > + ret, cdclk); > > + return; > > + } > > + > > + if (cdclk_crawl_and_squash(dev_priv, &dev_priv->display.cdclk.hw, > > + cdclk_config, &mid_cdclk_config)) { > > + _bxt_set_cdclk(dev_priv, &mid_cdclk_config, pipe); > > + _bxt_set_cdclk(dev_priv, cdclk_config, pipe); > > + } else { > > + _bxt_set_cdclk(dev_priv, cdclk_config, pipe); > > + } > > > > if (DISPLAY_VER(dev_priv) >= 11) { > > ret = snb_pcode_write(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL, > > @@ -1944,6 +2017,27 @@ void intel_cdclk_uninit_hw(struct drm_i915_private *i915) > > skl_cdclk_uninit_hw(i915); > > } > > > > +static bool intel_cdclk_can_crawl_and_squash(struct drm_i915_private *i915, > > + const struct intel_cdclk_config *a, > > + const struct intel_cdclk_config *b) > > +{ > > + u16 old_waveform; > > + u16 new_waveform; > > + > > + if (a->vco == 0 || b->vco == 0) > > + return false; > > + > > + if (HAS_CDCLK_CRAWL(i915) && has_cdclk_squasher(i915)) { > > + old_waveform = cdclk_squash_waveform(i915, a->cdclk); > > + new_waveform = cdclk_squash_waveform(i915, b->cdclk); > > + } else { > > + return false; > > + } > > + > > + return a->vco != b->vco && > > + old_waveform != new_waveform; > > +} > > + > > static bool intel_cdclk_can_crawl(struct drm_i915_private *dev_priv, > > const struct intel_cdclk_config *a, > > const struct intel_cdclk_config *b) > > @@ -2750,9 +2844,14 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state) > > pipe = INVALID_PIPE; > > } > > > > - if (intel_cdclk_can_squash(dev_priv, > > - &old_cdclk_state->actual, > > - &new_cdclk_state->actual)) { > > + if (intel_cdclk_can_crawl_and_squash(dev_priv, > > + &old_cdclk_state->actual, > > + &new_cdclk_state->actual)) { > > + drm_dbg_kms(&dev_priv->drm, > > + "Can change cdclk via crawler and squasher\n"); > > + } else if (intel_cdclk_can_squash(dev_priv, > > + &old_cdclk_state->actual, > > + &new_cdclk_state->actual)) { > > In the bxt_set_cdclk(), we perform crawl_and_squash only if neither crawl > and squash alone can't accomplish cdclk change. So move the > intel_cdclk_can_crawl_and_squash() check to after the checks for crawl > and squash individually. > This would just make sure the logs reflect how actually the cdclk is > changed. The current order seems fine to me. intel_cdclk_can_crawl_and_squash() shouldn't say yes unless both crawl and squash are needed. > > Regards, > Bala > > > drm_dbg_kms(&dev_priv->drm, > > "Can change cdclk via squasher\n"); > > } else if (intel_cdclk_can_crawl(dev_priv, > > -- > > 2.25.1 > >
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Thursday, October 20, 2022 4:33 AM > To: Srivatsa, Anusha <anusha.srivatsa@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [PATCH 1/2] drm/i915/display: Do both crawl and squash when > changing cdclk > > On Thu, Oct 13, 2022 at 04:32:22PM -0700, Anusha Srivatsa wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > For MTL, changing cdclk from between certain frequencies has both > > squash and crawl. Use the current cdclk config and the new(desired) > > cdclk config to construtc a mid cdclk config. > > Set the cdclk twice: > > - Current cdclk -> mid cdclk > > - mid cdclk -> desired cdclk > > > > v2: Add check in intel_modeset_calc_cdclk() to avoid cdclk change via > > modeset for platforms that support squash_crawl sequences(Ville) > > > > v3: Add checks for: > > - scenario where only slow clock is used and cdclk is actually 0 > > (bringing up display). > > - PLLs are on before looking up the waveform. > > - Squash and crawl capability checks.(Ville) > > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_cdclk.c | 157 > > +++++++++++++++++---- > > 1 file changed, 128 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > > b/drivers/gpu/drm/i915/display/intel_cdclk.c > > index ad401357ab66..430b4cb0a8ab 100644 > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > > @@ -1675,7 +1675,7 @@ static u32 cdclk_squash_waveform(struct > drm_i915_private *dev_priv, > > const struct intel_cdclk_vals *table = dev_priv->display.cdclk.table; > > int i; > > > > - if (cdclk == dev_priv->display.cdclk.hw.bypass) > > + if (cdclk == dev_priv->display.cdclk.hw.bypass || cdclk == 0) > > cdclk should never be zero. Why is that needed? Hmm. Ah, we do set it to > zero during sanitation. But that shouldn't matter since we shouldn't call this > in that case... > > > return 0; > > > > for (i = 0; table[i].refclk; i++) > > @@ -1689,37 +1689,72 @@ static u32 cdclk_squash_waveform(struct > drm_i915_private *dev_priv, > > return 0xffff; > > } > > > > -static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > > - const struct intel_cdclk_config *cdclk_config, > > - enum pipe pipe) > > +static int cdclk_squash_divider(u16 waveform) { > > + return hweight16(waveform ?: 0xffff); } > > + > > +static bool cdclk_crawl_and_squash(struct drm_i915_private *i915, > > + const struct intel_cdclk_config > *old_cdclk_config, > > + const struct intel_cdclk_config > *new_cdclk_config, > > + struct intel_cdclk_config *mid_cdclk_config) > { > > + u16 old_waveform = cdclk_squash_waveform(i915, old_cdclk_config- > >cdclk); > > + u16 new_waveform = cdclk_squash_waveform(i915, > > +new_cdclk_config->cdclk); > > ... but here we do call it. Just moving these to be called after the vco checks > should take care of that issue. Ok. Makes sense. > > + u16 mid_waveform; > > + int size = 16; > > + int div = 2; > > + > > + /* Return if both Squash and Crawl are not present */ > > + if (!HAS_CDCLK_CRAWL(i915) || !has_cdclk_squasher(i915)) > > + return false; > > + > > + /* Return if Squash only or Crawl only is the desired action */ > > + if (old_cdclk_config->vco <= 0 || new_cdclk_config->vco <= 0 || > > + old_cdclk_config->vco == new_cdclk_config->vco || > > + old_waveform == new_waveform) > > + return false; > > + > > + *mid_cdclk_config = *new_cdclk_config; > > + > > + /* If moving to a higher cdclk(squash) the mid cdclk config > > + * should have the new (squash) waveform. > > + * If moving to a lower cdclk (crawl) the mid cdclk config > > + * should have the new vco. > > + */ > > + > > + if (cdclk_squash_divider(new_waveform) > > cdclk_squash_divider(old_waveform)) { > > + mid_cdclk_config->vco = old_cdclk_config->vco; > > + mid_waveform = new_waveform; > > + } else { > > + mid_cdclk_config->vco = new_cdclk_config->vco; > > + mid_waveform = old_waveform; > > + } > > + > > + mid_cdclk_config->cdclk = > DIV_ROUND_CLOSEST(cdclk_squash_divider(mid_waveform) * > > + mid_cdclk_config->vco, size > * div); > > + > > + /* make sure the mid clock came out sane */ > > + > > + drm_WARN_ON(&i915->drm, mid_cdclk_config->cdclk < > > + min(old_cdclk_config->cdclk, new_cdclk_config->cdclk)); > > + drm_WARN_ON(&i915->drm, mid_cdclk_config->cdclk > > > + i915->display.cdclk.max_cdclk_freq); > > + drm_WARN_ON(&i915->drm, cdclk_squash_waveform(i915, > mid_cdclk_config->cdclk) != > > + mid_waveform); > > + > > + return true; > > +} > > + > > +static void _bxt_set_cdclk(struct drm_i915_private *dev_priv, > > + const struct intel_cdclk_config *cdclk_config, > > + enum pipe pipe) > > { > > int cdclk = cdclk_config->cdclk; > > int vco = cdclk_config->vco; > > u32 val; > > u16 waveform; > > int clock; > > - int ret; > > - > > - /* Inform power controller of upcoming frequency change. */ > > - if (DISPLAY_VER(dev_priv) >= 11) > > - ret = skl_pcode_request(&dev_priv->uncore, > SKL_PCODE_CDCLK_CONTROL, > > - SKL_CDCLK_PREPARE_FOR_CHANGE, > > - SKL_CDCLK_READY_FOR_CHANGE, > > - SKL_CDCLK_READY_FOR_CHANGE, 3); > > - else > > - /* > > - * BSpec requires us to wait up to 150usec, but that leads to > > - * timeouts; the 2ms used here is based on experiment. > > - */ > > - ret = snb_pcode_write_timeout(&dev_priv->uncore, > > - > HSW_PCODE_DE_WRITE_FREQ_REQ, > > - 0x80000000, 150, 2); > > - if (ret) { > > - drm_err(&dev_priv->drm, > > - "Failed to inform PCU about cdclk change (err %d, > freq %d)\n", > > - ret, cdclk); > > - return; > > - } > > > > if (HAS_CDCLK_CRAWL(dev_priv) && dev_priv->display.cdclk.hw.vco > > 0 && vco > 0) { > > if (dev_priv->display.cdclk.hw.vco != vco) @@ -1772,6 > +1807,44 @@ > > static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > > > > if (pipe != INVALID_PIPE) > > > intel_crtc_wait_for_next_vblank(intel_crtc_for_pipe(dev_priv, > > pipe)); > > +} > > + > > +static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > > + const struct intel_cdclk_config *cdclk_config, > > + enum pipe pipe) > > +{ > > + struct intel_cdclk_config mid_cdclk_config; > > + int cdclk = cdclk_config->cdclk; > > + int ret; > > + > > + /* Inform power controller of upcoming frequency change. */ > > + if (DISPLAY_VER(dev_priv) >= 11) > > + ret = skl_pcode_request(&dev_priv->uncore, > SKL_PCODE_CDCLK_CONTROL, > > + SKL_CDCLK_PREPARE_FOR_CHANGE, > > + SKL_CDCLK_READY_FOR_CHANGE, > > + SKL_CDCLK_READY_FOR_CHANGE, 3); > > + else > > + /* > > + * BSpec requires us to wait up to 150usec, but that leads to > > + * timeouts; the 2ms used here is based on experiment. > > + */ > > + ret = snb_pcode_write_timeout(&dev_priv->uncore, > > + > HSW_PCODE_DE_WRITE_FREQ_REQ, > > + 0x80000000, 150, 2); > > + if (ret) { > > + drm_err(&dev_priv->drm, > > + "Failed to inform PCU about cdclk change (err %d, > freq %d)\n", > > + ret, cdclk); > > + return; > > + } > > + > > + if (cdclk_crawl_and_squash(dev_priv, &dev_priv->display.cdclk.hw, > > + cdclk_config, &mid_cdclk_config)) { > > + _bxt_set_cdclk(dev_priv, &mid_cdclk_config, pipe); > > + _bxt_set_cdclk(dev_priv, cdclk_config, pipe); > > + } else { > > + _bxt_set_cdclk(dev_priv, cdclk_config, pipe); > > + } > > > > if (DISPLAY_VER(dev_priv) >= 11) { > > ret = snb_pcode_write(&dev_priv->uncore, > SKL_PCODE_CDCLK_CONTROL, > > @@ -1944,6 +2017,27 @@ void intel_cdclk_uninit_hw(struct > drm_i915_private *i915) > > skl_cdclk_uninit_hw(i915); > > } > > > > +static bool intel_cdclk_can_crawl_and_squash(struct drm_i915_private > *i915, > > + const struct intel_cdclk_config *a, > > + const struct intel_cdclk_config *b) { > > + u16 old_waveform; > > + u16 new_waveform; > > + > > + if (a->vco == 0 || b->vco == 0) > > + return false; > > + > > + if (HAS_CDCLK_CRAWL(i915) && has_cdclk_squasher(i915)) { > > + old_waveform = cdclk_squash_waveform(i915, a->cdclk); > > + new_waveform = cdclk_squash_waveform(i915, b->cdclk); > > + } else { > > + return false; > > + } > > A bit of weird construct this. I would make it > > if (!has_crawl || !has_squash) > return false; > ... > > just like you had in the other function. > > And doing that check before the vco checks would also be more consistent > with the other function. As @Vivekanandan, Balasubramani has also pointed out I will spin another patch that adds squash feature to the device info. That way both squash and crawl can be accessed similarly - like pointed above. > > > > + > > + return a->vco != b->vco && > > + old_waveform != new_waveform; } > > + > > static bool intel_cdclk_can_crawl(struct drm_i915_private *dev_priv, > > const struct intel_cdclk_config *a, > > const struct intel_cdclk_config *b) @@ - > 2750,9 +2844,14 @@ int > > intel_modeset_calc_cdclk(struct intel_atomic_state *state) > > pipe = INVALID_PIPE; > > } > > > > - if (intel_cdclk_can_squash(dev_priv, > > - &old_cdclk_state->actual, > > - &new_cdclk_state->actual)) { > > + if (intel_cdclk_can_crawl_and_squash(dev_priv, > > + &old_cdclk_state->actual, > > + &new_cdclk_state->actual)) { > > + drm_dbg_kms(&dev_priv->drm, > > + "Can change cdclk via crawler and squasher\n"); > > "crawler" is a bit weird term here. Maybe we should fix up all of thse to use > the terms "crawling" and "squashing" or something along those lines. I'd > make that a separate patch though. Yup on it. Separate patch for just changing the terminology and another patch for adding squash to the device info. Thanks for the feedback! Anusha > > > + } else if (intel_cdclk_can_squash(dev_priv, > > + &old_cdclk_state->actual, > > + &new_cdclk_state->actual)) { > > drm_dbg_kms(&dev_priv->drm, > > "Can change cdclk via squasher\n"); > > } else if (intel_cdclk_can_crawl(dev_priv, > > -- > > 2.25.1 > > -- > Ville Syrjälä > Intel
> -----Original Message----- > From: Vivekanandan, Balasubramani > <balasubramani.vivekanandan@intel.com> > Sent: Thursday, October 20, 2022 7:42 AM > To: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel- > gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: Do both crawl and > squash when changing cdclk > > On 13.10.2022 16:32, Anusha Srivatsa wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > For MTL, changing cdclk from between certain frequencies has both > > squash and crawl. Use the current cdclk config and the new(desired) > > cdclk config to construtc a mid cdclk config. > > Set the cdclk twice: > > - Current cdclk -> mid cdclk > > - mid cdclk -> desired cdclk > > > > v2: Add check in intel_modeset_calc_cdclk() to avoid cdclk change via > > modeset for platforms that support squash_crawl sequences(Ville) > > > > v3: Add checks for: > > - scenario where only slow clock is used and cdclk is actually 0 > > (bringing up display). > > - PLLs are on before looking up the waveform. > > - Squash and crawl capability checks.(Ville) > > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Please add the Bspec number. Will do. > > > --- > > drivers/gpu/drm/i915/display/intel_cdclk.c | 157 > > +++++++++++++++++---- > > 1 file changed, 128 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > > b/drivers/gpu/drm/i915/display/intel_cdclk.c > > index ad401357ab66..430b4cb0a8ab 100644 > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > > @@ -1675,7 +1675,7 @@ static u32 cdclk_squash_waveform(struct > drm_i915_private *dev_priv, > > const struct intel_cdclk_vals *table = dev_priv->display.cdclk.table; > > int i; > > > > - if (cdclk == dev_priv->display.cdclk.hw.bypass) > > + if (cdclk == dev_priv->display.cdclk.hw.bypass || cdclk == 0) > > return 0; > > > > for (i = 0; table[i].refclk; i++) > > @@ -1689,37 +1689,72 @@ static u32 cdclk_squash_waveform(struct > drm_i915_private *dev_priv, > > return 0xffff; > > } > > > > -static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > > - const struct intel_cdclk_config *cdclk_config, > > - enum pipe pipe) > > +static int cdclk_squash_divider(u16 waveform) { > > + return hweight16(waveform ?: 0xffff); } > > + > > +static bool cdclk_crawl_and_squash(struct drm_i915_private *i915, > > + const struct intel_cdclk_config > *old_cdclk_config, > > + const struct intel_cdclk_config > *new_cdclk_config, > > + struct intel_cdclk_config *mid_cdclk_config) > { > > I was thinking of asking to rename this function to a more descriptive one, > but then I myself was not able to come up with one. > For a fresh eyes, it is difficult to make out what this function is actually doing. > Can you please add a summary as a comment above this function pointing > out what is mid_cdclk and whats the meaning of its return value. I thought the commit message was explaining the needful. But I agree if it is indeed confusing, adding comments in code would help, will churn that change. > > > + u16 old_waveform = cdclk_squash_waveform(i915, old_cdclk_config- > >cdclk); > > + u16 new_waveform = cdclk_squash_waveform(i915, > new_cdclk_config->cdclk); > > + u16 mid_waveform; > > + int size = 16; > > + int div = 2; > > + > > + /* Return if both Squash and Crawl are not present */ > > + if (!HAS_CDCLK_CRAWL(i915) || !has_cdclk_squasher(i915)) > > + return false; > > Can cdclk_squasher feature availability be also made a part of device_info > structure like cdclk_crawl and create a macro similar to HAS_CDCLK_CRAWL? > Like Ville said it looks bit weird. Also we would avoid adding platform checks > inside has_cdclk_squasher() function like it is done now in your second > patch. Yes will make that change. > > > + > > + /* Return if Squash only or Crawl only is the desired action */ > > + if (old_cdclk_config->vco <= 0 || new_cdclk_config->vco <= 0 || > > + old_cdclk_config->vco == new_cdclk_config->vco || > > + old_waveform == new_waveform) > > + return false; > > + > > + *mid_cdclk_config = *new_cdclk_config; > > + > > + /* If moving to a higher cdclk(squash) the mid cdclk config > > + * should have the new (squash) waveform. > > + * If moving to a lower cdclk (crawl) the mid cdclk config > > + * should have the new vco. > > + */ > > + > > + if (cdclk_squash_divider(new_waveform) > > cdclk_squash_divider(old_waveform)) { > > + mid_cdclk_config->vco = old_cdclk_config->vco; > > + mid_waveform = new_waveform; > > + } else { > > + mid_cdclk_config->vco = new_cdclk_config->vco; > > + mid_waveform = old_waveform; > > + } > > + > > + mid_cdclk_config->cdclk = > DIV_ROUND_CLOSEST(cdclk_squash_divider(mid_waveform) * > > + mid_cdclk_config->vco, size > * div); > > + > > + /* make sure the mid clock came out sane */ > > + > > + drm_WARN_ON(&i915->drm, mid_cdclk_config->cdclk < > > + min(old_cdclk_config->cdclk, new_cdclk_config->cdclk)); > > + drm_WARN_ON(&i915->drm, mid_cdclk_config->cdclk > > > + i915->display.cdclk.max_cdclk_freq); > > + drm_WARN_ON(&i915->drm, cdclk_squash_waveform(i915, > mid_cdclk_config->cdclk) != > > + mid_waveform); > > + > > + return true; > > +} > > + > > +static void _bxt_set_cdclk(struct drm_i915_private *dev_priv, > > + const struct intel_cdclk_config *cdclk_config, > > + enum pipe pipe) > > { > > int cdclk = cdclk_config->cdclk; > > int vco = cdclk_config->vco; > > u32 val; > > u16 waveform; > > int clock; > > - int ret; > > - > > - /* Inform power controller of upcoming frequency change. */ > > - if (DISPLAY_VER(dev_priv) >= 11) > > - ret = skl_pcode_request(&dev_priv->uncore, > SKL_PCODE_CDCLK_CONTROL, > > - SKL_CDCLK_PREPARE_FOR_CHANGE, > > - SKL_CDCLK_READY_FOR_CHANGE, > > - SKL_CDCLK_READY_FOR_CHANGE, 3); > > - else > > - /* > > - * BSpec requires us to wait up to 150usec, but that leads to > > - * timeouts; the 2ms used here is based on experiment. > > - */ > > - ret = snb_pcode_write_timeout(&dev_priv->uncore, > > - > HSW_PCODE_DE_WRITE_FREQ_REQ, > > - 0x80000000, 150, 2); > > - if (ret) { > > - drm_err(&dev_priv->drm, > > - "Failed to inform PCU about cdclk change (err %d, > freq %d)\n", > > - ret, cdclk); > > - return; > > - } > > > > if (HAS_CDCLK_CRAWL(dev_priv) && dev_priv->display.cdclk.hw.vco > > 0 && vco > 0) { > > if (dev_priv->display.cdclk.hw.vco != vco) @@ -1772,6 > +1807,44 @@ > > static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > > > > if (pipe != INVALID_PIPE) > > > intel_crtc_wait_for_next_vblank(intel_crtc_for_pipe(dev_priv, > > pipe)); > > +} > > + > > +static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > > + const struct intel_cdclk_config *cdclk_config, > > + enum pipe pipe) > > +{ > > bxt_set_cdclk() is now bloated as it is reused for new platforms with new > features resulting in too many condition checks. I see it is now time to switch > to a new set_cdclk() function. I would prefer a new function for platforms > starting from which supports squash/crawl feature and add this new > crawl_and_squash feature implementation to the same function. But > definitely I dislike using bxt_set_cdclk for MTL. > > > + struct intel_cdclk_config mid_cdclk_config; > > + int cdclk = cdclk_config->cdclk; > > + int ret; > > + > > + /* Inform power controller of upcoming frequency change. */ > > + if (DISPLAY_VER(dev_priv) >= 11) > > + ret = skl_pcode_request(&dev_priv->uncore, > SKL_PCODE_CDCLK_CONTROL, > > + SKL_CDCLK_PREPARE_FOR_CHANGE, > > + SKL_CDCLK_READY_FOR_CHANGE, > > + SKL_CDCLK_READY_FOR_CHANGE, 3); > > + else > > + /* > > + * BSpec requires us to wait up to 150usec, but that leads to > > + * timeouts; the 2ms used here is based on experiment. > > + */ > > + ret = snb_pcode_write_timeout(&dev_priv->uncore, > > + > HSW_PCODE_DE_WRITE_FREQ_REQ, > > + 0x80000000, 150, 2); > > + if (ret) { > > + drm_err(&dev_priv->drm, > > + "Failed to inform PCU about cdclk change (err %d, > freq %d)\n", > > + ret, cdclk); > > + return; > > + } > > + > > + if (cdclk_crawl_and_squash(dev_priv, &dev_priv->display.cdclk.hw, > > + cdclk_config, &mid_cdclk_config)) { > > + _bxt_set_cdclk(dev_priv, &mid_cdclk_config, pipe); > > + _bxt_set_cdclk(dev_priv, cdclk_config, pipe); > > + } else { > > + _bxt_set_cdclk(dev_priv, cdclk_config, pipe); > > + } > > > > if (DISPLAY_VER(dev_priv) >= 11) { > > ret = snb_pcode_write(&dev_priv->uncore, > SKL_PCODE_CDCLK_CONTROL, > > @@ -1944,6 +2017,27 @@ void intel_cdclk_uninit_hw(struct > drm_i915_private *i915) > > skl_cdclk_uninit_hw(i915); > > } > > > > +static bool intel_cdclk_can_crawl_and_squash(struct drm_i915_private > *i915, > > + const struct intel_cdclk_config *a, > > + const struct intel_cdclk_config *b) { > > + u16 old_waveform; > > + u16 new_waveform; > > + > > + if (a->vco == 0 || b->vco == 0) > > + return false; > > + > > + if (HAS_CDCLK_CRAWL(i915) && has_cdclk_squasher(i915)) { > > + old_waveform = cdclk_squash_waveform(i915, a->cdclk); > > + new_waveform = cdclk_squash_waveform(i915, b->cdclk); > > + } else { > > + return false; > > + } > > + > > + return a->vco != b->vco && > > + old_waveform != new_waveform; } > > + > > static bool intel_cdclk_can_crawl(struct drm_i915_private *dev_priv, > > const struct intel_cdclk_config *a, > > const struct intel_cdclk_config *b) @@ - > 2750,9 +2844,14 @@ int > > intel_modeset_calc_cdclk(struct intel_atomic_state *state) > > pipe = INVALID_PIPE; > > } > > > > - if (intel_cdclk_can_squash(dev_priv, > > - &old_cdclk_state->actual, > > - &new_cdclk_state->actual)) { > > + if (intel_cdclk_can_crawl_and_squash(dev_priv, > > + &old_cdclk_state->actual, > > + &new_cdclk_state->actual)) { > > + drm_dbg_kms(&dev_priv->drm, > > + "Can change cdclk via crawler and squasher\n"); > > + } else if (intel_cdclk_can_squash(dev_priv, > > + &old_cdclk_state->actual, > > + &new_cdclk_state->actual)) { > > In the bxt_set_cdclk(), we perform crawl_and_squash only if neither crawl > and squash alone can't accomplish cdclk change. So move the > intel_cdclk_can_crawl_and_squash() check to after the checks for crawl and > squash individually. > This would just make sure the logs reflect how actually the cdclk is changed. As @ville.syrjala@linux.intel.com mentioned , this ordering is the right one to follow. We do not want MTL to fall into can_squash() for cases where it should actually be doing crawl_and_squash() Thanks for the feedback! Anusha > Regards, > Bala > > > drm_dbg_kms(&dev_priv->drm, > > "Can change cdclk via squasher\n"); > > } else if (intel_cdclk_can_crawl(dev_priv, > > -- > > 2.25.1 > >
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Thursday, October 20, 2022 8:15 AM > To: Vivekanandan, Balasubramani > <balasubramani.vivekanandan@intel.com> > Cc: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel- > gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: Do both crawl and > squash when changing cdclk > > On Thu, Oct 20, 2022 at 08:12:04PM +0530, Balasubramani Vivekanandan > wrote: > > On 13.10.2022 16:32, Anusha Srivatsa wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > For MTL, changing cdclk from between certain frequencies has both > > > squash and crawl. Use the current cdclk config and the new(desired) > > > cdclk config to construtc a mid cdclk config. > > > Set the cdclk twice: > > > - Current cdclk -> mid cdclk > > > - mid cdclk -> desired cdclk > > > > > > v2: Add check in intel_modeset_calc_cdclk() to avoid cdclk change > > > via modeset for platforms that support squash_crawl sequences(Ville) > > > > > > v3: Add checks for: > > > - scenario where only slow clock is used and cdclk is actually 0 > > > (bringing up display). > > > - PLLs are on before looking up the waveform. > > > - Squash and crawl capability checks.(Ville) > > > > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Please add the Bspec number. > > > > > --- > > > drivers/gpu/drm/i915/display/intel_cdclk.c | 157 > > > +++++++++++++++++---- > > > 1 file changed, 128 insertions(+), 29 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > > > b/drivers/gpu/drm/i915/display/intel_cdclk.c > > > index ad401357ab66..430b4cb0a8ab 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > > > @@ -1675,7 +1675,7 @@ static u32 cdclk_squash_waveform(struct > drm_i915_private *dev_priv, > > > const struct intel_cdclk_vals *table = dev_priv->display.cdclk.table; > > > int i; > > > > > > - if (cdclk == dev_priv->display.cdclk.hw.bypass) > > > + if (cdclk == dev_priv->display.cdclk.hw.bypass || cdclk == 0) > > > return 0; > > > > > > for (i = 0; table[i].refclk; i++) > > > @@ -1689,37 +1689,72 @@ static u32 cdclk_squash_waveform(struct > drm_i915_private *dev_priv, > > > return 0xffff; > > > } > > > > > > -static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > > > - const struct intel_cdclk_config *cdclk_config, > > > - enum pipe pipe) > > > +static int cdclk_squash_divider(u16 waveform) { > > > + return hweight16(waveform ?: 0xffff); } > > > + > > > +static bool cdclk_crawl_and_squash(struct drm_i915_private *i915, > > > + const struct intel_cdclk_config > *old_cdclk_config, > > > + const struct intel_cdclk_config > *new_cdclk_config, > > > + struct intel_cdclk_config *mid_cdclk_config) > { > > > > I was thinking of asking to rename this function to a more descriptive > > one, but then I myself was not able to come up with one. > > For a fresh eyes, it is difficult to make out what this function is > > actually doing. Can you please add a summary as a comment above this > > function pointing out what is mid_cdclk and whats the meaning of its > > return value. > > > > > + u16 old_waveform = cdclk_squash_waveform(i915, old_cdclk_config- > >cdclk); > > > + u16 new_waveform = cdclk_squash_waveform(i915, > new_cdclk_config->cdclk); > > > + u16 mid_waveform; > > > + int size = 16; > > > + int div = 2; > > > + > > > + /* Return if both Squash and Crawl are not present */ > > > + if (!HAS_CDCLK_CRAWL(i915) || !has_cdclk_squasher(i915)) > > > + return false; > > > > Can cdclk_squasher feature availability be also made a part of > > device_info structure like cdclk_crawl and create a macro similar to > > HAS_CDCLK_CRAWL? > > Like Ville said it looks bit weird. Also we would avoid adding > > platform checks inside has_cdclk_squasher() function like it is done > > now in your second patch. > > > > > + > > > + /* Return if Squash only or Crawl only is the desired action */ > > > + if (old_cdclk_config->vco <= 0 || new_cdclk_config->vco <= 0 || > > > + old_cdclk_config->vco == new_cdclk_config->vco || > > > + old_waveform == new_waveform) > > > + return false; > > > + > > > + *mid_cdclk_config = *new_cdclk_config; > > > + > > > + /* If moving to a higher cdclk(squash) the mid cdclk config > > > + * should have the new (squash) waveform. > > > + * If moving to a lower cdclk (crawl) the mid cdclk config > > > + * should have the new vco. > > > + */ > > > + > > > + if (cdclk_squash_divider(new_waveform) > > cdclk_squash_divider(old_waveform)) { > > > + mid_cdclk_config->vco = old_cdclk_config->vco; > > > + mid_waveform = new_waveform; > > > + } else { > > > + mid_cdclk_config->vco = new_cdclk_config->vco; > > > + mid_waveform = old_waveform; > > > + } > > > + > > > + mid_cdclk_config->cdclk = > DIV_ROUND_CLOSEST(cdclk_squash_divider(mid_waveform) * > > > + mid_cdclk_config->vco, size > * div); > > > + > > > + /* make sure the mid clock came out sane */ > > > + > > > + drm_WARN_ON(&i915->drm, mid_cdclk_config->cdclk < > > > + min(old_cdclk_config->cdclk, new_cdclk_config->cdclk)); > > > + drm_WARN_ON(&i915->drm, mid_cdclk_config->cdclk > > > > + i915->display.cdclk.max_cdclk_freq); > > > + drm_WARN_ON(&i915->drm, cdclk_squash_waveform(i915, > mid_cdclk_config->cdclk) != > > > + mid_waveform); > > > + > > > + return true; > > > +} > > > + > > > +static void _bxt_set_cdclk(struct drm_i915_private *dev_priv, > > > + const struct intel_cdclk_config *cdclk_config, > > > + enum pipe pipe) > > > { > > > int cdclk = cdclk_config->cdclk; > > > int vco = cdclk_config->vco; > > > u32 val; > > > u16 waveform; > > > int clock; > > > - int ret; > > > - > > > - /* Inform power controller of upcoming frequency change. */ > > > - if (DISPLAY_VER(dev_priv) >= 11) > > > - ret = skl_pcode_request(&dev_priv->uncore, > SKL_PCODE_CDCLK_CONTROL, > > > - SKL_CDCLK_PREPARE_FOR_CHANGE, > > > - SKL_CDCLK_READY_FOR_CHANGE, > > > - SKL_CDCLK_READY_FOR_CHANGE, 3); > > > - else > > > - /* > > > - * BSpec requires us to wait up to 150usec, but that leads to > > > - * timeouts; the 2ms used here is based on experiment. > > > - */ > > > - ret = snb_pcode_write_timeout(&dev_priv->uncore, > > > - > HSW_PCODE_DE_WRITE_FREQ_REQ, > > > - 0x80000000, 150, 2); > > > - if (ret) { > > > - drm_err(&dev_priv->drm, > > > - "Failed to inform PCU about cdclk change (err %d, > freq %d)\n", > > > - ret, cdclk); > > > - return; > > > - } > > > > > > if (HAS_CDCLK_CRAWL(dev_priv) && dev_priv->display.cdclk.hw.vco > > 0 && vco > 0) { > > > if (dev_priv->display.cdclk.hw.vco != vco) @@ -1772,6 > +1807,44 @@ > > > static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > > > > > > if (pipe != INVALID_PIPE) > > > > intel_crtc_wait_for_next_vblank(intel_crtc_for_pipe(dev_priv, > > > pipe)); > > > +} > > > + > > > +static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > > > + const struct intel_cdclk_config *cdclk_config, > > > + enum pipe pipe) > > > +{ > > > > bxt_set_cdclk() is now bloated as it is reused for new platforms with > > new features resulting in too many condition checks. I see it is now > > time to switch to a new set_cdclk() function. I would prefer a new > > function for platforms starting from which supports squash/crawl > > feature and add this new crawl_and_squash feature implementation to > > the same function. But definitely I dislike using bxt_set_cdclk for MTL. > > bxt vs. icl split might make sense since there are a bunch of if-else along > those lines. Beyond that it all we'd achieve is code duplication I think. > > Well, we might be able to avoid some code duplication if we managed to > chunk the different parts of bxt_set_cdclk() into lower level subfunctions, > and just cobble together higher level variants (crawl+squash,just crawl,just > squash, neither). But basically all of those are just subsets of the > crawl+squash version, hence the duplication. I agree. > Another approach I was musing about was to add vfuncs for lower level > operations (pll enable, pll disable, etc.) to get rid of the if-else stuff. But > dunno if enough of the platforms would fit that model to make it sensible. I will have to explore this myself. > > > > > + struct intel_cdclk_config mid_cdclk_config; > > > + int cdclk = cdclk_config->cdclk; > > > + int ret; > > > + > > > + /* Inform power controller of upcoming frequency change. */ > > > + if (DISPLAY_VER(dev_priv) >= 11) > > > + ret = skl_pcode_request(&dev_priv->uncore, > SKL_PCODE_CDCLK_CONTROL, > > > + SKL_CDCLK_PREPARE_FOR_CHANGE, > > > + SKL_CDCLK_READY_FOR_CHANGE, > > > + SKL_CDCLK_READY_FOR_CHANGE, 3); > > > + else > > > + /* > > > + * BSpec requires us to wait up to 150usec, but that leads to > > > + * timeouts; the 2ms used here is based on experiment. > > > + */ > > > + ret = snb_pcode_write_timeout(&dev_priv->uncore, > > > + > HSW_PCODE_DE_WRITE_FREQ_REQ, > > > + 0x80000000, 150, 2); > > > + if (ret) { > > > + drm_err(&dev_priv->drm, > > > + "Failed to inform PCU about cdclk change (err %d, > freq %d)\n", > > > + ret, cdclk); > > > + return; > > > + } > > > + > > > + if (cdclk_crawl_and_squash(dev_priv, &dev_priv->display.cdclk.hw, > > > + cdclk_config, &mid_cdclk_config)) { > > > + _bxt_set_cdclk(dev_priv, &mid_cdclk_config, pipe); > > > + _bxt_set_cdclk(dev_priv, cdclk_config, pipe); > > > + } else { > > > + _bxt_set_cdclk(dev_priv, cdclk_config, pipe); > > > + } > > > > > > if (DISPLAY_VER(dev_priv) >= 11) { > > > ret = snb_pcode_write(&dev_priv->uncore, > SKL_PCODE_CDCLK_CONTROL, > > > @@ -1944,6 +2017,27 @@ void intel_cdclk_uninit_hw(struct > drm_i915_private *i915) > > > skl_cdclk_uninit_hw(i915); > > > } > > > > > > +static bool intel_cdclk_can_crawl_and_squash(struct drm_i915_private > *i915, > > > + const struct intel_cdclk_config *a, > > > + const struct intel_cdclk_config *b) { > > > + u16 old_waveform; > > > + u16 new_waveform; > > > + > > > + if (a->vco == 0 || b->vco == 0) > > > + return false; > > > + > > > + if (HAS_CDCLK_CRAWL(i915) && has_cdclk_squasher(i915)) { > > > + old_waveform = cdclk_squash_waveform(i915, a->cdclk); > > > + new_waveform = cdclk_squash_waveform(i915, b->cdclk); > > > + } else { > > > + return false; > > > + } > > > + > > > + return a->vco != b->vco && > > > + old_waveform != new_waveform; } > > > + > > > static bool intel_cdclk_can_crawl(struct drm_i915_private *dev_priv, > > > const struct intel_cdclk_config *a, > > > const struct intel_cdclk_config *b) @@ - > 2750,9 +2844,14 @@ > > > int intel_modeset_calc_cdclk(struct intel_atomic_state *state) > > > pipe = INVALID_PIPE; > > > } > > > > > > - if (intel_cdclk_can_squash(dev_priv, > > > - &old_cdclk_state->actual, > > > - &new_cdclk_state->actual)) { > > > + if (intel_cdclk_can_crawl_and_squash(dev_priv, > > > + &old_cdclk_state->actual, > > > + &new_cdclk_state->actual)) { > > > + drm_dbg_kms(&dev_priv->drm, > > > + "Can change cdclk via crawler and squasher\n"); > > > + } else if (intel_cdclk_can_squash(dev_priv, > > > + &old_cdclk_state->actual, > > > + &new_cdclk_state->actual)) { > > > > In the bxt_set_cdclk(), we perform crawl_and_squash only if neither > > crawl and squash alone can't accomplish cdclk change. So move the > > intel_cdclk_can_crawl_and_squash() check to after the checks for crawl > > and squash individually. > > This would just make sure the logs reflect how actually the cdclk is > > changed. > > The current order seems fine to me. intel_cdclk_can_crawl_and_squash() > shouldn't say yes unless both crawl and squash are needed. Agreed here. Anusha > > > > Regards, > > Bala > > > > > drm_dbg_kms(&dev_priv->drm, > > > "Can change cdclk via squasher\n"); > > > } else if (intel_cdclk_can_crawl(dev_priv, > > > -- > > > 2.25.1 > > > > > -- > Ville Syrjälä > Intel
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index ad401357ab66..430b4cb0a8ab 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -1675,7 +1675,7 @@ static u32 cdclk_squash_waveform(struct drm_i915_private *dev_priv, const struct intel_cdclk_vals *table = dev_priv->display.cdclk.table; int i; - if (cdclk == dev_priv->display.cdclk.hw.bypass) + if (cdclk == dev_priv->display.cdclk.hw.bypass || cdclk == 0) return 0; for (i = 0; table[i].refclk; i++) @@ -1689,37 +1689,72 @@ static u32 cdclk_squash_waveform(struct drm_i915_private *dev_priv, return 0xffff; } -static void bxt_set_cdclk(struct drm_i915_private *dev_priv, - const struct intel_cdclk_config *cdclk_config, - enum pipe pipe) +static int cdclk_squash_divider(u16 waveform) +{ + return hweight16(waveform ?: 0xffff); +} + +static bool cdclk_crawl_and_squash(struct drm_i915_private *i915, + const struct intel_cdclk_config *old_cdclk_config, + const struct intel_cdclk_config *new_cdclk_config, + struct intel_cdclk_config *mid_cdclk_config) +{ + u16 old_waveform = cdclk_squash_waveform(i915, old_cdclk_config->cdclk); + u16 new_waveform = cdclk_squash_waveform(i915, new_cdclk_config->cdclk); + u16 mid_waveform; + int size = 16; + int div = 2; + + /* Return if both Squash and Crawl are not present */ + if (!HAS_CDCLK_CRAWL(i915) || !has_cdclk_squasher(i915)) + return false; + + /* Return if Squash only or Crawl only is the desired action */ + if (old_cdclk_config->vco <= 0 || new_cdclk_config->vco <= 0 || + old_cdclk_config->vco == new_cdclk_config->vco || + old_waveform == new_waveform) + return false; + + *mid_cdclk_config = *new_cdclk_config; + + /* If moving to a higher cdclk(squash) the mid cdclk config + * should have the new (squash) waveform. + * If moving to a lower cdclk (crawl) the mid cdclk config + * should have the new vco. + */ + + if (cdclk_squash_divider(new_waveform) > cdclk_squash_divider(old_waveform)) { + mid_cdclk_config->vco = old_cdclk_config->vco; + mid_waveform = new_waveform; + } else { + mid_cdclk_config->vco = new_cdclk_config->vco; + mid_waveform = old_waveform; + } + + mid_cdclk_config->cdclk = DIV_ROUND_CLOSEST(cdclk_squash_divider(mid_waveform) * + mid_cdclk_config->vco, size * div); + + /* make sure the mid clock came out sane */ + + drm_WARN_ON(&i915->drm, mid_cdclk_config->cdclk < + min(old_cdclk_config->cdclk, new_cdclk_config->cdclk)); + drm_WARN_ON(&i915->drm, mid_cdclk_config->cdclk > + i915->display.cdclk.max_cdclk_freq); + drm_WARN_ON(&i915->drm, cdclk_squash_waveform(i915, mid_cdclk_config->cdclk) != + mid_waveform); + + return true; +} + +static void _bxt_set_cdclk(struct drm_i915_private *dev_priv, + const struct intel_cdclk_config *cdclk_config, + enum pipe pipe) { int cdclk = cdclk_config->cdclk; int vco = cdclk_config->vco; u32 val; u16 waveform; int clock; - int ret; - - /* Inform power controller of upcoming frequency change. */ - if (DISPLAY_VER(dev_priv) >= 11) - ret = skl_pcode_request(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL, - SKL_CDCLK_PREPARE_FOR_CHANGE, - SKL_CDCLK_READY_FOR_CHANGE, - SKL_CDCLK_READY_FOR_CHANGE, 3); - else - /* - * BSpec requires us to wait up to 150usec, but that leads to - * timeouts; the 2ms used here is based on experiment. - */ - ret = snb_pcode_write_timeout(&dev_priv->uncore, - HSW_PCODE_DE_WRITE_FREQ_REQ, - 0x80000000, 150, 2); - if (ret) { - drm_err(&dev_priv->drm, - "Failed to inform PCU about cdclk change (err %d, freq %d)\n", - ret, cdclk); - return; - } if (HAS_CDCLK_CRAWL(dev_priv) && dev_priv->display.cdclk.hw.vco > 0 && vco > 0) { if (dev_priv->display.cdclk.hw.vco != vco) @@ -1772,6 +1807,44 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv, if (pipe != INVALID_PIPE) intel_crtc_wait_for_next_vblank(intel_crtc_for_pipe(dev_priv, pipe)); +} + +static void bxt_set_cdclk(struct drm_i915_private *dev_priv, + const struct intel_cdclk_config *cdclk_config, + enum pipe pipe) +{ + struct intel_cdclk_config mid_cdclk_config; + int cdclk = cdclk_config->cdclk; + int ret; + + /* Inform power controller of upcoming frequency change. */ + if (DISPLAY_VER(dev_priv) >= 11) + ret = skl_pcode_request(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL, + SKL_CDCLK_PREPARE_FOR_CHANGE, + SKL_CDCLK_READY_FOR_CHANGE, + SKL_CDCLK_READY_FOR_CHANGE, 3); + else + /* + * BSpec requires us to wait up to 150usec, but that leads to + * timeouts; the 2ms used here is based on experiment. + */ + ret = snb_pcode_write_timeout(&dev_priv->uncore, + HSW_PCODE_DE_WRITE_FREQ_REQ, + 0x80000000, 150, 2); + if (ret) { + drm_err(&dev_priv->drm, + "Failed to inform PCU about cdclk change (err %d, freq %d)\n", + ret, cdclk); + return; + } + + if (cdclk_crawl_and_squash(dev_priv, &dev_priv->display.cdclk.hw, + cdclk_config, &mid_cdclk_config)) { + _bxt_set_cdclk(dev_priv, &mid_cdclk_config, pipe); + _bxt_set_cdclk(dev_priv, cdclk_config, pipe); + } else { + _bxt_set_cdclk(dev_priv, cdclk_config, pipe); + } if (DISPLAY_VER(dev_priv) >= 11) { ret = snb_pcode_write(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL, @@ -1944,6 +2017,27 @@ void intel_cdclk_uninit_hw(struct drm_i915_private *i915) skl_cdclk_uninit_hw(i915); } +static bool intel_cdclk_can_crawl_and_squash(struct drm_i915_private *i915, + const struct intel_cdclk_config *a, + const struct intel_cdclk_config *b) +{ + u16 old_waveform; + u16 new_waveform; + + if (a->vco == 0 || b->vco == 0) + return false; + + if (HAS_CDCLK_CRAWL(i915) && has_cdclk_squasher(i915)) { + old_waveform = cdclk_squash_waveform(i915, a->cdclk); + new_waveform = cdclk_squash_waveform(i915, b->cdclk); + } else { + return false; + } + + return a->vco != b->vco && + old_waveform != new_waveform; +} + static bool intel_cdclk_can_crawl(struct drm_i915_private *dev_priv, const struct intel_cdclk_config *a, const struct intel_cdclk_config *b) @@ -2750,9 +2844,14 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state) pipe = INVALID_PIPE; } - if (intel_cdclk_can_squash(dev_priv, - &old_cdclk_state->actual, - &new_cdclk_state->actual)) { + if (intel_cdclk_can_crawl_and_squash(dev_priv, + &old_cdclk_state->actual, + &new_cdclk_state->actual)) { + drm_dbg_kms(&dev_priv->drm, + "Can change cdclk via crawler and squasher\n"); + } else if (intel_cdclk_can_squash(dev_priv, + &old_cdclk_state->actual, + &new_cdclk_state->actual)) { drm_dbg_kms(&dev_priv->drm, "Can change cdclk via squasher\n"); } else if (intel_cdclk_can_crawl(dev_priv,