Re: [Intel-gfx] [PATCH v2 1/1] vfio: remove VFIO_GROUP_NOTIFY_SET_KVM
> From: h...@infradead.org > Sent: Thursday, May 19, 2022 2:48 PM > > On Thu, May 19, 2022 at 06:43:06AM +, Tian, Kevin wrote: > > > This fixes a user-triggerable oops in GVT. > > > > No changelog. > > ?? > > the cover latter clearly states what has changed since v1, and this > patch has a good commit log. This is exactly how it is supposed to > be done. sigh... don't know why I missed the coverletter.
Re: [Intel-gfx] [PATCH v2 1/1] vfio: remove VFIO_GROUP_NOTIFY_SET_KVM
On Thu, May 19, 2022 at 06:43:06AM +, Tian, Kevin wrote: > > This fixes a user-triggerable oops in GVT. > > No changelog. ?? the cover latter clearly states what has changed since v1, and this patch has a good commit log. This is exactly how it is supposed to be done. > > > > > Signed-off-by: Jason Gunthorpe > > Signed-off-by: Matthew Rosato > > Not sure whether Christoph wants a s-o-b here when he wrote > the snippet to remove the release work of gvt... That's just tivial code removal, so no. > > > @@ -1083,11 +1083,22 @@ static struct file *vfio_device_open(struct > > vfio_device *device) > > > > mutex_lock(&device->dev_set->lock); > > device->open_count++; > > + down_read(&device->group->group_rwsem); > > + if (device->open_count == 1 && device->group->kvm) { > > + /* > > +* Here we pass the KVM pointer with the group under the > > read > > +* lock. If the device driver will use it, it must obtain a > > +* reference and release it during close_device. > > +*/ > > + device->kvm = device->group->kvm; > > + } > > + > > if (device->open_count == 1 && device->ops->open_device) { > > Merge the two branches so both are under if (device->open_count == 1) {} > (and group_rwsem can be also moved inside) Yeah. And we don't really need the device->group->kvm check, as it would otherwise assign NULL which is perfectly fine. But otherwise this also looks good to me: Reviewed-by: Christoph Hellwig
Re: [Intel-gfx] [PATCH v2 1/1] vfio: remove VFIO_GROUP_NOTIFY_SET_KVM
> From: Matthew Rosato > Sent: Thursday, May 19, 2022 5:26 AM > > Rather than relying on a notifier for associating the KVM with > the group, let's assume that the association has already been > made prior to device_open. The first time a device is opened > associate the group KVM with the device. > > This fixes a user-triggerable oops in GVT. No changelog. > > Signed-off-by: Jason Gunthorpe > Signed-off-by: Matthew Rosato Not sure whether Christoph wants a s-o-b here when he wrote the snippet to remove the release work of gvt... > @@ -1083,11 +1083,22 @@ static struct file *vfio_device_open(struct > vfio_device *device) > > mutex_lock(&device->dev_set->lock); > device->open_count++; > + down_read(&device->group->group_rwsem); > + if (device->open_count == 1 && device->group->kvm) { > + /* > + * Here we pass the KVM pointer with the group under the > read > + * lock. If the device driver will use it, it must obtain a > + * reference and release it during close_device. > + */ > + device->kvm = device->group->kvm; > + } > + > if (device->open_count == 1 && device->ops->open_device) { Merge the two branches so both are under if (device->open_count == 1) {} (and group_rwsem can be also moved inside) > @@ -1315,9 +1330,13 @@ static int vfio_device_fops_release(struct inode > *inode, struct file *filep) > > mutex_lock(&device->dev_set->lock); > vfio_assert_device_open(device); > + down_read(&device->group->group_rwsem); > if (device->open_count == 1 && device->ops->close_device) > device->ops->close_device(device); > device->open_count--; > + if (device->open_count == 0 && device->kvm) > + device->kvm = NULL; This can be moved out of group_rwsem as there is no reference to vfio_group. > + up_read(&device->group->group_rwsem); otherwise, Reviewed-by: Kevin Tian Thanks Kevin
[Intel-gfx] [PULL] drm-intel-fixes
Hi Dave & Daniel, Two final -fixes for v5.18. One is to reject DMC with out-of-spec MMIO (Cc: stable) and another to correctly mark guilty contexts on GuC reset. Regards, Joonas *** drm-intel-fixes-2022-05-19: - Reject DMC firmware with out-of-spec MMIO addresses. - Correctly mark guilty context on GuC reset The following changes since commit 42226c989789d8da4af1de0c31070c96726d990c: Linux 5.18-rc7 (2022-05-15 18:08:58 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2022-05-19 for you to fetch changes up to 89e96d822bd51f7afe2d3e95a34099480b5c3d55: i915/guc/reset: Make __guc_reset_context aware of guilty engines (2022-05-16 10:13:39 +0300) - Reject DMC firmware with out-of-spec MMIO addresses. - Correctly mark guilty context on GuC reset Anusha Srivatsa (1): drm/i915/dmc: Add MMIO range restrictions Umesh Nerlige Ramappa (1): i915/guc/reset: Make __guc_reset_context aware of guilty engines drivers/gpu/drm/i915/display/intel_dmc.c | 44 +++ drivers/gpu/drm/i915/gt/intel_reset.c | 2 +- drivers/gpu/drm/i915/gt/uc/intel_guc.h| 2 +- drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 16 - drivers/gpu/drm/i915/gt/uc/intel_uc.c | 2 +- drivers/gpu/drm/i915/gt/uc/intel_uc.h | 2 +- drivers/gpu/drm/i915/i915_reg.h | 16 + 7 files changed, 72 insertions(+), 12 deletions(-)
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/dg2: Extend Wa_22010954014 to DG2-G11 and DG2-G12
== Series Details == Series: drm/i915/dg2: Extend Wa_22010954014 to DG2-G11 and DG2-G12 URL : https://patchwork.freedesktop.org/series/104104/ State : success == Summary == CI Bug Log - changes from CI_DRM_11668_full -> Patchwork_104104v1_full Summary --- **SUCCESS** No regressions found. Participating hosts (11 -> 13) -- Additional (2): shard-rkl shard-dg1 Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_104104v1_full: ### IGT changes ### Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-180-hflip-async-flip: - {shard-rkl}:NOTRUN -> [INCOMPLETE][1] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104104v1/shard-rkl-5/igt@kms_big...@yf-tiled-max-hw-stride-64bpp-rotate-180-hflip-async-flip.html Known issues Here are the changes found in Patchwork_104104v1_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_ccs@ctrl-surf-copy: - shard-iclb: NOTRUN -> [SKIP][2] ([i915#5327]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104104v1/shard-iclb7/igt@gem_...@ctrl-surf-copy.html * igt@gem_ccs@suspend-resume: - shard-kbl: NOTRUN -> [SKIP][3] ([fdo#109271]) +27 similar issues [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104104v1/shard-kbl6/igt@gem_...@suspend-resume.html * igt@gem_eio@unwedge-stress: - shard-snb: NOTRUN -> [FAIL][4] ([i915#3354]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104104v1/shard-snb5/igt@gem_...@unwedge-stress.html * igt@gem_exec_fair@basic-deadline: - shard-glk: [PASS][5] -> [FAIL][6] ([i915#2846]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11668/shard-glk6/igt@gem_exec_f...@basic-deadline.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104104v1/shard-glk9/igt@gem_exec_f...@basic-deadline.html * igt@gem_exec_fair@basic-none-solo@rcs0: - shard-apl: [PASS][7] -> [FAIL][8] ([i915#2842]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11668/shard-apl8/igt@gem_exec_fair@basic-none-s...@rcs0.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104104v1/shard-apl8/igt@gem_exec_fair@basic-none-s...@rcs0.html * igt@gem_exec_fair@basic-pace-solo@rcs0: - shard-kbl: [PASS][9] -> [FAIL][10] ([i915#2842]) +1 similar issue [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11668/shard-kbl7/igt@gem_exec_fair@basic-pace-s...@rcs0.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104104v1/shard-kbl7/igt@gem_exec_fair@basic-pace-s...@rcs0.html * igt@gem_exec_flush@basic-batch-kernel-default-wb: - shard-snb: [PASS][11] -> [SKIP][12] ([fdo#109271]) +4 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11668/shard-snb4/igt@gem_exec_fl...@basic-batch-kernel-default-wb.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104104v1/shard-snb6/igt@gem_exec_fl...@basic-batch-kernel-default-wb.html * igt@gem_exec_suspend@basic-s0@smem: - shard-kbl: [PASS][13] -> [INCOMPLETE][14] ([i915#4831]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11668/shard-kbl7/igt@gem_exec_suspend@basic...@smem.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104104v1/shard-kbl6/igt@gem_exec_suspend@basic...@smem.html * igt@gem_lmem_swapping@verify-random: - shard-skl: NOTRUN -> [SKIP][15] ([fdo#109271] / [i915#4613]) +1 similar issue [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104104v1/shard-skl6/igt@gem_lmem_swapp...@verify-random.html * igt@gem_render_copy@y-tiled-mc-ccs-to-vebox-y-tiled: - shard-iclb: NOTRUN -> [SKIP][16] ([i915#768]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104104v1/shard-iclb7/igt@gem_render_c...@y-tiled-mc-ccs-to-vebox-y-tiled.html * igt@gem_userptr_blits@dmabuf-sync: - shard-iclb: NOTRUN -> [SKIP][17] ([i915#3323]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104104v1/shard-iclb7/igt@gem_userptr_bl...@dmabuf-sync.html * igt@gem_userptr_blits@dmabuf-unsync: - shard-iclb: NOTRUN -> [SKIP][18] ([i915#3297]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104104v1/shard-iclb7/igt@gem_userptr_bl...@dmabuf-unsync.html * igt@gem_userptr_blits@vma-merge: - shard-skl: NOTRUN -> [FAIL][19] ([i915#3318]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104104v1/shard-skl6/igt@gem_userptr_bl...@vma-merge.html * igt@gem_workarounds@suspend-resume-context: - shard-apl: [PASS][20] -> [DMESG-WARN][21] ([i915#180]) +2 similar issues [20]: https://intel-gfx-ci.01.org
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/dg2: Add workaround 22014600077 (rev2)
== Series Details == Series: drm/i915/dg2: Add workaround 22014600077 (rev2) URL : https://patchwork.freedesktop.org/series/104097/ State : success == Summary == CI Bug Log - changes from CI_DRM_11668_full -> Patchwork_104097v2_full Summary --- **SUCCESS** No regressions found. Participating hosts (11 -> 13) -- Additional (2): shard-rkl shard-dg1 Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_104097v2_full: ### IGT changes ### Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@gem_exec_schedule@smoketest@vcs0: - {shard-rkl}:NOTRUN -> [INCOMPLETE][1] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104097v2/shard-rkl-5/igt@gem_exec_schedule@smoket...@vcs0.html New tests - New tests have been introduced between CI_DRM_11668_full and Patchwork_104097v2_full: ### New IGT tests (2) ### * igt@kms_cursor_legacy@cursora-vs-flipb: - Statuses : 1 skip(s) - Exec time: [0.0] s * igt@kms_cursor_legacy@forked-move: - Statuses : - Exec time: [None] s Known issues Here are the changes found in Patchwork_104097v2_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_ccs@ctrl-surf-copy: - shard-iclb: NOTRUN -> [SKIP][2] ([i915#5327]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104097v2/shard-iclb6/igt@gem_...@ctrl-surf-copy.html * igt@gem_eio@in-flight-contexts-10ms: - shard-glk: [PASS][3] -> [TIMEOUT][4] ([i915#3063]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11668/shard-glk9/igt@gem_...@in-flight-contexts-10ms.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104097v2/shard-glk1/igt@gem_...@in-flight-contexts-10ms.html * igt@gem_eio@unwedge-stress: - shard-snb: NOTRUN -> [FAIL][5] ([i915#3354]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104097v2/shard-snb4/igt@gem_...@unwedge-stress.html * igt@gem_exec_fair@basic-none@vcs1: - shard-kbl: [PASS][6] -> [FAIL][7] ([i915#2842]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11668/shard-kbl4/igt@gem_exec_fair@basic-n...@vcs1.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104097v2/shard-kbl1/igt@gem_exec_fair@basic-n...@vcs1.html * igt@gem_exec_fair@basic-none@vecs0: - shard-apl: [PASS][8] -> [FAIL][9] ([i915#2842]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11668/shard-apl4/igt@gem_exec_fair@basic-n...@vecs0.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104097v2/shard-apl4/igt@gem_exec_fair@basic-n...@vecs0.html * igt@gem_exec_fair@basic-pace@bcs0: - shard-iclb: [PASS][10] -> [FAIL][11] ([i915#2842]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11668/shard-iclb2/igt@gem_exec_fair@basic-p...@bcs0.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104097v2/shard-iclb2/igt@gem_exec_fair@basic-p...@bcs0.html * igt@gem_lmem_swapping@parallel-multi: - shard-apl: NOTRUN -> [SKIP][12] ([fdo#109271] / [i915#4613]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104097v2/shard-apl3/igt@gem_lmem_swapp...@parallel-multi.html * igt@gem_lmem_swapping@verify-random: - shard-skl: NOTRUN -> [SKIP][13] ([fdo#109271] / [i915#4613]) +1 similar issue [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104097v2/shard-skl8/igt@gem_lmem_swapp...@verify-random.html * igt@gem_mmap_wc@coherency: - shard-snb: [PASS][14] -> [SKIP][15] ([fdo#109271]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11668/shard-snb2/igt@gem_mmap...@coherency.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104097v2/shard-snb6/igt@gem_mmap...@coherency.html * igt@gem_render_copy@y-tiled-mc-ccs-to-vebox-y-tiled: - shard-iclb: NOTRUN -> [SKIP][16] ([i915#768]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104097v2/shard-iclb6/igt@gem_render_c...@y-tiled-mc-ccs-to-vebox-y-tiled.html * igt@gem_spin_batch@spin-each: - shard-apl: [PASS][17] -> [FAIL][18] ([i915#2898]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11668/shard-apl7/igt@gem_spin_ba...@spin-each.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104097v2/shard-apl2/igt@gem_spin_ba...@spin-each.html * igt@gem_userptr_blits@dmabuf-sync: - shard-iclb: NOTRUN -> [SKIP][19] ([i915#3323]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104097v2/shard-iclb6/igt@gem_userptr_bl...@dmabuf-sync.html * igt@gem_userptr_blits@dmabuf-unsync: - shard-iclb: NOTRUN -> [SKIP][20] ([i915#3297]) [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_1
Re: [Intel-gfx] [RFC PATCH] drm/i915: Debugfs statistics interface for psr
On Wed, 2022-05-18 at 10:45 +0300, Jouni Högander wrote: > Currently there is no mean to get understanding how psr is utilized. > E.g. How much psr is actually enabled or how often driver falls back > to full update. But with this information what can you do? I don't see much value on it. We have now some cases that leads to full update, that needs to be properly fixed so it can always do partial updates. Did not checked the implementation details. > > This patch addresses this by adding new debugfs interface > i915_edp_psr_stats. Statistics gathering is started by writing 1/y/Y and > stopped by writing 0/n/N into this new interface. > > Following fields are provided for reading by this new interface: > > "PSR disabled vblank count" > > Over how many vblank periods PSR was disabled after statistics > gathering got started. I.e. How many normal updates were sent to panel. > > "Total vblank count" > > Total vblank count after statistics gathering got started. > > "Selective update count" > > How many selective updates (PSR2) were done after statistics gathering > got started. > > "Full update count" > > How many times driver decided to fall back to full update when trying to > perform selective update. > > Cc: José Roberto de Souza > Cc: Mika Kahola > Cc: Uma Shankar > Cc: Nischal Varide > Signed-off-by: Jouni Högander > --- > .../drm/i915/display/intel_display_debugfs.c | 100 > .../drm/i915/display/intel_display_types.h| 16 ++ > drivers/gpu/drm/i915/display/intel_psr.c | 144 ++ > drivers/gpu/drm/i915/display/intel_psr.h | 2 + > 4 files changed, 236 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > index 452d773fd4e3..c29f151062e4 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > @@ -9,6 +9,7 @@ > #include > > #include "i915_debugfs.h" > +#include "intel_crtc.h" > #include "intel_de.h" > #include "intel_display_debugfs.h" > #include "intel_display_power.h" > @@ -1868,6 +1869,95 @@ i915_fifo_underrun_reset_write(struct file *filp, > return cnt; > } > > +static int intel_psr_stats(struct seq_file *m, struct intel_dp *intel_dp) > +{ > + struct drm_i915_private *dev_priv = (m->private); > + struct intel_psr *psr = &intel_dp->psr; > + struct drm_crtc *crtc = &intel_crtc_for_pipe(dev_priv, psr->pipe)->base; > + u64 total_vblank_count = psr->stats.total_vblank_count, > + non_psr_vblank_count = psr->stats.non_psr_vblank_count; > + ktime_t vblanktime; > + > + if (!psr->active) > + non_psr_vblank_count += drm_crtc_vblank_count_and_time(crtc, > &vblanktime) - > + psr->stats.psr_disable_vblank; > + > + seq_printf(m, "PSR disabled vblank count : %llu\n", > non_psr_vblank_count); > + > + if (psr->stats.enable) > + total_vblank_count += drm_crtc_vblank_count_and_time(crtc, > &vblanktime) - > + psr->stats.start_vblank; > + > + seq_printf(m, "Total vblank count : %llu\n", total_vblank_count); > + seq_printf(m, "Selective update count : %llu\n", > psr->stats.selective_update_count); > + seq_printf(m, "Full update count : %llu\n", > psr->stats.full_update_count); > + > + return 0; > +} > + > +static int i915_edp_psr_stats_show(struct seq_file *m, void *data) > +{ > + struct drm_i915_private *dev_priv = (m->private); > + struct intel_dp *intel_dp = NULL; > + struct intel_encoder *encoder; > + > + if (!HAS_PSR(dev_priv)) > + return -ENODEV; > + > + /* Find the first EDP which supports PSR */ > + for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) { > + intel_dp = enc_to_intel_dp(encoder); > + break; > + } > + > + if (!intel_dp) > + return -ENODEV; > + > + return intel_psr_stats(m, intel_dp); > +} > + > +static ssize_t > +i915_edp_psr_stats_write(struct file *filp, const char __user *ubuf, > + size_t cnt, loff_t *ppos) > +{ > + struct seq_file *m = filp->private_data; > + struct drm_i915_private *dev_priv = m->private; > + struct intel_dp *intel_dp = NULL; > + struct intel_encoder *encoder; > + int ret; > + bool enable_stats; > + > + ret = kstrtobool_from_user(ubuf, cnt, &enable_stats); > + if (ret) > + return ret; > + > + if (!HAS_PSR(dev_priv)) > + return -ENODEV; > + > + /* Find the first EDP which supports PSR */ > + for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) { > + intel_dp = enc_to_intel_dp(encoder); > + break; > + } > + > + if (!intel_dp) > + return -ENODEV; > + > + if (enable_stats) > + intel_psr_stats_enable_stats(intel_dp); > + else > + intel_psr_stats_
[Intel-gfx] ✗ Fi.CI.BAT: failure for DG2 VRAM_SR Support (rev2)
== Series Details == Series: DG2 VRAM_SR Support (rev2) URL : https://patchwork.freedesktop.org/series/104128/ State : failure == Summary == CI Bug Log - changes from CI_DRM_11674 -> Patchwork_104128v2 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_104128v2 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_104128v2, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104128v2/index.html Participating hosts (48 -> 46) -- Additional (1): bat-dg2-8 Missing(3): bat-jsl-2 bat-dg2-9 fi-bdw-samus Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_104128v2: ### IGT changes ### Possible regressions * igt@i915_suspend@basic-s3-without-i915: - fi-cml-u2: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11674/fi-cml-u2/igt@i915_susp...@basic-s3-without-i915.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104128v2/fi-cml-u2/igt@i915_susp...@basic-s3-without-i915.html Known issues Here are the changes found in Patchwork_104128v2 that come from known issues: ### IGT changes ### Issues hit * igt@i915_selftest@live@gt_engines: - bat-dg1-5: [PASS][3] -> [INCOMPLETE][4] ([i915#4418]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11674/bat-dg1-5/igt@i915_selftest@live@gt_engines.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104128v2/bat-dg1-5/igt@i915_selftest@live@gt_engines.html * igt@i915_selftest@live@requests: - fi-pnv-d510:[PASS][5] -> [DMESG-FAIL][6] ([i915#4528]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11674/fi-pnv-d510/igt@i915_selftest@l...@requests.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104128v2/fi-pnv-d510/igt@i915_selftest@l...@requests.html * igt@kms_pipe_crc_basic@read-crc-pipe-b: - fi-cfl-8109u: [PASS][7] -> [DMESG-WARN][8] ([i915#62]) +16 similar issues [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11674/fi-cfl-8109u/igt@kms_pipe_crc_ba...@read-crc-pipe-b.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104128v2/fi-cfl-8109u/igt@kms_pipe_crc_ba...@read-crc-pipe-b.html Possible fixes * igt@gem_exec_suspend@basic-s0@smem: - {fi-ehl-2}: [DMESG-WARN][9] ([i915#5122]) -> [PASS][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11674/fi-ehl-2/igt@gem_exec_suspend@basic...@smem.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104128v2/fi-ehl-2/igt@gem_exec_suspend@basic...@smem.html * igt@i915_selftest@live@gt_mocs: - {bat-rpls-1}: [DMESG-WARN][11] -> [PASS][12] [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11674/bat-rpls-1/igt@i915_selftest@live@gt_mocs.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104128v2/bat-rpls-1/igt@i915_selftest@live@gt_mocs.html * igt@kms_flip@basic-flip-vs-modeset@a-edp1: - fi-tgl-u2: [DMESG-WARN][13] ([i915#402]) -> [PASS][14] +1 similar issue [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11674/fi-tgl-u2/igt@kms_flip@basic-flip-vs-mode...@a-edp1.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104128v2/fi-tgl-u2/igt@kms_flip@basic-flip-vs-mode...@a-edp1.html * igt@kms_force_connector_basic@force-connector-state: - {bat-adlp-6}: [DMESG-WARN][15] ([i915#3576]) -> [PASS][16] +1 similar issue [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11674/bat-adlp-6/igt@kms_force_connector_ba...@force-connector-state.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104128v2/bat-adlp-6/igt@kms_force_connector_ba...@force-connector-state.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284 [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285 [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827 [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072 [i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155 [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582 [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291 [i915#3547]: https://gitlab.freedesktop.org/drm/intel/issues/3547 [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555 [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576 [i915#3595]: https://gitlab.freedesktop.org/drm/intel/issues/3595 [i915#3708]: ht
Re: [Intel-gfx] [PATCH v3] uapi/drm/i915: Document memory residency and Flat-CCS capability of obj
Media driver never creates a BO with more than one backing regions. Acked-by: Tony Ye Thanks, Tony On 5/2/2022 7:15 AM, Ramalingam C wrote: Capture the impact of memory region preference list of the objects, on their memory residency and Flat-CCS capability. v2: Fix the Flat-CCS capability of an obj with {lmem, smem} preference list [Thomas] v3: Reworded the doc [Matt] Signed-off-by: Ramalingam C cc: Matthew Auld cc: Thomas Hellstrom cc: Daniel Vetter cc: Jon Bloomfield cc: Lionel Landwerlin cc: Kenneth Graunke cc: mesa-...@lists.freedesktop.org cc: Jordan Justen cc: Tony Ye Reviewed-by: Matthew Auld --- include/uapi/drm/i915_drm.h | 16 1 file changed, 16 insertions(+) diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index a2def7b27009..b7e1c2fe08dc 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -3443,6 +3443,22 @@ struct drm_i915_gem_create_ext { * At which point we get the object handle in &drm_i915_gem_create_ext.handle, * along with the final object size in &drm_i915_gem_create_ext.size, which * should account for any rounding up, if required. + * + * Note that userspace has no means of knowing the current backing region + * for objects where @num_regions is larger than one. The kernel will only + * ensure that the priority order of the @regions array is honoured, either + * when initially placing the object, or when moving memory around due to + * memory pressure + * + * On Flat-CCS capable HW, compression is supported for the objects residing + * in I915_MEMORY_CLASS_DEVICE. When such objects (compressed) has other + * memory class in @regions and migrated (by I915, due to memory + * constrain) to the non I915_MEMORY_CLASS_DEVICE region, then I915 needs to + * decompress the content. But I915 dosen't have the required information to + * decompress the userspace compressed objects. + * + * So I915 supports Flat-CCS, only on the objects which can reside only on + * I915_MEMORY_CLASS_DEVICE regions. */ struct drm_i915_gem_create_ext_memory_regions { /** @base: Extension link. See struct i915_user_extension. */
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for DG2 VRAM_SR Support (rev2)
== Series Details == Series: DG2 VRAM_SR Support (rev2) URL : https://patchwork.freedesktop.org/series/104128/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for DG2 VRAM_SR Support (rev2)
== Series Details == Series: DG2 VRAM_SR Support (rev2) URL : https://patchwork.freedesktop.org/series/104128/ State : warning == Summary == Error: dim checkpatch failed ee8880156960 drm/i915/dgfx: OpRegion VRAM Self Refresh Support 41f57c1513eb drm/i915/dg1: OpRegion PCON DG1 MBD config support b20625f7bd5e drm/i915/dg2: DG2 MBD config -:40: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'dev_priv' - possible side-effects? #40: FILE: drivers/gpu/drm/i915/i915_drv.h:1081: +#define IS_DG2_MBD(dev_priv) (IS_DG2(dev_priv) && \ + (INTEL_DEVID(dev_priv) & DG2_MBD_CONFIG_MASK) == DG2_MBD_CONFIG_VAL) total: 0 errors, 0 warnings, 1 checks, 23 lines checked 0bcbc42282e2 drm/i915/dgfx: Add has_lmem_sr b9322552deed drm/i915/pcode: DGFX PCODE MBOX headers 18499773956c drm/i915/dgfx: Setup VRAM SR with D3COLD -:96: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis #96: FILE: drivers/gpu/drm/i915/intel_pcode.c:247: + REG_FIELD_PREP(GEN6_PCODE_MB_COMMAND, + DG1_PCODE_D3_VRAM_SR) | -:98: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis #98: FILE: drivers/gpu/drm/i915/intel_pcode.c:249: + REG_FIELD_PREP(GEN6_PCODE_MB_PARAM1, + DG1_ENABLE_SR), 0); /* no data needed for this cmd */ total: 0 errors, 0 warnings, 2 checks, 127 lines checked f4732fb4b295 drm/i915/rpm: Enable D3Cold VRAM SR Support
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Individualize fences before adding to dma_resv obj (rev2)
== Series Details == Series: drm/i915: Individualize fences before adding to dma_resv obj (rev2) URL : https://patchwork.freedesktop.org/series/104074/ State : failure == Summary == CI Bug Log - changes from CI_DRM_11674 -> Patchwork_104074v2 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_104074v2 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_104074v2, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104074v2/index.html Participating hosts (48 -> 45) -- Additional (2): bat-dg2-8 fi-icl-u2 Missing(5): fi-tgl-dsi bat-dg2-9 bat-rpls-1 bat-jsl-2 fi-bdw-samus Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_104074v2: ### IGT changes ### Possible regressions * igt@i915_suspend@basic-s3-without-i915: - fi-cml-u2: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11674/fi-cml-u2/igt@i915_susp...@basic-s3-without-i915.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104074v2/fi-cml-u2/igt@i915_susp...@basic-s3-without-i915.html Known issues Here are the changes found in Patchwork_104074v2 that come from known issues: ### IGT changes ### Issues hit * igt@gem_huc_copy@huc-copy: - fi-icl-u2: NOTRUN -> [SKIP][3] ([i915#2190]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104074v2/fi-icl-u2/igt@gem_huc_c...@huc-copy.html * igt@gem_lmem_swapping@parallel-random-engines: - fi-icl-u2: NOTRUN -> [SKIP][4] ([i915#4613]) +3 similar issues [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104074v2/fi-icl-u2/igt@gem_lmem_swapp...@parallel-random-engines.html * igt@i915_selftest@live@gt_engines: - bat-dg1-5: [PASS][5] -> [INCOMPLETE][6] ([i915#4418]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11674/bat-dg1-5/igt@i915_selftest@live@gt_engines.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104074v2/bat-dg1-5/igt@i915_selftest@live@gt_engines.html * igt@i915_selftest@live@requests: - fi-pnv-d510:[PASS][7] -> [DMESG-FAIL][8] ([i915#4528]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11674/fi-pnv-d510/igt@i915_selftest@l...@requests.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104074v2/fi-pnv-d510/igt@i915_selftest@l...@requests.html * igt@i915_suspend@basic-s3-without-i915: - fi-icl-u2: NOTRUN -> [SKIP][9] ([i915#5903]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104074v2/fi-icl-u2/igt@i915_susp...@basic-s3-without-i915.html * igt@kms_chamelium@common-hpd-after-suspend: - fi-snb-2600:NOTRUN -> [SKIP][10] ([fdo#109271] / [fdo#111827]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104074v2/fi-snb-2600/igt@kms_chamel...@common-hpd-after-suspend.html * igt@kms_chamelium@hdmi-hpd-fast: - fi-icl-u2: NOTRUN -> [SKIP][11] ([fdo#111827]) +8 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104074v2/fi-icl-u2/igt@kms_chamel...@hdmi-hpd-fast.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy: - fi-icl-u2: NOTRUN -> [SKIP][12] ([fdo#109278]) +2 similar issues [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104074v2/fi-icl-u2/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html * igt@kms_force_connector_basic@force-load-detect: - fi-icl-u2: NOTRUN -> [SKIP][13] ([fdo#109285]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104074v2/fi-icl-u2/igt@kms_force_connector_ba...@force-load-detect.html * igt@kms_setmode@basic-clone-single-crtc: - fi-icl-u2: NOTRUN -> [SKIP][14] ([i915#3555]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104074v2/fi-icl-u2/igt@kms_setm...@basic-clone-single-crtc.html * igt@prime_vgem@basic-userptr: - fi-icl-u2: NOTRUN -> [SKIP][15] ([i915#3301]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104074v2/fi-icl-u2/igt@prime_v...@basic-userptr.html Possible fixes * igt@i915_selftest@live@hangcheck: - bat-dg1-6: [DMESG-FAIL][16] ([i915#4494] / [i915#4957]) -> [PASS][17] [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11674/bat-dg1-6/igt@i915_selftest@l...@hangcheck.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104074v2/bat-dg1-6/igt@i915_selftest@l...@hangcheck.html - fi-snb-2600:[INCOMPLETE][18] ([i915#3921]) -> [PASS][19] [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11674/fi-snb-2600/igt@i915_selftest@l...@han
Re: [Intel-gfx] [PATCH 12/14] drm/nouveau: Register ACPI video backlight when nv_backlight registration fails
On Tue, 2022-05-17 at 17:23 +0200, Hans de Goede wrote: > Typically the acpi_video driver will initialize before nouveau, which > used to cause /sys/class/backlight/acpi_video0 to get registered and then > nouveau would register its own nv_backlight device later. After which > the drivers/acpi/video_detect.c code unregistered the acpi_video0 device > to avoid there being 2 backlight devices. > > This means that userspace used to briefly see 2 devices and the > disappearing of acpi_video0 after a brief time confuses the systemd > backlight level save/restore code, see e.g.: > https://bbs.archlinux.org/viewtopic.php?id=269920 > > To fix this the ACPI video code has been modified to make backlight class > device registration a separate step, relying on the drm/kms driver to > ask for the acpi_video backlight registration after it is done setting up > its native backlight device. > > Add a call to the new acpi_video_register_backlight() when native backlight > device registration has failed / was skipped to ensure that there is a > backlight device available before the drm_device gets registered with > userspace. > > Signed-off-by: Hans de Goede > --- > drivers/gpu/drm/nouveau/nouveau_backlight.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c > b/drivers/gpu/drm/nouveau/nouveau_backlight.c > index f56ff797c78c..0ae8793357a4 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c > +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c > @@ -436,6 +436,13 @@ nouveau_backlight_init(struct drm_connector *connector) > > fail_alloc: > kfree(bl); > + /* > + * If we get here we have an internal panel, but no nv_backlight, > + * try registering an ACPI video backlight device instead. > + */ > + if (ret == 0) > + acpi_video_register_backlight(); Assuming we don't need to return the value of acpi_video_register_backlight() here: Reviewed-by: Lyude Paul > + > return ret; > } > -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
Re: [Intel-gfx] [RFC PATCH v3 02/19] KVM: x86: inhibit APICv/AVIC when the guest and/or host changes apic id/base from the defaults.
On Wed, 2022-05-18 at 15:39 +, Sean Christopherson wrote: > On Wed, May 18, 2022, Maxim Levitsky wrote: > > On Wed, 2022-05-18 at 16:28 +0800, Chao Gao wrote: > > > > struct kvm_arch { > > > > @@ -1258,6 +1260,7 @@ struct kvm_arch { > > > > hpa_t hv_root_tdp; > > > > spinlock_t hv_root_tdp_lock; > > > > #endif > > > > + bool apic_id_changed; > > > > > > What's the value of this boolean? No one reads it. > > > > I use it in later patches to kill the guest during nested VM entry > > if it attempts to use nested AVIC after any vCPU changed APIC ID. > > Then the flag should be introduced in the later patch, because (a) it's dead > code > if that patch is never merged and (b) it's impossible to review this patch for > correctness without seeing the usage, e.g. setting apic_id_changed isn't > guarded > with a lock and so the usage may or may not be susceptible to races. I can't disagree with you on this, this was just somewhat a hack I wasn't sure (and not yet 100% sure I will move forward with) so I cut this corner. Thanks for the review! Best regards, Maxim Levitsky > > > > > + apic->vcpu->kvm->arch.apic_id_changed = true; > > > > +} > > > > +
Re: [Intel-gfx] [RFC PATCH v3 01/19] KVM: x86: document AVIC/APICv inhibit reasons
On Wed, 2022-05-18 at 15:56 +, Sean Christopherson wrote: > On Wed, Apr 27, 2022, Maxim Levitsky wrote: > > These days there are too many AVIC/APICv inhibit > > reasons, and it doesn't hurt to have some documentation > > for them. > > Please wrap at ~75 chars. > > > Signed-off-by: Maxim Levitsky > > --- > > arch/x86/include/asm/kvm_host.h | 15 +++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/arch/x86/include/asm/kvm_host.h > > b/arch/x86/include/asm/kvm_host.h > > index f164c6c1514a4..63eae00625bda 100644 > > --- a/arch/x86/include/asm/kvm_host.h > > +++ b/arch/x86/include/asm/kvm_host.h > > @@ -1046,14 +1046,29 @@ struct kvm_x86_msr_filter { > > }; > > > > enum kvm_apicv_inhibit { > > + /* APICv/AVIC is disabled by module param and/or not supported in > > hardware */ > > Rather than tag every one as APICv vs. AVIC, what about reorganizing the > enums so > that the common vs. AVIC flags are bundled together? And then the redundant > info > in the comments about "XYZ is inhibited" can go away too, i.e. the individual > comments can focus on explaining what triggers the inhibit (and for some, why > that > action is incompatible with APIC virtualization). Very good idea, will do! Best regards, Maxim Levitsky > > E.g. > /***/ > /* INHIBITs are relevant to both Intel's APICv and AMD's AVIC. */ > /***/ > > /* APIC/AVIC is unsupported and/or disabled via module param. */ > APICV_INHIBIT_REASON_DISABLE, > > /* The local APIC is not in-kernel. See KVM_CREATE_IRQCHIP. */ > APICV_INHIBIT_REASON_ABSENT, > > /* >* At least one IRQ vector is configured for HyperV's AutoEOI, which >* requires manually injecting the IRQ to do EOI on behalf of the guest. >*/ > APICV_INHIBIT_REASON_HYPERV, > > > /**/ > /* INHIBITs relevant only to AMD's AVIC. */ > /**/ > > > APICV_INHIBIT_REASON_DISABLE, > > + /* APICv/AVIC is inhibited because AutoEOI feature is being used by a > > HyperV guest*/ > > APICV_INHIBIT_REASON_HYPERV, > > + /* AVIC is inhibited on a CPU because it runs a nested guest */ > > APICV_INHIBIT_REASON_NESTED, > > + /* AVIC is inhibited due to wait for an irq window (AVIC doesn't > > support this) */ > > APICV_INHIBIT_REASON_IRQWIN, > > + /* > > +* AVIC is inhibited because i8254 're-inject' mode is used > > +* which needs EOI intercept which AVIC doesn't support > > +*/ > > APICV_INHIBIT_REASON_PIT_REINJ, > > + /* AVIC is inhibited because the guest has x2apic in its CPUID*/ > > APICV_INHIBIT_REASON_X2APIC, > > + /* AVIC/APICv is inhibited because KVM_GUESTDBG_BLOCKIRQ was enabled */ > > APICV_INHIBIT_REASON_BLOCKIRQ, > > + /* > > +* AVIC/APICv is inhibited because the guest didn't yet > > s/guest/userspace > > > +* enable kernel/split irqchip > > +*/ > > APICV_INHIBIT_REASON_ABSENT, > > + /* AVIC is disabled because SEV doesn't support it */ > > APICV_INHIBIT_REASON_SEV, > > }; > > > > -- > > 2.26.3 > >
[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915/reg: fix undefined behavior due to shift overflowing the constant
== Series Details == Series: series starting with [1/2] drm/i915/reg: fix undefined behavior due to shift overflowing the constant URL : https://patchwork.freedesktop.org/series/104122/ State : failure == Summary == CI Bug Log - changes from CI_DRM_11673_full -> Patchwork_104122v1_full Summary --- **FAILURE** Serious unknown changes coming with Patchwork_104122v1_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_104122v1_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. Participating hosts (13 -> 12) -- Missing(1): shard-rkl Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_104122v1_full: ### IGT changes ### Possible regressions * igt@gem_eio@in-flight-contexts-10ms: - shard-glk: [PASS][1] -> [TIMEOUT][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-glk4/igt@gem_...@in-flight-contexts-10ms.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/shard-glk5/igt@gem_...@in-flight-contexts-10ms.html Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@perf_pmu@module-unload: - {shard-tglu}: [PASS][3] -> [FAIL][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-tglu-4/igt@perf_...@module-unload.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/shard-tglu-6/igt@perf_...@module-unload.html New tests - New tests have been introduced between CI_DRM_11673_full and Patchwork_104122v1_full: ### New IGT tests (1) ### * igt@kms_flip@basic-flip-vs-modeset@d-hdmi-a3: - Statuses : 1 pass(s) - Exec time: [0.66] s Known issues Here are the changes found in Patchwork_104122v1_full that come from known issues: ### CI changes ### Issues hit * boot: - shard-skl: ([PASS][5], [PASS][6], [PASS][7], [PASS][8], [PASS][9], [PASS][10], [PASS][11], [PASS][12], [PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18], [PASS][19], [PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24], [PASS][25]) -> ([PASS][26], [PASS][27], [PASS][28], [PASS][29], [PASS][30], [PASS][31], [PASS][32], [PASS][33], [PASS][34], [PASS][35], [PASS][36], [PASS][37], [PASS][38], [PASS][39], [PASS][40], [PASS][41], [PASS][42], [FAIL][43], [PASS][44], [PASS][45], [PASS][46], [PASS][47], [PASS][48], [PASS][49], [PASS][50]) ([i915#5032]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl9/boot.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl9/boot.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl9/boot.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl8/boot.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl8/boot.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl8/boot.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl7/boot.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl7/boot.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl7/boot.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl6/boot.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl6/boot.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl4/boot.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl4/boot.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl4/boot.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl3/boot.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl1/boot.html [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl1/boot.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl1/boot.html [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl10/boot.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl10/boot.html [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/shard-skl10/boot.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/shard-skl9/boot.html [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/shard-skl9/boot.html [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/shard-skl9/boot.html [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/shard-skl8/boot.html [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchw
Re: [Intel-gfx] [PATCH 05/14] drm/nouveau: Don't register backlight when another backlight should be used
Reviewed-by: Lyude Paul Also, ack on this being pushed to drm-misc, along with any other patches I r-b On Tue, 2022-05-17 at 17:23 +0200, Hans de Goede wrote: > Before this commit when we want userspace to use the acpi_video backlight > device we register both the GPU's native backlight device and acpi_video's > firmware acpi_video# backlight device. This relies on userspace preferring > firmware type backlight devices over native ones. > > Registering 2 backlight devices for a single display really is > undesirable, don't register the GPU's native backlight device when > another backlight device should be used. > > Signed-off-by: Hans de Goede > --- > drivers/gpu/drm/nouveau/nouveau_backlight.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c > b/drivers/gpu/drm/nouveau/nouveau_backlight.c > index daf9f87477ba..f56ff797c78c 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c > +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c > @@ -34,6 +34,8 @@ > #include > #include > > +#include > + > #include "nouveau_drv.h" > #include "nouveau_reg.h" > #include "nouveau_encoder.h" > @@ -404,6 +406,11 @@ nouveau_backlight_init(struct drm_connector *connector) > goto fail_alloc; > } > > + if (acpi_video_get_backlight_type(true) != acpi_backlight_native) { > + NV_INFO(drm, "Skipping nv_backlight registration\n"); > + goto fail_alloc; > + } > + > if (!nouveau_get_backlight_name(backlight_name, bl)) { > NV_ERROR(drm, "Failed to retrieve a unique name for the > backlight interface\n"); > goto fail_alloc; -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for i915: SSEU handling updates (rev4)
Filed a new issue https://gitlab.freedesktop.org/drm/intel/-/issues/6038 igt@kms_flip@flip-vs-panning-interruptible@c-edp1 - dmesg-warn - rcu: INFO: rcu_preempt detected stalls on CPUs/tasks Thanks, Lakshmi. -Original Message- From: Roper, Matthew D Sent: Tuesday, May 17, 2022 8:24 PM To: intel-gfx@lists.freedesktop.org Cc: Vudum, Lakshminarayana Subject: Re: ✗ Fi.CI.IGT: failure for i915: SSEU handling updates (rev4) On Wed, May 18, 2022 at 12:34:11AM +, Patchwork wrote: > == Series Details == > > Series: i915: SSEU handling updates (rev4) > URL : https://patchwork.freedesktop.org/series/103244/ > State : failure > > == Summary == > > CI Bug Log - changes from CI_DRM_11666_full -> Patchwork_103244v4_full > > > Summary > --- > > **FAILURE** > > Serious unknown changes coming with Patchwork_103244v4_full absolutely need > to be > verified manually. > > If you think the reported changes have nothing to do with the changes > introduced in Patchwork_103244v4_full, please notify your bug team to allow > them > to document this new failure mode, which will reduce false positives in CI. > > > > Participating hosts (11 -> 11) > -- > > No changes in participating hosts > > Possible new issues > --- > > Here are the unknown changes that may have been introduced in > Patchwork_103244v4_full: > > ### IGT changes ### > > Possible regressions > > * igt@kms_flip@flip-vs-panning-interruptible@c-edp1: > - shard-iclb: [PASS][1] -> [DMESG-WARN][2] >[1]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-iclb3/igt@kms_flip@flip-vs-panning-interrupti...@c-edp1.html >[2]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103244v4/shard-iclb8/igt@kms_flip@flip-vs-panning-interrupti...@c-edp1.html <3> [607.307030] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks: <3> [607.307121] rcu: 3-...!: (159 GPs behind) idle=834/0/0x0 softirq=27765/27765 fqs=1 (false positive?) <3> [607.307825] rcu: 7-...!: (172 GPs behind) idle=abc/0/0x0 softirq=23642/23643 fqs=1 (false positive?) <4> [607.307934](detected by 0, t=65002 jiffies, g=64137, q=5840) <6> [607.307942] Sending NMI from CPU 0 to CPUs 3: <4> [607.308032] NMI backtrace for cpu 3 skipped: idling at intel_idle+0x67/0xc0 <6> [607.308950] Sending NMI from CPU 0 to CPUs 7: <4> [607.309045] NMI backtrace for cpu 7 skipped: idling at intel_idle+0x67/0xc0 <3> [607.309957] rcu: rcu_preempt kthread timer wakeup didn't happen for 64995 jiffies! g64137 f0x0 RCU_GP_WAIT_FQS(5) ->state=0x402 <3> [607.310053] rcu: Possible timer handling issue on cpu=3 timer-softirq=16787 <3> [607.310110] rcu: rcu_preempt kthread starved for 64998 jiffies! g64137 f0x0 RCU_GP_WAIT_FQS(5) ->state=0x402 ->cpu=3 <3> [607.310195] rcu: Unless rcu_preempt kthread gets sufficient CPU time, OOM is now expected behavior. <3> [607.310267] rcu: RCU grace-period kthread stack dump: <6> [607.310310] task:rcu_preempt state:I stack:14472 pid: 13 ppid: 2 flags:0x4000 <6> [607.310326] Call Trace: <6> [607.310330] <6> [607.310339] __schedule+0x483/0xb50 <6> [607.310353] ? schedule_timeout+0x1b9/0x2e0 <6> [607.310367] schedule+0x3f/0xa0 <6> [607.310376] schedule_timeout+0x1be/0x2e0 <6> [607.310386] ? del_timer_sync+0xb0/0xb0 <6> [607.310398] ? 0x8100 <6> [607.310408] ? rcu_gp_cleanup+0x440/0x440 <6> [607.310413] rcu_gp_fqs_loop+0x273/0x3b0 <6> [607.310427] rcu_gp_kthread+0xb8/0x120 <6> [607.310436] kthread+0xed/0x120 <6> [607.310443] ? kthread_complete_and_exit+0x20/0x20 <6> [607.310452] ret_from_fork+0x1f/0x30 <6> [607.310478] <3> [607.310481] rcu: Stack dump where RCU GP kthread last ran: <6> [607.310527] Sending NMI from CPU 0 to CPUs 3: <4> [607.310602] NMI backtrace for cpu 3 skipped: idling at intel_idle+0x67/0xc0 It's not clear what this is from (no indication it's even related to the graphics driver). Definitely not the kind of thing that the SSEU rework in this series could cause to happen during a display test. Matt > > > Known issues > > > Here are the changes found in Patchwork_103244v4_full that come from known > issues: > > ### CI changes ### > > Possible fixes > > * boot: > - shard-skl: ([PASS][3], [PASS][4], [PASS][5], [PASS][6], > [PASS][7], [PASS][8], [PASS][9], [PASS][10], [PASS][11], [PASS][12], > [PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18], > [PASS][19], [FAIL][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24], > [PASS][25], [PASS][26]) ([i915#5032]) -> ([PASS][27], [PASS][28], [PASS][29], > [PASS][30], [PASS][31], [PASS][32], [PASS][33], [PASS][34], [PASS][35], > [PASS][36], [PASS][37], [PASS][38], [PASS][39], [PASS][40], [PASS][41], > [PASS][42], [PASS][43], [PASS][44], [PASS][45], [PASS][46], [PASS][47], > [PASS][48], [PASS][49]) >[3]: > htt
Re: [Intel-gfx] [PATCH 4/7] drm/i915/pcode: Init pcode on different gt's
On 13/05/2022 02:36, Ashutosh Dixit wrote: Extend pcode initialization to pcode on different gt's. Cc: Tvrtko Ursulin Cc: Jani Nikula Signed-off-by: Ashutosh Dixit --- drivers/gpu/drm/i915/i915_driver.c | 20 ++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index 0e9763868d68..e137bcf021ee 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -520,6 +520,22 @@ static int i915_set_dma_info(struct drm_i915_private *i915) return ret; } +static int i915_pcode_init(struct drm_i915_private *i915) +{ + struct intel_gt *gt; + int id, ret; + + for_each_gt(gt, i915, id) { + ret = intel_pcode_init(gt->uncore); + if (ret) { + drm_err(>->i915->drm, "gt%d: intel_pcode_init failed %d\n", id, ret); + return ret; + } + } + + return 0; +} + /** * i915_driver_hw_probe - setup state requiring device access * @dev_priv: device private @@ -629,7 +645,7 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv) intel_opregion_setup(dev_priv); - ret = intel_pcode_init(&dev_priv->uncore); + ret = i915_pcode_init(dev_priv); if (ret) goto err_msi; @@ -1251,7 +1267,7 @@ static int i915_drm_resume(struct drm_device *dev) disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); - ret = intel_pcode_init(&dev_priv->uncore); + ret = i915_pcode_init(dev_priv); if (ret) return ret; Reviewed-by: Tvrtko Ursulin Regards, Tvrtko
Re: [Intel-gfx] [PATCH v2 00/14] GSC support for XeHP SDV and DG2 platforms
> Subject: [PATCH v2 00/14] GSC support for XeHP SDV and DG2 platforms > > Add GSC support for XeHP SDV and DG2 platforms. > > The series includes changes for the mei driver: > - add ability to use polling instead of interrupts > - add ability to use extended timeouts > - setup extended operational memory for GSC > > The series includes changes for the i915 driver: > - allocate extended operational memory for GSC > - GSC on XeHP SDV offsets and definitions > > Greg KH, please review and ACK the MEI patches. > We are pushing these patches through gfx tree as > the auxiliary device belongs there. Hi Greg, Are there any problems with these patches? Or I did something wrong with this series publishing? -- Thanks, Sasha > > V2: rebase over merged DG1 series and DG2 enablement patch, > fix commit messages > > Alexander Usyskin (5): > drm/i915/gsc: add slow_fw flag to the mei auxiliary device > drm/i915/gsc: add slow_fw flag to the gsc device definition > drm/i915/gsc: add GSC XeHP SDV platform definition > mei: gsc: wait for reset thread on stop > mei: extend timeouts on slow devices. > > Daniele Ceraolo Spurio (1): > HAX: drm/i915: force INTEL_MEI_GSC on for CI > > Tomas Winkler (5): > mei: gsc: use polling instead of interrupts > mei: mkhi: add memory ready command > mei: gsc: setup gsc extended operational memory > mei: debugfs: add pxp mode to devstate in debugfs > drm/i915/gsc: allocate extended operational memory in LMEM > > Vitaly Lubart (3): > drm/i915/gsc: skip irq initialization if using polling > mei: bus: export common mkhi definitions into a separate header > mei: gsc: add transition to PXP mode in resume flow > > drivers/gpu/drm/i915/Kconfig.debug | 1 + > drivers/gpu/drm/i915/gt/intel_gsc.c | 119 +-- > - > drivers/gpu/drm/i915/gt/intel_gsc.h | 3 + > drivers/misc/mei/bus-fixup.c| 105 > drivers/misc/mei/client.c | 14 ++-- > drivers/misc/mei/debugfs.c | 17 > drivers/misc/mei/gsc-me.c | 77 +++--- > drivers/misc/mei/hbm.c | 12 +-- > drivers/misc/mei/hw-me-regs.h | 7 ++ > drivers/misc/mei/hw-me.c| 116 ++- > drivers/misc/mei/hw-me.h| 14 +++- > drivers/misc/mei/hw-txe.c | 2 +- > drivers/misc/mei/hw.h | 5 ++ > drivers/misc/mei/init.c | 21 - > drivers/misc/mei/main.c | 2 +- > drivers/misc/mei/mei_dev.h | 26 ++ > drivers/misc/mei/mkhi.h | 57 + > drivers/misc/mei/pci-me.c | 2 +- > include/linux/mei_aux.h | 2 + > 19 files changed, 511 insertions(+), 91 deletions(-) > create mode 100644 drivers/misc/mei/mkhi.h > > -- > 2.32.0
[Intel-gfx] ✓ Fi.CI.IGT: success for i915: SSEU handling updates (rev4)
== Series Details == Series: i915: SSEU handling updates (rev4) URL : https://patchwork.freedesktop.org/series/103244/ State : success == Summary == CI Bug Log - changes from CI_DRM_11666_full -> Patchwork_103244v4_full Summary --- **SUCCESS** No regressions found. Participating hosts (11 -> 13) -- Additional (2): shard-rkl shard-dg1 Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_103244v4_full: ### IGT changes ### Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@gem_exec_schedule@wide@vcs1: - {shard-dg1}:NOTRUN -> [DMESG-WARN][1] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103244v4/shard-dg1-13/igt@gem_exec_schedule@w...@vcs1.html * igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-25: - {shard-rkl}:NOTRUN -> [INCOMPLETE][2] [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103244v4/shard-rkl-5/igt@kms_plane_scal...@planes-unity-scaling-downscale-factor-0-25.html * igt@kms_vblank@pipe-b-wait-idle-hang: - {shard-dg1}:NOTRUN -> [SKIP][3] +1 similar issue [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103244v4/shard-dg1-15/igt@kms_vbl...@pipe-b-wait-idle-hang.html New tests - New tests have been introduced between CI_DRM_11666_full and Patchwork_103244v4_full: ### New IGT tests (4) ### * igt@kms_plane_scaling@downscale-with-modifier-factor-0-75@pipe-a-hdmi-a-3-downscale-with-modifier: - Statuses : 1 pass(s) - Exec time: [0.64] s * igt@kms_plane_scaling@downscale-with-modifier-factor-0-75@pipe-b-hdmi-a-3-downscale-with-modifier: - Statuses : 1 pass(s) - Exec time: [0.64] s * igt@kms_plane_scaling@downscale-with-modifier-factor-0-75@pipe-c-hdmi-a-3-downscale-with-modifier: - Statuses : 1 pass(s) - Exec time: [0.59] s * igt@kms_plane_scaling@downscale-with-modifier-factor-0-75@pipe-d-hdmi-a-3-downscale-with-modifier: - Statuses : 1 pass(s) - Exec time: [0.60] s Known issues Here are the changes found in Patchwork_103244v4_full that come from known issues: ### CI changes ### Possible fixes * boot: - shard-skl: ([PASS][4], [PASS][5], [PASS][6], [PASS][7], [PASS][8], [PASS][9], [PASS][10], [PASS][11], [FAIL][12], [PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18], [PASS][19], [PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24], [PASS][25], [PASS][26], [PASS][27]) ([i915#5032]) -> ([PASS][28], [PASS][29], [PASS][30], [PASS][31], [PASS][32], [PASS][33], [PASS][34], [PASS][35], [PASS][36], [PASS][37], [PASS][38], [PASS][39], [PASS][40], [PASS][41], [PASS][42], [PASS][43], [PASS][44], [PASS][45], [PASS][46], [PASS][47], [PASS][48], [PASS][49], [PASS][50]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl9/boot.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl6/boot.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl6/boot.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl4/boot.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl4/boot.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl4/boot.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl4/boot.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl3/boot.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl3/boot.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl1/boot.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl1/boot.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl1/boot.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl10/boot.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl10/boot.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl10/boot.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl9/boot.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl9/boot.html [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl8/boot.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl8/boot.html [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl8/boot.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl7/boot.html [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl7/boot.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11666/shard-skl7/boot.html [27]: https://intel-gfx-ci.01.org/tree
Re: [Intel-gfx] [PATCH v2] drm/i915/dg2: Add workaround 22014600077
On Tue, May 17, 2022 at 02:29:05PM -0700, Swathi Dhanavanthri wrote: > Signed-off-by: Swathi Dhanavanthri > --- > drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 + > drivers/gpu/drm/i915/gt/intel_workarounds.c | 10 ++ > 2 files changed, 11 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h > b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > index 98ede9c93f00..2063c8758934 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h > @@ -1068,6 +1068,7 @@ > #define GEN9_ENABLE_GPGPU_PREEMPTION REG_BIT(2) > > #define GEN10_CACHE_MODE_SS _MMIO(0xe420) > +#define ENABLE_EU_COUNT_FOR_TDL_FLUSH REG_BIT(10) The whitespace on this line is still wrong. I guess we can fix that up while applying the patch though. Reviewed-by: Matt Roper > #define ENABLE_PREFETCH_INTO_ICREG_BIT(3) > #define FLOAT_BLEND_OPTIMIZATION_ENABLEREG_BIT(4) > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c > b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index 756807c4b405..73b59ea6fd3b 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -2178,6 +2178,16 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, > struct i915_wa_list *wal) > wa_write_or(wal, GEN12_MERT_MOD_CTRL, FORCE_MISS_FTLB); > } > > + if (IS_DG2_GRAPHICS_STEP(i915, G11, STEP_B0, STEP_FOREVER) || > + IS_DG2_G10(i915)) { > + /* Wa_22014600077:dg2 */ > + wa_add(wal, GEN10_CACHE_MODE_SS, 0, > +_MASKED_BIT_ENABLE(ENABLE_EU_COUNT_FOR_TDL_FLUSH), > +0 /* Wa_14012342262 :write-only reg, so skip > + verification */, > +true); > + } > + > if (IS_DG1_GRAPHICS_STEP(i915, STEP_A0, STEP_B0) || > IS_TGL_UY_GRAPHICS_STEP(i915, STEP_A0, STEP_B0)) { > /* > -- > 2.20.1 > -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation
Re: [Intel-gfx] [PATCH 7/7] drm/i915/rpm: Enable D3Cold VRAM SR Support
> -Original Message- > From: Ville Syrjälä > Sent: Wednesday, May 18, 2022 7:46 PM > To: Gupta, Anshuman > Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani ; > Wilson, Chris P ; Vivi, Rodrigo > > Subject: Re: [Intel-gfx] [PATCH 7/7] drm/i915/rpm: Enable D3Cold VRAM SR > Support > > On Wed, May 18, 2022 at 06:37:16PM +0530, Anshuman Gupta wrote: > > Intel Client DGFX card supports D3Cold with two option. > > D3Cold-off zero watt, D3Cold-VRAM Self Refresh. > > > > i915 requires to evict the lmem objects to smem in order to support > > D3Cold-Off, which increases i915 the suspend/resume latency. Enabling > > VRAM Self Refresh feature optimize the latency with additional power > > cost which required to retain the lmem. > > > > Adding intel_runtime_idle (runtime_idle callback) to enable VRAM_SR, > > it will be used for policy to choose between D3Cold-off vs > > D3Cold-VRAM_SR. > > > > Since we have introduced i915 runtime_idle callback. > > It need to be warranted that Runtime PM Core invokes runtime_idle > > callback when runtime usages count becomes zero. That requires to use > > pm_runtime_put instead of pm_runtime_put_autosuspend. > > > > Cc: Rodrigo Vivi > > Cc: Chris Wilson > > Signed-off-by: Anshuman Gupta > > --- > > drivers/gpu/drm/i915/i915_driver.c | 26 + > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +-- > > 2 files changed, 27 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_driver.c > > b/drivers/gpu/drm/i915/i915_driver.c > > index 5a9d5529fc90..bbb11c632799 100644 > > --- a/drivers/gpu/drm/i915/i915_driver.c > > +++ b/drivers/gpu/drm/i915/i915_driver.c > > @@ -1541,6 +1541,31 @@ static int i915_pm_restore(struct device *kdev) > > return i915_pm_resume(kdev); > > } > > > > +static int intel_runtime_idle(struct device *kdev) { > > + struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > > + int ret = 1; > > + > > + if (!HAS_LMEM_SR(dev_priv)) { > > + /*TODO: Prepare for D3Cold-Off */ > > + goto out; > > + } > > + > > + disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > > + > > + ret = intel_pm_vram_sr(dev_priv, true) > > I don't get why this idle callback is here. Why aren't you just calling that > directly > from the suspend handler? We will be having a D3Cold policy in future to decide between D3Cold-VRAM_SR and D3Cold-Off based upon lmem usages also we need to evict the lmem content if D3cold-off option is used. It will be better to keep it in separate runtime idle call back to prepare the actual suspend. Thanks, Anshuman Gupta. > > > + if (!ret) > > + drm_dbg(&dev_priv->drm, "VRAM Self Refresh enabled\n"); > > + > > + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > > + > > +out: > > + pm_runtime_mark_last_busy(kdev); > > + pm_runtime_autosuspend(kdev); > > + > > + return ret; > > +} > > + > > static int intel_runtime_suspend(struct device *kdev) { > > struct drm_i915_private *dev_priv = kdev_to_i915(kdev); @@ -1726,6 > > +1751,7 @@ const struct dev_pm_ops i915_pm_ops = { > > .restore = i915_pm_restore, > > > > /* S0ix (via runtime suspend) event handlers */ > > + .runtime_idle = intel_runtime_idle, > > .runtime_suspend = intel_runtime_suspend, > > .runtime_resume = intel_runtime_resume, }; diff --git > > a/drivers/gpu/drm/i915/intel_runtime_pm.c > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index 6ed5786bcd29..4dade7e8a795 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -492,8 +492,7 @@ static void __intel_runtime_pm_put(struct > > intel_runtime_pm *rpm, > > > > intel_runtime_pm_release(rpm, wakelock); > > > > - pm_runtime_mark_last_busy(kdev); > > - pm_runtime_put_autosuspend(kdev); > > + pm_runtime_put(kdev); > > } > > > > /** > > -- > > 2.26.2 > > -- > Ville Syrjälä > Intel
Re: [Intel-gfx] [RFC PATCH v2 0/1] Replace shmem memory region and object backend
On Tue, 17 May 2022 at 21:45, Adrian Larumbe wrote: > > This patch is a second attempt at eliminating the old shmem memory region > and GEM object backend, in favour of a TTM-based one that is able to manage > objects placed on both system and local memory. > > Questions addressed since previous revision: > > * Creating an anonymous vfs mount for shmem files in TTM > * Fixing LLC caching properties and bit 17 swizzling before setting a TTM > bo's pages when calling get_pages > * Added handling of phys backend from TTM functions > * Added pread callback to TTM gem object backend > * In shmem_create_from_object, ensuring an shmem object we just got a filp > for has its pages marked dirty and accessed. Otherwise, the engine won't be > able to read the initial state and a GPU hung will ensue > > However, one of the issues persists: > > Many GPU hungs in machines of GEN <= 5. My assumption is this has something > to do with a caching pitfall, but everywhere across the TTM backend code > I've tried to handle object creation and getting its pages with the same > set of caching and coherency properties as in the old shmem backend. Some thoughts in case it's helpful: - We still look to be trampling the cache_level etc after object creation. AFAICT i915_ttm_adjust_gem_after_move can be called in various places after creation. - The i915_ttm_pwrite hook won't play nice on non-llc platforms, since it doesn't force a clflush or keep track of the writes with cache_dirty. The existing ->shmem_pwrite hook only works because we are guaranteed to have not yet populated the mm.pages, and on non-llc platforms we always force a clflush in __set_pages(). In i915_ttm_pwrite we are now just calling pin_pages() and then writing through the page-cache without forcing a clflush, or ensuring that we leave cache_dirty set. Also AFAIK the whole point of shmem_pwrite was to avoid needing to populate the entire object like when calling pin_pages(). Would it make sense to just fallback to using i915_gem_shmem_pwrite, which should already take care of the required flushing? For reference a common usage pattern is something like: bb = gem_create() <-- assume non-llc so must be CACHE_NONE gem_write(bb, BATCH_BUFFER_END) <-- might use cached pwrite internally execbuf(bb) <-- doesn't see BATCH_BUFFER_END if we don't clflush > > Adrian Larumbe (1): > drm/i915: Replace shmem memory region and object backend with TTM > > drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 12 +- > drivers/gpu/drm/i915/gem/i915_gem_mman.c | 32 +- > drivers/gpu/drm/i915/gem/i915_gem_object.h | 4 +- > drivers/gpu/drm/i915/gem/i915_gem_phys.c | 32 +- > drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 390 +-- > drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 267 - > drivers/gpu/drm/i915/gem/i915_gem_ttm.h | 3 + > drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 9 +- > drivers/gpu/drm/i915/gt/shmem_utils.c| 64 ++- > drivers/gpu/drm/i915/intel_memory_region.c | 7 +- > 10 files changed, 398 insertions(+), 422 deletions(-) > > -- > 2.35.1 >
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/reg: fix undefined behavior due to shift overflowing the constant
== Series Details == Series: series starting with [1/2] drm/i915/reg: fix undefined behavior due to shift overflowing the constant URL : https://patchwork.freedesktop.org/series/104122/ State : success == Summary == CI Bug Log - changes from CI_DRM_11673 -> Patchwork_104122v1 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/index.html Participating hosts (47 -> 47) -- Additional (3): bat-adlm-1 fi-icl-u2 fi-pnv-d510 Missing(3): bat-rpls-1 bat-dg2-9 fi-bdw-samus Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_104122v1: ### IGT changes ### Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@i915_selftest@live@gt_contexts: - {bat-adlm-1}: NOTRUN -> [INCOMPLETE][1] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/bat-adlm-1/igt@i915_selftest@live@gt_contexts.html Known issues Here are the changes found in Patchwork_104122v1 that come from known issues: ### IGT changes ### Issues hit * igt@gem_huc_copy@huc-copy: - fi-pnv-d510:NOTRUN -> [SKIP][2] ([fdo#109271]) +39 similar issues [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/fi-pnv-d510/igt@gem_huc_c...@huc-copy.html - fi-icl-u2: NOTRUN -> [SKIP][3] ([i915#2190]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/fi-icl-u2/igt@gem_huc_c...@huc-copy.html * igt@gem_lmem_swapping@parallel-random-engines: - fi-icl-u2: NOTRUN -> [SKIP][4] ([i915#4613]) +3 similar issues [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/fi-icl-u2/igt@gem_lmem_swapp...@parallel-random-engines.html * igt@i915_selftest@live@gem_contexts: - fi-bdw-5557u: [PASS][5] -> [INCOMPLETE][6] ([i915#5502] / [i915#5801]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/fi-bdw-5557u/igt@i915_selftest@live@gem_contexts.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/fi-bdw-5557u/igt@i915_selftest@live@gem_contexts.html * igt@i915_selftest@live@hangcheck: - fi-hsw-g3258: [PASS][7] -> [INCOMPLETE][8] ([i915#3303] / [i915#4785]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/fi-hsw-g3258/igt@i915_selftest@l...@hangcheck.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/fi-hsw-g3258/igt@i915_selftest@l...@hangcheck.html * igt@i915_selftest@live@requests: - fi-blb-e6850: [PASS][9] -> [DMESG-FAIL][10] ([i915#4528]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11673/fi-blb-e6850/igt@i915_selftest@l...@requests.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/fi-blb-e6850/igt@i915_selftest@l...@requests.html - fi-pnv-d510:NOTRUN -> [DMESG-FAIL][11] ([i915#4528]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/fi-pnv-d510/igt@i915_selftest@l...@requests.html * igt@i915_suspend@basic-s3-without-i915: - fi-icl-u2: NOTRUN -> [SKIP][12] ([i915#5903]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/fi-icl-u2/igt@i915_susp...@basic-s3-without-i915.html * igt@kms_chamelium@common-hpd-after-suspend: - fi-snb-2600:NOTRUN -> [SKIP][13] ([fdo#109271] / [fdo#111827]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/fi-snb-2600/igt@kms_chamel...@common-hpd-after-suspend.html * igt@kms_chamelium@hdmi-hpd-fast: - fi-icl-u2: NOTRUN -> [SKIP][14] ([fdo#111827]) +8 similar issues [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/fi-icl-u2/igt@kms_chamel...@hdmi-hpd-fast.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic: - fi-icl-u2: NOTRUN -> [SKIP][15] ([fdo#109278]) +2 similar issues [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/fi-icl-u2/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html * igt@kms_force_connector_basic@force-load-detect: - fi-icl-u2: NOTRUN -> [SKIP][16] ([fdo#109285]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/fi-icl-u2/igt@kms_force_connector_ba...@force-load-detect.html * igt@kms_setmode@basic-clone-single-crtc: - fi-icl-u2: NOTRUN -> [SKIP][17] ([i915#3555]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/fi-icl-u2/igt@kms_setm...@basic-clone-single-crtc.html * igt@prime_vgem@basic-userptr: - fi-icl-u2: NOTRUN -> [SKIP][18] ([i915#3301]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104122v1/fi-icl-u2/igt@prime_v...@basic-userptr.html * igt@runner@aborted: - fi-pnv-d510:NOTRUN -> [FAIL][19] ([fdo#109271] / [i
Re: [Intel-gfx] [PATCH 7/7] drm/i915/rpm: Enable D3Cold VRAM SR Support
On Wed, May 18, 2022 at 06:37:16PM +0530, Anshuman Gupta wrote: > Intel Client DGFX card supports D3Cold with two option. > D3Cold-off zero watt, D3Cold-VRAM Self Refresh. > > i915 requires to evict the lmem objects to smem in order to > support D3Cold-Off, which increases i915 the suspend/resume > latency. Enabling VRAM Self Refresh feature optimize the > latency with additional power cost which required to retain > the lmem. > > Adding intel_runtime_idle (runtime_idle callback) to enable > VRAM_SR, it will be used for policy to choose > between D3Cold-off vs D3Cold-VRAM_SR. > > Since we have introduced i915 runtime_idle callback. > It need to be warranted that Runtime PM Core invokes runtime_idle > callback when runtime usages count becomes zero. That requires > to use pm_runtime_put instead of pm_runtime_put_autosuspend. > > Cc: Rodrigo Vivi > Cc: Chris Wilson > Signed-off-by: Anshuman Gupta > --- > drivers/gpu/drm/i915/i915_driver.c | 26 + > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +-- > 2 files changed, 27 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_driver.c > b/drivers/gpu/drm/i915/i915_driver.c > index 5a9d5529fc90..bbb11c632799 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -1541,6 +1541,31 @@ static int i915_pm_restore(struct device *kdev) > return i915_pm_resume(kdev); > } > > +static int intel_runtime_idle(struct device *kdev) > +{ > + struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > + int ret = 1; > + > + if (!HAS_LMEM_SR(dev_priv)) { > + /*TODO: Prepare for D3Cold-Off */ > + goto out; > + } > + > + disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > + > + ret = intel_pm_vram_sr(dev_priv, true) I don't get why this idle callback is here. Why aren't you just calling that directly from the suspend handler? > + if (!ret) > + drm_dbg(&dev_priv->drm, "VRAM Self Refresh enabled\n"); > + > + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > + > +out: > + pm_runtime_mark_last_busy(kdev); > + pm_runtime_autosuspend(kdev); > + > + return ret; > +} > + > static int intel_runtime_suspend(struct device *kdev) > { > struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > @@ -1726,6 +1751,7 @@ const struct dev_pm_ops i915_pm_ops = { > .restore = i915_pm_restore, > > /* S0ix (via runtime suspend) event handlers */ > + .runtime_idle = intel_runtime_idle, > .runtime_suspend = intel_runtime_suspend, > .runtime_resume = intel_runtime_resume, > }; > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 6ed5786bcd29..4dade7e8a795 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -492,8 +492,7 @@ static void __intel_runtime_pm_put(struct > intel_runtime_pm *rpm, > > intel_runtime_pm_release(rpm, wakelock); > > - pm_runtime_mark_last_busy(kdev); > - pm_runtime_put_autosuspend(kdev); > + pm_runtime_put(kdev); > } > > /** > -- > 2.26.2 -- Ville Syrjälä Intel
Re: [Intel-gfx] [PATCH 2/2] drm/i915/uc: Fix undefined behavior due to shift overflowing the constant
On Wed, 18 May 2022, Michal Wajdeczko wrote: > On 18.05.2022 13:33, Jani Nikula wrote: >> From: Borislav Petkov >> >> Fix: >> >> In file included from :0:0: >> drivers/gpu/drm/i915/gt/uc/intel_guc.c: In function ‘intel_guc_send_mmio’: >> ././include/linux/compiler_types.h:352:38: error: call to >> ‘__compiletime_assert_1047’ \ >> declared with attribute error: FIELD_PREP: mask is not constant >> _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) >> >> and other build errors due to shift overflowing values. >> >> See https://lore.kernel.org/r/ykwq6%2btih8gqp...@zn.tnic for the gory >> details as to why it triggers with older gccs only. >> >> v2 by Jani: >> - Drop the i915_reg.h changes >> >> Cc: Joonas Lahtinen >> Cc: Tvrtko Ursulin >> Cc: Ruiqi GONG >> Cc: Randy Dunlap >> Signed-off-by: Borislav Petkov >> Signed-off-by: Jani Nikula >> --- >> drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 2 +- >> drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h | 2 +- >> drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h | 2 +- >> drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 2 +- >> 4 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h >> b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h >> index be9ac47fa9d0..4ef9990ed7f8 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h >> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h >> @@ -50,7 +50,7 @@ >> >> #define HOST2GUC_SELF_CFG_REQUEST_MSG_LEN >> (GUC_HXG_REQUEST_MSG_MIN_LEN + 3u) >> #define HOST2GUC_SELF_CFG_REQUEST_MSG_0_MBZ >> GUC_HXG_REQUEST_MSG_0_DATA0 >> -#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_KEY (0x << 16) >> +#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_KEY (0xU << 16) > > nit: maybe for consistency we should update all these hex constants to > be explicitly marked as "unsigned" (as that was the intention) and also > maybe we should use lowercase "u" - but both that can be done later, For the guc reg stuff we should probably use REG_GENMASK() and REG_FIELD_PREP() instead, like I did for i915_reg.h. I just don't know about the plethora of other uc headers though... > Reviewed-by: Michal Wajdeczko Thanks, Jani. > >> #define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_LEN (0x << 0) >> #define HOST2GUC_SELF_CFG_REQUEST_MSG_2_VALUE32 >> GUC_HXG_REQUEST_MSG_n_DATAn >> #define HOST2GUC_SELF_CFG_REQUEST_MSG_3_VALUE64 >> GUC_HXG_REQUEST_MSG_n_DATAn >> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h >> b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h >> index c9086a600bce..df83c1cc7c7a 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h >> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h >> @@ -82,7 +82,7 @@ static_assert(sizeof(struct guc_ct_buffer_desc) == 64); >> #define GUC_CTB_HDR_LEN 1u >> #define GUC_CTB_MSG_MIN_LEN GUC_CTB_HDR_LEN >> #define GUC_CTB_MSG_MAX_LEN 256u >> -#define GUC_CTB_MSG_0_FENCE (0x << 16) >> +#define GUC_CTB_MSG_0_FENCE (0xU << 16) >> #define GUC_CTB_MSG_0_FORMAT(0xf << 12) >> #define GUC_CTB_FORMAT_HXG0u >> #define GUC_CTB_MSG_0_RESERVED (0xf << 8) >> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h >> b/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h >> index 29ac823acd4c..7d5ba4d97d70 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h >> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h >> @@ -40,7 +40,7 @@ >> */ >> >> #define GUC_HXG_MSG_MIN_LEN 1u >> -#define GUC_HXG_MSG_0_ORIGIN(0x1 << 31) >> +#define GUC_HXG_MSG_0_ORIGIN(0x1U << 31) >> #define GUC_HXG_ORIGIN_HOST 0u >> #define GUC_HXG_ORIGIN_GUC1u >> #define GUC_HXG_MSG_0_TYPE (0x7 << 28) >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h >> b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h >> index 2516705b9f36..8dc063f087eb 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h >> @@ -28,7 +28,7 @@ >> #define GS_MIA_HALT_REQUESTED (0x02 << GS_MIA_SHIFT) >> #define GS_MIA_ISR_ENTRY(0x04 << GS_MIA_SHIFT) >> #define GS_AUTH_STATUS_SHIFT 30 >> -#define GS_AUTH_STATUS_MASK (0x03 << GS_AUTH_STATUS_SHIFT) >> +#define GS_AUTH_STATUS_MASK (0x03U << >> GS_AUTH_STATUS_SHIFT) >> #define GS_AUTH_STATUS_BAD (0x01 << GS_AUTH_STATUS_SHIFT) >> #define GS_AUTH_STATUS_GOOD (0x02 << GS_AUTH_STATUS_SHIFT) >> -- Jani Nikul
Re: [Intel-gfx] [PATCH 2/2] drm/i915/uc: Fix undefined behavior due to shift overflowing the constant
On 18.05.2022 13:33, Jani Nikula wrote: > From: Borislav Petkov > > Fix: > > In file included from :0:0: > drivers/gpu/drm/i915/gt/uc/intel_guc.c: In function ‘intel_guc_send_mmio’: > ././include/linux/compiler_types.h:352:38: error: call to > ‘__compiletime_assert_1047’ \ > declared with attribute error: FIELD_PREP: mask is not constant > _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) > > and other build errors due to shift overflowing values. > > See https://lore.kernel.org/r/ykwq6%2btih8gqp...@zn.tnic for the gory > details as to why it triggers with older gccs only. > > v2 by Jani: > - Drop the i915_reg.h changes > > Cc: Joonas Lahtinen > Cc: Tvrtko Ursulin > Cc: Ruiqi GONG > Cc: Randy Dunlap > Signed-off-by: Borislav Petkov > Signed-off-by: Jani Nikula > --- > drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 2 +- > drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h | 2 +- > drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h | 2 +- > drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 2 +- > 4 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h > b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h > index be9ac47fa9d0..4ef9990ed7f8 100644 > --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h > +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h > @@ -50,7 +50,7 @@ > > #define HOST2GUC_SELF_CFG_REQUEST_MSG_LEN > (GUC_HXG_REQUEST_MSG_MIN_LEN + 3u) > #define HOST2GUC_SELF_CFG_REQUEST_MSG_0_MBZ > GUC_HXG_REQUEST_MSG_0_DATA0 > -#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_KEY (0x << 16) > +#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_KEY (0xU << 16) nit: maybe for consistency we should update all these hex constants to be explicitly marked as "unsigned" (as that was the intention) and also maybe we should use lowercase "u" - but both that can be done later, Reviewed-by: Michal Wajdeczko > #define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_LEN (0x << 0) > #define HOST2GUC_SELF_CFG_REQUEST_MSG_2_VALUE32 > GUC_HXG_REQUEST_MSG_n_DATAn > #define HOST2GUC_SELF_CFG_REQUEST_MSG_3_VALUE64 > GUC_HXG_REQUEST_MSG_n_DATAn > diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h > b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h > index c9086a600bce..df83c1cc7c7a 100644 > --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h > +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h > @@ -82,7 +82,7 @@ static_assert(sizeof(struct guc_ct_buffer_desc) == 64); > #define GUC_CTB_HDR_LEN 1u > #define GUC_CTB_MSG_MIN_LEN GUC_CTB_HDR_LEN > #define GUC_CTB_MSG_MAX_LEN 256u > -#define GUC_CTB_MSG_0_FENCE (0x << 16) > +#define GUC_CTB_MSG_0_FENCE (0xU << 16) > #define GUC_CTB_MSG_0_FORMAT (0xf << 12) > #define GUC_CTB_FORMAT_HXG 0u > #define GUC_CTB_MSG_0_RESERVED (0xf << 8) > diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h > b/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h > index 29ac823acd4c..7d5ba4d97d70 100644 > --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h > +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h > @@ -40,7 +40,7 @@ > */ > > #define GUC_HXG_MSG_MIN_LEN 1u > -#define GUC_HXG_MSG_0_ORIGIN (0x1 << 31) > +#define GUC_HXG_MSG_0_ORIGIN (0x1U << 31) > #define GUC_HXG_ORIGIN_HOST0u > #define GUC_HXG_ORIGIN_GUC 1u > #define GUC_HXG_MSG_0_TYPE (0x7 << 28) > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h > b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h > index 2516705b9f36..8dc063f087eb 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h > @@ -28,7 +28,7 @@ > #define GS_MIA_HALT_REQUESTED(0x02 << GS_MIA_SHIFT) > #define GS_MIA_ISR_ENTRY (0x04 << GS_MIA_SHIFT) > #define GS_AUTH_STATUS_SHIFT 30 > -#define GS_AUTH_STATUS_MASK (0x03 << GS_AUTH_STATUS_SHIFT) > +#define GS_AUTH_STATUS_MASK (0x03U << > GS_AUTH_STATUS_SHIFT) > #define GS_AUTH_STATUS_BAD (0x01 << GS_AUTH_STATUS_SHIFT) > #define GS_AUTH_STATUS_GOOD (0x02 << GS_AUTH_STATUS_SHIFT) >
Re: [Intel-gfx] [greybus-dev] Re: [PATCH] [v2] Kbuild: move to -std=gnu11
On 5/18/22 00:46, Arnd Bergmann wrote: On Mon, May 16, 2022 at 3:19 PM Guenter Roeck wrote: On 5/16/22 06:31, Greg KH wrote: On Mon, May 16, 2022 at 06:10:23AM -0700, Guenter Roeck wrote: On Mon, Feb 28, 2022 at 11:27:43AM +0100, Arnd Bergmann wrote: From: Arnd Bergmann During a patch discussion, Linus brought up the option of changing the C standard version from gnu89 to gnu99, which allows using variable declaration inside of a for() loop. While the C99, C11 and later standards introduce many other features, most of these are already available in gnu89 as GNU extensions as well. The downside is that backporting affected patches to older kernel branches now fails with error messages such as mm/kfence/core.c: In function ‘kfence_init_pool’: mm/kfence/core.c:595:2: error: ‘for’ loop initial declarations are only allowed in C99 or C11 mode Just something to keep in mind when writing patches. I just ran across this very issue on this commit. It's an easy fixup for 5.17.y to make this work, so I did that in my tree. If this gets to be too much, we might need to reconsider adding c11 to older stable kernels. I think I'll do just that for ChromeOS; I don't want to have to deal with the backports, and we are using recent compilers anyway. I think it would be better not to have the --std=gnu11 change in the older stable kernels by default, as this has introduced build warnings and other smaller issues, as well as raising the minimum compiler version. The users that are stuck on older kernels for some reason tend to overlap with those on older compilers. One example here is Android, which used to ship with a gcc-4.9 build as the only non-clang toolchain, and was using this for building their kernels. If someone wants to pull in stable updates into an older Android, this would fail with -std=gnu11. Others may be in the same situation. Changing some of the 5.x stable branches to -std=gnu11 is probably less of a problem, but I would not know where to draw the line exactly. Maybe check with the Android team to see what the newest kernel is that they expect to be built with the old gcc-4.9. I don't think they still build anything with gcc. We (ChromeOS) only need it for test builds of chromeos-4.4 (sigh), and that will hopefully be gone in a couple of months. We already enabled -std=gnu11 in chromeos-5.10 and chromeos-5.15. We'll see if that is possible with chromeos-5.4 as well. We won't bother with older kernel branches, but those should not get many patches from upstream outside stable release merges, so it is less of a problem. Guenter
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915/reg: fix undefined behavior due to shift overflowing the constant
== Series Details == Series: series starting with [1/2] drm/i915/reg: fix undefined behavior due to shift overflowing the constant URL : https://patchwork.freedesktop.org/series/104122/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
Re: [Intel-gfx] [PATCH 1/2] drm/i915/reg: fix undefined behavior due to shift overflowing the constant
On Wed, 18 May 2022, Ville Syrjälä wrote: > On Wed, May 18, 2022 at 02:33:14PM +0300, Jani Nikula wrote: >> Use REG_GENMASK() and REG_FIELD_PREP() to avoid errors due to >> -fsanitize=shift. > > I presume it's just unhappy about shifting into the sign bit? Yeah, and apparently it also only happens on some GCC versions. *shrug*. > > Changes look correct: > Reviewed-by: Ville Syrjälä Thanks, Jani. > >> >> References: https://lore.kernel.org/r/20220405151517.29753-12...@alien8.de >> Reported-by: Borislav Petkov >> Reported-by: Ruiqi GONG >> Cc: Randy Dunlap >> Signed-off-by: Jani Nikula >> --- >> drivers/gpu/drm/i915/i915_reg.h | 32 >> 1 file changed, 16 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h >> index 321a08281a3f..dff3f88d8090 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -7607,25 +7607,25 @@ enum skl_power_gate { >> #define _PORT_CLK_SEL_A 0x46100 >> #define _PORT_CLK_SEL_B 0x46104 >> #define PORT_CLK_SEL(port) _MMIO_PORT(port, _PORT_CLK_SEL_A, >> _PORT_CLK_SEL_B) >> -#define PORT_CLK_SEL_LCPLL_2700(0 << 29) >> -#define PORT_CLK_SEL_LCPLL_1350(1 << 29) >> -#define PORT_CLK_SEL_LCPLL_810 (2 << 29) >> -#define PORT_CLK_SEL_SPLL (3 << 29) >> -#define PORT_CLK_SEL_WRPLL(pll)(((pll) + 4) << 29) >> -#define PORT_CLK_SEL_WRPLL1(4 << 29) >> -#define PORT_CLK_SEL_WRPLL2(5 << 29) >> -#define PORT_CLK_SEL_NONE (7 << 29) >> -#define PORT_CLK_SEL_MASK (7 << 29) >> +#define PORT_CLK_SEL_MASK REG_GENMASK(31, 29) >> +#define PORT_CLK_SEL_LCPLL_2700REG_FIELD_PREP(PORT_CLK_SEL_MASK, 0) >> +#define PORT_CLK_SEL_LCPLL_1350REG_FIELD_PREP(PORT_CLK_SEL_MASK, 1) >> +#define PORT_CLK_SEL_LCPLL_810 >> REG_FIELD_PREP(PORT_CLK_SEL_MASK, 2) >> +#define PORT_CLK_SEL_SPLL REG_FIELD_PREP(PORT_CLK_SEL_MASK, 3) >> +#define PORT_CLK_SEL_WRPLL(pll)REG_FIELD_PREP(PORT_CLK_SEL_MASK, 4 + >> (pll)) >> +#define PORT_CLK_SEL_WRPLL1 >> REG_FIELD_PREP(PORT_CLK_SEL_MASK, 4) >> +#define PORT_CLK_SEL_WRPLL2 >> REG_FIELD_PREP(PORT_CLK_SEL_MASK, 5) >> +#define PORT_CLK_SEL_NONE REG_FIELD_PREP(PORT_CLK_SEL_MASK, 7) >> >> /* On ICL+ this is the same as PORT_CLK_SEL, but all bits change. */ >> #define DDI_CLK_SEL(port) PORT_CLK_SEL(port) >> -#define DDI_CLK_SEL_NONE (0x0 << 28) >> -#define DDI_CLK_SEL_MG (0x8 << 28) >> -#define DDI_CLK_SEL_TBT_162(0xC << 28) >> -#define DDI_CLK_SEL_TBT_270(0xD << 28) >> -#define DDI_CLK_SEL_TBT_540(0xE << 28) >> -#define DDI_CLK_SEL_TBT_810(0xF << 28) >> -#define DDI_CLK_SEL_MASK (0xF << 28) >> +#define DDI_CLK_SEL_MASK REG_GENMASK(31, 28) >> +#define DDI_CLK_SEL_NONE REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0x0) >> +#define DDI_CLK_SEL_MG >> REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0x8) >> +#define DDI_CLK_SEL_TBT_162 >> REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xC) >> +#define DDI_CLK_SEL_TBT_270 >> REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xD) >> +#define DDI_CLK_SEL_TBT_540 >> REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xE) >> +#define DDI_CLK_SEL_TBT_810 >> REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xF) >> >> /* Transcoder clock selection */ >> #define _TRANS_CLK_SEL_A0x46140 >> -- >> 2.30.2 -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 2/2] x86/pat: add functions to query specific cache mode availability
On Tue, May 03, 2022 at 03:22:07PM +0200, Juergen Gross wrote: > Some drivers are using pat_enabled() in order to test availability of > special caching modes (WC and UC-). This will lead to false negatives > in case the system was booted e.g. with the "nopat" variant and the > BIOS did setup the PAT MSR supporting the queried mode, or if the > system is running as a Xen PV guest. > > Add test functions for those caching modes instead and use them at the > appropriate places. > > For symmetry reasons export the already existing x86_has_pat_wp() for > modules, too. No, we never export unused functionality.
[Intel-gfx] [PATCH 7/7] drm/i915/rpm: Enable D3Cold VRAM SR Support
Intel Client DGFX card supports D3Cold with two option. D3Cold-off zero watt, D3Cold-VRAM Self Refresh. i915 requires to evict the lmem objects to smem in order to support D3Cold-Off, which increases i915 the suspend/resume latency. Enabling VRAM Self Refresh feature optimize the latency with additional power cost which required to retain the lmem. Adding intel_runtime_idle (runtime_idle callback) to enable VRAM_SR, it will be used for policy to choose between D3Cold-off vs D3Cold-VRAM_SR. Since we have introduced i915 runtime_idle callback. It need to be warranted that Runtime PM Core invokes runtime_idle callback when runtime usages count becomes zero. That requires to use pm_runtime_put instead of pm_runtime_put_autosuspend. Cc: Rodrigo Vivi Cc: Chris Wilson Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/i915_driver.c | 26 + drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index 5a9d5529fc90..bbb11c632799 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -1541,6 +1541,31 @@ static int i915_pm_restore(struct device *kdev) return i915_pm_resume(kdev); } +static int intel_runtime_idle(struct device *kdev) +{ + struct drm_i915_private *dev_priv = kdev_to_i915(kdev); + int ret = 1; + + if (!HAS_LMEM_SR(dev_priv)) { + /*TODO: Prepare for D3Cold-Off */ + goto out; + } + + disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); + + ret = intel_pm_vram_sr(dev_priv, true); + if (!ret) + drm_dbg(&dev_priv->drm, "VRAM Self Refresh enabled\n"); + + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); + +out: + pm_runtime_mark_last_busy(kdev); + pm_runtime_autosuspend(kdev); + + return ret; +} + static int intel_runtime_suspend(struct device *kdev) { struct drm_i915_private *dev_priv = kdev_to_i915(kdev); @@ -1726,6 +1751,7 @@ const struct dev_pm_ops i915_pm_ops = { .restore = i915_pm_restore, /* S0ix (via runtime suspend) event handlers */ + .runtime_idle = intel_runtime_idle, .runtime_suspend = intel_runtime_suspend, .runtime_resume = intel_runtime_resume, }; diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 6ed5786bcd29..4dade7e8a795 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -492,8 +492,7 @@ static void __intel_runtime_pm_put(struct intel_runtime_pm *rpm, intel_runtime_pm_release(rpm, wakelock); - pm_runtime_mark_last_busy(kdev); - pm_runtime_put_autosuspend(kdev); + pm_runtime_put(kdev); } /** -- 2.26.2
[Intel-gfx] [PATCH 5/7] drm/i915/pcode: DGFX PCODE MBOX headers
DGFX uses similar PCODE MBOX interface as IGFX but uses distinct COMMAND and PARAM set of bit fields. Adding those headers Accordingly. Cc: Rodrigo Vivi Signed-off-by: Ashutosh Dixit Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 321a08281a3f..ec2609fa233b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6735,6 +6735,9 @@ #define GEN6_PCODE_MAILBOX _MMIO(0x138124) #define GEN6_PCODE_READY (1 << 31) +#define GEN6_PCODE_MB_PARAM2 REG_GENMASK(23, 16) +#define GEN6_PCODE_MB_PARAM1 REG_GENMASK(15, 8) +#define GEN6_PCODE_MB_COMMAND REG_GENMASK(7, 0) #define GEN6_PCODE_ERROR_MASK0xFF #define GEN6_PCODE_SUCCESS 0x0 #define GEN6_PCODE_ILLEGAL_CMD 0x1 -- 2.26.2
[Intel-gfx] [PATCH 6/7] drm/i915/dgfx: Setup VRAM SR with D3COLD
Setup VRAM Self Refresh with D3COLD state. VRAM Self Refresh will retain the context of VRAM, driver need to save any corresponding hardware state that needs to be restore on D3COLD exit, example PCI state. Cc: Jani Nikula Cc: Rodrigo Vivi Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/i915_driver.c | 2 ++ drivers/gpu/drm/i915/i915_drv.h| 7 + drivers/gpu/drm/i915/i915_reg.h| 4 +++ drivers/gpu/drm/i915/intel_pcode.c | 25 + drivers/gpu/drm/i915/intel_pcode.h | 1 + drivers/gpu/drm/i915/intel_pm.c| 43 ++ drivers/gpu/drm/i915/intel_pm.h| 2 ++ 7 files changed, 84 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index ed6028fd442d..5a9d5529fc90 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -633,6 +633,8 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv) if (ret) goto err_msi; + intel_pm_vram_sr_setup(dev_priv); + /* * Fill the dram structure to get the system dram info. This will be * used for memory latency calculation. diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 42463dc2979f..e15207e6a166 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -688,6 +688,13 @@ struct drm_i915_private { u32 bxt_phy_grc; u32 suspend_count; + + struct { + /* lock to protect vram_sr flags */ + struct mutex lock; + bool supported; + } vram_sr; + struct i915_suspend_saved_registers regfile; struct vlv_s0ix_state *vlv_s0ix_state; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index ec2609fa233b..50e6c7266f7a 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6803,11 +6803,15 @@ #define DG1_PCODE_STATUS 0x7E #define DG1_UNCORE_GET_INIT_STATUS 0x0 #define DG1_UNCORE_INIT_STATUS_COMPLETE0x1 +#define DG1_PCODE_D3_VRAM_SR 0x71 +#define DG1_ENABLE_SR0x1 #define GEN12_PCODE_READ_SAGV_BLOCK_TIME_US0x23 #define GEN6_PCODE_DATA_MMIO(0x138128) #define GEN6_PCODE_FREQ_IA_RATIO_SHIFT 8 #define GEN6_PCODE_FREQ_RING_RATIO_SHIFT 16 #define GEN6_PCODE_DATA1 _MMIO(0x13812C) +#define VRAM_CAPABILITY _MMIO(0x138144) +#define VRAM_SUPPORTEDREG_BIT(0) /* IVYBRIDGE DPF */ #define GEN7_L3CDERRST1(slice) _MMIO(0xB008 + (slice) * 0x200) /* L3CD Error Status 1 */ diff --git a/drivers/gpu/drm/i915/intel_pcode.c b/drivers/gpu/drm/i915/intel_pcode.c index ac727546868e..43b2e7cfc458 100644 --- a/drivers/gpu/drm/i915/intel_pcode.c +++ b/drivers/gpu/drm/i915/intel_pcode.c @@ -225,3 +225,28 @@ int intel_pcode_init(struct drm_i915_private *i915) return ret; } + +/** + * intel_pcode_enable_vram_sr - Enable pcode vram_sr. + * @dev_priv: i915 device + * + * This function triggers the required pcode flow to enable vram_sr. + * This function stictly need to call from rpm handlers, as i915 is + * transitioning to rpm idle/suspend, it doesn't require to grab + * rpm wakeref. + */ +int intel_pcode_enable_vram_sr(struct drm_i915_private *i915) +{ + int ret = 0; + + if (!HAS_LMEM_SR(i915)) + return ret; + + ret = snb_pcode_write(i915, + REG_FIELD_PREP(GEN6_PCODE_MB_COMMAND, + DG1_PCODE_D3_VRAM_SR) | + REG_FIELD_PREP(GEN6_PCODE_MB_PARAM1, + DG1_ENABLE_SR), 0); /* no data needed for this cmd */ + + return ret; +} diff --git a/drivers/gpu/drm/i915/intel_pcode.h b/drivers/gpu/drm/i915/intel_pcode.h index 0962a17fac48..3f695bd027a1 100644 --- a/drivers/gpu/drm/i915/intel_pcode.h +++ b/drivers/gpu/drm/i915/intel_pcode.h @@ -20,5 +20,6 @@ int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request, u32 reply_mask, u32 reply, int timeout_base_ms); int intel_pcode_init(struct drm_i915_private *i915); +int intel_pcode_enable_vram_sr(struct drm_i915_private *i915); #endif /* _INTEL_PCODE_H */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index ee0047fdc95d..6c14752f2dc8 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -8153,6 +8153,49 @@ void intel_pm_setup(struct drm_i915_private *dev_priv) atomic_set(&dev_priv->runtime_pm.wakeref_count, 0); } +void intel_pm_vram_sr_setup(struct drm_i915_private *i915) +{ + if (!HAS_LMEM_SR(i915)) + return; + + mutex_init(&i915->vram_sr.lock); + + i915->vram_sr.supported = intel_uncore_read(&i915->uncore, +
[Intel-gfx] [PATCH 4/7] drm/i915/dgfx: Add has_lmem_sr
Add has_lmem_sr platform specific flag to know, whether platform has VRAM self refresh support. As of now both DG1 and DG2 client platforms supports VRAM self refresh with D3Cold but let it enable first on DG2 as primary lead platform for D3Cold support. Let it get enable on DG1 once this feature is stable. Cc: Rodrigo Vivi Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_pci.c | 2 ++ drivers/gpu/drm/i915/intel_device_info.h | 1 + 3 files changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index c5ecc490dced..42463dc2979f 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1360,6 +1360,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, #define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i)) #define HAS_LMEM(i915) HAS_REGION(i915, REGION_LMEM) +#define HAS_LMEM_SR(i915) (INTEL_INFO(i915)->has_lmem_sr) /* * Platform has the dedicated compression control state for each lmem surfaces diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index d8d893bafa51..3347e3cce0a8 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -889,6 +889,7 @@ static const struct intel_device_info dg1_info = { DGFX_FEATURES, .graphics.rel = 10, PLATFORM(INTEL_DG1), + .has_lmem_sr = 0, .display.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C) | BIT(PIPE_D), .require_force_probe = 1, .platform_engine_mask = @@ -1036,6 +1037,7 @@ static const struct intel_device_info xehpsdv_info = { static const struct intel_device_info dg2_info = { DG2_FEATURES, XE_LPD_FEATURES, + .has_lmem_sr = 1, .display.cpu_transcoder_mask = BIT(TRANSCODER_A) | BIT(TRANSCODER_B) | BIT(TRANSCODER_C) | BIT(TRANSCODER_D), .require_force_probe = 1, diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index 60fc35ae81df..44bd993ef7fb 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -151,6 +151,7 @@ enum intel_ppgtt_type { func(has_l3_ccs_read); \ func(has_l3_dpf); \ func(has_llc); \ + func(has_lmem_sr); \ func(has_logical_ring_contexts); \ func(has_mslices); \ func(has_pooled_eu); \ -- 2.26.2
[Intel-gfx] [PATCH 3/7] drm/i915/dg2: DG2 MBD config
Add DG2 Motherboard Down Config check support. BSpec: 44477 Cc: Rodrigo Vivi Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/display/intel_opregion.c | 2 ++ drivers/gpu/drm/i915/i915_drv.h | 9 + 2 files changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c index 3dcd54517b89..dec7628522c5 100644 --- a/drivers/gpu/drm/i915/display/intel_opregion.c +++ b/drivers/gpu/drm/i915/display/intel_opregion.c @@ -1271,6 +1271,8 @@ intel_opregion_vram_sr_required(struct drm_i915_private *i915) if (IS_DG1(i915)) return intel_opregion_dg1_mbd_config(i915); + else if (IS_DG2_MBD(i915)) + return true; return false; } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 10f273800645..c5ecc490dced 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1071,6 +1071,15 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G11) #define IS_DG2_G12(dev_priv) \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G12) +/* + * FIXME: Need to define a new SUBPLATFORM INTEL_SUBPLATFORM_DG2_MBD + * for PCI id range 5690..5695, but G10, G11 SUBPLATFROM conflicts + * with those pci id range. + */ +#define DG2_MBD_CONFIG_MASKGENMASK(7, 4) +#define DG2_MBD_CONFIG_VAL FIELD_PREP(DG2_MBD_CONFIG_MASK, 9) +#define IS_DG2_MBD(dev_priv) (IS_DG2(dev_priv) && \ + (INTEL_DEVID(dev_priv) & DG2_MBD_CONFIG_MASK) == DG2_MBD_CONFIG_VAL) #define IS_ADLS_RPLS(dev_priv) \ IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_S, INTEL_SUBPLATFORM_RPL) #define IS_ADLP_N(dev_priv) \ -- 2.26.2
[Intel-gfx] [PATCH 2/7] drm/i915/dg1: OpRegion PCON DG1 MBD config support
DGFX cards support both Add in Card(AIC) and Mother Board Down(MBD) configs. MBD config requires HOST BIOS GPIO toggling support in order to enable/disable VRAM SR using ACPI OpRegion. i915 requires to check OpRegion PCON MBD Config bits to discover whether Gfx Card is MBD config before enabling VRSR. BSpec: 53440 Cc: Jani Nikula Cc: Rodrigo Vivi Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/display/intel_opregion.c | 36 +++ drivers/gpu/drm/i915/display/intel_opregion.h | 6 2 files changed, 42 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c index a728f4c2f532..3dcd54517b89 100644 --- a/drivers/gpu/drm/i915/display/intel_opregion.c +++ b/drivers/gpu/drm/i915/display/intel_opregion.c @@ -53,6 +53,8 @@ #define MBOX_ASLE_EXT BIT(4) /* Mailbox #5 */ #define MBOX_BACKLIGHT BIT(5) /* Mailbox #2 (valid from v3.x) */ +#define PCON_DG1_MBD_CONFIGBIT(9) +#define PCON_DG1_MBD_CONFIG_FIELD_VALIDBIT(10) #define PCON_DGFX_BIOS_SUPPORTS_VRSR BIT(11) #define PCON_DGFX_BIOS_SUPPORTS_VRSR_FIELD_VALID BIT(12) @@ -1242,6 +1244,37 @@ void intel_opregion_unregister(struct drm_i915_private *i915) opregion->lid_state = NULL; } +static bool intel_opregion_dg1_mbd_config(struct drm_i915_private *i915) +{ + struct intel_opregion *opregion = &i915->opregion; + + if (!IS_DG1(i915)) + return false; + + if (opregion->header->pcon & PCON_DG1_MBD_CONFIG_FIELD_VALID) + return opregion->header->pcon & PCON_DG1_MBD_CONFIG; + else + return false; +} + +/** + * intel_opregion_vram_sr_required(). + * @i915 i915 device priv data. + * It returns a boolean whether opregion vram_sr support + * is required. + */ +bool +intel_opregion_vram_sr_required(struct drm_i915_private *i915) +{ + if (!IS_DGFX(i915)) + return false; + + if (IS_DG1(i915)) + return intel_opregion_dg1_mbd_config(i915); + + return false; +} + /* * intel_opregion_bios_supports_vram_sr() get HOST BIOS VRAM Self * Refresh capability support. @@ -1269,6 +1302,9 @@ void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable) { struct intel_opregion *opregion = &i915->opregion; + if (!intel_opregion_vram_sr_required(i915)) + return; + if (drm_WARN(&i915->drm, !opregion->asle, "ASLE MAILBOX3 is not available\n")) return; diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h b/drivers/gpu/drm/i915/display/intel_opregion.h index 0aa1c4a8a482..a74686aa3cc3 100644 --- a/drivers/gpu/drm/i915/display/intel_opregion.h +++ b/drivers/gpu/drm/i915/display/intel_opregion.h @@ -77,6 +77,7 @@ int intel_opregion_get_panel_type(struct drm_i915_private *dev_priv); struct edid *intel_opregion_get_edid(struct intel_connector *connector); bool intel_opregion_bios_supports_vram_sr(struct drm_i915_private *i915); void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable); +bool intel_opregion_vram_sr_required(struct drm_i915_private *i915); #else /* CONFIG_ACPI*/ @@ -138,6 +139,11 @@ static void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable) { } +static bool intel_opregion_vram_sr_required(struct drm_i915_private *i915) +{ + return false; +} + #endif /* CONFIG_ACPI */ #endif -- 2.26.2
[Intel-gfx] [PATCH 1/7] drm/i915/dgfx: OpRegion VRAM Self Refresh Support
Intel DGFX cards provides a feature Video Ram Self Refrsh(VRSR). DGFX VRSR can be enabled with runtime suspend D3Cold flow and with opportunistic S0ix system wide suspend flow as well. Without VRSR enablement i915 has to evict the lmem objects to system memory. Depending on some heuristics driver will evict lmem objects without VRSR. VRSR feature requires Host BIOS support, VRSR will be enable/disable by HOST BIOS using ACPI OpRegion. Adding OpRegion VRSR support in order to enable/disable VRSR on discrete cards. BSpec: 53440 Cc: Jani Nikula Cc: Rodrigo Vivi Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/display/intel_opregion.c | 47 ++- drivers/gpu/drm/i915/display/intel_opregion.h | 11 + 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c index f31e8c3f8ce0..a728f4c2f532 100644 --- a/drivers/gpu/drm/i915/display/intel_opregion.c +++ b/drivers/gpu/drm/i915/display/intel_opregion.c @@ -53,6 +53,9 @@ #define MBOX_ASLE_EXT BIT(4) /* Mailbox #5 */ #define MBOX_BACKLIGHT BIT(5) /* Mailbox #2 (valid from v3.x) */ +#define PCON_DGFX_BIOS_SUPPORTS_VRSR BIT(11) +#define PCON_DGFX_BIOS_SUPPORTS_VRSR_FIELD_VALID BIT(12) + struct opregion_header { u8 signature[16]; u32 size; @@ -128,7 +131,8 @@ struct opregion_asle { u64 rvda; /* Physical (2.0) or relative from opregion (2.1+) * address of raw VBT data. */ u32 rvds; /* Size of raw vbt data */ - u8 rsvd[58]; + u8 vrsr;/* DGFX Video Ram Self Refresh */ + u8 rsvd[57]; } __packed; /* OpRegion mailbox #5: ASLE ext */ @@ -199,6 +203,9 @@ struct opregion_asle_ext { #define ASLE_PHED_EDID_VALID_MASK 0x3 +/* VRAM SR */ +#define ASLE_VRSR_ENABLE BIT(0) + /* Software System Control Interrupt (SWSCI) */ #define SWSCI_SCIC_INDICATOR (1 << 0) #define SWSCI_SCIC_MAIN_FUNCTION_SHIFT 1 @@ -919,6 +926,8 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv) opregion->header->over.minor, opregion->header->over.revision); + drm_dbg(&dev_priv->drm, "OpRegion PCON values 0x%x\n", opregion->header->pcon); + mboxes = opregion->header->mboxes; if (mboxes & MBOX_ACPI) { drm_dbg(&dev_priv->drm, "Public ACPI methods supported\n"); @@ -1232,3 +1241,39 @@ void intel_opregion_unregister(struct drm_i915_private *i915) opregion->vbt = NULL; opregion->lid_state = NULL; } + +/* + * intel_opregion_bios_supports_vram_sr() get HOST BIOS VRAM Self + * Refresh capability support. + * @i915: pointer to i915 device. + */ +bool intel_opregion_bios_supports_vram_sr(struct drm_i915_private *i915) +{ + struct intel_opregion *opregion = &i915->opregion; + + if (!IS_DGFX(i915)) + return false; + + if (opregion->header->pcon & PCON_DGFX_BIOS_SUPPORTS_VRSR_FIELD_VALID) + return opregion->header->pcon & PCON_DGFX_BIOS_SUPPORTS_VRSR; + else + return false; +} + +/* + * intel_opregion_vram_sr() enable/disable VRAM Self Refresh. + * @i915: pointer to i915 device. + * @enable: Argument to enable/disable VRSR. + */ +void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable) +{ + struct intel_opregion *opregion = &i915->opregion; + + if (drm_WARN(&i915->drm, !opregion->asle, "ASLE MAILBOX3 is not available\n")) + return; + + if (enable) + opregion->asle->vrsr |= ASLE_VRSR_ENABLE; + else + opregion->asle->vrsr &= ~ASLE_VRSR_ENABLE; +} diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h b/drivers/gpu/drm/i915/display/intel_opregion.h index 82cc0ba34af7..0aa1c4a8a482 100644 --- a/drivers/gpu/drm/i915/display/intel_opregion.h +++ b/drivers/gpu/drm/i915/display/intel_opregion.h @@ -75,6 +75,8 @@ int intel_opregion_notify_adapter(struct drm_i915_private *dev_priv, pci_power_t state); int intel_opregion_get_panel_type(struct drm_i915_private *dev_priv); struct edid *intel_opregion_get_edid(struct intel_connector *connector); +bool intel_opregion_bios_supports_vram_sr(struct drm_i915_private *i915); +void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable); #else /* CONFIG_ACPI*/ @@ -127,6 +129,15 @@ intel_opregion_get_edid(struct intel_connector *connector) return NULL; } +static bool intel_opregion_bios_supports_vram_sr(struct drm_i915_private *i915) +{ + return false; +} + +static void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable) +{ +} + #endif /* CONFIG_ACPI */ #endif -- 2.26.2
[Intel-gfx] [PATCH 0/7] DG2 VRAM_SR Support
This series add DG2 D3Cold VRAM_SR support. TODO: GuC Interface state save/restore on VRAM_SR entry/exit. Anshuman Gupta (7): drm/i915/dgfx: OpRegion VRAM Self Refresh Support drm/i915/dg1: OpRegion PCON DG1 MBD config support drm/i915/dg2: DG2 MBD config drm/i915/dgfx: Add has_lmem_sr drm/i915/pcode: DGFX PCODE MBOX headers drm/i915/dgfx: Setup VRAM SR with D3COLD drm/i915/rpm: Enable D3Cold VRAM SR Support drivers/gpu/drm/i915/display/intel_opregion.c | 85 ++- drivers/gpu/drm/i915/display/intel_opregion.h | 17 drivers/gpu/drm/i915/i915_driver.c| 28 ++ drivers/gpu/drm/i915/i915_drv.h | 17 drivers/gpu/drm/i915/i915_pci.c | 2 + drivers/gpu/drm/i915/i915_reg.h | 7 ++ drivers/gpu/drm/i915/intel_device_info.h | 1 + drivers/gpu/drm/i915/intel_pcode.c| 25 ++ drivers/gpu/drm/i915/intel_pcode.h| 1 + drivers/gpu/drm/i915/intel_pm.c | 43 ++ drivers/gpu/drm/i915/intel_pm.h | 2 + drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +- 12 files changed, 228 insertions(+), 3 deletions(-) -- 2.26.2
[Intel-gfx] [PATCH 0/7] DG2 VRAM_SR Support
This series add DG2 D3Cold VRAM_SR support. TODO: GuC Interface state save/restore on VRAM_SR entry/exit. Anshuman Gupta (7): drm/i915/dgfx: OpRegion VRAM Self Refresh Support drm/i915/dg1: OpRegion PCON DG1 MBD config support drm/i915/dg2: DG2 MBD config drm/i915/dgfx: Add has_lmem_sr drm/i915/pcode: DGFX PCODE MBOX headers drm/i915/dgfx: Setup VRAM SR with D3COLD drm/i915/rpm: Enable D3Cold VRAM SR Support drivers/gpu/drm/i915/display/intel_opregion.c | 85 ++- drivers/gpu/drm/i915/display/intel_opregion.h | 17 drivers/gpu/drm/i915/i915_driver.c| 28 ++ drivers/gpu/drm/i915/i915_drv.h | 17 drivers/gpu/drm/i915/i915_pci.c | 2 + drivers/gpu/drm/i915/i915_reg.h | 7 ++ drivers/gpu/drm/i915/intel_device_info.h | 1 + drivers/gpu/drm/i915/intel_pcode.c| 25 ++ drivers/gpu/drm/i915/intel_pcode.h| 1 + drivers/gpu/drm/i915/intel_pm.c | 43 ++ drivers/gpu/drm/i915/intel_pm.h | 2 + drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +- 12 files changed, 228 insertions(+), 3 deletions(-) -- 2.26.2
Re: [Intel-gfx] [PATCH] drm/i915: Write zero wms if we disable planes for icl+
On Wed, May 18, 2022 at 03:05:44PM +0300, Lisovskiy, Stanislav wrote: > On Wed, May 18, 2022 at 02:44:30PM +0300, Jani Nikula wrote: > > On Wed, 18 May 2022, Stanislav Lisovskiy > > wrote: > > > Otherwise we seem to get FIFO underruns. It is being disabled > > > anyway, so kind of logical to write those as zeroes, even if > > > disabling is temporary. > > > > > > Signed-off-by: Stanislav Lisovskiy > > > --- > > > .../drm/i915/display/skl_universal_plane.c| 2 +- > > > drivers/gpu/drm/i915/intel_pm.c | 46 +++ > > > drivers/gpu/drm/i915/intel_pm.h | 2 + > > > 3 files changed, 49 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c > > > b/drivers/gpu/drm/i915/display/skl_universal_plane.c > > > index caa03324a733..c0251189c042 100644 > > > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > > > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > > > @@ -633,7 +633,7 @@ icl_plane_disable_arm(struct intel_plane *plane, > > > if (icl_is_hdr_plane(dev_priv, plane_id)) > > > intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), 0); > > > > > > - skl_write_plane_wm(plane, crtc_state); > > > + skl_write_zero_plane_wm(plane, crtc_state); > > > > > > intel_psr2_disable_plane_sel_fetch(plane, crtc_state); > > > intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0); > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > b/drivers/gpu/drm/i915/intel_pm.c > > > index ee0047fdc95d..2470c037bfae 100644 > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > @@ -5885,6 +5885,52 @@ void skl_write_plane_wm(struct intel_plane *plane, > > > PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y); > > > } > > > > > > +void skl_write_zero_plane_wm(struct intel_plane *plane, > > > + const struct intel_crtc_state *crtc_state) > > > +{ > > > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > > > + int level, max_level = ilk_wm_max_level(dev_priv); > > > + enum plane_id plane_id = plane->id; > > > + enum pipe pipe = plane->pipe; > > > + struct skl_pipe_wm pipe_wm; > > > + const struct skl_ddb_entry *ddb = > > > + &crtc_state->wm.skl.plane_ddb[plane_id]; > > > + const struct skl_ddb_entry *ddb_y = > > > + &crtc_state->wm.skl.plane_ddb_y[plane_id]; > > > + > > > + for (level = 0; level <= max_level; level++) { > > > > Not your doing here, but why have we adopted this error prone inclusive > > max that always makes you take a double look in the for loops?! > > > > BR, > > Jani. > > No clue, really. It seems to be used same way all over the place in > intel_pm.c. I was suggesting there is some smart reasoning behind this, > so simply follow the common approach. > > For me it is more confusing that apparently it is called "ilk_*" which > is IronLake probably, however we use it everywhere. > I would call it simple i915_wm_max_level and make it determine itself based on > dev_priv which platform it is.. There are loads of (old by now) patches on the list for cleaning up all kinds of stuff in the wm code: https://patchwork.freedesktop.org/series/50802/ https://patchwork.freedesktop.org/series/83289/ https://patchwork.freedesktop.org/series/90164/ -- Ville Syrjälä Intel
Re: [Intel-gfx] [PATCH 1/2] drm/i915/reg: fix undefined behavior due to shift overflowing the constant
On Wed, May 18, 2022 at 02:33:14PM +0300, Jani Nikula wrote: > Use REG_GENMASK() and REG_FIELD_PREP() to avoid errors due to > -fsanitize=shift. I presume it's just unhappy about shifting into the sign bit? Changes look correct: Reviewed-by: Ville Syrjälä > > References: https://lore.kernel.org/r/20220405151517.29753-12...@alien8.de > Reported-by: Borislav Petkov > Reported-by: Ruiqi GONG > Cc: Randy Dunlap > Signed-off-by: Jani Nikula > --- > drivers/gpu/drm/i915/i915_reg.h | 32 > 1 file changed, 16 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 321a08281a3f..dff3f88d8090 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7607,25 +7607,25 @@ enum skl_power_gate { > #define _PORT_CLK_SEL_A 0x46100 > #define _PORT_CLK_SEL_B 0x46104 > #define PORT_CLK_SEL(port) _MMIO_PORT(port, _PORT_CLK_SEL_A, _PORT_CLK_SEL_B) > -#define PORT_CLK_SEL_LCPLL_2700 (0 << 29) > -#define PORT_CLK_SEL_LCPLL_1350 (1 << 29) > -#define PORT_CLK_SEL_LCPLL_810 (2 << 29) > -#define PORT_CLK_SEL_SPLL (3 << 29) > -#define PORT_CLK_SEL_WRPLL(pll) (((pll) + 4) << 29) > -#define PORT_CLK_SEL_WRPLL1 (4 << 29) > -#define PORT_CLK_SEL_WRPLL2 (5 << 29) > -#define PORT_CLK_SEL_NONE (7 << 29) > -#define PORT_CLK_SEL_MASK (7 << 29) > +#define PORT_CLK_SEL_MASK REG_GENMASK(31, 29) > +#define PORT_CLK_SEL_LCPLL_2700 REG_FIELD_PREP(PORT_CLK_SEL_MASK, 0) > +#define PORT_CLK_SEL_LCPLL_1350 REG_FIELD_PREP(PORT_CLK_SEL_MASK, 1) > +#define PORT_CLK_SEL_LCPLL_810 > REG_FIELD_PREP(PORT_CLK_SEL_MASK, 2) > +#define PORT_CLK_SEL_SPLL REG_FIELD_PREP(PORT_CLK_SEL_MASK, 3) > +#define PORT_CLK_SEL_WRPLL(pll) REG_FIELD_PREP(PORT_CLK_SEL_MASK, 4 + > (pll)) > +#define PORT_CLK_SEL_WRPLL1 REG_FIELD_PREP(PORT_CLK_SEL_MASK, 4) > +#define PORT_CLK_SEL_WRPLL2 REG_FIELD_PREP(PORT_CLK_SEL_MASK, 5) > +#define PORT_CLK_SEL_NONE REG_FIELD_PREP(PORT_CLK_SEL_MASK, 7) > > /* On ICL+ this is the same as PORT_CLK_SEL, but all bits change. */ > #define DDI_CLK_SEL(port)PORT_CLK_SEL(port) > -#define DDI_CLK_SEL_NONE(0x0 << 28) > -#define DDI_CLK_SEL_MG (0x8 << 28) > -#define DDI_CLK_SEL_TBT_162 (0xC << 28) > -#define DDI_CLK_SEL_TBT_270 (0xD << 28) > -#define DDI_CLK_SEL_TBT_540 (0xE << 28) > -#define DDI_CLK_SEL_TBT_810 (0xF << 28) > -#define DDI_CLK_SEL_MASK(0xF << 28) > +#define DDI_CLK_SEL_MASKREG_GENMASK(31, 28) > +#define DDI_CLK_SEL_NONEREG_FIELD_PREP(DDI_CLK_SEL_MASK, 0x0) > +#define DDI_CLK_SEL_MG > REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0x8) > +#define DDI_CLK_SEL_TBT_162 REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xC) > +#define DDI_CLK_SEL_TBT_270 REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xD) > +#define DDI_CLK_SEL_TBT_540 REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xE) > +#define DDI_CLK_SEL_TBT_810 REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xF) > > /* Transcoder clock selection */ > #define _TRANS_CLK_SEL_A 0x46140 > -- > 2.30.2 -- Ville Syrjälä Intel
[Intel-gfx] [PATCH] drm/i915: Individualize fences before adding to dma_resv obj
_i915_vma_move_to_active() can receive > 1 fences for multiple batch buffers submission. Because dma_resv_add_fence() can only accept one fence at a time, change _i915_vma_move_to_active() to be aware of multiple fences so that it can add individual fences to the dma resv object. v3: dma_resv_reserve_fences is not cumulative so pass num_fences. v2: make sure to reserve enough fence slots before adding. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5614 Signed-off-by: Nirmoy Das --- drivers/gpu/drm/i915/i915_vma.c | 47 +++-- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index 4f6db539571a..4a5222fc3a4a 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -23,6 +23,7 @@ */ #include +#include #include #include "display/intel_frontbuffer.h" @@ -1823,6 +1824,20 @@ int _i915_vma_move_to_active(struct i915_vma *vma, if (unlikely(err)) return err; + /* Reserve fences slot early to prevent an allocation after preparing +* the workload and associating fences with dma_resv. +*/ + if (fence && !(flags & __EXEC_OBJECT_NO_RESERVE)) { + struct dma_fence *curr; + int idx; + + dma_fence_array_for_each(curr, idx, fence) + ; + err = dma_resv_reserve_fences(vma->obj->base.resv, idx); + if (unlikely(err)) + return err; + } + if (flags & EXEC_OBJECT_WRITE) { struct intel_frontbuffer *front; @@ -1832,31 +1847,23 @@ int _i915_vma_move_to_active(struct i915_vma *vma, i915_active_add_request(&front->write, rq); intel_frontbuffer_put(front); } + } - if (!(flags & __EXEC_OBJECT_NO_RESERVE)) { - err = dma_resv_reserve_fences(vma->obj->base.resv, 1); - if (unlikely(err)) - return err; - } + if (fence) { + struct dma_fence *curr; + enum dma_resv_usage usage; + int idx; - if (fence) { - dma_resv_add_fence(vma->obj->base.resv, fence, - DMA_RESV_USAGE_WRITE); + obj->read_domains = 0; + if (flags & EXEC_OBJECT_WRITE) { + usage = DMA_RESV_USAGE_WRITE; obj->write_domain = I915_GEM_DOMAIN_RENDER; - obj->read_domains = 0; - } - } else { - if (!(flags & __EXEC_OBJECT_NO_RESERVE)) { - err = dma_resv_reserve_fences(vma->obj->base.resv, 1); - if (unlikely(err)) - return err; + } else { + usage = DMA_RESV_USAGE_READ; } - if (fence) { - dma_resv_add_fence(vma->obj->base.resv, fence, - DMA_RESV_USAGE_READ); - obj->write_domain = 0; - } + dma_fence_array_for_each(curr, idx, fence) + dma_resv_add_fence(vma->obj->base.resv, curr, usage); } if (flags & EXEC_OBJECT_NEEDS_FENCE && vma->fence) -- 2.35.1
Re: [Intel-gfx] [RFC PATCH v3 02/19] KVM: x86: inhibit APICv/AVIC when the guest and/or host changes apic id/base from the defaults.
On Wed, 2022-05-18 at 19:51 +0800, Chao Gao wrote: > On Wed, May 18, 2022 at 12:50:27PM +0300, Maxim Levitsky wrote: > > > > struct kvm_arch { > > > > @@ -1258,6 +1260,7 @@ struct kvm_arch { > > > > hpa_t hv_root_tdp; > > > > spinlock_t hv_root_tdp_lock; > > > > #endif > > > > + bool apic_id_changed; > > > > > > What's the value of this boolean? No one reads it. > > > > I use it in later patches to kill the guest during nested VM entry > > if it attempts to use nested AVIC after any vCPU changed APIC ID. > > > > I mentioned this boolean in the commit description. > > > > This boolean avoids the need to go over all vCPUs and checking > > if they still have the initial apic id. > > Do you want to kill the guest if APIC base got changed? If yes, > you can check if APICV_INHIBIT_REASON_RO_SETTINGS is set and save > the boolean. Yep, I thrown in the apic base just because I can. It doesn't matter to my nested AVIC logic at all, but since it is also something that guests don't change, I also don't care if this will lead to inhibit and killing the guest if it attempts to use nested AVIC. That boolean should have the same value as the APICV_INHIBIT_REASON_RO_SETTINGS inhibit, so yes I can instead check if the inhibit is active. I don't know if that is cleaner that this boolean though, individual inhibit value is currently not something that anybody uses in logic. Best regards, Maxim Levitsky > > > In the future maybe we can introduce a more generic 'taint' > > bitmap with various flags like that, indicating that the guest > > did something unexpected. > > > > BTW, the other option in regard to the nested AVIC is just to ignore this > > issue completely. > > The code itself always uses vcpu_id's, thus regardless of when/how often > > the guest changes > > its apic ids, my code would just use the initial APIC ID values > > consistently. > > > > In this case I won't need this boolean. > > > > > > }; > > > > > > > > struct kvm_vm_stat { > > > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > > > > index 66b0eb0bda94e..8996675b3ef4c 100644 > > > > --- a/arch/x86/kvm/lapic.c > > > > +++ b/arch/x86/kvm/lapic.c > > > > @@ -2038,6 +2038,19 @@ static void apic_manage_nmi_watchdog(struct > > > > kvm_lapic *apic, u32 lvt0_val) > > > > } > > > > } > > > > > > > > +static void kvm_lapic_check_initial_apic_id(struct kvm_lapic *apic) > > > > +{ > > > > + if (kvm_apic_has_initial_apic_id(apic)) > > > > + return; > > > > + > > > > + pr_warn_once("APIC ID change is unsupported by KVM"); > > > > > > It is misleading because changing xAPIC ID is supported by KVM; it just > > > isn't compatible with APICv. Probably this pr_warn_once() should be > > > removed. > > > > Honestly since nobody uses this feature, I am not sure if to call this > > supported, > > I am sure that KVM has more bugs in regard of using non standard APIC ID. > > This warning might hopefuly make someone complain about it if this > > feature is actually used somewhere. > > Now I got you. It is fine to me. >
Re: [Intel-gfx] [PATCH] drm/i915: Write zero wms if we disable planes for icl+
On Wed, May 18, 2022 at 02:44:30PM +0300, Jani Nikula wrote: > On Wed, 18 May 2022, Stanislav Lisovskiy > wrote: > > Otherwise we seem to get FIFO underruns. It is being disabled > > anyway, so kind of logical to write those as zeroes, even if > > disabling is temporary. > > > > Signed-off-by: Stanislav Lisovskiy > > --- > > .../drm/i915/display/skl_universal_plane.c| 2 +- > > drivers/gpu/drm/i915/intel_pm.c | 46 +++ > > drivers/gpu/drm/i915/intel_pm.h | 2 + > > 3 files changed, 49 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c > > b/drivers/gpu/drm/i915/display/skl_universal_plane.c > > index caa03324a733..c0251189c042 100644 > > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > > @@ -633,7 +633,7 @@ icl_plane_disable_arm(struct intel_plane *plane, > > if (icl_is_hdr_plane(dev_priv, plane_id)) > > intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), 0); > > > > - skl_write_plane_wm(plane, crtc_state); > > + skl_write_zero_plane_wm(plane, crtc_state); > > > > intel_psr2_disable_plane_sel_fetch(plane, crtc_state); > > intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0); > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > b/drivers/gpu/drm/i915/intel_pm.c > > index ee0047fdc95d..2470c037bfae 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -5885,6 +5885,52 @@ void skl_write_plane_wm(struct intel_plane *plane, > > PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y); > > } > > > > +void skl_write_zero_plane_wm(struct intel_plane *plane, > > +const struct intel_crtc_state *crtc_state) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > > + int level, max_level = ilk_wm_max_level(dev_priv); > > + enum plane_id plane_id = plane->id; > > + enum pipe pipe = plane->pipe; > > + struct skl_pipe_wm pipe_wm; > > + const struct skl_ddb_entry *ddb = > > + &crtc_state->wm.skl.plane_ddb[plane_id]; > > + const struct skl_ddb_entry *ddb_y = > > + &crtc_state->wm.skl.plane_ddb_y[plane_id]; > > + > > + for (level = 0; level <= max_level; level++) { > > Not your doing here, but why have we adopted this error prone inclusive > max that always makes you take a double look in the for loops?! > > BR, > Jani. No clue, really. It seems to be used same way all over the place in intel_pm.c. I was suggesting there is some smart reasoning behind this, so simply follow the common approach. For me it is more confusing that apparently it is called "ilk_*" which is IronLake probably, however we use it everywhere. I would call it simple i915_wm_max_level and make it determine itself based on dev_priv which platform it is.. Stan > > > + pipe_wm.planes[plane_id].wm[level].blocks = 0; > > + pipe_wm.planes[plane_id].wm[level].lines = 0; > > + } > > + > > + pipe_wm.planes[plane_id].trans_wm.blocks = 0; > > + pipe_wm.planes[plane_id].trans_wm.lines = 0; > > + > > + for (level = 0; level <= max_level; level++) > > + skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane_id, level), > > + skl_plane_wm_level(&pipe_wm, plane_id, > > level)); > > + > > + skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id), > > + skl_plane_trans_wm(&pipe_wm, plane_id)); > > + > > + if (HAS_HW_SAGV_WM(dev_priv)) { > > + const struct skl_plane_wm *wm = &pipe_wm.planes[plane_id]; > > + > > + skl_write_wm_level(dev_priv, PLANE_WM_SAGV(pipe, plane_id), > > + &wm->sagv.wm0); > > + skl_write_wm_level(dev_priv, PLANE_WM_SAGV_TRANS(pipe, > > plane_id), > > + &wm->sagv.trans_wm); > > + } > > + > > + skl_ddb_entry_write(dev_priv, > > + PLANE_BUF_CFG(pipe, plane_id), ddb); > > + > > + if (DISPLAY_VER(dev_priv) < 11) > > + skl_ddb_entry_write(dev_priv, > > + PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y); > > +} > > + > > + > > void skl_write_cursor_wm(struct intel_plane *plane, > > const struct intel_crtc_state *crtc_state) > > { > > diff --git a/drivers/gpu/drm/i915/intel_pm.h > > b/drivers/gpu/drm/i915/intel_pm.h > > index 50604cf7398c..fb0ac4f143ab 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.h > > +++ b/drivers/gpu/drm/i915/intel_pm.h > > @@ -61,6 +61,8 @@ bool skl_wm_level_equals(const struct skl_wm_level *l1, > > bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb, > > const struct skl_ddb_entry *entries, > > int num_entries, int ignore_idx); > > +void skl_write_zero_plane_wm(struct intel_plane *plane, > > +const struct inte
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Write zero wms if we disable planes for icl+
== Series Details == Series: drm/i915: Write zero wms if we disable planes for icl+ URL : https://patchwork.freedesktop.org/series/104120/ State : failure == Summary == CI Bug Log - changes from CI_DRM_11672 -> Patchwork_104120v1 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_104120v1 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_104120v1, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/index.html Participating hosts (44 -> 46) -- Additional (4): bat-rpls-1 fi-rkl-11600 fi-hsw-g3258 bat-adlm-1 Missing(2): fi-cml-u2 bat-dg2-9 Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_104120v1: ### IGT changes ### Possible regressions * igt@gem_exec_suspend@basic-s3@smem: - fi-rkl-guc: NOTRUN -> [DMESG-WARN][1] +4 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-guc/igt@gem_exec_suspend@basic...@smem.html * igt@i915_suspend@basic-s2idle-without-i915: - fi-cfl-8109u: [PASS][2] -> [DMESG-WARN][3] [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-cfl-8109u/igt@i915_susp...@basic-s2idle-without-i915.html [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-cfl-8109u/igt@i915_susp...@basic-s2idle-without-i915.html * igt@kms_busy@basic@flip: - fi-adl-ddr5:[PASS][4] -> [DMESG-WARN][5] +34 similar issues [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-adl-ddr5/igt@kms_busy@ba...@flip.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-adl-ddr5/igt@kms_busy@ba...@flip.html - bat-dg1-6: [PASS][6] -> [DMESG-WARN][7] +29 similar issues [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/bat-dg1-6/igt@kms_busy@ba...@flip.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-dg1-6/igt@kms_busy@ba...@flip.html * igt@kms_flip@basic-flip-vs-dpms@b-dp1: - fi-tgl-1115g4: [PASS][8] -> [DMESG-WARN][9] +30 similar issues [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-tgl-1115g4/igt@kms_flip@basic-flip-vs-d...@b-dp1.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-tgl-1115g4/igt@kms_flip@basic-flip-vs-d...@b-dp1.html * igt@kms_flip@basic-flip-vs-modeset@c-hdmi-a1: - fi-rkl-guc: [PASS][10] -> [DMESG-WARN][11] +24 similar issues [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-rkl-guc/igt@kms_flip@basic-flip-vs-mode...@c-hdmi-a1.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-guc/igt@kms_flip@basic-flip-vs-mode...@c-hdmi-a1.html * igt@kms_flip@basic-plain-flip@a-hdmi-a1: - fi-rkl-11600: NOTRUN -> [DMESG-WARN][12] +25 similar issues [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-rkl-11600/igt@kms_flip@basic-plain-f...@a-hdmi-a1.html * igt@kms_force_connector_basic@force-connector-state: - bat-dg1-5: [PASS][13] -> [DMESG-WARN][14] [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/bat-dg1-5/igt@kms_force_connector_ba...@force-connector-state.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/bat-dg1-5/igt@kms_force_connector_ba...@force-connector-state.html * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a: - fi-tgl-u2: [PASS][15] -> [DMESG-WARN][16] +145 similar issues [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-tgl-u2/igt@kms_pipe_crc_ba...@nonblocking-crc-pipe-a.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-tgl-u2/igt@kms_pipe_crc_ba...@nonblocking-crc-pipe-a.html * igt@vgem_basic@dmabuf-fence: - fi-tgl-u2: NOTRUN -> [DMESG-WARN][17] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-tgl-u2/igt@vgem_ba...@dmabuf-fence.html Warnings * igt@kms_flip@basic-flip-vs-modeset@a-edp1: - fi-tgl-u2: [DMESG-WARN][18] ([i915#402]) -> [DMESG-WARN][19] +2 similar issues [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/fi-tgl-u2/igt@kms_flip@basic-flip-vs-mode...@a-edp1.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104120v1/fi-tgl-u2/igt@kms_flip@basic-flip-vs-mode...@a-edp1.html Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@core_hotunplug@unbind-rebind: - {bat-jsl-2}:[PASS][20] -> [DMESG-WARN][21] +39 similar issues [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11672/bat-jsl-2/igt@core_hot
Re: [Intel-gfx] [RFC PATCH v3 02/19] KVM: x86: inhibit APICv/AVIC when the guest and/or host changes apic id/base from the defaults.
On Wed, May 18, 2022 at 12:50:27PM +0300, Maxim Levitsky wrote: >> > struct kvm_arch { >> > @@ -1258,6 +1260,7 @@ struct kvm_arch { >> >hpa_t hv_root_tdp; >> >spinlock_t hv_root_tdp_lock; >> > #endif >> > + bool apic_id_changed; >> >> What's the value of this boolean? No one reads it. > >I use it in later patches to kill the guest during nested VM entry >if it attempts to use nested AVIC after any vCPU changed APIC ID. > >I mentioned this boolean in the commit description. > >This boolean avoids the need to go over all vCPUs and checking >if they still have the initial apic id. Do you want to kill the guest if APIC base got changed? If yes, you can check if APICV_INHIBIT_REASON_RO_SETTINGS is set and save the boolean. > >In the future maybe we can introduce a more generic 'taint' >bitmap with various flags like that, indicating that the guest >did something unexpected. > >BTW, the other option in regard to the nested AVIC is just to ignore this >issue completely. >The code itself always uses vcpu_id's, thus regardless of when/how often the >guest changes >its apic ids, my code would just use the initial APIC ID values consistently. > >In this case I won't need this boolean. > >> >> > }; >> > >> > struct kvm_vm_stat { >> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c >> > index 66b0eb0bda94e..8996675b3ef4c 100644 >> > --- a/arch/x86/kvm/lapic.c >> > +++ b/arch/x86/kvm/lapic.c >> > @@ -2038,6 +2038,19 @@ static void apic_manage_nmi_watchdog(struct >> > kvm_lapic *apic, u32 lvt0_val) >> >} >> > } >> > >> > +static void kvm_lapic_check_initial_apic_id(struct kvm_lapic *apic) >> > +{ >> > + if (kvm_apic_has_initial_apic_id(apic)) >> > + return; >> > + >> > + pr_warn_once("APIC ID change is unsupported by KVM"); >> >> It is misleading because changing xAPIC ID is supported by KVM; it just >> isn't compatible with APICv. Probably this pr_warn_once() should be >> removed. > >Honestly since nobody uses this feature, I am not sure if to call this >supported, >I am sure that KVM has more bugs in regard of using non standard APIC ID. >This warning might hopefuly make someone complain about it if this >feature is actually used somewhere. Now I got you. It is fine to me.
Re: [Intel-gfx] [RFC PATCH] drm/i915: Debugfs statistics interface for psr
On Wed, 18 May 2022, Jouni Högander wrote: > Currently there is no mean to get understanding how psr is utilized. > E.g. How much psr is actually enabled or how often driver falls back > to full update. > > This patch addresses this by adding new debugfs interface > i915_edp_psr_stats. Statistics gathering is started by writing 1/y/Y and > stopped by writing 0/n/N into this new interface. IMO time to move all PSR debugfs handling to intel_psr.c before this. See e.g. commit d2de8ccfb299 ("drm/i915/fbc: Move FBC debugfs stuff into intel_fbc.c") for FBC. With that, I think you could also split out intel_psr_regs.h and encapsulate psr register access to intel_psr.c. BR, Jani. > > Following fields are provided for reading by this new interface: > > "PSR disabled vblank count" > > Over how many vblank periods PSR was disabled after statistics > gathering got started. I.e. How many normal updates were sent to panel. > > "Total vblank count" > > Total vblank count after statistics gathering got started. > > "Selective update count" > > How many selective updates (PSR2) were done after statistics gathering > got started. > > "Full update count" > > How many times driver decided to fall back to full update when trying to > perform selective update. > > Cc: José Roberto de Souza > Cc: Mika Kahola > Cc: Uma Shankar > Cc: Nischal Varide > Signed-off-by: Jouni Högander > --- > .../drm/i915/display/intel_display_debugfs.c | 100 > .../drm/i915/display/intel_display_types.h| 16 ++ > drivers/gpu/drm/i915/display/intel_psr.c | 144 ++ > drivers/gpu/drm/i915/display/intel_psr.h | 2 + > 4 files changed, 236 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > index 452d773fd4e3..c29f151062e4 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > @@ -9,6 +9,7 @@ > #include > > #include "i915_debugfs.h" > +#include "intel_crtc.h" > #include "intel_de.h" > #include "intel_display_debugfs.h" > #include "intel_display_power.h" > @@ -1868,6 +1869,95 @@ i915_fifo_underrun_reset_write(struct file *filp, > return cnt; > } > > +static int intel_psr_stats(struct seq_file *m, struct intel_dp *intel_dp) > +{ > + struct drm_i915_private *dev_priv = (m->private); > + struct intel_psr *psr = &intel_dp->psr; > + struct drm_crtc *crtc = &intel_crtc_for_pipe(dev_priv, psr->pipe)->base; > + u64 total_vblank_count = psr->stats.total_vblank_count, > + non_psr_vblank_count = psr->stats.non_psr_vblank_count; > + ktime_t vblanktime; > + > + if (!psr->active) > + non_psr_vblank_count += drm_crtc_vblank_count_and_time(crtc, > &vblanktime) - > + psr->stats.psr_disable_vblank; > + > + seq_printf(m, "PSR disabled vblank count : %llu\n", > non_psr_vblank_count); > + > + if (psr->stats.enable) > + total_vblank_count += drm_crtc_vblank_count_and_time(crtc, > &vblanktime) - > + psr->stats.start_vblank; > + > + seq_printf(m, "Total vblank count : %llu\n", total_vblank_count); > + seq_printf(m, "Selective update count : %llu\n", > psr->stats.selective_update_count); > + seq_printf(m, "Full update count : %llu\n", > psr->stats.full_update_count); > + > + return 0; > +} > + > +static int i915_edp_psr_stats_show(struct seq_file *m, void *data) > +{ > + struct drm_i915_private *dev_priv = (m->private); > + struct intel_dp *intel_dp = NULL; > + struct intel_encoder *encoder; > + > + if (!HAS_PSR(dev_priv)) > + return -ENODEV; > + > + /* Find the first EDP which supports PSR */ > + for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) { > + intel_dp = enc_to_intel_dp(encoder); > + break; > + } > + > + if (!intel_dp) > + return -ENODEV; > + > + return intel_psr_stats(m, intel_dp); > +} > + > +static ssize_t > +i915_edp_psr_stats_write(struct file *filp, const char __user *ubuf, > + size_t cnt, loff_t *ppos) > +{ > + struct seq_file *m = filp->private_data; > + struct drm_i915_private *dev_priv = m->private; > + struct intel_dp *intel_dp = NULL; > + struct intel_encoder *encoder; > + int ret; > + bool enable_stats; > + > + ret = kstrtobool_from_user(ubuf, cnt, &enable_stats); > + if (ret) > + return ret; > + > + if (!HAS_PSR(dev_priv)) > + return -ENODEV; > + > + /* Find the first EDP which supports PSR */ > + for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) { > + intel_dp = enc_to_intel_dp(encoder); > + break; > + } > + > + if (!intel_dp) > + return -ENODEV; > + > + if (enable_stats) > + intel_psr_stats_enable_stats(intel_dp); > + else > +
Re: [Intel-gfx] [PATCH] drm/i915: Write zero wms if we disable planes for icl+
On Wed, 18 May 2022, Stanislav Lisovskiy wrote: > Otherwise we seem to get FIFO underruns. It is being disabled > anyway, so kind of logical to write those as zeroes, even if > disabling is temporary. > > Signed-off-by: Stanislav Lisovskiy > --- > .../drm/i915/display/skl_universal_plane.c| 2 +- > drivers/gpu/drm/i915/intel_pm.c | 46 +++ > drivers/gpu/drm/i915/intel_pm.h | 2 + > 3 files changed, 49 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c > b/drivers/gpu/drm/i915/display/skl_universal_plane.c > index caa03324a733..c0251189c042 100644 > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > @@ -633,7 +633,7 @@ icl_plane_disable_arm(struct intel_plane *plane, > if (icl_is_hdr_plane(dev_priv, plane_id)) > intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), 0); > > - skl_write_plane_wm(plane, crtc_state); > + skl_write_zero_plane_wm(plane, crtc_state); > > intel_psr2_disable_plane_sel_fetch(plane, crtc_state); > intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0); > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index ee0047fdc95d..2470c037bfae 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -5885,6 +5885,52 @@ void skl_write_plane_wm(struct intel_plane *plane, > PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y); > } > > +void skl_write_zero_plane_wm(struct intel_plane *plane, > + const struct intel_crtc_state *crtc_state) > +{ > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > + int level, max_level = ilk_wm_max_level(dev_priv); > + enum plane_id plane_id = plane->id; > + enum pipe pipe = plane->pipe; > + struct skl_pipe_wm pipe_wm; > + const struct skl_ddb_entry *ddb = > + &crtc_state->wm.skl.plane_ddb[plane_id]; > + const struct skl_ddb_entry *ddb_y = > + &crtc_state->wm.skl.plane_ddb_y[plane_id]; > + > + for (level = 0; level <= max_level; level++) { Not your doing here, but why have we adopted this error prone inclusive max that always makes you take a double look in the for loops?! BR, Jani. > + pipe_wm.planes[plane_id].wm[level].blocks = 0; > + pipe_wm.planes[plane_id].wm[level].lines = 0; > + } > + > + pipe_wm.planes[plane_id].trans_wm.blocks = 0; > + pipe_wm.planes[plane_id].trans_wm.lines = 0; > + > + for (level = 0; level <= max_level; level++) > + skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane_id, level), > +skl_plane_wm_level(&pipe_wm, plane_id, > level)); > + > + skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id), > +skl_plane_trans_wm(&pipe_wm, plane_id)); > + > + if (HAS_HW_SAGV_WM(dev_priv)) { > + const struct skl_plane_wm *wm = &pipe_wm.planes[plane_id]; > + > + skl_write_wm_level(dev_priv, PLANE_WM_SAGV(pipe, plane_id), > +&wm->sagv.wm0); > + skl_write_wm_level(dev_priv, PLANE_WM_SAGV_TRANS(pipe, > plane_id), > +&wm->sagv.trans_wm); > + } > + > + skl_ddb_entry_write(dev_priv, > + PLANE_BUF_CFG(pipe, plane_id), ddb); > + > + if (DISPLAY_VER(dev_priv) < 11) > + skl_ddb_entry_write(dev_priv, > + PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y); > +} > + > + > void skl_write_cursor_wm(struct intel_plane *plane, >const struct intel_crtc_state *crtc_state) > { > diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h > index 50604cf7398c..fb0ac4f143ab 100644 > --- a/drivers/gpu/drm/i915/intel_pm.h > +++ b/drivers/gpu/drm/i915/intel_pm.h > @@ -61,6 +61,8 @@ bool skl_wm_level_equals(const struct skl_wm_level *l1, > bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb, >const struct skl_ddb_entry *entries, >int num_entries, int ignore_idx); > +void skl_write_zero_plane_wm(struct intel_plane *plane, > + const struct intel_crtc_state *crtc_state); > void skl_write_plane_wm(struct intel_plane *plane, > const struct intel_crtc_state *crtc_state); > void skl_write_cursor_wm(struct intel_plane *plane, -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH -next] drm/i915: fix compilation errors caused by `-fsanitize=shift`
On Tue, 17 May 2022, "GONG, Ruiqi" wrote: > Fix the compilation errors produced by building recent mainline on x86 > with allmodconfig: > > (1st type of errors) > drivers/gpu/drm/i915/display/intel_ddi.c:1916:2: error: case label does not > reduce to an integer constant > case PORT_CLK_SEL_WRPLL1: > ^~~~ > > (2nd type of errors) > ././include/linux/compiler_types.h:352:38: error: call to > ‘__compiletime_assert_1360’ declared with attribute error: FIELD_PREP: mask > is not constant > _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) > ^ > ... > drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:2316:3: note: in > expansion of macro ‘FIELD_PREP’ > FIELD_PREP(GUC_KLV_0_KEY, GUC_CONTEXT_POLICIES_KLV_ID_##id) | \ > ^~ > drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:2323:1: note: in > expansion of macro ‘MAKE_CONTEXT_POLICY_ADD’ >MAKE_CONTEXT_POLICY_ADD(preemption_timeout, PREEMPTION_TIMEOUT) >^~~ > > which are all induced by `-fsanitize=shift`. > > Signed-off-by: GONG, Ruiqi Please see [1] and [2]. BR, Jani. [1] https://lore.kernel.org/r/20220405151517.29753-12...@alien8.de [2] https://patchwork.freedesktop.org/series/104122/ > --- > drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 2 +- > .../i915/gt/uc/abi/guc_communication_ctb_abi.h | 2 +- > drivers/gpu/drm/i915/gt/uc/abi/guc_klvs_abi.h| 4 ++-- > .../gpu/drm/i915/gt/uc/abi/guc_messages_abi.h| 2 +- > drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 2 +- > drivers/gpu/drm/i915/i915_reg.h | 16 > 6 files changed, 14 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h > b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h > index be9ac47fa9d0..3ada7358a698 100644 > --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h > +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h > @@ -50,7 +50,7 @@ > > #define HOST2GUC_SELF_CFG_REQUEST_MSG_LEN > (GUC_HXG_REQUEST_MSG_MIN_LEN + 3u) > #define HOST2GUC_SELF_CFG_REQUEST_MSG_0_MBZ > GUC_HXG_REQUEST_MSG_0_DATA0 > -#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_KEY (0x << 16) > +#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_KEY (0xu << 16) > #define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_LEN (0x << 0) > #define HOST2GUC_SELF_CFG_REQUEST_MSG_2_VALUE32 > GUC_HXG_REQUEST_MSG_n_DATAn > #define HOST2GUC_SELF_CFG_REQUEST_MSG_3_VALUE64 > GUC_HXG_REQUEST_MSG_n_DATAn > diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h > b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h > index c9086a600bce..c97ff7c38576 100644 > --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h > +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h > @@ -82,7 +82,7 @@ static_assert(sizeof(struct guc_ct_buffer_desc) == 64); > #define GUC_CTB_HDR_LEN 1u > #define GUC_CTB_MSG_MIN_LEN GUC_CTB_HDR_LEN > #define GUC_CTB_MSG_MAX_LEN 256u > -#define GUC_CTB_MSG_0_FENCE (0x << 16) > +#define GUC_CTB_MSG_0_FENCE (0xu << 16) > #define GUC_CTB_MSG_0_FORMAT (0xf << 12) > #define GUC_CTB_FORMAT_HXG 0u > #define GUC_CTB_MSG_0_RESERVED (0xf << 8) > diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_klvs_abi.h > b/drivers/gpu/drm/i915/gt/uc/abi/guc_klvs_abi.h > index 4a59478c3b5c..e811896a80a0 100644 > --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_klvs_abi.h > +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_klvs_abi.h > @@ -29,8 +29,8 @@ > */ > > #define GUC_KLV_LEN_MIN 1u > -#define GUC_KLV_0_KEY(0x << 16) > -#define GUC_KLV_0_LEN(0x << 0) > +#define GUC_KLV_0_KEY(0xu << 16) > +#define GUC_KLV_0_LEN(0xu << 0) > #define GUC_KLV_n_VALUE (0x << 0) > > /** > diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h > b/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h > index 29ac823acd4c..901595300f82 100644 > --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h > +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h > @@ -40,7 +40,7 @@ > */ > > #define GUC_HXG_MSG_MIN_LEN 1u > -#define GUC_HXG_MSG_0_ORIGIN (0x1 << 31) > +#define GUC_HXG_MSG_0_ORIGIN (0x1u << 31) > #define GUC_HXG_ORIGIN_HOST0u > #define GUC_HXG_ORIGIN_GUC 1u > #define GUC_HXG_MSG_0_TYPE (0x7 << 28) > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h > b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h > index 66027a42cda9..22
Re: [Intel-gfx] [PATCH 11/11] drm/i915: Fix undefined behavior due to shift overflowing the constant
On Wed, 18 May 2022, Borislav Petkov wrote: > On Tue, May 17, 2022 at 04:05:46PM -0700, Randy Dunlap wrote: >> >> >> On 4/5/22 08:15, Borislav Petkov wrote: >> > From: Borislav Petkov >> > >> > Fix: >> > >> > In file included from :0:0: >> > drivers/gpu/drm/i915/gt/uc/intel_guc.c: In function >> > ‘intel_guc_send_mmio’: >> > ././include/linux/compiler_types.h:352:38: error: call to >> > ‘__compiletime_assert_1047’ \ >> > declared with attribute error: FIELD_PREP: mask is not constant >> > _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) >> > >> > and other build errors due to shift overflowing values. >> > >> > See https://lore.kernel.org/r/ykwq6%2btih8gqp...@zn.tnic for the gory >> > details as to why it triggers with older gccs only. >> > >> >> Acked-by: Randy Dunlap >> Tested-by: Randy Dunlap >> >> Is this merged anywhere? > > It's state is "new" in their patchwork: > > https://patchwork.freedesktop.org/patch/480756/ Basically we run all patches through CI before merging, and because only one patch was sent to intel-gfx, the CI just sat waiting for the rest of the series to arrive... Anyway, didn't really like the changes in i915_reg.h, sent my version of that and the rest separately [1]. > so I guess not yet. > >> It could/should at least be in linux-next so that other people >> don't waste time on it. > > -ETOOMANYPATCHES I guess. :-\ Yeah, sorry about that. BR, Jani. [1] https://patchwork.freedesktop.org/series/104122/ -- Jani Nikula, Intel Open Source Graphics Center
[Intel-gfx] [PATCH 2/2] drm/i915/uc: Fix undefined behavior due to shift overflowing the constant
From: Borislav Petkov Fix: In file included from :0:0: drivers/gpu/drm/i915/gt/uc/intel_guc.c: In function ‘intel_guc_send_mmio’: ././include/linux/compiler_types.h:352:38: error: call to ‘__compiletime_assert_1047’ \ declared with attribute error: FIELD_PREP: mask is not constant _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) and other build errors due to shift overflowing values. See https://lore.kernel.org/r/ykwq6%2btih8gqp...@zn.tnic for the gory details as to why it triggers with older gccs only. v2 by Jani: - Drop the i915_reg.h changes Cc: Joonas Lahtinen Cc: Tvrtko Ursulin Cc: Ruiqi GONG Cc: Randy Dunlap Signed-off-by: Borislav Petkov Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 2 +- drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h | 2 +- drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h | 2 +- drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h index be9ac47fa9d0..4ef9990ed7f8 100644 --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h @@ -50,7 +50,7 @@ #define HOST2GUC_SELF_CFG_REQUEST_MSG_LEN (GUC_HXG_REQUEST_MSG_MIN_LEN + 3u) #define HOST2GUC_SELF_CFG_REQUEST_MSG_0_MBZ GUC_HXG_REQUEST_MSG_0_DATA0 -#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_KEY(0x << 16) +#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_KEY(0xU << 16) #define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_LEN(0x << 0) #define HOST2GUC_SELF_CFG_REQUEST_MSG_2_VALUE32 GUC_HXG_REQUEST_MSG_n_DATAn #define HOST2GUC_SELF_CFG_REQUEST_MSG_3_VALUE64 GUC_HXG_REQUEST_MSG_n_DATAn diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h index c9086a600bce..df83c1cc7c7a 100644 --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h @@ -82,7 +82,7 @@ static_assert(sizeof(struct guc_ct_buffer_desc) == 64); #define GUC_CTB_HDR_LEN1u #define GUC_CTB_MSG_MIN_LENGUC_CTB_HDR_LEN #define GUC_CTB_MSG_MAX_LEN256u -#define GUC_CTB_MSG_0_FENCE(0x << 16) +#define GUC_CTB_MSG_0_FENCE(0xU << 16) #define GUC_CTB_MSG_0_FORMAT (0xf << 12) #define GUC_CTB_FORMAT_HXG 0u #define GUC_CTB_MSG_0_RESERVED (0xf << 8) diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h index 29ac823acd4c..7d5ba4d97d70 100644 --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h @@ -40,7 +40,7 @@ */ #define GUC_HXG_MSG_MIN_LEN1u -#define GUC_HXG_MSG_0_ORIGIN (0x1 << 31) +#define GUC_HXG_MSG_0_ORIGIN (0x1U << 31) #define GUC_HXG_ORIGIN_HOST 0u #define GUC_HXG_ORIGIN_GUC 1u #define GUC_HXG_MSG_0_TYPE (0x7 << 28) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h index 2516705b9f36..8dc063f087eb 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h @@ -28,7 +28,7 @@ #define GS_MIA_HALT_REQUESTED (0x02 << GS_MIA_SHIFT) #define GS_MIA_ISR_ENTRY (0x04 << GS_MIA_SHIFT) #define GS_AUTH_STATUS_SHIFT 30 -#define GS_AUTH_STATUS_MASK(0x03 << GS_AUTH_STATUS_SHIFT) +#define GS_AUTH_STATUS_MASK(0x03U << GS_AUTH_STATUS_SHIFT) #define GS_AUTH_STATUS_BAD (0x01 << GS_AUTH_STATUS_SHIFT) #define GS_AUTH_STATUS_GOOD(0x02 << GS_AUTH_STATUS_SHIFT) -- 2.30.2
[Intel-gfx] [PATCH 1/2] drm/i915/reg: fix undefined behavior due to shift overflowing the constant
Use REG_GENMASK() and REG_FIELD_PREP() to avoid errors due to -fsanitize=shift. References: https://lore.kernel.org/r/20220405151517.29753-12...@alien8.de Reported-by: Borislav Petkov Reported-by: Ruiqi GONG Cc: Randy Dunlap Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_reg.h | 32 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 321a08281a3f..dff3f88d8090 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7607,25 +7607,25 @@ enum skl_power_gate { #define _PORT_CLK_SEL_A0x46100 #define _PORT_CLK_SEL_B0x46104 #define PORT_CLK_SEL(port) _MMIO_PORT(port, _PORT_CLK_SEL_A, _PORT_CLK_SEL_B) -#define PORT_CLK_SEL_LCPLL_2700 (0 << 29) -#define PORT_CLK_SEL_LCPLL_1350 (1 << 29) -#define PORT_CLK_SEL_LCPLL_810(2 << 29) -#define PORT_CLK_SEL_SPLL (3 << 29) -#define PORT_CLK_SEL_WRPLL(pll) (((pll) + 4) << 29) -#define PORT_CLK_SEL_WRPLL1 (4 << 29) -#define PORT_CLK_SEL_WRPLL2 (5 << 29) -#define PORT_CLK_SEL_NONE (7 << 29) -#define PORT_CLK_SEL_MASK (7 << 29) +#define PORT_CLK_SEL_MASK REG_GENMASK(31, 29) +#define PORT_CLK_SEL_LCPLL_2700 REG_FIELD_PREP(PORT_CLK_SEL_MASK, 0) +#define PORT_CLK_SEL_LCPLL_1350 REG_FIELD_PREP(PORT_CLK_SEL_MASK, 1) +#define PORT_CLK_SEL_LCPLL_810 REG_FIELD_PREP(PORT_CLK_SEL_MASK, 2) +#define PORT_CLK_SEL_SPLL REG_FIELD_PREP(PORT_CLK_SEL_MASK, 3) +#define PORT_CLK_SEL_WRPLL(pll) REG_FIELD_PREP(PORT_CLK_SEL_MASK, 4 + (pll)) +#define PORT_CLK_SEL_WRPLL1 REG_FIELD_PREP(PORT_CLK_SEL_MASK, 4) +#define PORT_CLK_SEL_WRPLL2 REG_FIELD_PREP(PORT_CLK_SEL_MASK, 5) +#define PORT_CLK_SEL_NONE REG_FIELD_PREP(PORT_CLK_SEL_MASK, 7) /* On ICL+ this is the same as PORT_CLK_SEL, but all bits change. */ #define DDI_CLK_SEL(port) PORT_CLK_SEL(port) -#define DDI_CLK_SEL_NONE (0x0 << 28) -#define DDI_CLK_SEL_MG(0x8 << 28) -#define DDI_CLK_SEL_TBT_162 (0xC << 28) -#define DDI_CLK_SEL_TBT_270 (0xD << 28) -#define DDI_CLK_SEL_TBT_540 (0xE << 28) -#define DDI_CLK_SEL_TBT_810 (0xF << 28) -#define DDI_CLK_SEL_MASK (0xF << 28) +#define DDI_CLK_SEL_MASK REG_GENMASK(31, 28) +#define DDI_CLK_SEL_NONE REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0x0) +#define DDI_CLK_SEL_MG REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0x8) +#define DDI_CLK_SEL_TBT_162 REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xC) +#define DDI_CLK_SEL_TBT_270 REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xD) +#define DDI_CLK_SEL_TBT_540 REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xE) +#define DDI_CLK_SEL_TBT_810 REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xF) /* Transcoder clock selection */ #define _TRANS_CLK_SEL_A 0x46140 -- 2.30.2
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Write zero wms if we disable planes for icl+
== Series Details == Series: drm/i915: Write zero wms if we disable planes for icl+ URL : https://patchwork.freedesktop.org/series/104120/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately. + ^ + } +drivers/gpu/drm/i915/intel_pm.c:5931:1: warning: the frame size of 1296 bytes is larger than 1024 bytes [-Wframe-larger-than=] +drivers/gpu/drm/i915/intel_pm.c: In function ‘skl_write_zero_plane_wm’:
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Write zero wms if we disable planes for icl+
== Series Details == Series: drm/i915: Write zero wms if we disable planes for icl+ URL : https://patchwork.freedesktop.org/series/104120/ State : warning == Summary == Error: dim checkpatch failed ad72e46079ae drm/i915: Write zero wms if we disable planes for icl+ -:78: CHECK:LINE_SPACING: Please don't use multiple blank lines #78: FILE: drivers/gpu/drm/i915/intel_pm.c:5933: + + total: 0 errors, 0 warnings, 1 checks, 68 lines checked
[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: Debugfs statistics interface for psr
== Series Details == Series: drm/i915: Debugfs statistics interface for psr URL : https://patchwork.freedesktop.org/series/104115/ State : failure == Summary == CI Bug Log - changes from CI_DRM_11671_full -> Patchwork_104115v1_full Summary --- **FAILURE** Serious unknown changes coming with Patchwork_104115v1_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_104115v1_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. Participating hosts (12 -> 12) -- No changes in participating hosts Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_104115v1_full: ### IGT changes ### Possible regressions * igt@i915_pm_rpm@reg-read-ioctl: - shard-iclb: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-iclb1/igt@i915_pm_...@reg-read-ioctl.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/shard-iclb7/igt@i915_pm_...@reg-read-ioctl.html New tests - New tests have been introduced between CI_DRM_11671_full and Patchwork_104115v1_full: ### New IGT tests (1) ### * igt@kms_flip@single-buffer-flip-vs-dpms-off-vs-modeset-interruptible@d-hdmi-a3: - Statuses : 1 pass(s) - Exec time: [0.62] s Known issues Here are the changes found in Patchwork_104115v1_full that come from known issues: ### CI changes ### Possible fixes * boot: - shard-skl: ([PASS][3], [PASS][4], [PASS][5], [PASS][6], [PASS][7], [PASS][8], [PASS][9], [PASS][10], [PASS][11], [PASS][12], [PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [FAIL][18], [PASS][19], [PASS][20], [PASS][21], [PASS][22]) ([i915#5032]) -> ([PASS][23], [PASS][24], [PASS][25], [PASS][26], [PASS][27], [PASS][28], [PASS][29], [PASS][30], [PASS][31], [PASS][32], [PASS][33], [PASS][34], [PASS][35], [PASS][36], [PASS][37], [PASS][38], [PASS][39], [PASS][40], [PASS][41], [PASS][42], [PASS][43], [PASS][44]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl1/boot.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl8/boot.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl8/boot.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl8/boot.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl7/boot.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl10/boot.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl10/boot.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl9/boot.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl9/boot.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl9/boot.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl7/boot.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl6/boot.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl6/boot.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl5/boot.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl5/boot.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl5/boot.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl4/boot.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl4/boot.html [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl3/boot.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/shard-skl1/boot.html [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/shard-skl10/boot.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/shard-skl9/boot.html [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/shard-skl9/boot.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/shard-skl8/boot.html [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/shard-skl8/boot.html [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/shard-skl7/boot.html [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/shard-skl7/boot.html [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/shard-skl7/boot.html [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/shard-skl6/boot.html [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/shard-skl6/boot.html [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/shard-skl6/boot.html [34]: https://intel
[Intel-gfx] [PATCH] drm/i915: Write zero wms if we disable planes for icl+
Otherwise we seem to get FIFO underruns. It is being disabled anyway, so kind of logical to write those as zeroes, even if disabling is temporary. Signed-off-by: Stanislav Lisovskiy --- .../drm/i915/display/skl_universal_plane.c| 2 +- drivers/gpu/drm/i915/intel_pm.c | 46 +++ drivers/gpu/drm/i915/intel_pm.h | 2 + 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c index caa03324a733..c0251189c042 100644 --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c @@ -633,7 +633,7 @@ icl_plane_disable_arm(struct intel_plane *plane, if (icl_is_hdr_plane(dev_priv, plane_id)) intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), 0); - skl_write_plane_wm(plane, crtc_state); + skl_write_zero_plane_wm(plane, crtc_state); intel_psr2_disable_plane_sel_fetch(plane, crtc_state); intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index ee0047fdc95d..2470c037bfae 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5885,6 +5885,52 @@ void skl_write_plane_wm(struct intel_plane *plane, PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y); } +void skl_write_zero_plane_wm(struct intel_plane *plane, +const struct intel_crtc_state *crtc_state) +{ + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); + int level, max_level = ilk_wm_max_level(dev_priv); + enum plane_id plane_id = plane->id; + enum pipe pipe = plane->pipe; + struct skl_pipe_wm pipe_wm; + const struct skl_ddb_entry *ddb = + &crtc_state->wm.skl.plane_ddb[plane_id]; + const struct skl_ddb_entry *ddb_y = + &crtc_state->wm.skl.plane_ddb_y[plane_id]; + + for (level = 0; level <= max_level; level++) { + pipe_wm.planes[plane_id].wm[level].blocks = 0; + pipe_wm.planes[plane_id].wm[level].lines = 0; + } + + pipe_wm.planes[plane_id].trans_wm.blocks = 0; + pipe_wm.planes[plane_id].trans_wm.lines = 0; + + for (level = 0; level <= max_level; level++) + skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane_id, level), + skl_plane_wm_level(&pipe_wm, plane_id, level)); + + skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id), + skl_plane_trans_wm(&pipe_wm, plane_id)); + + if (HAS_HW_SAGV_WM(dev_priv)) { + const struct skl_plane_wm *wm = &pipe_wm.planes[plane_id]; + + skl_write_wm_level(dev_priv, PLANE_WM_SAGV(pipe, plane_id), + &wm->sagv.wm0); + skl_write_wm_level(dev_priv, PLANE_WM_SAGV_TRANS(pipe, plane_id), + &wm->sagv.trans_wm); + } + + skl_ddb_entry_write(dev_priv, + PLANE_BUF_CFG(pipe, plane_id), ddb); + + if (DISPLAY_VER(dev_priv) < 11) + skl_ddb_entry_write(dev_priv, + PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y); +} + + void skl_write_cursor_wm(struct intel_plane *plane, const struct intel_crtc_state *crtc_state) { diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h index 50604cf7398c..fb0ac4f143ab 100644 --- a/drivers/gpu/drm/i915/intel_pm.h +++ b/drivers/gpu/drm/i915/intel_pm.h @@ -61,6 +61,8 @@ bool skl_wm_level_equals(const struct skl_wm_level *l1, bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb, const struct skl_ddb_entry *entries, int num_entries, int ignore_idx); +void skl_write_zero_plane_wm(struct intel_plane *plane, +const struct intel_crtc_state *crtc_state); void skl_write_plane_wm(struct intel_plane *plane, const struct intel_crtc_state *crtc_state); void skl_write_cursor_wm(struct intel_plane *plane, -- 2.24.1.485.gad05a3d8e5
Re: [Intel-gfx] [PATCH 00/14] drm/kms: Stop registering multiple /sys/class/backlight devs for a single display
On Wed, 18 May 2022, Hans de Goede wrote: > Hi, > > On 5/18/22 10:44, Jani Nikula wrote: >> On Tue, 17 May 2022, Hans de Goede wrote: >>> Hi All, >>> >>> As mentioned in my RFC titled "drm/kms: control display brightness through >>> drm_connector properties": >>> https://lore.kernel.org/dri-devel/0d188965-d809-81b5-74ce-7d30c49fe...@redhat.com/ >>> >>> The first step towards this is to deal with some existing technical debt >>> in backlight handling on x86/ACPI boards, specifically we need to stop >>> registering multiple /sys/class/backlight devs for a single display. >> >> I guess my question here is, how do you know it's for a *single* >> display? >> >> There are already designs out there with two flat panels, with >> independent brightness controls. They're still rare and I don't think we >> handle them very well. But we've got code to register multiple native >> backlight interfaces, see e.g. commit 20f85ef89d94 ("drm/i915/backlight: >> use unique backlight device names"). >> >> So imagine a design where one of the panels needs backlight control via >> ACPI and the other via native backlight control. Granted, I don't know >> if one exists, but I think it's very much in the realm of possible >> things the OEMs might do. For example, have an EC PWM for primary panel >> backlight, and use GPU PWM for secondary. How do you know you actually >> do need to register two interfaces? > > On x86/ACPI devices this is all driven by acpi_video_get_backlight_type() / > by the drivers/acpi/video_detect.c code. That code already will break on > systems where there are 2 backlight controls, with one being ACPI based > and the other a native GPU PWM backlight device. > > In this scenario as soon as the native GPU PWM backlight device shows up > then, if acpi_video_get_backlight_type()==native, video_detect.c will > currently unregister the acpi_video# backlight device(s) since userspace > prefers the firmware type over the native type, so for userspace to > actually honor the acpi_video_get_backlight_type()==native we need to get > the acpi_video# backlight device "out of the way" which is currently > handled by unregistering it. > > Note in a way we already have a case where userspace sees 2 panels, > in hybrid laptop setups with a mux and on those systems we may see > either 2 native backlight devices; or 2 native backlight devices + > 2 acpi_video backlight devices with userspace preferring the ACPI > ones. > > Also note that userspace already has code to detect if the related > panel is active (iow which way the mux between the GPU and the panels > points) and then uses that backlight device. Userspace here very > much assumes a single panel though. > >> I'm fine with dealing with such cases as they arise to avoid >> over-engineering up front, but I also don't want us to completely paint >> ourselves in a corner either. > > Right. Note that the current code (both with and without this patchset) > already will work fine from a kernel pov as long as both panels > are either using native GPU PWM or are both using ACPI. But if we > ever get a mix then this will need special handling. > > Note that all userspace code I know is currently hardcoded > to assume a single panel. Userspace already picks one preferred > device under /sys/class/backlight and ignores the rest. Actually > atm userspace must behave this way, because on x86/ACPI boards we > do often register multiple backlight devices for a single panel. > > So in a way moving to registering only a single backlight device > prepares for actually having multiple panels work. > > Also keep in mind that this is preparation work for making the > panel brightness a drm_connector property. When the panel uses > a backlight device other then the native GPU PWM to control the > brightness then the helper code for this needs to have a way to > select which backlight_device to use then and the non native > types are not "linked" to a specific connector so in this case > we really need there to be only 1 backlight device registered > so that the code looking up the (non native) backlight device > for the connector gets the right one and not merely the one > which happened to get registered first. > > And I believe that having the panel brightness be a drm_connector > property is the way to make it possible for userspace to deal > with the multiple panels which each have a separate brightness > control case. Agreed. Thanks for the explanations and recording them here. BR, Jani. > > Regards, > > Hans > > > > > >> >> BR, >> Jani. >> >> >>> >>> This series implements my RFC describing my plan for these cleanups: >>> https://lore.kernel.org/dri-devel/98519ba0-7f18-201a-ea34-652f50343...@redhat.com/ >>> >>> Specifically patches 1-6 implement the "Fixing kms driver unconditionally >>> register their "native" backlight dev" part. >>> >>> And patches 7-14 implement the "Fixing acpi_video0 getting registered for >>> a brief time" time. >>> >>> Note this series does not deal
Re: [Intel-gfx] [PATCH 01/14] ACPI: video: Add a native function parameter to acpi_video_get_backlight_type()
Hi, On 5/18/22 10:55, Jani Nikula wrote: > On Tue, 17 May 2022, Hans de Goede wrote: >> ATM on x86 laptops where we want userspace to use the acpi_video backlight >> device we often register both the GPU's native backlight device and >> acpi_video's firmware acpi_video# backlight device. This relies on >> userspace preferring firmware type backlight devices over native ones, but >> registering 2 backlight devices for a single display really is undesirable. >> >> On x86 laptops where the native GPU backlight device should be used, >> the registering of other backlight devices is avoided by their drivers >> using acpi_video_get_backlight_type() and only registering their backlight >> if the return value matches their type. >> >> acpi_video_get_backlight_type() uses >> backlight_device_get_by_type(BACKLIGHT_RAW) to determine if a native >> driver is available and will never return native if this returns >> false. This means that the GPU's native backlight registering code >> cannot just call acpi_video_get_backlight_type() to determine if it >> should register its backlight, since acpi_video_get_backlight_type() will >> never return native until the native backlight has already registered. >> >> To fix this add a native function parameter to >> acpi_video_get_backlight_type(), which when set to true will make >> acpi_video_get_backlight_type() behave as if a native backlight has >> already been registered. >> >> Note that all current callers are updated to pass false for the new >> parameter, so this change in itself causes no functional changes. > > >> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c >> index becc198e4c22..0a06f0edd298 100644 >> --- a/drivers/acpi/video_detect.c >> +++ b/drivers/acpi/video_detect.c >> @@ -17,12 +17,14 @@ >> * Otherwise vendor specific drivers like thinkpad_acpi, asus-laptop, >> * sony_acpi,... can take care about backlight brightness. >> * >> - * Backlight drivers can use acpi_video_get_backlight_type() to determine >> - * which driver should handle the backlight. >> + * Backlight drivers can use acpi_video_get_backlight_type() to determine >> which >> + * driver should handle the backlight. RAW/GPU-driver backlight drivers must >> + * pass true for the native function argument, other drivers must pass >> false. >> * >> * If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module >> (m) >> * this file will not be compiled and acpi_video_get_backlight_type() will >> - * always return acpi_backlight_vendor. >> + * return acpi_backlight_native when its native argument is true and >> + * acpi_backlight_vendor when it is false. >> */ > > Frankly, I think the boolean native parameter here, and at the call > sites, is confusing, and the slightly different explanations in the > commit message and comment here aren't helping. Can you elaborate the "slightly different explanations in the commit message and comment" part a bit (so that I can fix this) ? > I suggest adding a separate function that the native backlight drivers > should use. I think it's more obvious all around, and easier to document > too. Code wise I think this would mean renaming the original and then adding 2 wrappers, but that is fine with me. I've no real preference either way and I'm happy with adding a new variant of acpi_video_get_backlight_type() for the native backlight drivers any suggestion for a name ? Regards, Hans
Re: [Intel-gfx] [PATCH 00/14] drm/kms: Stop registering multiple /sys/class/backlight devs for a single display
Hi, On 5/18/22 10:44, Jani Nikula wrote: > On Tue, 17 May 2022, Hans de Goede wrote: >> Hi All, >> >> As mentioned in my RFC titled "drm/kms: control display brightness through >> drm_connector properties": >> https://lore.kernel.org/dri-devel/0d188965-d809-81b5-74ce-7d30c49fe...@redhat.com/ >> >> The first step towards this is to deal with some existing technical debt >> in backlight handling on x86/ACPI boards, specifically we need to stop >> registering multiple /sys/class/backlight devs for a single display. > > I guess my question here is, how do you know it's for a *single* > display? > > There are already designs out there with two flat panels, with > independent brightness controls. They're still rare and I don't think we > handle them very well. But we've got code to register multiple native > backlight interfaces, see e.g. commit 20f85ef89d94 ("drm/i915/backlight: > use unique backlight device names"). > > So imagine a design where one of the panels needs backlight control via > ACPI and the other via native backlight control. Granted, I don't know > if one exists, but I think it's very much in the realm of possible > things the OEMs might do. For example, have an EC PWM for primary panel > backlight, and use GPU PWM for secondary. How do you know you actually > do need to register two interfaces? On x86/ACPI devices this is all driven by acpi_video_get_backlight_type() / by the drivers/acpi/video_detect.c code. That code already will break on systems where there are 2 backlight controls, with one being ACPI based and the other a native GPU PWM backlight device. In this scenario as soon as the native GPU PWM backlight device shows up then, if acpi_video_get_backlight_type()==native, video_detect.c will currently unregister the acpi_video# backlight device(s) since userspace prefers the firmware type over the native type, so for userspace to actually honor the acpi_video_get_backlight_type()==native we need to get the acpi_video# backlight device "out of the way" which is currently handled by unregistering it. Note in a way we already have a case where userspace sees 2 panels, in hybrid laptop setups with a mux and on those systems we may see either 2 native backlight devices; or 2 native backlight devices + 2 acpi_video backlight devices with userspace preferring the ACPI ones. Also note that userspace already has code to detect if the related panel is active (iow which way the mux between the GPU and the panels points) and then uses that backlight device. Userspace here very much assumes a single panel though. > I'm fine with dealing with such cases as they arise to avoid > over-engineering up front, but I also don't want us to completely paint > ourselves in a corner either. Right. Note that the current code (both with and without this patchset) already will work fine from a kernel pov as long as both panels are either using native GPU PWM or are both using ACPI. But if we ever get a mix then this will need special handling. Note that all userspace code I know is currently hardcoded to assume a single panel. Userspace already picks one preferred device under /sys/class/backlight and ignores the rest. Actually atm userspace must behave this way, because on x86/ACPI boards we do often register multiple backlight devices for a single panel. So in a way moving to registering only a single backlight device prepares for actually having multiple panels work. Also keep in mind that this is preparation work for making the panel brightness a drm_connector property. When the panel uses a backlight device other then the native GPU PWM to control the brightness then the helper code for this needs to have a way to select which backlight_device to use then and the non native types are not "linked" to a specific connector so in this case we really need there to be only 1 backlight device registered so that the code looking up the (non native) backlight device for the connector gets the right one and not merely the one which happened to get registered first. And I believe that having the panel brightness be a drm_connector property is the way to make it possible for userspace to deal with the multiple panels which each have a separate brightness control case. Regards, Hans > > BR, > Jani. > > >> >> This series implements my RFC describing my plan for these cleanups: >> https://lore.kernel.org/dri-devel/98519ba0-7f18-201a-ea34-652f50343...@redhat.com/ >> >> Specifically patches 1-6 implement the "Fixing kms driver unconditionally >> register their "native" backlight dev" part. >> >> And patches 7-14 implement the "Fixing acpi_video0 getting registered for >> a brief time" time. >> >> Note this series does not deal yet with the "Other issues" part, I plan >> to do a follow up series for that. >> >> The changes in this series are good to have regardless of the further >> "drm/kms: control display brightness through drm_connector properties" >> plans. So I plan to push these upstream once t
Re: [Intel-gfx] [RFC PATCH v3 02/19] KVM: x86: inhibit APICv/AVIC when the guest and/or host changes apic id/base from the defaults.
On Wed, 2022-05-18 at 16:28 +0800, Chao Gao wrote: > On Wed, Apr 27, 2022 at 11:02:57PM +0300, Maxim Levitsky wrote: > > Neither of these settings should be changed by the guest and it is > > a burden to support it in the acceleration code, so just inhibit > > it instead. > > > > Also add a boolean 'apic_id_changed' to indicate if apic id ever changed. > > > > Signed-off-by: Maxim Levitsky > > --- > > arch/x86/include/asm/kvm_host.h | 3 +++ > > arch/x86/kvm/lapic.c| 25 ++--- > > arch/x86/kvm/lapic.h| 8 > > 3 files changed, 33 insertions(+), 3 deletions(-) > > > > diff --git a/arch/x86/include/asm/kvm_host.h > > b/arch/x86/include/asm/kvm_host.h > > index 63eae00625bda..636df87542555 100644 > > --- a/arch/x86/include/asm/kvm_host.h > > +++ b/arch/x86/include/asm/kvm_host.h > > @@ -1070,6 +1070,8 @@ enum kvm_apicv_inhibit { > > APICV_INHIBIT_REASON_ABSENT, > > /* AVIC is disabled because SEV doesn't support it */ > > APICV_INHIBIT_REASON_SEV, > > + /* APIC ID and/or APIC base was changed by the guest */ > > + APICV_INHIBIT_REASON_RO_SETTINGS, > > You need to add it to check_apicv_inhibit_reasons as well. True, forgot about it. > > > }; > > > > struct kvm_arch { > > @@ -1258,6 +1260,7 @@ struct kvm_arch { > > hpa_t hv_root_tdp; > > spinlock_t hv_root_tdp_lock; > > #endif > > + bool apic_id_changed; > > What's the value of this boolean? No one reads it. I use it in later patches to kill the guest during nested VM entry if it attempts to use nested AVIC after any vCPU changed APIC ID. I mentioned this boolean in the commit description. This boolean avoids the need to go over all vCPUs and checking if they still have the initial apic id. In the future maybe we can introduce a more generic 'taint' bitmap with various flags like that, indicating that the guest did something unexpected. BTW, the other option in regard to the nested AVIC is just to ignore this issue completely. The code itself always uses vcpu_id's, thus regardless of when/how often the guest changes its apic ids, my code would just use the initial APIC ID values consistently. In this case I won't need this boolean. > > > }; > > > > struct kvm_vm_stat { > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > > index 66b0eb0bda94e..8996675b3ef4c 100644 > > --- a/arch/x86/kvm/lapic.c > > +++ b/arch/x86/kvm/lapic.c > > @@ -2038,6 +2038,19 @@ static void apic_manage_nmi_watchdog(struct > > kvm_lapic *apic, u32 lvt0_val) > > } > > } > > > > +static void kvm_lapic_check_initial_apic_id(struct kvm_lapic *apic) > > +{ > > + if (kvm_apic_has_initial_apic_id(apic)) > > + return; > > + > > + pr_warn_once("APIC ID change is unsupported by KVM"); > > It is misleading because changing xAPIC ID is supported by KVM; it just > isn't compatible with APICv. Probably this pr_warn_once() should be > removed. Honestly since nobody uses this feature, I am not sure if to call this supported, I am sure that KVM has more bugs in regard of using non standard APIC ID. This warning might hopefuly make someone complain about it if this feature is actually used somewhere. > > > + > > + kvm_set_apicv_inhibit(apic->vcpu->kvm, > > + APICV_INHIBIT_REASON_RO_SETTINGS); > > The indentation here looks incorrect to me. > kvm_set_apicv_inhibit(apic->vcpu->kvm, > APICV_INHIBIT_REASON_RO_SETTINGS); True, will fix. > > > + > > + apic->vcpu->kvm->arch.apic_id_changed = true; > > +} > > + > > static int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val) > > { > > int ret = 0; > > @@ -2046,9 +2059,11 @@ static int kvm_lapic_reg_write(struct kvm_lapic > > *apic, u32 reg, u32 val) > > > > switch (reg) { > > case APIC_ID: /* Local APIC ID */ > > - if (!apic_x2apic_mode(apic)) > > + if (!apic_x2apic_mode(apic)) { > > + > > kvm_apic_set_xapic_id(apic, val >> 24); > > - else > > + kvm_lapic_check_initial_apic_id(apic); > > + } else > > ret = 1; > > break; > > > > @@ -2335,8 +2350,11 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 > > value) > > MSR_IA32_APICBASE_BASE; > > > > if ((value & MSR_IA32_APICBASE_ENABLE) && > > -apic->base_address != APIC_DEFAULT_PHYS_BASE) > > +apic->base_address != APIC_DEFAULT_PHYS_BASE) { > > + kvm_set_apicv_inhibit(apic->vcpu->kvm, > > + APICV_INHIBIT_REASON_RO_SETTINGS); > > pr_warn_once("APIC base relocation is unsupported by KVM"); > > + } > > } > > > > void kvm_apic_update_apicv(struct kvm_vcpu *vcpu) > > @@ -2649,6 +2667,7 @@ static int kvm_apic_state_fixup(struct kvm_vcpu *vcpu, > > } > > } > > > > + kvm_lapic_check_initial_apic_id(vcpu->arch.apic); > > return 0; > > } > > > > diff --git a/arch/x86/kv
Re: [Intel-gfx] [PATCH] drm/i915: check fence to avoid null pointer dereference
Hi, On 17/05/2022 17:56, Yongzhi Liu wrote: if drm_syncobj_fence_get return null, we will get a null pointer. Fix this by adding the null pointer check on fence. Signed-off-by: Yongzhi Liu --- drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index fd0e15d..3a82a62 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -3184,6 +3184,10 @@ eb_fences_add(struct i915_execbuffer *eb, struct i915_request *rq, struct dma_fence *fence; fence = drm_syncobj_fence_get(eb->gem_context->syncobj); + if (!fence) { + DRM_DEBUG("Syncobj has no fence\n"); + return ERR_PTR(-EINVAL); Lookup can't fail here since reference to context is held and syncobj can't get replaced during that time. You could make it a single GEM_BUG_ON(!fence) instead if you really wanted but I am not convinced it would be useful. Regards, Tvrtko + } err = i915_request_await_dma_fence(rq, fence); dma_fence_put(fence); if (err)
Re: [Intel-gfx] [PATCH 01/14] ACPI: video: Add a native function parameter to acpi_video_get_backlight_type()
On Tue, 17 May 2022, Hans de Goede wrote: > ATM on x86 laptops where we want userspace to use the acpi_video backlight > device we often register both the GPU's native backlight device and > acpi_video's firmware acpi_video# backlight device. This relies on > userspace preferring firmware type backlight devices over native ones, but > registering 2 backlight devices for a single display really is undesirable. > > On x86 laptops where the native GPU backlight device should be used, > the registering of other backlight devices is avoided by their drivers > using acpi_video_get_backlight_type() and only registering their backlight > if the return value matches their type. > > acpi_video_get_backlight_type() uses > backlight_device_get_by_type(BACKLIGHT_RAW) to determine if a native > driver is available and will never return native if this returns > false. This means that the GPU's native backlight registering code > cannot just call acpi_video_get_backlight_type() to determine if it > should register its backlight, since acpi_video_get_backlight_type() will > never return native until the native backlight has already registered. > > To fix this add a native function parameter to > acpi_video_get_backlight_type(), which when set to true will make > acpi_video_get_backlight_type() behave as if a native backlight has > already been registered. > > Note that all current callers are updated to pass false for the new > parameter, so this change in itself causes no functional changes. > diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c > index becc198e4c22..0a06f0edd298 100644 > --- a/drivers/acpi/video_detect.c > +++ b/drivers/acpi/video_detect.c > @@ -17,12 +17,14 @@ > * Otherwise vendor specific drivers like thinkpad_acpi, asus-laptop, > * sony_acpi,... can take care about backlight brightness. > * > - * Backlight drivers can use acpi_video_get_backlight_type() to determine > - * which driver should handle the backlight. > + * Backlight drivers can use acpi_video_get_backlight_type() to determine > which > + * driver should handle the backlight. RAW/GPU-driver backlight drivers must > + * pass true for the native function argument, other drivers must pass false. > * > * If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module > (m) > * this file will not be compiled and acpi_video_get_backlight_type() will > - * always return acpi_backlight_vendor. > + * return acpi_backlight_native when its native argument is true and > + * acpi_backlight_vendor when it is false. > */ Frankly, I think the boolean native parameter here, and at the call sites, is confusing, and the slightly different explanations in the commit message and comment here aren't helping. I suggest adding a separate function that the native backlight drivers should use. I think it's more obvious all around, and easier to document too. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 00/14] drm/kms: Stop registering multiple /sys/class/backlight devs for a single display
On Tue, 17 May 2022, Hans de Goede wrote: > Hi All, > > As mentioned in my RFC titled "drm/kms: control display brightness through > drm_connector properties": > https://lore.kernel.org/dri-devel/0d188965-d809-81b5-74ce-7d30c49fe...@redhat.com/ > > The first step towards this is to deal with some existing technical debt > in backlight handling on x86/ACPI boards, specifically we need to stop > registering multiple /sys/class/backlight devs for a single display. I guess my question here is, how do you know it's for a *single* display? There are already designs out there with two flat panels, with independent brightness controls. They're still rare and I don't think we handle them very well. But we've got code to register multiple native backlight interfaces, see e.g. commit 20f85ef89d94 ("drm/i915/backlight: use unique backlight device names"). So imagine a design where one of the panels needs backlight control via ACPI and the other via native backlight control. Granted, I don't know if one exists, but I think it's very much in the realm of possible things the OEMs might do. For example, have an EC PWM for primary panel backlight, and use GPU PWM for secondary. How do you know you actually do need to register two interfaces? I'm fine with dealing with such cases as they arise to avoid over-engineering up front, but I also don't want us to completely paint ourselves in a corner either. BR, Jani. > > This series implements my RFC describing my plan for these cleanups: > https://lore.kernel.org/dri-devel/98519ba0-7f18-201a-ea34-652f50343...@redhat.com/ > > Specifically patches 1-6 implement the "Fixing kms driver unconditionally > register their "native" backlight dev" part. > > And patches 7-14 implement the "Fixing acpi_video0 getting registered for > a brief time" time. > > Note this series does not deal yet with the "Other issues" part, I plan > to do a follow up series for that. > > The changes in this series are good to have regardless of the further > "drm/kms: control display brightness through drm_connector properties" > plans. So I plan to push these upstream once they are ready (once > reviewed). Since this crosses various subsystems / touches multiple > kms drivers my plan is to provide an immutable branch based on say > 5.19-rc1 and then have that get merged into all the relevant trees. > > Please review. > > Regards, > > Hans > > > Hans de Goede (14): > ACPI: video: Add a native function parameter to > acpi_video_get_backlight_type() > drm/i915: Don't register backlight when another backlight should be > used > drm/amdgpu: Don't register backlight when another backlight should be > used > drm/radeon: Don't register backlight when another backlight should be > used > drm/nouveau: Don't register backlight when another backlight should be > used > ACPI: video: Drop backlight_device_get_by_type() call from > acpi_video_get_backlight_type() > ACPI: video: Remove acpi_video_bus from list before tearing it down > ACPI: video: Simplify acpi_video_unregister_backlight() > ACPI: video: Make backlight class device registration a separate step > ACPI: video: Remove code to unregister acpi_video backlight when a > native backlight registers > drm/i915: Call acpi_video_register_backlight() > drm/nouveau: Register ACPI video backlight when nv_backlight > registration fails > drm/amdgpu: Register ACPI video backlight when skipping amdgpu > backlight registration > drm/radeon: Register ACPI video backlight when skipping radeon > backlight registration > > drivers/acpi/acpi_video.c | 69 ++- > drivers/acpi/video_detect.c | 53 +++--- > drivers/gpu/drm/Kconfig | 2 + > .../gpu/drm/amd/amdgpu/atombios_encoders.c| 14 +++- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 9 +++ > .../gpu/drm/i915/display/intel_backlight.c| 7 ++ > drivers/gpu/drm/i915/display/intel_display.c | 1 + > drivers/gpu/drm/i915/display/intel_opregion.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_backlight.c | 14 > drivers/gpu/drm/radeon/atombios_encoders.c| 7 ++ > drivers/gpu/drm/radeon/radeon_encoders.c | 11 ++- > .../gpu/drm/radeon/radeon_legacy_encoders.c | 7 ++ > drivers/platform/x86/acer-wmi.c | 2 +- > drivers/platform/x86/asus-laptop.c| 2 +- > drivers/platform/x86/asus-wmi.c | 4 +- > drivers/platform/x86/compal-laptop.c | 2 +- > drivers/platform/x86/dell/dell-laptop.c | 2 +- > drivers/platform/x86/eeepc-laptop.c | 2 +- > drivers/platform/x86/fujitsu-laptop.c | 4 +- > drivers/platform/x86/ideapad-laptop.c | 2 +- > drivers/platform/x86/intel/oaktrail.c | 2 +- > drivers/platform/x86/msi-laptop.c | 2 +- > drivers/platform/x86/msi-wmi.c| 2 +- > drivers/platform/x86/samsung-laptop.c | 2 +- > drivers
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Debugfs statistics interface for psr
== Series Details == Series: drm/i915: Debugfs statistics interface for psr URL : https://patchwork.freedesktop.org/series/104115/ State : success == Summary == CI Bug Log - changes from CI_DRM_11671 -> Patchwork_104115v1 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/index.html Participating hosts (44 -> 40) -- Missing(4): bat-rpls-2 fi-rkl-11600 fi-icl-u2 bat-dg2-9 Known issues Here are the changes found in Patchwork_104115v1 that come from known issues: ### IGT changes ### Issues hit * igt@i915_selftest@live@gem_contexts: - fi-bdw-5557u: [PASS][1] -> [INCOMPLETE][2] ([i915#5502] / [i915#5801]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/fi-bdw-5557u/igt@i915_selftest@live@gem_contexts.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/fi-bdw-5557u/igt@i915_selftest@live@gem_contexts.html * igt@i915_selftest@live@requests: - fi-blb-e6850: [PASS][3] -> [DMESG-FAIL][4] ([i915#4528]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/fi-blb-e6850/igt@i915_selftest@l...@requests.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/fi-blb-e6850/igt@i915_selftest@l...@requests.html * igt@kms_chamelium@common-hpd-after-suspend: - fi-pnv-d510:NOTRUN -> [SKIP][5] ([fdo#109271]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/fi-pnv-d510/igt@kms_chamel...@common-hpd-after-suspend.html Possible fixes * igt@gem_exec_suspend@basic-s0@smem: - {fi-ehl-2}: [DMESG-WARN][6] ([i915#5122]) -> [PASS][7] [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/fi-ehl-2/igt@gem_exec_suspend@basic...@smem.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/fi-ehl-2/igt@gem_exec_suspend@basic...@smem.html * igt@i915_selftest@live@gem: - fi-pnv-d510:[DMESG-FAIL][8] ([i915#4528]) -> [PASS][9] [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/fi-pnv-d510/igt@i915_selftest@l...@gem.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/fi-pnv-d510/igt@i915_selftest@l...@gem.html * igt@i915_selftest@live@gt_timelines: - {bat-adlm-1}: [INCOMPLETE][10] ([i915#5801]) -> [PASS][11] [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11671/bat-adlm-1/igt@i915_selftest@live@gt_timelines.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/bat-adlm-1/igt@i915_selftest@live@gt_timelines.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827 [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576 [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528 [i915#5122]: https://gitlab.freedesktop.org/drm/intel/issues/5122 [i915#5502]: https://gitlab.freedesktop.org/drm/intel/issues/5502 [i915#5763]: https://gitlab.freedesktop.org/drm/intel/issues/5763 [i915#5801]: https://gitlab.freedesktop.org/drm/intel/issues/5801 [i915#5885]: https://gitlab.freedesktop.org/drm/intel/issues/5885 Build changes - * Linux: CI_DRM_11671 -> Patchwork_104115v1 CI-20190529: 20190529 CI_DRM_11671: 18756156cc1e97e635bcb4dd9da7d1465278f2c4 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_6478: 7c3ceb08b633a66f77f42e596593de394da5dccc @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_104115v1: 18756156cc1e97e635bcb4dd9da7d1465278f2c4 @ git://anongit.freedesktop.org/gfx-ci/linux ### Linux commits 889d0e1fb48e drm/i915: Debugfs statistics interface for psr == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104115v1/index.html
Re: [Intel-gfx] [RFC PATCH v3 02/19] KVM: x86: inhibit APICv/AVIC when the guest and/or host changes apic id/base from the defaults.
On Wed, Apr 27, 2022 at 11:02:57PM +0300, Maxim Levitsky wrote: >Neither of these settings should be changed by the guest and it is >a burden to support it in the acceleration code, so just inhibit >it instead. > >Also add a boolean 'apic_id_changed' to indicate if apic id ever changed. > >Signed-off-by: Maxim Levitsky >--- > arch/x86/include/asm/kvm_host.h | 3 +++ > arch/x86/kvm/lapic.c| 25 ++--- > arch/x86/kvm/lapic.h| 8 > 3 files changed, 33 insertions(+), 3 deletions(-) > >diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h >index 63eae00625bda..636df87542555 100644 >--- a/arch/x86/include/asm/kvm_host.h >+++ b/arch/x86/include/asm/kvm_host.h >@@ -1070,6 +1070,8 @@ enum kvm_apicv_inhibit { > APICV_INHIBIT_REASON_ABSENT, > /* AVIC is disabled because SEV doesn't support it */ > APICV_INHIBIT_REASON_SEV, >+ /* APIC ID and/or APIC base was changed by the guest */ >+ APICV_INHIBIT_REASON_RO_SETTINGS, You need to add it to check_apicv_inhibit_reasons as well. > }; > > struct kvm_arch { >@@ -1258,6 +1260,7 @@ struct kvm_arch { > hpa_t hv_root_tdp; > spinlock_t hv_root_tdp_lock; > #endif >+ bool apic_id_changed; What's the value of this boolean? No one reads it. > }; > > struct kvm_vm_stat { >diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c >index 66b0eb0bda94e..8996675b3ef4c 100644 >--- a/arch/x86/kvm/lapic.c >+++ b/arch/x86/kvm/lapic.c >@@ -2038,6 +2038,19 @@ static void apic_manage_nmi_watchdog(struct kvm_lapic >*apic, u32 lvt0_val) > } > } > >+static void kvm_lapic_check_initial_apic_id(struct kvm_lapic *apic) >+{ >+ if (kvm_apic_has_initial_apic_id(apic)) >+ return; >+ >+ pr_warn_once("APIC ID change is unsupported by KVM"); It is misleading because changing xAPIC ID is supported by KVM; it just isn't compatible with APICv. Probably this pr_warn_once() should be removed. >+ >+ kvm_set_apicv_inhibit(apic->vcpu->kvm, >+ APICV_INHIBIT_REASON_RO_SETTINGS); The indentation here looks incorrect to me. kvm_set_apicv_inhibit(apic->vcpu->kvm, APICV_INHIBIT_REASON_RO_SETTINGS); >+ >+ apic->vcpu->kvm->arch.apic_id_changed = true; >+} >+ > static int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val) > { > int ret = 0; >@@ -2046,9 +2059,11 @@ static int kvm_lapic_reg_write(struct kvm_lapic *apic, >u32 reg, u32 val) > > switch (reg) { > case APIC_ID: /* Local APIC ID */ >- if (!apic_x2apic_mode(apic)) >+ if (!apic_x2apic_mode(apic)) { >+ > kvm_apic_set_xapic_id(apic, val >> 24); >- else >+ kvm_lapic_check_initial_apic_id(apic); >+ } else > ret = 1; > break; > >@@ -2335,8 +2350,11 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 >value) >MSR_IA32_APICBASE_BASE; > > if ((value & MSR_IA32_APICBASE_ENABLE) && >- apic->base_address != APIC_DEFAULT_PHYS_BASE) >+ apic->base_address != APIC_DEFAULT_PHYS_BASE) { >+ kvm_set_apicv_inhibit(apic->vcpu->kvm, >+ APICV_INHIBIT_REASON_RO_SETTINGS); > pr_warn_once("APIC base relocation is unsupported by KVM"); >+ } > } > > void kvm_apic_update_apicv(struct kvm_vcpu *vcpu) >@@ -2649,6 +2667,7 @@ static int kvm_apic_state_fixup(struct kvm_vcpu *vcpu, > } > } > >+ kvm_lapic_check_initial_apic_id(vcpu->arch.apic); > return 0; > } > >diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h >index 4e4f8a22754f9..b9c406d383080 100644 >--- a/arch/x86/kvm/lapic.h >+++ b/arch/x86/kvm/lapic.h >@@ -252,4 +252,12 @@ static inline u8 kvm_xapic_id(struct kvm_lapic *apic) > return kvm_lapic_get_reg(apic, APIC_ID) >> 24; > } > >+static inline bool kvm_apic_has_initial_apic_id(struct kvm_lapic *apic) >+{ >+ if (apic_x2apic_mode(apic)) >+ return true; I suggest warning of x2apic mode: if (WARN_ON_ONCE(apic_x2apic_mode(apic))) Because it is weird that callers care about initial apic id when apic is in x2apic mode.
Re: [Intel-gfx] [PATCH 1/1] vfio: remove VFIO_GROUP_NOTIFY_SET_KVM
With this the release_work in gvt can go away as well: diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h index 633acfcf76bf2..aee1a45da74bc 100644 --- a/drivers/gpu/drm/i915/gvt/gvt.h +++ b/drivers/gpu/drm/i915/gvt/gvt.h @@ -227,7 +227,6 @@ struct intel_vgpu { struct mutex cache_lock; struct notifier_block iommu_notifier; - struct work_struct release_work; atomic_t released; struct kvm_page_track_notifier_node track_node; diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index b317ae4cc7d2d..917617d7599a9 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -228,8 +228,6 @@ static void intel_gvt_cleanup_vgpu_type_groups(struct intel_gvt *gvt) } } -static void intel_vgpu_release_work(struct work_struct *work); - static void gvt_unpin_guest_page(struct intel_vgpu *vgpu, unsigned long gfn, unsigned long size) { @@ -850,8 +848,9 @@ static void intel_vgpu_release_msi_eventfd_ctx(struct intel_vgpu *vgpu) } } -static void __intel_vgpu_release(struct intel_vgpu *vgpu) +static void intel_vgpu_close_device(struct vfio_device *vfio_dev) { + struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev); struct drm_i915_private *i915 = vgpu->gvt->gt->i915; int ret; @@ -880,19 +879,6 @@ static void __intel_vgpu_release(struct intel_vgpu *vgpu) vgpu->attached = false; } -static void intel_vgpu_close_device(struct vfio_device *vfio_dev) -{ - __intel_vgpu_release(vfio_dev_to_vgpu(vfio_dev)); -} - -static void intel_vgpu_release_work(struct work_struct *work) -{ - struct intel_vgpu *vgpu = - container_of(work, struct intel_vgpu, release_work); - - __intel_vgpu_release(vgpu); -} - static u64 intel_vgpu_get_bar_addr(struct intel_vgpu *vgpu, int bar) { u32 start_lo, start_hi; @@ -1639,7 +1625,6 @@ static int intel_vgpu_probe(struct mdev_device *mdev) return PTR_ERR(vgpu); } - INIT_WORK(&vgpu->release_work, intel_vgpu_release_work); vfio_init_group_dev(&vgpu->vfio_device, &mdev->dev, &intel_vgpu_dev_ops);
[Intel-gfx] [RFC PATCH] drm/i915: Debugfs statistics interface for psr
Currently there is no mean to get understanding how psr is utilized. E.g. How much psr is actually enabled or how often driver falls back to full update. This patch addresses this by adding new debugfs interface i915_edp_psr_stats. Statistics gathering is started by writing 1/y/Y and stopped by writing 0/n/N into this new interface. Following fields are provided for reading by this new interface: "PSR disabled vblank count" Over how many vblank periods PSR was disabled after statistics gathering got started. I.e. How many normal updates were sent to panel. "Total vblank count" Total vblank count after statistics gathering got started. "Selective update count" How many selective updates (PSR2) were done after statistics gathering got started. "Full update count" How many times driver decided to fall back to full update when trying to perform selective update. Cc: José Roberto de Souza Cc: Mika Kahola Cc: Uma Shankar Cc: Nischal Varide Signed-off-by: Jouni Högander --- .../drm/i915/display/intel_display_debugfs.c | 100 .../drm/i915/display/intel_display_types.h| 16 ++ drivers/gpu/drm/i915/display/intel_psr.c | 144 ++ drivers/gpu/drm/i915/display/intel_psr.h | 2 + 4 files changed, 236 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c index 452d773fd4e3..c29f151062e4 100644 --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c @@ -9,6 +9,7 @@ #include #include "i915_debugfs.h" +#include "intel_crtc.h" #include "intel_de.h" #include "intel_display_debugfs.h" #include "intel_display_power.h" @@ -1868,6 +1869,95 @@ i915_fifo_underrun_reset_write(struct file *filp, return cnt; } +static int intel_psr_stats(struct seq_file *m, struct intel_dp *intel_dp) +{ + struct drm_i915_private *dev_priv = (m->private); + struct intel_psr *psr = &intel_dp->psr; + struct drm_crtc *crtc = &intel_crtc_for_pipe(dev_priv, psr->pipe)->base; + u64 total_vblank_count = psr->stats.total_vblank_count, + non_psr_vblank_count = psr->stats.non_psr_vblank_count; + ktime_t vblanktime; + + if (!psr->active) + non_psr_vblank_count += drm_crtc_vblank_count_and_time(crtc, &vblanktime) - + psr->stats.psr_disable_vblank; + + seq_printf(m, "PSR disabled vblank count : %llu\n", non_psr_vblank_count); + + if (psr->stats.enable) + total_vblank_count += drm_crtc_vblank_count_and_time(crtc, &vblanktime) - + psr->stats.start_vblank; + + seq_printf(m, "Total vblank count : %llu\n", total_vblank_count); + seq_printf(m, "Selective update count : %llu\n", psr->stats.selective_update_count); + seq_printf(m, "Full update count : %llu\n", psr->stats.full_update_count); + + return 0; +} + +static int i915_edp_psr_stats_show(struct seq_file *m, void *data) +{ + struct drm_i915_private *dev_priv = (m->private); + struct intel_dp *intel_dp = NULL; + struct intel_encoder *encoder; + + if (!HAS_PSR(dev_priv)) + return -ENODEV; + + /* Find the first EDP which supports PSR */ + for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) { + intel_dp = enc_to_intel_dp(encoder); + break; + } + + if (!intel_dp) + return -ENODEV; + + return intel_psr_stats(m, intel_dp); +} + +static ssize_t +i915_edp_psr_stats_write(struct file *filp, const char __user *ubuf, +size_t cnt, loff_t *ppos) +{ + struct seq_file *m = filp->private_data; + struct drm_i915_private *dev_priv = m->private; + struct intel_dp *intel_dp = NULL; + struct intel_encoder *encoder; + int ret; + bool enable_stats; + + ret = kstrtobool_from_user(ubuf, cnt, &enable_stats); + if (ret) + return ret; + + if (!HAS_PSR(dev_priv)) + return -ENODEV; + + /* Find the first EDP which supports PSR */ + for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) { + intel_dp = enc_to_intel_dp(encoder); + break; + } + + if (!intel_dp) + return -ENODEV; + + if (enable_stats) + intel_psr_stats_enable_stats(intel_dp); + else + intel_psr_stats_disable_stats(intel_dp); + + return cnt; +} + +static int i915_edp_psr_stats_open(struct inode *inode, struct file *file) +{ + struct drm_i915_private *dev_priv = inode->i_private; + + return single_open(file, i915_edp_psr_stats_show, dev_priv); +} + static const struct file_operations i915_fifo_underrun_reset_ops = { .owner = THIS_MODULE, .open = simple_open, @@ -1875,6 +1965,15 @@ static const struct file_operations i915_fifo_unde
Re: [Intel-gfx] [PATCH 1/1] vfio: remove VFIO_GROUP_NOTIFY_SET_KVM
> if (device->ops->flags & VFIO_DEVICE_NEEDS_KVM) > { Nit: this is not the normal brace placement. But what is you diff against anyway? The one Matthew sent did away with the VFIO_DEVICE_NEEDS_KVM flags, which does the wrong thing for zpci, so it can't be that.. Also if we want to do major code movement, it really needs to go into a separate patch or patches, as the combinations of all these moves with actual code changes is almost unreadable.
Re: [Intel-gfx] [PATCH v1 2/2] drm/i915: Reject the atomic modeset if an associated Type-C port is disconnected
Hi Imre, > On Mon, May 16, 2022 at 01:54:02AM -0700, Vivek Kasireddy wrote: > > Although, doing a modeset on any disconnected connector might be futile, > > it can be particularly problematic if the connector is a Type-C port > > without a sink. And, the spec only says "Display software must not use > > a disconnected port" while referring to the Type-C DDI seqeuence, it does > > not spell out what happens if such an attempt is made. Experimental results > > have shown that this can lead to serious issues including a system hang. > > Therefore, reject the atomic modeset if we detect that the Type-C port > > is not connected. > > > > Cc: Ville Syrjälä > > Signed-off-by: Vivek Kasireddy > > --- > > drivers/gpu/drm/i915/display/intel_atomic.c | 20 > > 1 file changed, 20 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c > b/drivers/gpu/drm/i915/display/intel_atomic.c > > index 40da7910f845..40576964b8c1 100644 > > --- a/drivers/gpu/drm/i915/display/intel_atomic.c > > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c > > @@ -114,6 +114,8 @@ int intel_digital_connector_atomic_set_property(struct > drm_connector *connector, > > int intel_digital_connector_atomic_check(struct drm_connector *conn, > > struct drm_atomic_state *state) > > { > > + struct drm_device *dev = conn->dev; > > + struct drm_i915_private *dev_priv = to_i915(dev); > > struct drm_connector_state *new_state = > > drm_atomic_get_new_connector_state(state, conn); > > struct intel_digital_connector_state *new_conn_state = > > @@ -122,6 +124,10 @@ int intel_digital_connector_atomic_check(struct > drm_connector *conn, > > drm_atomic_get_old_connector_state(state, conn); > > struct intel_digital_connector_state *old_conn_state = > > to_intel_digital_connector_state(old_state); > > + struct intel_encoder *encoder = > > + intel_attached_encoder(to_intel_connector(conn)); > > + struct intel_digital_port *dig_port = > > + encoder ? enc_to_dig_port(encoder) : NULL; > > struct drm_crtc_state *crtc_state; > > > > intel_hdcp_atomic_check(conn, old_state, new_state); > > @@ -131,6 +137,20 @@ int intel_digital_connector_atomic_check(struct > drm_connector *conn, > > > > crtc_state = drm_atomic_get_new_crtc_state(state, new_state->crtc); > > > > + /* > > +* The spec says that it is not safe to use a disconnected Type-C port. > > +* Therefore, check to see if this connector is connected and reject > > +* the modeset if there is no sink detected. > > +*/ > > + if (dig_port && !dig_port->connected(encoder) && > > This check is racy, as right after dig_port->connected() returns true, > the port can become disconnected. [Kasireddy, Vivek] Given that, do you think the only way to reliably determine if the Type-C port has a sink is to check the live status and ignore dig_port->tc_mode? If that is the case, should I just add a function pointer to dig_port to call tc_port_live_status_mask()? Or, should I just change intel_tc_port_connected() to ignore dig_port->tc_mode like below: @@ -764,8 +764,7 @@ bool intel_tc_port_connected(struct intel_encoder *encoder) intel_tc_port_lock(dig_port); - is_connected = tc_port_live_status_mask(dig_port) & - BIT(dig_port->tc_mode); + is_connected = tc_port_live_status_mask(dig_port); Or, are there any other elegant ways that you can think of to determine whether a tc port has a sink or not? Thanks, Vivek > > > + intel_phy_is_tc(dev_priv, > > + intel_port_to_phy(dev_priv, encoder->port))) { > > + drm_dbg_atomic(&dev_priv->drm, > > + "[CONNECTOR:%d:%s] is not connected; rejecting > > the > modeset\n", > > + conn->base.id, conn->name); > > + return -EINVAL; > > + } > > + > > /* > > * These properties are handled by fastset, and might not end > > * up in a modeset. > > -- > > 2.35.1 > >