RE: [PATCH 18/22] drm/i915: Handle joined pipes inside hsw_crtc_disable()
> -Original Message- > From: Ville Syrjälä > Sent: Wednesday, April 3, 2024 1:20 AM > To: Murthy, Arun R > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [PATCH 18/22] drm/i915: Handle joined pipes inside > hsw_crtc_disable() > > On Mon, Apr 01, 2024 at 06:46:20AM +, Murthy, Arun R wrote: > > > > > -Original Message- > > > From: Intel-gfx On Behalf > > > Of Ville Syrjala > > > Sent: Friday, March 29, 2024 6:43 AM > > > To: intel-gfx@lists.freedesktop.org > > > Subject: [PATCH 18/22] drm/i915: Handle joined pipes inside > > > hsw_crtc_disable() > > > > > > From: Ville Syrjälä > > > > > > Reorganize the crtc disable path to only deal with the master > > > pipes/transcoders in intel_old_crtc_state_disables() and offload the > > > handling of joined pipes to hsw_crtc_disable(). > > > This makes the whole thing much more sensible since we can actually > > > control the order in which we do the per-pipe vs. > > > per-transcoder modeset steps. > > > > > > v2: Use the name 'pipe_crtc' for the per-pipe crtc pointer > > > > > > Signed-off-by: Ville Syrjälä > > > --- > > > drivers/gpu/drm/i915/display/intel_display.c | 64 > > > > > > 1 file changed, 39 insertions(+), 25 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > index 58ee40786d5c..c15ea046c62a 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > @@ -1791,29 +1791,28 @@ static void hsw_crtc_disable(struct > > > intel_atomic_state *state, > > > const struct intel_crtc_state *old_crtc_state = > > > intel_atomic_get_old_crtc_state(state, crtc); > > > struct drm_i915_private *i915 = to_i915(crtc->base.dev); > > > + struct intel_crtc *pipe_crtc; > > > > > > /* > > >* FIXME collapse everything to one hook. > > >* Need care with mst->ddi interactions. > > >*/ > > > - if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) { > > > - intel_encoders_disable(state, crtc); > > > - intel_encoders_post_disable(state, crtc); > > > - } > > > - > > > - intel_disable_shared_dpll(old_crtc_state); > > > + intel_encoders_disable(state, crtc); > > > + intel_encoders_post_disable(state, crtc); > > > > > > - if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) { > > > - struct intel_crtc *slave_crtc; > > > + for_each_intel_crtc_in_pipe_mask(>drm, pipe_crtc, > > > + > > > intel_crtc_joined_pipe_mask(old_crtc_state)) { > > > + const struct intel_crtc_state *old_pipe_crtc_state = > > > + intel_atomic_get_old_crtc_state(state, pipe_crtc); > > > > > > - intel_encoders_post_pll_disable(state, crtc); > > > + intel_disable_shared_dpll(old_pipe_crtc_state); > > > + } > > > > As per the sequence is considered, should the pll be disabled prior to > > disabling > the encoders and then followed by post_pll_disable? > > The correct disable order is: > 1. encoder disable() > 2. disable transcoder/etc. (nop for hsw+ as that stuff >has been sucked into the encoder hooks) 3. encoder post_disable() 4. pll > disable 5. encoder post_pll_disable() > > which we should be following here, thouh the diff is rather hard to read due > to > the indentation changes. > Thanks for the clarification. I verified in the source code. Reviewed-by: Arun R Murthy Thanks and Regards, Arun R Murthy > -- > Ville Syrjälä > Intel
[linux-next:master] BUILD REGRESSION c0b832517f627ead3388c6f0c74e8ac10ad5774b
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: c0b832517f627ead3388c6f0c74e8ac10ad5774b Add linux-next specific files for 20240402 Error/Warning reports: https://lore.kernel.org/oe-kbuild-all/202404021504.ytp51bl3-...@intel.com https://lore.kernel.org/oe-kbuild-all/202404021556.0jvcnc13-...@intel.com https://lore.kernel.org/oe-kbuild-all/202404021638.qkhbdq4e-...@intel.com https://lore.kernel.org/oe-kbuild-all/202404021700.lbyyzgfd-...@intel.com https://lore.kernel.org/oe-kbuild-all/202404022106.mywpypit-...@intel.com Error/Warning: (recently discovered and may have been fixed) ERROR: modpost: "__drm_atomic_helper_private_obj_duplicate_state" [drivers/gpu/drm/display/drm_display_helper.ko] undefined! ERROR: modpost: "drm_kms_helper_hotplug_event" [drivers/gpu/drm/display/drm_display_helper.ko] undefined! arch/csky/include/asm/cmpxchg.h:138:25: error: implicit declaration of function 'cmpxchg_emu_u8' [-Werror=implicit-function-declaration] arch/csky/include/asm/cmpxchg.h:141:25: error: implicit declaration of function 'cmpxchg_emu_u16' [-Werror=implicit-function-declaration] arch/riscv/include/asm/cmpxchg.h:329:23: warning: assignment to 'const struct gre_protocol *' from 'uintptr_t' {aka 'long unsigned int'} makes pointer from integer without a cast [-Wint-conversion] arch/riscv/include/asm/cmpxchg.h:329:23: warning: assignment to 'struct bdi_writeback *' from 'uintptr_t' {aka 'long unsigned int'} makes pointer from integer without a cast [-Wint-conversion] arch/riscv/include/asm/cmpxchg.h:329:23: warning: assignment to 'struct rtrs_clt_path *' from 'uintptr_t' {aka 'long unsigned int'} makes pointer from integer without a cast [-Wint-conversion] arch/riscv/include/asm/cmpxchg.h:329:23: warning: assignment to 'struct tty_struct *' from 'uintptr_t' {aka 'long unsigned int'} makes pointer from integer without a cast [-Wint-conversion] arch/riscv/include/asm/cmpxchg.h:329:23: warning: assignment to 'z_erofs_next_pcluster_t' {aka 'void *'} from 'uintptr_t' {aka 'long unsigned int'} makes pointer from integer without a cast [-Wint-conversion] cfi_probe.c:(.xiptext+0xdb): dangerous relocation: windowed longcall crosses 1GB boundary; return may fail: __kmalloc_noprof cfi_util.c:(.xiptext+0x40b): dangerous relocation: windowed longcall crosses 1GB boundary; return may fail: __kmalloc_noprof drivers/gpu/drm/display/drm_dp_mst_topology.c:5074:(.text+0x4da0): undefined reference to `drm_kms_helper_hotplug_event' drivers/gpu/drm/display/drm_dp_mst_topology.c:5088:(.text+0x40a8): undefined reference to `__drm_atomic_helper_private_obj_duplicate_state' drivers/gpu/drm/lima/lima_drv.c:387:13: error: cast to smaller integer type 'enum lima_gpu_id' from 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast] drivers/gpu/drm/panthor/panthor_sched.c:2048:6: error: variable 'csg_mod_mask' set but not used [-Werror,-Wunused-but-set-variable] drivers/gpu/drm/pl111/pl111_versatile.c:488:24: error: cast to smaller integer type 'enum versatile_clcd' from 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast] drivers/gpu/drm/qxl/qxl_cmd.c:424:6: error: variable 'count' set but not used [-Werror,-Wunused-but-set-variable] drivers/gpu/drm/qxl/qxl_ioctl.c:148:14: error: variable 'num_relocs' set but not used [-Werror,-Wunused-but-set-variable] drivers/s390/char/hmcdrv_cache.c:221:13: error: '__section__' attribute only applies to functions, global variables, Objective-C methods, and Objective-C properties drivers/s390/char/hmcdrv_cache.c:221:13: error: 'section' attribute only applies to functions, global variables, Objective-C methods, and Objective-C properties drivers/s390/char/hmcdrv_cache.c:221:13: error: non-extern declaration of '__pcpu_unique__alloc_tag_cntr' follows extern declaration drivers/s390/char/hmcdrv_ftp.c:196:21: error: '__section__' attribute only applies to functions, global variables, Objective-C methods, and Objective-C properties drivers/s390/char/hmcdrv_ftp.c:196:21: error: 'section' attribute only applies to functions, global variables, Objective-C methods, and Objective-C properties drivers/s390/char/hmcdrv_ftp.c:196:21: error: non-extern declaration of '__pcpu_unique__alloc_tag_cntr' follows extern declaration drivers/s390/char/hmcdrv_ftp.c:196:21: error: non-extern declaration of '_alloc_tag_cntr' follows extern declaration drivers/s390/char/hmcdrv_ftp.c:196:21: error: weak declaration cannot have internal linkage drm_dp_helper.c:(.text+0x36e8): undefined reference to `devm_backlight_device_register' drm_dp_mst_topology.c:(.text+0x350c): undefined reference to `__drm_atomic_helper_private_obj_duplicate_state' drm_dp_mst_topology.c:(.text+0x3f00): undefined reference to `drm_kms_helper_hotplug_event' include/asm-generic/io.h:547:31: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic] include/linux/alloc_tag.h:43:2: error: "Me
linux-next: build failure after merge of the drm-misc tree
Hi all, After merging the drm-misc tree, today's linux-next build (x86_64_allmodconfig) failed like this: In file included from drivers/gpu/drm/panthor/panthor_fw.c:19: drivers/gpu/drm/panthor/panthor_fw.c: In function 'panthor_job_irq_suspend': drivers/gpu/drm/panthor/panthor_device.h:326:13: error: unused variable 'cookie' [-Werror=unused-variable] 326 | int cookie; \ | ^~ drivers/gpu/drm/panthor/panthor_fw.c:979:1: note: in expansion of macro 'PANTHOR_IRQ_HANDLER' 979 | PANTHOR_IRQ_HANDLER(job, JOB, panthor_job_irq_handler); | ^~~ drivers/gpu/drm/panthor/panthor_fw.c: In function 'panthor_job_irq_resume': drivers/gpu/drm/panthor/panthor_device.h:336:13: error: unused variable 'cookie' [-Werror=unused-variable] 336 | int cookie; \ | ^~ drivers/gpu/drm/panthor/panthor_fw.c:979:1: note: in expansion of macro 'PANTHOR_IRQ_HANDLER' 979 | PANTHOR_IRQ_HANDLER(job, JOB, panthor_job_irq_handler); | ^~~ cc1: all warnings being treated as errors In file included from drivers/gpu/drm/panthor/panthor_gpu.c:19: drivers/gpu/drm/panthor/panthor_gpu.c: In function 'panthor_gpu_irq_suspend': drivers/gpu/drm/panthor/panthor_device.h:326:13: error: unused variable 'cookie' [-Werror=unused-variable] 326 | int cookie; \ | ^~ drivers/gpu/drm/panthor/panthor_gpu.c:166:1: note: in expansion of macro 'PANTHOR_IRQ_HANDLER' 166 | PANTHOR_IRQ_HANDLER(gpu, GPU, panthor_gpu_irq_handler); | ^~~ drivers/gpu/drm/panthor/panthor_gpu.c: In function 'panthor_gpu_irq_resume': drivers/gpu/drm/panthor/panthor_device.h:336:13: error: unused variable 'cookie' [-Werror=unused-variable] 336 | int cookie; \ | ^~ drivers/gpu/drm/panthor/panthor_gpu.c:166:1: note: in expansion of macro 'PANTHOR_IRQ_HANDLER' 166 | PANTHOR_IRQ_HANDLER(gpu, GPU, panthor_gpu_irq_handler); | ^~~ cc1: all warnings being treated as errors In file included from drivers/gpu/drm/panthor/panthor_mmu.c:30: drivers/gpu/drm/panthor/panthor_mmu.c: In function 'panthor_mmu_irq_suspend': drivers/gpu/drm/panthor/panthor_device.h:326:13: error: unused variable 'cookie' [-Werror=unused-variable] 326 | int cookie; \ | ^~ drivers/gpu/drm/panthor/panthor_mmu.c:1689:1: note: in expansion of macro 'PANTHOR_IRQ_HANDLER' 1689 | PANTHOR_IRQ_HANDLER(mmu, MMU, panthor_mmu_irq_handler); | ^~~ drivers/gpu/drm/panthor/panthor_mmu.c: In function 'panthor_mmu_irq_resume': drivers/gpu/drm/panthor/panthor_device.h:336:13: error: unused variable 'cookie' [-Werror=unused-variable] 336 | int cookie; \ | ^~ drivers/gpu/drm/panthor/panthor_mmu.c:1689:1: note: in expansion of macro 'PANTHOR_IRQ_HANDLER' 1689 | PANTHOR_IRQ_HANDLER(mmu, MMU, panthor_mmu_irq_handler); | ^~~ cc1: all warnings being treated as errors Caused by commit 962f88b9c916 ("drm/panthor: Drop the dev_enter/exit() sections in _irq_suspend/resume()") I have used the drm-misc tree from next-20240402 for today. -- Cheers, Stephen Rothwell pgpHuhSODs95K.pgp Description: OpenPGP digital signature
Re: [PATCH 18/22] drm/i915: Handle joined pipes inside hsw_crtc_disable()
On Mon, Apr 01, 2024 at 06:46:20AM +, Murthy, Arun R wrote: > > > -Original Message- > > From: Intel-gfx On Behalf Of Ville > > Syrjala > > Sent: Friday, March 29, 2024 6:43 AM > > To: intel-gfx@lists.freedesktop.org > > Subject: [PATCH 18/22] drm/i915: Handle joined pipes inside > > hsw_crtc_disable() > > > > From: Ville Syrjälä > > > > Reorganize the crtc disable path to only deal with the master > > pipes/transcoders > > in intel_old_crtc_state_disables() and offload the handling of joined pipes > > to > > hsw_crtc_disable(). > > This makes the whole thing much more sensible since we can actually control > > the order in which we do the per-pipe vs. > > per-transcoder modeset steps. > > > > v2: Use the name 'pipe_crtc' for the per-pipe crtc pointer > > > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 64 > > 1 file changed, 39 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 58ee40786d5c..c15ea046c62a 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -1791,29 +1791,28 @@ static void hsw_crtc_disable(struct > > intel_atomic_state *state, > > const struct intel_crtc_state *old_crtc_state = > > intel_atomic_get_old_crtc_state(state, crtc); > > struct drm_i915_private *i915 = to_i915(crtc->base.dev); > > + struct intel_crtc *pipe_crtc; > > > > /* > > * FIXME collapse everything to one hook. > > * Need care with mst->ddi interactions. > > */ > > - if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) { > > - intel_encoders_disable(state, crtc); > > - intel_encoders_post_disable(state, crtc); > > - } > > - > > - intel_disable_shared_dpll(old_crtc_state); > > + intel_encoders_disable(state, crtc); > > + intel_encoders_post_disable(state, crtc); > > > > - if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) { > > - struct intel_crtc *slave_crtc; > > + for_each_intel_crtc_in_pipe_mask(>drm, pipe_crtc, > > + > > intel_crtc_joined_pipe_mask(old_crtc_state)) { > > + const struct intel_crtc_state *old_pipe_crtc_state = > > + intel_atomic_get_old_crtc_state(state, pipe_crtc); > > > > - intel_encoders_post_pll_disable(state, crtc); > > + intel_disable_shared_dpll(old_pipe_crtc_state); > > + } > > As per the sequence is considered, should the pll be disabled prior to > disabling the encoders and then followed by post_pll_disable? The correct disable order is: 1. encoder disable() 2. disable transcoder/etc. (nop for hsw+ as that stuff has been sucked into the encoder hooks) 3. encoder post_disable() 4. pll disable 5. encoder post_pll_disable() which we should be following here, thouh the diff is rather hard to read due to the indentation changes. -- Ville Syrjälä Intel
✗ Fi.CI.BAT: failure for drm/i915/display: fix display param dup for NULL char * params
== Series Details == Series: drm/i915/display: fix display param dup for NULL char * params URL : https://patchwork.freedesktop.org/series/131949/ State : failure == Summary == CI Bug Log - changes from CI_DRM_14516 -> Patchwork_131949v1 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_131949v1 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_131949v1, please notify your bug team (i915-ci-in...@lists.freedesktop.org) to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131949v1/index.html Participating hosts (37 -> 32) -- Additional (1): fi-cfl-8109u Missing(6): bat-dg1-7 fi-snb-2520m bat-dg2-11 fi-bsw-nick bat-jsl-1 bat-arls-3 Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_131949v1: ### IGT changes ### Possible regressions * igt@debugfs_test@read_all_entries: - fi-kbl-x1275: [PASS][1] -> [ABORT][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14516/fi-kbl-x1275/igt@debugfs_test@read_all_entries.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131949v1/fi-kbl-x1275/igt@debugfs_test@read_all_entries.html - bat-adlp-11:[PASS][3] -> [ABORT][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14516/bat-adlp-11/igt@debugfs_test@read_all_entries.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131949v1/bat-adlp-11/igt@debugfs_test@read_all_entries.html - fi-cfl-8109u: NOTRUN -> [ABORT][5] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131949v1/fi-cfl-8109u/igt@debugfs_test@read_all_entries.html - fi-kbl-7567u: [PASS][6] -> [ABORT][7] [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14516/fi-kbl-7567u/igt@debugfs_test@read_all_entries.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131949v1/fi-kbl-7567u/igt@debugfs_test@read_all_entries.html - bat-adln-1: [PASS][8] -> [ABORT][9] [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14516/bat-adln-1/igt@debugfs_test@read_all_entries.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131949v1/bat-adln-1/igt@debugfs_test@read_all_entries.html - fi-ivb-3770:[PASS][10] -> [ABORT][11] [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14516/fi-ivb-3770/igt@debugfs_test@read_all_entries.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131949v1/fi-ivb-3770/igt@debugfs_test@read_all_entries.html - bat-mtlp-8: [PASS][12] -> [ABORT][13] [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14516/bat-mtlp-8/igt@debugfs_test@read_all_entries.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131949v1/bat-mtlp-8/igt@debugfs_test@read_all_entries.html - fi-elk-e7500: [PASS][14] -> [ABORT][15] [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14516/fi-elk-e7500/igt@debugfs_test@read_all_entries.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131949v1/fi-elk-e7500/igt@debugfs_test@read_all_entries.html - bat-dg2-8: [PASS][16] -> [ABORT][17] [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14516/bat-dg2-8/igt@debugfs_test@read_all_entries.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131949v1/bat-dg2-8/igt@debugfs_test@read_all_entries.html - fi-kbl-guc: [PASS][18] -> [ABORT][19] [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14516/fi-kbl-guc/igt@debugfs_test@read_all_entries.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131949v1/fi-kbl-guc/igt@debugfs_test@read_all_entries.html - bat-adls-6: [PASS][20] -> [ABORT][21] [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14516/bat-adls-6/igt@debugfs_test@read_all_entries.html [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131949v1/bat-adls-6/igt@debugfs_test@read_all_entries.html - bat-adlm-1: [PASS][22] -> [ABORT][23] [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14516/bat-adlm-1/igt@debugfs_test@read_all_entries.html [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131949v1/bat-adlm-1/igt@debugfs_test@read_all_entries.html - fi-ilk-650: [PASS][24] -> [ABORT][25] [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14516/fi-ilk-650/igt@debugfs_test@read_all_entries.html [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131949v1/fi-ilk-650/igt@debugfs_test@read_all_entries.html - fi-tgl-1115g4: [PASS][26] -> [ABORT][27] [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14516/fi-tgl-1115g4/igt@debugfs_test@read_all_entries.html [27]:
✗ Fi.CI.CHECKPATCH: warning for drm/i915/display: fix display param dup for NULL char * params
== Series Details == Series: drm/i915/display: fix display param dup for NULL char * params URL : https://patchwork.freedesktop.org/series/131949/ State : warning == Summary == Error: dim checkpatch failed b15c6c58c760 drm/i915/display: fix display param dup for NULL char * params -:11: WARNING:REPEATED_WORD: Possible repeated word: 'to' #11: duplication in that it converts NULL params to to allocated empty total: 0 errors, 1 warnings, 0 checks, 11 lines checked
✓ Fi.CI.BAT: success for drm/i915: Implemnt vblank sycnhronized mbus joining changes (rev3)
== Series Details == Series: drm/i915: Implemnt vblank sycnhronized mbus joining changes (rev3) URL : https://patchwork.freedesktop.org/series/131700/ State : success == Summary == CI Bug Log - changes from CI_DRM_14516 -> Patchwork_131700v3 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131700v3/index.html Participating hosts (37 -> 34) -- Additional (2): fi-cfl-8109u fi-kbl-8809g Missing(5): bat-dg1-7 fi-snb-2520m bat-dg2-11 fi-bsw-nick bat-arls-3 Known issues Here are the changes found in Patchwork_131700v3 that come from known issues: ### IGT changes ### Issues hit * igt@gem_huc_copy@huc-copy: - fi-cfl-8109u: NOTRUN -> [SKIP][1] ([i915#2190]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131700v3/fi-cfl-8109u/igt@gem_huc_c...@huc-copy.html * igt@gem_lmem_swapping@verify-random: - fi-cfl-8109u: NOTRUN -> [SKIP][2] ([i915#4613]) +3 other tests skip [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131700v3/fi-cfl-8109u/igt@gem_lmem_swapp...@verify-random.html * igt@kms_force_connector_basic@force-edid: - bat-dg2-8: [PASS][3] -> [INCOMPLETE][4] ([i915#10583] / [i915#2295]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14516/bat-dg2-8/igt@kms_force_connector_ba...@force-edid.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131700v3/bat-dg2-8/igt@kms_force_connector_ba...@force-edid.html * igt@kms_pm_rpm@basic-rte: - fi-cfl-8109u: NOTRUN -> [SKIP][5] +14 other tests skip [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131700v3/fi-cfl-8109u/igt@kms_pm_...@basic-rte.html * igt@runner@aborted: - fi-kbl-8809g: NOTRUN -> [FAIL][6] ([i915#4991]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131700v3/fi-kbl-8809g/igt@run...@aborted.html [i915#10583]: https://gitlab.freedesktop.org/drm/intel/issues/10583 [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190 [i915#2295]: https://gitlab.freedesktop.org/drm/intel/issues/2295 [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613 [i915#4991]: https://gitlab.freedesktop.org/drm/intel/issues/4991 Build changes - * Linux: CI_DRM_14516 -> Patchwork_131700v3 CI-20190529: 20190529 CI_DRM_14516: 5100fcc57dc5d45b246a0aeb068f4f8062d29b09 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7796: 2cfed18f6aa776c1593d7cc328d23225dd61bdf9 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_131700v3: 5100fcc57dc5d45b246a0aeb068f4f8062d29b09 @ git://anongit.freedesktop.org/gfx-ci/linux ### Linux commits 97b4459423db drm/i915: Optimize out redundant dbuf slice updates cc8ef49ad736 drm/i915: Use a plain old int for the cdclk/mdclk ratio 0b7ae17afa8f drm/i915: Implement vblank synchronized MBUS join changes a170abea595f drm/i915: Use the correct mdclk/cdclk ratio in MBUS updates c7b8dd0b2ed6 drm/i915: Use old mbus_join value when increasing CDCLK 1e446d439134 drm/i915: Add debugs for mbus joining and dbuf ratio programming 11a3a410d938 drm/i915: Extract intel_dbuf_mdclk_min_tracker_update() 5568a37c27ae drm/i915: Extract intel_dbuf_mbus_join_update() 4bb6fd91461f drm/i915: Relocate intel_mbus_dbox_update() 2a778d017929 drm/i915: Loop over all active pipes in intel_mbus_dbox_update fd6b0dd31d0d drm/i915/cdclk: Indicate whether CDCLK change happens during pre or post plane update 8588464a74e7 drm/i915/cdclk: Drop tgl/dg2 cdclk bump hacks a32776e3a36b drm/i915/cdclk: Fix voltage_level programming edge case 75619744b0c5 drm/i915/cdclk: Fix CDCLK programming order when pipes are active == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131700v3/index.html
✗ Fi.CI.SPARSE: warning for drm/i915: Implemnt vblank sycnhronized mbus joining changes (rev3)
== Series Details == Series: drm/i915: Implemnt vblank sycnhronized mbus joining changes (rev3) URL : https://patchwork.freedesktop.org/series/131700/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
Re: [RFC] drm/i915/dp: Log message when limiting SST link rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Thanks Charlton for the patch. I think in general it is a good idea to log this when the max rate is dropped to HBR3 for SST case. Please find my comments below, On Thu, Feb 29, 2024 at 11:49 PM Charlton Lin wrote: > > Driver currently limits link rate up to HBR3 in SST mode. Log a > message with monitor vendor, product id, and MSTM_CAP to > help understand what monitors are being downgraded by this limit. > > Cc: Ville Syrjälä > Cc: Khaled Almahallawy > Cc: Sean Paul > Signed-off-by: Charlton Lin > --- > drivers/gpu/drm/i915/display/intel_dp.c | 19 +++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index 6ece2c563c7a..0b2d6d88fd37 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -2437,6 +2437,25 @@ intel_dp_compute_link_config(struct intel_encoder > *encoder, > false, > ); > > + if (intel_dp_max_common_rate(intel_dp) > limits.max_rate) { > + u8 mstm_cap; > + u32 panel_id = drm_edid_get_panel_id(_dp->aux.ddc); > + char vend[4]; > + u16 product_id; > + > + drm_dbg_kms(>drm, > + "Limiting LR from max common rate %d to %d\n", We dont use LR acronym anywhere in the kernel for link rate, just say link rate here. Also I think would be good to log the reason why we are dropping this to HBR3 or add a comment with a Todo for this Manasi > + intel_dp_max_common_rate(intel_dp), > limits.max_rate); > + > + drm_edid_decode_panel_id(panel_id, vend, _id); > + > + if (intel_dp->dpcd[DP_DPCD_REV] >= DP_DPCD_REV_12 && > + drm_dp_dpcd_readb(_dp->aux, DP_MSTM_CAP, _cap) > == 1) > + drm_dbg_kms(>drm, > + "Manufacturer=%s Model=%x Sink > MSTM_CAP=%x\n", > + vend, product_id, mstm_cap); > + } > + > if (!dsc_needed) { > /* > * Optimize for slow and wide for everything, because there > are some > -- > 2.25.1 >
✓ Fi.CI.BAT: success for drm: ensure drm headers are self-contained and pass kernel-doc
== Series Details == Series: drm: ensure drm headers are self-contained and pass kernel-doc URL : https://patchwork.freedesktop.org/series/131944/ State : success == Summary == CI Bug Log - changes from CI_DRM_14516 -> Patchwork_131944v1 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131944v1/index.html Participating hosts (37 -> 35) -- Additional (1): fi-cfl-8109u Missing(3): bat-dg1-7 fi-bsw-nick fi-snb-2520m Known issues Here are the changes found in Patchwork_131944v1 that come from known issues: ### IGT changes ### Issues hit * igt@gem_huc_copy@huc-copy: - fi-cfl-8109u: NOTRUN -> [SKIP][1] ([i915#2190]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131944v1/fi-cfl-8109u/igt@gem_huc_c...@huc-copy.html * igt@gem_lmem_swapping@verify-random: - fi-cfl-8109u: NOTRUN -> [SKIP][2] ([i915#4613]) +3 other tests skip [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131944v1/fi-cfl-8109u/igt@gem_lmem_swapp...@verify-random.html * igt@i915_selftest@live@workarounds: - bat-dg2-11: [PASS][3] -> [DMESG-FAIL][4] ([i915#9500]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14516/bat-dg2-11/igt@i915_selftest@l...@workarounds.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131944v1/bat-dg2-11/igt@i915_selftest@l...@workarounds.html * igt@kms_pm_rpm@basic-rte: - fi-cfl-8109u: NOTRUN -> [SKIP][5] +14 other tests skip [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131944v1/fi-cfl-8109u/igt@kms_pm_...@basic-rte.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [i915#10435]: https://gitlab.freedesktop.org/drm/intel/issues/10435 [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190 [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613 [i915#9157]: https://gitlab.freedesktop.org/drm/intel/issues/9157 [i915#9500]: https://gitlab.freedesktop.org/drm/intel/issues/9500 Build changes - * Linux: CI_DRM_14516 -> Patchwork_131944v1 CI-20190529: 20190529 CI_DRM_14516: 5100fcc57dc5d45b246a0aeb068f4f8062d29b09 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7796: 2cfed18f6aa776c1593d7cc328d23225dd61bdf9 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_131944v1: 5100fcc57dc5d45b246a0aeb068f4f8062d29b09 @ git://anongit.freedesktop.org/gfx-ci/linux ### Linux commits 126cd9eac05a drm: ensure drm headers are self-contained and pass kernel-doc == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131944v1/index.html
✗ Fi.CI.CHECKPATCH: warning for drm: ensure drm headers are self-contained and pass kernel-doc
== Series Details == Series: drm: ensure drm headers are self-contained and pass kernel-doc URL : https://patchwork.freedesktop.org/series/131944/ State : warning == Summary == Error: dim checkpatch failed 54d61aa07c56 drm: ensure drm headers are self-contained and pass kernel-doc Traceback (most recent call last): File "scripts/spdxcheck.py", line 6, in from ply import lex, yacc ModuleNotFoundError: No module named 'ply' -:82: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating? #82: new file mode 100644 total: 0 errors, 1 warnings, 0 checks, 58 lines checked
Re: [PATCH v0 03/14] drm/gma500,drm/i915: Make I2C terminology more inclusive
On Tue, 02 Apr 2024, Easwar Hariharan wrote: > On 4/2/2024 7:32 AM, Jani Nikula wrote: >> On Tue, 02 Apr 2024, Easwar Hariharan wrote: >>> On 4/2/2024 12:48 AM, Jani Nikula wrote: On Fri, 29 Mar 2024, Easwar Hariharan wrote: > I2C v7, SMBus 3.2, and I3C specifications have replaced "master/slave" > with more appropriate terms. Inspired by and following on to Wolfram's > series to fix drivers/i2c/[1], fix the terminology for users of > I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists > in the specification. gma500 and i915 changes should be split. See MAINTAINERS. Might also split the i915 changes to smaller pieces, it's kind of random. And the changes here are not strictly related to I2C AFAICT, so the commit message should be updated. BR, Jani. >>> >>> >>> >>> I will split gma500 and i915 into their respective patches if possible in >>> v2. >>> >>> Can you say more about the changes being "not strictly related to I2C"? My >>> heuristic was to grep for master/slave, and look in the surrounding context >>> for >>> i2c-related terminology (i2c_pin, 7-bit address, struct i2c_adapter, >>> i2c_bus, etc) >>> to confirm that they are i2c-related, then following the references around >>> to >>> make the compiler happy. For e.g., I did not change the many references to >>> bigjoiner >>> master and slave because I understood from context they were not i2c >>> references. >>> >>> A couple examples would help me restrict the changes to I2C, since as >>> mentioned in the >>> discussion on Wolfram's thread, there are places where migrating away from >>> master/slave >>> terms in the code would conflict with the original technical manuals and >>> reduce correlation >>> and understanding of the code. >> >> I guess I was looking at the VBT changes in intel_bios.c. Granted, they >> do end up being used as i2c addresses. No big deal. >> >> I think I'd expect the treewide i2c adapter changes to land first, via >> i2c, and subsequent cleanups to happen next, via individual driver >> trees. There's quite a bit of conflict potential merging this outside of >> drm-intel-next, and there's really no need for that. >> >> BR, >> Jani. >> > > Great! Just so I'm clear, do you still want the i915 changes split up more, > along with them being > split off from gma500? If we can merge the i915 changes via drm-intel-next, it's probably fine as a big i915 patch. Just the gma500 separated. (The struct i2c_algorithm change etc. necessarily has to go via I2C tree of course.) BR, Jani. > > Thanks, > Easwar -- Jani Nikula, Intel
Re: [PATCH] drm/i915/display: fix display param dup for NULL char * params
On Tue, 02 Apr 2024, Lucas De Marchi wrote: > On Tue, Apr 02, 2024 at 06:55:34PM +0300, Jani Nikula wrote: >>The display param duplication deviates from the original param >>duplication in that it converts NULL params to to allocated empty >>strings. This works for the vbt_firmware parameter, but not for >>dmc_firmware_path, the user of which interprets NULL and the empty >>string as distinct values. Specifically, the empty dmc_firmware_path >>leads to DMC and PM being disabled. >> >>Just remove the NULL check and pass it to kstrdup(), which safely >>returns NULL for NULL input. >> >>Fixes: 8015bee0bfec ("drm/i915/display: Add framework to add parameters >>specific to display") >>Fixes: 0d82a0d6f556 ("drm/i915/display: move dmc_firmware_path to display >>params") >>Cc: Jouni Högander >>Cc: Luca Coelho >>Cc: # v6.8+ >>Signed-off-by: Jani Nikula > > Reviewed-by: Lucas De Marchi Thanks! > ... but what's the purpose of the duplication? How one is supposed to > use the dmc_firmware with e.g. LNL + BMG? Setting it later via debugfs > doesn´t change the behavior. AFAIR this was done to support multiple > devices, but I don't think it achieves its purpose or I'm missing > something. > > By leaving a param writable and not duplicate it at all, we are at > least be allowed to: > > 1) disable autoprobe > 2) load module > 3) bind do LNL > 4) set dmc_firmware param > 5) bind to BMG > > Yeah, it's manual and not intuitive, but should only be used by > developers with targeted debug. How would we do something like that > with the current code? > > I know that for params via sysfs, it's impossible to get them back to > NULL, so I think we should make sure NULL and empty is handled the same > way. Getting it back to empty is hard enough but at least possible (see > https://lore.kernel.org/igt-dev/20240228223134.3908035-4-lucas.demar...@intel.com/), > but I think this is not the case for debugfs. There are a lot of angles to this. :) First of all, I think when we do copy the params, they should be preserved as they were, instead of changed. This patch fixes this part, and the bug that currently disables DMC altogether. We do copies for a few reasons. From module params to device params, and from device params to error capture. I agree that making a distinction between an unset parameter and a parameter set to NULL is probably a mistake, because as you mention it's not feasible to go back to NULL. In this case, NULL means default and "" means disabled. No way to go back to default. For params that only make sense at probe, we should perhaps leave the module parameter writable, and the device parameter read only. Even in this case, the duplication is a feature and makes sense: you can modify the module parameter, but it only makes a difference to devices bound after the change. For devices already bound, you can look up the value that was used from debugfs. BR, Jani. -- Jani Nikula, Intel
✗ Fi.CI.SPARSE: warning for drm/i915: Bigjoiner prep work
== Series Details == Series: drm/i915: Bigjoiner prep work URL : https://patchwork.freedesktop.org/series/131942/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
✗ Fi.CI.CHECKPATCH: warning for drm/i915: Bigjoiner prep work
== Series Details == Series: drm/i915: Bigjoiner prep work URL : https://patchwork.freedesktop.org/series/131942/ State : warning == Summary == Error: dim checkpatch failed 79f3132bda07 drm/i915: Remove DRM_MODE_FLAG_DBLSCAN checks from .mode_valid() hooks 24b6b2eb8c3b drm/i915: Shuffle DP .mode_valid() checks bf6debe75399 drm/i915: Clean up glk_pipe_scaler_clock_gating_wa() 7dbb77f01be2 drm/i915: Extract glk_need_scaler_clock_gating_wa() 84e96c39fc96 drm/i915/mst: Limit MST+DSC to TGL+ -:28: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__i915' - possible side-effects? #28: FILE: drivers/gpu/drm/i915/display/intel_display_device.h:50: +#define HAS_DSC_MST(__i915)(DISPLAY_VER(__i915) >= 12 && HAS_DSC(__i915)) total: 0 errors, 0 warnings, 1 checks, 15 lines checked 1c3066d956b8 drm/i915/mst: Reject FEC+MST on ICL 60bc803b5d92 drm/i915: Use debugfs_create_bool() for "i915_bigjoiner_force_enable"
Re: [PATCH] drm/i915/display: fix display param dup for NULL char * params
On Tue, Apr 02, 2024 at 06:55:34PM +0300, Jani Nikula wrote: The display param duplication deviates from the original param duplication in that it converts NULL params to to allocated empty strings. This works for the vbt_firmware parameter, but not for dmc_firmware_path, the user of which interprets NULL and the empty string as distinct values. Specifically, the empty dmc_firmware_path leads to DMC and PM being disabled. Just remove the NULL check and pass it to kstrdup(), which safely returns NULL for NULL input. Fixes: 8015bee0bfec ("drm/i915/display: Add framework to add parameters specific to display") Fixes: 0d82a0d6f556 ("drm/i915/display: move dmc_firmware_path to display params") Cc: Jouni Högander Cc: Luca Coelho Cc: # v6.8+ Signed-off-by: Jani Nikula Reviewed-by: Lucas De Marchi ... but what's the purpose of the duplication? How one is supposed to use the dmc_firmware with e.g. LNL + BMG? Setting it later via debugfs doesn´t change the behavior. AFAIR this was done to support multiple devices, but I don't think it achieves its purpose or I'm missing something. By leaving a param writable and not duplicate it at all, we are at least be allowed to: 1) disable autoprobe 2) load module 3) bind do LNL 4) set dmc_firmware param 5) bind to BMG Yeah, it's manual and not intuitive, but should only be used by developers with targeted debug. How would we do something like that with the current code? I know that for params via sysfs, it's impossible to get them back to NULL, so I think we should make sure NULL and empty is handled the same way. Getting it back to empty is hard enough but at least possible (see https://lore.kernel.org/igt-dev/20240228223134.3908035-4-lucas.demar...@intel.com/), but I think this is not the case for debugfs. Lucas De Marchi
[PATCH] drm/i915/display: fix display param dup for NULL char * params
The display param duplication deviates from the original param duplication in that it converts NULL params to to allocated empty strings. This works for the vbt_firmware parameter, but not for dmc_firmware_path, the user of which interprets NULL and the empty string as distinct values. Specifically, the empty dmc_firmware_path leads to DMC and PM being disabled. Just remove the NULL check and pass it to kstrdup(), which safely returns NULL for NULL input. Fixes: 8015bee0bfec ("drm/i915/display: Add framework to add parameters specific to display") Fixes: 0d82a0d6f556 ("drm/i915/display: move dmc_firmware_path to display params") Cc: Jouni Högander Cc: Luca Coelho Cc: # v6.8+ Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_display_params.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_params.c b/drivers/gpu/drm/i915/display/intel_display_params.c index c8e3d6892e23..49c6b42077dc 100644 --- a/drivers/gpu/drm/i915/display/intel_display_params.c +++ b/drivers/gpu/drm/i915/display/intel_display_params.c @@ -176,9 +176,9 @@ void intel_display_params_dump(struct drm_i915_private *i915, struct drm_printer #undef PRINT } -__maybe_unused static void _param_dup_charp(char **valp) +static void _param_dup_charp(char **valp) { - *valp = kstrdup(*valp ? *valp : "", GFP_ATOMIC); + *valp = kstrdup(*valp, GFP_ATOMIC); } __maybe_unused static void _param_nop(void *valp) -- 2.39.2
[PATCH v2 14/14] drm/i915: Optimize out redundant dbuf slice updates
From: Ville Syrjälä if the new dbuf slices are a superset of the old dbuf slices then we don't have to do anything in intel_dbuf_post_plane_update(). Restructure the code to skip such redundant dbuf slice updates. The main benefit is slightly less confusing logs. Reviewed-by: Uma Shankar Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/skl_watermark.c | 27 +--- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index 1b48009efe2b..50ec51065118 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -3788,16 +3788,20 @@ void intel_dbuf_pre_plane_update(struct intel_atomic_state *state) intel_atomic_get_new_dbuf_state(state); const struct intel_dbuf_state *old_dbuf_state = intel_atomic_get_old_dbuf_state(state); + u8 old_slices, new_slices; - if (!new_dbuf_state || - new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices) + if (!new_dbuf_state) + return; + + old_slices = old_dbuf_state->enabled_slices; + new_slices = old_dbuf_state->enabled_slices | new_dbuf_state->enabled_slices; + + if (old_slices == new_slices) return; WARN_ON(!new_dbuf_state->base.changed); - gen9_dbuf_slices_update(i915, - old_dbuf_state->enabled_slices | - new_dbuf_state->enabled_slices); + gen9_dbuf_slices_update(i915, new_slices); } void intel_dbuf_post_plane_update(struct intel_atomic_state *state) @@ -3807,15 +3811,20 @@ void intel_dbuf_post_plane_update(struct intel_atomic_state *state) intel_atomic_get_new_dbuf_state(state); const struct intel_dbuf_state *old_dbuf_state = intel_atomic_get_old_dbuf_state(state); + u8 old_slices, new_slices; - if (!new_dbuf_state || - new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices) + if (!new_dbuf_state) + return; + + old_slices = old_dbuf_state->enabled_slices | new_dbuf_state->enabled_slices; + new_slices = new_dbuf_state->enabled_slices; + + if (old_slices == new_slices) return; WARN_ON(!new_dbuf_state->base.changed); - gen9_dbuf_slices_update(i915, - new_dbuf_state->enabled_slices); + gen9_dbuf_slices_update(i915, new_slices); } static int skl_watermark_ipc_status_show(struct seq_file *m, void *data) -- 2.43.2
[PATCH v2 13/14] drm/i915: Use a plain old int for the cdclk/mdclk ratio
From: Ville Syrjälä No point in throwing around u8 when we're dealing with just an integer. Use a plain old boring 'int'. Reviewed-by: Uma Shankar Reviewed-by: Gustavo Sousa Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_cdclk.c | 6 +++--- drivers/gpu/drm/i915/display/intel_cdclk.h | 4 ++-- drivers/gpu/drm/i915/display/skl_watermark.c | 6 -- drivers/gpu/drm/i915/display/skl_watermark.h | 6 -- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index c23b7ee2837c..d61aa5b7cbdb 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -1893,8 +1893,8 @@ static u32 xe2lpd_mdclk_source_sel(struct drm_i915_private *i915) return MDCLK_SOURCE_SEL_CD2XCLK; } -u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915, - const struct intel_cdclk_config *cdclk_config) +int intel_mdclk_cdclk_ratio(struct drm_i915_private *i915, + const struct intel_cdclk_config *cdclk_config) { if (mdclk_source_is_cdclk_pll(i915)) return DIV_ROUND_UP(cdclk_config->vco, cdclk_config->cdclk); @@ -,7 +,7 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state) if (intel_mdclk_cdclk_ratio(dev_priv, _cdclk_state->actual) != intel_mdclk_cdclk_ratio(dev_priv, _cdclk_state->actual)) { - u8 ratio = intel_mdclk_cdclk_ratio(dev_priv, _cdclk_state->actual); + int ratio = intel_mdclk_cdclk_ratio(dev_priv, _cdclk_state->actual); ret = intel_dbuf_state_set_mdclk_cdclk_ratio(state, ratio); if (ret) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h index 5d4faf401774..cfdcdec07a4d 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.h +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h @@ -67,8 +67,8 @@ void intel_update_cdclk(struct drm_i915_private *dev_priv); u32 intel_read_rawclk(struct drm_i915_private *dev_priv); bool intel_cdclk_clock_changed(const struct intel_cdclk_config *a, const struct intel_cdclk_config *b); -u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915, - const struct intel_cdclk_config *cdclk_config); +int intel_mdclk_cdclk_ratio(struct drm_i915_private *i915, + const struct intel_cdclk_config *cdclk_config); bool intel_cdclk_is_decreasing_later(struct intel_atomic_state *state); void intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state); void intel_set_cdclk_post_plane_update(struct intel_atomic_state *state); diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index ca0f1f89e6d9..1b48009efe2b 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -3616,7 +3616,8 @@ static void intel_mbus_dbox_update(struct intel_atomic_state *state) } } -int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8 ratio) +int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, + int ratio) { struct intel_dbuf_state *dbuf_state; @@ -3629,7 +3630,8 @@ int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8 return intel_atomic_lock_global_state(_state->base); } -void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio, bool joined_mbus) +void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, +int ratio, bool joined_mbus) { enum dbuf_slice slice; diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h index 3323a1d973f9..ef1a008466be 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.h +++ b/drivers/gpu/drm/i915/display/skl_watermark.h @@ -74,11 +74,13 @@ intel_atomic_get_dbuf_state(struct intel_atomic_state *state); to_intel_dbuf_state(intel_atomic_get_new_global_obj_state(state, _i915(state->base.dev)->display.dbuf.obj)) int intel_dbuf_init(struct drm_i915_private *i915); -int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8 ratio); +int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, + int ratio); void intel_dbuf_pre_plane_update(struct intel_atomic_state *state); void intel_dbuf_post_plane_update(struct intel_atomic_state *state); -void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio, bool joined_mbus); +void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, +int ratio, bool joined_mbus); void
[PATCH v2 12/14] drm/i915: Implement vblank synchronized MBUS join changes
From: Stanislav Lisovskiy Currently we can't change MBUS join status without doing a modeset, because we are lacking mechanism to synchronize those with vblank. However then this means that we can't do a fastset, if there is a need to change MBUS join state. Fix that by implementing such change. We already call correspondent check and update at pre_plane dbuf update, so the only thing left is to have a non-modeset version of that. If active pipes stay the same then fastset is possible and only MBUS join state/ddb allocation updates would be committed. The full mbus/cdclk sequence will look as follows: 1. disable pipes 2. increase cdclk if necessary 2.1 reprogram cdclk 2.2 update dbuf tracker value 3. enable mbus joining if necessary 3.1 update mbus_ctl 3.2 update dbuf tracker value 4. reallocate dbuf for planes on active pipes 5. disable mbus joining if necessary 5.1 update dbuf tracker value 5.2 update mbus_ctl 6. enable pipes 7. decrease cdclk if necessary 7.1 update dbuf tracker value 7.2 reprogram cdclk And in order to keep things in sync we need: Step 2: - mbus_join == old - mdclk/cdclk ratio == new Step 3: - mbus_join == new - mdclk/cdclk ratio == old when cdclk is changing in step 7 - mdclk/cdclk ratio == new when cdclk is changing in step 2 Step 5: - mbus_join == new - mdclk/cdclk ratio == old when cdclk is changing in step 7 - mdclk/cdclk ratio == new when cdclk is changing in step 2 Step 7: - mbus_join == new - mdclk/cdclk ratio == new v2: - Removed redundant parentheses(Ville Syrjälä) - Constified new_crtc_state in intel_mbus_joined_pipe(Ville Syrjälä) - Removed pipe_select variable(Ville Syrjälä) [v3: vsyrjala: Correctly sequence vs. cdclk updates, properly describe the full sequence, shuffle code around to make the diff more legible, streamline a few things] [v4: vsyrjala: Move the intel_cdclk_is_decreasing_later() stuff to a separate patch] Cc: Gustavo Sousa Reviewed-by: Uma Shankar #v3 Signed-off-by: Stanislav Lisovskiy Co-developed-by: Ville Syrjälä Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 5 +- drivers/gpu/drm/i915/display/skl_watermark.c | 122 +-- drivers/gpu/drm/i915/display/skl_watermark.h | 3 +- 3 files changed, 92 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 614e60420a29..e0616f3e4d27 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -6906,6 +6906,8 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state) intel_pre_update_crtc(state, crtc); } + intel_dbuf_mbus_pre_ddb_update(state); + while (update_pipes) { for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { @@ -6936,6 +6938,8 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state) } } + intel_dbuf_mbus_post_ddb_update(state); + update_pipes = modeset_pipes; /* @@ -7182,7 +7186,6 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) intel_encoders_update_prepare(state); intel_dbuf_pre_plane_update(state); - intel_mbus_dbox_update(state); for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { if (new_crtc_state->do_async_flip) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index 028c3e6d6b1d..ca0f1f89e6d9 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -2636,13 +2636,6 @@ skl_compute_ddb(struct intel_atomic_state *state) if (ret) return ret; - if (old_dbuf_state->joined_mbus != new_dbuf_state->joined_mbus) { - /* TODO: Implement vblank synchronized MBUS joining changes */ - ret = intel_modeset_all_pipes_late(state, "MBUS joining change"); - if (ret) - return ret; - } - drm_dbg_kms(>drm, "Enabled dbuf slices 0x%x -> 0x%x (total dbuf slices 0x%x), mbus joined? %s->%s\n", old_dbuf_state->enabled_slices, @@ -3559,7 +3552,7 @@ static bool xelpdp_is_only_pipe_per_dbuf_bank(enum pipe pipe, u8 active_pipes) return false; } -void intel_mbus_dbox_update(struct intel_atomic_state *state) +static void intel_mbus_dbox_update(struct intel_atomic_state *state) { struct drm_i915_private *i915 = to_i915(state->base.dev); const struct intel_dbuf_state *new_dbuf_state, *old_dbuf_state; @@ -3640,6 +3633,9 @@ void
[PATCH v2 11/14] drm/i915: Use the correct mdclk/cdclk ratio in MBUS updates
From: Ville Syrjälä The current cdclk/mbus programming sequence is as follows: 1. intel_set_cdclk_pre_plane_update() 2. update_mbus_pre_enable() 3. intel_set_cdclk_post_plane_update() when the actual mdclk/cdclk programming is postponed to intel_set_cdclk_post_plane_update() we must keep using the old mdclk/cdclk ratio during update_mbus_pre_enable(). This guarantees the programmed ratio matches the rest of the hardware state (mdlk/cdclk/mbus joining). v2: Extracted from the vblank synchronized mbus programming patch Cc: Gustavo Sousa Reviewed-by: Uma Shankar #v1 Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_cdclk.c | 11 +++ drivers/gpu/drm/i915/display/intel_cdclk.h | 1 + drivers/gpu/drm/i915/display/skl_watermark.c | 19 --- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index e0c69d85e733..c23b7ee2837c 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2576,6 +2576,17 @@ static void intel_cdclk_pcode_post_notify(struct intel_atomic_state *state) update_cdclk, update_pipe_count); } +bool intel_cdclk_is_decreasing_later(struct intel_atomic_state *state) +{ + const struct intel_cdclk_state *old_cdclk_state = + intel_atomic_get_old_cdclk_state(state); + const struct intel_cdclk_state *new_cdclk_state = + intel_atomic_get_new_cdclk_state(state); + + return new_cdclk_state && !new_cdclk_state->disable_pipes && + new_cdclk_state->actual.cdclk < old_cdclk_state->actual.cdclk; +} + /** * intel_set_cdclk_pre_plane_update - Push the CDCLK state to the hardware * @state: intel atomic state diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h index 2843fc091086..5d4faf401774 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.h +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h @@ -69,6 +69,7 @@ bool intel_cdclk_clock_changed(const struct intel_cdclk_config *a, const struct intel_cdclk_config *b); u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915, const struct intel_cdclk_config *cdclk_config); +bool intel_cdclk_is_decreasing_later(struct intel_atomic_state *state); void intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state); void intel_set_cdclk_post_plane_update(struct intel_atomic_state *state); void intel_cdclk_dump_config(struct drm_i915_private *i915, diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index a118ecf9e532..028c3e6d6b1d 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -3663,20 +3663,17 @@ static void intel_dbuf_mdclk_min_tracker_update(struct intel_atomic_state *state intel_atomic_get_old_dbuf_state(state); const struct intel_dbuf_state *new_dbuf_state = intel_atomic_get_new_dbuf_state(state); + int mdclk_cdclk_ratio; - if (DISPLAY_VER(i915) >= 20 && - old_dbuf_state->mdclk_cdclk_ratio != new_dbuf_state->mdclk_cdclk_ratio) { - /* -* For Xe2LPD and beyond, when there is a change in the ratio -* between MDCLK and CDCLK, updates to related registers need to -* happen at a specific point in the CDCLK change sequence. In -* that case, we defer to the call to -* intel_dbuf_mdclk_cdclk_ratio_update() to the CDCLK logic. -*/ - return; + if (intel_cdclk_is_decreasing_later(state)) { + /* cdclk/mdclk will be changed later by intel_set_cdclk_post_plane_update() */ + mdclk_cdclk_ratio = old_dbuf_state->mdclk_cdclk_ratio; + } else { + /* cdclk/mdclk already changed by intel_set_cdclk_pre_plane_update() */ + mdclk_cdclk_ratio = new_dbuf_state->mdclk_cdclk_ratio; } - intel_dbuf_mdclk_cdclk_ratio_update(i915, new_dbuf_state->mdclk_cdclk_ratio, + intel_dbuf_mdclk_cdclk_ratio_update(i915, mdclk_cdclk_ratio, new_dbuf_state->joined_mbus); } -- 2.43.2
[PATCH v2 10/14] drm/i915: Use old mbus_join value when increasing CDCLK
From: Stanislav Lisovskiy In order to make sure we are not breaking the proper sequence let's do updates step by step and don't change MBUS join value during MDCLK/CDCLK programming stage. MBUS join programming would be taken care by pre/post ddb hooks. v2: - Reworded comment about using old mbus_join value in intel_set_cdclk(Ville Syrjälä) Reviewed-by: Uma Shankar Signed-off-by: Stanislav Lisovskiy [v3: vsyrjala: rebase on top of cdclk changes, reword a bit more] Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_cdclk.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index ed8d9ee094b8..e0c69d85e733 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2617,6 +2617,12 @@ intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state) old_cdclk_state->actual.voltage_level); } + /* +* mbus joining will be changed later by +* intel_dbuf_mbus_{pre,post}_ddb_update() +*/ + cdclk_config.joined_mbus = old_cdclk_state->actual.joined_mbus; + drm_WARN_ON(>drm, !new_cdclk_state->base.changed); intel_set_cdclk(i915, _config, pipe, -- 2.43.2
[PATCH v2 09/14] drm/i915: Add debugs for mbus joining and dbuf ratio programming
From: Ville Syrjälä Add some debugs so that we can actually observe what is actually happening during the mbus/dbuf programming steps. We can just shove them into fairly low level functions as none of them are called during any critical sections/etc. Reviewed-by: Uma Shankar Reviewed-by: Gustavo Sousa Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/skl_watermark.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index 7767c5eada36..a118ecf9e532 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -3647,6 +3647,9 @@ void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio if (joined_mbus) ratio *= 2; + drm_dbg_kms(>drm, "Updating dbuf ratio to %d (mbus joined: %s)\n", + ratio, str_yes_no(joined_mbus)); + for_each_dbuf_slice(i915, slice) intel_de_rmw(i915, DBUF_CTL_S(slice), DBUF_MIN_TRACKER_STATE_SERVICE_MASK, @@ -3680,10 +3683,16 @@ static void intel_dbuf_mdclk_min_tracker_update(struct intel_atomic_state *state static void intel_dbuf_mbus_join_update(struct intel_atomic_state *state) { struct drm_i915_private *i915 = to_i915(state->base.dev); + const struct intel_dbuf_state *old_dbuf_state = + intel_atomic_get_old_dbuf_state(state); const struct intel_dbuf_state *new_dbuf_state = intel_atomic_get_new_dbuf_state(state); u32 mbus_ctl; + drm_dbg_kms(>drm, "Changing mbus joined: %s -> %s\n", + str_yes_no(old_dbuf_state->joined_mbus), + str_yes_no(new_dbuf_state->joined_mbus)); + /* * TODO: Implement vblank synchronized MBUS joining changes. * Must be properly coordinated with dbuf reprogramming. -- 2.43.2
[PATCH v2 08/14] drm/i915: Extract intel_dbuf_mdclk_min_tracker_update()
From: Ville Syrjälä Extract the stuff that writes the dbuf/mbus ratio stuff into its own function. Will help with correctly sequencing the operations done during mbus programming. Reviewed-by: Uma Shankar Reviewed-by: Gustavo Sousa Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/skl_watermark.c | 43 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index f7e03078bd43..7767c5eada36 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -3653,6 +3653,30 @@ void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio DBUF_MIN_TRACKER_STATE_SERVICE(ratio - 1)); } +static void intel_dbuf_mdclk_min_tracker_update(struct intel_atomic_state *state) +{ + struct drm_i915_private *i915 = to_i915(state->base.dev); + const struct intel_dbuf_state *old_dbuf_state = + intel_atomic_get_old_dbuf_state(state); + const struct intel_dbuf_state *new_dbuf_state = + intel_atomic_get_new_dbuf_state(state); + + if (DISPLAY_VER(i915) >= 20 && + old_dbuf_state->mdclk_cdclk_ratio != new_dbuf_state->mdclk_cdclk_ratio) { + /* +* For Xe2LPD and beyond, when there is a change in the ratio +* between MDCLK and CDCLK, updates to related registers need to +* happen at a specific point in the CDCLK change sequence. In +* that case, we defer to the call to +* intel_dbuf_mdclk_cdclk_ratio_update() to the CDCLK logic. +*/ + return; + } + + intel_dbuf_mdclk_cdclk_ratio_update(i915, new_dbuf_state->mdclk_cdclk_ratio, + new_dbuf_state->joined_mbus); +} + static void intel_dbuf_mbus_join_update(struct intel_atomic_state *state) { struct drm_i915_private *i915 = to_i915(state->base.dev); @@ -3683,10 +3707,6 @@ static void intel_dbuf_mbus_join_update(struct intel_atomic_state *state) static void update_mbus_pre_enable(struct intel_atomic_state *state) { struct drm_i915_private *i915 = to_i915(state->base.dev); - const struct intel_dbuf_state *old_dbuf_state = - intel_atomic_get_old_dbuf_state(state); - const struct intel_dbuf_state *new_dbuf_state = - intel_atomic_get_new_dbuf_state(state); if (!HAS_MBUS_JOINING(i915)) return; @@ -3697,20 +3717,7 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state) */ intel_dbuf_mbus_join_update(state); - if (DISPLAY_VER(i915) >= 20 && - old_dbuf_state->mdclk_cdclk_ratio != new_dbuf_state->mdclk_cdclk_ratio) { - /* -* For Xe2LPD and beyond, when there is a change in the ratio -* between MDCLK and CDCLK, updates to related registers need to -* happen at a specific point in the CDCLK change sequence. In -* that case, we defer to the call to -* intel_dbuf_mdclk_cdclk_ratio_update() to the CDCLK logic. -*/ - return; - } - - intel_dbuf_mdclk_cdclk_ratio_update(i915, new_dbuf_state->mdclk_cdclk_ratio, - new_dbuf_state->joined_mbus); + intel_dbuf_mdclk_min_tracker_update(state); } void intel_dbuf_pre_plane_update(struct intel_atomic_state *state) -- 2.43.2
[PATCH v2 07/14] drm/i915: Extract intel_dbuf_mbus_join_update()
From: Ville Syrjälä Extact the stuff that writes the joining bits in MBUS_CTL into its own function. Will help with correctly sequencing the operations done during mbus programming. Reviewed-by: Uma Shankar Reviewed-by: Gustavo Sousa Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/skl_watermark.c | 37 +--- 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index 6bd3fec0aa56..f7e03078bd43 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -3653,21 +3653,12 @@ void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio DBUF_MIN_TRACKER_STATE_SERVICE(ratio - 1)); } -/* - * Configure MBUS_CTL and all DBUF_CTL_S of each slice to join_mbus state before - * update the request state of all DBUS slices. - */ -static void update_mbus_pre_enable(struct intel_atomic_state *state) +static void intel_dbuf_mbus_join_update(struct intel_atomic_state *state) { struct drm_i915_private *i915 = to_i915(state->base.dev); + const struct intel_dbuf_state *new_dbuf_state = + intel_atomic_get_new_dbuf_state(state); u32 mbus_ctl; - const struct intel_dbuf_state *old_dbuf_state = - intel_atomic_get_old_dbuf_state(state); - const struct intel_dbuf_state *new_dbuf_state = - intel_atomic_get_new_dbuf_state(state); - - if (!HAS_MBUS_JOINING(i915)) - return; /* * TODO: Implement vblank synchronized MBUS joining changes. @@ -3683,6 +3674,28 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state) intel_de_rmw(i915, MBUS_CTL, MBUS_HASHING_MODE_MASK | MBUS_JOIN | MBUS_JOIN_PIPE_SELECT_MASK, mbus_ctl); +} + +/* + * Configure MBUS_CTL and all DBUF_CTL_S of each slice to join_mbus state before + * update the request state of all DBUS slices. + */ +static void update_mbus_pre_enable(struct intel_atomic_state *state) +{ + struct drm_i915_private *i915 = to_i915(state->base.dev); + const struct intel_dbuf_state *old_dbuf_state = + intel_atomic_get_old_dbuf_state(state); + const struct intel_dbuf_state *new_dbuf_state = + intel_atomic_get_new_dbuf_state(state); + + if (!HAS_MBUS_JOINING(i915)) + return; + + /* +* TODO: Implement vblank synchronized MBUS joining changes. +* Must be properly coordinated with dbuf reprogramming. +*/ + intel_dbuf_mbus_join_update(state); if (DISPLAY_VER(i915) >= 20 && old_dbuf_state->mdclk_cdclk_ratio != new_dbuf_state->mdclk_cdclk_ratio) { -- 2.43.2
[PATCH v2 06/14] drm/i915: Relocate intel_mbus_dbox_update()
From: Ville Syrjälä intel_mbus_dbox_update() will become static soon. Relocate it into a place that avoids having to add a forward declaration for it. Reviewed-by: Uma Shankar Reviewed-by: Gustavo Sousa Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/skl_watermark.c | 166 +-- 1 file changed, 83 insertions(+), 83 deletions(-) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index f582992592c1..6bd3fec0aa56 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -3540,6 +3540,89 @@ int intel_dbuf_init(struct drm_i915_private *i915) return 0; } +static bool xelpdp_is_only_pipe_per_dbuf_bank(enum pipe pipe, u8 active_pipes) +{ + switch (pipe) { + case PIPE_A: + return !(active_pipes & BIT(PIPE_D)); + case PIPE_D: + return !(active_pipes & BIT(PIPE_A)); + case PIPE_B: + return !(active_pipes & BIT(PIPE_C)); + case PIPE_C: + return !(active_pipes & BIT(PIPE_B)); + default: /* to suppress compiler warning */ + MISSING_CASE(pipe); + break; + } + + return false; +} + +void intel_mbus_dbox_update(struct intel_atomic_state *state) +{ + struct drm_i915_private *i915 = to_i915(state->base.dev); + const struct intel_dbuf_state *new_dbuf_state, *old_dbuf_state; + const struct intel_crtc *crtc; + u32 val = 0; + + if (DISPLAY_VER(i915) < 11) + return; + + new_dbuf_state = intel_atomic_get_new_dbuf_state(state); + old_dbuf_state = intel_atomic_get_old_dbuf_state(state); + if (!new_dbuf_state || + (new_dbuf_state->joined_mbus == old_dbuf_state->joined_mbus && +new_dbuf_state->active_pipes == old_dbuf_state->active_pipes)) + return; + + if (DISPLAY_VER(i915) >= 14) + val |= MBUS_DBOX_I_CREDIT(2); + + if (DISPLAY_VER(i915) >= 12) { + val |= MBUS_DBOX_B2B_TRANSACTIONS_MAX(16); + val |= MBUS_DBOX_B2B_TRANSACTIONS_DELAY(1); + val |= MBUS_DBOX_REGULATE_B2B_TRANSACTIONS_EN; + } + + if (DISPLAY_VER(i915) >= 14) + val |= new_dbuf_state->joined_mbus ? MBUS_DBOX_A_CREDIT(12) : +MBUS_DBOX_A_CREDIT(8); + else if (IS_ALDERLAKE_P(i915)) + /* Wa_22010947358:adl-p */ + val |= new_dbuf_state->joined_mbus ? MBUS_DBOX_A_CREDIT(6) : +MBUS_DBOX_A_CREDIT(4); + else + val |= MBUS_DBOX_A_CREDIT(2); + + if (DISPLAY_VER(i915) >= 14) { + val |= MBUS_DBOX_B_CREDIT(0xA); + } else if (IS_ALDERLAKE_P(i915)) { + val |= MBUS_DBOX_BW_CREDIT(2); + val |= MBUS_DBOX_B_CREDIT(8); + } else if (DISPLAY_VER(i915) >= 12) { + val |= MBUS_DBOX_BW_CREDIT(2); + val |= MBUS_DBOX_B_CREDIT(12); + } else { + val |= MBUS_DBOX_BW_CREDIT(1); + val |= MBUS_DBOX_B_CREDIT(8); + } + + for_each_intel_crtc_in_pipe_mask(>drm, crtc, new_dbuf_state->active_pipes) { + u32 pipe_val = val; + + if (DISPLAY_VER(i915) >= 14) { + if (xelpdp_is_only_pipe_per_dbuf_bank(crtc->pipe, + new_dbuf_state->active_pipes)) + pipe_val |= MBUS_DBOX_BW_8CREDITS_MTL; + else + pipe_val |= MBUS_DBOX_BW_4CREDITS_MTL; + } + + intel_de_write(i915, PIPE_MBUS_DBOX_CTL(crtc->pipe), pipe_val); + } +} + int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8 ratio) { struct intel_dbuf_state *dbuf_state; @@ -3657,89 +3740,6 @@ void intel_dbuf_post_plane_update(struct intel_atomic_state *state) new_dbuf_state->enabled_slices); } -static bool xelpdp_is_only_pipe_per_dbuf_bank(enum pipe pipe, u8 active_pipes) -{ - switch (pipe) { - case PIPE_A: - return !(active_pipes & BIT(PIPE_D)); - case PIPE_D: - return !(active_pipes & BIT(PIPE_A)); - case PIPE_B: - return !(active_pipes & BIT(PIPE_C)); - case PIPE_C: - return !(active_pipes & BIT(PIPE_B)); - default: /* to suppress compiler warning */ - MISSING_CASE(pipe); - break; - } - - return false; -} - -void intel_mbus_dbox_update(struct intel_atomic_state *state) -{ - struct drm_i915_private *i915 = to_i915(state->base.dev); - const struct intel_dbuf_state *new_dbuf_state, *old_dbuf_state; - const struct intel_crtc *crtc; - u32 val = 0; - - if
[PATCH v2 05/14] drm/i915: Loop over all active pipes in intel_mbus_dbox_update
From: Stanislav Lisovskiy We need to loop through all active pipes, not just the ones, that are in current state, because disabling and enabling even a particular pipe affects credits in another one. Reviewed-by: Uma Shankar Signed-off-by: Stanislav Lisovskiy Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/skl_watermark.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index bc341abcab2f..f582992592c1 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -3680,10 +3680,8 @@ void intel_mbus_dbox_update(struct intel_atomic_state *state) { struct drm_i915_private *i915 = to_i915(state->base.dev); const struct intel_dbuf_state *new_dbuf_state, *old_dbuf_state; - const struct intel_crtc_state *new_crtc_state; const struct intel_crtc *crtc; u32 val = 0; - int i; if (DISPLAY_VER(i915) < 11) return; @@ -3727,12 +3725,9 @@ void intel_mbus_dbox_update(struct intel_atomic_state *state) val |= MBUS_DBOX_B_CREDIT(8); } - for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { + for_each_intel_crtc_in_pipe_mask(>drm, crtc, new_dbuf_state->active_pipes) { u32 pipe_val = val; - if (!new_crtc_state->hw.active) - continue; - if (DISPLAY_VER(i915) >= 14) { if (xelpdp_is_only_pipe_per_dbuf_bank(crtc->pipe, new_dbuf_state->active_pipes)) -- 2.43.2
[PATCH v2 04/14] drm/i915/cdclk: Indicate whether CDCLK change happens during pre or post plane update
From: Ville Syrjälä Currently we just get a plain "Changing CDCLK to ..." in the logs. It would actually be interesting to see whether we're doing the programming during the pre or post plane phase of the commit. Include that information in the debug message. Reviewed-by: Uma Shankar Reviewed-by: Gustavo Sousa Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_cdclk.c | 19 ++- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 8d937e77f91a..ed8d9ee094b8 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2434,18 +2434,9 @@ static void intel_pcode_notify(struct drm_i915_private *i915, ret); } -/** - * intel_set_cdclk - Push the CDCLK configuration to the hardware - * @dev_priv: i915 device - * @cdclk_config: new CDCLK configuration - * @pipe: pipe with which to synchronize the update - * - * Program the hardware based on the passed in CDCLK state, - * if necessary. - */ static void intel_set_cdclk(struct drm_i915_private *dev_priv, const struct intel_cdclk_config *cdclk_config, - enum pipe pipe) + enum pipe pipe, const char *context) { struct intel_encoder *encoder; @@ -2455,7 +2446,7 @@ static void intel_set_cdclk(struct drm_i915_private *dev_priv, if (drm_WARN_ON_ONCE(_priv->drm, !dev_priv->display.funcs.cdclk->set_cdclk)) return; - intel_cdclk_dump_config(dev_priv, cdclk_config, "Changing CDCLK to"); + intel_cdclk_dump_config(dev_priv, cdclk_config, context); for_each_intel_encoder_with_psr(_priv->drm, encoder) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder); @@ -2628,7 +2619,8 @@ intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state) drm_WARN_ON(>drm, !new_cdclk_state->base.changed); - intel_set_cdclk(i915, _config, pipe); + intel_set_cdclk(i915, _config, pipe, + "Pre changing CDCLK to"); } /** @@ -2663,7 +2655,8 @@ intel_set_cdclk_post_plane_update(struct intel_atomic_state *state) drm_WARN_ON(>drm, !new_cdclk_state->base.changed); - intel_set_cdclk(i915, _cdclk_state->actual, pipe); + intel_set_cdclk(i915, _cdclk_state->actual, pipe, + "Post changing CDCLK to"); } static int intel_pixel_rate_to_cdclk(const struct intel_crtc_state *crtc_state) -- 2.43.2
[PATCH v2 03/14] drm/i915/cdclk: Drop tgl/dg2 cdclk bump hacks
From: Ville Syrjälä No one ever figured out why bumping the cdclk helped with whatever issue we were having at the time. Remove the hacks and start from scratch so that we can actually see if any problems still remain. Reviewed-by: Uma Shankar Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_cdclk.c | 19 --- 1 file changed, 19 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 131721b08819..8d937e77f91a 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2814,25 +2814,6 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) if (crtc_state->dsc.compression_enable) min_cdclk = max(min_cdclk, intel_vdsc_min_cdclk(crtc_state)); - /* -* HACK. Currently for TGL/DG2 platforms we calculate -* min_cdclk initially based on pixel_rate divided -* by 2, accounting for also plane requirements, -* however in some cases the lowest possible CDCLK -* doesn't work and causing the underruns. -* Explicitly stating here that this seems to be currently -* rather a Hack, than final solution. -*/ - if (IS_TIGERLAKE(dev_priv) || IS_DG2(dev_priv)) { - /* -* Clamp to max_cdclk_freq in case pixel rate is higher, -* in order not to break an 8K, but still leave W/A at place. -*/ - min_cdclk = max_t(int, min_cdclk, - min_t(int, crtc_state->pixel_rate, - dev_priv->display.cdclk.max_cdclk_freq)); - } - return min_cdclk; } -- 2.43.2
[PATCH v2 02/14] drm/i915/cdclk: Fix voltage_level programming edge case
From: Ville Syrjälä Currently we only consider the relationship of the old and new CDCLK frequencies when determining whether to do the repgramming from intel_set_cdclk_pre_plane_update() or intel_set_cdclk_post_plane_update(). It is technically possible to have a situation where the CDCLK frequency is decreasing, but the voltage_level is increasing due a DDI port. In this case we should bump the voltage level already in intel_set_cdclk_pre_plane_update() (so that the voltage_level will have been increased by the time the port gets enabled), while leaving the CDCLK frequency unchanged (as active planes/etc. may still depend on it). We can then reduce the CDCLK frequency to its final value from intel_set_cdclk_post_plane_update(). In order to handle that correctly we shall construct a suitable amalgam of the old and new cdclk states in intel_set_cdclk_pre_plane_update(). And we can simply call intel_set_cdclk() unconditionally in both places as it will not do anything if nothing actually changes vs. the current hw state. v2: Handle cdclk_state->disable_pipes v3: Only synchronize the cd2x update against the pipe's vblank when the cdclk frequency is changing during the current commit phase (Gustavo) Cc: Gustavo Sousa Reviewed-by: Uma Shankar Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_cdclk.c | 37 -- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 3a04061fb100..131721b08819 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2600,7 +2600,8 @@ intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state) intel_atomic_get_old_cdclk_state(state); const struct intel_cdclk_state *new_cdclk_state = intel_atomic_get_new_cdclk_state(state); - enum pipe pipe = new_cdclk_state->pipe; + struct intel_cdclk_config cdclk_config; + enum pipe pipe; if (!intel_cdclk_changed(_cdclk_state->actual, _cdclk_state->actual)) @@ -2609,12 +2610,25 @@ intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state) if (IS_DG2(i915)) intel_cdclk_pcode_pre_notify(state); - if (new_cdclk_state->disable_pipes || - old_cdclk_state->actual.cdclk <= new_cdclk_state->actual.cdclk) { - drm_WARN_ON(>drm, !new_cdclk_state->base.changed); + if (new_cdclk_state->disable_pipes) { + cdclk_config = new_cdclk_state->actual; + pipe = INVALID_PIPE; + } else { + if (new_cdclk_state->actual.cdclk >= old_cdclk_state->actual.cdclk) { + cdclk_config = new_cdclk_state->actual; + pipe = new_cdclk_state->pipe; + } else { + cdclk_config = old_cdclk_state->actual; + pipe = INVALID_PIPE; + } - intel_set_cdclk(i915, _cdclk_state->actual, pipe); + cdclk_config.voltage_level = max(new_cdclk_state->actual.voltage_level, + old_cdclk_state->actual.voltage_level); } + + drm_WARN_ON(>drm, !new_cdclk_state->base.changed); + + intel_set_cdclk(i915, _config, pipe); } /** @@ -2632,7 +2646,7 @@ intel_set_cdclk_post_plane_update(struct intel_atomic_state *state) intel_atomic_get_old_cdclk_state(state); const struct intel_cdclk_state *new_cdclk_state = intel_atomic_get_new_cdclk_state(state); - enum pipe pipe = new_cdclk_state->pipe; + enum pipe pipe; if (!intel_cdclk_changed(_cdclk_state->actual, _cdclk_state->actual)) @@ -2642,11 +2656,14 @@ intel_set_cdclk_post_plane_update(struct intel_atomic_state *state) intel_cdclk_pcode_post_notify(state); if (!new_cdclk_state->disable_pipes && - old_cdclk_state->actual.cdclk > new_cdclk_state->actual.cdclk) { - drm_WARN_ON(>drm, !new_cdclk_state->base.changed); + new_cdclk_state->actual.cdclk < old_cdclk_state->actual.cdclk) + pipe = new_cdclk_state->pipe; + else + pipe = INVALID_PIPE; - intel_set_cdclk(i915, _cdclk_state->actual, pipe); - } + drm_WARN_ON(>drm, !new_cdclk_state->base.changed); + + intel_set_cdclk(i915, _cdclk_state->actual, pipe); } static int intel_pixel_rate_to_cdclk(const struct intel_crtc_state *crtc_state) -- 2.43.2
[PATCH v2 01/14] drm/i915/cdclk: Fix CDCLK programming order when pipes are active
From: Ville Syrjälä Currently we always reprogram CDCLK from the intel_set_cdclk_pre_plane_update() when using squash/crawl. The code only works correctly for the cd2x update or full modeset cases, and it was simply never updated to deal with squash/crawl. If the CDCLK frequency is increasing we must reprogram it before we do anything else that might depend on the new higher frequency, and conversely we must not decrease the frequency until everything that might still depend on the old higher frequency has been dealt with. Since cdclk_state->pipe is only relevant when doing a cd2x update we can't use it to determine the correct sequence during squash/crawl. To that end introduce cdclk_state->disable_pipes which simply indicates that we must perform the update while the pipes are disable (ie. during intel_set_cdclk_pre_plane_update()). Otherwise we use the same old vs. new CDCLK frequency comparsiong as for cd2x updates. The only remaining problem case is when the voltage_level needs to increase due to a DDI port, but the CDCLK frequency is decreasing (and not all pipes are being disabled). The current approach will not bump the voltage level up until after the port has already been enabled, which is too late. But we'll take care of that case separately. v2: Don't break the "must disable pipes case" v3: Keep the on stack 'pipe' for future use Reviewed-by: Uma Shankar Reviewed-by: Gustavo Sousa Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_cdclk.c | 7 +-- drivers/gpu/drm/i915/display/intel_cdclk.h | 3 +++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 31aaa9780dfc..3a04061fb100 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2609,7 +2609,7 @@ intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state) if (IS_DG2(i915)) intel_cdclk_pcode_pre_notify(state); - if (pipe == INVALID_PIPE || + if (new_cdclk_state->disable_pipes || old_cdclk_state->actual.cdclk <= new_cdclk_state->actual.cdclk) { drm_WARN_ON(>drm, !new_cdclk_state->base.changed); @@ -2641,7 +2641,7 @@ intel_set_cdclk_post_plane_update(struct intel_atomic_state *state) if (IS_DG2(i915)) intel_cdclk_pcode_post_notify(state); - if (pipe != INVALID_PIPE && + if (!new_cdclk_state->disable_pipes && old_cdclk_state->actual.cdclk > new_cdclk_state->actual.cdclk) { drm_WARN_ON(>drm, !new_cdclk_state->base.changed); @@ -3124,6 +3124,7 @@ static struct intel_global_state *intel_cdclk_duplicate_state(struct intel_globa return NULL; cdclk_state->pipe = INVALID_PIPE; + cdclk_state->disable_pipes = false; return _state->base; } @@ -3316,6 +3317,8 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state) if (ret) return ret; + new_cdclk_state->disable_pipes = true; + drm_dbg_kms(_priv->drm, "Modeset required for cdclk change\n"); } diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h index bc8f86e292d8..2843fc091086 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.h +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h @@ -53,6 +53,9 @@ struct intel_cdclk_state { /* bitmask of active pipes */ u8 active_pipes; + + /* update cdclk with pipes disabled */ + bool disable_pipes; }; int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state); -- 2.43.2
[PATCH v2 00/14] drm/i915: Implemnt vblank sycnhronized mbus joining changes
From: Ville Syrjälä Get rid of the full modeset requirement for changing mbus joining. Things got quite a bit more complicated than originally envisioned due to the dynamic cdclk/mdclk ratio. Sadly we have to do a fairly careful dance between the dbuf and cdclk code to make sure everything is programmed in the correct sequence. Stan did the grunt work, but the sequence vs. cdclk updates was still not right so I finished that part. I also reorganized the code quite a bit to make the resulting patches more legible. And I tossed in more debugs and whatnot so we can actually observe what it's doing. Quickly smoke tested on tgl and adl, and things seem pretty decent. Unfortunately I don't have a LNL on me right now so I haven't fully tested the mdclk/cdclk ratio changes on real hw, but I did hack my adl to pretend that the ratio changes with cdclk and double checked that the logs look sensible for all the combinations of cdclk increase/decrease and mbus join enable/disable. So should work (tm) on real hw too. v2: Be more careful with the cd2x pipe select (Gustavo) Split the intel_cdclk_is_decreasing_later() stuff to a separate patch (Gustavo) Cc: Gustavo Sousa Cc: Uma Shankar Stanislav Lisovskiy (3): drm/i915: Loop over all active pipes in intel_mbus_dbox_update drm/i915: Use old mbus_join value when increasing CDCLK drm/i915: Implement vblank synchronized MBUS join changes Ville Syrjälä (11): drm/i915/cdclk: Fix CDCLK programming order when pipes are active drm/i915/cdclk: Fix voltage_level programming edge case drm/i915/cdclk: Drop tgl/dg2 cdclk bump hacks drm/i915/cdclk: Indicate whether CDCLK change happens during pre or post plane update drm/i915: Relocate intel_mbus_dbox_update() drm/i915: Extract intel_dbuf_mbus_join_update() drm/i915: Extract intel_dbuf_mdclk_min_tracker_update() drm/i915: Add debugs for mbus joining and dbuf ratio programming drm/i915: Use the correct mdclk/cdclk ratio in MBUS updates drm/i915: Use a plain old int for the cdclk/mdclk ratio drm/i915: Optimize out redundant dbuf slice updates drivers/gpu/drm/i915/display/intel_cdclk.c | 99 +++--- drivers/gpu/drm/i915/display/intel_cdclk.h | 8 +- drivers/gpu/drm/i915/display/intel_display.c | 5 +- drivers/gpu/drm/i915/display/skl_watermark.c | 344 --- drivers/gpu/drm/i915/display/skl_watermark.h | 9 +- 5 files changed, 284 insertions(+), 181 deletions(-) -- 2.43.2
Re: [PATCH 02/13] drm/i915/cdclk: Fix voltage_level programming edge case
On Fri, Mar 29, 2024 at 02:04:55PM -0300, Gustavo Sousa wrote: > Quoting Ville Syrjala (2024-03-27 14:45:33-03:00) > >From: Ville Syrjälä > > > >Currently we only consider the relationship of the > >old and new CDCLK frequencies when determining whether > >to do the repgramming from intel_set_cdclk_pre_plane_update() > >or intel_set_cdclk_post_plane_update(). > > > >It is technically possible to have a situation where the > >CDCLK frequency is decreasing, but the voltage_level is > >increasing due a DDI port. In this case we should bump > >the voltage level already in intel_set_cdclk_pre_plane_update() > >(so that the voltage_level will have been increased by the > >time the port gets enabled), while leaving the CDCLK frequency > >unchanged (as active planes/etc. may still depend on it). > >We can then reduce the CDCLK frequency to its final value > >from intel_set_cdclk_post_plane_update(). > > > >In order to handle that correctly we shall construct a > >suitable amalgam of the old and new cdclk states in > >intel_set_cdclk_pre_plane_update(). > > > >And we can simply call intel_set_cdclk() unconditionally > >in both places as it will not do anything if nothing actually > >changes vs. the current hw state. > > > >v2: Handle cdclk_state->disable_pipes > > > >Signed-off-by: Ville Syrjälä > >--- > > drivers/gpu/drm/i915/display/intel_cdclk.c | 27 +- > > 1 file changed, 16 insertions(+), 11 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > >b/drivers/gpu/drm/i915/display/intel_cdclk.c > >index 619529dba095..504c5cbbcfff 100644 > >--- a/drivers/gpu/drm/i915/display/intel_cdclk.c > >+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > >@@ -2600,6 +2600,7 @@ intel_set_cdclk_pre_plane_update(struct > >intel_atomic_state *state) > > intel_atomic_get_old_cdclk_state(state); > > const struct intel_cdclk_state *new_cdclk_state = > > intel_atomic_get_new_cdclk_state(state); > >+struct intel_cdclk_config cdclk_config; > > > > if (!intel_cdclk_changed(_cdclk_state->actual, > > _cdclk_state->actual)) > >@@ -2608,13 +2609,21 @@ intel_set_cdclk_pre_plane_update(struct > >intel_atomic_state *state) > > if (IS_DG2(i915)) > > intel_cdclk_pcode_pre_notify(state); > > > >-if (new_cdclk_state->disable_pipes || > >-old_cdclk_state->actual.cdclk <= new_cdclk_state->actual.cdclk) > >{ > >-drm_WARN_ON(>drm, !new_cdclk_state->base.changed); > >+if (new_cdclk_state->disable_pipes) { > >+cdclk_config = new_cdclk_state->actual; > >+} else { > >+if (new_cdclk_state->actual.cdclk >= > >old_cdclk_state->actual.cdclk) > >+cdclk_config = new_cdclk_state->actual; > >+else > >+cdclk_config = old_cdclk_state->actual; > > > >-intel_set_cdclk(i915, _cdclk_state->actual, > >-new_cdclk_state->pipe); > >+cdclk_config.voltage_level = > >max(new_cdclk_state->actual.voltage_level, > >+ > >old_cdclk_state->actual.voltage_level); > > } > >+ > >+drm_WARN_ON(>drm, !new_cdclk_state->base.changed); > >+ > >+intel_set_cdclk(i915, _config, new_cdclk_state->pipe); > > Not sure if there could be unwanted side effects with passing > new_cdclk_state->pipe when using old_cdclk_state->actual. Because > voltage_level might have changed, parts of the cdclk change sequence end > up being exercised even when cdclk_config == old_cdclk_state->actual. > > Well, even if those side effects might be harmless, I wonder if it would > be better if we used INVALID_PIPE when using old_cdclk_state->actual. Yeah. I doubt there should be any really bad side effects, but probably a good idea to sidestep the whole question by passing in INVALID_PIPE. -- Ville Syrjälä Intel
Re: [PATCH 12/13] drm/i915: Use a plain old int for the cdclk/mdclk ratio
On Fri, Mar 29, 2024 at 03:23:34PM -0300, Gustavo Sousa wrote: > Quoting Ville Syrjala (2024-03-27 14:45:43-03:00) > >From: Ville Syrjälä > > > >No point in throwing around u8 when we're dealing with > >just an integer. Use a plain old boring 'int'. > > Learned and noted :-) > > Thanks for fixing that. > > Should we also modify the member mdclk_cdclk_ratio of intel_dbuf_state? My rule of thumb is to prefer the smallest type when embedded in structures. Whether that's a good idea I'm not really sure. I suppose there is always some risk of forgeting to bump up the type size when needed. On the other hand we do have some absolutely massive structs (looking at you intel_crtc_state!) where trying to keep things small and optimally packed seems like a good idea. For the dbuf state it probably doesn't make a lick of difference. The other case is eg. big arrays/lists of structs where I think generally it's a good idea to minimize the size as there the overhead is potentially multiplied by the number of elements Otherwise these just waste unswappable kernel memory. I suppose one argument for using small types in all structures is consistency. People tend to cargo cult what they see so if existing code does it then new code should end up with the same approach. Though I suppose it might also work against us and trick people into also using the smaller types for function arguments and on stack variables as well. Apart from running out of bits, the main danger with the smaller types is C's integer promotion and arithmetic conversion rules. I feel most people don't generally know how those work. Eg. people see a u8 and assume everything to do with it is unsigned, and then they might get confused whether negative values are possible or not as a result of some artihmetic expression. So the safest type is pretty much the plain old 'int', and that is in fact what u8 & co. end up being due to the aforementioned language rules. tldr I don't really have a great answer here :/ > > In any case, > > Reviewed-by: Gustavo Sousa Thanks. > > > > >Signed-off-by: Ville Syrjälä > >--- > > drivers/gpu/drm/i915/display/intel_cdclk.c | 6 +++--- > > drivers/gpu/drm/i915/display/intel_cdclk.h | 4 ++-- > > drivers/gpu/drm/i915/display/skl_watermark.c | 6 -- > > drivers/gpu/drm/i915/display/skl_watermark.h | 6 -- > > 4 files changed, 13 insertions(+), 9 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > >b/drivers/gpu/drm/i915/display/intel_cdclk.c > >index 66c161d7b485..5cba0d08189b 100644 > >--- a/drivers/gpu/drm/i915/display/intel_cdclk.c > >+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > >@@ -1893,8 +1893,8 @@ static u32 xe2lpd_mdclk_source_sel(struct > >drm_i915_private *i915) > > return MDCLK_SOURCE_SEL_CD2XCLK; > > } > > > >-u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915, > >- const struct intel_cdclk_config *cdclk_config) > >+int intel_mdclk_cdclk_ratio(struct drm_i915_private *i915, > >+const struct intel_cdclk_config *cdclk_config) > > { > > if (mdclk_source_is_cdclk_pll(i915)) > > return DIV_ROUND_UP(cdclk_config->vco, cdclk_config->cdclk); > >@@ -3321,7 +3321,7 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state > >*state) > > > > if (intel_mdclk_cdclk_ratio(dev_priv, _cdclk_state->actual) != > > intel_mdclk_cdclk_ratio(dev_priv, _cdclk_state->actual)) { > >-u8 ratio = intel_mdclk_cdclk_ratio(dev_priv, > >_cdclk_state->actual); > >+int ratio = intel_mdclk_cdclk_ratio(dev_priv, > >_cdclk_state->actual); > > > > ret = intel_dbuf_state_set_mdclk_cdclk_ratio(state, ratio); > > if (ret) > >diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h > >b/drivers/gpu/drm/i915/display/intel_cdclk.h > >index 5d4faf401774..cfdcdec07a4d 100644 > >--- a/drivers/gpu/drm/i915/display/intel_cdclk.h > >+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h > >@@ -67,8 +67,8 @@ void intel_update_cdclk(struct drm_i915_private *dev_priv); > > u32 intel_read_rawclk(struct drm_i915_private *dev_priv); > > bool intel_cdclk_clock_changed(const struct intel_cdclk_config *a, > >const struct intel_cdclk_config *b); > >-u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915, > >- const struct intel_cdclk_config *cdclk_config); > >+int intel_mdclk_cdclk_ratio(struct drm_i915_private *i915, > >+const struct intel_cdclk_config *cdclk_config); > > bool intel_cdclk_is_decreasing_later(struct intel_atomic_state *state); > > void intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state); > > void intel_set_cdclk_post_plane_update(struct intel_atomic_state *state); > >diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c > >b/drivers/gpu/drm/i915/display/skl_watermark.c > >index ca0f1f89e6d9..1b48009efe2b 100644 > >---
Re: [PATCH v0 03/14] drm/gma500,drm/i915: Make I2C terminology more inclusive
On Tue, 02 Apr 2024, Easwar Hariharan wrote: > On 4/2/2024 12:48 AM, Jani Nikula wrote: >> On Fri, 29 Mar 2024, Easwar Hariharan wrote: >>> I2C v7, SMBus 3.2, and I3C specifications have replaced "master/slave" >>> with more appropriate terms. Inspired by and following on to Wolfram's >>> series to fix drivers/i2c/[1], fix the terminology for users of >>> I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists >>> in the specification. >> >> gma500 and i915 changes should be split. See MAINTAINERS. >> >> Might also split the i915 changes to smaller pieces, it's kind of >> random. And the changes here are not strictly related to I2C AFAICT, so >> the commit message should be updated. >> >> BR, >> Jani. >> >> > > > > I will split gma500 and i915 into their respective patches if possible in v2. > > Can you say more about the changes being "not strictly related to I2C"? My > heuristic was to grep for master/slave, and look in the surrounding context > for > i2c-related terminology (i2c_pin, 7-bit address, struct i2c_adapter, i2c_bus, > etc) > to confirm that they are i2c-related, then following the references around to > make the compiler happy. For e.g., I did not change the many references to > bigjoiner > master and slave because I understood from context they were not i2c > references. > > A couple examples would help me restrict the changes to I2C, since as > mentioned in the > discussion on Wolfram's thread, there are places where migrating away from > master/slave > terms in the code would conflict with the original technical manuals and > reduce correlation > and understanding of the code. I guess I was looking at the VBT changes in intel_bios.c. Granted, they do end up being used as i2c addresses. No big deal. I think I'd expect the treewide i2c adapter changes to land first, via i2c, and subsequent cleanups to happen next, via individual driver trees. There's quite a bit of conflict potential merging this outside of drm-intel-next, and there's really no need for that. BR, Jani. > > Thanks, > Easwar > -- Jani Nikula, Intel
Re: [PATCH 11/13] drm/i915: Implement vblank synchronized MBUS join changes
On Fri, Mar 29, 2024 at 03:15:02PM -0300, Gustavo Sousa wrote: > Quoting Ville Syrjala (2024-03-27 14:45:42-03:00) > >@@ -3663,24 +3659,42 @@ static void > >intel_dbuf_mdclk_min_tracker_update(struct intel_atomic_state *state > > intel_atomic_get_old_dbuf_state(state); > > const struct intel_dbuf_state *new_dbuf_state = > > intel_atomic_get_new_dbuf_state(state); > >+int mdclk_cdclk_ratio; > > > >-if (DISPLAY_VER(i915) >= 20 && > >-old_dbuf_state->mdclk_cdclk_ratio != > >new_dbuf_state->mdclk_cdclk_ratio) { > >-/* > >- * For Xe2LPD and beyond, when there is a change in the > >ratio > >- * between MDCLK and CDCLK, updates to related registers > >need to > >- * happen at a specific point in the CDCLK change sequence. > >In > >- * that case, we defer to the call to > >- * intel_dbuf_mdclk_cdclk_ratio_update() to the CDCLK logic. > >- */ > >-return; > >+if (intel_cdclk_is_decreasing_later(state)) { > >+/* cdclk/mdclk will be changed later by > >intel_set_cdclk_post_plane_update() */ > >+mdclk_cdclk_ratio = old_dbuf_state->mdclk_cdclk_ratio; > >+} else { > >+/* cdclk/mdclk already changed by > >intel_set_cdclk_pre_plane_update() */ > >+mdclk_cdclk_ratio = new_dbuf_state->mdclk_cdclk_ratio; > > } > > > >-intel_dbuf_mdclk_cdclk_ratio_update(i915, > >new_dbuf_state->mdclk_cdclk_ratio, > >+intel_dbuf_mdclk_cdclk_ratio_update(i915, mdclk_cdclk_ratio, > > new_dbuf_state->joined_mbus); > > I get the feeling that this part actually belongs to the previous patch. Hmm, right. In fact I think it can just be its own patch. I'll carve it out. -- Ville Syrjälä Intel
[PATCH v2] drm: ensure drm headers are self-contained and pass kernel-doc
Ensure drm headers build, are self-contained, have header guards, and have no kernel-doc warnings, when CONFIG_DRM_HEADER_TEST=y. The mechanism follows similar patters used in i915, xe, and usr/include. To cover include/drm, we need to recurse there using the top level Kbuild and the new include/Kbuild files. v2: make DRM_HEADER_TEST depend on DRM Suggested-by: Daniel Vetter Cc: David Airlie Cc: Daniel Vetter Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: Masahiro Yamada Acked-by: Thomas Zimmermann Signed-off-by: Jani Nikula --- Kbuild | 1 + drivers/gpu/drm/Kconfig | 11 +++ drivers/gpu/drm/Makefile | 18 ++ include/Kbuild | 1 + include/drm/Makefile | 18 ++ 5 files changed, 49 insertions(+) create mode 100644 include/Kbuild create mode 100644 include/drm/Makefile diff --git a/Kbuild b/Kbuild index 464b34a08f51..f327ca86990c 100644 --- a/Kbuild +++ b/Kbuild @@ -97,3 +97,4 @@ obj-$(CONFIG_SAMPLES) += samples/ obj-$(CONFIG_NET) += net/ obj-y += virt/ obj-y += $(ARCH_DRIVERS) +obj-$(CONFIG_DRM_HEADER_TEST) += include/ diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 3914aaf443a8..a388c4fda984 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -431,3 +431,14 @@ config DRM_WERROR this config option is disabled by default. If in doubt, say N. + +config DRM_HEADER_TEST + bool "Ensure DRM headers are self-contained and pass kernel-doc" + depends on DRM && EXPERT + default n + help + Ensure the DRM subsystem headers both under drivers/gpu/drm and + include/drm compile, are self-contained, have header guards, and have + no kernel-doc warnings. + + If in doubt, say N. diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index a73c04d2d7a3..6605d5686d01 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -218,3 +218,21 @@ obj-y += solomon/ obj-$(CONFIG_DRM_SPRD) += sprd/ obj-$(CONFIG_DRM_LOONGSON) += loongson/ obj-$(CONFIG_DRM_POWERVR) += imagination/ + +# Ensure drm headers are self-contained and pass kernel-doc +hdrtest-files := \ + $(shell cd $(srctree)/$(src) && find . -maxdepth 1 -name 'drm_*.h') \ + $(shell cd $(srctree)/$(src) && find display lib -name '*.h') + +always-$(CONFIG_DRM_HEADER_TEST) += \ + $(patsubst %.h,%.hdrtest, $(hdrtest-files)) + +# Include the header twice to detect missing include guard. +quiet_cmd_hdrtest = HDRTEST $(patsubst %.hdrtest,%.h,$@) + cmd_hdrtest = \ + $(CC) $(c_flags) -fsyntax-only -x c /dev/null -include $< -include $<; \ + $(srctree)/scripts/kernel-doc -none $(if $(CONFIG_DRM_WERROR),-Werror) $<; \ + touch $@ + +$(obj)/%.hdrtest: $(src)/%.h FORCE + $(call if_changed_dep,hdrtest) diff --git a/include/Kbuild b/include/Kbuild new file mode 100644 index ..5e76a599e2dd --- /dev/null +++ b/include/Kbuild @@ -0,0 +1 @@ +obj-$(CONFIG_DRM_HEADER_TEST) += drm/ diff --git a/include/drm/Makefile b/include/drm/Makefile new file mode 100644 index ..b9f391d7aadd --- /dev/null +++ b/include/drm/Makefile @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: GPL-2.0 + +# Ensure drm headers are self-contained and pass kernel-doc +hdrtest-files := \ + $(shell cd $(srctree)/$(src) && find * -name '*.h' 2>/dev/null) + +always-$(CONFIG_DRM_HEADER_TEST) += \ + $(patsubst %.h,%.hdrtest, $(hdrtest-files)) + +# Include the header twice to detect missing include guard. +quiet_cmd_hdrtest = HDRTEST $(patsubst %.hdrtest,%.h,$@) + cmd_hdrtest = \ + $(CC) $(c_flags) -fsyntax-only -x c /dev/null -include $< -include $<; \ + $(srctree)/scripts/kernel-doc -none $(if $(CONFIG_DRM_WERROR),-Werror) $<; \ + touch $@ + +$(obj)/%.hdrtest: $(src)/%.h FORCE + $(call if_changed_dep,hdrtest) -- 2.39.2
[PATCH 7/7] drm/i915: Use debugfs_create_bool() for "i915_bigjoiner_force_enable"
From: Ville Syrjälä There is no reason to make this debugfs file for a simple boolean so complicated. Just use debugfs_create_bool(). Reviewed-by: Arun R Murthy Signed-off-by: Ville Syrjälä --- .../drm/i915/display/intel_display_debugfs.c | 44 +-- 1 file changed, 2 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c index b99c024b0934..3e364891dcd0 100644 --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c @@ -1402,20 +1402,6 @@ out: drm_modeset_unlock(>drm.mode_config.connection_mutex); return ret; } -static int i915_bigjoiner_enable_show(struct seq_file *m, void *data) -{ - struct intel_connector *connector = m->private; - struct drm_crtc *crtc; - - crtc = connector->base.state->crtc; - if (connector->base.status != connector_status_connected || !crtc) - return -ENODEV; - - seq_printf(m, "Bigjoiner enable: %d\n", connector->force_bigjoiner_enable); - - return 0; -} - static ssize_t i915_dsc_output_format_write(struct file *file, const char __user *ubuf, size_t len, loff_t *offp) @@ -1437,30 +1423,6 @@ static ssize_t i915_dsc_output_format_write(struct file *file, return len; } -static ssize_t i915_bigjoiner_enable_write(struct file *file, - const char __user *ubuf, - size_t len, loff_t *offp) -{ - struct seq_file *m = file->private_data; - struct intel_connector *connector = m->private; - struct drm_crtc *crtc; - bool bigjoiner_en = 0; - int ret; - - crtc = connector->base.state->crtc; - if (connector->base.status != connector_status_connected || !crtc) - return -ENODEV; - - ret = kstrtobool_from_user(ubuf, len, _en); - if (ret < 0) - return ret; - - connector->force_bigjoiner_enable = bigjoiner_en; - *offp += len; - - return len; -} - static int i915_dsc_output_format_open(struct inode *inode, struct file *file) { @@ -1554,8 +1516,6 @@ static const struct file_operations i915_dsc_fractional_bpp_fops = { .write = i915_dsc_fractional_bpp_write }; -DEFINE_SHOW_STORE_ATTRIBUTE(i915_bigjoiner_enable); - /* * Returns the Current CRTC's bpc. * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc @@ -1640,8 +1600,8 @@ void intel_connector_debugfs_add(struct intel_connector *connector) if (DISPLAY_VER(i915) >= 11 && (connector_type == DRM_MODE_CONNECTOR_DisplayPort || connector_type == DRM_MODE_CONNECTOR_eDP)) { - debugfs_create_file("i915_bigjoiner_force_enable", 0644, root, - connector, _bigjoiner_enable_fops); + debugfs_create_bool("i915_bigjoiner_force_enable", 0644, root, + >force_bigjoiner_enable); } if (connector_type == DRM_MODE_CONNECTOR_DSI || -- 2.43.2
[PATCH 5/7] drm/i915/mst: Limit MST+DSC to TGL+
From: Ville Syrjälä The MST code currently assumes that glk+ already supports MST+DSC, which is incorrect. We need to check for TGL+ actually. ICL does support SST+DSC, but supposedly it can't do MST+FEC which will also rule MST+DSC. Note that a straight TGL+ check doesn't work here because DSC support can get fused out, so we do need to also check 'has_dsc'. Reviewed-by: Uma Shankar Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display_device.h | 1 + drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h index fe4268813786..9b1bce2624b9 100644 --- a/drivers/gpu/drm/i915/display/intel_display_device.h +++ b/drivers/gpu/drm/i915/display/intel_display_device.h @@ -47,6 +47,7 @@ struct drm_printer; #define HAS_DPT(i915) (DISPLAY_VER(i915) >= 13) #define HAS_DSB(i915) (DISPLAY_INFO(i915)->has_dsb) #define HAS_DSC(__i915) (DISPLAY_RUNTIME_INFO(__i915)->has_dsc) +#define HAS_DSC_MST(__i915)(DISPLAY_VER(__i915) >= 12 && HAS_DSC(__i915)) #define HAS_FBC(i915) (DISPLAY_RUNTIME_INFO(i915)->fbc_mask != 0) #define HAS_FPGA_DBG_UNCLAIMED(i915) (DISPLAY_INFO(i915)->has_fpga_dbg) #define HAS_FW_BLC(i915) (DISPLAY_VER(i915) >= 3) diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 1405ab5e3acc..6497542e3e65 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -1349,7 +1349,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector, return 0; } - if (DISPLAY_VER(dev_priv) >= 10 && + if (HAS_DSC_MST(dev_priv) && drm_dp_sink_supports_dsc(intel_connector->dp.dsc_dpcd)) { /* * TBD pass the connector BPC, -- 2.43.2
[PATCH 6/7] drm/i915/mst: Reject FEC+MST on ICL
From: Ville Syrjälä ICL supposedly doesn't support FEC on MST. Reject it. Reviewed-by: Uma Shankar Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_dp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 819f3234de03..a11bc4fd3d65 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -1418,7 +1418,8 @@ static bool intel_dp_source_supports_fec(struct intel_dp *intel_dp, if (DISPLAY_VER(dev_priv) >= 12) return true; - if (DISPLAY_VER(dev_priv) == 11 && encoder->port != PORT_A) + if (DISPLAY_VER(dev_priv) == 11 && encoder->port != PORT_A && + !intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST)) return true; return false; -- 2.43.2
[PATCH 2/7] drm/i915: Shuffle DP .mode_valid() checks
From: Ville Syrjälä Move some of the more trivial checks in the DP .mode_valid() hooks upwards to lessen the noise amongst the more complex checks. Reviewed-by: Vandita Kulkarni Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_dp.c | 6 +++--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 21 ++--- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index b393ddbb7b35..819f3234de03 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -1229,6 +1229,9 @@ intel_dp_mode_valid(struct drm_connector *_connector, if (mode->flags & DRM_MODE_FLAG_DBLCLK) return MODE_H_ILLEGAL; + if (mode->clock < 1) + return MODE_CLOCK_LOW; + fixed_mode = intel_panel_fixed_mode(connector, mode); if (intel_dp_is_edp(intel_dp) && fixed_mode) { status = intel_panel_mode_valid(connector, mode); @@ -1238,9 +1241,6 @@ intel_dp_mode_valid(struct drm_connector *_connector, target_clock = fixed_mode->clock; } - if (mode->clock < 1) - return MODE_CLOCK_LOW; - if (intel_dp_need_bigjoiner(intel_dp, mode->hdisplay, target_clock)) { bigjoiner = true; max_dotclk *= 2; diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 9a7c75039989..1405ab5e3acc 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -1302,6 +1302,16 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector, if (*status != MODE_OK) return 0; + if (mode->flags & DRM_MODE_FLAG_DBLCLK) { + *status = MODE_H_ILLEGAL; + return 0; + } + + if (mode->clock < 1) { + *status = MODE_CLOCK_LOW; + return 0; + } + max_link_clock = intel_dp_max_link_rate(intel_dp); max_lanes = intel_dp_max_lane_count(intel_dp); @@ -1330,17 +1340,6 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector, *status = MODE_CLOCK_HIGH; return 0; } - - if (mode->clock < 1) { - *status = MODE_CLOCK_LOW; - return 0; - } - - if (mode->flags & DRM_MODE_FLAG_DBLCLK) { - *status = MODE_H_ILLEGAL; - return 0; - } - if (intel_dp_need_bigjoiner(intel_dp, mode->hdisplay, target_clock)) { bigjoiner = true; max_dotclk *= 2; -- 2.43.2
[PATCH 4/7] drm/i915: Extract glk_need_scaler_clock_gating_wa()
From: Ville Syrjälä Simplify our life by extracting the "do we need the glk scaler clock gating w/a?" check into a small helper. Reviewed-by: Vandita Kulkarni Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 8f9d1a9caba2..02c377f61ed5 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -1551,6 +1551,14 @@ static void ilk_crtc_enable(struct intel_atomic_state *state, intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true); } +/* Display WA #1180: WaDisableScalarClockGating: glk */ +static bool glk_need_scaler_clock_gating_wa(const struct intel_crtc_state *crtc_state) +{ + struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev); + + return DISPLAY_VER(i915) == 10 && crtc_state->pch_pfit.enabled; +} + static void glk_pipe_scaler_clock_gating_wa(struct intel_crtc *crtc, bool enable) { struct drm_i915_private *i915 = to_i915(crtc->base.dev); @@ -1635,7 +1643,6 @@ static void hsw_crtc_enable(struct intel_atomic_state *state, struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); enum transcoder cpu_transcoder = new_crtc_state->cpu_transcoder; enum pipe hsw_workaround_pipe; - bool psl_clkgate_wa; if (drm_WARN_ON(_priv->drm, crtc->active)) return; @@ -1668,10 +1675,7 @@ static void hsw_crtc_enable(struct intel_atomic_state *state, crtc->active = true; - /* Display WA #1180: WaDisableScalarClockGating: glk */ - psl_clkgate_wa = DISPLAY_VER(dev_priv) == 10 && - new_crtc_state->pch_pfit.enabled; - if (psl_clkgate_wa) + if (glk_need_scaler_clock_gating_wa(new_crtc_state)) glk_pipe_scaler_clock_gating_wa(crtc, true); if (DISPLAY_VER(dev_priv) >= 9) @@ -1702,7 +1706,7 @@ static void hsw_crtc_enable(struct intel_atomic_state *state, intel_encoders_enable(state, crtc); - if (psl_clkgate_wa) { + if (glk_need_scaler_clock_gating_wa(new_crtc_state)) { intel_crtc_wait_for_next_vblank(crtc); glk_pipe_scaler_clock_gating_wa(crtc, false); } -- 2.43.2
[PATCH 3/7] drm/i915: Clean up glk_pipe_scaler_clock_gating_wa()
From: Ville Syrjälä glk_pipe_scaler_clock_gating_wa() is messy. Clean it up via intel_de_rmw(), and also just pass in the whole crtc so the caller doesn't have to dance around so much. Reviewed-by: Vandita Kulkarni Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 19 +++ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 614e60420a29..8f9d1a9caba2 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -1551,18 +1551,13 @@ static void ilk_crtc_enable(struct intel_atomic_state *state, intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true); } -static void glk_pipe_scaler_clock_gating_wa(struct drm_i915_private *dev_priv, - enum pipe pipe, bool apply) +static void glk_pipe_scaler_clock_gating_wa(struct intel_crtc *crtc, bool enable) { - u32 val = intel_de_read(dev_priv, CLKGATE_DIS_PSL(pipe)); + struct drm_i915_private *i915 = to_i915(crtc->base.dev); u32 mask = DPF_GATING_DIS | DPF_RAM_GATING_DIS | DPFR_GATING_DIS; - if (apply) - val |= mask; - else - val &= ~mask; - - intel_de_write(dev_priv, CLKGATE_DIS_PSL(pipe), val); + intel_de_rmw(i915, CLKGATE_DIS_PSL(crtc->pipe), +mask, enable ? mask : 0); } static void hsw_set_linetime_wm(const struct intel_crtc_state *crtc_state) @@ -1638,8 +1633,8 @@ static void hsw_crtc_enable(struct intel_atomic_state *state, const struct intel_crtc_state *new_crtc_state = intel_atomic_get_new_crtc_state(state, crtc); struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); - enum pipe pipe = crtc->pipe, hsw_workaround_pipe; enum transcoder cpu_transcoder = new_crtc_state->cpu_transcoder; + enum pipe hsw_workaround_pipe; bool psl_clkgate_wa; if (drm_WARN_ON(_priv->drm, crtc->active)) @@ -1677,7 +1672,7 @@ static void hsw_crtc_enable(struct intel_atomic_state *state, psl_clkgate_wa = DISPLAY_VER(dev_priv) == 10 && new_crtc_state->pch_pfit.enabled; if (psl_clkgate_wa) - glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true); + glk_pipe_scaler_clock_gating_wa(crtc, true); if (DISPLAY_VER(dev_priv) >= 9) skl_pfit_enable(new_crtc_state); @@ -1709,7 +1704,7 @@ static void hsw_crtc_enable(struct intel_atomic_state *state, if (psl_clkgate_wa) { intel_crtc_wait_for_next_vblank(crtc); - glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, false); + glk_pipe_scaler_clock_gating_wa(crtc, false); } /* If we change the relative order between pipe/planes enabling, we need -- 2.43.2
[PATCH 0/7] drm/i915: Bigjoiner prep work
From: Ville Syrjälä An extract of the large bigjoiner series, containing: - the pure refactoring patches not directly related to bigjoiner - Reject MST+DSC/FEC on ICL - tweak the debugfs i915_bigjoiner_force_enable file (needs corresponding IGT changes). Test-with: 20240402054231.1499492-1-kunal1.jo...@intel.com Ville Syrjälä (7): drm/i915: Remove DRM_MODE_FLAG_DBLSCAN checks from .mode_valid() hooks drm/i915: Shuffle DP .mode_valid() checks drm/i915: Clean up glk_pipe_scaler_clock_gating_wa() drm/i915: Extract glk_need_scaler_clock_gating_wa() drm/i915/mst: Limit MST+DSC to TGL+ drm/i915/mst: Reject FEC+MST on ICL drm/i915: Use debugfs_create_bool() for "i915_bigjoiner_force_enable" drivers/gpu/drm/i915/display/intel_crt.c | 3 -- drivers/gpu/drm/i915/display/intel_display.c | 35 +++ .../drm/i915/display/intel_display_debugfs.c | 44 +-- .../drm/i915/display/intel_display_device.h | 1 + drivers/gpu/drm/i915/display/intel_dp.c | 9 ++-- drivers/gpu/drm/i915/display/intel_dp_mst.c | 22 -- drivers/gpu/drm/i915/display/intel_dsi.c | 3 -- drivers/gpu/drm/i915/display/intel_dvo.c | 3 -- drivers/gpu/drm/i915/display/intel_lvds.c | 3 -- drivers/gpu/drm/i915/display/intel_sdvo.c | 3 -- drivers/gpu/drm/i915/display/intel_tv.c | 3 -- 11 files changed, 33 insertions(+), 96 deletions(-) -- 2.43.2
[PATCH 1/7] drm/i915: Remove DRM_MODE_FLAG_DBLSCAN checks from .mode_valid() hooks
From: Ville Syrjälä We never set connector->doublescan_allowed, so the probe helper already filters out all doublescan modes for us. Sadly we still need to keep the explicit doublescan checks in .compute_config as outlined in commit e4dd27aadd20 ("drm/i915: Allow DBLSCAN user modes with eDP/LVDS/DSI") Reviewed-by: Vandita Kulkarni Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_crt.c| 3 --- drivers/gpu/drm/i915/display/intel_dp_mst.c | 5 - drivers/gpu/drm/i915/display/intel_dsi.c| 3 --- drivers/gpu/drm/i915/display/intel_dvo.c| 3 --- drivers/gpu/drm/i915/display/intel_lvds.c | 3 --- drivers/gpu/drm/i915/display/intel_sdvo.c | 3 --- drivers/gpu/drm/i915/display/intel_tv.c | 3 --- 7 files changed, 23 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c index 93479db0f89f..2e95093aa4d4 100644 --- a/drivers/gpu/drm/i915/display/intel_crt.c +++ b/drivers/gpu/drm/i915/display/intel_crt.c @@ -356,9 +356,6 @@ intel_crt_mode_valid(struct drm_connector *connector, if (status != MODE_OK) return status; - if (mode->flags & DRM_MODE_FLAG_DBLSCAN) - return MODE_NO_DBLESCAN; - if (mode->clock < 25000) return MODE_CLOCK_LOW; diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 53aec023ce92..9a7c75039989 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -1302,11 +1302,6 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector, if (*status != MODE_OK) return 0; - if (mode->flags & DRM_MODE_FLAG_DBLSCAN) { - *status = MODE_NO_DBLESCAN; - return 0; - } - max_link_clock = intel_dp_max_link_rate(intel_dp); max_lanes = intel_dp_max_lane_count(intel_dp); diff --git a/drivers/gpu/drm/i915/display/intel_dsi.c b/drivers/gpu/drm/i915/display/intel_dsi.c index d3cf6a652221..2dfc60e4b615 100644 --- a/drivers/gpu/drm/i915/display/intel_dsi.c +++ b/drivers/gpu/drm/i915/display/intel_dsi.c @@ -69,9 +69,6 @@ enum drm_mode_status intel_dsi_mode_valid(struct drm_connector *connector, drm_dbg_kms(_priv->drm, "\n"); - if (mode->flags & DRM_MODE_FLAG_DBLSCAN) - return MODE_NO_DBLESCAN; - status = intel_panel_mode_valid(intel_connector, mode); if (status != MODE_OK) return status; diff --git a/drivers/gpu/drm/i915/display/intel_dvo.c b/drivers/gpu/drm/i915/display/intel_dvo.c index c076da75b066..060328c0df7e 100644 --- a/drivers/gpu/drm/i915/display/intel_dvo.c +++ b/drivers/gpu/drm/i915/display/intel_dvo.c @@ -231,9 +231,6 @@ intel_dvo_mode_valid(struct drm_connector *_connector, if (status != MODE_OK) return status; - if (mode->flags & DRM_MODE_FLAG_DBLSCAN) - return MODE_NO_DBLESCAN; - /* XXX: Validate clock range */ if (fixed_mode) { diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c index 221f5c6c871b..24860945f2e4 100644 --- a/drivers/gpu/drm/i915/display/intel_lvds.c +++ b/drivers/gpu/drm/i915/display/intel_lvds.c @@ -399,9 +399,6 @@ intel_lvds_mode_valid(struct drm_connector *_connector, if (status != MODE_OK) return status; - if (mode->flags & DRM_MODE_FLAG_DBLSCAN) - return MODE_NO_DBLESCAN; - status = intel_panel_mode_valid(connector, mode); if (status != MODE_OK) return status; diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c index 50f0557d9ca2..df76044a739a 100644 --- a/drivers/gpu/drm/i915/display/intel_sdvo.c +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c @@ -1952,9 +1952,6 @@ intel_sdvo_mode_valid(struct drm_connector *connector, if (status != MODE_OK) return status; - if (mode->flags & DRM_MODE_FLAG_DBLSCAN) - return MODE_NO_DBLESCAN; - if (clock > max_dotclk) return MODE_CLOCK_HIGH; diff --git a/drivers/gpu/drm/i915/display/intel_tv.c b/drivers/gpu/drm/i915/display/intel_tv.c index ba5d2b7174b7..79d35c1b3c81 100644 --- a/drivers/gpu/drm/i915/display/intel_tv.c +++ b/drivers/gpu/drm/i915/display/intel_tv.c @@ -969,9 +969,6 @@ intel_tv_mode_valid(struct drm_connector *connector, if (status != MODE_OK) return status; - if (mode->flags & DRM_MODE_FLAG_DBLSCAN) - return MODE_NO_DBLESCAN; - if (mode->clock > max_dotclk) return MODE_CLOCK_HIGH; -- 2.43.2
Re: [PATCH 05/11] drm/i915/dp_mst: Account with the DSC DPT bpp limit on MTL
On Fri, Mar 29, 2024 at 11:39:39AM -0700, Manasi Navare wrote: > Hi Imre, > > While we are adding these checks here for DSC for MST, I see that in > intel_dp_mst_mode_valid_ctx() we still check against DISPLAY_VER() > > 10 for checking for DSC where as in all other places we rely on > runtime has_dsc and check for HAS_DSC(), can we fix that and use > HAS_DSC() in this function as well as part of this series that in > general fixes some DSC issues? The check in intel_dp_mst_check_constraints() this patch changes is not about whether a platform supports DSC or not, rather if the platform has a DSC/DPT interface limit that needs to be checked. The caller has already determined that DSC is supported by the platform and it's needed for the given mode being computed (dsc==true). > > Manasi > > On Tue, Mar 26, 2024 at 5:59 AM Nautiyal, Ankit K > wrote: > > > > > > On 3/26/2024 5:41 PM, Imre Deak wrote: > > > On Tue, Mar 26, 2024 at 03:47:05PM +0530, Nautiyal, Ankit K wrote: > > >> On 3/21/2024 1:41 AM, Imre Deak wrote: > > >>> The DPT/DSC bpp limit should be accounted for on MTL platforms as well, > > >>> do so. > > >>> > > >>> Bspec: 49259 > > >>> > > >>> Signed-off-by: Imre Deak > > >>> --- > > >>>drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +- > > >>>1 file changed, 1 insertion(+), 1 deletion(-) > > >>> > > >>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > >>> b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > >>> index 79f34be5c89da..40660dc5edb45 100644 > > >>> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > >>> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > >>> @@ -56,7 +56,7 @@ static int intel_dp_mst_check_constraints(struct > > >>> drm_i915_private *i915, int bpp > > >>> struct intel_crtc_state > > >>> *crtc_state, > > >>> bool dsc) > > >>>{ > > >>> - if (intel_dp_is_uhbr(crtc_state) && DISPLAY_VER(i915) < 14 && dsc) { > > >> Should this be DISPLAY_VER() <= 14 to include MTL? > > > The actual change is the DISPLAY_VER() < 20 below, which is the usual > > > way in the driver (AFAIU) to check for an upper bound. > > > > Makes sense. > > > > > > > >> For DISPLAY_VER 20, is there another check? > > >> > > >> in Bspec:68912 it mentions output bpp * pixel clock < DDICLK * 144 bits > > > Yes LNL is different, but there this DPT limit should never be a > > > bottleneck. Ville has an idea to abstract this more, but this patchset > > > keeps things as-is, skipping the check on LNL+. > > > > Agreed. Bspec indeed mentions the same thing, and its mentioned > > appropriately in the next patch. > > > > Regards, > > > > Ankit > > > > > > > >> Regards, > > >> > > >> Ankit > > >> > > >>> + if (intel_dp_is_uhbr(crtc_state) && DISPLAY_VER(i915) < 20 && dsc) { > > >>> int output_bpp = bpp; > > >>> int symbol_clock = > > >>> intel_dp_link_symbol_clock(crtc_state->port_clock); > > >>> int available_bw = mul_u32_u32(symbol_clock * 72,
✗ Fi.CI.BAT: failure for Panel replay selective update support (rev4)
== Series Details == Series: Panel replay selective update support (rev4) URL : https://patchwork.freedesktop.org/series/128193/ State : failure == Summary == CI Bug Log - changes from CI_DRM_14515 -> Patchwork_128193v4 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_128193v4 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_128193v4, please notify your bug team (i915-ci-in...@lists.freedesktop.org) to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v4/index.html Participating hosts (33 -> 31) -- Additional (2): bat-kbl-2 bat-arls-1 Missing(4): bat-mtlp-8 fi-blb-e6850 fi-cfl-8109u fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_128193v4: ### IGT changes ### Possible regressions * igt@i915_selftest@live@slpc: - bat-arls-1: NOTRUN -> [DMESG-WARN][1] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v4/bat-arls-1/igt@i915_selftest@l...@slpc.html * igt@kms_psr@psr-cursor-plane-move@edp-1: - bat-adln-1: [PASS][2] -> [FAIL][3] +3 other tests fail [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14515/bat-adln-1/igt@kms_psr@psr-cursor-plane-m...@edp-1.html [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v4/bat-adln-1/igt@kms_psr@psr-cursor-plane-m...@edp-1.html - bat-jsl-1: [PASS][4] -> [FAIL][5] +3 other tests fail [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14515/bat-jsl-1/igt@kms_psr@psr-cursor-plane-m...@edp-1.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v4/bat-jsl-1/igt@kms_psr@psr-cursor-plane-m...@edp-1.html * igt@kms_psr@psr-primary-page-flip@edp-1: - bat-jsl-3: [PASS][6] -> [FAIL][7] +3 other tests fail [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14515/bat-jsl-3/igt@kms_psr@psr-primary-page-f...@edp-1.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v4/bat-jsl-3/igt@kms_psr@psr-primary-page-f...@edp-1.html Known issues Here are the changes found in Patchwork_128193v4 that come from known issues: ### IGT changes ### Issues hit * igt@debugfs_test@basic-hwmon: - bat-arls-1: NOTRUN -> [SKIP][8] ([i915#9318]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v4/bat-arls-1/igt@debugfs_t...@basic-hwmon.html * igt@fbdev@info: - bat-kbl-2: NOTRUN -> [SKIP][9] ([i915#1849]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v4/bat-kbl-2/igt@fb...@info.html * igt@gem_lmem_swapping@parallel-random-engines: - bat-kbl-2: NOTRUN -> [SKIP][10] +42 other tests skip [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v4/bat-kbl-2/igt@gem_lmem_swapp...@parallel-random-engines.html * igt@gem_lmem_swapping@random-engines: - bat-arls-1: NOTRUN -> [SKIP][11] ([i915#10213]) +3 other tests skip [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v4/bat-arls-1/igt@gem_lmem_swapp...@random-engines.html * igt@gem_mmap@basic: - bat-arls-1: NOTRUN -> [SKIP][12] ([i915#4083]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v4/bat-arls-1/igt@gem_m...@basic.html * igt@gem_render_tiled_blits@basic: - bat-arls-1: NOTRUN -> [SKIP][13] ([i915#10197] / [i915#10211] / [i915#4079]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v4/bat-arls-1/igt@gem_render_tiled_bl...@basic.html * igt@gem_tiled_blits@basic: - bat-arls-1: NOTRUN -> [SKIP][14] ([i915#10196] / [i915#4077]) +2 other tests skip [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v4/bat-arls-1/igt@gem_tiled_bl...@basic.html * igt@gem_tiled_pread_basic: - bat-arls-1: NOTRUN -> [SKIP][15] ([i915#10206] / [i915#4079]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v4/bat-arls-1/igt@gem_tiled_pread_basic.html * igt@i915_pm_rps@basic-api: - bat-arls-1: NOTRUN -> [SKIP][16] ([i915#10209]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v4/bat-arls-1/igt@i915_pm_...@basic-api.html * igt@i915_selftest@live@gt_lrc: - bat-dg1-7: [PASS][17] -> [ABORT][18] ([i915#9413]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14515/bat-dg1-7/igt@i915_selftest@live@gt_lrc.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v4/bat-dg1-7/igt@i915_selftest@live@gt_lrc.html * igt@i915_selftest@live@guc_multi_lrc: - bat-arls-1: NOTRUN -> [DMESG-FAIL][19] ([i915#10262]) +3 other tests dmesg-fail [19]:
✗ Fi.CI.SPARSE: warning for Panel replay selective update support (rev4)
== Series Details == Series: Panel replay selective update support (rev4) URL : https://patchwork.freedesktop.org/series/128193/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
✗ Fi.CI.CHECKPATCH: warning for Panel replay selective update support (rev4)
== Series Details == Series: Panel replay selective update support (rev4) URL : https://patchwork.freedesktop.org/series/128193/ State : warning == Summary == Error: dim checkpatch failed a184a7194320 drm/i915/psr: Add some documentation of variables used in psr code 12561b72f253 drm/i915/psr: Set intel_crtc_state->has_psr on panel replay as well -:44: CHECK:MULTIPLE_ASSIGNMENTS: multiple assignments should be avoided #44: FILE: drivers/gpu/drm/i915/display/intel_psr.c:1636: + pipe_config->has_psr = pipe_config->has_panel_replay = true; total: 0 errors, 0 warnings, 1 checks, 30 lines checked 7304d962caba drm/i915/psr: Intel_psr_pause/resume needs to support panel replay dd7d076b1442 drm/i915/psr: Do not update phy power state in case of non-eDP panel replay 0208990060a7 drm/i915/psr: Check possible errors for panel replay as well cd41d72b9856 drm/i915/psr: Do not write registers/bits not applicable for panel replay c25e2d87c349 drm/i915/psr: Call intel_psr_init_dpcd in intel_dp_detect 69422ffa4fcb drm/i915/psr: Unify panel replay enable/disable sink f7a49ff77619 drm/i915/psr: Panel replay has to be enabled before link training -:80: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'intel_dp' - possible side-effects? #80: FILE: drivers/gpu/drm/i915/display/intel_psr.h:24: +#define CAN_PANEL_REPLAY(intel_dp) ((intel_dp)->psr.sink_panel_replay_support && \ + (intel_dp)->psr.source_panel_replay_support) total: 0 errors, 0 warnings, 1 checks, 50 lines checked 266b67bb093b drm/i915/psr: Rename has_psr2 as has_sel_update -:31: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis #31: FILE: drivers/gpu/drm/i915/display/intel_crtc_state_dump.c:255: + drm_printf(, "psr: %s, selective update: %s, panel replay: %s, selective fetch: %s\n", + str_enabled_disabled(pipe_config->has_psr), total: 0 errors, 0 warnings, 1 checks, 88 lines checked bebf3b6bad60 drm/i915/psr: Rename psr2_enabled as sel_update_enabled 02e2a904e89c drm/panelreplay: dpcd register definition for panelreplay SU 4cb79990b2d9 drm/i915/psr: Detect panel replay selective update support dec44ca620d4 drm/i915/psr: Modify intel_dp_get_su_granularity to support panel replay 187e243373bd drm/i915/psr: Panel replay uses SRD_STATUS to track it's status 392ed6e2b0c4 drm/i915/psr: Do not apply workarounds in case of panel replay 672703b16872 drm/i915/psr: Update PSR module parameter descriptions c43f152fc02b drm/i915/psr: Split intel_psr2_config_valid for panel replay 97053ae0c227 drm/i915/psr: Add panel replay sel update support to debugfs interface -:13: WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #13: Sink support: PSR = no, Panel Replay = yes, Panel Replay Selective Update = yes total: 0 errors, 1 warnings, 0 checks, 22 lines checked
Intel Gfx Kernel CI downtime 12.04-15.04
Hello, There is an upcoming planned shutdown in Intel labs due to mandatory building maintenance. We will disable CI pipelines 8am CEST April 12th and enable them back as soon as possible on Monday, April 15th. Please plan your work accordingly :) Thanks, Ryszard
Re: [PATCH v3 04/21] drm/i915/psr: Rename intel_psr_enabled
On Mon, 2024-02-05 at 04:50 +, Manna, Animesh wrote: > > > > -Original Message- > > From: Hogander, Jouni > > Sent: Friday, February 2, 2024 1:17 PM > > To: Manna, Animesh ; intel- > > g...@lists.freedesktop.org > > Subject: Re: [PATCH v3 04/21] drm/i915/psr: Rename > > intel_psr_enabled > > > > On Fri, 2024-02-02 at 07:34 +, Manna, Animesh wrote: > > > > > > > > > > -Original Message- > > > > From: Hogander, Jouni > > > > Sent: Friday, January 19, 2024 3:40 PM > > > > To: intel-gfx@lists.freedesktop.org > > > > Cc: Manna, Animesh ; Hogander, Jouni > > > > > > > > Subject: [PATCH v3 04/21] drm/i915/psr: Rename > > > > intel_psr_enabled > > > > > > > > Intel_psr_enabled is now misleading name as we are using main > > > > link > > > > on with panel replay. I.e. link retraining is not controlled by > > > > hardware. > > > > Rename > > > > intel_psr_enabled as intel_psr_hw_controls_link_retrain. > > > > > > > > Signed-off-by: Jouni Högander > > > > --- > > > > drivers/gpu/drm/i915/display/intel_dp.c | 2 +- > > > > drivers/gpu/drm/i915/display/intel_psr.c | 2 +- > > > > drivers/gpu/drm/i915/display/intel_psr.h | 2 +- > > > > 3 files changed, 3 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > > > b/drivers/gpu/drm/i915/display/intel_dp.c > > > > index ab415f41924d..e7cda3162ea2 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > > @@ -4951,7 +4951,7 @@ intel_dp_needs_link_retrain(struct > > > > intel_dp > > > > *intel_dp) > > > > * Also when exiting PSR, HW will retrain the link > > > > anyways > > > > fixing > > > > * any link status error. > > > > */ > > > > - if (intel_psr_enabled(intel_dp)) > > > > + if (intel_psr_hw_controls_link_retrain(intel_dp)) > > > > return false; > > > > > > > > if (drm_dp_dpcd_read_phy_link_status(_dp->aux, > > > > DP_PHY_DPRX, diff --git > > > > a/drivers/gpu/drm/i915/display/intel_psr.c > > > > b/drivers/gpu/drm/i915/display/intel_psr.c > > > > index d11f8ea6e0a9..7b3290f4e0b4 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > > @@ -3011,7 +3011,7 @@ void intel_psr_short_pulse(struct > > > > intel_dp > > > > *intel_dp) > > > > mutex_unlock(>lock); > > > > } > > > > > > > > -bool intel_psr_enabled(struct intel_dp *intel_dp) > > > > +bool intel_psr_hw_controls_link_retrain(struct intel_dp > > > > *intel_dp) > > > > > > Based on CAN_PSR() check the function will return early and only > > > get > > > executed for psr. No sure still do we need to rename it? > > > > Ok. For me it was just surprice what it does and why this function > > exists, thus > > renaming. Much more descriptive. Also we will soon have main link > > off with > > Panel Replay as well then this is not about having PSR or Panel > > Replay > > enabled, but HW controlling link retraining. > > > > I'm fine with dropping the patch if you have strong opinion on > > this. > > Do not see any value addition, though no strong objection. I sent updated version of this set. I have addressed some of your comments there including this on. Can you please recheck my patches. BR, Jouni Högander > > Regards, > Animesh > > > > BR, > > > > Jouni Högander > > > > > > > > Regards, > > > Animesh > > > > { > > > > bool ret; > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h > > > > b/drivers/gpu/drm/i915/display/intel_psr.h > > > > index cde781df84d5..f7c5cc07864f 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.h > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.h > > > > @@ -45,7 +45,7 @@ void intel_psr_get_config(struct > > > > intel_encoder > > > > *encoder, void intel_psr_irq_handler(struct intel_dp > > > > *intel_dp, > > > > u32 psr_iir); > > > > void intel_psr_short_pulse(struct intel_dp *intel_dp); void > > > > intel_psr_wait_for_idle_locked(const struct intel_crtc_state > > > > *new_crtc_state); -bool intel_psr_enabled(struct intel_dp > > > > *intel_dp); > > > > +bool intel_psr_hw_controls_link_retrain(struct intel_dp > > > > *intel_dp); > > > > int intel_psr2_sel_fetch_update(struct intel_atomic_state > > > > *state, > > > > struct intel_crtc *crtc); > > > > void intel_psr2_program_trans_man_trk_ctl(const struct > > > > intel_crtc_state *crtc_state); > > > > -- > > > > 2.34.1 > > > >
[PATCH v4 17/19] drm/i915/psr: Update PSR module parameter descriptions
We are re-using PSR module parameters for panel replay. Update module parameter descriptions with panel replay information: enable_psr: -1 (default) == follow what is in VBT 0 == disable PSR/PR 1 == Allow PSR1 and PR full frame update 2 == allow PSR1/PSR2 and PR Selective Update enable_psr2_sel_fetch 0 == disable selective fetch for PSR and PR 1 (default) == allow selective fetch for PSR PR Signed-off-by: Jouni Högander Reviewed-by: Animesh Manna --- drivers/gpu/drm/i915/display/intel_display_params.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_params.c b/drivers/gpu/drm/i915/display/intel_display_params.c index c8e3d6892e23..b798933683be 100644 --- a/drivers/gpu/drm/i915/display/intel_display_params.c +++ b/drivers/gpu/drm/i915/display/intel_display_params.c @@ -105,7 +105,8 @@ intel_display_param_named_unsafe(enable_fbc, int, 0400, intel_display_param_named_unsafe(enable_psr, int, 0400, "Enable PSR " - "(0=disabled, 1=enable up to PSR1, 2=enable up to PSR2) " + "(0=disabled, 1=enable up to PSR1 and Panel Replay full frame update, " + "2=enable up to PSR2 and Panel Replay Selective Update) " "Default: -1 (use per-chip default)"); intel_display_param_named(psr_safest_params, bool, 0400, @@ -115,7 +116,7 @@ intel_display_param_named(psr_safest_params, bool, 0400, "Default: 0"); intel_display_param_named_unsafe(enable_psr2_sel_fetch, bool, 0400, - "Enable PSR2 selective fetch " + "Enable PSR2 and Panel Replay selective fetch " "(0=disabled, 1=enabled) " "Default: 1"); -- 2.34.1
[PATCH v4 19/19] drm/i915/psr: Add panel replay sel update support to debugfs interface
Add panel replay selective update support to debugfs status interface. In case of sink supporting panel replay we will print out: Sink support: PSR = no, Panel Replay = yes, Panel Replay Selective Update = yes and PSR mode will look like this if printing out enabled panel replay selective update: PSR mode: Panel Replay Selective Update Enabled Current PSR and panel replay printouts remain same. Cc: Kunal Joshi Signed-off-by: Jouni Högander --- drivers/gpu/drm/i915/display/intel_psr.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 41c54009196f..37c7e4d9353c 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -3564,7 +3564,9 @@ static int intel_psr_status(struct seq_file *m, struct intel_dp *intel_dp) if (psr->sink_support) seq_printf(m, " [0x%02x]", intel_dp->psr_dpcd[0]); - seq_printf(m, ", Panel Replay = %s\n", str_yes_no(psr->sink_panel_replay_support)); + seq_printf(m, ", Panel Replay = %s", str_yes_no(psr->sink_panel_replay_support)); + seq_printf(m, ", Panel Replay Selective Update = %s\n", + str_yes_no(psr->sink_panel_replay_su_support)); if (!(psr->sink_support || psr->sink_panel_replay_support)) return 0; @@ -3573,9 +3575,10 @@ static int intel_psr_status(struct seq_file *m, struct intel_dp *intel_dp) mutex_lock(>lock); if (psr->panel_replay_enabled) - status = "Panel Replay Enabled"; + status = psr->sel_update_enabled ? "Panel Replay Selective Update Enabled" : + "Panel Replay Enabled"; else if (psr->enabled) - status = psr->sel_update_enabled ? "PSR2 enabled" : "PSR1 enabled"; + status = psr->sel_update_enabled ? "PSR2" : "PSR1"; else status = "disabled"; seq_printf(m, "PSR mode: %s\n", status); -- 2.34.1
[PATCH v4 18/19] drm/i915/psr: Split intel_psr2_config_valid for panel replay
Part of intel_psr2_config_valid is valid for panel replay. rename it as intel_sel_update_config_valid. Split psr2 specific part and name it as intel_psr2_config_valid. v3: - move early transport check to psr2 specific check - check intel_psr2_config_valid only for non-Panel Replay case v2: - use psr2_global_enabled for panel replay as well - goto unsupported instead of return when global enabled check fails Signed-off-by: Jouni Högander --- drivers/gpu/drm/i915/display/intel_psr.c | 76 ++-- 1 file changed, 46 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 3879bdbad6fd..41c54009196f 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -1142,9 +1142,6 @@ static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp, return false; } - if (psr2_su_region_et_valid(intel_dp)) - crtc_state->enable_psr2_su_region_et = true; - return crtc_state->enable_psr2_sel_fetch = true; } @@ -1515,11 +1512,6 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp, return false; } - if (!psr2_global_enabled(intel_dp)) { - drm_dbg_kms(_priv->drm, "PSR2 disabled by flag\n"); - return false; - } - /* * DSC and PSR2 cannot be enabled simultaneously. If a requested * resolution requires DSC to be enabled, priority is given to DSC @@ -1532,12 +1524,6 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp, return false; } - if (crtc_state->crc_enabled) { - drm_dbg_kms(_priv->drm, - "PSR2 not enabled because it would inhibit pipe CRC calculation\n"); - return false; - } - if (DISPLAY_VER(dev_priv) >= 12) { psr_max_h = 5120; psr_max_v = 3200; @@ -1588,30 +1574,60 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp, return false; } - if (HAS_PSR2_SEL_FETCH(dev_priv)) { - if (!intel_psr2_sel_fetch_config_valid(intel_dp, crtc_state) && - !HAS_PSR_HW_TRACKING(dev_priv)) { - drm_dbg_kms(_priv->drm, - "PSR2 not enabled, selective fetch not valid and no HW tracking available\n"); - return false; - } - } - - if (!psr2_granularity_check(intel_dp, crtc_state)) { - drm_dbg_kms(_priv->drm, "PSR2 not enabled, SU granularity not compatible\n"); - goto unsupported; - } - if (!crtc_state->enable_psr2_sel_fetch && (crtc_hdisplay > psr_max_h || crtc_vdisplay > psr_max_v)) { drm_dbg_kms(_priv->drm, "PSR2 not enabled, resolution %dx%d > max supported %dx%d\n", crtc_hdisplay, crtc_vdisplay, psr_max_h, psr_max_v); - goto unsupported; + return false; } tgl_dc3co_exitline_compute_config(intel_dp, crtc_state); + + if (psr2_su_region_et_valid(intel_dp)) + crtc_state->enable_psr2_su_region_et = true; + + return true; +} + +static bool intel_sel_update_config_valid(struct intel_dp *intel_dp, + struct intel_crtc_state *crtc_state) +{ + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); + + if (HAS_PSR2_SEL_FETCH(dev_priv) && + !intel_psr2_sel_fetch_config_valid(intel_dp, crtc_state) && + !HAS_PSR_HW_TRACKING(dev_priv)) { + drm_dbg_kms(_priv->drm, + "Selective update not enabled, selective fetch not valid and no HW tracking available\n"); + goto unsupported; + } + + if (!psr2_global_enabled(intel_dp)) { + drm_dbg_kms(_priv->drm, "Selective update disabled by flag\n"); + goto unsupported; + } + + if (!crtc_state->has_panel_replay && !intel_psr2_config_valid(intel_dp, crtc_state)) + goto unsupported; + + if (crtc_state->has_panel_replay && (DISPLAY_VER(dev_priv) < 14 || + !intel_dp->psr.sink_panel_replay_su_support)) + goto unsupported; + + if (crtc_state->crc_enabled) { + drm_dbg_kms(_priv->drm, + "Selective update not enabled because it would inhibit pipe CRC calculation\n"); + goto unsupported; + } + + if (!psr2_granularity_check(intel_dp, crtc_state)) { + drm_dbg_kms(_priv->drm, + "Selective update not enabled, SU granularity not compatible\n"); + goto unsupported; + } + return
[PATCH v4 16/19] drm/i915/psr: Do not apply workarounds in case of panel replay
There are some workarounds that are not applicable for panel replay. Do not apply these if panel replay is used. Bspec: 66624, 50422 Signed-off-by: Jouni Högander --- drivers/gpu/drm/i915/display/intel_fbc.c | 5 +++-- drivers/gpu/drm/i915/display/intel_hdmi.c | 3 ++- drivers/gpu/drm/i915/display/intel_psr.c | 16 ++-- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 5bfce36fb892..555058c0a1dc 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1250,7 +1250,8 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, * Recommendation is to keep this combination disabled * Bspec: 50422 HSD: 14010260002 */ - if (IS_DISPLAY_VER(i915, 12, 14) && crtc_state->has_sel_update) { + if (IS_DISPLAY_VER(i915, 12, 14) && crtc_state->has_sel_update && + !crtc_state->has_panel_replay) { plane_state->no_fbc_reason = "PSR2 enabled"; return 0; } @@ -1258,7 +1259,7 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, /* Wa_14016291713 */ if ((IS_DISPLAY_VER(i915, 12, 13) || IS_DISPLAY_IP_STEP(i915, IP_VER(14, 0), STEP_A0, STEP_C0)) && - crtc_state->has_psr) { + crtc_state->has_psr && !crtc_state->has_panel_replay) { plane_state->no_fbc_reason = "PSR1 enabled (Wa_14016291713)"; return 0; } diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index a32cb4d25bc7..0bfd352d9f14 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -524,7 +524,8 @@ void hsw_write_infoframe(struct intel_encoder *encoder, 0); /* Wa_14013475917 */ - if (!(IS_DISPLAY_VER(dev_priv, 13, 14) && crtc_state->has_psr && type == DP_SDP_VSC)) + if (!(IS_DISPLAY_VER(dev_priv, 13, 14) && crtc_state->has_psr && + !crtc_state->has_panel_replay && type == DP_SDP_VSC)) val |= hsw_infoframe_enable(type); if (type == DP_SDP_VSC) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index bba1063ccd2e..3879bdbad6fd 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -1948,13 +1948,15 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, * All supported adlp panels have 1-based X granularity, this may * cause issues if non-supported panels are used. */ - if (IS_DISPLAY_IP_STEP(dev_priv, IP_VER(14, 0), STEP_A0, STEP_B0) || - IS_ALDERLAKE_P(dev_priv)) + if (!intel_dp->psr.panel_replay_enabled && + (IS_DISPLAY_IP_STEP(dev_priv, IP_VER(14, 0), STEP_A0, STEP_B0) || +IS_ALDERLAKE_P(dev_priv))) intel_de_rmw(dev_priv, hsw_chicken_trans_reg(dev_priv, cpu_transcoder), 0, ADLP_1_BASED_X_GRANULARITY); /* Wa_16012604467:adlp,mtl[a0,b0] */ - if (IS_DISPLAY_IP_STEP(dev_priv, IP_VER(14, 0), STEP_A0, STEP_B0)) + if (!intel_dp->psr.panel_replay_enabled && + IS_DISPLAY_IP_STEP(dev_priv, IP_VER(14, 0), STEP_A0, STEP_B0)) intel_de_rmw(dev_priv, MTL_CLKGATE_DIS_TRANS(cpu_transcoder), 0, MTL_CLKGATE_DIS_TRANS_DMASC_GATING_DIS); @@ -2130,7 +2132,8 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp) if (intel_dp->psr.sel_update_enabled) { /* Wa_16012604467:adlp,mtl[a0,b0] */ - if (IS_DISPLAY_IP_STEP(dev_priv, IP_VER(14, 0), STEP_A0, STEP_B0)) + if (!intel_dp->psr.panel_replay_enabled && + IS_DISPLAY_IP_STEP(dev_priv, IP_VER(14, 0), STEP_A0, STEP_B0)) intel_de_rmw(dev_priv, MTL_CLKGATE_DIS_TRANS(cpu_transcoder), MTL_CLKGATE_DIS_TRANS_DMASC_GATING_DIS, 0); @@ -2616,8 +2619,9 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state, goto skip_sel_fetch_set_loop; /* Wa_14014971492 */ - if ((IS_DISPLAY_IP_STEP(dev_priv, IP_VER(14, 0), STEP_A0, STEP_B0) || -IS_ALDERLAKE_P(dev_priv) || IS_TIGERLAKE(dev_priv)) && + if (!crtc_state->has_panel_replay && + ((IS_DISPLAY_IP_STEP(dev_priv, IP_VER(14, 0), STEP_A0, STEP_B0) || + IS_ALDERLAKE_P(dev_priv) || IS_TIGERLAKE(dev_priv))) && crtc_state->splitter.enable) crtc_state->psr2_su_area.y1 = 0; -- 2.34.1
[PATCH v4 15/19] drm/i915/psr: Panel replay uses SRD_STATUS to track it's status
DP Panel replay uses SRD_STATUS to track it's status despite selective update mode. Bspec: 53370, 68920 v2: - use intel_dp_is_edp to differentiate - modify debugfs status as well Signed-off-by: Jouni Högander --- drivers/gpu/drm/i915/display/intel_psr.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 111eb7d6bc1c..bba1063ccd2e 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -2849,7 +2849,8 @@ void intel_psr_wait_for_idle_locked(const struct intel_crtc_state *new_crtc_stat if (!intel_dp->psr.enabled) continue; - if (intel_dp->psr.sel_update_enabled) + if (intel_dp_is_edp(intel_dp) && + intel_dp->psr.sel_update_enabled) ret = _psr2_ready_for_pipe_update_locked(intel_dp); else ret = _psr1_ready_for_pipe_update_locked(intel_dp); @@ -2870,7 +2871,8 @@ static bool __psr_wait_for_idle_locked(struct intel_dp *intel_dp) if (!intel_dp->psr.enabled) return false; - if (intel_dp->psr.sel_update_enabled) { + if (!intel_dp->psr.panel_replay_enabled && + intel_dp->psr.sel_update_enabled) { reg = EDP_PSR2_STATUS(cpu_transcoder); mask = EDP_PSR2_STATUS_STATE_MASK; } else { @@ -3489,7 +3491,7 @@ psr_source_status(struct intel_dp *intel_dp, struct seq_file *m) const char *status = "unknown"; u32 val, status_val; - if (intel_dp->psr.sel_update_enabled) { + if (intel_dp_is_edp(intel_dp)) { static const char * const live_status[] = { "IDLE", "CAPTURE", -- 2.34.1
[PATCH v4 13/19] drm/i915/psr: Detect panel replay selective update support
Add new boolean to store panel replay selective update support of sink into intel_psr struct. Detect panel replay selective update support and store it into this new boolean. v3: Clear sink_panel_replay_su_support in intel_dp_detect v2: Merge adding new boolean into this patch Signed-off-by: Jouni Högander --- drivers/gpu/drm/i915/display/intel_display_types.h | 1 + drivers/gpu/drm/i915/display/intel_dp.c| 1 + drivers/gpu/drm/i915/display/intel_psr.c | 10 -- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 63097b10bc4d..fac568c312be 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1758,6 +1758,7 @@ struct intel_psr { u16 su_y_granularity; bool source_panel_replay_support; bool sink_panel_replay_support; + bool sink_panel_replay_su_support; bool panel_replay_enabled; u32 dc3co_exitline; u32 dc3co_exit_delay; diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index f1b917041192..5d896c338b61 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5730,6 +5730,7 @@ intel_dp_detect(struct drm_connector *connector, memset(_dp->compliance, 0, sizeof(intel_dp->compliance)); memset(intel_connector->dp.dsc_dpcd, 0, sizeof(intel_connector->dp.dsc_dpcd)); intel_dp->psr.sink_panel_replay_support = false; + intel_dp->psr.sink_panel_replay_su_support = false; intel_dp_mst_disconnect(intel_dp); diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index f4c014eb28eb..fdc6d699d9c2 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -520,9 +520,15 @@ static void _panel_replay_init_dpcd(struct intel_dp *intel_dp) return; } - drm_dbg_kms(>drm, - "Panel replay is supported by panel\n"); intel_dp->psr.sink_panel_replay_support = true; + + if (pr_dpcd & DP_PANEL_REPLAY_SU_SUPPORT) + intel_dp->psr.sink_panel_replay_su_support = true; + + drm_dbg_kms(>drm, + "Panel replay %sis supported by panel\n", + intel_dp->psr.sink_panel_replay_su_support ? + "selective_update " : ""); } static void _psr_init_dpcd(struct intel_dp *intel_dp) -- 2.34.1
[PATCH v4 14/19] drm/i915/psr: Modify intel_dp_get_su_granularity to support panel replay
Currently intel_dp_get_su_granularity doesn't support panel replay. This fix modifies it to support panel replay as well. v2: rely on PSR definitions on common bits Signed-off-by: Jouni Högander Reviewed-by: Animesh Manna --- drivers/gpu/drm/i915/display/intel_psr.c | 62 +--- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index fdc6d699d9c2..111eb7d6bc1c 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -466,6 +466,40 @@ static u8 intel_dp_get_sink_sync_latency(struct intel_dp *intel_dp) return val; } +static u8 intel_dp_get_su_capability(struct intel_dp *intel_dp) +{ + u8 su_capability; + + if (intel_dp->psr.sink_panel_replay_su_support) + drm_dp_dpcd_read(_dp->aux, +DP_PANEL_PANEL_REPLAY_X_GRANULARITY, +_capability, 1); + else + su_capability = intel_dp->psr_dpcd[1]; + + return su_capability; +} + +static unsigned int +intel_dp_get_su_x_granularity_offset(struct intel_dp *intel_dp) +{ + return intel_dp->psr.sink_panel_replay_su_support ? + DP_PANEL_PANEL_REPLAY_X_GRANULARITY : + DP_PSR2_SU_X_GRANULARITY; +} + +static unsigned int +intel_dp_get_su_y_granularity_offset(struct intel_dp *intel_dp) +{ + return intel_dp->psr.sink_panel_replay_su_support ? + DP_PANEL_PANEL_REPLAY_Y_GRANULARITY : + DP_PSR2_SU_Y_GRANULARITY; +} + +/* + * Note: Bits related to granularity are same in panel replay and psr + * registers. Rely on PSR definitions on these "common" bits. + */ static void intel_dp_get_su_granularity(struct intel_dp *intel_dp) { struct drm_i915_private *i915 = dp_to_i915(intel_dp); @@ -473,18 +507,29 @@ static void intel_dp_get_su_granularity(struct intel_dp *intel_dp) u16 w; u8 y; - /* If sink don't have specific granularity requirements set legacy ones */ - if (!(intel_dp->psr_dpcd[1] & DP_PSR2_SU_GRANULARITY_REQUIRED)) { + /* +* TODO: Do we need to take into account panel supporting both PSR and +* Panel replay? +*/ + + /* +* If sink don't have specific granularity requirements set legacy +* ones. +*/ + if (!(intel_dp_get_su_capability(intel_dp) & + DP_PSR2_SU_GRANULARITY_REQUIRED)) { /* As PSR2 HW sends full lines, we do not care about x granularity */ w = 4; y = 4; goto exit; } - r = drm_dp_dpcd_read(_dp->aux, DP_PSR2_SU_X_GRANULARITY, , 2); + r = drm_dp_dpcd_read(_dp->aux, +intel_dp_get_su_x_granularity_offset(intel_dp), +, 2); if (r != 2) drm_dbg_kms(>drm, - "Unable to read DP_PSR2_SU_X_GRANULARITY\n"); + "Unable to read selective update x granularity\n"); /* * Spec says that if the value read is 0 the default granularity should * be used instead. @@ -492,10 +537,12 @@ static void intel_dp_get_su_granularity(struct intel_dp *intel_dp) if (r != 2 || w == 0) w = 4; - r = drm_dp_dpcd_read(_dp->aux, DP_PSR2_SU_Y_GRANULARITY, , 1); + r = drm_dp_dpcd_read(_dp->aux, +intel_dp_get_su_y_granularity_offset(intel_dp), +, 1); if (r != 1) { drm_dbg_kms(>drm, - "Unable to read DP_PSR2_SU_Y_GRANULARITY\n"); + "Unable to read selective update y granularity\n"); y = 4; } if (y == 0) @@ -588,7 +635,8 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp) if (intel_dp->psr_dpcd[0]) _psr_init_dpcd(intel_dp); - if (intel_dp->psr.sink_psr2_support) + if (intel_dp->psr.sink_psr2_support || + intel_dp->psr.sink_panel_replay_su_support) intel_dp_get_su_granularity(intel_dp); } -- 2.34.1
[PATCH v4 12/19] drm/panelreplay: dpcd register definition for panelreplay SU
Add definitions for panel replay selective update v2: Remove unnecessary Cc from commit message Signed-off-by: Jouni Högander Reviewed-by: Animesh Manna --- include/drm/display/drm_dp.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h index 4891bd916d26..ff04b2af2844 100644 --- a/include/drm/display/drm_dp.h +++ b/include/drm/display/drm_dp.h @@ -548,6 +548,12 @@ # define DP_PANEL_REPLAY_SUPPORT(1 << 0) # define DP_PANEL_REPLAY_SU_SUPPORT (1 << 1) +#define DP_PANEL_PANEL_REPLAY_CAPABILITY 0xb1 +# define DP_PANEL_PANEL_REPLAY_SU_GRANULARITY_REQUIRED (1 << 5) + +#define DP_PANEL_PANEL_REPLAY_X_GRANULARITY0xb2 +#define DP_PANEL_PANEL_REPLAY_Y_GRANULARITY0xb4 + /* Link Configuration */ #defineDP_LINK_BW_SET 0x100 # define DP_LINK_RATE_TABLE0x00/* eDP 1.4 */ -- 2.34.1
[PATCH v4 11/19] drm/i915/psr: Rename psr2_enabled as sel_update_enabled
We are about to reuse psr2_enabled for panel replay as well. Rename it as sel_update_enabled to avoid confusion. v3: Rebase v2: Rebase Signed-off-by: Jouni Högander Reviewed-by: Animesh Manna --- .../drm/i915/display/intel_display_types.h| 2 +- drivers/gpu/drm/i915/display/intel_psr.c | 52 +-- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 28b087d009ea..63097b10bc4d 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1734,7 +1734,7 @@ struct intel_psr { unsigned int busy_frontbuffer_bits; bool sink_psr2_support; bool link_standby; - bool psr2_enabled; + bool sel_update_enabled; bool psr2_sel_fetch_enabled; bool psr2_sel_fetch_cff_enabled; bool req_psr2_sdp_prior_scanline; diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index f2ea995578f8..f4c014eb28eb 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -356,12 +356,12 @@ static void psr_irq_control(struct intel_dp *intel_dp) } static void psr_event_print(struct drm_i915_private *i915, - u32 val, bool psr2_enabled) + u32 val, bool sel_update_enabled) { drm_dbg_kms(>drm, "PSR exit events: 0x%x\n", val); if (val & PSR_EVENT_PSR2_WD_TIMER_EXPIRE) drm_dbg_kms(>drm, "\tPSR2 watchdog timer expired\n"); - if ((val & PSR_EVENT_PSR2_DISABLED) && psr2_enabled) + if ((val & PSR_EVENT_PSR2_DISABLED) && sel_update_enabled) drm_dbg_kms(>drm, "\tPSR2 disabled\n"); if (val & PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN) drm_dbg_kms(>drm, "\tSU dirty FIFO underrun\n"); @@ -389,7 +389,7 @@ static void psr_event_print(struct drm_i915_private *i915, drm_dbg_kms(>drm, "\tVBI enabled\n"); if (val & PSR_EVENT_LPSP_MODE_EXIT) drm_dbg_kms(>drm, "\tLPSP mode exited\n"); - if ((val & PSR_EVENT_PSR_DISABLE) && !psr2_enabled) + if ((val & PSR_EVENT_PSR_DISABLE) && !sel_update_enabled) drm_dbg_kms(>drm, "\tPSR disabled\n"); } @@ -417,7 +417,7 @@ void intel_psr_irq_handler(struct intel_dp *intel_dp, u32 psr_iir) val = intel_de_rmw(dev_priv, PSR_EVENT(cpu_transcoder), 0, 0); - psr_event_print(dev_priv, val, intel_dp->psr.psr2_enabled); + psr_event_print(dev_priv, val, intel_dp->psr.sel_update_enabled); } } @@ -1661,10 +1661,10 @@ void intel_psr_get_config(struct intel_encoder *encoder, pipe_config->has_psr = true; } - pipe_config->has_sel_update = intel_dp->psr.psr2_enabled; + pipe_config->has_sel_update = intel_dp->psr.sel_update_enabled; pipe_config->infoframes.enable |= intel_hdmi_infoframe_enable(DP_SDP_VSC); - if (!intel_dp->psr.psr2_enabled) + if (!intel_dp->psr.sel_update_enabled) goto unlock; if (HAS_PSR2_SEL_FETCH(dev_priv)) { @@ -1700,7 +1700,7 @@ static void intel_psr_activate(struct intel_dp *intel_dp) /* psr1, psr2 and panel-replay are mutually exclusive.*/ if (intel_dp->psr.panel_replay_enabled) dg2_activate_panel_replay(intel_dp); - else if (intel_dp->psr.psr2_enabled) + else if (intel_dp->psr.sel_update_enabled) hsw_activate_psr2(intel_dp); else hsw_activate_psr1(intel_dp); @@ -1759,7 +1759,7 @@ static void lnl_alpm_configure(struct intel_dp *intel_dp) struct intel_psr *psr = _dp->psr; u32 alpm_ctl; - if (DISPLAY_VER(dev_priv) < 20 || (!intel_dp->psr.psr2_enabled && + if (DISPLAY_VER(dev_priv) < 20 || (!intel_dp->psr.sel_update_enabled && !intel_dp_is_edp(intel_dp))) return; @@ -1883,7 +1883,7 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, */ wm_optimization_wa(intel_dp, crtc_state); - if (intel_dp->psr.psr2_enabled) { + if (intel_dp->psr.sel_update_enabled) { if (DISPLAY_VER(dev_priv) == 9) intel_de_rmw(dev_priv, CHICKEN_TRANS(cpu_transcoder), 0, PSR2_VSC_ENABLE_PROG_HEADER | @@ -1949,7 +1949,7 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp, drm_WARN_ON(_priv->drm, intel_dp->psr.enabled); - intel_dp->psr.psr2_enabled = crtc_state->has_sel_update; + intel_dp->psr.sel_update_enabled = crtc_state->has_sel_update; intel_dp->psr.panel_replay_enabled = crtc_state->has_panel_replay; intel_dp->psr.busy_frontbuffer_bits = 0; intel_dp->psr.pipe =
[PATCH v4 10/19] drm/i915/psr: Rename has_psr2 as has_sel_update
We are going to reuse has_psr2 for panel_replay as well. Rename it as has_sel_update to avoid confusion. v2: Rebase Signed-off-by: Jouni Högander Reviewed-by: Animesh Manna --- drivers/gpu/drm/i915/display/intel_crtc_state_dump.c | 10 +- drivers/gpu/drm/i915/display/intel_display.c | 2 +- drivers/gpu/drm/i915/display/intel_display_types.h | 2 +- drivers/gpu/drm/i915/display/intel_dp.c | 2 +- drivers/gpu/drm/i915/display/intel_fbc.c | 2 +- drivers/gpu/drm/i915/display/intel_psr.c | 10 +- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c index cd78c200d483..7b853ee623d8 100644 --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c @@ -251,11 +251,11 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config, drm_printf(, "sdp split: %s\n", str_enabled_disabled(pipe_config->sdp_split_enable)); - drm_printf(, "psr: %s, psr2: %s, panel replay: %s, selective fetch: %s\n", - str_enabled_disabled(pipe_config->has_psr), - str_enabled_disabled(pipe_config->has_psr2), - str_enabled_disabled(pipe_config->has_panel_replay), - str_enabled_disabled(pipe_config->enable_psr2_sel_fetch)); + drm_printf(, "psr: %s, selective update: %s, panel replay: %s, selective fetch: %s\n", + str_enabled_disabled(pipe_config->has_psr), + str_enabled_disabled(pipe_config->has_sel_update), + str_enabled_disabled(pipe_config->has_panel_replay), + str_enabled_disabled(pipe_config->enable_psr2_sel_fetch)); } drm_printf(, "framestart delay: %d, MSA timing delay: %d\n", diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 233f602ea3da..d9a45598ccff 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -5260,7 +5260,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, */ if (current_config->has_panel_replay || pipe_config->has_panel_replay) { PIPE_CONF_CHECK_BOOL(has_psr); - PIPE_CONF_CHECK_BOOL(has_psr2); + PIPE_CONF_CHECK_BOOL(has_sel_update); PIPE_CONF_CHECK_BOOL(enable_psr2_sel_fetch); PIPE_CONF_CHECK_BOOL(enable_psr2_su_region_et); PIPE_CONF_CHECK_BOOL(has_panel_replay); diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 4ef1f5f709d8..28b087d009ea 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1230,7 +1230,7 @@ struct intel_crtc_state { /* PSR is supported but might not be enabled due the lack of enabled planes */ bool has_psr; - bool has_psr2; + bool has_sel_update; bool enable_psr2_sel_fetch; bool enable_psr2_su_region_et; bool req_psr2_sdp_prior_scanline; diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index a1c3be4a79af..f1b917041192 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2630,7 +2630,7 @@ static void intel_dp_compute_vsc_sdp(struct intel_dp *intel_dp, if (intel_dp_needs_vsc_sdp(crtc_state, conn_state)) { intel_dp_compute_vsc_colorimetry(crtc_state, conn_state, vsc); - } else if (crtc_state->has_psr2) { + } else if (crtc_state->has_psr && crtc_state->has_sel_update) { /* * [PSR2 without colorimetry] * Prepare VSC Header for SU as per eDP 1.4 spec, Table 6-11 diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 7c4d2b2bf20b..5bfce36fb892 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1250,7 +1250,7 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, * Recommendation is to keep this combination disabled * Bspec: 50422 HSD: 14010260002 */ - if (IS_DISPLAY_VER(i915, 12, 14) && crtc_state->has_psr2) { + if (IS_DISPLAY_VER(i915, 12, 14) && crtc_state->has_sel_update) { plane_state->no_fbc_reason = "PSR2 enabled"; return 0; } diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 4db6c19731e9..f2ea995578f8 100644 ---
[PATCH v4 09/19] drm/i915/psr: Panel replay has to be enabled before link training
Panel replay has to be enabled on sink side before link training. Take this into account in fastset check and in initial fastset check. Signed-off-by: Jouni Högander Reviewed-by: Animesh Manna --- drivers/gpu/drm/i915/display/intel_display.c | 12 drivers/gpu/drm/i915/display/intel_dp.c | 8 drivers/gpu/drm/i915/display/intel_psr.c | 3 --- drivers/gpu/drm/i915/display/intel_psr.h | 3 +++ 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 614e60420a29..233f602ea3da 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -5254,6 +5254,18 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, PIPE_CONF_CHECK_CSC(output_csc); } + /* +* Panel replay has to be enabled before link training. PSR doesn't have +* this requirement -> check these only if using panel replay +*/ + if (current_config->has_panel_replay || pipe_config->has_panel_replay) { + PIPE_CONF_CHECK_BOOL(has_psr); + PIPE_CONF_CHECK_BOOL(has_psr2); + PIPE_CONF_CHECK_BOOL(enable_psr2_sel_fetch); + PIPE_CONF_CHECK_BOOL(enable_psr2_su_region_et); + PIPE_CONF_CHECK_BOOL(has_panel_replay); + } + PIPE_CONF_CHECK_BOOL(double_wide); if (dev_priv->display.dpll.mgr) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index b8976bb67510..a1c3be4a79af 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -3353,6 +3353,14 @@ bool intel_dp_initial_fastset_check(struct intel_encoder *encoder, fastset = false; } + if (CAN_PANEL_REPLAY(intel_dp)) { + drm_dbg_kms(>drm, + "[ENCODER:%d:%s] Forcing full modeset to compute panel replay state\n", + encoder->base.base.id, encoder->base.name); + crtc_state->uapi.mode_changed = true; + fastset = false; + } + return fastset; } diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 4355fb02d8fd..4db6c19731e9 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -192,9 +192,6 @@ #define CAN_PSR(intel_dp) ((intel_dp)->psr.sink_support && \ (intel_dp)->psr.source_support) -#define CAN_PANEL_REPLAY(intel_dp) ((intel_dp)->psr.sink_panel_replay_support && \ - (intel_dp)->psr.source_panel_replay_support) - bool intel_encoder_can_psr(struct intel_encoder *encoder) { if (intel_encoder_is_dp(encoder) || encoder->type == INTEL_OUTPUT_DP_MST) diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h index 2537dcb8765c..d483c85870e1 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.h +++ b/drivers/gpu/drm/i915/display/intel_psr.h @@ -21,6 +21,9 @@ struct intel_encoder; struct intel_plane; struct intel_plane_state; +#define CAN_PANEL_REPLAY(intel_dp) ((intel_dp)->psr.sink_panel_replay_support && \ + (intel_dp)->psr.source_panel_replay_support) + bool intel_encoder_can_psr(struct intel_encoder *encoder); void intel_psr_init_dpcd(struct intel_dp *intel_dp); void intel_psr_enable_sink(struct intel_dp *intel_dp, -- 2.34.1
[PATCH v4 08/19] drm/i915/psr: Unify panel replay enable/disable sink
Unify enabling and disabling of psr/panel replay for a sink. Modify intel_psr_enable_sink accordingly and use it for both cases. v3: - move psr2_su_region_et_valid to be check for PSR2 only v2: - enable panel replay for sink before link training - write ALPM_CONFIG only for PSR - add DP_PSR_CRC_VERIFICATION only for PSR - take care of disable sink as well Signed-off-by: Jouni Högander --- drivers/gpu/drm/i915/display/intel_ddi.c | 11 ++--- drivers/gpu/drm/i915/display/intel_psr.c | 60 +--- drivers/gpu/drm/i915/display/intel_psr.h | 2 + 3 files changed, 50 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index a3d3d4942eb1..4cdc218653b1 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -2809,15 +2809,14 @@ static void intel_ddi_pre_enable_dp(struct intel_atomic_state *state, const struct drm_connector_state *conn_state) { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); - struct intel_dp *intel_dp = enc_to_intel_dp(encoder); - if (HAS_DP20(dev_priv)) { + if (HAS_DP20(dev_priv)) intel_dp_128b132b_sdp_crc16(enc_to_intel_dp(encoder), crtc_state); - if (crtc_state->has_panel_replay) - drm_dp_dpcd_writeb(_dp->aux, PANEL_REPLAY_CONFIG, - DP_PANEL_REPLAY_ENABLE); - } + + /* Panel replay has to be enabled in sink dpcd before link training. */ + if (crtc_state->has_panel_replay) + intel_psr_enable_sink(enc_to_intel_dp(encoder), crtc_state); if (DISPLAY_VER(dev_priv) >= 14) mtl_ddi_pre_enable_dp(state, encoder, crtc_state, conn_state); diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index b7538a4405b8..4355fb02d8fd 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -638,40 +638,59 @@ static bool psr2_su_region_et_valid(struct intel_dp *intel_dp) return false; } -static void intel_psr_enable_sink(struct intel_dp *intel_dp) +static unsigned int intel_psr_get_enable_sink_offset(struct intel_dp *intel_dp) +{ + return intel_dp->psr.panel_replay_enabled ? + PANEL_REPLAY_CONFIG : DP_PSR_EN_CFG; +} + +/* + * Note: Most of the bits are same in PANEL_REPLAY_CONFIG and DP_PSR_EN_CFG. We + * are relying on PSR definitions on these "common" bits. + */ +void intel_psr_enable_sink(struct intel_dp *intel_dp, + const struct intel_crtc_state *crtc_state) { struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); u8 dpcd_val = DP_PSR_ENABLE; - if (intel_dp->psr.panel_replay_enabled) - return; - - if (intel_dp->psr.psr2_enabled) { + if (crtc_state->has_psr2) { /* Enable ALPM at sink for psr2 */ - drm_dp_dpcd_writeb(_dp->aux, DP_RECEIVER_ALPM_CONFIG, - DP_ALPM_ENABLE | - DP_ALPM_LOCK_ERROR_IRQ_HPD_ENABLE); + if (!crtc_state->has_panel_replay) { + drm_dp_dpcd_writeb(_dp->aux, + DP_RECEIVER_ALPM_CONFIG, + DP_ALPM_ENABLE | + DP_ALPM_LOCK_ERROR_IRQ_HPD_ENABLE); + + if (psr2_su_region_et_valid(intel_dp)) + dpcd_val |= DP_PSR_ENABLE_SU_REGION_ET; + } dpcd_val |= DP_PSR_ENABLE_PSR2 | DP_PSR_IRQ_HPD_WITH_CRC_ERRORS; - if (psr2_su_region_et_valid(intel_dp)) - dpcd_val |= DP_PSR_ENABLE_SU_REGION_ET; } else { if (intel_dp->psr.link_standby) dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE; - if (DISPLAY_VER(dev_priv) >= 8) + if (!crtc_state->has_panel_replay && DISPLAY_VER(dev_priv) >= 8) dpcd_val |= DP_PSR_CRC_VERIFICATION; } - if (intel_dp->psr.req_psr2_sdp_prior_scanline) + if (crtc_state->has_panel_replay) + dpcd_val |= DP_PANEL_REPLAY_UNRECOVERABLE_ERROR_EN | + DP_PANEL_REPLAY_RFB_STORAGE_ERROR_EN; + + if (crtc_state->req_psr2_sdp_prior_scanline) dpcd_val |= DP_PSR_SU_REGION_SCANLINE_CAPTURE; if (intel_dp->psr.entry_setup_frames > 0) dpcd_val |= DP_PSR_FRAME_CAPTURE; - drm_dp_dpcd_writeb(_dp->aux, DP_PSR_EN_CFG, dpcd_val); + drm_dp_dpcd_writeb(_dp->aux, + intel_psr_get_enable_sink_offset(intel_dp), + dpcd_val); - drm_dp_dpcd_writeb(_dp->aux, DP_SET_POWER,
[PATCH v4 04/19] drm/i915/psr: Do not update phy power state in case of non-eDP panel replay
Currently panel replay is supporting only main link on mode -> Do not update phy power state for non-eDP panel replay. Bspec: 53370 v2: use intel_dp_is_edp to differentiate Signed-off-by: Jouni Högander --- drivers/gpu/drm/i915/display/intel_psr.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index bd99b9953274..07c1ddec2d86 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -1930,13 +1930,16 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp, if (!psr_interrupt_error_check(intel_dp)) return; - if (intel_dp->psr.panel_replay_enabled) + if (intel_dp->psr.panel_replay_enabled) { drm_dbg_kms(_priv->drm, "Enabling Panel Replay\n"); - else + } else { drm_dbg_kms(_priv->drm, "Enabling PSR%s\n", intel_dp->psr.psr2_enabled ? "2" : "1"); + } + + if (intel_dp_is_edp(intel_dp)) + intel_snps_phy_update_psr_power_state(_port->base, true); - intel_snps_phy_update_psr_power_state(_port->base, true); intel_psr_enable_sink(intel_dp); intel_psr_enable_source(intel_dp, crtc_state); intel_dp->psr.enabled = true; @@ -2041,7 +2044,8 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp) CLKGATE_DIS_MISC_DMASC_GATING_DIS, 0); } - intel_snps_phy_update_psr_power_state(_to_dig_port(intel_dp)->base, false); + if (intel_dp_is_edp(intel_dp)) + intel_snps_phy_update_psr_power_state(_to_dig_port(intel_dp)->base, false); /* Panel Replay on eDP is always using ALPM aux less. */ if (intel_dp->psr.panel_replay_enabled && intel_dp_is_edp(intel_dp)) { -- 2.34.1
[PATCH v4 07/19] drm/i915/psr: Call intel_psr_init_dpcd in intel_dp_detect
Currently panel replay dpcd capability isn't properly checked after plugging in DP panel. Fix this by calling intel_psr_init_dpcd in intel_dp_detect. Signed-off-by: Jouni Högander --- drivers/gpu/drm/i915/display/intel_dp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index b393ddbb7b35..b8976bb67510 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5744,6 +5744,8 @@ intel_dp_detect(struct drm_connector *connector, intel_dp_mst_configure(intel_dp); + intel_psr_init_dpcd(intel_dp); + /* * TODO: Reset link params when switching to MST mode, until MST * supports link training fallback params. -- 2.34.1
[PATCH v4 06/19] drm/i915/psr: Do not write registers/bits not applicable for panel replay
Bspec is saying this mask register: Only PSR_MASK[Mask FBC modify] and PSR_MASK[Mask Hotplug] are used in panel replay mode. Status register: Only SRD_STATUS[SRD state] field is used in panel replay mode. Due to this stop writing and reading registers and bits not used by panel replay if panel replay is used. Bspec: 53370, 68920 v2: - use intel_dp_is_edp with PSR_MASK register - handle LunarLake as well - hanle ALPM configuration as well Signed-off-by: Jouni Högander --- drivers/gpu/drm/i915/display/intel_psr.c | 70 +++- 1 file changed, 45 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index d7547eefc2fa..b7538a4405b8 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -346,6 +346,9 @@ static void psr_irq_control(struct intel_dp *intel_dp) enum transcoder cpu_transcoder = intel_dp->psr.transcoder; u32 mask; + if (intel_dp->psr.panel_replay_enabled) + return; + mask = psr_irq_psr_error_bit_get(intel_dp); if (intel_dp->psr.debug & I915_PSR_DEBUG_IRQ) mask |= psr_irq_post_exit_bit_get(intel_dp) | @@ -1783,7 +1786,7 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, { struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); enum transcoder cpu_transcoder = intel_dp->psr.transcoder; - u32 mask; + u32 mask = 0; /* * Only HSW and BDW have PSR AUX registers that need to be setup. @@ -1797,34 +1800,46 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, * mask LPSP to avoid dependency on other drivers that might block * runtime_pm besides preventing other hw tracking issues now we * can rely on frontbuffer tracking. +* +* From bspec prior LunarLake: +* Only PSR_MASK[Mask FBC modify] and PSR_MASK[Mask Hotplug] are used in +* panel replay mode. +* +* From bspec beyod LunarLake: +* Panel Replay on DP: No bits are applicable +* Panel Replay on eDP: All bits are applicable */ - mask = EDP_PSR_DEBUG_MASK_MEMUP | - EDP_PSR_DEBUG_MASK_HPD; + if (DISPLAY_VER(dev_priv) < 20 || intel_dp_is_edp(intel_dp)) + mask = EDP_PSR_DEBUG_MASK_HPD; - /* -* For some unknown reason on HSW non-ULT (or at least on -* Dell Latitude E6540) external displays start to flicker -* when PSR is enabled on the eDP. SR/PC6 residency is much -* higher than should be possible with an external display. -* As a workaround leave LPSP unmasked to prevent PSR entry -* when external displays are active. -*/ - if (DISPLAY_VER(dev_priv) >= 8 || IS_HASWELL_ULT(dev_priv)) - mask |= EDP_PSR_DEBUG_MASK_LPSP; + if (intel_dp_is_edp(intel_dp)) { + mask |= EDP_PSR_DEBUG_MASK_MEMUP; - if (DISPLAY_VER(dev_priv) < 20) - mask |= EDP_PSR_DEBUG_MASK_MAX_SLEEP; + /* +* For some unknown reason on HSW non-ULT (or at least on +* Dell Latitude E6540) external displays start to flicker +* when PSR is enabled on the eDP. SR/PC6 residency is much +* higher than should be possible with an external display. +* As a workaround leave LPSP unmasked to prevent PSR entry +* when external displays are active. +*/ + if (DISPLAY_VER(dev_priv) >= 8 || IS_HASWELL_ULT(dev_priv)) + mask |= EDP_PSR_DEBUG_MASK_LPSP; - /* -* No separate pipe reg write mask on hsw/bdw, so have to unmask all -* registers in order to keep the CURSURFLIVE tricks working :( -*/ - if (IS_DISPLAY_VER(dev_priv, 9, 10)) - mask |= EDP_PSR_DEBUG_MASK_DISP_REG_WRITE; + if (DISPLAY_VER(dev_priv) < 20) + mask |= EDP_PSR_DEBUG_MASK_MAX_SLEEP; - /* allow PSR with sprite enabled */ - if (IS_HASWELL(dev_priv)) - mask |= EDP_PSR_DEBUG_MASK_SPRITE_ENABLE; + /* +* No separate pipe reg write mask on hsw/bdw, so have to unmask all +* registers in order to keep the CURSURFLIVE tricks working :( +*/ + if (IS_DISPLAY_VER(dev_priv, 9, 10)) + mask |= EDP_PSR_DEBUG_MASK_DISP_REG_WRITE; + + /* allow PSR with sprite enabled */ + if (IS_HASWELL(dev_priv)) + mask |= EDP_PSR_DEBUG_MASK_SPRITE_ENABLE; + } intel_de_write(dev_priv, psr_debug_reg(dev_priv, cpu_transcoder), mask); @@ -1843,7 +1858,8 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, intel_dp->psr.psr2_sel_fetch_enabled ?
[PATCH v4 05/19] drm/i915/psr: Check possible errors for panel replay as well
On HPD interrupt we want to check if the reason for HPD was some panel replay error detected by monitor/panel. This is already done for PSR. We want to do this for panel replay as well. Modify intel_psr_short_pulse to support panel replay as well. Signed-off-by: Jouni Högander Reviewed-by: Animesh Manna --- drivers/gpu/drm/i915/display/intel_psr.c | 21 - 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 07c1ddec2d86..d7547eefc2fa 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -3256,6 +3256,13 @@ static void psr_capability_changed_check(struct intel_dp *intel_dp) } } +/* + * On common bits: + * DP_PSR_RFB_STORAGE_ERROR == DP_PANEL_REPLAY_RFB_STORAGE_ERROR + * DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR == DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR + * DP_PSR_LINK_CRC_ERROR == DP_PANEL_REPLAY_LINK_CRC_ERROR + * this function is relying on PSR definitions + */ void intel_psr_short_pulse(struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); @@ -3265,7 +3272,7 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp) DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR | DP_PSR_LINK_CRC_ERROR; - if (!CAN_PSR(intel_dp)) + if (!CAN_PSR(intel_dp) && !CAN_PANEL_REPLAY(intel_dp)) return; mutex_lock(>lock); @@ -3279,12 +3286,14 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp) goto exit; } - if (status == DP_PSR_SINK_INTERNAL_ERROR || (error_status & errors)) { + if ((!psr->panel_replay_enabled && status == DP_PSR_SINK_INTERNAL_ERROR) || + (error_status & errors)) { intel_psr_disable_locked(intel_dp); psr->sink_not_reliable = true; } - if (status == DP_PSR_SINK_INTERNAL_ERROR && !error_status) + if (!psr->panel_replay_enabled && status == DP_PSR_SINK_INTERNAL_ERROR && + !error_status) drm_dbg_kms(_priv->drm, "PSR sink internal error, disabling PSR\n"); if (error_status & DP_PSR_RFB_STORAGE_ERROR) @@ -3304,8 +3313,10 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp) /* clear status register */ drm_dp_dpcd_writeb(_dp->aux, DP_PSR_ERROR_STATUS, error_status); - psr_alpm_check(intel_dp); - psr_capability_changed_check(intel_dp); + if (!psr->panel_replay_enabled) { + psr_alpm_check(intel_dp); + psr_capability_changed_check(intel_dp); + } exit: mutex_unlock(>lock); -- 2.34.1
[PATCH v4 03/19] drm/i915/psr: Intel_psr_pause/resume needs to support panel replay
Currently intel_psr_pause and intel_psr_resume do nothing in case of panel replay. Change them to perform pause and return also in case of panel replay. Signed-off-by: Jouni Högander Reviewed-by: Animesh Manna --- drivers/gpu/drm/i915/display/intel_psr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 10a7795cdb6f..bd99b9953274 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -2104,7 +2104,7 @@ void intel_psr_pause(struct intel_dp *intel_dp) struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); struct intel_psr *psr = _dp->psr; - if (!CAN_PSR(intel_dp)) + if (!CAN_PSR(intel_dp) && !CAN_PANEL_REPLAY(intel_dp)) return; mutex_lock(>lock); @@ -2137,7 +2137,7 @@ void intel_psr_resume(struct intel_dp *intel_dp) { struct intel_psr *psr = _dp->psr; - if (!CAN_PSR(intel_dp)) + if (!CAN_PSR(intel_dp) && !CAN_PANEL_REPLAY(intel_dp)) return; mutex_lock(>lock); -- 2.34.1
[PATCH v4 02/19] drm/i915/psr: Set intel_crtc_state->has_psr on panel replay as well
Current code is setting only intel_crtc_state->has_panel_replay in panel replay case. There are lots of stuff behind intel_crtc_state->has_psr that is needed for panel replay as well. Instead of converting each check to has_psr || has_panel_replay set has_psr in case of panel replay as well. Code can then differentiate between psr and panel replay by using intel_crtc_state->has_panel_replay. Signed-off-by: Jouni Högander Reviewed-by: Animesh Manna --- drivers/gpu/drm/i915/display/intel_psr.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 45d13e042ade..10a7795cdb6f 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -1602,10 +1602,11 @@ void intel_psr_compute_config(struct intel_dp *intel_dp, if (CAN_PANEL_REPLAY(intel_dp)) crtc_state->has_panel_replay = true; - else - crtc_state->has_psr = _psr_compute_config(intel_dp, crtc_state); - if (!(crtc_state->has_panel_replay || crtc_state->has_psr)) + crtc_state->has_psr = crtc_state->has_panel_replay ? true : + _psr_compute_config(intel_dp, crtc_state); + + if (!crtc_state->has_psr) return; crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state); @@ -1632,7 +1633,7 @@ void intel_psr_get_config(struct intel_encoder *encoder, goto unlock; if (intel_dp->psr.panel_replay_enabled) { - pipe_config->has_panel_replay = true; + pipe_config->has_psr = pipe_config->has_panel_replay = true; } else { /* * Not possible to read EDP_PSR/PSR2_CTL registers as it is @@ -2651,7 +2652,7 @@ void intel_psr_post_plane_update(struct intel_atomic_state *state, intel_atomic_get_new_crtc_state(state, crtc); struct intel_encoder *encoder; - if (!(crtc_state->has_psr || crtc_state->has_panel_replay)) + if (!crtc_state->has_psr) return; for_each_intel_encoder_mask_with_psr(state->base.dev, encoder, -- 2.34.1
[PATCH v4 00/19] Panel replay selective update support
This patch set is implementing panel replay selective update support for Intel hardware. It is also fixing several exisiting issues in current panel replay implementation: Several needed functions are not executed for panel replay Ensure link training follows enabling panel replay on sink side Do not update phy power state for panel replay. Do not apply workarounds not applicable for panel replay Do not write registers/bits not applicable for panel replay v4: - do not rename intel_psr_enabled - do not add sel_update_et_enabled into struct intel_psr v3: - do not disable panel replay by default - set has_psr for panel replay as well - enable sink before link training - do not apply all PSR workarounds for panel replay - do not write/read registers/bits not applicable for panel replay - use psr bit definitions in granularity configuration as well - goto unsupported instead of return when global enabled check fails - update module parameter descriptions. v2: - make psr pause/resume to work for panel replay as well Jouni Högander (19): drm/i915/psr: Add some documentation of variables used in psr code drm/i915/psr: Set intel_crtc_state->has_psr on panel replay as well drm/i915/psr: Intel_psr_pause/resume needs to support panel replay drm/i915/psr: Do not update phy power state in case of non-eDP panel replay drm/i915/psr: Check possible errors for panel replay as well drm/i915/psr: Do not write registers/bits not applicable for panel replay drm/i915/psr: Call intel_psr_init_dpcd in intel_dp_detect drm/i915/psr: Unify panel replay enable/disable sink drm/i915/psr: Panel replay has to be enabled before link training drm/i915/psr: Rename has_psr2 as has_sel_update drm/i915/psr: Rename psr2_enabled as sel_update_enabled drm/panelreplay: dpcd register definition for panelreplay SU drm/i915/psr: Detect panel replay selective update support drm/i915/psr: Modify intel_dp_get_su_granularity to support panel replay drm/i915/psr: Panel replay uses SRD_STATUS to track it's status drm/i915/psr: Do not apply workarounds in case of panel replay drm/i915/psr: Update PSR module parameter descriptions drm/i915/psr: Split intel_psr2_config_valid for panel replay drm/i915/psr: Add panel replay sel update support to debugfs interface .../drm/i915/display/intel_crtc_state_dump.c | 10 +- drivers/gpu/drm/i915/display/intel_ddi.c | 11 +- drivers/gpu/drm/i915/display/intel_display.c | 12 + .../drm/i915/display/intel_display_params.c | 5 +- .../drm/i915/display/intel_display_types.h| 5 +- drivers/gpu/drm/i915/display/intel_dp.c | 13 +- drivers/gpu/drm/i915/display/intel_fbc.c | 5 +- drivers/gpu/drm/i915/display/intel_hdmi.c | 3 +- drivers/gpu/drm/i915/display/intel_psr.c | 420 -- drivers/gpu/drm/i915/display/intel_psr.h | 5 + include/drm/display/drm_dp.h | 6 + 11 files changed, 343 insertions(+), 152 deletions(-) -- 2.34.1
[PATCH v4 01/19] drm/i915/psr: Add some documentation of variables used in psr code
We are adding more boolean variable into intel_psr and intel_crtc_state structs. Add some documentation about these for sake of clarity. v2: Modify has_psr + has_panel_replay to mean panel replay Signed-off-by: Jouni Högander Reviewed-by: Animesh Manna --- drivers/gpu/drm/i915/display/intel_psr.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index eef62983e9db..45d13e042ade 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -171,6 +171,22 @@ * * The rest of the bits are more self-explanatory and/or * irrelevant for normal operation. + * + * Description of intel_crtc_state variables. has_psr, has_panel_replay and + * has_sel_update: + * + * has_psr (alone): PSR1 + * has_psr + has_sel_update: PSR2 + * has_psr + has_panel_replay:Panel Replay + * has_psr + has_panel_replay + has_sel_update: Panel Replay Selective Update + * + * Description of some intel_psr varibles. enabled, panel_replay_enabled, + * sel_update_enabled + * + * enabled (alone): PSR1 + * enabled + sel_update_enabled: PSR2 + * enabled + panel_replay_enabled:Panel Replay + * enabled + panel_replay_enabled + sel_update_enabled: Panel Replay SU */ #define CAN_PSR(intel_dp) ((intel_dp)->psr.sink_support && \ -- 2.34.1
Re: [PATCH v5 0/5] ALPM AUX-Less
On Thu, 2024-03-28 at 05:33 +, Manna, Animesh wrote: > > > > -Original Message- > > From: Hogander, Jouni > > Sent: Friday, March 22, 2024 4:00 PM > > To: intel-gfx@lists.freedesktop.org > > Cc: Ville Syrjälä ; Manna, Animesh > > ; Murthy, Arun R > > ; > > Hogander, Jouni > > Subject: [PATCH v5 0/5] ALPM AUX-Less > > > > This patch set is implementing calculation of ALPM AUX-Less > > parameters for > > Intel HW and writing them in case of AUX-Less is enabled. It is > > also enabling > > ALPM AUX-Less for eDP Panel Replay. Current code is not allowing > > Panel > > Replay on eDP. Patches for this are coming later. > > > > This implementation is only for Panel Replay usage. LOBF (Link Off > > Between > > Active Frames) usage needs more work. > > > > v5: > > - mention AUX Less enable is only on source side in commit > > message > > v4: > > - drop patch adding AUX LESS dpcd defines > > - re-use fast_wake_lines to store aux_less_wake_lines > > - add comment explaining why AUX less is enabled on eDP panel > > replay > > without any extra checks > > v3: > > - use definitions instead of numbers for max values > > - do not use alpm_ctl as uninitialized variable > > v2: > > - use variables instead of values directly > > - fix several max values > > - move converting port clock to Mhz into _lnl_compute_* > > - do not set AUX-Wake related bits for AUX-Less case > > - do not write ALPM configuration for DP2.0 Panel Replay or PSR1 > > > > Jouni Högander (5): > > drm/i915/psr: Add missing ALPM AUX-Less register definitions > > drm/i915/psr: Calculate aux less wake time > > drm/i915/psr: Silence period and lfps half cycle > > drm/i915/psr: Enable ALPM on source side for eDP Panel replay > > drm/i915/psr: Do not write ALPM configuration for PSR1 or DP2.0 > > Panel > > Replay > > The above patches LGTM. For whole patch series: > Reviewed-by: Animesh Manna Thank you Animesh for your review. There are now pushed to drm-intel- next. BR, Jouni Högander > > > > > .../drm/i915/display/intel_display_types.h | 2 + > > drivers/gpu/drm/i915/display/intel_psr.c | 188 > > +- > > drivers/gpu/drm/i915/display/intel_psr_regs.h | 12 +- > > 3 files changed, 193 insertions(+), 9 deletions(-) > > > > -- > > 2.34.1 >
Re: [PATCH 0/4] drm/edid & drm/i915: vendor and product id logging improvements
On Thu, 21 Mar 2024, Jani Nikula wrote: > Jani Nikula (4): > drm/edid: add drm_edid_get_product_id() > drm/edid: add drm_edid_print_product_id() > drm/i915/bios: switch to struct drm_edid and struct > drm_edid_product_id > drm/i915/bios: return drm_edid_product_id from get_lvds_pnp_id() Ping for reviews please? This should be helpful in eradicating one class of drm_edid_raw() uses. BR, Jani. > > drivers/gpu/drm/drm_edid.c| 50 +++ > drivers/gpu/drm/i915/display/intel_bios.c | 49 ++ > include/drm/drm_edid.h| 28 ++--- > 3 files changed, 94 insertions(+), 33 deletions(-) -- Jani Nikula, Intel
Re: [PATCH] drm/i915/display: move dmc_firmware_path to display params
On Fri, 22 Mar 2024, Lucas De Marchi wrote: > oh, now I understand. You mean that xe module doesn't have the param > because it's only declared in drivers/gpu/drm/i915/i915_params.c. > > Could you extend the commit message with something like this? > > The dmc_firmware_path parameter is clearly a display parameter. Move it > there so it's available to both i915 and xe modules > > thanks > Reviewed-by: Lucas De Marchi Thanks for the review, pushed to drm-intel-next with an amended commit message. BR, Jani. -- Jani Nikula, Intel
Re: [REGRESSION] external monitor+Dell dock in 6.8
[Adding a few folks and list while dropping the stable list, as this is unrelated to it] On 31.03.24 07:59, Andrei Gaponenko wrote: > > I noticed a regression with the mailine kernel pre-compiled by EPEL. > I have just tried linux-6.9-rc1.tar.gz from kernel.org, and it still > misbehaves. > > The default setup: a laptop is connected to a dock, Dell WD22TB4, via > a USB-C cable. The dock is connected to an external monitor via a > Display Port cable. With a "good" kernel everything works. With a > "broken" kernel, the external monitor is still correctly identified by > the system, and is shown as enabled in plasma systemsettings. The > system also behaves like the monitor is working, for example, one can > move the mouse pointer off the laptop screen. However the external > monitor screen stays black, and it eventually goes to sleep. Just a quick heads up to ensure people are aware of it: Imre Deak, turns out this is caused by a patch of yours: 55eaef16417448 ("drm/i915/dp_mst: Handle the Synaptics HBlank expansion quirk"). Andrei Gaponenko meanwhile filed a ticket about it here: https://gitlab.freedesktop.org/drm/intel/-/issues/10637 Ciao, Thorsten > Everything worked with EPEL mainline kernels up to and including > kernel-ml-6.7.9-1.el9.elrepo.x86_64 > > The breakage is observed in > > kernel-ml-6.8.1-1.el9.elrepo.x86_64 > kernel-ml-6.8.2-1.el9.elrepo.x86_64 > linux-6.9-rc1.tar.gz from kernel.org (with olddefconfig) > > Other tests: using an HDMI cable instead of the Display Port cable > between the monitor and the dock does not change things, black screen > with the newer kernels. > > Using a small HDMI-to-USB-C adapter instead of the dock results in a > working system, even with the newer kernels. So the breakage appears > to be specific to the Dell WD22TB4 dock. > > Operating System: AlmaLinux 9.3 (Shamrock Pampas Cat) > > uname -mi: x86_64 x86_64 > > Laptop: Dell Precision 5470/02RK6V > > lsusb |grep dock > Bus 003 Device 007: ID 413c:b06e Dell Computer Corp. Dell dock > Bus 003 Device 008: ID 413c:b06f Dell Computer Corp. Dell dock > Bus 003 Device 006: ID 0bda:5413 Realtek Semiconductor Corp. Dell dock > Bus 003 Device 005: ID 0bda:5487 Realtek Semiconductor Corp. Dell dock > Bus 002 Device 004: ID 0bda:0413 Realtek Semiconductor Corp. Dell dock > Bus 002 Device 003: ID 0bda:0487 Realtek Semiconductor Corp. Dell dock > > dmesg and kernel config are attached to > https://bugzilla.kernel.org/show_bug.cgi?id=218663 > > #regzbot introduced: v6.7.9..v6.8.1 P.S.: #regzbot duplicate: https://bugzilla.kernel.org/show_bug.cgi?id=218663 #regzbot duplicate: https://gitlab.freedesktop.org/drm/intel/-/issues/10637 #regzbot title: drm/i915/dp_mst: external monitor on Dell dock broke
Re: [PATCH] drm/i915: use fine grained -Woverride-init disable
On Thu, 28 Mar 2024, "Arnd Bergmann" wrote: > On Thu, Mar 28, 2024, at 11:46, Jani Nikula wrote: >> On Thu, 28 Mar 2024, "Arnd Bergmann" wrote: >>> On Thu, Mar 28, 2024, at 11:24, Jani Nikula wrote: Use localized __diag_push(), __diag_ignore_all() with rationale, and __diag_pop() for specific initializations instead of blanket disabling of -Woverride-init across several files. Note that we've tried this before with commit 88e9664434c9 ("drm/i915: use localized __diag_ignore_all() instead of per file") and reverted in commit 290d16104575 ("Revert "drm/i915: use localized __diag_ignore_all() instead of per file""). The issue turned out to be in __diag_ignore_all() and it was fixed by commit 689b097a06ba ("compiler-gcc: Suppress -Wmissing-prototypes warning for all supported GCC"). So we should be able to pull this off now. Cc: "Arnd Bergmann" Cc: Lucas De Marchi Signed-off-by: Jani Nikula >>> >>> Looks good to me, >>> >>> Acked-by: Arnd Bergmann >> >> Thanks! I'll take this via drm-intel-next for v6.10. Up to you what to >> do with your patch [1], either drop the i915 and xe changes, or we can >> handle the trivial conflict too if keeping the changes helps you >> somehow. > > I'll just drop all of the parts for drivers/gpu and send another > patch for the amdgpu driver to do the same as your patch. Works for me, thanks! In the mean time, merged this one to drm-intel-next, thanks for the ack and review! BR, Jani. -- Jani Nikula, Intel
Re: 2024 X.Org Foundation Membership deadline for voting in the election
Hi Pekka, On Tue, Apr 02, 2024 at 10:46:08AM +0300, Pekka Paalanen wrote: > On Tue, 26 Mar 2024 11:42:48 -0400 Christopher Michael wrote: > > > The 2024 X.Org Foundation membership renewal period has been extended > > one additional week and elections will start the following week on 01 > > April 2024. > > > > Please note that only current members can vote in the upcoming election, > > and that the deadline for new memberships or renewals to vote in the > > upcoming election is 01 April 2024 at 23:59 UTC. > > > > If you are interested in joining the X.Org Foundation or in renewing > > your membership, please visit the membership system site at: > > https://members.x.org/ > > > > Christopher Michael, on behalf of the X.Org elections committee > > Hi everyone, > > given that the year's first email reminding everyone to renew their > memberships was sent on Feb 7 when the renewal was NOT open yet, I > wonder how many people thought they had already renewed and are now > thinking they don't need to do anything? > > I fell for that: On Feb 7, I went to members.x.org to check my status, > it said I was registered for "2023-2024" and there was no button to > renew, so I closed the page confident that I was a member for 2024. > After all, it said 2024. This was a mistake I realised only after being > personally poked to renew. I know for sure of one other person falling > for the same. Make that two. Thanks for the notice. > Now, the members page for this year says "Application for the period: > 02/2024-02/2025". Thanks to the people adding the month to reduce > confusion. -- Regards, Laurent Pinchart
Re: [PATCH v0 03/14] drm/gma500,drm/i915: Make I2C terminology more inclusive
On Fri, 29 Mar 2024, Easwar Hariharan wrote: > I2C v7, SMBus 3.2, and I3C specifications have replaced "master/slave" > with more appropriate terms. Inspired by and following on to Wolfram's > series to fix drivers/i2c/[1], fix the terminology for users of > I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists > in the specification. gma500 and i915 changes should be split. See MAINTAINERS. Might also split the i915 changes to smaller pieces, it's kind of random. And the changes here are not strictly related to I2C AFAICT, so the commit message should be updated. BR, Jani. > > Compile tested, no functionality changes intended > > [1]: > https://lore.kernel.org/all/20240322132619.6389-1-wsa+rene...@sang-engineering.com/ > > Signed-off-by: Easwar Hariharan > --- > drivers/gpu/drm/gma500/cdv_intel_lvds.c | 2 +- > drivers/gpu/drm/gma500/intel_bios.c | 22 +++--- > drivers/gpu/drm/gma500/intel_bios.h | 4 +-- > drivers/gpu/drm/gma500/intel_gmbus.c | 2 +- > drivers/gpu/drm/gma500/psb_drv.h | 2 +- > drivers/gpu/drm/gma500/psb_intel_drv.h| 2 +- > drivers/gpu/drm/gma500/psb_intel_lvds.c | 4 +-- > drivers/gpu/drm/gma500/psb_intel_sdvo.c | 26 > drivers/gpu/drm/i915/display/dvo_ch7017.c | 14 - > drivers/gpu/drm/i915/display/dvo_ch7xxx.c | 18 +-- > drivers/gpu/drm/i915/display/dvo_ivch.c | 16 +- > drivers/gpu/drm/i915/display/dvo_ns2501.c | 18 +-- > drivers/gpu/drm/i915/display/dvo_sil164.c | 18 +-- > drivers/gpu/drm/i915/display/dvo_tfp410.c | 18 +-- > drivers/gpu/drm/i915/display/intel_bios.c | 22 +++--- > drivers/gpu/drm/i915/display/intel_ddi.c | 2 +- > .../gpu/drm/i915/display/intel_display_core.h | 2 +- > drivers/gpu/drm/i915/display/intel_dsi.h | 2 +- > drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 18 +-- > drivers/gpu/drm/i915/display/intel_dvo.c | 14 - > drivers/gpu/drm/i915/display/intel_dvo_dev.h | 2 +- > drivers/gpu/drm/i915/display/intel_gmbus.c| 4 +-- > drivers/gpu/drm/i915/display/intel_sdvo.c | 30 +-- > drivers/gpu/drm/i915/display/intel_vbt_defs.h | 4 +-- > drivers/gpu/drm/i915/gvt/edid.c | 28 - > drivers/gpu/drm/i915/gvt/edid.h | 4 +-- > drivers/gpu/drm/i915/gvt/opregion.c | 2 +- > 27 files changed, 150 insertions(+), 150 deletions(-) > > diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c > b/drivers/gpu/drm/gma500/cdv_intel_lvds.c > index f08a6803dc18..84c9122062c4 100644 > --- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c > +++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c > @@ -565,7 +565,7 @@ void cdv_intel_lvds_init(struct drm_device *dev, > dev->dev, "I2C bus registration failed.\n"); > goto err_encoder_cleanup; > } > - gma_encoder->i2c_bus->slave_addr = 0x2C; > + gma_encoder->i2c_bus->client_addr = 0x2C; > dev_priv->lvds_i2c_bus = gma_encoder->i2c_bus; > > /* > diff --git a/drivers/gpu/drm/gma500/intel_bios.c > b/drivers/gpu/drm/gma500/intel_bios.c > index 8245b5603d2c..333bece1a64f 100644 > --- a/drivers/gpu/drm/gma500/intel_bios.c > +++ b/drivers/gpu/drm/gma500/intel_bios.c > @@ -14,8 +14,8 @@ > #include "psb_intel_drv.h" > #include "psb_intel_reg.h" > > -#define SLAVE_ADDR1 0x70 > -#define SLAVE_ADDR2 0x72 > +#define CLIENT_ADDR10x70 > +#define CLIENT_ADDR20x72 > > static void *find_section(struct bdb_header *bdb, int section_id) > { > @@ -357,10 +357,10 @@ parse_sdvo_device_mapping(struct drm_psb_private > *dev_priv, > /* skip the device block if device type is invalid */ > continue; > } > - if (p_child->slave_addr != SLAVE_ADDR1 && > - p_child->slave_addr != SLAVE_ADDR2) { > + if (p_child->client_addr != CLIENT_ADDR1 && > + p_child->client_addr != CLIENT_ADDR2) { > /* > - * If the slave address is neither 0x70 nor 0x72, > + * If the client address is neither 0x70 nor 0x72, >* it is not a SDVO device. Skip it. >*/ > continue; > @@ -371,22 +371,22 @@ parse_sdvo_device_mapping(struct drm_psb_private > *dev_priv, > DRM_DEBUG_KMS("Incorrect SDVO port. Skip it\n"); > continue; > } > - DRM_DEBUG_KMS("the SDVO device with slave addr %2x is found on" > + DRM_DEBUG_KMS("the SDVO device with client addr %2x is found on" > " %s port\n", > - p_child->slave_addr, > + p_child->client_addr, >
Re: 2024 X.Org Foundation Membership deadline for voting in the election
On Tue, 26 Mar 2024 11:42:48 -0400 Christopher Michael wrote: > The 2024 X.Org Foundation membership renewal period has been extended > one additional week and elections will start the following week on 01 > April 2024. > > Please note that only current members can vote in the upcoming election, > and that the deadline for new memberships or renewals to vote in the > upcoming election is 01 April 2024 at 23:59 UTC. > > If you are interested in joining the X.Org Foundation or in renewing > your membership, please visit the membership system site at: > https://members.x.org/ > > Christopher Michael, on behalf of the X.Org elections committee Hi everyone, given that the year's first email reminding everyone to renew their memberships was sent on Feb 7 when the renewal was NOT open yet, I wonder how many people thought they had already renewed and are now thinking they don't need to do anything? I fell for that: On Feb 7, I went to members.x.org to check my status, it said I was registered for "2023-2024" and there was no button to renew, so I closed the page confident that I was a member for 2024. After all, it said 2024. This was a mistake I realised only after being personally poked to renew. I know for sure of one other person falling for the same. Now, the members page for this year says "Application for the period: 02/2024-02/2025". Thanks to the people adding the month to reduce confusion. Thanks, pq pgp3GIQU8hI9m.pgp Description: OpenPGP digital signature
RE: [PATCH 22/22] drm/i915: Use debugfs_create_bool() for "i915_bigjoiner_force_enable"
Hello Ville, Thank you very much for the series. 6K detects fine and works. Tested-by: Vidya Srinivas > -Original Message- > From: Intel-gfx On Behalf Of Ville > Syrjala > Sent: Friday, March 29, 2024 6:43 AM > To: intel-gfx@lists.freedesktop.org > Subject: [PATCH 22/22] drm/i915: Use debugfs_create_bool() for > "i915_bigjoiner_force_enable" > > From: Ville Syrjälä > > There is no reason to make this debugfs file for a simple boolean so > complicated. Just use debugfs_create_bool(). > > Signed-off-by: Ville Syrjälä > --- > .../drm/i915/display/intel_display_debugfs.c | 44 +-- > 1 file changed, 2 insertions(+), 42 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > index b99c024b0934..3e364891dcd0 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > @@ -1402,20 +1402,6 @@ out: drm_modeset_unlock( > >drm.mode_config.connection_mutex); > return ret; > } > > -static int i915_bigjoiner_enable_show(struct seq_file *m, void *data) -{ > - struct intel_connector *connector = m->private; > - struct drm_crtc *crtc; > - > - crtc = connector->base.state->crtc; > - if (connector->base.status != connector_status_connected || !crtc) > - return -ENODEV; > - > - seq_printf(m, "Bigjoiner enable: %d\n", connector- > >force_bigjoiner_enable); > - > - return 0; > -} > - > static ssize_t i915_dsc_output_format_write(struct file *file, > const char __user *ubuf, > size_t len, loff_t *offp) > @@ -1437,30 +1423,6 @@ static ssize_t i915_dsc_output_format_write(struct > file *file, > return len; > } > > -static ssize_t i915_bigjoiner_enable_write(struct file *file, > -const char __user *ubuf, > -size_t len, loff_t *offp) > -{ > - struct seq_file *m = file->private_data; > - struct intel_connector *connector = m->private; > - struct drm_crtc *crtc; > - bool bigjoiner_en = 0; > - int ret; > - > - crtc = connector->base.state->crtc; > - if (connector->base.status != connector_status_connected || !crtc) > - return -ENODEV; > - > - ret = kstrtobool_from_user(ubuf, len, _en); > - if (ret < 0) > - return ret; > - > - connector->force_bigjoiner_enable = bigjoiner_en; > - *offp += len; > - > - return len; > -} > - > static int i915_dsc_output_format_open(struct inode *inode, > struct file *file) > { > @@ -1554,8 +1516,6 @@ static const struct file_operations > i915_dsc_fractional_bpp_fops = { > .write = i915_dsc_fractional_bpp_write }; > > -DEFINE_SHOW_STORE_ATTRIBUTE(i915_bigjoiner_enable); > - > /* > * Returns the Current CRTC's bpc. > * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc > @@ -1640,8 +1600,8 @@ void intel_connector_debugfs_add(struct > intel_connector *connector) > if (DISPLAY_VER(i915) >= 11 && > (connector_type == DRM_MODE_CONNECTOR_DisplayPort || >connector_type == DRM_MODE_CONNECTOR_eDP)) { > - debugfs_create_file("i915_bigjoiner_force_enable", 0644, > root, > - connector, _bigjoiner_enable_fops); > + debugfs_create_bool("i915_bigjoiner_force_enable", 0644, > root, > + >force_bigjoiner_enable); > } > > if (connector_type == DRM_MODE_CONNECTOR_DSI || > -- > 2.43.2