[Intel-gfx] [PATCH v3 0/2] Enable GVT suspend/resume
This patchset enables GVT suspend/resume so that GVT enabled VM can continue running after host resuming from suspend state. V2: - Change kzalloc/kfree to vzalloc/vfree since the space allocated from kmalloc may not enough for all saved GGTT entries. - Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and move the actual implementation to gvt.h/gvt.c. (zhenyu) - Check gvt config on and active with intel_gvt_active(). (zhenyu) V3: (zhenyu) - Incorrect copy length. Should be num entries * entry size. - Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem. - Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM. Colin Xu (2): drm/i915/gvt: Save/restore HW status for GVT during suspend/resume drm/i915/gvt: Add GVT suspend/resume routine to i915 drivers/gpu/drm/i915/gvt/gtt.c | 75 + drivers/gpu/drm/i915/gvt/gtt.h | 2 + drivers/gpu/drm/i915/gvt/gvt.c | 15 ++ drivers/gpu/drm/i915/gvt/gvt.h | 8 +++ drivers/gpu/drm/i915/gvt/handlers.c | 39 +-- drivers/gpu/drm/i915/gvt/mmio.h | 3 ++ drivers/gpu/drm/i915/gvt/vgpu.c | 1 + drivers/gpu/drm/i915/i915_drv.c | 4 ++ drivers/gpu/drm/i915/intel_gvt.c| 29 +++ drivers/gpu/drm/i915/intel_gvt.h| 10 10 files changed, 183 insertions(+), 3 deletions(-) -- 2.28.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 1/2] drm/i915/gvt: Save/restore HW status for GVT during suspend/resume
This patch save/restore necessary GVT info during i915 suspend/resume so that GVT enabled QEMU VM can continue running. Only GGTT and fence regs are saved/restored now. GVT will save GGTT entries into GVT in suspend routine, and restore the saved entries and re-init fence regs in resume routine. V2: - Change kzalloc/kfree to vzalloc/vfree since the space allocated from kmalloc may not enough for all saved GGTT entries. - Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and move the actual implementation to gvt.h/gvt.c. (zhenyu) - Check gvt config on and active with intel_gvt_active(). (zhenyu) V3: (zhenyu) - Incorrect copy length. Should be num entries * entry size. - Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem. - Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM. Signed-off-by: Hang Yuan Signed-off-by: Colin Xu --- drivers/gpu/drm/i915/gvt/gtt.c | 75 + drivers/gpu/drm/i915/gvt/gtt.h | 2 + drivers/gpu/drm/i915/gvt/gvt.c | 15 ++ drivers/gpu/drm/i915/gvt/gvt.h | 8 +++ drivers/gpu/drm/i915/gvt/handlers.c | 39 +-- drivers/gpu/drm/i915/gvt/mmio.h | 3 ++ drivers/gpu/drm/i915/gvt/vgpu.c | 1 + drivers/gpu/drm/i915/intel_gvt.c| 29 +++ drivers/gpu/drm/i915/intel_gvt.h| 10 9 files changed, 179 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c index 04bf018ecc34..817095dd221b 100644 --- a/drivers/gpu/drm/i915/gvt/gtt.c +++ b/drivers/gpu/drm/i915/gvt/gtt.c @@ -2533,6 +2533,11 @@ static void intel_vgpu_destroy_ggtt_mm(struct intel_vgpu *vgpu) } intel_vgpu_destroy_mm(vgpu->gtt.ggtt_mm); vgpu->gtt.ggtt_mm = NULL; + + if (vgpu->ggtt_entries) { + vfree(vgpu->ggtt_entries); + vgpu->ggtt_entries = NULL; + } } /** @@ -2834,3 +2839,73 @@ void intel_vgpu_reset_ggtt(struct intel_vgpu *vgpu, bool invalidate_old) ggtt_invalidate(gvt->gt); } + +/** + * intel_gvt_save_ggtt - save all vGPU's ggtt entries + * @gvt: intel gvt device + * + * This function is called at driver suspend stage to save + * GGTT entries of every active vGPU. + * + */ +void intel_gvt_save_ggtt(struct intel_gvt *gvt) +{ + struct intel_vgpu *vgpu; + int id; + u32 index, num_low, num_hi; + void __iomem *addr; + + for_each_active_vgpu(gvt, vgpu, id) { + num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT; + num_hi = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT; + vgpu->ggtt_entries = vzalloc((num_low + num_hi) * sizeof(u64)); + if (!vgpu->ggtt_entries) + continue; + + index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT; + addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index; + memcpy_fromio(vgpu->ggtt_entries, addr, num_low * sizeof(u64)); + + index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT; + addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index; + memcpy_fromio(vgpu->ggtt_entries + num_low, + addr, num_hi * sizeof(u64)); + } +} + +/** + * intel_gvt_restore_ggtt - restore all vGPU's ggtt entries + * @gvt: intel gvt device + * + * This function is called at driver resume stage to restore + * GGTT entries of every active vGPU. + * + */ +void intel_gvt_restore_ggtt(struct intel_gvt *gvt) +{ + struct intel_vgpu *vgpu; + int id; + u32 index, num_low, num_hi; + void __iomem *addr; + + for_each_active_vgpu(gvt, vgpu, id) { + if (!vgpu->ggtt_entries) { + gvt_vgpu_err("fail to get saved ggtt\n"); + continue; + } + + num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT; + num_hi = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT; + + index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT; + addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index; + memcpy_toio(addr, vgpu->ggtt_entries, num_low * sizeof(u64)); + index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT; + addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index; + memcpy_toio(addr, vgpu->ggtt_entries + num_low, + num_hi * sizeof(u64)); + + vfree(vgpu->ggtt_entries); + vgpu->ggtt_entries = NULL; + } +} diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h index b76a262dd9bc..0d2fb2714852 100644 --- a/drivers/gpu/drm/i915/gvt/gtt.h +++ b/drivers/gpu/drm/i915/gvt/gtt.h @@ -279,5 +279,7 @@ int intel_vgpu_emulate_ggtt_mmio_write(struct intel_vgpu *vgpu, unsigned int off, void *p_data, unsigned int bytes); void intel_vgpu_destroy_all_ppgtt_mm(struct intel_vgpu *vgpu); +void intel_gvt_save_ggtt(struct intel_gvt *gvt
[Intel-gfx] [PATCH v3 2/2] drm/i915/gvt: Add GVT suspend/resume routine to i915
This patch add gvt suspend/resume wrapper into i915: i915_drm_suspend() and i915_drm_resume(). GVT relies on i915 so suspend gvt ahead of other i915 sub-routine and resume gvt at last. V2: - Direct call into gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c. The wrapper and implementation will check and call gvt routine. (zhenyu) V3: Refresh. Signed-off-by: Hang Yuan Signed-off-by: Colin Xu --- drivers/gpu/drm/i915/i915_drv.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index d66fe09d337e..28055dc65ecd 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1090,6 +1090,8 @@ static int i915_drm_suspend(struct drm_device *dev) drm_kms_helper_poll_disable(dev); + intel_gvt_suspend(dev_priv); + pci_save_state(pdev); intel_display_suspend(dev); @@ -1267,6 +1269,8 @@ static int i915_drm_resume(struct drm_device *dev) intel_power_domains_enable(dev_priv); + intel_gvt_resume(dev_priv); + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); return 0; -- 2.28.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] linux-next: manual merge of the drm-intel tree with Linus' tree
Hi Stephen, On 9/8/20 6:00 AM, Stephen Rothwell wrote: Hi all, Today's linux-next merge of the drm-intel tree got a conflict in: drivers/gpu/drm/i915/display/intel_panel.c between commit: f8bd54d21904 ("drm/i915: panel: Use atomic PWM API for devs with an external PWM controller") from Linus' tree and commit: 6b51e7d23aa8 ("drm/i915: panel: Honor the VBT PWM frequency for devs with an external PWM controller") That doesn't sound correct, those are both commits from the drm-intel tree. from the drm-intel tree. I fixed it up (I just used the latter) Just taking the drivers/gpu/drm/i915/display/intel_panel.c contents of: f8bd54d21904 ("drm/i915: panel: Use atomic PWM API for devs with an external PWM controller") Is the right thing to do, the problem is a difference in a line which gets removed in that commit. Regards, Hans ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/3] drm/atomic-helper: Extract drm_atomic_helper_calc_timestamping_constants()
== Series Details == Series: series starting with [1/3] drm/atomic-helper: Extract drm_atomic_helper_calc_timestamping_constants() URL : https://patchwork.freedesktop.org/series/81419/ State : failure == Summary == CI Bug Log - changes from CI_DRM_8971_full -> Patchwork_18448_full Summary --- **FAILURE** Serious unknown changes coming with Patchwork_18448_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_18448_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_18448_full: ### IGT changes ### Possible regressions * igt@perf_pmu@rc6-runtime-pm-long: - shard-hsw: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-hsw1/igt@perf_...@rc6-runtime-pm-long.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-hsw7/igt@perf_...@rc6-runtime-pm-long.html Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * {igt@kms_writeback@writeback-invalid-parameters}: - shard-tglb: NOTRUN -> [SKIP][3] +3 similar issues [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-tglb2/igt@kms_writeb...@writeback-invalid-parameters.html Known issues Here are the changes found in Patchwork_18448_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_reloc@basic-many-active@vecs0: - shard-glk: [PASS][4] -> [FAIL][5] ([i915#2389]) +1 similar issue [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-glk1/igt@gem_exec_reloc@basic-many-act...@vecs0.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-glk7/igt@gem_exec_reloc@basic-many-act...@vecs0.html * igt@gem_exec_whisper@basic-fds-priority-all: - shard-glk: [PASS][6] -> [DMESG-WARN][7] ([i915#118] / [i915#95]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-glk4/igt@gem_exec_whis...@basic-fds-priority-all.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-glk6/igt@gem_exec_whis...@basic-fds-priority-all.html * igt@gem_userptr_blits@sync-unmap-cycles: - shard-skl: [PASS][8] -> [TIMEOUT][9] ([i915#1958] / [i915#2424]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-skl4/igt@gem_userptr_bl...@sync-unmap-cycles.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-skl6/igt@gem_userptr_bl...@sync-unmap-cycles.html * igt@kms_atomic_transition@plane-all-transition-nonblocking-fencing@edp-1-pipe-a: - shard-skl: [PASS][10] -> [DMESG-WARN][11] ([i915#1982]) +3 similar issues [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-skl8/igt@kms_atomic_transition@plane-all-transition-nonblocking-fenc...@edp-1-pipe-a.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-skl5/igt@kms_atomic_transition@plane-all-transition-nonblocking-fenc...@edp-1-pipe-a.html * igt@kms_cursor_crc@pipe-a-cursor-128x42-sliding: - shard-glk: [PASS][12] -> [FAIL][13] ([i915#54]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-glk1/igt@kms_cursor_...@pipe-a-cursor-128x42-sliding.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-glk7/igt@kms_cursor_...@pipe-a-cursor-128x42-sliding.html * igt@kms_cursor_crc@pipe-b-cursor-suspend: - shard-skl: [PASS][14] -> [INCOMPLETE][15] ([i915#300]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-skl3/igt@kms_cursor_...@pipe-b-cursor-suspend.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-skl7/igt@kms_cursor_...@pipe-b-cursor-suspend.html * igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a2: - shard-glk: [PASS][16] -> [FAIL][17] ([i915#79]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-glk1/igt@kms_flip@flip-vs-expired-vblank-interrupti...@a-hdmi-a2.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-glk9/igt@kms_flip@flip-vs-expired-vblank-interrupti...@a-hdmi-a2.html * igt@kms_frontbuffer_tracking@fbc-badstride: - shard-tglb: [PASS][18] -> [DMESG-WARN][19] ([i915#1982]) +1 similar issue [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8971/shard-tglb3/igt@kms_frontbuffer_track...@fbc-badstride.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18448/shard-tglb6/igt@kms_frontbuffer_track...@fbc-badstride.html * igt@kms_psr@psr2_cursor_plane_move: - shard-iclb: [PASS][20] -> [SK
Re: [Intel-gfx] [PATCH 4/5] drm_dp_cec: add plumbing in preparation for MST support
On 01/09/2020 08:22, Sam McNally wrote: > From: Hans Verkuil > > Signed-off-by: Hans Verkuil > [sa...@chromium.org: > - rebased > - removed polling-related changes > - moved the calls to drm_dp_cec_(un)set_edid() into the next patch > ] > Signed-off-by: Sam McNally > --- > > .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +- > drivers/gpu/drm/drm_dp_cec.c | 22 ++- > drivers/gpu/drm/i915/display/intel_dp.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_connector.c | 2 +- > include/drm/drm_dp_helper.h | 6 +++-- > 5 files changed, 19 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > index 461fa4da0a34..6e7075893ec9 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > @@ -419,7 +419,7 @@ void amdgpu_dm_initialize_dp_connector(struct > amdgpu_display_manager *dm, > > drm_dp_aux_init(&aconnector->dm_dp_aux.aux); > drm_dp_cec_register_connector(&aconnector->dm_dp_aux.aux, > - &aconnector->base); > + &aconnector->base, false); > > if (aconnector->base.connector_type == DRM_MODE_CONNECTOR_eDP) > return; > diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c > index 3ab2609f9ec7..04ab7b88055c 100644 > --- a/drivers/gpu/drm/drm_dp_cec.c > +++ b/drivers/gpu/drm/drm_dp_cec.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > > /* > * Unfortunately it turns out that we have a chicken-and-egg situation > @@ -338,8 +339,6 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const > struct edid *edid) > if (aux->cec.adap) { > if (aux->cec.adap->capabilities == cec_caps && > aux->cec.adap->available_log_addrs == num_las) { > - /* Unchanged, so just set the phys addr */ > - cec_s_phys_addr_from_edid(aux->cec.adap, edid); > goto unlock; > } > /* > @@ -364,15 +363,16 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const > struct edid *edid) > if (cec_register_adapter(aux->cec.adap, connector->dev->dev)) { > cec_delete_adapter(aux->cec.adap); > aux->cec.adap = NULL; > - } else { > - /* > - * Update the phys addr for the new CEC adapter. When called > - * from drm_dp_cec_register_connector() edid == NULL, so in > - * that case the phys addr is just invalidated. > - */ > - cec_s_phys_addr_from_edid(aux->cec.adap, edid); > } > unlock: > + /* > + * Update the phys addr for the new CEC adapter. When called > + * from drm_dp_cec_register_connector() edid == NULL, so in > + * that case the phys addr is just invalidated. > + */ The comment is no longer in sync with the code: if EDID == NULL, then nothing is done due to the edid check in the 'if' below. > + if (aux->cec.adap && edid) { I think this should just be: if (aux->cec.adap) Also, the {} aren't necessary here. > + cec_s_phys_addr_from_edid(aux->cec.adap, edid); > + } > mutex_unlock(&aux->cec.lock); > } > EXPORT_SYMBOL(drm_dp_cec_set_edid); Frankly, the changes to this function should be dropped completely, from what I can see they are not necessary. It was done in my original patch because of the way I handled mst, but you did it differently (and I think better), so these changes are no longer needed. I know I am actually commenting on my old patch, but that patch was from a work-in-progress git branch and was never meant as a 'proper' patch. However, what complicates matters is that after digging a bit more I discovered that commit 732300154980 ("drm: Do not call drm_dp_cec_set_edid() while registering DP connectors") changed drm_dp_cec_register_connector() so that it no longer calls drm_dp_cec_set_edid(), but the comments there and in this function were not updated. It would be nice if you can add a patch fixing these outdated comments. Regardless of that change in commit 732300154980, the edid pointer can still be NULL and the existing behavior should be kept (i.e. create a CEC device, but with an invalid physical address since there is no EDID for some reason). Regards, Hans > @@ -418,6 +418,7 @@ EXPORT_SYMBOL(drm_dp_cec_unset_edid); > * drm_dp_cec_register_connector() - register a new connector > * @aux: DisplayPort AUX channel > * @connector: drm connector > + * @is_mst: set to true if this is an MST branch > * > * A new connector was registered with associated CEC adapter name and > * CEC adapter parent device. After registering the name and parent > @@ -425,12 +426,13 @@ EXPORT_SYMBOL(drm_dp_cec_unset_
Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
On 2020/9/8 14:23, Christoph Hellwig wrote: On Tue, Sep 08, 2020 at 02:04:53PM +0800, Lu Baolu wrote: Do you mind telling where can I find Marek's series? [PATCH v10 00/30] DRM: fix struct sg_table nents vs. orig_nents misuse on various lists including the iommu one. Get it. Thank you! Best regards, baolu ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gvt: Introduce per object locking in GVT scheduler.
On 2020.09.07 23:02:03 +0300, Zhi Wang wrote: > To support ww locking and per-object implemented in i915, GVT scheduler needs > to be refined. Most of the changes are located in shadow batch buffer, shadow > wa context in GVT-g, where use quite a lot of i915 gem object APIs. > > Cc: Maarten Lankhorst > Cc: Joonas Lahtinen > Cc: Zhenyu Wang > Signed-off-by: Zhi Wang > --- > drivers/gpu/drm/i915/gvt/scheduler.c | 68 > ++-- > 1 file changed, 57 insertions(+), 11 deletions(-) Looks ok to me. Reviewed-by: Zhenyu Wang Pls verify against latest gvt-staging then create pull against gt-next for 5.10. Thanks > > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c > b/drivers/gpu/drm/i915/gvt/scheduler.c > index 1570eb8..fe7ee10 100644 > --- a/drivers/gpu/drm/i915/gvt/scheduler.c > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c > @@ -396,7 +396,9 @@ static void release_shadow_wa_ctx(struct > intel_shadow_wa_ctx *wa_ctx) > if (!wa_ctx->indirect_ctx.obj) > return; > > + i915_gem_object_lock(wa_ctx->indirect_ctx.obj, NULL); > i915_gem_object_unpin_map(wa_ctx->indirect_ctx.obj); > + i915_gem_object_unlock(wa_ctx->indirect_ctx.obj); > i915_gem_object_put(wa_ctx->indirect_ctx.obj); > > wa_ctx->indirect_ctx.obj = NULL; > @@ -504,6 +506,7 @@ static int prepare_shadow_batch_buffer(struct > intel_vgpu_workload *workload) > struct intel_gvt *gvt = workload->vgpu->gvt; > const int gmadr_bytes = gvt->device_info.gmadr_bytes_in_cmd; > struct intel_vgpu_shadow_bb *bb; > + struct i915_gem_ww_ctx ww; > int ret; > > list_for_each_entry(bb, &workload->shadow_bb, list) { > @@ -528,10 +531,19 @@ static int prepare_shadow_batch_buffer(struct > intel_vgpu_workload *workload) >* directly >*/ > if (!bb->ppgtt) { > - bb->vma = i915_gem_object_ggtt_pin(bb->obj, > -NULL, 0, 0, 0); > + i915_gem_ww_ctx_init(&ww, false); > +retry: > + i915_gem_object_lock(bb->obj, &ww); > + > + bb->vma = i915_gem_object_ggtt_pin_ww(bb->obj, &ww, > + NULL, 0, 0, 0); > if (IS_ERR(bb->vma)) { > ret = PTR_ERR(bb->vma); > + if (ret == -EDEADLK) { > + ret = i915_gem_ww_ctx_backoff(&ww); > + if (!ret) > + goto retry; > + } > goto err; > } > > @@ -545,13 +557,18 @@ static int prepare_shadow_batch_buffer(struct > intel_vgpu_workload *workload) > 0); > if (ret) > goto err; > + > + /* No one is going to touch shadow bb from now on. */ > + i915_gem_object_flush_map(bb->obj); > + > + i915_gem_object_unlock(bb->obj); > + i915_gem_ww_ctx_fini(&ww); > } > > - /* No one is going to touch shadow bb from now on. */ > - i915_gem_object_flush_map(bb->obj); > } > return 0; > err: > + i915_gem_ww_ctx_fini(&ww); > release_shadow_batch_buffer(workload); > return ret; > } > @@ -578,14 +595,30 @@ static int prepare_shadow_wa_ctx(struct > intel_shadow_wa_ctx *wa_ctx) > unsigned char *per_ctx_va = > (unsigned char *)wa_ctx->indirect_ctx.shadow_va + > wa_ctx->indirect_ctx.size; > + struct i915_gem_ww_ctx ww; > + int ret; > > if (wa_ctx->indirect_ctx.size == 0) > return 0; > > - vma = i915_gem_object_ggtt_pin(wa_ctx->indirect_ctx.obj, NULL, > -0, CACHELINE_BYTES, 0); > - if (IS_ERR(vma)) > - return PTR_ERR(vma); > + i915_gem_ww_ctx_init(&ww, false); > +retry: > + i915_gem_object_lock(wa_ctx->indirect_ctx.obj, &ww); > + > + vma = i915_gem_object_ggtt_pin_ww(wa_ctx->indirect_ctx.obj, &ww, NULL, > + 0, CACHELINE_BYTES, 0); > + if (IS_ERR(vma)) { > + ret = PTR_ERR(vma); > + if (ret == -EDEADLK) { > + ret = i915_gem_ww_ctx_backoff(&ww); > + if (!ret) > + goto retry; > + } > + return ret; > + } > + > + i915_gem_object_unlock(wa_ctx->indirect_ctx.obj); > + i915_gem_ww_ctx_fini(&ww); > > /* FIXME: we are not tracking our pinned VMA leaving it >* up to the core to fix up the stray pin_count upon > @@ -619,12 +652,14 @@ static void release_shadow_batch_buffer(struct > intel_vgpu_workload *workload) >
Re: [Intel-gfx] linux-next: manual merge of the drm-intel tree with Linus' tree
Hi Hans, On Tue, 8 Sep 2020 10:22:06 +0200 Hans de Goede wrote: > > On 9/8/20 6:00 AM, Stephen Rothwell wrote: > > > > Today's linux-next merge of the drm-intel tree got a conflict in: > > > >drivers/gpu/drm/i915/display/intel_panel.c > > > > between commit: > > > >f8bd54d21904 ("drm/i915: panel: Use atomic PWM API for devs with an > > external PWM controller") This should have been 899c537c25f9 ("drm/i915: Use 64-bit division macro") > > > > from Linus' tree and commit: > > > >6b51e7d23aa8 ("drm/i915: panel: Honor the VBT PWM frequency for devs > > with an external PWM controller") > > That doesn't sound correct, those are both commits from the drm-intel tree. > > > from the drm-intel tree. > > > > I fixed it up (I just used the latter) > > Just taking the drivers/gpu/drm/i915/display/intel_panel.c contents of: > > f8bd54d21904 ("drm/i915: panel: Use atomic PWM API for devs with an external > PWM controller") > > Is the right thing to do, the problem is a difference in a line which gets > removed in that commit. Which is what I actually did, I guess :-) Sorry about that. -- Cheers, Stephen Rothwell pgpLDwW1KT85B.pgp Description: OpenPGP digital signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Nuke dpio_phy_iosf_port[]
== Series Details == Series: drm/i915: Nuke dpio_phy_iosf_port[] URL : https://patchwork.freedesktop.org/series/81431/ State : success == Summary == CI Bug Log - changes from CI_DRM_8973_full -> Patchwork_18449_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_18449_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_eio@kms: - shard-snb: [PASS][1] -> [DMESG-WARN][2] ([i915#1982]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-snb2/igt@gem_...@kms.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-snb4/igt@gem_...@kms.html * igt@gem_exec_reloc@basic-many-active@rcs0: - shard-glk: [PASS][3] -> [FAIL][4] ([i915#2389]) +1 similar issue [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-glk5/igt@gem_exec_reloc@basic-many-act...@rcs0.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-glk2/igt@gem_exec_reloc@basic-many-act...@rcs0.html * igt@i915_selftest@mock@contexts: - shard-apl: [PASS][5] -> [INCOMPLETE][6] ([i915#1635] / [i915#2278]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-apl4/igt@i915_selftest@m...@contexts.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-apl1/igt@i915_selftest@m...@contexts.html * igt@kms_atomic_transition@plane-all-transition-nonblocking-fencing@edp-1-pipe-a: - shard-skl: [PASS][7] -> [DMESG-WARN][8] ([i915#1982]) +10 similar issues [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-skl4/igt@kms_atomic_transition@plane-all-transition-nonblocking-fenc...@edp-1-pipe-a.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-skl9/igt@kms_atomic_transition@plane-all-transition-nonblocking-fenc...@edp-1-pipe-a.html * igt@kms_big_fb@linear-64bpp-rotate-0: - shard-glk: [PASS][9] -> [DMESG-WARN][10] ([i915#1982]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-glk6/igt@kms_big...@linear-64bpp-rotate-0.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-glk6/igt@kms_big...@linear-64bpp-rotate-0.html * igt@kms_flip@2x-flip-vs-expired-vblank@bc-hdmi-a1-hdmi-a2: - shard-glk: [PASS][11] -> [FAIL][12] ([i915#79]) +1 similar issue [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-glk1/igt@kms_flip@2x-flip-vs-expired-vbl...@bc-hdmi-a1-hdmi-a2.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-glk1/igt@kms_flip@2x-flip-vs-expired-vbl...@bc-hdmi-a1-hdmi-a2.html * igt@kms_flip@flip-vs-expired-vblank@c-dp1: - shard-kbl: [PASS][13] -> [FAIL][14] ([i915#79]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-kbl7/igt@kms_flip@flip-vs-expired-vbl...@c-dp1.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-kbl7/igt@kms_flip@flip-vs-expired-vbl...@c-dp1.html * igt@kms_flip@flip-vs-suspend-interruptible@c-edp1: - shard-skl: [PASS][15] -> [INCOMPLETE][16] ([i915#198]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-skl10/igt@kms_flip@flip-vs-suspend-interrupti...@c-edp1.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-skl5/igt@kms_flip@flip-vs-suspend-interrupti...@c-edp1.html * igt@kms_flip@flip-vs-suspend@b-dp1: - shard-kbl: [PASS][17] -> [DMESG-WARN][18] ([i915#180]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-kbl1/igt@kms_flip@flip-vs-susp...@b-dp1.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-kbl6/igt@kms_flip@flip-vs-susp...@b-dp1.html * igt@kms_flip@plain-flip-ts-check-interruptible@b-edp1: - shard-skl: [PASS][19] -> [FAIL][20] ([i915#2122]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-skl1/igt@kms_flip@plain-flip-ts-check-interrupti...@b-edp1.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-skl2/igt@kms_flip@plain-flip-ts-check-interrupti...@b-edp1.html * igt@kms_frontbuffer_tracking@fbc-farfromfence: - shard-kbl: [PASS][21] -> [DMESG-WARN][22] ([i915#1982]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-kbl1/igt@kms_frontbuffer_track...@fbc-farfromfence.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/shard-kbl6/igt@kms_frontbuffer_track...@fbc-farfromfence.html * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-mmap-cpu: - shard-tglb: [PASS][23] -> [DMESG-WARN][24] ([i915#1982]) +3 similar issues [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-tglb5/igt@kms_frontbuffer_track...@psr-1p-offscren-pri-shrfb-draw-mmap-cpu.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18449/s
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Drop the drm_atomic_helper_calc_timestamping_constants() call
On Mon, Sep 07, 2020 at 08:14:38PM +0200, Daniel Vetter wrote: > On Mon, Sep 07, 2020 at 03:00:26PM +0300, Ville Syrjala wrote: > > From: Ville Syrjälä > > > > We update the timestamping constants per-crtc explicitly in > > intel_crtc_update_active_timings(). Furtermore the helper will > > use uapi.adjusted_mode whereas we want hw.adjusted_mode. Thus > > let's drop the helper call an rely on what we already have in > > intel_crtc_update_active_timings(). We can now also drop the > > hw.adjusted_mode -> uapi.adjusted_mode copy hack that was added > > to keep the helper from deriving the timestamping constants from > > the wrong thing. > > > > Signed-off-by: Ville Syrjälä > > Does this fix some CI fail? I'd kinda expect/hope for that ... Nah. Just trying to get rid of some of the confusing stuff before we add even more confusing stuff for bigjoiner. > > Anyway looks like a good idea to not mess with the uapi state like this. > > Reviewed-by: Daniel Vetter > > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 7 --- > > 1 file changed, 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 035840ce3825..a846f414c759 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -13472,12 +13472,6 @@ intel_modeset_pipe_config(struct intel_crtc_state > > *pipe_config) > > "hw max bpp: %i, pipe bpp: %i, dithering: %i\n", > > base_bpp, pipe_config->pipe_bpp, pipe_config->dither); > > > > - /* > > -* Make drm_calc_timestamping_constants in > > -* drm_atomic_helper_update_legacy_modeset_state() happy > > -*/ > > - pipe_config->uapi.adjusted_mode = pipe_config->hw.adjusted_mode; > > - > > return 0; > > } > > > > @@ -15578,7 +15572,6 @@ static void intel_atomic_commit_tail(struct > > intel_atomic_state *state) > > > > if (state->modeset) { > > drm_atomic_helper_update_legacy_modeset_state(dev, > > &state->base); > > - drm_atomic_helper_calc_timestamping_constants(&state->base); > > > > intel_set_cdclk_pre_plane_update(state); > > > > -- > > 2.26.2 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/gvt: Introduce per object locking in GVT scheduler.
== Series Details == Series: drm/i915/gvt: Introduce per object locking in GVT scheduler. URL : https://patchwork.freedesktop.org/series/81434/ State : success == Summary == CI Bug Log - changes from CI_DRM_8973_full -> Patchwork_18450_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_18450_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_ctx_isolation@preservation-s3@vcs0: - shard-kbl: [PASS][1] -> [DMESG-WARN][2] ([i915#180]) +1 similar issue [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-kbl6/igt@gem_ctx_isolation@preservation...@vcs0.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-kbl3/igt@gem_ctx_isolation@preservation...@vcs0.html * igt@gem_exec_reloc@basic-many-active@rcs0: - shard-glk: [PASS][3] -> [FAIL][4] ([i915#2389]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-glk5/igt@gem_exec_reloc@basic-many-act...@rcs0.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-glk2/igt@gem_exec_reloc@basic-many-act...@rcs0.html * igt@gem_exec_whisper@basic-queues-forked-all: - shard-glk: [PASS][5] -> [DMESG-WARN][6] ([i915#118] / [i915#95]) +1 similar issue [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-glk5/igt@gem_exec_whis...@basic-queues-forked-all.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-glk2/igt@gem_exec_whis...@basic-queues-forked-all.html * igt@gem_huc_copy@huc-copy: - shard-tglb: [PASS][7] -> [SKIP][8] ([i915#2190]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-tglb1/igt@gem_huc_c...@huc-copy.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-tglb6/igt@gem_huc_c...@huc-copy.html * igt@gem_userptr_blits@sync-unmap-cycles: - shard-skl: [PASS][9] -> [TIMEOUT][10] ([i915#1958] / [i915#2424]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-skl1/igt@gem_userptr_bl...@sync-unmap-cycles.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-skl4/igt@gem_userptr_bl...@sync-unmap-cycles.html * igt@kms_atomic_transition@plane-all-transition-nonblocking-fencing@edp-1-pipe-a: - shard-skl: [PASS][11] -> [DMESG-WARN][12] ([i915#1982]) +6 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-skl4/igt@kms_atomic_transition@plane-all-transition-nonblocking-fenc...@edp-1-pipe-a.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-skl2/igt@kms_atomic_transition@plane-all-transition-nonblocking-fenc...@edp-1-pipe-a.html * igt@kms_flip@flip-vs-suspend@a-edp1: - shard-skl: [PASS][13] -> [INCOMPLETE][14] ([i915#198]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-skl8/igt@kms_flip@flip-vs-susp...@a-edp1.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-skl6/igt@kms_flip@flip-vs-susp...@a-edp1.html * igt@kms_flip@flip-vs-suspend@c-hdmi-a1: - shard-hsw: [PASS][15] -> [INCOMPLETE][16] ([i915#2055]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-hsw5/igt@kms_flip@flip-vs-susp...@c-hdmi-a1.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-hsw1/igt@kms_flip@flip-vs-susp...@c-hdmi-a1.html * igt@kms_flip@plain-flip-fb-recreate-interruptible@c-edp1: - shard-skl: [PASS][17] -> [FAIL][18] ([i915#2122]) +1 similar issue [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-skl7/igt@kms_flip@plain-flip-fb-recreate-interrupti...@c-edp1.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-skl5/igt@kms_flip@plain-flip-fb-recreate-interrupti...@c-edp1.html * igt@kms_flip_tiling@flip-changes-tiling-y: - shard-skl: [PASS][19] -> [FAIL][20] ([i915#699]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-skl9/igt@kms_flip_til...@flip-changes-tiling-y.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-skl7/igt@kms_flip_til...@flip-changes-tiling-y.html * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-msflip-blt: - shard-iclb: [PASS][21] -> [DMESG-WARN][22] ([i915#1982]) +1 similar issue [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-iclb3/igt@kms_frontbuffer_track...@fbc-1p-primscrn-shrfb-msflip-blt.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18450/shard-iclb3/igt@kms_frontbuffer_track...@fbc-1p-primscrn-shrfb-msflip-blt.html * igt@kms_frontbuffer_tracking@fbc-farfromfence: - shard-kbl: [PASS][23] -> [DMESG-WARN][24] ([i915#1982]) +1 similar issue [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8973/shard-kbl1/igt@kms_frontbuffer_track...@fbc
[Intel-gfx] Patch "drm/i915: Fix sha_text population code" has been added to the 5.8-stable tree
This is a note to let you know that I've just added the patch titled drm/i915: Fix sha_text population code to the 5.8-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: drm-i915-fix-sha_text-population-code.patch and it can be found in the queue-5.8 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let know about it. >From 9ab57658a608f879469ffa22b723c4539c05a58f Mon Sep 17 00:00:00 2001 From: Sean Paul Date: Tue, 18 Aug 2020 11:38:49 -0400 Subject: drm/i915: Fix sha_text population code From: Sean Paul commit 9ab57658a608f879469ffa22b723c4539c05a58f upstream. This patch fixes a few bugs: 1- We weren't taking into account sha_leftovers when adding multiple ksvs to sha_text. As such, we were or'ing the end of ksv[j - 1] with the beginning of ksv[j] 2- In the sha_leftovers == 2 and sha_leftovers == 3 case, bstatus was being placed on the wrong half of sha_text, overlapping the leftover ksv value 3- In the sha_leftovers == 2 case, we need to manually terminate the byte stream with 0x80 since the hardware doesn't have enough room to add it after writing M0 The upside is that all of the HDCP supported HDMI repeaters I could find on Amazon just strip HDCP anyways, so it turns out to be _really_ hard to hit any of these cases without an MST hub, which is not (yet) supported. Oh, and the sha_leftovers == 1 case works perfectly! Fixes: ee5e5e7a5e0f ("drm/i915: Add HDCP framework + base implementation") Cc: Chris Wilson Cc: Ramalingam C Cc: Daniel Vetter Cc: Sean Paul Cc: Jani Nikula Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: intel-gfx@lists.freedesktop.org Cc: # v4.17+ Reviewed-by: Ramalingam C Signed-off-by: Sean Paul Signed-off-by: Ramalingam C Link: https://patchwork.freedesktop.org/patch/msgid/20200818153910.27894-2-s...@poorly.run (cherry picked from commit 1f0882214fd0037b74f245d9be75c31516fed040) Signed-off-by: Jani Nikula Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/i915/display/intel_hdcp.c | 26 -- include/drm/drm_hdcp.h|3 +++ 2 files changed, 23 insertions(+), 6 deletions(-) --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -336,8 +336,10 @@ int intel_hdcp_validate_v_prime(struct i /* Fill up the empty slots in sha_text and write it out */ sha_empty = sizeof(sha_text) - sha_leftovers; - for (j = 0; j < sha_empty; j++) - sha_text |= ksv[j] << ((sizeof(sha_text) - j - 1) * 8); + for (j = 0; j < sha_empty; j++) { + u8 off = ((sizeof(sha_text) - j - 1 - sha_leftovers) * 8); + sha_text |= ksv[j] << off; + } ret = intel_write_sha_text(dev_priv, sha_text); if (ret < 0) @@ -435,7 +437,7 @@ int intel_hdcp_validate_v_prime(struct i /* Write 32 bits of text */ intel_de_write(dev_priv, HDCP_REP_CTL, rep_ctl | HDCP_SHA1_TEXT_32); - sha_text |= bstatus[0] << 24 | bstatus[1] << 16; + sha_text |= bstatus[0] << 8 | bstatus[1]; ret = intel_write_sha_text(dev_priv, sha_text); if (ret < 0) return ret; @@ -450,17 +452,29 @@ int intel_hdcp_validate_v_prime(struct i return ret; sha_idx += sizeof(sha_text); } + + /* +* Terminate the SHA-1 stream by hand. For the other leftover +* cases this is appended by the hardware. +*/ + intel_de_write(dev_priv, HDCP_REP_CTL, + rep_ctl | HDCP_SHA1_TEXT_32); + sha_text = DRM_HDCP_SHA1_TERMINATOR << 24; + ret = intel_write_sha_text(dev_priv, sha_text); + if (ret < 0) + return ret; + sha_idx += sizeof(sha_text); } else if (sha_leftovers == 3) { - /* Write 32 bits of text */ + /* Write 32 bits of text (filled from LSB) */ intel_de_write(dev_priv, HDCP_REP_CTL, rep_ctl | HDCP_SHA1_TEXT_32); - sha_text |= bstatus[0] << 24; + sha_text |= bstatus[0]; ret = intel_write_sha_text(dev_priv, sha_text); if (ret < 0) return ret; sha_idx += sizeof(sha_text); - /* Write 8 bits of text, 24 bits of M0 */ + /* Write 8 bits of text (filled from LSB), 24 bits of M0 */ intel_de_write(dev_priv, HDCP_REP_CTL, rep_ctl | HDCP_SHA1_TEXT_8); ret = intel_write_sha_text(dev_
[Intel-gfx] ✗ Fi.CI.BUILD: failure for Patch "drm/i915: Fix sha_text population code" has been added to the 5.8-stable tree
== Series Details == Series: Patch "drm/i915: Fix sha_text population code" has been added to the 5.8-stable tree URL : https://patchwork.freedesktop.org/series/81458/ State : failure == Summary == Patch is empty. When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] linux-next: manual merge of the drm-intel tree with Linus' tree
Hi, On 9/8/20 1:04 PM, Stephen Rothwell wrote: Hi Hans, On Tue, 8 Sep 2020 10:22:06 +0200 Hans de Goede wrote: On 9/8/20 6:00 AM, Stephen Rothwell wrote: Today's linux-next merge of the drm-intel tree got a conflict in: drivers/gpu/drm/i915/display/intel_panel.c between commit: f8bd54d21904 ("drm/i915: panel: Use atomic PWM API for devs with an external PWM controller") This should have been 899c537c25f9 ("drm/i915: Use 64-bit division macro") Yes that makes more sense. from Linus' tree and commit: 6b51e7d23aa8 ("drm/i915: panel: Honor the VBT PWM frequency for devs with an external PWM controller") That doesn't sound correct, those are both commits from the drm-intel tree. from the drm-intel tree. I fixed it up (I just used the latter) Just taking the drivers/gpu/drm/i915/display/intel_panel.c contents of: f8bd54d21904 ("drm/i915: panel: Use atomic PWM API for devs with an external PWM controller") Is the right thing to do, the problem is a difference in a line which gets removed in that commit. Which is what I actually did, I guess :-) Yes, looks good. Sorry about that. No problem and thank you for all the work you do on -next. Regards, Hans ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915: Kill unused savePCH_PORT_HOTPLUG
From: Ville Syrjälä We don't save/restore PCH_PORT_HOTPLUG so no point in reseving space for the value. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_drv.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a455752221cc..2e4438e8e3eb 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -543,7 +543,6 @@ struct i915_suspend_saved_registers { u32 saveSWF0[16]; u32 saveSWF1[16]; u32 saveSWF3[3]; - u32 savePCH_PORT_HOTPLUG; u16 saveGCDGMBUS; }; -- 2.26.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: Nuke the magic FBC_CONTROL save/restore
From: Ville Syrjälä The FBC_CONTROL save restore is there just to preserve the compression interval setting. Since commit a68ce21ba0c4 ("drm/i915/fbc: Store the fbc1 compression interval in the params") we've been explicitly setting the interval to a specific value, so the sace/restore is now entirely pointless. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/i915_suspend.c | 8 2 files changed, 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 2e4438e8e3eb..3917bb1a6157 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -537,7 +537,6 @@ struct intel_gmbus { struct i915_suspend_saved_registers { u32 saveDSPARB; - u32 saveFBC_CONTROL; u32 saveCACHE_MODE_0; u32 saveMI_ARB_STATE; u32 saveSWF0[16]; diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c index ed2be3489f8e..592c230e6914 100644 --- a/drivers/gpu/drm/i915/i915_suspend.c +++ b/drivers/gpu/drm/i915/i915_suspend.c @@ -37,10 +37,6 @@ static void i915_save_display(struct drm_i915_private *dev_priv) /* Display arbitration control */ if (INTEL_GEN(dev_priv) <= 4) dev_priv->regfile.saveDSPARB = I915_READ(DSPARB); - - /* save FBC interval */ - if (HAS_FBC(dev_priv) && INTEL_GEN(dev_priv) <= 4 && !IS_G4X(dev_priv)) - dev_priv->regfile.saveFBC_CONTROL = I915_READ(FBC_CONTROL); } static void i915_restore_display(struct drm_i915_private *dev_priv) @@ -52,10 +48,6 @@ static void i915_restore_display(struct drm_i915_private *dev_priv) /* only restore FBC info on the platform that supports FBC*/ intel_fbc_global_disable(dev_priv); - /* restore FBC interval */ - if (HAS_FBC(dev_priv) && INTEL_GEN(dev_priv) <= 4 && !IS_G4X(dev_priv)) - I915_WRITE(FBC_CONTROL, dev_priv->regfile.saveFBC_CONTROL); - intel_vga_redisable(dev_priv); } -- 2.26.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: Nuke MI_ARB_STATE save/restore
From: Ville Syrjälä Originally added in commit 1f84e550a870 ("drm/i915 more registers for S3 (DSPCLK_GATE_D, CACHE_MODE_0, MI_ARB_STATE)") to fix some underruns. I suspect that was due to the trickle feed settings getting clobbered during suspend. We've been disabling trickle feed explicitly since commit 20f949670f51 ("drm/i915: Disable trickle feed via MI_ARB_STATE for the gen4") so this magic save/restore should no longer be needed. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/i915_suspend.c | 6 -- 2 files changed, 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3917bb1a6157..cf51246b5402 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -538,7 +538,6 @@ struct intel_gmbus { struct i915_suspend_saved_registers { u32 saveDSPARB; u32 saveCACHE_MODE_0; - u32 saveMI_ARB_STATE; u32 saveSWF0[16]; u32 saveSWF1[16]; u32 saveSWF3[3]; diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c index 592c230e6914..34c7d7bccec5 100644 --- a/drivers/gpu/drm/i915/i915_suspend.c +++ b/drivers/gpu/drm/i915/i915_suspend.c @@ -66,9 +66,6 @@ int i915_save_state(struct drm_i915_private *dev_priv) if (INTEL_GEN(dev_priv) < 7) dev_priv->regfile.saveCACHE_MODE_0 = I915_READ(CACHE_MODE_0); - /* Memory Arbitration state */ - dev_priv->regfile.saveMI_ARB_STATE = I915_READ(MI_ARB_STATE); - /* Scratch space */ if (IS_GEN(dev_priv, 2) && IS_MOBILE(dev_priv)) { for (i = 0; i < 7; i++) { @@ -107,9 +104,6 @@ int i915_restore_state(struct drm_i915_private *dev_priv) I915_WRITE(CACHE_MODE_0, dev_priv->regfile.saveCACHE_MODE_0 | 0x); - /* Memory arbitration state */ - I915_WRITE(MI_ARB_STATE, dev_priv->regfile.saveMI_ARB_STATE | 0x); - /* Scratch space */ if (IS_GEN(dev_priv, 2) && IS_MOBILE(dev_priv)) { for (i = 0; i < 7; i++) { -- 2.26.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: Nuke CACHE_MODE_0 save/restore
From: Ville Syrjälä The CACHE_MODE_0 save/restore was added without explanation in commit 1f84e550a870 ("drm/i915 more registers for S3 (DSPCLK_GATE_D, CACHE_MODE_0, MI_ARB_STATE)"). If there are any bits we care about those should be set explicitly during some appropriate init function. Let's assume it's all good and just nuke this magic save/restore. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/i915_suspend.c | 9 - 2 files changed, 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index cf51246b5402..537c780af858 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -537,7 +537,6 @@ struct intel_gmbus { struct i915_suspend_saved_registers { u32 saveDSPARB; - u32 saveCACHE_MODE_0; u32 saveSWF0[16]; u32 saveSWF1[16]; u32 saveSWF3[3]; diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c index 34c7d7bccec5..932c7df64dd3 100644 --- a/drivers/gpu/drm/i915/i915_suspend.c +++ b/drivers/gpu/drm/i915/i915_suspend.c @@ -62,10 +62,6 @@ int i915_save_state(struct drm_i915_private *dev_priv) pci_read_config_word(pdev, GCDGMBUS, &dev_priv->regfile.saveGCDGMBUS); - /* Cache mode state */ - if (INTEL_GEN(dev_priv) < 7) - dev_priv->regfile.saveCACHE_MODE_0 = I915_READ(CACHE_MODE_0); - /* Scratch space */ if (IS_GEN(dev_priv, 2) && IS_MOBILE(dev_priv)) { for (i = 0; i < 7; i++) { @@ -99,11 +95,6 @@ int i915_restore_state(struct drm_i915_private *dev_priv) dev_priv->regfile.saveGCDGMBUS); i915_restore_display(dev_priv); - /* Cache mode state */ - if (INTEL_GEN(dev_priv) < 7) - I915_WRITE(CACHE_MODE_0, dev_priv->regfile.saveCACHE_MODE_0 | - 0x); - /* Scratch space */ if (IS_GEN(dev_priv, 2) && IS_MOBILE(dev_priv)) { for (i = 0; i < 7; i++) { -- 2.26.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/managed: Cleanup of unused functions and polishing docs
On 2020-09-03 10:26 a.m., Daniel Vetter wrote: > On Wed, Sep 02, 2020 at 09:26:27AM +0200, Daniel Vetter wrote: >> Following functions are only used internally, not by drivers: >> - devm_drm_dev_init >> >> Also, now that we have a very slick and polished way to allocate a >> drm_device with devm_drm_dev_alloc, update all the docs to reflect the >> new reality. Mostly this consists of deleting old and misleading >> hints. Two main ones: >> >> - it is no longer required that the drm_device base class is first in >> the structure. devm_drm_dev_alloc can cope with it being anywhere >> >> - obviously embedded now strongly recommends using devm_drm_dev_alloc >> >> v2: Fix typos (Noralf) >> >> v3: Split out the removal of drm_dev_init, that's blocked on some >> discussions on how to convert vgem/vkms/i915-selftests. Adjust commit >> message to reflect that. >> >> Cc: Noralf Trønnes >> Acked-by: Noralf Trønnes (v2) >> Acked-by: Sam Ravnborg >> Cc: Luben Tuikov >> Cc: amd-...@lists.freedesktop.org >> Signed-off-by: Daniel Vetter > > Ok pushed that now to drm-misc-next. I'm also working on getting the > remaining bits of the basic drm managed conversion resubmitted. That was > unfortunately massively sidelined for the dma-fence discussions. > > Quick heads-up: > drmm_add_final_kfree and drm_dev_init will both disappear, please use > devm_drm_dev_alloc. Hi Daniel, In drm_drv.c, in the "DOC: driver instance overview" section, it would help a lot, if you could add/summarize/clarify, succinctly, the two paths, device instantiation: devm_drm_dev_alloc(); ... drm_dev_init(); ... drm_dev_register(); ... And, device destruction, and where/how the "release" method of drm_driver plays out. The allocation part is mostly there, that's good, but the release/destruction part is not. That is, the platform driver callbacks are there, but not the DRM driver part. In this patch, there is no mention of drm_dev_init(), and the documentation update of this patch doesn't mention it, while it is being used by at least one driver. If, on the other hand, you're thinking of removing the "release" callback, a lucid explanation on kref reaching 0--what should be done by DRM and what should be done by drivers, would be very nice and helpful to low-level DRM device driver maintainers/writers. Also, consider renaming "drm_add_action()" to something qualifying the action: either a bitmask as an argument, or right in the name, "drm_add_action_release()", else one begs the question "What action?" It would be helpful, to describe the order of "release" actions on kref reaching 0, and what is expected of DRM and what of drivers, in the order of the expected callbacks. Regards, Luben > > Cheers, Daniel > >> --- >> .../driver-api/driver-model/devres.rst| 2 +- >> drivers/gpu/drm/drm_drv.c | 78 +-- >> drivers/gpu/drm/drm_managed.c | 2 +- >> include/drm/drm_device.h | 2 +- >> include/drm/drm_drv.h | 16 ++-- >> 5 files changed, 30 insertions(+), 70 deletions(-) >> >> diff --git a/Documentation/driver-api/driver-model/devres.rst >> b/Documentation/driver-api/driver-model/devres.rst >> index efc21134..aa4d2420f79e 100644 >> --- a/Documentation/driver-api/driver-model/devres.rst >> +++ b/Documentation/driver-api/driver-model/devres.rst >> @@ -263,7 +263,7 @@ DMA >>dmam_pool_destroy() >> >> DRM >> - devm_drm_dev_init() >> + devm_drm_dev_alloc() >> >> GPIO >>devm_gpiod_get() >> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c >> index d4506f7a234e..7c1689842ec0 100644 >> --- a/drivers/gpu/drm/drm_drv.c >> +++ b/drivers/gpu/drm/drm_drv.c >> @@ -240,13 +240,13 @@ void drm_minor_release(struct drm_minor *minor) >> * DOC: driver instance overview >> * >> * A device instance for a drm driver is represented by &struct drm_device. >> This >> - * is initialized with drm_dev_init(), usually from bus-specific ->probe() >> - * callbacks implemented by the driver. The driver then needs to initialize >> all >> - * the various subsystems for the drm device like memory management, vblank >> - * handling, modesetting support and intial output configuration plus >> obviously >> - * initialize all the corresponding hardware bits. Finally when everything >> is up >> - * and running and ready for userspace the device instance can be published >> - * using drm_dev_register(). >> + * is allocated and initialized with devm_drm_dev_alloc(), usually from >> + * bus-specific ->probe() callbacks implemented by the driver. The driver >> then >> + * needs to initialize all the various subsystems for the drm device like >> memory >> + * management, vblank handling, modesetting support and initial output >> + * configuration plus obviously initialize all the corresponding hardware >> bits. >> + * Finally when everything is up and running and ready for userspace the >> device >> + * i
Re: [Intel-gfx] [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api
On Fri, 28 Aug 2020 at 00:34, Tom Murphy wrote: > > On Thu, 27 Aug 2020 at 22:36, Logan Gunthorpe wrote: > > > > > > > > On 2020-08-23 6:04 p.m., Tom Murphy wrote: > > > I have added a check for the sg_dma_len == 0 : > > > """ > > > } __sgt_iter(struct scatterlist *sgl, bool dma) { > > > struct sgt_iter s = { .sgp = sgl }; > > > > > > + if (sgl && sg_dma_len(sgl) == 0) > > > + s.sgp = NULL; > > > > > > if (s.sgp) { > > > . > > > """ > > > at location [1]. > > > but it doens't fix the problem. > > > > Based on my read of the code, it looks like we also need to change usage > > of sgl->length... Something like the rough patch below, maybe? > > > > Also, Tom, do you have an updated version of the patchset to convert the > > Intel IOMMU to dma-iommu available? The last one I've found doesn't > > apply cleanly (I'm assuming parts of it have been merged in slightly > > modified forms). > > > > I'll try and post one in the next 24hours I have just posted this now: The subject of the cover letter is: "[PATCH V2 0/5] Convert the intel iommu driver to the dma-iommu api" > > > Thanks, > > > > Logan > > > > -- > > > > diff --git a/drivers/gpu/drm/i915/i915_scatterlist.h > > b/drivers/gpu/drm/i915/i915 > > index b7b59328cb76..9367ac801f0c 100644 > > --- a/drivers/gpu/drm/i915/i915_scatterlist.h > > +++ b/drivers/gpu/drm/i915/i915_scatterlist.h > > @@ -27,13 +27,19 @@ static __always_inline struct sgt_iter { > > } __sgt_iter(struct scatterlist *sgl, bool dma) { > > struct sgt_iter s = { .sgp = sgl }; > > > > + if (sgl && !sg_dma_len(s.sgp)) > > + s.sgp = NULL; > > + > > if (s.sgp) { > > s.max = s.curr = s.sgp->offset; > > - s.max += s.sgp->length; > > - if (dma) > > + > > + if (dma) { > > + s.max += sg_dma_len(s.sgp); > > s.dma = sg_dma_address(s.sgp); > > - else > > + } else { > > + s.max += s.sgp->length; > > s.pfn = page_to_pfn(sg_page(s.sgp)); > > + } > > } > > > > return s; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
On Thu, Sep 03, 2020 at 09:18:37PM +0100, Tom Murphy wrote: > Disable combining sg segments in the dma-iommu api. > Combining the sg segments exposes a bug in the intel i915 driver which > causes visual artifacts and the screen to freeze. This is most likely > because of how the i915 handles the returned list. It probably doesn't > respect the returned value specifying the number of elements in the list > and instead depends on the previous behaviour of the intel iommu driver > which would return the same number of elements in the output list as in > the input list. So what is the state of addressing this properly in i915? IF we can't get it done ASAP I wonder if we need a runtime quirk to disable merging instead of blocking this conversion.. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
On Tue, Sep 08, 2020 at 06:36:19AM +0100, Christoph Hellwig wrote: > On Mon, Sep 07, 2020 at 09:18:50PM +0100, Tom Murphy wrote: > > Yeah we talked about passing an attr to map_sg to disable merging at > > the following microconfernce: > > https://linuxplumbersconf.org/event/7/contributions/846/ > > As far as I can remember everyone seemed happy with that solution. I > > won't be working on this though as I don't have any more time to > > dedicate to this. It seems Lu Baolu will take over this. > > I'm absolutely again passing a flag. Tha just invites further > abuse. We need a PCI ID based quirk or something else that can't > be as easily abused. Also, I looked at i915 and there are just three dma_map_sg callers. The two dmabuf related ones are fixed by Marek in his series, leaving just the one in i915_gem_gtt_prepare_pages, which does indeed look very fishy. But if that one is so hard to fix it can just be replaced by an open coded for_each_sg loop that contains manual dma_map_page calls. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
On Mon, Sep 07, 2020 at 09:18:50PM +0100, Tom Murphy wrote: > Yeah we talked about passing an attr to map_sg to disable merging at > the following microconfernce: > https://linuxplumbersconf.org/event/7/contributions/846/ > As far as I can remember everyone seemed happy with that solution. I > won't be working on this though as I don't have any more time to > dedicate to this. It seems Lu Baolu will take over this. I'm absolutely again passing a flag. Tha just invites further abuse. We need a PCI ID based quirk or something else that can't be as easily abused. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
On Mon, 7 Sep 2020 at 08:00, Christoph Hellwig wrote: > > On Thu, Sep 03, 2020 at 09:18:37PM +0100, Tom Murphy wrote: > > Disable combining sg segments in the dma-iommu api. > > Combining the sg segments exposes a bug in the intel i915 driver which > > causes visual artifacts and the screen to freeze. This is most likely > > because of how the i915 handles the returned list. It probably doesn't > > respect the returned value specifying the number of elements in the list > > and instead depends on the previous behaviour of the intel iommu driver > > which would return the same number of elements in the output list as in > > the input list. > > So what is the state of addressing this properly in i915? IF we can't I think this is the latest on addressing this issue: https://patchwork.kernel.org/cover/11306999/ tl;dr: some people seem to be looking at it but I'm not sure if it's being actively worked on > get it done ASAP I wonder if we need a runtime quirk to disable > merging instead of blocking this conversion.. Yeah we talked about passing an attr to map_sg to disable merging at the following microconfernce: https://linuxplumbersconf.org/event/7/contributions/846/ As far as I can remember everyone seemed happy with that solution. I won't be working on this though as I don't have any more time to dedicate to this. It seems Lu Baolu will take over this. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] Fixed NULL pointer dereference in handle_edid functions for GPUs that do not support EDID
In the function intel_vgpu_reg_rw_edid of gvt/kvmgt.c, pos can get equal to NULL for GPUs that do not properly support EDID. In those cases, when pos gets passed to the handle_edid functions, it gets added a short offset then it's dereferenced in memcpy's, leading to a kernel oops: null pointer dereference. More concretely, that kernel oops renders Broadwell GPUs users completely unable to set up virtual machines with virtual GPU passthrough (virtual machines hang indefinitely when trying to make use of the virtual GPU), and make them have huge problems when trying to remove the virtual GPUs once the kernel oops has happened (writing 1 in the vGPU remove file just makes the operation hang undefinitely, again, and the kernel is unable to shutdown since the vGPU removing hangs indefinitely). More information on the issues that this causes are described in details in this github issue post: https://github.com/intel/gvt-linux/issues/170#issuecomment-685806160 This patch solves this problem by checking is pos is equal to NULL, and if it is, it sets ret to a nagative value, making the module simply indicate that the access to EDID region has failed, without any fatal repercussion. When this patch is applied, Broadwell GPU users do not suffer from that kernel oops anymore, and thus do not encounter any of the described problems and get able to set up virtual machines with GPU passthrough without problems. Users of GPUs with proper EDID support, are of course, still able to benefit from the EDID features. Signed-off-by: Alejandro W. Sior --- drivers/gpu/drm/i915/gvt/kvmgt.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index ad8a9df49f29..49163363ba4a 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -557,7 +557,9 @@ static size_t intel_vgpu_reg_rw_edid(struct intel_vgpu *vgpu, char *buf, (struct vfio_edid_region *)kvmgt_vdev(vgpu)->region[i].data; loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK; - if (pos < region->vfio_edid_regs.edid_offset) { + if (pos == NULL) { + ret = -EINVAL; + } else if (pos < region->vfio_edid_regs.edid_offset) { ret = handle_edid_regs(vgpu, region, buf, count, pos, iswrite); } else { pos -= EDID_BLOB_OFFSET; -- 2.28.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 6/7] drm: Validate encoder->possible_crtcs
On 11.02.20 18:04, Daniel Vetter wrote: > On Tue, Feb 11, 2020 at 06:22:07PM +0200, Ville Syrjala wrote: >> From: Ville Syrjälä >> >> WARN if the encoder possible_crtcs is effectively empty or contains >> bits for non-existing crtcs. >> >> v2: Move to drm_mode_config_validate() (Daniel) >> Make the docs say we WARN when this is wrong (Daniel) >> Extract full_crtc_mask() >> >> Cc: Thomas Zimmermann >> Cc: Daniel Vetter >> Signed-off-by: Ville Syrjälä > > When pushing the fixup needs to be applied before the validation patch > here, because we don't want to anger the bisect gods. > > Reviewed-by: Daniel Vetter > > I think with the fixup we should be good enough with the existing nonsense > in drivers. Fingers crossed. > -Daniel > > >> --- >> drivers/gpu/drm/drm_mode_config.c | 27 ++- >> include/drm/drm_encoder.h | 2 +- >> 2 files changed, 27 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_mode_config.c >> b/drivers/gpu/drm/drm_mode_config.c >> index afc91447293a..4c1b350ddb95 100644 >> --- a/drivers/gpu/drm/drm_mode_config.c >> +++ b/drivers/gpu/drm/drm_mode_config.c >> @@ -581,6 +581,29 @@ static void validate_encoder_possible_clones(struct >> drm_encoder *encoder) >> encoder->possible_clones, encoder_mask); >> } >> >> +static u32 full_crtc_mask(struct drm_device *dev) >> +{ >> +struct drm_crtc *crtc; >> +u32 crtc_mask = 0; >> + >> +drm_for_each_crtc(crtc, dev) >> +crtc_mask |= drm_crtc_mask(crtc); >> + >> +return crtc_mask; >> +} >> + >> +static void validate_encoder_possible_crtcs(struct drm_encoder *encoder) >> +{ >> +u32 crtc_mask = full_crtc_mask(encoder->dev); >> + >> +WARN((encoder->possible_crtcs & crtc_mask) == 0 || >> + (encoder->possible_crtcs & ~crtc_mask) != 0, >> + "Bogus possible_crtcs: " >> + "[ENCODER:%d:%s] possible_crtcs=0x%x (full crtc mask=0x%x)\n", >> + encoder->base.id, encoder->name, >> + encoder->possible_crtcs, crtc_mask); >> +} >> + >> void drm_mode_config_validate(struct drm_device *dev) >> { >> struct drm_encoder *encoder; >> @@ -588,6 +611,8 @@ void drm_mode_config_validate(struct drm_device *dev) >> drm_for_each_encoder(encoder, dev) >> fixup_encoder_possible_clones(encoder); >> >> -drm_for_each_encoder(encoder, dev) >> +drm_for_each_encoder(encoder, dev) { >> validate_encoder_possible_clones(encoder); >> +validate_encoder_possible_crtcs(encoder); >> +} >> } >> diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h >> index 3741963b9587..b236269f41ac 100644 >> --- a/include/drm/drm_encoder.h >> +++ b/include/drm/drm_encoder.h >> @@ -142,7 +142,7 @@ struct drm_encoder { >> * the bits for all &drm_crtc objects this encoder can be connected to >> * before calling drm_dev_register(). >> * >> - * In reality almost every driver gets this wrong. >> + * You will get a WARN if you get this wrong in the driver. >> * >> * Note that since CRTC objects can't be hotplugged the assigned indices >> * are stable and hence known before registering all objects. >> -- >> 2.24.1 >> > Triggers on an Advantech AIMB-228 (R1505G, 3 DP outputs): [ 14.033246] [ cut here ] [ 14.033248] Bogus possible_crtcs: [ENCODER:65:TMDS-65] possible_crtcs=0xf (full crtc mask=0x7) [ 14.033279] WARNING: CPU: 0 PID: 282 at ../drivers/gpu/drm/drm_mode_config.c:622 drm_mode_config_validate+0x17d/0x200 [drm] [ 14.033279] Modules linked in: amdgpu(E+) mfd_core(E) snd_hda_codec_realtek(E) kvm_amd(E) gpu_sched(E) i2c_algo_bit(E) ttm(E) snd_hda_codec_generic(E) kvm(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) drm_kms_helper(E) snd_hda_intel(E) snd_intel_dspcfg(E) snd_hda_codec(E) cec(E) snd_hwdep(E) drm(E) irqbypass(E) snd_hda_core(E) crc32_pclmul(E) snd_pcm(E) ghash_clmulni_intel(E) snd_timer(E) sg(E) aesni_intel(E) snd(E) ccp(E) soundcore(E) rng_core(E) glue_helper(E) libaes(E) crypto_simd(E) k10temp(E) efi_pstore(E) sp5100_tco(E) evdev(E) cryptd(E) pcspkr(E) efivars(E) video(E) button(E) acpi_cpufreq(E) w83627hf_wdt(E) watchdog(E) nct6775(E) hwmon_vid(E) efivarfs(E) ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E) crc16(E) mbcache(E) jbd2(E) sd_mod(E) t10_pi(E) crc_t10dif(E) crct10dif_generic(E) uas(E) usb_storage(E) ahci(E) libahci(E) xhci_pci(E) r8169(E) realtek(E) mdio_devres(E) xhci_hcd(E) libata(E) i2c_amd_mp2_pci(E) crct10dif_pclmul(E) crct10dif_common(E) scsi_mod(E) [ 14.033306] crc32c_intel(E) i2c_piix4(E) usbcore(E) libphy(E) [ 14.033310] CPU: 0 PID: 282 Comm: systemd-udevd Tainted: GE 5.9.0-rc3+ #2 [ 14.033311] Hardware name: Default string Default string/Default string, BIOS 5.0.1.4 02/14/2020 [ 14.033324] RIP: 0010:drm_mode_config_validate+0x17d/0x200 [drm] [ 14.033327] Code: 42 f0 75 e6 41 85 f8 74 09 44 89 c0 f7 d0 85 f8 74 1a 49 8b 54 24
Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
On Tue, Sep 08, 2020 at 02:04:53PM +0800, Lu Baolu wrote: > Do you mind telling where can I find Marek's series? [PATCH v10 00/30] DRM: fix struct sg_table nents vs. orig_nents misuse on various lists including the iommu one. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Kill unused savePCH_PORT_HOTPLUG
== Series Details == Series: series starting with [1/4] drm/i915: Kill unused savePCH_PORT_HOTPLUG URL : https://patchwork.freedesktop.org/series/81461/ State : failure == Summary == CI Bug Log - changes from CI_DRM_8982 -> Patchwork_18453 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_18453 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_18453, 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_18453/index.html Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_18453: ### IGT changes ### Possible regressions * igt@gem_exec_gttfill@basic: - fi-kbl-7500u: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-kbl-7500u/igt@gem_exec_gttf...@basic.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-kbl-7500u/igt@gem_exec_gttf...@basic.html - fi-byt-j1900: [PASS][3] -> [INCOMPLETE][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-byt-j1900/igt@gem_exec_gttf...@basic.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-byt-j1900/igt@gem_exec_gttf...@basic.html * igt@gem_exec_parallel@engines@basic: - fi-skl-guc: NOTRUN -> [INCOMPLETE][5] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-skl-guc/igt@gem_exec_parallel@engi...@basic.html - fi-skl-6600u: NOTRUN -> [INCOMPLETE][6] [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-skl-6600u/igt@gem_exec_parallel@engi...@basic.html - fi-bdw-5557u: NOTRUN -> [INCOMPLETE][7] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-bdw-5557u/igt@gem_exec_parallel@engi...@basic.html * igt@i915_selftest@live@gem_execbuf: - fi-bsw-nick:[PASS][8] -> [INCOMPLETE][9] [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bsw-nick/igt@i915_selftest@live@gem_execbuf.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-bsw-nick/igt@i915_selftest@live@gem_execbuf.html - fi-bsw-n3050: [PASS][10] -> [INCOMPLETE][11] [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bsw-n3050/igt@i915_selftest@live@gem_execbuf.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-bsw-n3050/igt@i915_selftest@live@gem_execbuf.html - fi-ilk-650: NOTRUN -> [INCOMPLETE][12] [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-ilk-650/igt@i915_selftest@live@gem_execbuf.html * igt@runner@aborted: - fi-byt-j1900: NOTRUN -> [FAIL][13] [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-byt-j1900/igt@run...@aborted.html Known issues Here are the changes found in Patchwork_18453 that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_gttfill@basic: - fi-apl-guc: [PASS][14] -> [INCOMPLETE][15] ([i915#1635]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-apl-guc/igt@gem_exec_gttf...@basic.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-apl-guc/igt@gem_exec_gttf...@basic.html * igt@i915_selftest@live@gem_execbuf: - fi-snb-2600:[PASS][16] -> [INCOMPLETE][17] ([i915#82]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-snb-2600/igt@i915_selftest@live@gem_execbuf.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-snb-2600/igt@i915_selftest@live@gem_execbuf.html Possible fixes * igt@gem_exec_gttfill@basic: - fi-bdw-5557u: [INCOMPLETE][18] -> [PASS][19] [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bdw-5557u/igt@gem_exec_gttf...@basic.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-bdw-5557u/igt@gem_exec_gttf...@basic.html - fi-skl-guc: [INCOMPLETE][20] -> [PASS][21] [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-skl-guc/igt@gem_exec_gttf...@basic.html [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-skl-guc/igt@gem_exec_gttf...@basic.html - fi-skl-6600u: [INCOMPLETE][22] -> [PASS][23] [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-skl-6600u/igt@gem_exec_gttf...@basic.html [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-skl-6600u/igt@gem_exec_gttf...@basic.html - fi-ilk-650: [INCOMPLETE][24] -> [PASS][25] [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-ilk-650/igt@gem_exec_gttf...@basic.html [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18453/fi-ilk-650/igt@gem_exec_gttf...@basic.html
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Fixed NULL pointer dereference in handle_edid functions for GPUs that do not support EDID
== Series Details == Series: Fixed NULL pointer dereference in handle_edid functions for GPUs that do not support EDID URL : https://patchwork.freedesktop.org/series/81463/ State : warning == Summary == $ dim checkpatch origin/drm-tip 24f0f91b0b97 Fixed NULL pointer dereference in handle_edid functions for GPUs that do not support EDID -:7: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) #7: In the function intel_vgpu_reg_rw_edid of gvt/kvmgt.c, pos can get equal to NULL for GPUs that do not -:42: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "!pos" #42: FILE: drivers/gpu/drm/i915/gvt/kvmgt.c:560: + if (pos == NULL) { -:47: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Alejandro Sior ' total: 0 errors, 2 warnings, 1 checks, 10 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for Fixed NULL pointer dereference in handle_edid functions for GPUs that do not support EDID
== Series Details == Series: Fixed NULL pointer dereference in handle_edid functions for GPUs that do not support EDID URL : https://patchwork.freedesktop.org/series/81463/ State : failure == Summary == CI Bug Log - changes from CI_DRM_8982 -> Patchwork_18454 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_18454 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_18454, 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_18454/index.html Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_18454: ### IGT changes ### Possible regressions * igt@gem_exec_gttfill@basic: - fi-kbl-7500u: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-kbl-7500u/igt@gem_exec_gttf...@basic.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-kbl-7500u/igt@gem_exec_gttf...@basic.html * igt@gem_exec_parallel@engines@basic: - fi-skl-6600u: NOTRUN -> [INCOMPLETE][3] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-skl-6600u/igt@gem_exec_parallel@engi...@basic.html * igt@gem_exec_parallel@engines@contexts: - fi-byt-j1900: [PASS][4] -> [INCOMPLETE][5] [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-byt-j1900/igt@gem_exec_parallel@engi...@contexts.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-byt-j1900/igt@gem_exec_parallel@engi...@contexts.html * igt@i915_selftest@live@gem_execbuf: - fi-bsw-nick:[PASS][6] -> [INCOMPLETE][7] [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bsw-nick/igt@i915_selftest@live@gem_execbuf.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-bsw-nick/igt@i915_selftest@live@gem_execbuf.html - fi-ilk-650: NOTRUN -> [INCOMPLETE][8] [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-ilk-650/igt@i915_selftest@live@gem_execbuf.html * igt@runner@aborted: - fi-byt-j1900: NOTRUN -> [FAIL][9] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-byt-j1900/igt@run...@aborted.html Warnings * igt@runner@aborted: - fi-bdw-5557u: [FAIL][10] ([i915#483]) -> [FAIL][11] [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bdw-5557u/igt@run...@aborted.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-bdw-5557u/igt@run...@aborted.html Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@gem_exec_parallel@engines@contexts: - {fi-ehl-1}: NOTRUN -> [INCOMPLETE][12] [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-ehl-1/igt@gem_exec_parallel@engi...@contexts.html Known issues Here are the changes found in Patchwork_18454 that come from known issues: ### IGT changes ### Issues hit * igt@i915_selftest@live@gem_execbuf: - fi-snb-2600:[PASS][13] -> [INCOMPLETE][14] ([i915#82]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-snb-2600/igt@i915_selftest@live@gem_execbuf.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-snb-2600/igt@i915_selftest@live@gem_execbuf.html Possible fixes * igt@gem_exec_gttfill@basic: - fi-skl-6600u: [INCOMPLETE][15] -> [PASS][16] [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-skl-6600u/igt@gem_exec_gttf...@basic.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-skl-6600u/igt@gem_exec_gttf...@basic.html - fi-ilk-650: [INCOMPLETE][17] -> [PASS][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-ilk-650/igt@gem_exec_gttf...@basic.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-ilk-650/igt@gem_exec_gttf...@basic.html * igt@gem_exec_parallel@engines@basic: - {fi-ehl-1}: [INCOMPLETE][19] -> [PASS][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-ehl-1/igt@gem_exec_parallel@engi...@basic.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-ehl-1/igt@gem_exec_parallel@engi...@basic.html * igt@i915_selftest@live@gem_execbuf: - fi-gdg-551: [INCOMPLETE][21] ([i915#172]) -> [PASS][22] [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-gdg-551/igt@i915_selftest@live@gem_execbuf.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18454/fi-gdg-551/igt@i915_selftest@live@gem_execbuf.html - fi-snb-2520m: [INCOMPLETE][23] -> [PASS][24] [23]: https:
Re: [Intel-gfx] [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api
Hi, On 27/08/2020 22:36, Logan Gunthorpe wrote: On 2020-08-23 6:04 p.m., Tom Murphy wrote: I have added a check for the sg_dma_len == 0 : """ } __sgt_iter(struct scatterlist *sgl, bool dma) { struct sgt_iter s = { .sgp = sgl }; + if (sgl && sg_dma_len(sgl) == 0) + s.sgp = NULL; if (s.sgp) { . """ at location [1]. but it doens't fix the problem. Based on my read of the code, it looks like we also need to change usage of sgl->length... Something like the rough patch below, maybe? This thread was brought to my attention and I initially missed this reply. Essentially I came to the same conclusion about the need to use sg_dma_len. One small correction below: Also, Tom, do you have an updated version of the patchset to convert the Intel IOMMU to dma-iommu available? The last one I've found doesn't apply cleanly (I'm assuming parts of it have been merged in slightly modified forms). Thanks, Logan -- diff --git a/drivers/gpu/drm/i915/i915_scatterlist.h b/drivers/gpu/drm/i915/i915 index b7b59328cb76..9367ac801f0c 100644 --- a/drivers/gpu/drm/i915/i915_scatterlist.h +++ b/drivers/gpu/drm/i915/i915_scatterlist.h @@ -27,13 +27,19 @@ static __always_inline struct sgt_iter { } __sgt_iter(struct scatterlist *sgl, bool dma) { struct sgt_iter s = { .sgp = sgl }; + if (sgl && !sg_dma_len(s.sgp)) I'd extend the condition to be, just to be safe: if (dma && sgl && !sg_dma_len(s.sgp)) + s.sgp = NULL; + if (s.sgp) { s.max = s.curr = s.sgp->offset; - s.max += s.sgp->length; - if (dma) + + if (dma) { + s.max += sg_dma_len(s.sgp); s.dma = sg_dma_address(s.sgp); - else + } else { + s.max += s.sgp->length; s.pfn = page_to_pfn(sg_page(s.sgp)); + } Otherwise has this been tested or alternatively how to test it? (How to repro the issue.) Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5.8 162/186] drm/i915: Fix sha_text population code
From: Sean Paul commit 9ab57658a608f879469ffa22b723c4539c05a58f upstream. This patch fixes a few bugs: 1- We weren't taking into account sha_leftovers when adding multiple ksvs to sha_text. As such, we were or'ing the end of ksv[j - 1] with the beginning of ksv[j] 2- In the sha_leftovers == 2 and sha_leftovers == 3 case, bstatus was being placed on the wrong half of sha_text, overlapping the leftover ksv value 3- In the sha_leftovers == 2 case, we need to manually terminate the byte stream with 0x80 since the hardware doesn't have enough room to add it after writing M0 The upside is that all of the HDCP supported HDMI repeaters I could find on Amazon just strip HDCP anyways, so it turns out to be _really_ hard to hit any of these cases without an MST hub, which is not (yet) supported. Oh, and the sha_leftovers == 1 case works perfectly! Fixes: ee5e5e7a5e0f ("drm/i915: Add HDCP framework + base implementation") Cc: Chris Wilson Cc: Ramalingam C Cc: Daniel Vetter Cc: Sean Paul Cc: Jani Nikula Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: intel-gfx@lists.freedesktop.org Cc: # v4.17+ Reviewed-by: Ramalingam C Signed-off-by: Sean Paul Signed-off-by: Ramalingam C Link: https://patchwork.freedesktop.org/patch/msgid/20200818153910.27894-2-s...@poorly.run (cherry picked from commit 1f0882214fd0037b74f245d9be75c31516fed040) Signed-off-by: Jani Nikula Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/i915/display/intel_hdcp.c | 26 -- include/drm/drm_hdcp.h|3 +++ 2 files changed, 23 insertions(+), 6 deletions(-) --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -336,8 +336,10 @@ int intel_hdcp_validate_v_prime(struct i /* Fill up the empty slots in sha_text and write it out */ sha_empty = sizeof(sha_text) - sha_leftovers; - for (j = 0; j < sha_empty; j++) - sha_text |= ksv[j] << ((sizeof(sha_text) - j - 1) * 8); + for (j = 0; j < sha_empty; j++) { + u8 off = ((sizeof(sha_text) - j - 1 - sha_leftovers) * 8); + sha_text |= ksv[j] << off; + } ret = intel_write_sha_text(dev_priv, sha_text); if (ret < 0) @@ -435,7 +437,7 @@ int intel_hdcp_validate_v_prime(struct i /* Write 32 bits of text */ intel_de_write(dev_priv, HDCP_REP_CTL, rep_ctl | HDCP_SHA1_TEXT_32); - sha_text |= bstatus[0] << 24 | bstatus[1] << 16; + sha_text |= bstatus[0] << 8 | bstatus[1]; ret = intel_write_sha_text(dev_priv, sha_text); if (ret < 0) return ret; @@ -450,17 +452,29 @@ int intel_hdcp_validate_v_prime(struct i return ret; sha_idx += sizeof(sha_text); } + + /* +* Terminate the SHA-1 stream by hand. For the other leftover +* cases this is appended by the hardware. +*/ + intel_de_write(dev_priv, HDCP_REP_CTL, + rep_ctl | HDCP_SHA1_TEXT_32); + sha_text = DRM_HDCP_SHA1_TERMINATOR << 24; + ret = intel_write_sha_text(dev_priv, sha_text); + if (ret < 0) + return ret; + sha_idx += sizeof(sha_text); } else if (sha_leftovers == 3) { - /* Write 32 bits of text */ + /* Write 32 bits of text (filled from LSB) */ intel_de_write(dev_priv, HDCP_REP_CTL, rep_ctl | HDCP_SHA1_TEXT_32); - sha_text |= bstatus[0] << 24; + sha_text |= bstatus[0]; ret = intel_write_sha_text(dev_priv, sha_text); if (ret < 0) return ret; sha_idx += sizeof(sha_text); - /* Write 8 bits of text, 24 bits of M0 */ + /* Write 8 bits of text (filled from LSB), 24 bits of M0 */ intel_de_write(dev_priv, HDCP_REP_CTL, rep_ctl | HDCP_SHA1_TEXT_8); ret = intel_write_sha_text(dev_priv, bstatus[1]); --- a/include/drm/drm_hdcp.h +++ b/include/drm/drm_hdcp.h @@ -29,6 +29,9 @@ /* Slave address for the HDCP registers in the receiver */ #define DRM_HDCP_DDC_ADDR 0x3A +/* Value to use at the end of the SHA-1 bytestream used for repeaters */ +#define DRM_HDCP_SHA1_TERMINATOR 0x80 + /* HDCP register offsets for HDMI/DVI devices */ #define DRM_HDCP_DDC_BKSV 0x00 #define DRM_HDCP_DDC_RI_PRIME 0x08 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api
On 2020-09-08 9:28 a.m., Tvrtko Ursulin wrote: >> >> diff --git a/drivers/gpu/drm/i915/i915_scatterlist.h >> b/drivers/gpu/drm/i915/i915 >> index b7b59328cb76..9367ac801f0c 100644 >> --- a/drivers/gpu/drm/i915/i915_scatterlist.h >> +++ b/drivers/gpu/drm/i915/i915_scatterlist.h >> @@ -27,13 +27,19 @@ static __always_inline struct sgt_iter { >> } __sgt_iter(struct scatterlist *sgl, bool dma) { >> struct sgt_iter s = { .sgp = sgl }; >> >> + if (sgl && !sg_dma_len(s.sgp)) > > I'd extend the condition to be, just to be safe: > if (dma && sgl && !sg_dma_len(s.sgp)) > Right, good catch, that's definitely necessary. >> + s.sgp = NULL; >> + >> if (s.sgp) { >> s.max = s.curr = s.sgp->offset; >> - s.max += s.sgp->length; >> - if (dma) >> + >> + if (dma) { >> + s.max += sg_dma_len(s.sgp); >> s.dma = sg_dma_address(s.sgp); >> - else >> + } else { >> + s.max += s.sgp->length; >> s.pfn = page_to_pfn(sg_page(s.sgp)); >> + } > > Otherwise has this been tested or alternatively how to test it? (How to > repro the issue.) It has not been tested. To test it, you need Tom's patch set without the last "DO NOT MERGE" patch: https://lkml.kernel.org/lkml/20200907070035.ga25...@infradead.org/T/ Thanks, Logan ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api
On 08/09/2020 16:44, Logan Gunthorpe wrote: On 2020-09-08 9:28 a.m., Tvrtko Ursulin wrote: diff --git a/drivers/gpu/drm/i915/i915_scatterlist.h b/drivers/gpu/drm/i915/i915 index b7b59328cb76..9367ac801f0c 100644 --- a/drivers/gpu/drm/i915/i915_scatterlist.h +++ b/drivers/gpu/drm/i915/i915_scatterlist.h @@ -27,13 +27,19 @@ static __always_inline struct sgt_iter { } __sgt_iter(struct scatterlist *sgl, bool dma) { struct sgt_iter s = { .sgp = sgl }; + if (sgl && !sg_dma_len(s.sgp)) I'd extend the condition to be, just to be safe: if (dma && sgl && !sg_dma_len(s.sgp)) Right, good catch, that's definitely necessary. + s.sgp = NULL; + if (s.sgp) { s.max = s.curr = s.sgp->offset; - s.max += s.sgp->length; - if (dma) + + if (dma) { + s.max += sg_dma_len(s.sgp); s.dma = sg_dma_address(s.sgp); - else + } else { + s.max += s.sgp->length; s.pfn = page_to_pfn(sg_page(s.sgp)); + } Otherwise has this been tested or alternatively how to test it? (How to repro the issue.) It has not been tested. To test it, you need Tom's patch set without the last "DO NOT MERGE" patch: https://lkml.kernel.org/lkml/20200907070035.ga25...@infradead.org/T/ Tom, do you have a branch somewhere I could pull from? (Just being lazy about downloading a bunch of messages from the archives.) What GPU is in your Lenovo x1 carbon 5th generation and what graphical/desktop setup I need to repro? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gvt: Fix NULL pointer dereference in intel_vgpu_reg_rw_edid()
> In the function intel_vgpu_reg_rw_edid of gvt/kvmgt.c, pos can get equal to > NULL for GPUs that do not > properly support EDID. … * I propose to reconsider the previous patch subject once more. * Did the script “checkpatch.pl” point any adjustment possibilities out for the change description? > This patch solves this problem by … Would an imperative wording be preferred for the commit message? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst#n151 > … to a nagative value, … Please avoid a typo here. I suggest to add the tag “Fixes” to the commit message. Regards, Markus ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [01/12] drm/i915: Add more AUX CHs to the enum
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote: > From: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > > We need to go up to AUX_CH_I (aka. AUX CH USBC6) these days. > Reviewed-by: José Roberto de Souza > Signed-off-by: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > --- > drivers/gpu/drm/i915/display/intel_display.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.h > b/drivers/gpu/drm/i915/display/intel_display.h > index f68007ff8a13..5b736883cd11 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.h > +++ b/drivers/gpu/drm/i915/display/intel_display.h > @@ -282,6 +282,8 @@ enum aux_ch { > AUX_CH_E, /* ICL+ */ > AUX_CH_F, > AUX_CH_G, > + AUX_CH_H, > + AUX_CH_I, > }; > > #define aux_ch_name(a) ((a) + 'A') > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [02/12] drm/i915: Add PORT_{H, I} to intel_port_to_power_domain()
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote: > From: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > > We need to go up to PORT_I (aka. TC6) these days. > Reviewed-by: José Roberto de Souza > Signed-off-by: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > --- > drivers/gpu/drm/i915/display/intel_display.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 182cef0dc2fd..665aa4283fb9 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -7289,6 +7289,10 @@ enum intel_display_power_domain > intel_port_to_power_domain(enum port port) > return POWER_DOMAIN_PORT_DDI_F_LANES; > case PORT_G: > return POWER_DOMAIN_PORT_DDI_G_LANES; > + case PORT_H: > + return POWER_DOMAIN_PORT_DDI_H_LANES; > + case PORT_I: > + return POWER_DOMAIN_PORT_DDI_I_LANES; > default: > MISSING_CASE(port); > return POWER_DOMAIN_PORT_OTHER; > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [03/12] drm/i915: Add AUX_CH_{H, I} power domain handling
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote: > From: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > > AUX CH H/I need their power domains too. Reviewed-by: José Roberto de Souza > > Signed-off-by: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > --- > drivers/gpu/drm/i915/display/intel_display.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 665aa4283fb9..87831fd9e1e1 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -7318,6 +7318,10 @@ intel_aux_power_domain(struct intel_digital_port > *dig_port) > return POWER_DOMAIN_AUX_F_TBT; > case AUX_CH_G: > return POWER_DOMAIN_AUX_G_TBT; > + case AUX_CH_H: > + return POWER_DOMAIN_AUX_H_TBT; > + case AUX_CH_I: > + return POWER_DOMAIN_AUX_I_TBT; > default: > MISSING_CASE(dig_port->aux_ch); > return POWER_DOMAIN_AUX_C_TBT; > @@ -7349,6 +7353,10 @@ intel_legacy_aux_to_power_domain(enum aux_ch aux_ch) > return POWER_DOMAIN_AUX_F; > case AUX_CH_G: > return POWER_DOMAIN_AUX_G; > + case AUX_CH_H: > + return POWER_DOMAIN_AUX_H; > + case AUX_CH_I: > + return POWER_DOMAIN_AUX_I; > default: > MISSING_CASE(aux_ch); > return POWER_DOMAIN_AUX_A; > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [04/12] drm/i915: Add VBT DVO ports H and I
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote: > From: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > > VBT has ports H and I since version 217. > Reviewed-by: José Roberto de Souza > Signed-off-by: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > --- > drivers/gpu/drm/i915/display/intel_bios.c | 2 ++ > drivers/gpu/drm/i915/display/intel_vbt_defs.h | 8 ++-- > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c > b/drivers/gpu/drm/i915/display/intel_bios.c > index 6593e2c38043..2bf0bc0deee8 100644 > --- a/drivers/gpu/drm/i915/display/intel_bios.c > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > @@ -1653,6 +1653,8 @@ static enum port dvo_port_to_port(struct > drm_i915_private *dev_priv, > [PORT_E] = { DVO_PORT_HDMIE, DVO_PORT_DPE, DVO_PORT_CRT }, > [PORT_F] = { DVO_PORT_HDMIF, DVO_PORT_DPF, -1 }, > [PORT_G] = { DVO_PORT_HDMIG, DVO_PORT_DPG, -1 }, > + [PORT_H] = { DVO_PORT_HDMIH, DVO_PORT_DPH, -1 }, > + [PORT_I] = { DVO_PORT_HDMII, DVO_PORT_DPI, -1 }, > }; > /* >* Bspec lists the ports as A, B, C, D - however internally in our > diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h > b/drivers/gpu/drm/i915/display/intel_vbt_defs.h > index aef7fe932d1a..e502d65300fa 100644 > --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h > +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h > @@ -293,8 +293,12 @@ struct bdb_general_features { > #define DVO_PORT_HDMIE 12 /* 193 > */ > #define DVO_PORT_DPF 13 /* N/A */ > #define DVO_PORT_HDMIF 14 /* N/A > */ > -#define DVO_PORT_DPG 15 > -#define DVO_PORT_HDMIG 16 > +#define DVO_PORT_DPG 15 /* 217 */ > +#define DVO_PORT_HDMIG 16 /* 217 > */ > +#define DVO_PORT_DPH 17 /* 217 */ > +#define DVO_PORT_HDMIH 18 /* 217 > */ > +#define DVO_PORT_DPI 19 /* 217 */ > +#define DVO_PORT_HDMII 20 /* 217 > */ > #define DVO_PORT_MIPIA 21 /* 171 > */ > #define DVO_PORT_MIPIB 22 /* 171 > */ > #define DVO_PORT_MIPIC 23 /* 171 > */ > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [05/12] drm/i915: Add VBT AUX CH H and I
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote: > From: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > > As with everything else VBT can now specify AUX CH H or I. > Reviewed-by: José Roberto de Souza > Signed-off-by: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > --- > drivers/gpu/drm/i915/display/intel_bios.c | 6 ++ > drivers/gpu/drm/i915/display/intel_vbt_defs.h | 2 ++ > 2 files changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c > b/drivers/gpu/drm/i915/display/intel_bios.c > index 2bf0bc0deee8..05eb88ee73f8 100644 > --- a/drivers/gpu/drm/i915/display/intel_bios.c > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > @@ -2649,6 +2649,12 @@ enum aux_ch intel_bios_port_aux_ch(struct > drm_i915_private *dev_priv, > case DP_AUX_G: > aux_ch = AUX_CH_G; > break; > + case DP_AUX_H: > + aux_ch = AUX_CH_H; > + break; > + case DP_AUX_I: > + aux_ch = AUX_CH_I; > + break; > default: > MISSING_CASE(info->alternate_aux_channel); > aux_ch = AUX_CH_A; > diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h > b/drivers/gpu/drm/i915/display/intel_vbt_defs.h > index e502d65300fa..b5f7a52f751a 100644 > --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h > +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h > @@ -334,6 +334,8 @@ enum vbt_gmbus_ddi { > #define DP_AUX_E 0x50 > #define DP_AUX_F 0x60 > #define DP_AUX_G 0x70 > +#define DP_AUX_H 0x80 > +#define DP_AUX_I 0x90 > > #define VBT_DP_MAX_LINK_RATE_HBR30 > #define VBT_DP_MAX_LINK_RATE_HBR21 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [06/12] drm/i915: Nuke the redundant TC/TBT HPD bit defines
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote: > From: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > > We have nice parametrized GEN11_{TC,TBT}_HOTPLUG() so nuke > the overlapping defines. Reviewed-by: José Roberto de Souza > > Signed-off-by: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > --- > drivers/gpu/drm/i915/i915_irq.c | 20 +- > drivers/gpu/drm/i915/i915_reg.h | 36 +++-- > 2 files changed, 22 insertions(+), 34 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 562b43ed077f..ad52109c747d 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -131,19 +131,19 @@ static const u32 hpd_bxt[HPD_NUM_PINS] = { > }; > > static const u32 hpd_gen11[HPD_NUM_PINS] = { > - [HPD_PORT_C] = GEN11_TC1_HOTPLUG | GEN11_TBT1_HOTPLUG, > - [HPD_PORT_D] = GEN11_TC2_HOTPLUG | GEN11_TBT2_HOTPLUG, > - [HPD_PORT_E] = GEN11_TC3_HOTPLUG | GEN11_TBT3_HOTPLUG, > - [HPD_PORT_F] = GEN11_TC4_HOTPLUG | GEN11_TBT4_HOTPLUG, > + [HPD_PORT_C] = GEN11_TC_HOTPLUG(PORT_TC1) | GEN11_TBT_HOTPLUG(PORT_TC1), > + [HPD_PORT_D] = GEN11_TC_HOTPLUG(PORT_TC2) | GEN11_TBT_HOTPLUG(PORT_TC2), > + [HPD_PORT_E] = GEN11_TC_HOTPLUG(PORT_TC3) | GEN11_TBT_HOTPLUG(PORT_TC3), > + [HPD_PORT_F] = GEN11_TC_HOTPLUG(PORT_TC4) | GEN11_TBT_HOTPLUG(PORT_TC4), > }; > > static const u32 hpd_gen12[HPD_NUM_PINS] = { > - [HPD_PORT_D] = GEN11_TC1_HOTPLUG | GEN11_TBT1_HOTPLUG, > - [HPD_PORT_E] = GEN11_TC2_HOTPLUG | GEN11_TBT2_HOTPLUG, > - [HPD_PORT_F] = GEN11_TC3_HOTPLUG | GEN11_TBT3_HOTPLUG, > - [HPD_PORT_G] = GEN11_TC4_HOTPLUG | GEN11_TBT4_HOTPLUG, > - [HPD_PORT_H] = GEN12_TC5_HOTPLUG | GEN12_TBT5_HOTPLUG, > - [HPD_PORT_I] = GEN12_TC6_HOTPLUG | GEN12_TBT6_HOTPLUG, > + [HPD_PORT_D] = GEN11_TC_HOTPLUG(PORT_TC1) | GEN11_TBT_HOTPLUG(PORT_TC1), > + [HPD_PORT_E] = GEN11_TC_HOTPLUG(PORT_TC2) | GEN11_TBT_HOTPLUG(PORT_TC2), > + [HPD_PORT_F] = GEN11_TC_HOTPLUG(PORT_TC3) | GEN11_TBT_HOTPLUG(PORT_TC3), > + [HPD_PORT_G] = GEN11_TC_HOTPLUG(PORT_TC4) | GEN11_TBT_HOTPLUG(PORT_TC4), > + [HPD_PORT_H] = GEN11_TC_HOTPLUG(PORT_TC5) | GEN11_TBT_HOTPLUG(PORT_TC5), > + [HPD_PORT_I] = GEN11_TC_HOTPLUG(PORT_TC6) | GEN11_TBT_HOTPLUG(PORT_TC6), > }; > > static const u32 hpd_icp[HPD_NUM_PINS] = { > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 2ecde5c2e357..d7359f3bbc64 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7681,32 +7681,20 @@ enum { > #define GEN11_DE_HPD_IMR _MMIO(0x44474) > #define GEN11_DE_HPD_IIR _MMIO(0x44478) > #define GEN11_DE_HPD_IER _MMIO(0x4447c) > -#define GEN12_TC6_HOTPLUG (1 << 21) > -#define GEN12_TC5_HOTPLUG (1 << 20) > -#define GEN11_TC4_HOTPLUG (1 << 19) > -#define GEN11_TC3_HOTPLUG (1 << 18) > -#define GEN11_TC2_HOTPLUG (1 << 17) > -#define GEN11_TC1_HOTPLUG (1 << 16) > #define GEN11_TC_HOTPLUG(tc_port) (1 << ((tc_port) + 16)) > -#define GEN11_DE_TC_HOTPLUG_MASK(GEN12_TC6_HOTPLUG | \ > - GEN12_TC5_HOTPLUG | \ > - GEN11_TC4_HOTPLUG | \ > - GEN11_TC3_HOTPLUG | \ > - GEN11_TC2_HOTPLUG | \ > - GEN11_TC1_HOTPLUG) > -#define GEN12_TBT6_HOTPLUG (1 << 5) > -#define GEN12_TBT5_HOTPLUG (1 << 4) > -#define GEN11_TBT4_HOTPLUG (1 << 3) > -#define GEN11_TBT3_HOTPLUG (1 << 2) > -#define GEN11_TBT2_HOTPLUG (1 << 1) > -#define GEN11_TBT1_HOTPLUG (1 << 0) > +#define GEN11_DE_TC_HOTPLUG_MASK(GEN11_TC_HOTPLUG(PORT_TC6) | \ > + GEN11_TC_HOTPLUG(PORT_TC5) | \ > + GEN11_TC_HOTPLUG(PORT_TC4) | \ > + GEN11_TC_HOTPLUG(PORT_TC3) | \ > + GEN11_TC_HOTPLUG(PORT_TC2) | \ > + GEN11_TC_HOTPLUG(PORT_TC1)) > #define GEN11_TBT_HOTPLUG(tc_port) (1 << (tc_port)) > -#define GEN11_DE_TBT_HOTPLUG_MASK (GEN12_TBT6_HOTPLUG | \ > - GEN12_TBT5_HOTPLUG | \ > - GEN11_TBT4_HOTPLUG | \ > - GEN11_TBT3_HOTPLUG | \ > - GEN11_TBT2_HOTPLUG | \ > - GEN11_TBT1_HOTPLUG) > +#define GEN11_DE_TBT_HOTPLUG_MASK
Re: [Intel-gfx] [07/12] drm/i915: Configure GEN11_{TBT, TC}_HOTPLUG_CTL for ports TC5/6
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote: > From: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > > gen11_hpd_detection_setup() is missing ports TC5/6. Add them. > > TODO: Might be nice to only enable the hpd detection logic > for ports we actually have. Should be rolled out for all > platforms if/when done... TC5 and TC6 don't exist in ICL but this is not a reserved register, should not cause any harm. Reviewed-by: José Roberto de Souza > > Signed-off-by: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > --- > drivers/gpu/drm/i915/i915_irq.c | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index ad52109c747d..839ae674bc44 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -3109,14 +3109,18 @@ static void gen11_hpd_detection_setup(struct > drm_i915_private *dev_priv) > hotplug |= GEN11_HOTPLUG_CTL_ENABLE(PORT_TC1) | > GEN11_HOTPLUG_CTL_ENABLE(PORT_TC2) | > GEN11_HOTPLUG_CTL_ENABLE(PORT_TC3) | > -GEN11_HOTPLUG_CTL_ENABLE(PORT_TC4); > +GEN11_HOTPLUG_CTL_ENABLE(PORT_TC4) | > +GEN11_HOTPLUG_CTL_ENABLE(PORT_TC5) | > +GEN11_HOTPLUG_CTL_ENABLE(PORT_TC6); > I915_WRITE(GEN11_TC_HOTPLUG_CTL, hotplug); > > hotplug = I915_READ(GEN11_TBT_HOTPLUG_CTL); > hotplug |= GEN11_HOTPLUG_CTL_ENABLE(PORT_TC1) | > GEN11_HOTPLUG_CTL_ENABLE(PORT_TC2) | > GEN11_HOTPLUG_CTL_ENABLE(PORT_TC3) | > -GEN11_HOTPLUG_CTL_ENABLE(PORT_TC4); > +GEN11_HOTPLUG_CTL_ENABLE(PORT_TC4) | > +GEN11_HOTPLUG_CTL_ENABLE(PORT_TC5) | > +GEN11_HOTPLUG_CTL_ENABLE(PORT_TC6); > I915_WRITE(GEN11_TBT_HOTPLUG_CTL, hotplug); > } > > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [08/12] drm/i915: Split icp_hpd_detection_setup() into ddi vs. tc parts
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote: > From: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > > No reason to stuff both DDI and TC port handling into the same > function. Split it into two. Reviewed-by: José Roberto de Souza > > Signed-off-by: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > --- > drivers/gpu/drm/i915/i915_irq.c | 48 ++--- > 1 file changed, 27 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 839ae674bc44..92d74448ee03 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -3045,21 +3045,24 @@ static void ibx_hpd_irq_setup(struct drm_i915_private > *dev_priv) > ibx_hpd_detection_setup(dev_priv); > } > > -static void icp_hpd_detection_setup(struct drm_i915_private *dev_priv, > - u32 ddi_hotplug_enable_mask, > - u32 tc_hotplug_enable_mask) > +static void icp_ddi_hpd_detection_setup(struct drm_i915_private *dev_priv, > + u32 enable_mask) > { > u32 hotplug; > > hotplug = I915_READ(SHOTPLUG_CTL_DDI); > - hotplug |= ddi_hotplug_enable_mask; > + hotplug |= enable_mask; > I915_WRITE(SHOTPLUG_CTL_DDI, hotplug); > +} > > - if (tc_hotplug_enable_mask) { > - hotplug = I915_READ(SHOTPLUG_CTL_TC); > - hotplug |= tc_hotplug_enable_mask; > - I915_WRITE(SHOTPLUG_CTL_TC, hotplug); > - } > +static void icp_tc_hpd_detection_setup(struct drm_i915_private *dev_priv, > +u32 enable_mask) > +{ > + u32 hotplug; > + > + hotplug = I915_READ(SHOTPLUG_CTL_TC); > + hotplug |= enable_mask; > + I915_WRITE(SHOTPLUG_CTL_TC, hotplug); > } > > static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv, > @@ -3075,7 +3078,9 @@ static void icp_hpd_irq_setup(struct drm_i915_private > *dev_priv, > > ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs); > > - icp_hpd_detection_setup(dev_priv, ddi_enable_mask, tc_enable_mask); > + icp_ddi_hpd_detection_setup(dev_priv, ddi_enable_mask); > + if (tc_enable_mask) > + icp_tc_hpd_detection_setup(dev_priv, tc_enable_mask); > } > > /* > @@ -3493,17 +3498,18 @@ static void icp_irq_postinstall(struct > drm_i915_private *dev_priv) > gen3_assert_iir_is_zero(&dev_priv->uncore, SDEIIR); > I915_WRITE(SDEIMR, ~mask); > > - if (HAS_PCH_TGP(dev_priv)) > - icp_hpd_detection_setup(dev_priv, TGP_DDI_HPD_ENABLE_MASK, > - TGP_TC_HPD_ENABLE_MASK); > - else if (HAS_PCH_JSP(dev_priv)) > - icp_hpd_detection_setup(dev_priv, TGP_DDI_HPD_ENABLE_MASK, 0); > - else if (HAS_PCH_MCC(dev_priv)) > - icp_hpd_detection_setup(dev_priv, ICP_DDI_HPD_ENABLE_MASK, > - ICP_TC_HPD_ENABLE(PORT_TC1)); > - else > - icp_hpd_detection_setup(dev_priv, ICP_DDI_HPD_ENABLE_MASK, > - ICP_TC_HPD_ENABLE_MASK); > + if (HAS_PCH_TGP(dev_priv)) { > + icp_ddi_hpd_detection_setup(dev_priv, TGP_DDI_HPD_ENABLE_MASK); > + icp_tc_hpd_detection_setup(dev_priv, TGP_TC_HPD_ENABLE_MASK); > + } else if (HAS_PCH_JSP(dev_priv)) { > + icp_ddi_hpd_detection_setup(dev_priv, TGP_DDI_HPD_ENABLE_MASK); > + } else if (HAS_PCH_MCC(dev_priv)) { > + icp_ddi_hpd_detection_setup(dev_priv, ICP_DDI_HPD_ENABLE_MASK); > + icp_tc_hpd_detection_setup(dev_priv, > ICP_TC_HPD_ENABLE(PORT_TC1)); > + } else { > + icp_ddi_hpd_detection_setup(dev_priv, ICP_DDI_HPD_ENABLE_MASK); > + icp_tc_hpd_detection_setup(dev_priv, ICP_TC_HPD_ENABLE_MASK); > + } > } > > static void gen11_irq_postinstall(struct drm_i915_private *dev_priv) > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [09/12] drm/i915: Move hpd_pin setup to encoder init
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote: > From: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > > Currently DP/HDMI/DDI encoders init their hpd_pin from the > connector init. Let's move it to the encoder init so that > we don't need to add platform specific junk to the connector > init (which is shared by all g4x+ platforms). > Reviewed-by: José Roberto de Souza > Signed-off-by: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 1 + > drivers/gpu/drm/i915/display/intel_dp.c | 2 +- > drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- > 3 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > b/drivers/gpu/drm/i915/display/intel_ddi.c > index 884b507c5f55..d024491738b3 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -4907,6 +4907,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, > enum port port) > encoder->port = port; > encoder->cloneable = 0; > encoder->pipe_mask = ~0; > + encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port); > > if (INTEL_GEN(dev_priv) >= 11) > intel_dig_port->saved_port_bits = intel_de_read(dev_priv, > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index 3df5d901dd9d..cd516cd8acb8 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -8211,7 +8211,6 @@ intel_dp_init_connector(struct intel_digital_port > *intel_dig_port, > if (INTEL_GEN(dev_priv) >= 11) > connector->ycbcr_420_allowed = true; > > - intel_encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port); > intel_connector->polled = DRM_CONNECTOR_POLL_HPD; > > intel_dp_aux_init(intel_dp); > @@ -8354,6 +8353,7 @@ bool intel_dp_init(struct drm_i915_private *dev_priv, > } > intel_encoder->cloneable = 0; > intel_encoder->port = port; > + intel_encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port); > > intel_dig_port->hpd_pulse = intel_dp_hpd_pulse; > > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c > b/drivers/gpu/drm/i915/display/intel_hdmi.c > index 864a1642e81c..f515d0fce968 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c > @@ -3253,7 +3253,6 @@ void intel_hdmi_init_connector(struct > intel_digital_port *intel_dig_port, > if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) > connector->ycbcr_420_allowed = true; > > - intel_encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port); > intel_connector->polled = DRM_CONNECTOR_POLL_HPD; > > if (HAS_DDI(dev_priv)) > @@ -3385,6 +3384,7 @@ void intel_hdmi_init(struct drm_i915_private *dev_priv, > intel_encoder->pipe_mask = ~0; > } > intel_encoder->cloneable = 1 << INTEL_OUTPUT_ANALOG; > + intel_encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port); > /* >* BSpec is unclear about HDMI+HDMI cloning on g4x, but it seems >* to work on real hardware. And since g4x can send infoframes to > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [12/12] drm/i915: Nuke pointless variable
On Wed, 2020-07-01 at 00:56 +0300, Ville Syrjala wrote: > From: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > > No point in assigning the function return value to a local > variable if we're just going to use it the one time. > Reviewed-by: José Roberto de Souza > Signed-off-by: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > --- > drivers/gpu/drm/i915/display/intel_hotplug.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c > b/drivers/gpu/drm/i915/display/intel_hotplug.c > index 8a8e77314a4e..938466b2c9d1 100644 > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c > @@ -480,7 +480,6 @@ void intel_hpd_irq_handler(struct drm_i915_private > *dev_priv, >* only the one of them (DP) will have ->hpd_pulse(). >*/ > for_each_intel_encoder(&dev_priv->drm, encoder) { > - bool has_hpd_pulse = intel_encoder_has_hpd_pulse(encoder); > enum port port = encoder->port; > bool long_hpd; > > @@ -488,7 +487,7 @@ void intel_hpd_irq_handler(struct drm_i915_private > *dev_priv, > if (!(BIT(pin) & pin_mask)) > continue; > > - if (!has_hpd_pulse) > + if (!intel_encoder_has_hpd_pulse(encoder)) > continue; > > long_hpd = long_mask & BIT(pin); > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 06/18] drm/dp: Add helpers to identify downstream facing port types
On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Add a few helpers to let us better identify which kind of DFP > we're dealing with. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_dp_helper.c | 60 + > include/drm/drm_dp_helper.h | 5 +++ > 2 files changed, 65 insertions(+) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c > b/drivers/gpu/drm/drm_dp_helper.c > index c21bbfc3d714..0fcb94f7dbe5 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -363,6 +363,66 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux > *aux, > } > EXPORT_SYMBOL(drm_dp_dpcd_read_link_status); > > +static bool is_edid_digital_input_dp(const struct edid *edid) > +{ > + return edid && edid->revision >= 4 && > + edid->input & DRM_EDID_INPUT_DIGITAL && > + (edid->input & DRM_EDID_DIGITAL_TYPE_MASK) == > DRM_EDID_DIGITAL_TYPE_DP; > +} > + > +/** > + * drm_dp_downstream_is_type() - is the downstream facing port of certain > type? > + * @dpcd: DisplayPort configuration data > + * @port_cap: port capabilities > + * > + * Caveat: Only works with DPCD 1.1+ port caps. > + * > + * Returns whether the downstream facing port matches the type. Nitpick: s/Returns/Returns:/ for kdoc purposes > + */ > +bool drm_dp_downstream_is_type(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > +const u8 port_cap[4], u8 type) > +{ > + return drm_dp_is_branch(dpcd) && > + dpcd[DP_DPCD_REV] >= 0x11 && > + (port_cap[0] & DP_DS_PORT_TYPE_MASK) == type; > +} > +EXPORT_SYMBOL(drm_dp_downstream_is_type); > + > +/** > + * drm_dp_downstream_is_tmds() - is the downstream facing port TMDS? > + * @dpcd: DisplayPort configuration data > + * @port_cap: port capabilities > + * @edid: EDID > + * > + * Returns whether the downstream facing port is TMDS (HDMI/DVI). Same nitpick here > + */ > +bool drm_dp_downstream_is_tmds(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > +const u8 port_cap[4], > +const struct edid *edid) > +{ > + if (dpcd[DP_DPCD_REV] < 0x11) { > + switch (dpcd[DP_DOWNSTREAMPORT_PRESENT] & > DP_DWN_STRM_PORT_TYPE_MASK) { > + case DP_DWN_STRM_PORT_TYPE_TMDS: > + return true; > + default: > + return false; > + } > + } > + > + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) { > + case DP_DS_PORT_TYPE_DP_DUALMODE: > + if (is_edid_digital_input_dp(edid)) > + return false; > + fallthrough; > + case DP_DS_PORT_TYPE_DVI: > + case DP_DS_PORT_TYPE_HDMI: > + return true; > + default: > + return false; > + } > +} > +EXPORT_SYMBOL(drm_dp_downstream_is_tmds); > + > /** > * drm_dp_send_real_edid_checksum() - send back real edid checksum value > * @aux: DisplayPort AUX channel > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index 86461a40066b..4f946826dfce 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -1638,6 +1638,11 @@ bool drm_dp_send_real_edid_checksum(struct drm_dp_aux > *aux, > int drm_dp_read_downstream_info(struct drm_dp_aux *aux, > const u8 dpcd[DP_RECEIVER_CAP_SIZE], > u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS]); > +bool drm_dp_downstream_is_type(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > +const u8 port_cap[4], u8 type); > +bool drm_dp_downstream_is_tmds(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > +const u8 port_cap[4], > +const struct edid *edid); > int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > const u8 port_cap[4]); > int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE], -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 07/18] drm/dp: Pimp drm_dp_downstream_max_bpc()
On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Deal with more cases in drm_dp_downstream_max_bpc(): > - DPCD 1.0 -> assume 8bpc for non-DP > - DPCD 1.1+ DP (or DP++ with DP sink) -> allow anything > - DPCD 1.1+ TMDS -> check the caps, assume 8bpc if the value is crap > - anything else -> assume 8bpc > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_dp_helper.c | 69 +++ > .../drm/i915/display/intel_display_debugfs.c | 3 +- > drivers/gpu/drm/i915/display/intel_dp.c | 2 +- > include/drm/drm_dp_helper.h | 10 ++- > 4 files changed, 51 insertions(+), 33 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c > b/drivers/gpu/drm/drm_dp_helper.c > index 0fcb94f7dbe5..ab87209c25d8 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -653,36 +653,44 @@ int drm_dp_downstream_max_clock(const u8 > dpcd[DP_RECEIVER_CAP_SIZE], > EXPORT_SYMBOL(drm_dp_downstream_max_clock); > > /** > - * drm_dp_downstream_max_bpc() - extract branch device max > - * bits per component > - * @dpcd: DisplayPort configuration data > - * @port_cap: port capabilities > - * > - * See also: > - * drm_dp_read_downstream_info() > - * drm_dp_downstream_max_clock() > - * > - * Returns: Max bpc on success or 0 if max bpc not defined > - */ > + * drm_dp_downstream_max_bpc() - extract downstream facing port max > + * bits per component > + * @dpcd: DisplayPort configuration data > + * @port_cap: downstream facing port capabilities > + * @edid: EDID > + * > + * Returns max bpc on success or 0 if max bpc not defined s/Returns/Returns:/ > + */ > int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > - const u8 port_cap[4]) > + const u8 port_cap[4], > + const struct edid *edid) > { > - int type = port_cap[0] & DP_DS_PORT_TYPE_MASK; > - bool detailed_cap_info = dpcd[DP_DOWNSTREAMPORT_PRESENT] & > - DP_DETAILED_CAP_INFO_AVAILABLE; > - int bpc; > - > - if (!detailed_cap_info) > + if (!drm_dp_is_branch(dpcd)) > return 0; > > - switch (type) { > - case DP_DS_PORT_TYPE_VGA: > - case DP_DS_PORT_TYPE_DVI: > - case DP_DS_PORT_TYPE_HDMI: > + if (dpcd[DP_DPCD_REV] < 0x11) { > + switch (dpcd[DP_DOWNSTREAMPORT_PRESENT] & > DP_DWN_STRM_PORT_TYPE_MASK) { > + case DP_DWN_STRM_PORT_TYPE_DP: > + return 0; > + default: > + return 8; > + } > + } > + > + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) { > + case DP_DS_PORT_TYPE_DP: > + return 0; > case DP_DS_PORT_TYPE_DP_DUALMODE: > - bpc = port_cap[2] & DP_DS_MAX_BPC_MASK; > + if (is_edid_digital_input_dp(edid)) > + return 0; > + fallthrough; > + case DP_DS_PORT_TYPE_HDMI: > + case DP_DS_PORT_TYPE_DVI: > + case DP_DS_PORT_TYPE_VGA: > + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] & > DP_DETAILED_CAP_INFO_AVAILABLE) == 0) > + return 8; > > - switch (bpc) { > + switch (port_cap[2] & DP_DS_MAX_BPC_MASK) { > case DP_DS_8BPC: > return 8; > case DP_DS_10BPC: > @@ -691,10 +699,12 @@ int drm_dp_downstream_max_bpc(const u8 > dpcd[DP_RECEIVER_CAP_SIZE], > return 12; > case DP_DS_16BPC: > return 16; > + default: > + return 8; > } > - fallthrough; > + break; > default: > - return 0; > + return 8; > } > } > EXPORT_SYMBOL(drm_dp_downstream_max_bpc); > @@ -717,12 +727,15 @@ EXPORT_SYMBOL(drm_dp_downstream_id); > * @m: pointer for debugfs file > * @dpcd: DisplayPort configuration data > * @port_cap: port capabilities > + * @edid: EDID > * @aux: DisplayPort AUX channel > * > */ > void drm_dp_downstream_debug(struct seq_file *m, >const u8 dpcd[DP_RECEIVER_CAP_SIZE], > - const u8 port_cap[4], struct drm_dp_aux *aux) > + const u8 port_cap[4], > + const struct edid *edid, > + struct drm_dp_aux *aux) > { > bool detailed_cap_info = dpcd[DP_DOWNSTREAMPORT_PRESENT] & >DP_DETAILED_CAP_INFO_AVAILABLE; > @@ -789,7 +802,7 @@ void drm_dp_downstream_debug(struct seq_file *m, > seq_printf(m, "\t\tMax TMDS clock: %d kHz\n", > clk); > } > > - bpc = drm_dp_downstream_max_bpc(dpcd, port_cap); > + bpc = drm_dp_downstream_max_bpc(dpcd, port_cap, edid); > > if (bpc > 0) >
Re: [Intel-gfx] [PATCH v2 07/18] drm/dp: Pimp drm_dp_downstream_max_bpc()
On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Deal with more cases in drm_dp_downstream_max_bpc(): > - DPCD 1.0 -> assume 8bpc for non-DP > - DPCD 1.1+ DP (or DP++ with DP sink) -> allow anything > - DPCD 1.1+ TMDS -> check the caps, assume 8bpc if the value is crap > - anything else -> assume 8bpc > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_dp_helper.c | 69 +++ > .../drm/i915/display/intel_display_debugfs.c | 3 +- > drivers/gpu/drm/i915/display/intel_dp.c | 2 +- > include/drm/drm_dp_helper.h | 10 ++- > 4 files changed, 51 insertions(+), 33 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c > b/drivers/gpu/drm/drm_dp_helper.c > index 0fcb94f7dbe5..ab87209c25d8 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -653,36 +653,44 @@ int drm_dp_downstream_max_clock(const u8 > dpcd[DP_RECEIVER_CAP_SIZE], > EXPORT_SYMBOL(drm_dp_downstream_max_clock); > > /** > - * drm_dp_downstream_max_bpc() - extract branch device max > - * bits per component > - * @dpcd: DisplayPort configuration data > - * @port_cap: port capabilities > - * > - * See also: > - * drm_dp_read_downstream_info() > - * drm_dp_downstream_max_clock() > - * > - * Returns: Max bpc on success or 0 if max bpc not defined > - */ > + * drm_dp_downstream_max_bpc() - extract downstream facing port max > + * bits per component > + * @dpcd: DisplayPort configuration data > + * @port_cap: downstream facing port capabilities > + * @edid: EDID > + * > + * Returns max bpc on success or 0 if max bpc not defined > + */ > int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > - const u8 port_cap[4]) > + const u8 port_cap[4], > + const struct edid *edid) > { > - int type = port_cap[0] & DP_DS_PORT_TYPE_MASK; > - bool detailed_cap_info = dpcd[DP_DOWNSTREAMPORT_PRESENT] & > - DP_DETAILED_CAP_INFO_AVAILABLE; > - int bpc; > - > - if (!detailed_cap_info) I don't think we can drop this check. There's a somewhat surprising blurb about downstream port caps in the DP 2.0 spec (section 5.3.3.1): In addition, the adapter shall set the Detailed Capabilities Info registers (DPCD Addresses 00080h through 0008Fh) to show all the downstream types, including DFP 0. Either one or four bytes are used, per DFP type indication. Therefore, up to 16 (with 1-byte descriptor) or four (with 4- byte descriptor) DFP capabilities can be stored. I've never once actually seen a sink do this, but this does mean it's technically possible tthat if we don't check the detailed caps bit then we might end up reading another port's DFP type instead of max_bpc info. Note though that we can make the assumption the four byte version of the field is used for DP 1.4+ > + if (!drm_dp_is_branch(dpcd)) > return 0; > > - switch (type) { > - case DP_DS_PORT_TYPE_VGA: > - case DP_DS_PORT_TYPE_DVI: > - case DP_DS_PORT_TYPE_HDMI: > + if (dpcd[DP_DPCD_REV] < 0x11) { > + switch (dpcd[DP_DOWNSTREAMPORT_PRESENT] & > DP_DWN_STRM_PORT_TYPE_MASK) { > + case DP_DWN_STRM_PORT_TYPE_DP: > + return 0; > + default: > + return 8; > + } > + } > + > + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) { > + case DP_DS_PORT_TYPE_DP: > + return 0; > case DP_DS_PORT_TYPE_DP_DUALMODE: > - bpc = port_cap[2] & DP_DS_MAX_BPC_MASK; > + if (is_edid_digital_input_dp(edid)) > + return 0; > + fallthrough; > + case DP_DS_PORT_TYPE_HDMI: > + case DP_DS_PORT_TYPE_DVI: > + case DP_DS_PORT_TYPE_VGA: > + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] & > DP_DETAILED_CAP_INFO_AVAILABLE) == 0) > + return 8; > > - switch (bpc) { > + switch (port_cap[2] & DP_DS_MAX_BPC_MASK) { > case DP_DS_8BPC: > return 8; > case DP_DS_10BPC: > @@ -691,10 +699,12 @@ int drm_dp_downstream_max_bpc(const u8 > dpcd[DP_RECEIVER_CAP_SIZE], > return 12; > case DP_DS_16BPC: > return 16; > + default: > + return 8; > } > - fallthrough; > + break; > default: > - return 0; > + return 8; > } > } > EXPORT_SYMBOL(drm_dp_downstream_max_bpc); > @@ -717,12 +727,15 @@ EXPORT_SYMBOL(drm_dp_downstream_id); > * @m: pointer for debugfs file > * @dpcd: DisplayPort configuration data > * @port_cap: port capabilities > + * @edid: EDID > * @aux: DisplayPort AUX channel > * > */ > void drm_dp_downstream_debug(struct seq_file *m, >
Re: [Intel-gfx] [PATCH v2 08/18] drm/dp: Redo drm_dp_downstream_max_clock() as drm_dp_downstream_max_dotclock()
BTW - we started using drm_dp_downstream_max_clock() in nouveau, so you'll need to update the function call there too. On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > We want to differentiate between the DFP dotclock and TMDS clock > limits. Let's convert the current thing to just give us the > dotclock limit. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_dp_helper.c | 46 + > drivers/gpu/drm/i915/display/intel_dp.c | 4 +-- > include/drm/drm_dp_helper.h | 4 +-- > 3 files changed, 20 insertions(+), 34 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c > b/drivers/gpu/drm/drm_dp_helper.c > index ab87209c25d8..822a30e609ef 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -616,41 +616,32 @@ int drm_dp_read_downstream_info(struct drm_dp_aux > *aux, > EXPORT_SYMBOL(drm_dp_read_downstream_info); > > /** > - * drm_dp_downstream_max_clock() - extract branch device max > - * pixel rate for legacy VGA > - * converter or max TMDS clock > - * rate for others > + * drm_dp_downstream_max_dotclock() - extract downstream facing port max > dot clock > * @dpcd: DisplayPort configuration data > * @port_cap: port capabilities > * > - * See also: > - * drm_dp_read_downstream_info() > - * drm_dp_downstream_max_bpc() > - * > - * Returns: Max clock in kHz on success or 0 if max clock not defined > + * Returns downstream facing port max dot clock in kHz on success, > + * or 0 if max clock not defined > */ > -int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > - const u8 port_cap[4]) > +int drm_dp_downstream_max_dotclock(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > +const u8 port_cap[4]) > { > - int type = port_cap[0] & DP_DS_PORT_TYPE_MASK; > - bool detailed_cap_info = dpcd[DP_DOWNSTREAMPORT_PRESENT] & > - DP_DETAILED_CAP_INFO_AVAILABLE; > + if (!drm_dp_is_branch(dpcd)) > + return 0; > > - if (!detailed_cap_info) > + if (dpcd[DP_DPCD_REV] < 0x11) > return 0; > > - switch (type) { > + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) { > case DP_DS_PORT_TYPE_VGA: > - return port_cap[1] * 8 * 1000; > - case DP_DS_PORT_TYPE_DVI: > - case DP_DS_PORT_TYPE_HDMI: > - case DP_DS_PORT_TYPE_DP_DUALMODE: > - return port_cap[1] * 2500; > + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] & > DP_DETAILED_CAP_INFO_AVAILABLE) == 0) > + return 0; > + return port_cap[1] * 8000; > default: > return 0; > } > } > -EXPORT_SYMBOL(drm_dp_downstream_max_clock); > +EXPORT_SYMBOL(drm_dp_downstream_max_dotclock); > > /** >* drm_dp_downstream_max_bpc() - extract downstream facing port max > @@ -793,14 +784,9 @@ void drm_dp_downstream_debug(struct seq_file *m, > seq_printf(m, "\t\tSW: %d.%d\n", rev[0], rev[1]); > > if (detailed_cap_info) { > - clk = drm_dp_downstream_max_clock(dpcd, port_cap); > - > - if (clk > 0) { > - if (type == DP_DS_PORT_TYPE_VGA) > - seq_printf(m, "\t\tMax dot clock: %d kHz\n", > clk); > - else > - seq_printf(m, "\t\tMax TMDS clock: %d kHz\n", > clk); > - } > + clk = drm_dp_downstream_max_dotclock(dpcd, port_cap); > + if (clk > 0) > + seq_printf(m, "\t\tMax dot clock: %d kHz\n", clk); > > bpc = drm_dp_downstream_max_bpc(dpcd, port_cap, edid); > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index c73b3efd84e0..8f4aee35c203 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -261,8 +261,8 @@ intel_dp_downstream_max_dotclock(struct intel_dp > *intel_dp) > if (type != DP_DS_PORT_TYPE_VGA) > return max_dotclk; > > - ds_max_dotclk = drm_dp_downstream_max_clock(intel_dp->dpcd, > - intel_dp- > >downstream_ports); > + ds_max_dotclk = drm_dp_downstream_max_dotclock(intel_dp->dpcd, > +intel_dp- > >downstream_ports); > > if (ds_max_dotclk != 0) > max_dotclk = min(max_dotclk, ds_max_dotclk); > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index 6218de1294c1..19bc04207788 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -1643,8 +1643,8 @@ bool drm_dp_downstream_is_type(const u8 > dpcd[DP_RECEIVER_CAP_SIZE], > bool drm_dp_downstream_is_tmds(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > const u8 po
Re: [Intel-gfx] [PATCH v2 10/18] drm/dp: Add drm_dp_downstream_{min, max}_tmds_clock()
On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Add helpers to get the TMDS clock limits for HDMI/DVI downstream > facing ports. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_dp_helper.c | 116 > include/drm/drm_dp_helper.h | 6 ++ > 2 files changed, 122 insertions(+) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c > b/drivers/gpu/drm/drm_dp_helper.c > index 822a30e609ef..f567428f2aef 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -643,6 +643,114 @@ int drm_dp_downstream_max_dotclock(const u8 > dpcd[DP_RECEIVER_CAP_SIZE], > } > EXPORT_SYMBOL(drm_dp_downstream_max_dotclock); > > +/** > + * drm_dp_downstream_max_tmds_clock() - extract downstream facing port max > TMDS clock > + * @dpcd: DisplayPort configuration data > + * @port_cap: port capabilities > + * @edid: EDID > + * > + * Returns HDMI/DVI downstream facing port max TMDS clock in kHz on > success, > + * or 0 if max TMDS clock not defined > + */ > +int drm_dp_downstream_max_tmds_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > + const u8 port_cap[4], > + const struct edid *edid) > +{ > + if (!drm_dp_is_branch(dpcd)) > + return 0; > + > + if (dpcd[DP_DPCD_REV] < 0x11) { > + switch (dpcd[DP_DOWNSTREAMPORT_PRESENT] & > DP_DWN_STRM_PORT_TYPE_MASK) { > + case DP_DWN_STRM_PORT_TYPE_TMDS: > + return 165000; > + default: > + return 0; > + } > + } > + > + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) { > + case DP_DS_PORT_TYPE_DP_DUALMODE: > + if (is_edid_digital_input_dp(edid)) > + return 0; > + /* > + * It's left up to the driver to check the > + * DP dual mode adapter's max TMDS clock. > + * > + * Unfortunatley it looks like branch devices > + * may not fordward that the DP dual mode i2c > + * access so we just usually get i2c nak :( > + */ > + fallthrough; > + case DP_DS_PORT_TYPE_HDMI: > + /* > + * We should perhaps assume 165 MHz when detailed cap > + * info is not available. But looks like many typical > + * branch devices fall into that category and so we'd > + * probably end up with users complaining that they can't > + * get high resolution modes with their favorite dongle. > + * > + * So let's limit to 300 MHz instead since DPCD 1.4 > + * HDMI 2.0 DFPs are required to have the detailed cap > + * info. So it's more likely we're dealing with a HDMI 1.4 > + * compatible* device here. > + */ > + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] & > DP_DETAILED_CAP_INFO_AVAILABLE) == 0) > + return 30; > + return port_cap[1] * 2500; > + case DP_DS_PORT_TYPE_DVI: > + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] & > DP_DETAILED_CAP_INFO_AVAILABLE) == 0) > + return 165000; > + /* FIXME what to do about DVI dual link? */ > + return port_cap[1] * 2500; > + default: > + return 0; > + } > +} > +EXPORT_SYMBOL(drm_dp_downstream_max_tmds_clock); > + > +/** > + * drm_dp_downstream_min_tmds_clock() - extract downstream facing port min > TMDS clock > + * @dpcd: DisplayPort configuration data > + * @port_cap: port capabilities > + * @edid: EDID > + * > + * Returns HDMI/DVI downstream facing port min TMDS clock in kHz on > success, > + * or 0 if max TMDS clock not defined s/max/min/ Also, I would assume if callers are interested in min they're also interested in max and vice versa, would it maybe make sense to combine the min/max functions here? Also, we should probably note the existence of this function in the max dotclock functions and vice-versa > + */ > +int drm_dp_downstream_min_tmds_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > + const u8 port_cap[4], > + const struct edid *edid) > +{ > + if (!drm_dp_is_branch(dpcd)) > + return 0; > + > + if (dpcd[DP_DPCD_REV] < 0x11) { > + switch (dpcd[DP_DOWNSTREAMPORT_PRESENT] & > DP_DWN_STRM_PORT_TYPE_MASK) { > + case DP_DWN_STRM_PORT_TYPE_TMDS: > + return 25000; > + default: > + return 0; > + } > + } > + > + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) { > + case DP_DS_PORT_TYPE_DP_DUALMODE: > + if (is_edid_digital_input_dp(edid)) > + return 0; > + fallthrough; > + case DP_DS_PORT_TYPE_DVI: > + case DP_DS_PORT_TYPE_HDMI: > + /* > +
Re: [Intel-gfx] [PATCH v2 10/18] drm/dp: Add drm_dp_downstream_{min, max}_tmds_clock()
On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Add helpers to get the TMDS clock limits for HDMI/DVI downstream > facing ports. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_dp_helper.c | 116 > include/drm/drm_dp_helper.h | 6 ++ > 2 files changed, 122 insertions(+) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c > b/drivers/gpu/drm/drm_dp_helper.c > index 822a30e609ef..f567428f2aef 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -643,6 +643,114 @@ int drm_dp_downstream_max_dotclock(const u8 > dpcd[DP_RECEIVER_CAP_SIZE], > } > EXPORT_SYMBOL(drm_dp_downstream_max_dotclock); > > +/** > + * drm_dp_downstream_max_tmds_clock() - extract downstream facing port max > TMDS clock > + * @dpcd: DisplayPort configuration data > + * @port_cap: port capabilities > + * @edid: EDID > + * > + * Returns HDMI/DVI downstream facing port max TMDS clock in kHz on > success, > + * or 0 if max TMDS clock not defined > + */ > +int drm_dp_downstream_max_tmds_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > + const u8 port_cap[4], > + const struct edid *edid) > +{ > + if (!drm_dp_is_branch(dpcd)) > + return 0; > + > + if (dpcd[DP_DPCD_REV] < 0x11) { > + switch (dpcd[DP_DOWNSTREAMPORT_PRESENT] & > DP_DWN_STRM_PORT_TYPE_MASK) { > + case DP_DWN_STRM_PORT_TYPE_TMDS: > + return 165000; > + default: > + return 0; > + } > + } > + > + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) { > + case DP_DS_PORT_TYPE_DP_DUALMODE: > + if (is_edid_digital_input_dp(edid)) > + return 0; > + /* > + * It's left up to the driver to check the > + * DP dual mode adapter's max TMDS clock. > + * > + * Unfortunatley it looks like branch devices > + * may not fordward that the DP dual mode i2c > + * access so we just usually get i2c nak :( > + */ > + fallthrough; > + case DP_DS_PORT_TYPE_HDMI: > + /* > + * We should perhaps assume 165 MHz when detailed cap > + * info is not available. But looks like many typical > + * branch devices fall into that category and so we'd > + * probably end up with users complaining that they can't > + * get high resolution modes with their favorite dongle. > + * > + * So let's limit to 300 MHz instead since DPCD 1.4 > + * HDMI 2.0 DFPs are required to have the detailed cap > + * info. So it's more likely we're dealing with a HDMI 1.4 > + * compatible* device here. Forgot to mention - not directly related to this series, there's some hidden i2c bits that I think can also be probed for this sort of information on passive adapters, I know amdgpu actually supports this. I wonder how many of them also apply to older active adapters... > + */ > + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] & > DP_DETAILED_CAP_INFO_AVAILABLE) == 0) > + return 30; > + return port_cap[1] * 2500; > + case DP_DS_PORT_TYPE_DVI: > + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] & > DP_DETAILED_CAP_INFO_AVAILABLE) == 0) > + return 165000; > + /* FIXME what to do about DVI dual link? */ > + return port_cap[1] * 2500; > + default: > + return 0; > + } > +} > +EXPORT_SYMBOL(drm_dp_downstream_max_tmds_clock); > + > +/** > + * drm_dp_downstream_min_tmds_clock() - extract downstream facing port min > TMDS clock > + * @dpcd: DisplayPort configuration data > + * @port_cap: port capabilities > + * @edid: EDID > + * > + * Returns HDMI/DVI downstream facing port min TMDS clock in kHz on > success, > + * or 0 if max TMDS clock not defined > + */ > +int drm_dp_downstream_min_tmds_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > + const u8 port_cap[4], > + const struct edid *edid) > +{ > + if (!drm_dp_is_branch(dpcd)) > + return 0; > + > + if (dpcd[DP_DPCD_REV] < 0x11) { > + switch (dpcd[DP_DOWNSTREAMPORT_PRESENT] & > DP_DWN_STRM_PORT_TYPE_MASK) { > + case DP_DWN_STRM_PORT_TYPE_TMDS: > + return 25000; > + default: > + return 0; > + } > + } > + > + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) { > + case DP_DS_PORT_TYPE_DP_DUALMODE: > + if (is_edid_digital_input_dp(edid)) > + return 0; > + fallthrough; > + case DP_DS_PORT_TYPE_DVI: > + case DP_DS_PORT_TYPE_HDMI: > + /* > + * Unclear wh
Re: [Intel-gfx] [PATCH v2 12/18] drm/i915: Configure DP 1.3+ protocol converted HDMI mode
On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > DP 1.3 adds some extra control knobs for DP->HDMI protocol conversion. > Let's use that to configure the "HDMI mode" (ie. infoframes vs. not) > based on the capabilities of the sink. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 1 + > drivers/gpu/drm/i915/display/intel_dp.c | 23 +++ > drivers/gpu/drm/i915/display/intel_dp.h | 1 + > 3 files changed, 25 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > b/drivers/gpu/drm/i915/display/intel_ddi.c > index 28ff85493f25..9adba0d0b4aa 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -3470,6 +3470,7 @@ static void hsw_ddi_pre_enable_dp(struct > intel_atomic_state *state, > intel_ddi_init_dp_buf_reg(encoder); > if (!is_mst) > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > + intel_dp_configure_protocol_converter(intel_dp); > intel_dp_sink_set_decompression_state(intel_dp, crtc_state, > true); > intel_dp_sink_set_fec_ready(intel_dp, crtc_state); > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index a703e4659e47..047449253a54 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -3792,6 +3792,28 @@ static void intel_dp_enable_port(struct intel_dp > *intel_dp, > intel_de_posting_read(dev_priv, intel_dp->output_reg); > } > > +void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp) > +{ > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > + u8 tmp; > + > + if (intel_dp->dpcd[DP_DPCD_REV] < 0x13) > + return; > + > + if (!drm_dp_is_branch(intel_dp->dpcd)) > + return; > + > + tmp = intel_dp->has_hdmi_sink ? > + DP_HDMI_DVI_OUTPUT_CONFIG : 0; > + > + if (drm_dp_dpcd_writeb(&intel_dp->aux, > +DP_PROTOCOL_CONVERTER_CONTROL_0, tmp) <= 0) > + drm_dbg_kms(&i915->drm, "Failed to set protocol converter HDMI > mode to %s\n", > + enableddisabled(intel_dp->has_hdmi_sink)); > + > + /* TODO: configure YCbCr 4:2:2/4:2:0 conversion */ > +} Maybe add a DP helper for this while we're at it? Up to you > + > static void intel_enable_dp(struct intel_atomic_state *state, > struct intel_encoder *encoder, > const struct intel_crtc_state *pipe_config, > @@ -3829,6 +3851,7 @@ static void intel_enable_dp(struct intel_atomic_state > *state, > } > > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > + intel_dp_configure_protocol_converter(intel_dp); > intel_dp_start_link_train(intel_dp); > intel_dp_stop_link_train(intel_dp); > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h > b/drivers/gpu/drm/i915/display/intel_dp.h > index ec5688a21f66..08a1c0aa8b94 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.h > +++ b/drivers/gpu/drm/i915/display/intel_dp.h > @@ -51,6 +51,7 @@ int intel_dp_get_link_train_fallback_values(struct > intel_dp *intel_dp, > int intel_dp_retrain_link(struct intel_encoder *encoder, > struct drm_modeset_acquire_ctx *ctx); > void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode); > +void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp); > void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp, > const struct intel_crtc_state > *crtc_state, > bool enable); -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915/gvt: Prevent NULL pointer dereference in intel_vgpu_reg_rw_edid()
In the function intel_vgpu_reg_rw_edid of kvmgt.c, pos can be equal to NULL for GPUs that do not properly support EDID. In those cases, when pos gets passed to the handle_edid functions, it gets added a short offset then it's dereferenced in memcpy's, leading to NULL pointer dereference kernel oops. More concretely, that kernel oops renders some Broadwell GPUs users unable to set up virtual machines with virtual GPU passthrough (virtual machines hang indefinitely when trying to make use of the virtual GPU), and make them unable to remove the virtual GPUs once the kernel oops has happened (it hangs indefinitely, and notably too when the kernel tries to shutdown). The issues that this causes and steps to reproduce are discussed in more details in this github issue post: https://github.com/intel/gvt-linux/issues/170#issuecomment-685806160 Check if pos is equal to NULL, and if it is, set ret to a negative value, making the module simply indicate that the access to EDID region has failed, without any fatal repercussion. Signed-off-by: Alejandro Sior --- Changes in v2: - removed middle name of author to comply with git name - rephrased the patch description with imperative phrasing - removed useless paragraph - made a paragraph more concise - fixed typos - made individual lines shorter than 75 chars drivers/gpu/drm/i915/gvt/kvmgt.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index ad8a9df49f29..49163363ba4a 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -557,7 +557,9 @@ static size_t intel_vgpu_reg_rw_edid(struct intel_vgpu *vgpu, char *buf, (struct vfio_edid_region *)kvmgt_vdev(vgpu)->region[i].data; loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK; - if (pos < region->vfio_edid_regs.edid_offset) { + if (pos == NULL) { + ret = -EINVAL; + } else if (pos < region->vfio_edid_regs.edid_offset) { ret = handle_edid_regs(vgpu, region, buf, count, pos, iswrite); } else { pos -= EDID_BLOB_OFFSET; -- 2.28.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 13/18] drm/dp: Add drm_dp_downstream_mode()
On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > The downstream facing port caps in the DPCD can give us a hint > as to what kind of display mode the sink can use if it doesn't > have an EDID. Use that information to pick a suitable mode. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_dp_helper.c | 54 + > drivers/gpu/drm/drm_edid.c | 19 > include/drm/drm_dp_helper.h | 12 > include/drm/drm_edid.h | 4 +++ > 4 files changed, 89 insertions(+) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c > b/drivers/gpu/drm/drm_dp_helper.c > index f567428f2aef..0d5e9bcf11d0 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -808,6 +808,60 @@ int drm_dp_downstream_max_bpc(const u8 > dpcd[DP_RECEIVER_CAP_SIZE], > } > EXPORT_SYMBOL(drm_dp_downstream_max_bpc); > > +/** > + * drm_dp_downstream_mode() - return a mode for downstream facing port > + * @dpcd: DisplayPort configuration data > + * @port_cap: port capabilities > + * > + * Provides a suitable mode for downstream facing ports without EDID. > + * > + * Returns a new drm_display_mode on success or NULL on failure > + */ > +struct drm_display_mode * > +drm_dp_downstream_mode(struct drm_device *dev, > +const u8 dpcd[DP_RECEIVER_CAP_SIZE], > +const u8 port_cap[4]) > + > +{ > + u8 vic; > + > + if (!drm_dp_is_branch(dpcd)) > + return NULL; > + > + if (dpcd[DP_DPCD_REV] < 0x11) > + return NULL; > + > + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) { > + case DP_DS_PORT_TYPE_NON_EDID: > + switch (port_cap[0] & DP_DS_NON_EDID_MASK) { > + case DP_DS_NON_EDID_720x480i_60: > + vic = 6; > + break; > + case DP_DS_NON_EDID_720x480i_50: > + vic = 21; > + break; > + case DP_DS_NON_EDID_1920x1080i_60: > + vic = 5; > + break; > + case DP_DS_NON_EDID_1920x1080i_50: > + vic = 20; > + break; > + case DP_DS_NON_EDID_1280x720_60: > + vic = 4; > + break; > + case DP_DS_NON_EDID_1280x720_50: > + vic = 19; > + break; > + default: > + return NULL; > + } > + return drm_display_mode_from_cea_vic(dev, vic); > + default: > + return NULL; > + } > +} > +EXPORT_SYMBOL(drm_dp_downstream_mode); > + > /** > * drm_dp_downstream_id() - identify branch device > * @aux: DisplayPort AUX channel > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 6840f0530a38..b9419fed6c28 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -3738,6 +3738,25 @@ drm_add_cmdb_modes(struct drm_connector *connector, > u8 svd) > bitmap_set(hdmi->y420_cmdb_modes, vic, 1); > } > > +struct drm_display_mode * > +drm_display_mode_from_cea_vic(struct drm_device *dev, > + u8 video_code) > +{ > + const struct drm_display_mode *cea_mode; > + struct drm_display_mode *newmode; > + > + cea_mode = cea_mode_for_vic(video_code); > + if (!cea_mode) > + return NULL; > + > + newmode = drm_mode_duplicate(dev, cea_mode); > + if (!newmode) > + return NULL; > + > + return newmode; > +} > +EXPORT_SYMBOL(drm_display_mode_from_cea_vic); Forgot the kdocs > + > static int > do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len) > { > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index 6812a3e0de8d..fbba4a0f7366 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -28,6 +28,8 @@ > #include > #include > > +struct drm_device; > + > /* > * Unless otherwise noted, all values are from the DP 1.1a spec. Note that > * DP and DPCD versions are independent. Differences from 1.0 are not > noted, > @@ -385,6 +387,13 @@ > # define DP_DS_PORT_TYPE_DP_DUALMODE5 > # define DP_DS_PORT_TYPE_WIRELESS 6 > # define DP_DS_PORT_HPD (1 << 3) > +# define DP_DS_NON_EDID_MASK (0xf << 4) > +# define DP_DS_NON_EDID_720x480i_60 (1 << 4) > +# define DP_DS_NON_EDID_720x480i_50 (2 << 4) > +# define DP_DS_NON_EDID_1920x1080i_60(3 << 4) > +# define DP_DS_NON_EDID_1920x1080i_50(4 << 4) > +# define DP_DS_NON_EDID_1280x720_60 (5 << 4) > +# define DP_DS_NON_EDID_1280x720_50 (7 << 4) > /* offset 1 for VGA is maximum megapixels per second / 8 */ > /* offset 1 for DVI/HDMI is maximum TMDS clock in Mbps / 2.5 */ > /* offset 2 for VGA/DVI/HDMI */ > @@ -1654,6 +1663,9 @@ int drm_dp_downstream_min_tmds_clock(const u8 > dpcd[DP_R
Re: [Intel-gfx] [PATCH v2 17/18] drm/dp: Add helpers for DFP YCbCr 4:2:0 handling
On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Add helpers to determine whether the DFP supports > YCbCr 4:2:0 passthrough or YCbCr 4:4:4->4:2:0 conversion. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_dp_helper.c | 44 + > include/drm/drm_dp_helper.h | 8 ++ > 2 files changed, 52 insertions(+) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c > b/drivers/gpu/drm/drm_dp_helper.c > index 0d5e9bcf11d0..dc68e10aa1fd 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -808,6 +808,50 @@ int drm_dp_downstream_max_bpc(const u8 > dpcd[DP_RECEIVER_CAP_SIZE], > } > EXPORT_SYMBOL(drm_dp_downstream_max_bpc); > > +bool drm_dp_downstream_420_passthrough(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > +const u8 port_cap[4]) > +{ > + if (!drm_dp_is_branch(dpcd)) > + return false; > + > + if (dpcd[DP_DPCD_REV] < 0x13) > + return false; > + > + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) { > + case DP_DS_PORT_TYPE_DP: > + return true; > + case DP_DS_PORT_TYPE_HDMI: > + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] & > DP_DETAILED_CAP_INFO_AVAILABLE) == 0) > + return false; > + > + return port_cap[3] & DP_DS_HDMI_YCBCR420_PASS_THROUGH; > + default: > + return false; > + } > +} > +EXPORT_SYMBOL(drm_dp_downstream_420_passthrough); Forgot the kdocs again > + > +bool drm_dp_downstream_444_to_420_conversion(const u8 > dpcd[DP_RECEIVER_CAP_SIZE], > + const u8 port_cap[4]) > +{ > + if (!drm_dp_is_branch(dpcd)) > + return false; > + > + if (dpcd[DP_DPCD_REV] < 0x13) > + return false; > + > + switch (port_cap[0] & DP_DS_PORT_TYPE_MASK) { > + case DP_DS_PORT_TYPE_HDMI: > + if ((dpcd[DP_DOWNSTREAMPORT_PRESENT] & > DP_DETAILED_CAP_INFO_AVAILABLE) == 0) > + return false; > + > + return port_cap[3] & DP_DS_HDMI_YCBCR444_TO_420_CONV; > + default: > + return false; > + } > +} > +EXPORT_SYMBOL(drm_dp_downstream_444_to_420_conversion); > + > /** > * drm_dp_downstream_mode() - return a mode for downstream facing port > * @dpcd: DisplayPort configuration data > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index fbba4a0f7366..c9f2851904d0 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -407,6 +407,10 @@ struct drm_device; > # define DP_DS_DVI_HIGH_COLOR_DEPTH (1 << 2) > /* offset 3 for HDMI */ > # define DP_DS_HDMI_FRAME_SEQ_TO_FRAME_PACK (1 << 0) > +# define DP_DS_HDMI_YCBCR422_PASS_THROUGH (1 << 1) > +# define DP_DS_HDMI_YCBCR420_PASS_THROUGH (1 << 2) > +# define DP_DS_HDMI_YCBCR444_TO_422_CONV(1 << 3) > +# define DP_DS_HDMI_YCBCR444_TO_420_CONV(1 << 4) > > #define DP_MAX_DOWNSTREAM_PORTS 0x10 > > @@ -1663,6 +1667,10 @@ int drm_dp_downstream_min_tmds_clock(const u8 > dpcd[DP_RECEIVER_CAP_SIZE], > int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > const u8 port_cap[4], > const struct edid *edid); > +bool drm_dp_downstream_420_passthrough(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > +const u8 port_cap[4]); > +bool drm_dp_downstream_444_to_420_conversion(const u8 > dpcd[DP_RECEIVER_CAP_SIZE], > + const u8 port_cap[4]); > struct drm_display_mode *drm_dp_downstream_mode(struct drm_device *dev, > const u8 > dpcd[DP_RECEIVER_CAP_SIZE], > const u8 port_cap[4]); -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gvt: Prevent NULL pointer dereference in intel_vgpu_reg_rw_edid()
== Series Details == Series: drm/i915/gvt: Prevent NULL pointer dereference in intel_vgpu_reg_rw_edid() URL : https://patchwork.freedesktop.org/series/81470/ State : warning == Summary == $ dim checkpatch origin/drm-tip cc442c9d2d41 drm/i915/gvt: Prevent NULL pointer dereference in intel_vgpu_reg_rw_edid() -:37: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "!pos" #37: FILE: drivers/gpu/drm/i915/gvt/kvmgt.c:560: + if (pos == NULL) { total: 0 errors, 0 warnings, 1 checks, 10 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 00/18] drm/i915: Pimp DP DFP handling
With the nitpicks addressed (note there were a couple of other spots where we wanted to use Return: in the kdocs, but I didn't bother pointing all of them out), all but patch 07 is: Reviewed-by: Lyude Paul On Fri, 2020-09-04 at 14:53 +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Attempt to deal with DP downstream facing ports (DFP) more > thoroughly. This involves reading more of the port caps > and dealing with various clock/bpc limitations. > > And we try to enable YCbCr 444->420 conversion for HDMI DFPs > which could allow some 4k displays to actually use 4k on > pre-icl hardware (which doesn't have native 420 output), > assuming we don't run into some other hardware limits. > > I dropped my earlier patches to also hook in the DP dual mode > adapter probing since sadly I've not actually seen a DP->DP++ > dongle that passes through the i2c traffic for those. > > Only pimped the SST side of things. Not sure what would > be required to get it all working for MST. > > Ville Syrjälä (18): > drm/dp: Dump downstream facing port caps > drm/i915/lspcon: Do not send infoframes to non-HDMI sinks > drm/dp: Define protocol converter DPCD registers > drm/dp: Define more downstream facing port caps > drm/i915: Reworkd DFP max bpc handling > drm/dp: Add helpers to identify downstream facing port types > drm/dp: Pimp drm_dp_downstream_max_bpc() > drm/dp: Redo drm_dp_downstream_max_clock() as > drm_dp_downstream_max_dotclock() > drm/i915: Reworkd DP DFP clock handling > drm/dp: Add drm_dp_downstream_{min,max}_tmds_clock() > drm/i915: Deal with TMDS DFP clock limits > drm/i915: Configure DP 1.3+ protocol converted HDMI mode > drm/dp: Add drm_dp_downstream_mode() > drm/i915: Handle downstream facing ports w/o EDID > drm/i915: Extract intel_hdmi_has_audio() > drm/i915: DP->HDMI TMDS clock limits vs. deep color > drm/dp: Add helpers for DFP YCbCr 4:2:0 handling > drm/i915: Do YCbCr 444->420 conversion via DP protocol converters > > drivers/gpu/drm/drm_dp_helper.c | 382 +++--- > drivers/gpu/drm/drm_edid.c| 19 + > drivers/gpu/drm/i915/display/intel_ddi.c | 11 +- > .../drm/i915/display/intel_display_debugfs.c | 3 +- > .../drm/i915/display/intel_display_types.h| 9 + > drivers/gpu/drm/i915/display/intel_dp.c | 304 +++--- > drivers/gpu/drm/i915/display/intel_dp.h | 1 + > drivers/gpu/drm/i915/display/intel_hdmi.c | 82 ++-- > drivers/gpu/drm/i915/display/intel_hdmi.h | 2 + > include/drm/drm_dp_helper.h | 63 ++- > include/drm/drm_edid.h| 4 + > 11 files changed, 738 insertions(+), 142 deletions(-) > -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gvt: Prevent NULL pointer dereference in intel_vgpu_reg_rw_edid()
== Series Details == Series: drm/i915/gvt: Prevent NULL pointer dereference in intel_vgpu_reg_rw_edid() URL : https://patchwork.freedesktop.org/series/81470/ State : success == Summary == CI Bug Log - changes from CI_DRM_8982 -> Patchwork_18455 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/index.html Known issues Here are the changes found in Patchwork_18455 that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_gttfill@basic: - fi-kbl-7500u: [PASS][1] -> [INCOMPLETE][2] ([i915#2439]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-kbl-7500u/igt@gem_exec_gttf...@basic.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-kbl-7500u/igt@gem_exec_gttf...@basic.html * igt@i915_selftest@live@gem_execbuf: - fi-bsw-nick:[PASS][3] -> [INCOMPLETE][4] ([i915#2439]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bsw-nick/igt@i915_selftest@live@gem_execbuf.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-bsw-nick/igt@i915_selftest@live@gem_execbuf.html Possible fixes * igt@gem_exec_gttfill@basic: - fi-cfl-8109u: [INCOMPLETE][5] ([i915#2439]) -> [PASS][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-cfl-8109u/igt@gem_exec_gttf...@basic.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-cfl-8109u/igt@gem_exec_gttf...@basic.html - fi-bdw-5557u: [INCOMPLETE][7] ([i915#2439]) -> [PASS][8] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bdw-5557u/igt@gem_exec_gttf...@basic.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-bdw-5557u/igt@gem_exec_gttf...@basic.html - fi-skl-6600u: [INCOMPLETE][9] ([i915#2439]) -> [PASS][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-skl-6600u/igt@gem_exec_gttf...@basic.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-skl-6600u/igt@gem_exec_gttf...@basic.html - fi-elk-e7500: [INCOMPLETE][11] ([i915#66]) -> [PASS][12] [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-elk-e7500/igt@gem_exec_gttf...@basic.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-elk-e7500/igt@gem_exec_gttf...@basic.html - fi-skl-lmem:[INCOMPLETE][13] ([i915#2439]) -> [PASS][14] [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-skl-lmem/igt@gem_exec_gttf...@basic.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-skl-lmem/igt@gem_exec_gttf...@basic.html - fi-ilk-650: [INCOMPLETE][15] ([i915#2439]) -> [PASS][16] [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-ilk-650/igt@gem_exec_gttf...@basic.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-ilk-650/igt@gem_exec_gttf...@basic.html * igt@gem_exec_parallel@engines@basic: - fi-bsw-kefka: [INCOMPLETE][17] ([i915#2439]) -> [PASS][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bsw-kefka/igt@gem_exec_parallel@engi...@basic.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-bsw-kefka/igt@gem_exec_parallel@engi...@basic.html - {fi-ehl-1}: [INCOMPLETE][19] ([i915#2439]) -> [PASS][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-ehl-1/igt@gem_exec_parallel@engi...@basic.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-ehl-1/igt@gem_exec_parallel@engi...@basic.html * igt@i915_selftest@live@gem_execbuf: - fi-gdg-551: [INCOMPLETE][21] ([i915#172]) -> [PASS][22] [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-gdg-551/igt@i915_selftest@live@gem_execbuf.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-gdg-551/igt@i915_selftest@live@gem_execbuf.html - fi-snb-2520m: [INCOMPLETE][23] ([i915#2439]) -> [PASS][24] [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-snb-2520m/igt@i915_selftest@live@gem_execbuf.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-snb-2520m/igt@i915_selftest@live@gem_execbuf.html Warnings * igt@gem_exec_parallel@engines@basic: - fi-bxt-dsi: [INCOMPLETE][25] ([i915#1635]) -> [INCOMPLETE][26] ([i915#1635] / [i915#2439]) [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-bxt-dsi/igt@gem_exec_parallel@engi...@basic.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/fi-bxt-dsi/igt@gem_exec_parallel@engi...@basic.html - fi-apl-guc: [INCOMPLETE][27] ([i915#1635]) -> [INCOMPLETE][28] ([i915#1635] / [i915#2439]) [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/fi-apl-guc/igt@gem_exec_parallel@engi...@basic.html [28]: https://intel-
[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/gvt: Prevent NULL pointer dereference in intel_vgpu_reg_rw_edid()
== Series Details == Series: drm/i915/gvt: Prevent NULL pointer dereference in intel_vgpu_reg_rw_edid() URL : https://patchwork.freedesktop.org/series/81470/ State : failure == Summary == CI Bug Log - changes from CI_DRM_8982_full -> Patchwork_18455_full Summary --- **FAILURE** Serious unknown changes coming with Patchwork_18455_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_18455_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_18455_full: ### IGT changes ### Possible regressions * igt@perf@enable-disable: - shard-skl: [PASS][1] -> [FAIL][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-skl8/igt@p...@enable-disable.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-skl10/igt@p...@enable-disable.html Known issues Here are the changes found in Patchwork_18455_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_reloc@basic-cpu-active: - shard-skl: [PASS][3] -> [INCOMPLETE][4] ([i915#198] / [i915#2439]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-skl5/igt@gem_exec_re...@basic-cpu-active.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-skl3/igt@gem_exec_re...@basic-cpu-active.html * igt@gem_exec_reloc@basic-cpu-gtt-active: - shard-tglb: [PASS][5] -> [INCOMPLETE][6] ([i915#2439]) +2 similar issues [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-tglb7/igt@gem_exec_re...@basic-cpu-gtt-active.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-tglb7/igt@gem_exec_re...@basic-cpu-gtt-active.html - shard-snb: [PASS][7] -> [INCOMPLETE][8] ([i915#2439] / [i915#82]) +3 similar issues [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-snb2/igt@gem_exec_re...@basic-cpu-gtt-active.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-snb1/igt@gem_exec_re...@basic-cpu-gtt-active.html * igt@gem_exec_reloc@basic-wc-active: - shard-iclb: [PASS][9] -> [INCOMPLETE][10] ([i915#2439]) +2 similar issues [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-iclb5/igt@gem_exec_re...@basic-wc-active.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-iclb8/igt@gem_exec_re...@basic-wc-active.html * igt@gem_exec_reloc@basic-wc-cpu-active: - shard-apl: [PASS][11] -> [INCOMPLETE][12] ([i915#1635] / [i915#2439]) +3 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-apl7/igt@gem_exec_re...@basic-wc-cpu-active.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-apl3/igt@gem_exec_re...@basic-wc-cpu-active.html * igt@gem_exec_reloc@basic-write-read-active: - shard-glk: [PASS][13] -> [INCOMPLETE][14] ([i915#2439]) +3 similar issues [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-glk7/igt@gem_exec_re...@basic-write-read-active.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-glk1/igt@gem_exec_re...@basic-write-read-active.html * igt@gem_exec_schedule@deep@rcs0: - shard-skl: [PASS][15] -> [INCOMPLETE][16] ([i915#2439]) +2 similar issues [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-skl10/igt@gem_exec_schedule@d...@rcs0.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-skl5/igt@gem_exec_schedule@d...@rcs0.html * igt@gem_exec_whisper@basic-contexts-all: - shard-hsw: [PASS][17] -> [INCOMPLETE][18] ([i915#2439]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-hsw8/igt@gem_exec_whis...@basic-contexts-all.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-hsw1/igt@gem_exec_whis...@basic-contexts-all.html * igt@gem_exec_whisper@basic-forked: - shard-kbl: [PASS][19] -> [INCOMPLETE][20] ([i915#2439]) +2 similar issues [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-kbl2/igt@gem_exec_whis...@basic-forked.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-kbl1/igt@gem_exec_whis...@basic-forked.html * igt@gem_userptr_blits@unsync-unmap-cycles: - shard-skl: [PASS][21] -> [TIMEOUT][22] ([i915#1958]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8982/shard-skl3/igt@gem_userptr_bl...@unsync-unmap-cycles.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18455/shard-skl3/igt@gem_userptr_bl...@unsync-unmap-cycles.html * igt@i915_suspend@forcewake: -
Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
> On Sep 3, 2020, at 10:04 AM, Srivatsa, Anusha > wrote: > > > >> -Original Message- >> From: Vivi, Rodrigo >> Sent: Wednesday, September 2, 2020 2:32 PM >> To: Srivatsa, Anusha >> Cc: intel-gfx@lists.freedesktop.org >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register >> lookup >> >> >> >>> On Sep 2, 2020, at 12:30 PM, Srivatsa, Anusha >> wrote: >>> >>> >>> -Original Message- From: Rodrigo Vivi Sent: Tuesday, September 1, 2020 12:30 PM To: Srivatsa, Anusha Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup On Tue, Sep 01, 2020 at 11:27:58AM -0700, Anusha Srivatsa wrote: > We currenty check for platform at multiple parts in the driver to > grab the correct PLL. Let us begin to centralize it through a helper > function. > > v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg() > (Ville) > > Suggested-by: Matt Roper > Cc: Ville Syrjälä > Cc: Matt Roper > Signed-off-by: Anusha Srivatsa > --- > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 25 > +++ > 1 file changed, 15 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > index c9013f8f766f..7440836c5e44 100644 > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > @@ -147,6 +147,18 @@ void assert_shared_dpll(struct >> drm_i915_private *dev_priv, > pll->info->name, onoff(state), onoff(cur_state)); } > > +static > +i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private *dev_priv, > + struct intel_shared_dpll *pll) { > + > + if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id == DPLL_ID_EHL_DPLL4)) > + return MG_PLL_ENABLE(0); > + > + return CNL_DPLL_ENABLE(pll->info->id); > + > + > +} > /** > * intel_prepare_shared_dpll - call a dpll's prepare hook > * @crtc_state: CRTC, and its state, which has a shared dpll @@ > -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv, > struct intel_shared_dpll *pll, > struct intel_dpll_hw_state *hw_state) { > - i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); > - > - if (IS_ELKHARTLAKE(dev_priv) && > - pll->info->id == DPLL_ID_EHL_DPLL4) { > - enable_reg = MG_PLL_ENABLE(0); > - } > + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll); > > return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg); > } @@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct > drm_i915_private *dev_priv, static void combo_pll_enable(struct drm_i915_private *dev_priv, >struct intel_shared_dpll *pll) { > - i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); > + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll); > > if (IS_ELKHARTLAKE(dev_priv) && > pll->info->id == DPLL_ID_EHL_DPLL4) { there's probably something else that we can do now with the power_{put,get} to get rid of the, now, doubled if checks... >>> >>> Don't follow you here Rodrigo. >> >> me neither ;) >> I'm just brainstorming... thinking out lout. >> >>> Are you suggesting using power_{put/get} to somehow get rid of doubled >> if? >> >> after this patch, on this path we will do this if check twice. >> not a big deal, but we can probably do something better. >> >> However I don't understand why we had this get/put here at first place. >> Only for this platform and only for this pll4. So, what I am wondering is >> that >> we have something better to do with the power_well infrastructure in >> general that would allow us to avoid the if (platform && pll4) in favor of >> something more generic. >> >> but definitely not a blocker for this patch itself. > Ok. So maybe the power well infrastructure change can be part a later patch? sure > >> >>> > - enable_reg = MG_PLL_ENABLE(0); > > /* >* We need to disable DC states when this DPLL is enabled. > @@ -4157,11 +4163,10 @@ static void icl_pll_disable(struct > drm_i915_private *dev_priv, static void combo_pll_disable(struct drm_i915_private *dev_priv, > struct intel_shared_dpll *pll) { > - i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); > + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll); > > if (IS_ELKHARTLAKE(dev_priv) && > pll->info->id == DPLL_ID_EHL_DPLL4) { > - enable_reg = MG_PLL_ENABLE(0); >
Re: [Intel-gfx] [PATCH 1/2] drm/i915/gem: Replace reloc chain with terminator on error unwind
Hi! > > > > Thanks for the patches. I assume this should fix problem from > > > > "5.9-rc1: graphics regression moved from -next to mainline" thread. > > > > > > > > I have applied them over current -next, and my machine seems to be > > > > working so far (but uptime is less than 30 minutes). > > > > > > > > If the machine still works tommorow, I'll assume problem is solved. > > > > > > Aye, best wait until we have to start competing with Chromium for > > > memory... The suspicion is that it was the resource allocation failure > > > path. > > > > Yep, my machines are low on memory. > > > > But ... test did not work that well. I have dead X and blinking > > screen. Machine still works reasonably well over ssh, so I guess > > that's an improvement. > > Well my last remaining 32bit gen3 device is currently pushing up the > daises, so could you try removing the attempt to use WC? Something like > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > @@ -955,10 +955,7 @@ static u32 *__reloc_gpu_map(struct reloc_cache *cache, > { > u32 *map; > > - map = i915_gem_object_pin_map(pool->obj, > - cache->has_llc ? > - I915_MAP_FORCE_WB : > - I915_MAP_FORCE_WC); > + map = i915_gem_object_pin_map(pool->obj, I915_MAP_FORCE_WB); > > on top of the previous patch. Faultinjection didn't turn up anything in > eb_relocate_vma, so we need to dig deeper. With this on top of other patches, it works. Tested-by: Pavel Machek Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/24] drm/vkms: Use devm_drm_dev_alloc
Hi Daniel, Thanks for this work. This change works smoothly to me. However, there is a check in the vkms_exit that doesn't look very good. Please, see inline comment. On 09/04, Daniel Vetter wrote: > This means we also need to slightly restructure the exit code, so that > final cleanup of the drm_device is triggered by unregistering the > platform device. Note that devres is both clean up when the driver is > unbound (not the case for vkms, we don't bind), and also when unregistering > the device (very much the case for vkms). Therefore we can rely on devres > even though vkms isn't a proper platform device driver. > > This also somewhat untangles the load code, since the drm and platform device > setup are no longer interleaved, but two distinct steps. > > v2: use devres_open/release_group so we can use devm without real > hacks in the driver core or having to create an entire fake bus for > testing drivers. Might want to extract this into helpers eventually, > maybe as a mock_drm_dev_alloc or test_drm_dev_alloc. > > Signed-off-by: Daniel Vetter > Cc: Rodrigo Siqueira > Cc: Haneen Mohammed > Cc: Daniel Vetter > --- > drivers/gpu/drm/vkms/vkms_drv.c | 54 - > 1 file changed, 26 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > index 83dd5567de8b..0d2c6dcf73c3 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.c > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > @@ -61,9 +61,6 @@ static void vkms_release(struct drm_device *dev) > { > struct vkms_device *vkms = container_of(dev, struct vkms_device, drm); > > - platform_device_unregister(vkms->platform); > - drm_atomic_helper_shutdown(&vkms->drm); > - drm_mode_config_cleanup(&vkms->drm); > destroy_workqueue(vkms->output.composer_workq); > } > > @@ -144,30 +141,31 @@ static int vkms_modeset_init(struct vkms_device > *vkmsdev) > static int __init vkms_init(void) > { > int ret; > + struct platform_device *pdev; > > - vkms_device = kzalloc(sizeof(*vkms_device), GFP_KERNEL); > - if (!vkms_device) > - return -ENOMEM; > + pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0); > + if (IS_ERR(pdev)) > + return PTR_ERR(pdev); > > - vkms_device->platform = > - platform_device_register_simple(DRIVER_NAME, -1, NULL, 0); > - if (IS_ERR(vkms_device->platform)) { > - ret = PTR_ERR(vkms_device->platform); > - goto out_free; > + if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL)) { > + ret = -ENOMEM; > + goto out_unregister; > } > > - ret = drm_dev_init(&vkms_device->drm, &vkms_driver, > -&vkms_device->platform->dev); > - if (ret) > - goto out_unregister; > - drmm_add_final_kfree(&vkms_device->drm, vkms_device); > + vkms_device = devm_drm_dev_alloc(&pdev->dev, &vkms_driver, > + struct vkms_device, drm); > + if (IS_ERR(vkms_device)) { > + ret = PTR_ERR(vkms_device); > + goto out_devres; > + } > + vkms_device->platform = pdev; > > ret = dma_coerce_mask_and_coherent(vkms_device->drm.dev, > DMA_BIT_MASK(64)); > > if (ret) { > DRM_ERROR("Could not initialize DMA support\n"); > - goto out_put; > + goto out_devres; > } > > vkms_device->drm.irq_enabled = true; > @@ -175,39 +173,39 @@ static int __init vkms_init(void) > ret = drm_vblank_init(&vkms_device->drm, 1); > if (ret) { > DRM_ERROR("Failed to vblank\n"); > - goto out_put; > + goto out_devres; > } > > ret = vkms_modeset_init(vkms_device); > if (ret) > - goto out_put; > + goto out_devres; > > ret = drm_dev_register(&vkms_device->drm, 0); > if (ret) > - goto out_put; > + goto out_devres; > > return 0; > > -out_put: > - drm_dev_put(&vkms_device->drm); > - platform_device_unregister(vkms_device->platform); > - return ret; > +out_devres: > + devres_release_group(&pdev->dev, NULL); > out_unregister: > - platform_device_unregister(vkms_device->platform); > -out_free: > - kfree(vkms_device); > + platform_device_unregister(pdev); > return ret; > } > > static void __exit vkms_exit(void) > { > + struct platform_device *pdev = vkms_device->platform; > + > if (!vkms_device) { > DRM_INFO("vkms_device is NULL.\n"); > return; > } I think it would be better to check vkms_device before assigning vkms_device->platform to the pdev above. I don't know if the compiler handles this, but doing the validation first seems more clear. > > drm_dev_unregister(&vkms_device->drm); > - drm_dev_put(&vkms_device->drm); > + d
[Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
We currenty check for platform at multiple parts in the driver to grab the correct PLL. Let us begin to centralize it through a helper function. v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg() (Ville) v3: Clean up combo_pll_disable() (Rodrigo) Suggested-by: Matt Roper Cc: Ville Syrjälä Cc: Matt Roper Signed-off-by: Anusha Srivatsa Reviewed-by: Rodrigo Vivi --- drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 29 +++ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c index c9013f8f766f..441b6f52e808 100644 --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c @@ -147,6 +147,18 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv, pll->info->name, onoff(state), onoff(cur_state)); } +static +i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private *dev_priv, + struct intel_shared_dpll *pll) +{ + + if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id == DPLL_ID_EHL_DPLL4)) + return MG_PLL_ENABLE(0); + + return CNL_DPLL_ENABLE(pll->info->id); + + +} /** * intel_prepare_shared_dpll - call a dpll's prepare hook * @crtc_state: CRTC, and its state, which has a shared dpll @@ -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv, struct intel_shared_dpll *pll, struct intel_dpll_hw_state *hw_state) { - i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); - - if (IS_ELKHARTLAKE(dev_priv) && - pll->info->id == DPLL_ID_EHL_DPLL4) { - enable_reg = MG_PLL_ENABLE(0); - } + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll); return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg); } @@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv, static void combo_pll_enable(struct drm_i915_private *dev_priv, struct intel_shared_dpll *pll) { - i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll); if (IS_ELKHARTLAKE(dev_priv) && pll->info->id == DPLL_ID_EHL_DPLL4) { - enable_reg = MG_PLL_ENABLE(0); /* * We need to disable DC states when this DPLL is enabled. @@ -4157,19 +4163,18 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv, static void combo_pll_disable(struct drm_i915_private *dev_priv, struct intel_shared_dpll *pll) { - i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll); + + icl_pll_disable(dev_priv, pll, enable_reg); if (IS_ELKHARTLAKE(dev_priv) && pll->info->id == DPLL_ID_EHL_DPLL4) { - enable_reg = MG_PLL_ENABLE(0); - icl_pll_disable(dev_priv, pll, enable_reg); intel_display_power_put(dev_priv, POWER_DOMAIN_DPLL_DC_OFF, pll->wakeref); return; } - icl_pll_disable(dev_priv, pll, enable_reg); } static void tbt_pll_disable(struct drm_i915_private *dev_priv, -- 2.25.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/pll: Centralize PLL_ENABLE register lookup (rev3)
== Series Details == Series: drm/i915/pll: Centralize PLL_ENABLE register lookup (rev3) URL : https://patchwork.freedesktop.org/series/81150/ State : warning == Summary == $ dim checkpatch origin/drm-tip 3be3651c4fb7 drm/i915/pll: Centralize PLL_ENABLE register lookup -:33: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis #33: FILE: drivers/gpu/drm/i915/display/intel_dpll_mgr.c:152: +i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private *dev_priv, + struct intel_shared_dpll *pll) -:35: CHECK:BRACES: Blank lines aren't necessary after an open brace '{' #35: FILE: drivers/gpu/drm/i915/display/intel_dpll_mgr.c:154: +{ + -:36: WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (8, 24) #36: FILE: drivers/gpu/drm/i915/display/intel_dpll_mgr.c:155: + if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id == DPLL_ID_EHL_DPLL4)) + return MG_PLL_ENABLE(0); -:36: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'pll->info->id == DPLL_ID_EHL_DPLL4' #36: FILE: drivers/gpu/drm/i915/display/intel_dpll_mgr.c:155: + if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id == DPLL_ID_EHL_DPLL4)) -:41: CHECK:LINE_SPACING: Please don't use multiple blank lines #41: FILE: drivers/gpu/drm/i915/display/intel_dpll_mgr.c:160: + + -:42: CHECK:BRACES: Blank lines aren't necessary before a close brace '}' #42: FILE: drivers/gpu/drm/i915/display/intel_dpll_mgr.c:161: + +} total: 0 errors, 1 warnings, 5 checks, 65 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/pll: Centralize PLL_ENABLE register lookup (rev3)
== Series Details == Series: drm/i915/pll: Centralize PLL_ENABLE register lookup (rev3) URL : https://patchwork.freedesktop.org/series/81150/ State : failure == Summary == CI Bug Log - changes from CI_DRM_8985 -> Patchwork_18456 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_18456 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_18456, 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_18456/index.html Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_18456: ### IGT changes ### Possible regressions * igt@gem_ctx_create@basic-files: - fi-tgl-u2: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-tgl-u2/igt@gem_ctx_cre...@basic-files.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-tgl-u2/igt@gem_ctx_cre...@basic-files.html Known issues Here are the changes found in Patchwork_18456 that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_gttfill@basic: - fi-kbl-7500u: [PASS][3] -> [INCOMPLETE][4] ([i915#2439]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-kbl-7500u/igt@gem_exec_gttf...@basic.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-kbl-7500u/igt@gem_exec_gttf...@basic.html - fi-elk-e7500: [PASS][5] -> [INCOMPLETE][6] ([i915#2439] / [i915#66]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-elk-e7500/igt@gem_exec_gttf...@basic.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-elk-e7500/igt@gem_exec_gttf...@basic.html - fi-ilk-650: [PASS][7] -> [INCOMPLETE][8] ([i915#2439]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-ilk-650/igt@gem_exec_gttf...@basic.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-ilk-650/igt@gem_exec_gttf...@basic.html * igt@i915_selftest@live@gem_execbuf: - fi-gdg-551: [PASS][9] -> [INCOMPLETE][10] ([i915#172] / [i915#2440]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-gdg-551/igt@i915_selftest@live@gem_execbuf.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-gdg-551/igt@i915_selftest@live@gem_execbuf.html - fi-snb-2600:[PASS][11] -> [INCOMPLETE][12] ([i915#2440] / [i915#82]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-snb-2600/igt@i915_selftest@live@gem_execbuf.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-snb-2600/igt@i915_selftest@live@gem_execbuf.html - fi-bsw-n3050: [PASS][13] -> [INCOMPLETE][14] ([i915#2439]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-bsw-n3050/igt@i915_selftest@live@gem_execbuf.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-bsw-n3050/igt@i915_selftest@live@gem_execbuf.html - fi-snb-2520m: [PASS][15] -> [INCOMPLETE][16] ([i915#2439]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-snb-2520m/igt@i915_selftest@live@gem_execbuf.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-snb-2520m/igt@i915_selftest@live@gem_execbuf.html Possible fixes * igt@gem_exec_gttfill@basic: - fi-kbl-r: [INCOMPLETE][17] ([i915#2439]) -> [PASS][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-kbl-r/igt@gem_exec_gttf...@basic.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-kbl-r/igt@gem_exec_gttf...@basic.html - fi-bdw-5557u: [INCOMPLETE][19] ([i915#2439]) -> [PASS][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-bdw-5557u/igt@gem_exec_gttf...@basic.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-bdw-5557u/igt@gem_exec_gttf...@basic.html * igt@gem_tiled_fence_blits@basic: - fi-pnv-d510:[INCOMPLETE][21] ([i915#2439] / [i915#299]) -> [PASS][22] [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-pnv-d510/igt@gem_tiled_fence_bl...@basic.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-pnv-d510/igt@gem_tiled_fence_bl...@basic.html Warnings * igt@runner@aborted: - fi-kbl-r: [FAIL][23] ([i915#1186] / [i915#1784] / [i915#2439]) -> [FAIL][24] ([i915#2398] / [i915#2439]) [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8985/fi-kbl-r/igt@run...@aborted.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18456/fi-kbl-r/igt@run...@aborted.html - fi-kbl-7500u: [FAIL][25] ([i915#1784] / [i915#2398] / [i915#2439]) -> [FA
Re: [Intel-gfx] [11/12] drm/i915: Introduce intel_hpd_hotplug_irqs()
On Wed, 2020-07-01 at 00:56 +0300, Ville Syrjala wrote: > From: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > > Introduce intel_hpd_hotplug_irqs() as a partner to > intel_hpd_enabled_irqs(). There's no need to care about the > encoders which we're not exposing, so we can avoid hardocoding hard-coding > the masks in various places. Pretty nice patch, you only missed to do this change in the irq_handlers so we could nuke the SDE_DDI_MASKs, or are you planning to do this in a follow up patch? If later consider this Reviewed-by: José Roberto de Souza > > Signed-off-by: Ville Syrjälä < > ville.syrj...@linux.intel.com > > > --- > drivers/gpu/drm/i915/i915_irq.c | 50 +++-- > 1 file changed, 23 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 95ab4432a87d..b8a6a21f4c54 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2943,6 +2943,18 @@ static u32 intel_hpd_enabled_irqs(struct > drm_i915_private *dev_priv, > return enabled_irqs; > } > > +static u32 intel_hpd_hotplug_irqs(struct drm_i915_private *dev_priv, > + const u32 hpd[HPD_NUM_PINS]) > +{ > + struct intel_encoder *encoder; > + u32 hotplug_irqs = 0; > + > + for_each_intel_encoder(&dev_priv->drm, encoder) > + hotplug_irqs |= hpd[encoder->hpd_pin]; > + > + return hotplug_irqs; > +} > + > static void ibx_hpd_detection_setup(struct drm_i915_private *dev_priv) > { > u32 hotplug; > @@ -2972,12 +2984,8 @@ static void ibx_hpd_irq_setup(struct drm_i915_private > *dev_priv) > { > u32 hotplug_irqs, enabled_irqs; > > - if (HAS_PCH_IBX(dev_priv)) > - hotplug_irqs = SDE_HOTPLUG_MASK; > - else > - hotplug_irqs = SDE_HOTPLUG_MASK_CPT; > - > enabled_irqs = intel_hpd_enabled_irqs(dev_priv, > dev_priv->hotplug.pch_hpd); > + hotplug_irqs = intel_hpd_hotplug_irqs(dev_priv, > dev_priv->hotplug.pch_hpd); > > ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs); > > @@ -3005,13 +3013,12 @@ static void icp_tc_hpd_detection_setup(struct > drm_i915_private *dev_priv, > } > > static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv, > - u32 sde_ddi_mask, u32 sde_tc_mask, > u32 ddi_enable_mask, u32 tc_enable_mask) > { > u32 hotplug_irqs, enabled_irqs; > > - hotplug_irqs = sde_ddi_mask | sde_tc_mask; > enabled_irqs = intel_hpd_enabled_irqs(dev_priv, > dev_priv->hotplug.pch_hpd); > + hotplug_irqs = intel_hpd_hotplug_irqs(dev_priv, > dev_priv->hotplug.pch_hpd); > > I915_WRITE(SHPD_FILTER_CNT, SHPD_FILTER_CNT_500_ADJ); > > @@ -3029,7 +3036,6 @@ static void icp_hpd_irq_setup(struct drm_i915_private > *dev_priv, > static void mcc_hpd_irq_setup(struct drm_i915_private *dev_priv) > { > icp_hpd_irq_setup(dev_priv, > - SDE_DDI_MASK_ICP, SDE_TC_HOTPLUG_ICP(PORT_TC1), > ICP_DDI_HPD_ENABLE_MASK, ICP_TC_HPD_ENABLE(PORT_TC1)); > } > > @@ -3041,7 +3047,6 @@ static void mcc_hpd_irq_setup(struct drm_i915_private > *dev_priv) > static void jsp_hpd_irq_setup(struct drm_i915_private *dev_priv) > { > icp_hpd_irq_setup(dev_priv, > - SDE_DDI_MASK_TGP, 0, > TGP_DDI_HPD_ENABLE_MASK, 0); > } > > @@ -3074,7 +3079,7 @@ static void gen11_hpd_irq_setup(struct drm_i915_private > *dev_priv) > u32 val; > > enabled_irqs = intel_hpd_enabled_irqs(dev_priv, dev_priv->hotplug.hpd); > - hotplug_irqs = GEN11_DE_TC_HOTPLUG_MASK | GEN11_DE_TBT_HOTPLUG_MASK; > + hotplug_irqs = intel_hpd_hotplug_irqs(dev_priv, dev_priv->hotplug.hpd); > > val = I915_READ(GEN11_DE_HPD_IMR); > val &= ~hotplug_irqs; > @@ -3085,10 +3090,10 @@ static void gen11_hpd_irq_setup(struct > drm_i915_private *dev_priv) > gen11_hpd_detection_setup(dev_priv); > > if (INTEL_PCH_TYPE(dev_priv) >= PCH_TGP) > - icp_hpd_irq_setup(dev_priv, SDE_DDI_MASK_TGP, SDE_TC_MASK_TGP, > + icp_hpd_irq_setup(dev_priv, > TGP_DDI_HPD_ENABLE_MASK, > TGP_TC_HPD_ENABLE_MASK); > else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) > - icp_hpd_irq_setup(dev_priv, SDE_DDI_MASK_ICP, SDE_TC_MASK_ICP, > + icp_hpd_irq_setup(dev_priv, > ICP_DDI_HPD_ENABLE_MASK, > ICP_TC_HPD_ENABLE_MASK); > } > > @@ -3124,8 +3129,8 @@ static void spt_hpd_irq_setup(struct drm_i915_private > *dev_priv) > if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) > I915_WRITE(SHPD_FILTER_CNT, SHPD_FILTER_CNT_500_ADJ); > > - hotplug_irqs = SDE_HOTPLUG_MASK_SPT; > enabled_irqs = intel_hpd_enabled_irqs(dev_priv, > dev_priv->hotplug.pch_hpd); > + hotplug_irqs = intel_hpd_hotplug_irqs(dev_priv, > dev_priv->hotpl
Re: [Intel-gfx] [PATCH] drm/i915/gvt: Introduce per object locking in GVT scheduler.
I tested this patch on the suspend/resume case with vGPU created (no need really activate), can still observer the system freeze issue as mentioned in another patch I sent. So I suppose we still need decouple context pin/unpin with submission setup/clean, but move to workload create/destroy? After made similar changes based on this one, plus the suspend/resume support patch, below tests can pass: - Create vGPU then suspend/resume. - Run VM w/ vGPU then suspend/resume. https://lists.freedesktop.org/archives/intel-gvt-dev/2020-September/007061.html On 2020-09-08 04:02, Zhi Wang wrote: To support ww locking and per-object implemented in i915, GVT scheduler needs to be refined. Most of the changes are located in shadow batch buffer, shadow wa context in GVT-g, where use quite a lot of i915 gem object APIs. Cc: Maarten Lankhorst Cc: Joonas Lahtinen Cc: Zhenyu Wang Signed-off-by: Zhi Wang --- drivers/gpu/drm/i915/gvt/scheduler.c | 68 ++-- 1 file changed, 57 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index 1570eb8..fe7ee10 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -396,7 +396,9 @@ static void release_shadow_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx) if (!wa_ctx->indirect_ctx.obj) return; + i915_gem_object_lock(wa_ctx->indirect_ctx.obj, NULL); i915_gem_object_unpin_map(wa_ctx->indirect_ctx.obj); + i915_gem_object_unlock(wa_ctx->indirect_ctx.obj); i915_gem_object_put(wa_ctx->indirect_ctx.obj); wa_ctx->indirect_ctx.obj = NULL; @@ -504,6 +506,7 @@ static int prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload) struct intel_gvt *gvt = workload->vgpu->gvt; const int gmadr_bytes = gvt->device_info.gmadr_bytes_in_cmd; struct intel_vgpu_shadow_bb *bb; + struct i915_gem_ww_ctx ww; int ret; list_for_each_entry(bb, &workload->shadow_bb, list) { @@ -528,10 +531,19 @@ static int prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload) * directly */ if (!bb->ppgtt) { - bb->vma = i915_gem_object_ggtt_pin(bb->obj, - NULL, 0, 0, 0); + i915_gem_ww_ctx_init(&ww, false); +retry: + i915_gem_object_lock(bb->obj, &ww); + + bb->vma = i915_gem_object_ggtt_pin_ww(bb->obj, &ww, + NULL, 0, 0, 0); if (IS_ERR(bb->vma)) { ret = PTR_ERR(bb->vma); + if (ret == -EDEADLK) { + ret = i915_gem_ww_ctx_backoff(&ww); + if (!ret) + goto retry; + } goto err; } @@ -545,13 +557,18 @@ static int prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload) 0); if (ret) goto err; + + /* No one is going to touch shadow bb from now on. */ + i915_gem_object_flush_map(bb->obj); + + i915_gem_object_unlock(bb->obj); + i915_gem_ww_ctx_fini(&ww); } - /* No one is going to touch shadow bb from now on. */ - i915_gem_object_flush_map(bb->obj); } return 0; err: + i915_gem_ww_ctx_fini(&ww); release_shadow_batch_buffer(workload); return ret; } @@ -578,14 +595,30 @@ static int prepare_shadow_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx) unsigned char *per_ctx_va = (unsigned char *)wa_ctx->indirect_ctx.shadow_va + wa_ctx->indirect_ctx.size; + struct i915_gem_ww_ctx ww; + int ret; if (wa_ctx->indirect_ctx.size == 0) return 0; - vma = i915_gem_object_ggtt_pin(wa_ctx->indirect_ctx.obj, NULL, - 0, CACHELINE_BYTES, 0); - if (IS_ERR(vma)) - return PTR_ERR(vma); + i915_gem_ww_ctx_init(&ww, false); +retry: + i915_gem_object_lock(wa_ctx->indirect_ctx.obj, &ww); + + vma = i915_gem_object_ggtt_pin_ww(wa_ctx->indirect_ctx.obj, &ww, NULL, + 0, CACHELINE_BYTES, 0); + if (IS_ERR(vma)) { + ret = PTR_ERR(vma); + if (ret == -EDEADLK) { + ret = i915_gem_ww_ctx_backoff(&ww); + if (!ret) + goto retry; + } + return ret; + } + + i915_gem_obj
Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
Hi Christoph, On 9/8/20 2:23 PM, Christoph Hellwig wrote: On Tue, Sep 08, 2020 at 02:04:53PM +0800, Lu Baolu wrote: Do you mind telling where can I find Marek's series? [PATCH v10 00/30] DRM: fix struct sg_table nents vs. orig_nents misuse on various lists including the iommu one. It seems that more work is needed in i915 driver. I will added below quirk as you suggested. --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -851,6 +851,31 @@ static int __finalise_sg(struct device *dev, struct scatterlist *sg, int nents, unsigned int cur_len = 0, max_len = dma_get_max_seg_size(dev); int i, count = 0; + /* +* The Intel graphic device driver is used to assume that the returned +* sg list is not combound. This blocks the efforts of converting the +* Intel IOMMU driver to dma-iommu api's. Add this quirk to make the +* device driver work and should be removed once it's fixed in i915 +* driver. +*/ + if (dev_is_pci(dev) && + to_pci_dev(dev)->vendor == PCI_VENDOR_ID_INTEL && + (to_pci_dev(dev)->class >> 16) == PCI_BASE_CLASS_DISPLAY) { + for_each_sg(sg, s, nents, i) { + unsigned int s_iova_off = sg_dma_address(s); + unsigned int s_length = sg_dma_len(s); + unsigned int s_iova_len = s->length; + + s->offset += s_iova_off; + s->length = s_length; + sg_dma_address(s) = dma_addr + s_iova_off; + sg_dma_len(s) = s_length; + dma_addr += s_iova_len; + } + + return nents; + } + Best regards, baolu ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gvt: Introduce per object locking in GVT scheduler.
On 2020.09.09 09:43:21 +0800, Colin Xu wrote: > I tested this patch on the suspend/resume case with vGPU created (no need > really activate), can still observer the system freeze issue as mentioned in > another patch I sent. So I suppose we still need decouple context pin/unpin > with submission setup/clean, but move to workload create/destroy? > yeah, I observed that conflict too. How about merging Zhi's ww lock fix first then next for left suspend/resume fixes? And better if you could pack all fixes in one series. Thanks > After made similar changes based on this one, plus the suspend/resume > support patch, below tests can pass: > > - Create vGPU then suspend/resume. > > - Run VM w/ vGPU then suspend/resume. > > https://lists.freedesktop.org/archives/intel-gvt-dev/2020-September/007061.html > > On 2020-09-08 04:02, Zhi Wang wrote: > > To support ww locking and per-object implemented in i915, GVT scheduler > > needs > > to be refined. Most of the changes are located in shadow batch buffer, > > shadow > > wa context in GVT-g, where use quite a lot of i915 gem object APIs. > > > > Cc: Maarten Lankhorst > > Cc: Joonas Lahtinen > > Cc: Zhenyu Wang > > Signed-off-by: Zhi Wang > > --- > > drivers/gpu/drm/i915/gvt/scheduler.c | 68 > > ++-- > > 1 file changed, 57 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c > > b/drivers/gpu/drm/i915/gvt/scheduler.c > > index 1570eb8..fe7ee10 100644 > > --- a/drivers/gpu/drm/i915/gvt/scheduler.c > > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c > > @@ -396,7 +396,9 @@ static void release_shadow_wa_ctx(struct > > intel_shadow_wa_ctx *wa_ctx) > > if (!wa_ctx->indirect_ctx.obj) > > return; > > + i915_gem_object_lock(wa_ctx->indirect_ctx.obj, NULL); > > i915_gem_object_unpin_map(wa_ctx->indirect_ctx.obj); > > + i915_gem_object_unlock(wa_ctx->indirect_ctx.obj); > > i915_gem_object_put(wa_ctx->indirect_ctx.obj); > > wa_ctx->indirect_ctx.obj = NULL; > > @@ -504,6 +506,7 @@ static int prepare_shadow_batch_buffer(struct > > intel_vgpu_workload *workload) > > struct intel_gvt *gvt = workload->vgpu->gvt; > > const int gmadr_bytes = gvt->device_info.gmadr_bytes_in_cmd; > > struct intel_vgpu_shadow_bb *bb; > > + struct i915_gem_ww_ctx ww; > > int ret; > > list_for_each_entry(bb, &workload->shadow_bb, list) { > > @@ -528,10 +531,19 @@ static int prepare_shadow_batch_buffer(struct > > intel_vgpu_workload *workload) > > * directly > > */ > > if (!bb->ppgtt) { > > - bb->vma = i915_gem_object_ggtt_pin(bb->obj, > > - NULL, 0, 0, 0); > > + i915_gem_ww_ctx_init(&ww, false); > > +retry: > > + i915_gem_object_lock(bb->obj, &ww); > > + > > + bb->vma = i915_gem_object_ggtt_pin_ww(bb->obj, &ww, > > + NULL, 0, 0, 0); > > if (IS_ERR(bb->vma)) { > > ret = PTR_ERR(bb->vma); > > + if (ret == -EDEADLK) { > > + ret = i915_gem_ww_ctx_backoff(&ww); > > + if (!ret) > > + goto retry; > > + } > > goto err; > > } > > @@ -545,13 +557,18 @@ static int prepare_shadow_batch_buffer(struct > > intel_vgpu_workload *workload) > > 0); > > if (ret) > > goto err; > > + > > + /* No one is going to touch shadow bb from now on. */ > > + i915_gem_object_flush_map(bb->obj); > > + > > + i915_gem_object_unlock(bb->obj); > > + i915_gem_ww_ctx_fini(&ww); > > } > > - /* No one is going to touch shadow bb from now on. */ > > - i915_gem_object_flush_map(bb->obj); > > } > > return 0; > > err: > > + i915_gem_ww_ctx_fini(&ww); > > release_shadow_batch_buffer(workload); > > return ret; > > } > > @@ -578,14 +595,30 @@ static int prepare_shadow_wa_ctx(struct > > intel_shadow_wa_ctx *wa_ctx) > > unsigned char *per_ctx_va = > > (unsigned char *)wa_ctx->indirect_ctx.shadow_va + > > wa_ctx->indirect_ctx.size; > > + struct i915_gem_ww_ctx ww; > > + int ret; > > if (wa_ctx->indirect_ctx.size == 0) > > return 0; > > - vma = i915_gem_object_ggtt_pin(wa_ctx->indirect_ctx.obj, NULL, > > - 0, CACHELINE_BYTES, 0); > > - if (IS_ERR(vma)) > > - return PTR_ERR(vma); > > + i915_gem_ww_ctx_init(&ww, false); > > +retry: > > + i915_gem_object_lock(wa_ctx->indirect_ctx.obj, &ww); > > + > > + vma = i915_gem_object_ggtt_pin_ww(wa_
Re: [Intel-gfx] [PATCH] drm/i915/gvt: Introduce per object locking in GVT scheduler.
On 2020-09-09 10:06, Zhenyu Wang wrote: On 2020.09.09 09:43:21 +0800, Colin Xu wrote: I tested this patch on the suspend/resume case with vGPU created (no need really activate), can still observer the system freeze issue as mentioned in another patch I sent. So I suppose we still need decouple context pin/unpin with submission setup/clean, but move to workload create/destroy? yeah, I observed that conflict too. How about merging Zhi's ww lock fix first then next for left suspend/resume fixes? And better if you could pack all fixes in one series. Thanks No problem at all. And I would like to hear more comments if moving the context pin/unpin to workload lifecycle is reasonable. After made similar changes based on this one, plus the suspend/resume support patch, below tests can pass: - Create vGPU then suspend/resume. - Run VM w/ vGPU then suspend/resume. https://lists.freedesktop.org/archives/intel-gvt-dev/2020-September/007061.html On 2020-09-08 04:02, Zhi Wang wrote: To support ww locking and per-object implemented in i915, GVT scheduler needs to be refined. Most of the changes are located in shadow batch buffer, shadow wa context in GVT-g, where use quite a lot of i915 gem object APIs. Cc: Maarten Lankhorst Cc: Joonas Lahtinen Cc: Zhenyu Wang Signed-off-by: Zhi Wang --- drivers/gpu/drm/i915/gvt/scheduler.c | 68 ++-- 1 file changed, 57 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index 1570eb8..fe7ee10 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -396,7 +396,9 @@ static void release_shadow_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx) if (!wa_ctx->indirect_ctx.obj) return; + i915_gem_object_lock(wa_ctx->indirect_ctx.obj, NULL); i915_gem_object_unpin_map(wa_ctx->indirect_ctx.obj); + i915_gem_object_unlock(wa_ctx->indirect_ctx.obj); i915_gem_object_put(wa_ctx->indirect_ctx.obj); wa_ctx->indirect_ctx.obj = NULL; @@ -504,6 +506,7 @@ static int prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload) struct intel_gvt *gvt = workload->vgpu->gvt; const int gmadr_bytes = gvt->device_info.gmadr_bytes_in_cmd; struct intel_vgpu_shadow_bb *bb; + struct i915_gem_ww_ctx ww; int ret; list_for_each_entry(bb, &workload->shadow_bb, list) { @@ -528,10 +531,19 @@ static int prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload) * directly */ if (!bb->ppgtt) { - bb->vma = i915_gem_object_ggtt_pin(bb->obj, - NULL, 0, 0, 0); + i915_gem_ww_ctx_init(&ww, false); +retry: + i915_gem_object_lock(bb->obj, &ww); + + bb->vma = i915_gem_object_ggtt_pin_ww(bb->obj, &ww, + NULL, 0, 0, 0); if (IS_ERR(bb->vma)) { ret = PTR_ERR(bb->vma); + if (ret == -EDEADLK) { + ret = i915_gem_ww_ctx_backoff(&ww); + if (!ret) + goto retry; + } goto err; } @@ -545,13 +557,18 @@ static int prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload) 0); if (ret) goto err; + + /* No one is going to touch shadow bb from now on. */ + i915_gem_object_flush_map(bb->obj); + + i915_gem_object_unlock(bb->obj); + i915_gem_ww_ctx_fini(&ww); } - /* No one is going to touch shadow bb from now on. */ - i915_gem_object_flush_map(bb->obj); } return 0; err: + i915_gem_ww_ctx_fini(&ww); release_shadow_batch_buffer(workload); return ret; } @@ -578,14 +595,30 @@ static int prepare_shadow_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx) unsigned char *per_ctx_va = (unsigned char *)wa_ctx->indirect_ctx.shadow_va + wa_ctx->indirect_ctx.size; + struct i915_gem_ww_ctx ww; + int ret; if (wa_ctx->indirect_ctx.size == 0) return 0; - vma = i915_gem_object_ggtt_pin(wa_ctx->indirect_ctx.obj, NULL, - 0, CACHELINE_BYTES, 0); - if (IS_ERR(vma)) - return PTR_ERR(vma); + i915_gem_ww_ctx_init(&ww, false); +retry: + i915_gem_object_lock(wa_ctx->indirect_ctx.obj, &ww); + + vma = i915_gem_object_ggtt_pin_ww(wa_ctx