Re: [Intel-gfx] [PATCH] drm/i915: unpin on error in intel_vgpu_shadow_mm_pin()
On 2022.11.15 16:15:18 +0300, Dan Carpenter wrote:
> Call intel_vgpu_unpin_mm() on this error path.
>
> Fixes: 418741480809 ("drm/i915/gvt: Adding ppgtt to GVT GEM context after
> shadow pdps settled.")
> Signed-off-by: Dan Carpenter
> ---
> drivers/gpu/drm/i915/gvt/scheduler.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
> b/drivers/gpu/drm/i915/gvt/scheduler.c
> index d6fe94cd0fdb..8342d95f56cb 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -696,6 +696,7 @@ intel_vgpu_shadow_mm_pin(struct intel_vgpu_workload
> *workload)
>
> if (workload->shadow_mm->type != INTEL_GVT_MM_PPGTT ||
> !workload->shadow_mm->ppgtt_mm.shadowed) {
> + intel_vgpu_unpin_mm(workload->shadow_mm);
> gvt_vgpu_err("workload shadow ppgtt isn't ready\n");
> return -EINVAL;
> }
> --
Thanks, Dan. Looks fine to me.
Reviewed-by: Zhenyu Wang
signature.asc
Description: PGP signature
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/dmc: Update DG2 DMC version to v2.08 (rev3)
== Series Details == Series: drm/i915/dmc: Update DG2 DMC version to v2.08 (rev3) URL : https://patchwork.freedesktop.org/series/64/ State : success == Summary == CI Bug Log - changes from CI_DRM_12432 -> Patchwork_64v3 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/index.html Participating hosts (36 -> 36) -- Additional (1): bat-dg1-6 Missing(1): fi-ctg-p8600 Known issues Here are the changes found in Patchwork_64v3 that come from known issues: ### IGT changes ### Issues hit * igt@gem_mmap@basic: - bat-dg1-6: NOTRUN -> [SKIP][1] ([i915#4083]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/bat-dg1-6/igt@gem_m...@basic.html * igt@gem_render_tiled_blits@basic: - bat-dg1-6: NOTRUN -> [SKIP][2] ([i915#4079]) +1 similar issue [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/bat-dg1-6/igt@gem_render_tiled_bl...@basic.html * igt@gem_tiled_blits@basic: - fi-pnv-d510:[PASS][3] -> [SKIP][4] ([fdo#109271]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12432/fi-pnv-d510/igt@gem_tiled_bl...@basic.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/fi-pnv-d510/igt@gem_tiled_bl...@basic.html * igt@gem_tiled_fence_blits@basic: - bat-dg1-6: NOTRUN -> [SKIP][5] ([i915#4077]) +2 similar issues [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/bat-dg1-6/igt@gem_tiled_fence_bl...@basic.html * igt@i915_pm_backlight@basic-brightness: - bat-dg1-6: NOTRUN -> [SKIP][6] ([i915#7561]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/bat-dg1-6/igt@i915_pm_backli...@basic-brightness.html * igt@i915_pm_rps@basic-api: - bat-dg1-6: NOTRUN -> [SKIP][7] ([i915#6621]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/bat-dg1-6/igt@i915_pm_...@basic-api.html * igt@i915_selftest@live@hangcheck: - fi-hsw-4770:[PASS][8] -> [INCOMPLETE][9] ([i915#4785]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12432/fi-hsw-4770/igt@i915_selftest@l...@hangcheck.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/fi-hsw-4770/igt@i915_selftest@l...@hangcheck.html * igt@i915_selftest@live@mman: - fi-rkl-guc: [PASS][10] -> [TIMEOUT][11] ([i915#6794]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12432/fi-rkl-guc/igt@i915_selftest@l...@mman.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/fi-rkl-guc/igt@i915_selftest@l...@mman.html * igt@kms_addfb_basic@basic-y-tiled-legacy: - bat-dg1-6: NOTRUN -> [SKIP][12] ([i915#4215]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/bat-dg1-6/igt@kms_addfb_ba...@basic-y-tiled-legacy.html * igt@kms_addfb_basic@tile-pitch-mismatch: - bat-dg1-6: NOTRUN -> [SKIP][13] ([i915#4212]) +7 similar issues [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/bat-dg1-6/igt@kms_addfb_ba...@tile-pitch-mismatch.html * igt@kms_chamelium@hdmi-crc-fast: - bat-dg1-6: NOTRUN -> [SKIP][14] ([fdo#111827]) +8 similar issues [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/bat-dg1-6/igt@kms_chamel...@hdmi-crc-fast.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor: - bat-dg1-6: NOTRUN -> [SKIP][15] ([i915#4103] / [i915#4213]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/bat-dg1-6/igt@kms_cursor_leg...@basic-busy-flip-before-cursor.html * igt@kms_force_connector_basic@force-load-detect: - bat-dg1-6: NOTRUN -> [SKIP][16] ([fdo#109285]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/bat-dg1-6/igt@kms_force_connector_ba...@force-load-detect.html * igt@kms_psr@sprite_plane_onoff: - bat-dg1-6: NOTRUN -> [SKIP][17] ([i915#1072] / [i915#4078]) +3 similar issues [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/bat-dg1-6/igt@kms_psr@sprite_plane_onoff.html * igt@kms_setmode@basic-clone-single-crtc: - bat-dg1-6: NOTRUN -> [SKIP][18] ([i915#3555]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/bat-dg1-6/igt@kms_setm...@basic-clone-single-crtc.html * igt@prime_vgem@basic-gtt: - bat-dg1-6: NOTRUN -> [SKIP][19] ([i915#3708] / [i915#4077]) +1 similar issue [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/bat-dg1-6/igt@prime_v...@basic-gtt.html * igt@prime_vgem@basic-read: - bat-dg1-6: NOTRUN -> [SKIP][20] ([i915#3708]) +3 similar issues [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_64v3/bat-dg1-6/igt@prime_v...@basic-read.html * igt@prime_vgem@basic-userptr: - bat-dg1-6:
Re: [Intel-gfx] [PATCH v3] drm/i915/dmc: Update DG2 DMC version to v2.08
Just a quick note: firmware PR hasn't been applied yet. Waiting... -- Gustavo Sousa
[Intel-gfx] [PATCH v3] drm/i915/dmc: Update DG2 DMC version to v2.08
Release notes:
1. Fixes for Register noclaims and few restore.
Fixes: c4cf059d9c2c ("drm/i915/dmc: Update DG2 DMC firmware to v2.07")
Signed-off-by: Gustavo Sousa
---
v2:
- Use correct numbering for the minor version (8 instead of the
invalid octal 08).
v3:
- Add "Fixes:" trailer.
drivers/gpu/drm/i915/display/intel_dmc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
b/drivers/gpu/drm/i915/display/intel_dmc.c
index 081a4d0083b1..eff3add70611 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -52,8 +52,8 @@
#define DISPLAY_VER12_DMC_MAX_FW_SIZE ICL_DMC_MAX_FW_SIZE
-#define DG2_DMC_PATH DMC_PATH(dg2, 2, 07)
-#define DG2_DMC_VERSION_REQUIRED DMC_VERSION(2, 07)
+#define DG2_DMC_PATH DMC_PATH(dg2, 2, 08)
+#define DG2_DMC_VERSION_REQUIRED DMC_VERSION(2, 8)
MODULE_FIRMWARE(DG2_DMC_PATH);
#define ADLP_DMC_PATH DMC_PATH(adlp, 2, 16)
--
2.38.1
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v4,1/3] drm/i915/migrate: Account for the reserved_space
== Series Details == Series: series starting with [v4,1/3] drm/i915/migrate: Account for the reserved_space URL : https://patchwork.freedesktop.org/series/111314/ State : success == Summary == CI Bug Log - changes from CI_DRM_12432 -> Patchwork_111314v1 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111314v1/index.html Participating hosts (36 -> 35) -- Additional (1): bat-dg1-6 Missing(2): fi-ctg-p8600 fi-rkl-11600 Known issues Here are the changes found in Patchwork_111314v1 that come from known issues: ### IGT changes ### Issues hit * igt@gem_mmap@basic: - bat-dg1-6: NOTRUN -> [SKIP][1] ([i915#4083]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111314v1/bat-dg1-6/igt@gem_m...@basic.html * igt@gem_render_tiled_blits@basic: - bat-dg1-6: NOTRUN -> [SKIP][2] ([i915#4079]) +1 similar issue [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111314v1/bat-dg1-6/igt@gem_render_tiled_bl...@basic.html * igt@gem_tiled_fence_blits@basic: - bat-dg1-6: NOTRUN -> [SKIP][3] ([i915#4077]) +2 similar issues [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111314v1/bat-dg1-6/igt@gem_tiled_fence_bl...@basic.html * igt@i915_pm_backlight@basic-brightness: - bat-dg1-6: NOTRUN -> [SKIP][4] ([i915#7561]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111314v1/bat-dg1-6/igt@i915_pm_backli...@basic-brightness.html * igt@i915_pm_rps@basic-api: - bat-dg1-6: NOTRUN -> [SKIP][5] ([i915#6621]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111314v1/bat-dg1-6/igt@i915_pm_...@basic-api.html * igt@i915_selftest@live@gt_timelines: - fi-apl-guc: [PASS][6] -> [INCOMPLETE][7] ([i915#7511]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12432/fi-apl-guc/igt@i915_selftest@live@gt_timelines.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111314v1/fi-apl-guc/igt@i915_selftest@live@gt_timelines.html * igt@kms_addfb_basic@basic-y-tiled-legacy: - bat-dg1-6: NOTRUN -> [SKIP][8] ([i915#4215]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111314v1/bat-dg1-6/igt@kms_addfb_ba...@basic-y-tiled-legacy.html * igt@kms_addfb_basic@tile-pitch-mismatch: - bat-dg1-6: NOTRUN -> [SKIP][9] ([i915#4212]) +7 similar issues [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111314v1/bat-dg1-6/igt@kms_addfb_ba...@tile-pitch-mismatch.html * igt@kms_chamelium@hdmi-crc-fast: - bat-dg1-6: NOTRUN -> [SKIP][10] ([fdo#111827]) +8 similar issues [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111314v1/bat-dg1-6/igt@kms_chamel...@hdmi-crc-fast.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor: - bat-dg1-6: NOTRUN -> [SKIP][11] ([i915#4103] / [i915#4213]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111314v1/bat-dg1-6/igt@kms_cursor_leg...@basic-busy-flip-before-cursor.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions: - fi-bsw-kefka: [PASS][12] -> [FAIL][13] ([i915#6298]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12432/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cur...@atomic-transitions.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111314v1/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cur...@atomic-transitions.html * igt@kms_force_connector_basic@force-load-detect: - bat-dg1-6: NOTRUN -> [SKIP][14] ([fdo#109285]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111314v1/bat-dg1-6/igt@kms_force_connector_ba...@force-load-detect.html * igt@kms_psr@sprite_plane_onoff: - bat-dg1-6: NOTRUN -> [SKIP][15] ([i915#1072] / [i915#4078]) +3 similar issues [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111314v1/bat-dg1-6/igt@kms_psr@sprite_plane_onoff.html * igt@kms_setmode@basic-clone-single-crtc: - bat-dg1-6: NOTRUN -> [SKIP][16] ([i915#3555]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111314v1/bat-dg1-6/igt@kms_setm...@basic-clone-single-crtc.html * igt@prime_vgem@basic-gtt: - bat-dg1-6: NOTRUN -> [SKIP][17] ([i915#3708] / [i915#4077]) +1 similar issue [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111314v1/bat-dg1-6/igt@prime_v...@basic-gtt.html * igt@prime_vgem@basic-read: - bat-dg1-6: NOTRUN -> [SKIP][18] ([i915#3708]) +3 similar issues [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111314v1/bat-dg1-6/igt@prime_v...@basic-read.html * igt@prime_vgem@basic-userptr: - bat-dg1-6: NOTRUN -> [SKIP][19] ([i915#3708] / [i915#4873]) [19]:
Re: [Intel-gfx] [RFC 11/13] cgroup/drm: Introduce weight based drm cgroup control
On 22/11/2022 21:29, Tejun Heo wrote:
On Wed, Nov 09, 2022 at 04:11:39PM +, Tvrtko Ursulin wrote:
+DRM scheduling soft limits
+~~
+
+Because of the heterogenous hardware and driver DRM capabilities, soft limits
+are implemented as a loose co-operative (bi-directional) interface between the
+controller and DRM core.
+
+The controller configures the GPU time allowed per group and periodically scans
+the belonging tasks to detect the over budget condition, at which point it
+invokes a callback notifying the DRM core of the condition.
+
+DRM core provides an API to query per process GPU utilization and 2nd API to
+receive notification from the cgroup controller when the group enters or exits
+the over budget condition.
+
+Individual DRM drivers which implement the interface are expected to act on
this
+in the best-effort manner only. There are no guarantees that the soft limits
+will be respected.
Soft limits is a bit of misnomer and can be confused with best-effort limits
such as memory.high. Prolly best to not use the term.
Are you suggesting "best effort limits" or "best effort "? It
would sounds good to me if we found the right . Best effort
budget perhaps?
+static bool
+__start_scanning(struct drm_cgroup_state *root, unsigned int period_us)
+{
+ struct cgroup_subsys_state *node;
+ bool ok = false;
+
+ rcu_read_lock();
+
+ css_for_each_descendant_post(node, >css) {
+ struct drm_cgroup_state *drmcs = css_to_drmcs(node);
+
+ if (!css_tryget_online(node))
+ goto out;
+
+ drmcs->active_us = 0;
+ drmcs->sum_children_weights = 0;
+
+ if (node == >css)
+ drmcs->per_s_budget_ns =
+ DIV_ROUND_UP_ULL(NSEC_PER_SEC * period_us,
+USEC_PER_SEC);
+ else
+ drmcs->per_s_budget_ns = 0;
+
+ css_put(node);
+ }
+
+ css_for_each_descendant_post(node, >css) {
+ struct drm_cgroup_state *drmcs = css_to_drmcs(node);
+ struct drm_cgroup_state *parent;
+ u64 active;
+
+ if (!css_tryget_online(node))
+ goto out;
+ if (!node->parent) {
+ css_put(node);
+ continue;
+ }
+ if (!css_tryget_online(node->parent)) {
+ css_put(node);
+ goto out;
+ }
+ parent = css_to_drmcs(node->parent);
+
+ active = drmcs_get_active_time_us(drmcs);
+ if (active > drmcs->prev_active_us)
+ drmcs->active_us += active - drmcs->prev_active_us;
+ drmcs->prev_active_us = active;
+
+ parent->active_us += drmcs->active_us;
+ parent->sum_children_weights += drmcs->weight;
+
+ css_put(node);
+ css_put(>css);
+ }
+
+ ok = true;
+
+out:
+ rcu_read_unlock();
+
+ return ok;
+}
A more conventional and scalable way to go about this would be using an
rbtree keyed by virtual time. Both CFS and blk-iocost are examples of this,
but I think for drm, it can be a lot simpler.
It's well impressive you were able to figure out what I am doing there.
:) And probably you can see that this is the first time I am attempting
an algorithm like this one. I think I made it /dtrt/ with a few post/pre
walks so the right pieces of data propagate correctly.
Are you suggesting a parallel/shadow tree to be kept in the drm
controller (which would shadow the cgroup hierarchy)? Or something else?
The mention of rbtree is not telling me much, but I will look into the
referenced examples. (Although I will refrain from major rework until
more people start "biting" into all this.)
Also, when you mention scalability you are concerned about multiple tree
walks I have per iteration? I wasn't so much worried about that,
definitely not for the RFC, but even in general due relatively low
frequency of scanning and a good amount of less trivial cost being
outside the actual tree walks (drm client walks, GPU utilisation
calculations, maybe more). But perhaps I don't have the right idea on
how big cgroups hierarchies can be compared to number of drm clients etc.
Regards,
Tvrtko
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v4,1/3] drm/i915/migrate: Account for the reserved_space
== Series Details ==
Series: series starting with [v4,1/3] drm/i915/migrate: Account for the
reserved_space
URL : https://patchwork.freedesktop.org/series/111314/
State : warning
== Summary ==
Error: dim checkpatch failed
a161f13b1e36 drm/i915/migrate: Account for the reserved_space
-:36: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#36: FILE: drivers/gpu/drm/i915/gt/intel_migrate.c:347:
+ struct intel_ring *ring = rq->ring;$
-:38: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#38: FILE: drivers/gpu/drm/i915/gt/intel_migrate.c:349:
+ pkt = min_t(int, pkt, (ring->space - rq->reserved_space) / sizeof(u32)
+ 5);$
-:39: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#39: FILE: drivers/gpu/drm/i915/gt/intel_migrate.c:350:
+ pkt = min_t(int, pkt, (ring->size - ring->emit) / sizeof(u32) + 5);$
-:41: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#41: FILE: drivers/gpu/drm/i915/gt/intel_migrate.c:352:
+ return pkt;$
total: 0 errors, 4 warnings, 0 checks, 34 lines checked
e82b1ca63779 drm/i915/selftests: use live_subtests for live_migrate
f9c9bf06c390 drm/i915/selftests: exercise emit_pte() with nearly full ring
-:77: WARNING:TRACING_LOGGING: Unnecessary ftrace-like logging - prefer using
ftrace
#77: FILE: drivers/gpu/drm/i915/gt/selftest_migrate.c:539:
+ pr_info("%s\n", __func__);
-:189: WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see
Documentation/timers/timers-howto.rst
#189: FILE: drivers/gpu/drm/i915/gt/selftest_migrate.c:651:
+ msleep(10); /* start all threads before we kthread_stop() */
total: 0 errors, 2 warnings, 0 checks, 200 lines checked
Re: [Intel-gfx] [PATCH v10 06/19] drm/modes: Add a function to generate analog display modes
Den 17.11.2022 10.28, skrev Maxime Ripard: > Multiple drivers (meson, vc4, sun4i) define analog TV 525-lines and > 625-lines modes in their drivers. > > Since those modes are fairly standard, and that we'll need to use them > in more places in the future, it makes sense to move their definition > into the core framework. > > However, analog display usually have fairly loose timings requirements, > the only discrete parameters being the total number of lines and pixel > clock frequency. Thus, we created a function that will create a display > mode from the standard, the pixel frequency and the active area. > > Tested-by: Mateusz Kwiatkowski > Signed-off-by: Maxime Ripard > > --- I'm no domain expert so apart from the timing details which I can't comment on, it looks fine. I personally advocated for a much simpler solution for these NTSC and PAL modes, but AIUI this is part of a grander plan to support devices with other timings. Acked-by: Noralf Trønnes
Re: [Intel-gfx] [PATCH v10 05/19] drm/connector: Add TV standard property
Den 17.11.2022 10.28, skrev Maxime Ripard:
> The TV mode property has been around for a while now to select and get the
> current TV mode output on an analog TV connector.
>
> Despite that property name being generic, its content isn't and has been
> driver-specific which makes it hard to build any generic behaviour on top
> of it, both in kernel and user-space.
>
> Let's create a new enum tv norm property, that can contain any of the
> analog TV standards currently supported by kernel drivers. Each driver can
> then pass in a bitmask of the modes it supports, and the property
> creation function will filter out the modes not supported.
>
> We'll then be able to phase out the older tv mode property.
>
> Tested-by: Mateusz Kwiatkowski
> Signed-off-by: Maxime Ripard
>
> ---
> Changes in v10:
> - Fix checkpatch warning
>
> Changes in v5:
> - Create an analog TV properties documentation section, and document TV
> Mode there instead of the csv file
>
> Changes in v4:
> - Add property documentation to kms-properties.csv
> - Fix documentation
> ---
> Documentation/gpu/drm-kms.rst | 6 ++
> drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
> drivers/gpu/drm/drm_connector.c | 122
> +-
> include/drm/drm_connector.h | 64
> include/drm/drm_mode_config.h | 8 +++
> 5 files changed, 203 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index b4377a545425..321f2f582c64 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -520,6 +520,12 @@ HDMI Specific Connector Properties
> .. kernel-doc:: drivers/gpu/drm/drm_connector.c
> :doc: HDMI connector properties
>
> +Analog TV Specific Connector Properties
> +--
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c
> + :doc: Analog TV Connector Properties
> +
> Standard CRTC Properties
>
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 7f2b9a07fbdf..d867e7f9f2cd 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -700,6 +700,8 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> state->tv.margins.bottom = val;
> } else if (property == config->legacy_tv_mode_property) {
> state->tv.legacy_mode = val;
> + } else if (property == config->tv_mode_property) {
> + state->tv.mode = val;
> } else if (property == config->tv_brightness_property) {
> state->tv.brightness = val;
> } else if (property == config->tv_contrast_property) {
> @@ -810,6 +812,8 @@ drm_atomic_connector_get_property(struct drm_connector
> *connector,
> *val = state->tv.margins.bottom;
> } else if (property == config->legacy_tv_mode_property) {
> *val = state->tv.legacy_mode;
> + } else if (property == config->tv_mode_property) {
> + *val = state->tv.mode;
> } else if (property == config->tv_brightness_property) {
> *val = state->tv.brightness;
> } else if (property == config->tv_contrast_property) {
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 06e737ed15f5..07d449736956 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -984,6 +984,17 @@ static const struct drm_prop_enum_list
> drm_dvi_i_subconnector_enum_list[] = {
> DRM_ENUM_NAME_FN(drm_get_dvi_i_subconnector_name,
>drm_dvi_i_subconnector_enum_list)
>
> +static const struct drm_prop_enum_list drm_tv_mode_enum_list[] = {
> + { DRM_MODE_TV_MODE_NTSC, "NTSC" },
> + { DRM_MODE_TV_MODE_NTSC_443, "NTSC-443" },
> + { DRM_MODE_TV_MODE_NTSC_J, "NTSC-J" },
> + { DRM_MODE_TV_MODE_PAL, "PAL" },
> + { DRM_MODE_TV_MODE_PAL_M, "PAL-M" },
> + { DRM_MODE_TV_MODE_PAL_N, "PAL-N" },
> + { DRM_MODE_TV_MODE_SECAM, "SECAM" },
> +};
> +DRM_ENUM_NAME_FN(drm_get_tv_mode_name, drm_tv_mode_enum_list)
> +
This patch looks good but since I'm no TV standards expert I can't say
if the content of this list is a good choice for reflecting the world of
TV standards.
Acked-by: Noralf Trønnes
> static const struct drm_prop_enum_list drm_tv_select_enum_list[] = {
> { DRM_MODE_SUBCONNECTOR_Automatic, "Automatic" }, /* DVI-I and TV-out */
> { DRM_MODE_SUBCONNECTOR_Composite, "Composite" }, /* TV-out */
> @@ -1552,6 +1563,71 @@
> EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property);
> * infoframe values is done through drm_hdmi_avi_infoframe_content_type().
> */
>
> +/*
> + * TODO: Document the properties:
> + * - left margin
> + * - right margin
> + * - top margin
> + * - bottom margin
> + * - brightness
> + * - contrast
> + * - flicker reduction
> + * - hue
> + * - mode
> + * - overscan
> + *
Re: [Intel-gfx] [PATCH i-g-t] tests/i915/gem_exec_balancer: exercise dmabuf import
On 18.11.2022 16:53, Matthew Auld wrote:
With parallel submission it should be easy to get a fence array as the
output fence. Try importing this into dma-buf reservation object, to see
if anything explodes.
References: https://gitlab.freedesktop.org/drm/intel/-/issues/7532
Signed-off-by: Matthew Auld
Cc: Andrzej Hajda
Cc: Nirmoy Das
---
tests/i915/gem_exec_balancer.c | 39 ++
1 file changed, 39 insertions(+)
diff --git a/tests/i915/gem_exec_balancer.c b/tests/i915/gem_exec_balancer.c
index 4300dbd1..fdae8de5 100644
--- a/tests/i915/gem_exec_balancer.c
+++ b/tests/i915/gem_exec_balancer.c
@@ -37,6 +37,7 @@
#include "igt_sysfs.h"
#include "igt_types.h"
#include "sw_sync.h"
+#include
IGT_TEST_DESCRIPTION("Exercise in-kernel load-balancing");
@@ -2856,6 +2857,24 @@ static void logical_sort_siblings(int i915,
#define PARALLEL_SUBMIT_FENCE (0x1 << 3)
#define PARALLEL_CONTEXTS (0x1 << 4)
#define PARALLEL_VIRTUAL (0x1 << 5)
+#define PARALLEL_OUT_FENCE_DMABUF (0x1 << 6)
+
+struct igt_dma_buf_sync_file {
+__u32 flags;
+__s32 fd;
+};
+
+#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct
igt_dma_buf_sync_file)
+#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct
igt_dma_buf_sync_file)
+
+static void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
+{
+struct igt_dma_buf_sync_file arg;
+
+arg.flags = flags;
+arg.fd = sync_fd;
+do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, );
+}
Wouldn't be good to move code above to some common lib?
Anyway:
Reviewed-by: Andrzej Hajda
Regards
Andrzej
static void parallel_thread(int i915, unsigned int flags,
struct i915_engine_class_instance *siblings,
@@ -2871,6 +2890,8 @@ static void parallel_thread(int i915, unsigned int flags,
uint32_t target_bo_idx = 0;
uint32_t first_bb_idx = 1;
intel_ctx_cfg_t cfg;
+ uint32_t dmabuf_handle;
+ int dmabuf;
igt_assert(bb_per_execbuf < 32);
@@ -2924,11 +2945,20 @@ static void parallel_thread(int i915, unsigned int flags,
execbuf.buffers_ptr = to_user_pointer(obj);
execbuf.rsvd1 = ctx->id;
+ if (flags & PARALLEL_OUT_FENCE_DMABUF) {
+ dmabuf_handle = gem_create(i915, 4096);
+ dmabuf = prime_handle_to_fd(i915, dmabuf_handle);
+ }
+
for (n = 0; n < PARALLEL_BB_LOOP_COUNT; ++n) {
execbuf.flags &= ~0x3full;
gem_execbuf_wr(i915, );
if (flags & PARALLEL_OUT_FENCE) {
+ if (flags & PARALLEL_OUT_FENCE_DMABUF)
+ dmabuf_import_sync_file(dmabuf,
DMA_BUF_SYNC_WRITE,
+ execbuf.rsvd2 >> 32);
+
igt_assert_eq(sync_fence_wait(execbuf.rsvd2 >> 32,
1000), 0);
igt_assert_eq(sync_fence_status(execbuf.rsvd2 >> 32),
1);
@@ -2959,6 +2989,11 @@ static void parallel_thread(int i915, unsigned int flags,
if (fence)
close(fence);
+ if (flags & PARALLEL_OUT_FENCE_DMABUF) {
+ gem_close(i915, dmabuf_handle);
+ close(dmabuf);
+ }
+
check_bo(i915, obj[target_bo_idx].handle,
bb_per_execbuf * PARALLEL_BB_LOOP_COUNT, true);
@@ -3420,6 +3455,10 @@ igt_main
igt_subtest("parallel-out-fence")
parallel(i915, PARALLEL_OUT_FENCE);
+ igt_subtest("parallel-out-fence-import-dmabuf")
+ parallel(i915, PARALLEL_OUT_FENCE |
+PARALLEL_OUT_FENCE_DMABUF);
+
igt_subtest("parallel-keep-in-fence")
parallel(i915, PARALLEL_OUT_FENCE | PARALLEL_IN_FENCE);
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915: Never return 0 if not all requests retired
On 23/11/2022 16:21, Janusz Krzysztofik wrote:
On Wednesday, 23 November 2022 13:57:26 CET Tvrtko Ursulin wrote:
On 23/11/2022 09:28, Janusz Krzysztofik wrote:
Hi Tvrtko,
Thanks for your comments.
On Tuesday, 22 November 2022 11:50:38 CET Tvrtko Ursulin wrote:
On 21/11/2022 14:56, Janusz Krzysztofik wrote:
Users of intel_gt_retire_requests_timeout() expect 0 return value on
success. However, we have no protection from passing back 0 potentially
returned by a call to dma_fence_wait_timeout() when it succedes right
after its timeout has expired.
Is this talking about a potential weakness, or ambiguous kerneldoc, of
dma_fence_wait_timeout, dma_fence_default_wait and
i915_request_wait_timeout? They appear to say 0 return means timeout,
implying unsignaled fence. In other words signaled must return positive
remaining timeout. Implementations seems to allow a race which indeed
appears that return 0 and signaled fence is possible.
While my initial analysis was indeed focused on inconsistent semantics of 0
return values from different dma_fence_default_wait() backends, I should have
also mentioned in this commit description that users may perfectly
call intel_gt_retire_requests_timeout() with 0 timeout, in which case the
false positive 0 value can be returned regardless of dma_fence_wait_timeout()
potential issues. Would you like me to reword and resubmit?
Not sure yet.
So the only caller which passes in zero to
intel_gt_retire_requests_timeout appears to be intel_gt_retire_requests
and it eats the return value anyway so this patch is immaterial for that
one.
Right.
I guess it can change how intel_gt_wait_for_idle behaves with short-ish
timeouts. In case this race is hit. But then wouldn't it make sense to
follow up with a patch which addresses this race by re-checking the "is
signaled" when timeout expires,
But inside intel_gt_retire_requests_timeout() we generally don't care if
fences have been signaled. As long as user requested timeout hasn't expired,
we use dma_fence_wait_timeout() as an aid, otherwise we keep trying to retire
requests without waiting on fences. If the retirement succeeds then we return
0 (success) regardless of what the return value from the last called
dma_fence_wait_timeout() was. If it was 0 then the only useful information is
that no more time has been left, and no matter if that 0 meant signaled or not
signaled, we must return an error if there are still some requests not
retired, I believe.
Yes I agree with all that. Sorry if my reply was misleading, I mentioned
a follow-up, didn't mean that these two are not ready to go.
either in dma_fence_wait_timeout, or to
intel_gt_retire_requests_timeout. Or if not that at least better
document the dma_fence_wait_timeout and/or
intel_gt_retire_requests_timeout. Makes sense?
Documenting -- yes, as soon as we get into an agreement on what's the core of
this issue -- whether that potential weakness, or ambiguous kerneldoc, of
dma_fence_wait_timeout, dma_fence_default_wait and i915_request_wait_timeout,
as you've stated, that we have to address somehow, or potentially incorrect
direct use of the timeout variable, intended for storing time left to spend on
fence waits, as our return value when timeout has expired. And if the former
then maybe we should also try to finally resolve that over a year old conflict
(whether 0 means signaled on not signaled) inside our implementation of
dma_fence_ops.wait, and simply use a wrapper around it for either our internal
use if we decide to follow the reference implementation, or for dma_fence_ops
use otherwise. Or maybe the reference implementation should be fixed if
problematic. I don't feel competent enough to decide.
Right, we can leave that for later. I'll pull these two in if someone
hasn't already.
Regards,
Tvrtko
Thanks,
Janusz
Regards,
Tvrtko
If dma_fence_wait can indeed return 0 even when a request is signaled,
then how is timeout ?: -ETIME below correct? It isn't a chance for false
negative in its' callers?
The goal of intel_gt_retire_requests_timeout() is to retire requests. When
that goal has been reached, i.e., all requests have been retired, active count
is 0, and 0 is correctly returned, regardless of timeout value.
The value of timeout is used only when there are still pending requests, which
means that the goal hasn't been reached and the function hasn't succeeded.
Then, no false negative is possible, unlike the false positive that we now
have when we return 0 while some requests are still pending.
Thanks,
Janusz
Regards,
Tvrtko
Replace 0 with -ETIME before potentially using the timeout value as return
code, so -ETIME is returned if there are still some requests not retired
after timeout, 0 otherwise.
v3: Use conditional expression, more compact but also better reflecting
intention standing behind the change.
v2: Move the added lines down so flush_submission() is not affected.
Fixes: f33a8a51602c ("drm/i915: Merge
[Intel-gfx] [PATCH v4 3/3] drm/i915/selftests: exercise emit_pte() with nearly full ring
Simple regression test to check that we don't trample the
rq->reserved_space when returning from emit_pte(), if the ring is nearly
full.
v2: Make spinner_kill() static
v3: Reduce the ring size further, which should mean we need to execute less
noops; hopefully this appeases bsw. Also add some debug logging.
v4: Fix the min request construction to account for reserved_space +
I915_EMIT_PTE_NUM_DWORDS
References: https://gitlab.freedesktop.org/drm/intel/-/issues/7535
References: https://gitlab.freedesktop.org/drm/intel/-/issues/6889
Signed-off-by: Matthew Auld
Cc: Chris Wilson
Cc: Andi Shyti
Cc: Andrzej Hajda
Cc: Nirmoy Das
---
drivers/gpu/drm/i915/gt/intel_migrate.c| 6 +-
drivers/gpu/drm/i915/gt/selftest_migrate.c | 158 +
2 files changed, 162 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c
b/drivers/gpu/drm/i915/gt/intel_migrate.c
index 48c3b5168558..6df728b82a73 100644
--- a/drivers/gpu/drm/i915/gt/intel_migrate.c
+++ b/drivers/gpu/drm/i915/gt/intel_migrate.c
@@ -352,6 +352,8 @@ static int max_pte_pkt_size(struct i915_request *rq, int
pkt)
return pkt;
}
+#define I915_EMIT_PTE_NUM_DWORDS 6
+
static int emit_pte(struct i915_request *rq,
struct sgt_dma *it,
enum i915_cache_level cache_level,
@@ -393,7 +395,7 @@ static int emit_pte(struct i915_request *rq,
offset += (u64)rq->engine->instance << 32;
- cs = intel_ring_begin(rq, 6);
+ cs = intel_ring_begin(rq, I915_EMIT_PTE_NUM_DWORDS);
if (IS_ERR(cs))
return PTR_ERR(cs);
@@ -416,7 +418,7 @@ static int emit_pte(struct i915_request *rq,
intel_ring_advance(rq, cs);
intel_ring_update_space(ring);
- cs = intel_ring_begin(rq, 6);
+ cs = intel_ring_begin(rq, I915_EMIT_PTE_NUM_DWORDS);
if (IS_ERR(cs))
return PTR_ERR(cs);
diff --git a/drivers/gpu/drm/i915/gt/selftest_migrate.c
b/drivers/gpu/drm/i915/gt/selftest_migrate.c
index 1eab025ac002..1da51add2c02 100644
--- a/drivers/gpu/drm/i915/gt/selftest_migrate.c
+++ b/drivers/gpu/drm/i915/gt/selftest_migrate.c
@@ -8,6 +8,7 @@
#include "gem/i915_gem_internal.h"
#include "gem/i915_gem_lmem.h"
+#include "selftests/igt_spinner.h"
#include "selftests/i915_random.h"
static const unsigned int sizes[] = {
@@ -529,6 +530,162 @@ static int live_migrate_clear(void *arg)
return 0;
}
+static int spinner_kill(void *arg)
+{
+ struct igt_spinner *spin = arg;
+
+ msleep(2000); /* Should be plenty */
+ igt_spinner_end(spin);
+ pr_info("%s\n", __func__);
+ return 0;
+}
+
+static int live_emit_pte_full_ring(void *arg)
+{
+ struct intel_gt *gt = arg;
+ struct intel_migrate *migrate = >migrate;
+ struct drm_i915_private *i915 = migrate->context->engine->i915;
+ struct drm_i915_gem_object *obj;
+ struct intel_context *ce;
+ struct i915_request *rq, *prev;
+ struct igt_spinner spin;
+ struct task_struct *tsk = NULL;
+ struct sgt_dma it;
+ int len, sz, err;
+ int status;
+ u32 *cs;
+
+ /*
+* Simple regression test to check that we don't trample the
+* rq->reserved_space when returning from emit_pte(), if the ring is
+* nearly full.
+*/
+
+ if (igt_spinner_init(, to_gt(i915)))
+ return -ENOMEM;
+
+ obj = i915_gem_object_create_internal(i915, 2 * PAGE_SIZE);
+ if (IS_ERR(obj)) {
+ err = PTR_ERR(obj);
+ goto out_spinner;
+ }
+
+ err = i915_gem_object_pin_pages_unlocked(obj);
+ if (err)
+ goto out_obj;
+
+ ce = intel_migrate_create_context(migrate);
+ if (IS_ERR(ce)) {
+ err = PTR_ERR(ce);
+ goto out_obj;
+ }
+
+ ce->ring_size = SZ_4K; /* Not too big */
+
+ err = intel_context_pin(ce);
+ if (err)
+ goto out_put;
+
+ rq = igt_spinner_create_request(, ce, MI_ARB_CHECK);
+ if (IS_ERR(rq)) {
+ err = PTR_ERR(rq);
+ goto out_unpin;
+ }
+
+ i915_request_add(rq);
+ if (!igt_wait_for_spinner(, rq)) {
+ err = -EIO;
+ goto out_unpin;
+ }
+
+ /*
+* Fill the rest of the ring leaving I915_EMIT_PTE_NUM_DWORDS +
+* ring->reserved_space at the end. To actually emit the PTEs we require
+* slightly more than I915_EMIT_PTE_NUM_DWORDS, since our object size is
+* greater than PAGE_SIZE. The correct behaviour is to wait for more
+* ring space in emit_pte(), otherwise we trample on the reserved_space
+* resulting in crashes when later submitting the rq.
+*/
+
+ prev = NULL;
+ do {
+ if (prev)
+ i915_request_add(rq);
+
+
[Intel-gfx] [PATCH v4 2/3] drm/i915/selftests: use live_subtests for live_migrate
Probably a good idea to do an igt_flush_test() at the end of each
subtest, just to be sure the previous work has been flushed and doesn't
somehow interfere with the current subtest.
Signed-off-by: Matthew Auld
Cc: Chris Wilson
Cc: Andi Shyti
Cc: Andrzej Hajda
Cc: Nirmoy Das
---
drivers/gpu/drm/i915/gt/selftest_migrate.c | 28 --
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_migrate.c
b/drivers/gpu/drm/i915/gt/selftest_migrate.c
index 0dc5309c90a4..1eab025ac002 100644
--- a/drivers/gpu/drm/i915/gt/selftest_migrate.c
+++ b/drivers/gpu/drm/i915/gt/selftest_migrate.c
@@ -486,7 +486,8 @@ global_clear(struct intel_migrate *migrate, u32 sz, struct
rnd_state *prng)
static int live_migrate_copy(void *arg)
{
- struct intel_migrate *migrate = arg;
+ struct intel_gt *gt = arg;
+ struct intel_migrate *migrate = >migrate;
struct drm_i915_private *i915 = migrate->context->engine->i915;
I915_RND_STATE(prng);
int i;
@@ -507,7 +508,8 @@ static int live_migrate_copy(void *arg)
static int live_migrate_clear(void *arg)
{
- struct intel_migrate *migrate = arg;
+ struct intel_gt *gt = arg;
+ struct intel_migrate *migrate = >migrate;
struct drm_i915_private *i915 = migrate->context->engine->i915;
I915_RND_STATE(prng);
int i;
@@ -593,7 +595,10 @@ static int __thread_migrate_copy(void *arg)
static int thread_migrate_copy(void *arg)
{
- return threaded_migrate(arg, __thread_migrate_copy, 0);
+ struct intel_gt *gt = arg;
+ struct intel_migrate *migrate = >migrate;
+
+ return threaded_migrate(migrate, __thread_migrate_copy, 0);
}
static int __thread_global_copy(void *arg)
@@ -605,7 +610,10 @@ static int __thread_global_copy(void *arg)
static int thread_global_copy(void *arg)
{
- return threaded_migrate(arg, __thread_global_copy, 0);
+ struct intel_gt *gt = arg;
+ struct intel_migrate *migrate = >migrate;
+
+ return threaded_migrate(migrate, __thread_global_copy, 0);
}
static int __thread_migrate_clear(void *arg)
@@ -624,12 +632,18 @@ static int __thread_global_clear(void *arg)
static int thread_migrate_clear(void *arg)
{
- return threaded_migrate(arg, __thread_migrate_clear, 0);
+ struct intel_gt *gt = arg;
+ struct intel_migrate *migrate = >migrate;
+
+ return threaded_migrate(migrate, __thread_migrate_clear, 0);
}
static int thread_global_clear(void *arg)
{
- return threaded_migrate(arg, __thread_global_clear, 0);
+ struct intel_gt *gt = arg;
+ struct intel_migrate *migrate = >migrate;
+
+ return threaded_migrate(migrate, __thread_global_clear, 0);
}
int intel_migrate_live_selftests(struct drm_i915_private *i915)
@@ -647,7 +661,7 @@ int intel_migrate_live_selftests(struct drm_i915_private
*i915)
if (!gt->migrate.context)
return 0;
- return i915_subtests(tests, >migrate);
+ return intel_gt_live_subtests(tests, gt);
}
static struct drm_i915_gem_object *
--
2.38.1
[Intel-gfx] [PATCH v4 1/3] drm/i915/migrate: Account for the reserved_space
From: Chris Wilson
If the ring is nearly full when calling into emit_pte(), we might
incorrectly trample the reserved_space when constructing the packet to
emit the PTEs. This then triggers the GEM_BUG_ON(rq->reserved_space >
ring->space) when later submitting the request, since the request itself
doesn't have enough space left in the ring to emit things like
workarounds, breadcrumbs etc.
Testcase: igt@i915_selftests@live_emit_pte_full_ring
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7535
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6889
Fixes: cf586021642d ("drm/i915/gt: Pipelined page migration")
Signed-off-by: Chris Wilson
Signed-off-by: Matthew Auld
Cc: Andrzej Hajda
Cc: Andi Shyti
Cc: Nirmoy Das
Cc: # v5.15+
Tested-by: Nirmoy Das
Reviewed-by: Nirmoy Das
---
drivers/gpu/drm/i915/gt/intel_migrate.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c
b/drivers/gpu/drm/i915/gt/intel_migrate.c
index b405a04135ca..48c3b5168558 100644
--- a/drivers/gpu/drm/i915/gt/intel_migrate.c
+++ b/drivers/gpu/drm/i915/gt/intel_migrate.c
@@ -342,6 +342,16 @@ static int emit_no_arbitration(struct i915_request *rq)
return 0;
}
+static int max_pte_pkt_size(struct i915_request *rq, int pkt)
+{
+ struct intel_ring *ring = rq->ring;
+
+ pkt = min_t(int, pkt, (ring->space - rq->reserved_space) / sizeof(u32)
+ 5);
+ pkt = min_t(int, pkt, (ring->size - ring->emit) / sizeof(u32) + 5);
+
+ return pkt;
+}
+
static int emit_pte(struct i915_request *rq,
struct sgt_dma *it,
enum i915_cache_level cache_level,
@@ -388,8 +398,7 @@ static int emit_pte(struct i915_request *rq,
return PTR_ERR(cs);
/* Pack as many PTE updates as possible into a single MI command */
- pkt = min_t(int, dword_length, ring->space / sizeof(u32) + 5);
- pkt = min_t(int, pkt, (ring->size - ring->emit) / sizeof(u32) + 5);
+ pkt = max_pte_pkt_size(rq, dword_length);
hdr = cs;
*cs++ = MI_STORE_DATA_IMM | REG_BIT(21); /* as qword elements */
@@ -422,8 +431,7 @@ static int emit_pte(struct i915_request *rq,
}
}
- pkt = min_t(int, dword_rem, ring->space / sizeof(u32) +
5);
- pkt = min_t(int, pkt, (ring->size - ring->emit) /
sizeof(u32) + 5);
+ pkt = max_pte_pkt_size(rq, dword_rem);
hdr = cs;
*cs++ = MI_STORE_DATA_IMM | REG_BIT(21);
--
2.38.1
Re: [Intel-gfx] [PATCH v2 2/4] drm/i915: Introduce guard pages to i915_vma
On 23/11/2022 18:54, Andi Shyti wrote:
Hi Tvrtko,
[...]
@@ -768,6 +768,9 @@ i915_vma_insert(struct i915_vma *vma, struct
i915_gem_ww_ctx *ww,
GEM_BUG_ON(!IS_ALIGNED(alignment, I915_GTT_MIN_ALIGNMENT));
GEM_BUG_ON(!is_power_of_2(alignment));
+ guard = vma->guard; /* retain guard across rebinds */
+ guard = ALIGN(guard, alignment);
Why does guard area needs the same alignment as the requested mapping? What
about the fact on 32-bit builds guard is 32-bit and alignment u64?
I guess this just to round up/down guard to something, not
necessarily to that alignment.
Shall I remove it?
Don't know, initially I thought it maybe needs a comment on what's it
doing and why. If it is about aligning to "something" then should it be
I915_GTT_MIN_ALIGNMENT?
@@ -777,6 +780,7 @@ i915_vma_insert(struct i915_vma *vma, struct
i915_gem_ww_ctx *ww,
if (flags & PIN_ZONE_4G)
end = min_t(u64, end, (1ULL << 32) - I915_GTT_PAGE_SIZE);
GEM_BUG_ON(!IS_ALIGNED(end, I915_GTT_PAGE_SIZE));
+ GEM_BUG_ON(2 * guard > end);
End is the size of relevant VA area at this point so what and why is this
checking?
I think because we want to make sure the padding is at least not
bigger that the size. What is actually wrong with this.
Same as above - if there is subtle special meaning please add a comment.
Otherwise, for the whole object and not just the guards, it is covered by:
+ if (size > end - 2 * guard) {
I don't follow what is the point on only checking the guards.
[...]
@@ -855,6 +869,7 @@ i915_vma_insert(struct i915_vma *vma, struct
i915_gem_ww_ctx *ww,
GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, color));
list_move_tail(>vm_link, >vm->bound_list);
+ vma->guard = guard;
unsigned long into u32 - what guarantees no truncation?
we are missing here this part above:
guard = vma->guard; /* retain guard across rebinds */
if (flags & PIN_OFFSET_GUARD) {
GEM_BUG_ON(overflows_type(flags & PIN_OFFSET_MASK, u32));
guard = max_t(u32, guard, flags & PIN_OFFSET_MASK);
}
that should make sure that we fit into 32 bits.
Hm okay. I guess the u64 alignment and that "guard = ALIGN(guard,
alignment);" is what is bothering me to begin with. In other words with
that there is a chance to overflow vma->guard with a small guard and
large alignment.
[...]
@@ -197,14 +197,15 @@ struct i915_vma {
struct i915_fence_reg *fence;
u64 size;
- u64 display_alignment;
struct i915_page_sizes page_sizes;
/* mmap-offset associated with fencing for this vma */
struct i915_mmap_offset *mmo;
+ u32 guard; /* padding allocated around vma->pages within the node */
u32 fence_size;
u32 fence_alignment;
+ u32 display_alignment;
u64 -> u32 for display_alignment looks unrelated change.
./display/intel_fb_pin.c: vma->display_alignment = max_t(u64,
vma->display_alignment, alignment);
./gem/i915_gem_domain.c:vma->display_alignment = max_t(u64,
vma->display_alignment, alignment);
These two sites need to be changed not to use u64.
Do this part in a separate patch?
Right! will remove it.
Okay, to be clear, refactoring of vma->display_alignemnt to be u32 as a
separate patch in the series. Thanks!
Regards,
Tvrtko
/**
* Count of the number of times this vma has been opened by different
Regards,
Thanks,
Andi
Tvrtko
Re: [Intel-gfx] [PATCH v10 00/19] drm: Analog TV Improvements
On Mon, Nov 21, 2022 at 03:51:26PM +0100, Daniel Vetter wrote: > On Thu, Nov 17, 2022 at 10:28:43AM +0100, Maxime Ripard wrote: > > Hi, > > > > Here's a series aiming at improving the command line named modes support, > > and more importantly how we deal with all the analog TV variants. > > > > The named modes support were initially introduced to allow to specify the > > analog TV mode to be used. > > > > However, this was causing multiple issues: > > > > * The mode name parsed on the command line was passed directly to the > > driver, which had to figure out which mode it was suppose to match; > > > > * Figuring that out wasn't really easy, since the video= argument or what > > the userspace might not even have a name in the first place, but > > instead could have passed a mode with the same timings; > > > > * The fallback to matching on the timings was mostly working as long as > > we were supporting one 525 lines (most likely NSTC) and one 625 lines > > (PAL), but couldn't differentiate between two modes with the same > > timings (NTSC vs PAL-M vs NSTC-J for example); > > > > * There was also some overlap with the tv mode property registered by > > drm_mode_create_tv_properties(), but named modes weren't interacting > > with that property at all. > > > > * Even though that property was generic, its possible values were > > specific to each drivers, which made some generic support difficult. > > > > Thus, I chose to tackle in multiple steps: > > > > * A new TV mode property was introduced, with generic values, each driver > > reporting through a bitmask what standard it supports to the userspace; > > > > * This option was added to the command line parsing code to be able to > > specify it on the kernel command line, and new atomic_check and reset > > helpers were created to integrate properly into atomic KMS; > > > > * The named mode parsing code is now creating a proper display mode for > > the given named mode, and the TV standard will thus be part of the > > connector state; > > > > * Two drivers were converted and tested for now (vc4 and sun4i), with > > some backward compatibility code to translate the old TV mode to the > > new TV mode; > > > > Unit tests were created along the way. > > > > One can switch from NTSC to PAL now using (on vc4) > > > > modetest -M vc4 -s 53:720x480i -w 53:'TV mode':1 # NTSC > > modetest -M vc4 -s 53:720x576i -w 53:'TV mode':4 # PAL > > > > Let me know what you think, > > Maxime > > Maxime asked me to drop an Ack-in-principle on this, and I'm not sure I > have any useful input here with my utter lack of understanding for TV > things (I never even had one in my entire life, that's how much I don't > care). But it seems to check all the design boxes around solving annoying > uapi/kms-config issues properly, so > > Acked-in-principle-or-something-like-that-by: Daniel Vetter > Thanks! I jumped the gun a bit too fast and forgot to amend the TV property commit message before pushing it out. For the record though, that property is usable through xrandr, xorg.conf or any equivalent compositor mechanism Maxime signature.asc Description: PGP signature
[Intel-gfx] ✓ Fi.CI.BAT: success for Implement workaround for PLL enabling for DG2/MTL
== Series Details ==
Series: Implement workaround for PLL enabling for DG2/MTL
URL : https://patchwork.freedesktop.org/series/111311/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12429 -> Patchwork_111311v1
Summary
---
**SUCCESS**
No regressions found.
External URL:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111311v1/index.html
Participating hosts (36 -> 36)
--
Additional (1): fi-hsw-4770
Missing(1): fi-ctg-p8600
Known issues
Here are the changes found in Patchwork_111311v1 that come from known issues:
### IGT changes ###
Issues hit
* igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
- fi-hsw-4770:NOTRUN -> [SKIP][1] ([fdo#109271]) +11 similar issues
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111311v1/fi-hsw-4770/igt@kms_addfb_ba...@addfb25-y-tiled-small-legacy.html
* igt@kms_chamelium@dp-crc-fast:
- fi-hsw-4770:NOTRUN -> [SKIP][2] ([fdo#109271] / [fdo#111827]) +8
similar issues
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111311v1/fi-hsw-4770/igt@kms_chamel...@dp-crc-fast.html
* igt@kms_psr@sprite_plane_onoff:
- fi-hsw-4770:NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#1072]) +3
similar issues
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111311v1/fi-hsw-4770/igt@kms_psr@sprite_plane_onoff.html
Possible fixes
* igt@gem_exec_suspend@basic-s3@smem:
- {bat-rpls-1}: [DMESG-WARN][4] ([i915#6687]) -> [PASS][5]
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12429/bat-rpls-1/igt@gem_exec_suspend@basic...@smem.html
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111311v1/bat-rpls-1/igt@gem_exec_suspend@basic...@smem.html
* igt@i915_selftest@live@migrate:
- {bat-adlp-6}: [INCOMPLETE][6] ([i915#7348]) -> [PASS][7]
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12429/bat-adlp-6/igt@i915_selftest@l...@migrate.html
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111311v1/bat-adlp-6/igt@i915_selftest@l...@migrate.html
Warnings
* igt@i915_suspend@basic-s3-without-i915:
- fi-rkl-11600: [FAIL][8] ([fdo#103375]) -> [INCOMPLETE][9]
([i915#4817])
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12429/fi-rkl-11600/igt@i915_susp...@basic-s3-without-i915.html
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111311v1/fi-rkl-11600/igt@i915_susp...@basic-s3-without-i915.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4817]: https://gitlab.freedesktop.org/drm/intel/issues/4817
[i915#5278]: https://gitlab.freedesktop.org/drm/intel/issues/5278
[i915#6434]: https://gitlab.freedesktop.org/drm/intel/issues/6434
[i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
[i915#6997]: https://gitlab.freedesktop.org/drm/intel/issues/6997
[i915#7328]: https://gitlab.freedesktop.org/drm/intel/issues/7328
[i915#7348]: https://gitlab.freedesktop.org/drm/intel/issues/7348
Build changes
-
* Linux: CI_DRM_12429 -> Patchwork_111311v1
CI-20190529: 20190529
CI_DRM_12429: 30adb45f7b1eddfcda3ef1de5e8dbcd2c1f5a57d @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_7072: 69ba7163475925cdc69aebbdfa0e87453ae165c7 @
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_111311v1: 30adb45f7b1eddfcda3ef1de5e8dbcd2c1f5a57d @
git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
75d1f65bee73 drm/i915: Implement workaround for CDCLK PLL disable/enable
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111311v1/index.html
[Intel-gfx] ✗ Fi.CI.DOCS: warning for Implement workaround for PLL enabling for DG2/MTL
== Series Details == Series: Implement workaround for PLL enabling for DG2/MTL URL : https://patchwork.freedesktop.org/series/111311/ State : warning == Summary == Error: make htmldocs had i915 warnings ./drivers/gpu/drm/i915/gt/intel_gt_mcr.c:739: warning: expecting prototype for intel_gt_mcr_wait_for_reg_fw(). Prototype was for intel_gt_mcr_wait_for_reg() instead ./drivers/gpu/drm/i915/gt/intel_gt_mcr.c:739: warning: expecting prototype for intel_gt_mcr_wait_for_reg_fw(). Prototype was for intel_gt_mcr_wait_for_reg() instead
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Implement workaround for PLL enabling for DG2/MTL
== Series Details == Series: Implement workaround for PLL enabling for DG2/MTL URL : https://patchwork.freedesktop.org/series/111311/ State : warning == Summary == Error: dim checkpatch failed 526c2d192fcc drm/i915: Implement workaround for CDCLK PLL disable/enable -:25: CHECK:LOGICAL_CONTINUATIONS: Logical continuations should be on the previous line #25: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:1807: + return ((IS_DG2(dev_priv) || IS_METEORLAKE(dev_priv)) + && dev_priv->display.cdclk.hw.vco > 0 -:26: CHECK:LOGICAL_CONTINUATIONS: Logical continuations should be on the previous line #26: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:1808: + && dev_priv->display.cdclk.hw.vco > 0 + && HAS_CDCLK_SQUASH(dev_priv)); -:43: CHECK:BRACES: Unbalanced braces around else statement #43: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:1830: + } else total: 0 errors, 0 warnings, 3 checks, 27 lines checked
Re: [Intel-gfx] [PATCH 12/12] drm/i915/fbc: switch to intel_de_* register accessors in display code
On Wed, Nov 23, 2022 at 11:18:25PM +0200, Jani Nikula wrote:
> Avoid direct uncore use in display code. Use the new
> intel_de_rewrite_fw().
>
> Cc: Maarten Lankhorst
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/display/intel_fbc.c | 10 ++
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
> b/drivers/gpu/drm/i915/display/intel_fbc.c
> index b5ee5ea0d010..6066ac412e6f 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -323,10 +323,7 @@ static void i8xx_fbc_nuke(struct intel_fbc *fbc)
> enum i9xx_plane_id i9xx_plane = fbc_state->plane->i9xx_plane;
> struct drm_i915_private *dev_priv = fbc->i915;
>
> - spin_lock_irq(_priv->uncore.lock);
> - intel_de_write_fw(dev_priv, DSPADDR(i9xx_plane),
> - intel_de_read_fw(dev_priv, DSPADDR(i9xx_plane)));
> - spin_unlock_irq(_priv->uncore.lock);
> + intel_de_rewrite_fw(dev_priv, DSPADDR(i9xx_plane));
intel_de_rewrite_fw() seems to imply some kind of atomicicity guarantee
here. But that entirely depends on whether the other writers of this
register also protect it with unore.lock. So just a misleading illusion.
That said, this locking stuff shouldn't even be needed since
commit de5bd083d247 ("drm/i915/fbc: Skip nuke when flip is pending")
commit 7cfd1a18c5f9 ("drm/i915: Remove remaining locks from i9xx plane udpates")
> }
>
> static void i8xx_fbc_program_cfb(struct intel_fbc *fbc)
> @@ -359,10 +356,7 @@ static void i965_fbc_nuke(struct intel_fbc *fbc)
> enum i9xx_plane_id i9xx_plane = fbc_state->plane->i9xx_plane;
> struct drm_i915_private *dev_priv = fbc->i915;
>
> - spin_lock_irq(_priv->uncore.lock);
> - intel_de_write_fw(dev_priv, DSPSURF(i9xx_plane),
> - intel_de_read_fw(dev_priv, DSPSURF(i9xx_plane)));
> - spin_unlock_irq(_priv->uncore.lock);
> + intel_de_rewrite_fw(dev_priv, DSPSURF(i9xx_plane));
> }
>
> static const struct intel_fbc_funcs i965_fbc_funcs = {
> --
> 2.34.1
--
Ville Syrjälä
Intel
[Intel-gfx] [PATCH 1/1] drm/i915: Implement workaround for CDCLK PLL disable/enable
It was reported that we might get a hung and loss of register access in
some cases when CDCLK PLL is disabled and then enabled, while squashing
is enabled.
As a workaround it was proposed by HW team that SW should disable squashing
when CDCLK PLL is being reenabled.
Signed-off-by: Stanislav Lisovskiy
---
drivers/gpu/drm/i915/display/intel_cdclk.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 0c107a38f9d0..e338f288c9ac 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1801,6 +1801,13 @@ static bool
cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private *i91
return true;
}
+static bool pll_enable_wa_needed(struct drm_i915_private *dev_priv)
+{
+ return ((IS_DG2(dev_priv) || IS_METEORLAKE(dev_priv))
+ && dev_priv->display.cdclk.hw.vco > 0
+ && HAS_CDCLK_SQUASH(dev_priv));
+}
+
static void _bxt_set_cdclk(struct drm_i915_private *dev_priv,
const struct intel_cdclk_config *cdclk_config,
enum pipe pipe)
@@ -1815,9 +1822,12 @@ static void _bxt_set_cdclk(struct drm_i915_private
*dev_priv,
!cdclk_pll_is_unknown(dev_priv->display.cdclk.hw.vco)) {
if (dev_priv->display.cdclk.hw.vco != vco)
adlp_cdclk_pll_crawl(dev_priv, vco);
- } else if (DISPLAY_VER(dev_priv) >= 11)
+ } else if (DISPLAY_VER(dev_priv) >= 11) {
+ if (pll_enable_wa_needed(dev_priv))
+ dg2_cdclk_squash_program(dev_priv, 0);
+
icl_cdclk_pll_update(dev_priv, vco);
- else
+ } else
bxt_cdclk_pll_update(dev_priv, vco);
waveform = cdclk_squash_waveform(dev_priv, cdclk);
--
2.37.3
[Intel-gfx] [PATCH 0/1] Implement workaround for PLL enabling for DG2/MTL
It has been noticed by HW team, that there are might be problems when PLL is being enabled with CDCLK squashing being turned on, which might result in loosing register access and/or FIFO underrun. As a workaround it has been proposed to disable CDCLK squashing right before PLL is enabled and enable squashing later, if needed. Stanislav Lisovskiy (1): drm/i915: Implement workaround for CDCLK PLL disable/enable drivers/gpu/drm/i915/display/intel_cdclk.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) -- 2.37.3
[Intel-gfx] [PULL] drm-misc-fixes
Hey Daniel and Dae, Not much here, a few fixes to dma-fence handling and a fix to amdgpu and logo. Enjoy! Maarten Lankhorst drm-misc-fixes-2022-11-24: drm-misc-fixes for v6.1-rc7: - Another amdgpu gang submit fix. - Use dma_fence_unwrap_for_each when importing sync files. - Fix race in dma_heap_add(). - Fix use of uninitialized memory in logo. The following changes since commit 5954acbacbd1946b96ce8ee799d309cb0cd3cb9d: drm/display: Don't assume dual mode adaptors support i2c sub-addressing (2022-11-15 23:31:02 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2022-11-24 for you to fetch changes up to a6a00d7e8ffd78d1cdb7a43f1278f081038c638f: fbcon: Use kzalloc() in fbcon_prepare_logo() (2022-11-22 15:48:02 +0100) drm-misc-fixes for v6.1-rc7: - Another amdgpu gang submit fix. - Use dma_fence_unwrap_for_each when importing sync files. - Fix race in dma_heap_add(). - Fix use of uninitialized memory in logo. Christian König (1): drm/amdgpu: handle gang submit before VMID Dawei Li (1): dma-buf: fix racing conflict of dma_heap_add() Jason Ekstrand (1): dma-buf: Use dma_fence_unwrap_for_each when importing fences Tetsuo Handa (1): fbcon: Use kzalloc() in fbcon_prepare_logo() drivers/dma-buf/dma-buf.c | 23 +-- drivers/dma-buf/dma-heap.c | 28 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 6 +++--- drivers/video/fbdev/core/fbcon.c| 2 +- 4 files changed, 36 insertions(+), 23 deletions(-)
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/display: Don't disable DDI/Transcoder when setting phy test pattern (rev6)
== Series Details ==
Series: drm/i915/display: Don't disable DDI/Transcoder when setting phy test
pattern (rev6)
URL : https://patchwork.freedesktop.org/series/108636/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12427 -> Patchwork_108636v6
Summary
---
**SUCCESS**
No regressions found.
External URL:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108636v6/index.html
Participating hosts (38 -> 36)
--
Missing(2): fi-ctg-p8600 fi-bdw-gvtdvm
Known issues
Here are the changes found in Patchwork_108636v6 that come from known issues:
### IGT changes ###
Issues hit
* igt@gem_exec_gttfill@basic:
- fi-pnv-d510:[PASS][1] -> [FAIL][2] ([i915#7229])
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12427/fi-pnv-d510/igt@gem_exec_gttf...@basic.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108636v6/fi-pnv-d510/igt@gem_exec_gttf...@basic.html
Possible fixes
* igt@i915_pm_rpm@module-reload:
- {bat-rpls-2}: [DMESG-WARN][3] ([i915#6434]) -> [PASS][4]
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12427/bat-rpls-2/igt@i915_pm_...@module-reload.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108636v6/bat-rpls-2/igt@i915_pm_...@module-reload.html
* igt@i915_selftest@live@gt_lrc:
- {bat-adln-1}: [INCOMPLETE][5] -> [PASS][6]
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12427/bat-adln-1/igt@i915_selftest@live@gt_lrc.html
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108636v6/bat-adln-1/igt@i915_selftest@live@gt_lrc.html
* igt@i915_selftest@live@requests:
- {bat-rpls-1}: [INCOMPLETE][7] ([i915#6257]) -> [PASS][8]
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12427/bat-rpls-1/igt@i915_selftest@l...@requests.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108636v6/bat-rpls-1/igt@i915_selftest@l...@requests.html
* igt@i915_selftest@live@reset:
- {bat-rpls-2}: [DMESG-FAIL][9] ([i915#4983]) -> [PASS][10]
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12427/bat-rpls-2/igt@i915_selftest@l...@reset.html
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108636v6/bat-rpls-2/igt@i915_selftest@l...@reset.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
[i915#6257]: https://gitlab.freedesktop.org/drm/intel/issues/6257
[i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
[i915#6434]: https://gitlab.freedesktop.org/drm/intel/issues/6434
[i915#6559]: https://gitlab.freedesktop.org/drm/intel/issues/6559
[i915#6997]: https://gitlab.freedesktop.org/drm/intel/issues/6997
[i915#7229]: https://gitlab.freedesktop.org/drm/intel/issues/7229
[i915#7348]: https://gitlab.freedesktop.org/drm/intel/issues/7348
Build changes
-
* Linux: CI_DRM_12427 -> Patchwork_108636v6
CI-20190529: 20190529
CI_DRM_12427: 24694cd2b234fb097fda693f13be131116f1a79f @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_7072: 69ba7163475925cdc69aebbdfa0e87453ae165c7 @
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_108636v6: 24694cd2b234fb097fda693f13be131116f1a79f @
git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
7835c7fdf7ce drm/i915/display: Don't disable DDI/Transcoder when setting phy
test pattern
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108636v6/index.html
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915: Never return 0 if not all requests retired
On Wednesday, 23 November 2022 13:57:26 CET Tvrtko Ursulin wrote: > > On 23/11/2022 09:28, Janusz Krzysztofik wrote: > > Hi Tvrtko, > > > > Thanks for your comments. > > > > On Tuesday, 22 November 2022 11:50:38 CET Tvrtko Ursulin wrote: > >> > >> On 21/11/2022 14:56, Janusz Krzysztofik wrote: > >>> Users of intel_gt_retire_requests_timeout() expect 0 return value on > >>> success. However, we have no protection from passing back 0 potentially > >>> returned by a call to dma_fence_wait_timeout() when it succedes right > >>> after its timeout has expired. > >> > >> Is this talking about a potential weakness, or ambiguous kerneldoc, of > >> dma_fence_wait_timeout, dma_fence_default_wait and > >> i915_request_wait_timeout? They appear to say 0 return means timeout, > >> implying unsignaled fence. In other words signaled must return positive > >> remaining timeout. Implementations seems to allow a race which indeed > >> appears that return 0 and signaled fence is possible. > > > > While my initial analysis was indeed focused on inconsistent semantics of 0 > > return values from different dma_fence_default_wait() backends, I should > > have > > also mentioned in this commit description that users may perfectly > > call intel_gt_retire_requests_timeout() with 0 timeout, in which case the > > false positive 0 value can be returned regardless of > > dma_fence_wait_timeout() > > potential issues. Would you like me to reword and resubmit? > > Not sure yet. > > So the only caller which passes in zero to > intel_gt_retire_requests_timeout appears to be intel_gt_retire_requests > and it eats the return value anyway so this patch is immaterial for that > one. Right. > I guess it can change how intel_gt_wait_for_idle behaves with short-ish > timeouts. In case this race is hit. But then wouldn't it make sense to > follow up with a patch which addresses this race by re-checking the "is > signaled" when timeout expires, But inside intel_gt_retire_requests_timeout() we generally don't care if fences have been signaled. As long as user requested timeout hasn't expired, we use dma_fence_wait_timeout() as an aid, otherwise we keep trying to retire requests without waiting on fences. If the retirement succeeds then we return 0 (success) regardless of what the return value from the last called dma_fence_wait_timeout() was. If it was 0 then the only useful information is that no more time has been left, and no matter if that 0 meant signaled or not signaled, we must return an error if there are still some requests not retired, I believe. > either in dma_fence_wait_timeout, or to > intel_gt_retire_requests_timeout. Or if not that at least better > document the dma_fence_wait_timeout and/or > intel_gt_retire_requests_timeout. Makes sense? Documenting -- yes, as soon as we get into an agreement on what's the core of this issue -- whether that potential weakness, or ambiguous kerneldoc, of dma_fence_wait_timeout, dma_fence_default_wait and i915_request_wait_timeout, as you've stated, that we have to address somehow, or potentially incorrect direct use of the timeout variable, intended for storing time left to spend on fence waits, as our return value when timeout has expired. And if the former then maybe we should also try to finally resolve that over a year old conflict (whether 0 means signaled on not signaled) inside our implementation of dma_fence_ops.wait, and simply use a wrapper around it for either our internal use if we decide to follow the reference implementation, or for dma_fence_ops use otherwise. Or maybe the reference implementation should be fixed if problematic. I don't feel competent enough to decide. Thanks, Janusz > > Regards, > > Tvrtko > > > > >> If dma_fence_wait can indeed return 0 even when a request is signaled, > >> then how is timeout ?: -ETIME below correct? It isn't a chance for false > >> negative in its' callers? > > > > The goal of intel_gt_retire_requests_timeout() is to retire requests. When > > that goal has been reached, i.e., all requests have been retired, active > > count > > is 0, and 0 is correctly returned, regardless of timeout value. > > > > The value of timeout is used only when there are still pending requests, > > which > > means that the goal hasn't been reached and the function hasn't succeeded. > > Then, no false negative is possible, unlike the false positive that we now > > have when we return 0 while some requests are still pending. > > > > Thanks, > > Janusz > > > >> > >> Regards, > >> > >> Tvrtko > >> > >>> Replace 0 with -ETIME before potentially using the timeout value as return > >>> code, so -ETIME is returned if there are still some requests not retired > >>> after timeout, 0 otherwise. > >>> > >>> v3: Use conditional expression, more compact but also better reflecting > >>> intention standing behind the change. > >>> > >>> v2: Move the added lines down so flush_submission() is not affected. > >>> > >>>
Re: [Intel-gfx] [PATCH v2 2/4] drm/i915: Introduce guard pages to i915_vma
> > > @@ -768,6 +768,9 @@ i915_vma_insert(struct i915_vma *vma, struct > > > i915_gem_ww_ctx *ww, > > > GEM_BUG_ON(!IS_ALIGNED(alignment, I915_GTT_MIN_ALIGNMENT)); > > > GEM_BUG_ON(!is_power_of_2(alignment)); > > > + guard = vma->guard; /* retain guard across rebinds */ > > > + guard = ALIGN(guard, alignment); > > > > Why does guard area needs the same alignment as the requested mapping? What > > about the fact on 32-bit builds guard is 32-bit and alignment u64? > > I guess this just to round up/down guard to something, not > necessarily to that alignment. > > Shall I remove it? or we could just add a comment to explain that this is just to do some rounding in order to avoid weird values of guard. Andi
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/display: Don't disable DDI/Transcoder when setting phy test pattern (rev6)
== Series Details == Series: drm/i915/display: Don't disable DDI/Transcoder when setting phy test pattern (rev6) URL : https://patchwork.freedesktop.org/series/108636/ State : warning == Summary == Error: dim checkpatch failed f4d1bf1f641a drm/i915/display: Don't disable DDI/Transcoder when setting phy test pattern -:25: WARNING:BAD_SIGN_OFF: Duplicate signature #25: Signed-off-by: Khaled Almahallawy total: 0 errors, 1 warnings, 0 checks, 75 lines checked
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/dp: wait on timeout before retry include sw delay
== Series Details ==
Series: drm/i915/dp: wait on timeout before retry include sw delay
URL : https://patchwork.freedesktop.org/series/111303/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12427 -> Patchwork_111303v1
Summary
---
**SUCCESS**
No regressions found.
External URL:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111303v1/index.html
Participating hosts (38 -> 35)
--
Missing(3): fi-ctg-p8600 fi-bdw-gvtdvm fi-rkl-11600
Known issues
Here are the changes found in Patchwork_111303v1 that come from known issues:
### IGT changes ###
Possible fixes
* igt@i915_selftest@live@gt_lrc:
- {bat-adln-1}: [INCOMPLETE][1] -> [PASS][2]
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12427/bat-adln-1/igt@i915_selftest@live@gt_lrc.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111303v1/bat-adln-1/igt@i915_selftest@live@gt_lrc.html
* igt@i915_selftest@live@gt_pm:
- {bat-rpls-2}: [DMESG-FAIL][3] ([i915#4258]) -> [PASS][4]
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12427/bat-rpls-2/igt@i915_selftest@live@gt_pm.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111303v1/bat-rpls-2/igt@i915_selftest@live@gt_pm.html
* igt@i915_selftest@live@requests:
- {bat-rpls-1}: [INCOMPLETE][5] ([i915#6257]) -> [PASS][6]
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12427/bat-rpls-1/igt@i915_selftest@l...@requests.html
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111303v1/bat-rpls-1/igt@i915_selftest@l...@requests.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
[i915#2867]: https://gitlab.freedesktop.org/drm/intel/issues/2867
[i915#4258]: https://gitlab.freedesktop.org/drm/intel/issues/4258
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
[i915#6257]: https://gitlab.freedesktop.org/drm/intel/issues/6257
[i915#6434]: https://gitlab.freedesktop.org/drm/intel/issues/6434
[i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
[i915#7346]: https://gitlab.freedesktop.org/drm/intel/issues/7346
Build changes
-
* Linux: CI_DRM_12427 -> Patchwork_111303v1
CI-20190529: 20190529
CI_DRM_12427: 24694cd2b234fb097fda693f13be131116f1a79f @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_7072: 69ba7163475925cdc69aebbdfa0e87453ae165c7 @
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_111303v1: 24694cd2b234fb097fda693f13be131116f1a79f @
git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
c26cf159f605 drm/i915/dp: wait on timeout before retry include sw delay
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111303v1/index.html
[Intel-gfx] [PULL] drm-intel-fixes
Hi Dave, Daniel, A couple of fixes for 6.1-rc. One TTM backend fix, one display warning fixlet and a merge of two GVT patches which fix KVM reference count handling there. Regards, Tvrtko drm-intel-fixes-2022-11-24: - Fix GVT KVM reference count handling (Sean Christopherson) - Never purge busy TTM objects (Matthew Auld) - Fix warn in intel_display_power_*_domain() functions (Imre Deak) The following changes since commit eb7081409f94a9a8608593d0fb63a1aa3d6f95d8: Linux 6.1-rc6 (2022-11-20 16:02:16 -0800) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2022-11-24 for you to fetch changes up to 14af5d385878d22546914d37f13a314b14825a42: Merge tag 'gvt-fixes-2022-11-11' of https://github.com/intel/gvt-linux into drm-intel-fixes (2022-11-22 07:59:17 +) - Fix GVT KVM reference count handling (Sean Christopherson) - Never purge busy TTM objects (Matthew Auld) - Fix warn in intel_display_power_*_domain() functions (Imre Deak) Imre Deak (1): drm/i915: Fix warn in intel_display_power_*_domain() functions Matthew Auld (1): drm/i915/ttm: never purge busy objects Sean Christopherson (2): drm/i915/gvt: Get reference to KVM iff attachment to VM is successful drm/i915/gvt: Unconditionally put reference to KVM when detaching vGPU Tvrtko Ursulin (1): Merge tag 'gvt-fixes-2022-11-11' of https://github.com/intel/gvt-linux into drm-intel-fixes drivers/gpu/drm/i915/display/intel_display_power.c | 8 drivers/gpu/drm/i915/gem/i915_gem_ttm.c| 4 drivers/gpu/drm/i915/gvt/kvmgt.c | 8 +++- 3 files changed, 11 insertions(+), 9 deletions(-)
