[Intel-gfx] [PATCH] RFT gem_mmap_gtt/gdg Throw in a wmb
--- drivers/gpu/drm/i915/i915_gem.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 89bf5d67cb74..4c26e1fed6c6 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2089,6 +2089,7 @@ int i915_gem_fault(struct vm_fault *vmf) GEM_BUG_ON(!obj->userfault_count); i915_vma_set_ggtt_write(vma); + wmb(); err_fence: i915_vma_unpin_fence(vma); -- 2.17.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 RFT gem_mmap_gtt/gdg Throw in a wmb
== Series Details == Series: RFT gem_mmap_gtt/gdg Throw in a wmb URL : https://patchwork.freedesktop.org/series/42988/ State : warning == Summary == $ dim checkpatch origin/drm-tip 36ebf10700bc RFT gem_mmap_gtt/gdg Throw in a wmb -:15: WARNING:MEMORY_BARRIER: memory barrier without comment #15: FILE: drivers/gpu/drm/i915/i915_gem.c:2092: + wmb(); -:18: ERROR:MISSING_SIGN_OFF: Missing Signed-off-by: line(s) total: 1 errors, 1 warnings, 0 checks, 7 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] RFT gem_mmap_gtt/gdg: 2 tile rows
--- drivers/gpu/drm/i915/i915_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 89bf5d67cb74..d92857c879c1 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1958,7 +1958,7 @@ compute_partial_view(struct drm_i915_gem_object *obj, struct i915_ggtt_view view; if (i915_gem_object_is_tiled(obj)) - chunk = roundup(chunk, tile_row_pages(obj)); + chunk = roundup(chunk, 2 * tile_row_pages(obj)); view.type = I915_GGTT_VIEW_PARTIAL; view.partial.offset = rounddown(page_offset, chunk); -- 2.17.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for RFT gem_mmap_gtt/gdg Throw in a wmb
== Series Details == Series: RFT gem_mmap_gtt/gdg Throw in a wmb URL : https://patchwork.freedesktop.org/series/42988/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4163 -> Patchwork_8970 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/42988/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_8970 that come from known issues: === IGT changes === Issues hit igt@drv_module_reload@basic-reload: fi-glk-j4005: PASS -> DMESG-WARN (fdo#106248) igt@kms_frontbuffer_tracking@basic: {fi-hsw-peppy}: PASS -> DMESG-FAIL (fdo#106103, fdo#102614) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: fi-glk-j4005: PASS -> DMESG-WARN (fdo#106097) igt@prime_self_import@basic-with_one_bo_two_files: fi-glk-j4005: PASS -> DMESG-WARN (fdo#105719) Possible fixes igt@drv_module_reload@basic-reload-inject: fi-glk-j4005: DMESG-WARN (fdo#106248) -> PASS igt@kms_flip@basic-flip-vs-wf_vblank: fi-hsw-4770r: FAIL (fdo#100368) -> PASS {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614 fdo#105719 https://bugs.freedesktop.org/show_bug.cgi?id=105719 fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097 fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103 fdo#106248 https://bugs.freedesktop.org/show_bug.cgi?id=106248 == Participating hosts (41 -> 37) == Missing(4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-skl-6700hq == Build changes == * Linux: CI_DRM_4163 -> Patchwork_8970 CI_DRM_4163: 8e1dab6e913be7d014eb9bc355ec65b6b56dcd56 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4468: 548a894dc904c4628522dbbc77cb179a4c965ebc @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_8970: 36ebf10700bc0ce26ff92db5755f0b649180b00c @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4468: 1e60f1499e5b71b6d5a747189d7c28f57359a87f @ git://anongit.freedesktop.org/piglit == Linux commits == 36ebf10700bc RFT gem_mmap_gtt/gdg Throw in a wmb == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8970/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] RFT gem_mmap_gtt/gdg Throw in a wmb
Quoting Chris Wilson (2018-05-10 08:47:31) > --- > drivers/gpu/drm/i915/i915_gem.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 89bf5d67cb74..4c26e1fed6c6 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2089,6 +2089,7 @@ int i915_gem_fault(struct vm_fault *vmf) > GEM_BUG_ON(!obj->userfault_count); > > i915_vma_set_ggtt_write(vma); > + wmb(); gdg still fails. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for RFT gem_mmap_gtt/gdg: 2 tile rows
== Series Details == Series: RFT gem_mmap_gtt/gdg: 2 tile rows URL : https://patchwork.freedesktop.org/series/42990/ State : warning == Summary == $ dim checkpatch origin/drm-tip 4c164840dbb9 RFT gem_mmap_gtt/gdg: 2 tile rows -:19: ERROR:MISSING_SIGN_OFF: Missing Signed-off-by: line(s) total: 1 errors, 0 warnings, 0 checks, 8 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for RFT gem_mmap_gtt/gdg: 2 tile rows
== Series Details == Series: RFT gem_mmap_gtt/gdg: 2 tile rows URL : https://patchwork.freedesktop.org/series/42990/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4163 -> Patchwork_8971 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/42990/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_8971 that come from known issues: === IGT changes === Issues hit igt@drv_module_reload@basic-reload: fi-glk-j4005: PASS -> DMESG-WARN (fdo#106248) igt@kms_busy@basic-flip-b: fi-glk-j4005: PASS -> FAIL (fdo#103182) igt@kms_pipe_crc_basic@hang-read-crc-pipe-c: fi-glk-j4005: PASS -> DMESG-WARN (fdo#106235) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: fi-snb-2520m: PASS -> INCOMPLETE (fdo#103713) Possible fixes igt@drv_module_reload@basic-reload-inject: fi-glk-j4005: DMESG-WARN (fdo#106248) -> PASS igt@kms_flip@basic-flip-vs-wf_vblank: fi-hsw-4770r: FAIL (fdo#100368) -> PASS fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#103182 https://bugs.freedesktop.org/show_bug.cgi?id=103182 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#106235 https://bugs.freedesktop.org/show_bug.cgi?id=106235 fdo#106248 https://bugs.freedesktop.org/show_bug.cgi?id=106248 == Participating hosts (41 -> 37) == Missing(4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-skl-6700hq == Build changes == * Linux: CI_DRM_4163 -> Patchwork_8971 CI_DRM_4163: 8e1dab6e913be7d014eb9bc355ec65b6b56dcd56 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4468: 548a894dc904c4628522dbbc77cb179a4c965ebc @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_8971: 4c164840dbb9075ee94e40f7e7073fca63da46c4 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4468: 1e60f1499e5b71b6d5a747189d7c28f57359a87f @ git://anongit.freedesktop.org/piglit == Linux commits == 4c164840dbb9 RFT gem_mmap_gtt/gdg: 2 tile rows == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8971/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] RFT gem_mmap_gtt/gdg: 2 tile rows
Quoting Chris Wilson (2018-05-10 09:13:02) > --- > drivers/gpu/drm/i915/i915_gem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 89bf5d67cb74..d92857c879c1 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -1958,7 +1958,7 @@ compute_partial_view(struct drm_i915_gem_object *obj, > struct i915_ggtt_view view; > > if (i915_gem_object_is_tiled(obj)) > - chunk = roundup(chunk, tile_row_pages(obj)); > + chunk = roundup(chunk, 2 * tile_row_pages(obj)); Also no silver bullet. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/dp: add module parameter for the dpcd access max retries
On Wed, May 09, 2018 at 12:28:15PM +0300, Jani Nikula wrote: > On Wed, 09 May 2018, Feng Tang wrote: > > On Wed, May 09, 2018 at 10:53:53AM +0300, Jani Nikula wrote: > >> On Wed, 09 May 2018, Feng Tang wrote: > >> > On Tue, May 08, 2018 at 10:30:19PM +0300, Jani Nikula wrote: > >> >> On Wed, 09 May 2018, Feng Tang wrote: > >> >> >> Well if it's edp probing, then atm we do need to block since we have > >> >> >> no support for panel hotplugging. And userspace generally no > >> >> >> expectations that built-in panels come&go. async_synchronize_full > >> >> >> making our fbdev code less async than desired is kinda a different > >> >> >> story I think here. First step would be trying to figure out why we > >> >> >> even bother with edp probing on this platform, when the thing isn't > >> >> >> there. Sounds like broken VBT. > >> >> > > >> >> > Hi Daniel, > >> >> > > >> >> > Here are some of the VBT and DPCD related logs on my A3900 (bxt + > >> >> > GEN9 LP) > >> >> > based NUC. but I don't have the knowledge to tell if the VBT is > >> >> > broken :) > >> >> > >> >> Please run current upstream drm-tip when you're suggesting changes to > >> >> upstream code. Looks like you're running at most v4.14. This can't be > >> >> emphasized enough. We can't and won't merge the changes unless they make > >> >> sense with current code. > >> > > >> > Yes, I understand, the patch posted was created right after git-pulling > >> > Linus' tree, and back-ported to test with 4.14 kernel. > >> > > >> >> > >> >> As to vbt, please send me /sys/kernel/debug/dri/0/i915_vbt. > >> > > >> > Sure. as attached > >> > >> Your VBT claims the device has an eDP panel. Does it have one or not? > > > > After asking around, our platform (BXT NUC) does have a eDP interface > > (someone > > has tested with a eDP screen), but most of our platforms are connected > > with 2 HDMI LCD monitors. > > Sounds like you should have a different VBT for the cases where you ship > with/without eDP connected. As you've noticed, we generally try pretty Yep, this is a good idea. Currently I'm not able to change VBT, so I hacked the code to simulating a no-eDP VBT like: --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -1261,7 +1261,8 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, is_crt = child->device_type & DEVICE_TYPE_ANALOG_OUTPUT; is_hdmi = is_dvi && (child->device_type & DEVICE_TYPE_NOT_HDMI_OUTPUT) == 0; - is_edp = is_dp && (child->device_type & DEVICE_TYPE_INTERNAL_CONNECTOR); + is_edp = 0; And it does cut the boottime a lot!! as avoiding the dpcd access. And later i915_hpd_poll_init_work() will still call intel_dp_detect() and call the time-eater drm_dp_dpcd_access() finally, but the situation is better as it runs in an async way at this point. Thanks, Feng ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v7 3/6] drm/i915: Add skl_check_nv12_surface for NV12
> -Original Message- > From: Srinivas, Vidya > Sent: Tuesday, May 8, 2018 5:08 PM > To: 'Maarten Lankhorst' ; 'intel- > g...@lists.freedesktop.org' > Subject: RE: [PATCH v7 3/6] drm/i915: Add skl_check_nv12_surface for NV12 > > > > > -Original Message- > > From: Srinivas, Vidya > > Sent: Tuesday, May 8, 2018 8:36 AM > > To: Maarten Lankhorst ; intel- > > g...@lists.freedesktop.org > > Subject: RE: [PATCH v7 3/6] drm/i915: Add skl_check_nv12_surface for > > NV12 > > > > > > > > > -Original Message- > > > From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com] > > > Sent: Monday, May 7, 2018 2:56 PM > > > To: Srinivas, Vidya ; intel- > > > g...@lists.freedesktop.org > > > Subject: Re: [PATCH v7 3/6] drm/i915: Add skl_check_nv12_surface for > > > NV12 > > > > > > Op 10-05-18 om 10:31 schreef Vidya Srinivas: > > > > From: Maarten Lankhorst > > > > > > > > We skip src trunction/adjustments for > > > > NV12 case and handle the sizes directly. > > > > Without this, pipe fifo underruns are seen on APL/KBL. > > > > > > > > v2: For NV12, making the src coordinates multiplier of 4 > > > > > > > > v3: Moving all the src coords handling code for NV12 to > > > > skl_check_nv12_surface > > > > > > > > v4: Added RB from Mika > > > > > > > > v5: Rebased the series. Removed checks of mult of 4 in > > > > skl_update_scaler, Added NV12 condition in > > > > intel_check_sprite_plane where src x/w is being checked for mult of 2 > for yuv planes. > > > > > > > > Reviewed-by: Mika Kahola > > > > Signed-off-by: Maarten Lankhorst > > > > > > > > Signed-off-by: Vidya Srinivas > > > > --- > > > > drivers/gpu/drm/i915/intel_display.c | 42 > > > > ++-- > > > > drivers/gpu/drm/i915/intel_sprite.c | 1 + > > > > 2 files changed, 41 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > > b/drivers/gpu/drm/i915/intel_display.c > > > > index dfca71e..cca46f9 100644 > > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > > @@ -3102,6 +3102,42 @@ static int skl_check_main_surface(const > > > > struct > > > intel_crtc_state *crtc_state, > > > > return 0; > > > > } > > > > > > > > +static int > > > > +skl_check_nv12_surface(const struct intel_crtc_state *crtc_state, > > > > + struct intel_plane_state *plane_state) { > > > > + int crtc_x2 = plane_state->base.crtc_x + > > > > plane_state->base.crtc_w; > > > > + int crtc_y2 = plane_state->base.crtc_y + > > > > +plane_state->base.crtc_h; > > > > + > > > > + if (((plane_state->base.src_x >> 16) % 4) != 0 || > > > > + ((plane_state->base.src_y >> 16) % 4) != 0 || > > > > + ((plane_state->base.src_w >> 16) % 4) != 0 || > > > > + ((plane_state->base.src_h >> 16) % 4) != 0) { > > > > + DRM_DEBUG_KMS("src coords must be multiple of 4 for > > > NV12\n"); > > > > + return -EINVAL; > > > > + } > > > > + > > > > + /* Clipping would cause a 1-3 pixel gap at the edge of the > > > > screen? */ > > > > + if ((crtc_x2 > crtc_state->pipe_src_w && crtc_state->pipe_src_w > > > > +% > > > 4) || > > > > + (crtc_y2 > crtc_state->pipe_src_h && crtc_state->pipe_src_h > > > > +% > > > > +4)) > > > { > > > > + DRM_DEBUG_KMS("It's not possible to clip %u,%u to > > > %u,%u\n", > > > > + crtc_x2, crtc_y2, > > > > + crtc_state->pipe_src_w, > > > > crtc_state->pipe_src_h); > > > > + return -EINVAL; > > > > + } > > > Oops, wrong checks here.. > > > > > > skl_check_nv12_surface is only needed for Display WA #1106, so check > > > might need to be something like: > > > > > > static int > > > skl_check_nv12_surface(const struct intel_crtc_state *crtc_state, > > > struct intel_plane_state *plane_state) { > > > /* Display WA #1106 */ > > > if (plane_state->base.rotation != (DRM_MODE_REFLECT_X | > > > DRM_MODE_ROTATE_90) && > > > plane_state->base.rotation != DRM_MODE_ROTATE_270) > > > return 0; > > > > > > /* src coordinates are rotated here. We check height but report it > > > as width. */ > > > if (((drm_rect_height(&plane_state->base.src) >> 16) % 4) != 0) { > > > DRM_DEBUG_KMS("src width must be multiple of 4 for > > rotated NV12\n"); > > > return -EINVAL; > > > } > > > > > > return 0; > > > } > > > > > > Would this hit FIFO underruns? > > > > Thank you. I have made the change and floated the series. Please have > > a check. > > When I tested It on my end on GLK, I did not observe any fifo > > underruns. Will wait for IGT test results from BAT. > > > > IGT BAT shows PASS on rev 6 of > https://patchwork.freedesktop.org/series/41674/ > This has the change for skl_check_nv12_surface. Can you please have a > check? > Thank you. Sorry to ask. Can these go in for merge? I
[Intel-gfx] [PATCH i-g-t] igt/gem_exec_latency: Report stddev for rthog
Report and compare stddev instead of variance so that direct submission passes ;) --- tests/gem_exec_latency.c | 19 +-- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/tests/gem_exec_latency.c b/tests/gem_exec_latency.c index 547d728b3..757b390d0 100644 --- a/tests/gem_exec_latency.c +++ b/tests/gem_exec_latency.c @@ -429,7 +429,7 @@ rthog_latency_on_ring(int fd, unsigned int ring, const char *name) igt_warn("Failed to set scheduling policy!\n"); msg.status = RT_FAIL; write(link[1], &msg, sizeof(msg)); - exit(1); + break; } } @@ -439,7 +439,7 @@ rthog_latency_on_ring(int fd, unsigned int ring, const char *name) passname[pass]); msg.status = RT_FAIL; write(link[1], &msg, sizeof(msg)); - exit(1); + break; } igt_spin_busywait_until_running(spin); @@ -457,7 +457,7 @@ rthog_latency_on_ring(int fd, unsigned int ring, const char *name) igt_warn("Wait timeout! (%s)\n", passname[pass]); write(link[1], &msg, sizeof(msg)); - exit(1); + break; } if (t > msg.max) @@ -468,17 +468,15 @@ rthog_latency_on_ring(int fd, unsigned int ring, const char *name) igt_spin_batch_free(fd, spin); - igt_info("%10s: mean=%.2fus variance=%.2fus max=%.2fus (n=%lu)\n", + igt_info("%10s: mean=%.2fus stddev=%.3fus max=%.2fus (n=%lu)\n", passname[pass], igt_mean_get(&msg.mean) * 1e6, -igt_mean_get_variance(&msg.mean) * 1e6, +sqrt(igt_mean_get_variance(&msg.mean)) * 1e6, msg.max * 1e6, msg.mean.count); write(link[1], &msg, sizeof(msg)); } while (++pass < 3); - - exit(0); } for (i = 0; i < 3; i++) { @@ -494,11 +492,12 @@ rthog_latency_on_ring(int fd, unsigned int ring, const char *name) igt_waitchildren(); /* -* Check that the submission latency variance for a task with RT -* priority is no larger than three times the same of a normal task. +* Check that the submission latency stddev for a task with RT +* priority is no larger than three times the same of a normal task, +* (since variance is the square of stddev, we check 9 times) */ igt_assert(igt_mean_get_variance(&res[2].mean) < - igt_mean_get_variance(&res[1].mean) * 3); + igt_mean_get_variance(&res[1].mean) * 9); } igt_main -- 2.17.0 ___ 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/dp: add module parameter for the dpcd access max retries (rev2)
== Series Details == Series: drm/dp: add module parameter for the dpcd access max retries (rev2) URL : https://patchwork.freedesktop.org/series/42803/ State : failure == Summary == Applying: drm/dp: add module parameter for the dpcd access max retries error: corrupt patch at line 9 error: could not build fake ancestor Patch failed at 0001 drm/dp: add module parameter for the dpcd access max retries The copy of the patch that failed is found in: .git/rebase-apply/patch 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
[Intel-gfx] ✓ Fi.CI.IGT: success for RFT gem_mmap_gtt/gdg Throw in a wmb
== Series Details == Series: RFT gem_mmap_gtt/gdg Throw in a wmb URL : https://patchwork.freedesktop.org/series/42988/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4163_full -> Patchwork_8970_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_8970_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_8970_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/42988/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_8970_full: === IGT changes === Warnings igt@gem_exec_schedule@deep-bsd2: shard-kbl: SKIP -> PASS +1 igt@kms_force_connector_basic@force-connector-state: shard-snb: PASS -> SKIP igt@kms_properties@plane-properties-legacy: shard-snb: SKIP -> PASS +4 == Known issues == Here are the changes found in Patchwork_8970_full that come from known issues: === IGT changes === Issues hit igt@kms_flip@flip-vs-expired-vblank: shard-glk: PASS -> FAIL (fdo#102887, fdo#105363) igt@kms_flip@flip-vs-wf_vblank-interruptible: shard-glk: PASS -> FAIL (fdo#100368) igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc: shard-apl: PASS -> FAIL (fdo#104724, fdo#103167) Possible fixes igt@kms_flip@absolute-wf_vblank-interruptible: shard-glk: FAIL (fdo#106087) -> PASS igt@kms_flip@flip-vs-expired-vblank-interruptible: shard-apl: FAIL (fdo#102887, fdo#105363) -> PASS shard-glk: FAIL (fdo#102887) -> PASS igt@kms_flip@wf_vblank-ts-check-interruptible: shard-apl: FAIL (fdo#100368) -> PASS fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#106087 https://bugs.freedesktop.org/show_bug.cgi?id=106087 == Participating hosts (9 -> 9) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4163 -> Patchwork_8970 CI_DRM_4163: 8e1dab6e913be7d014eb9bc355ec65b6b56dcd56 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4468: 548a894dc904c4628522dbbc77cb179a4c965ebc @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_8970: 36ebf10700bc0ce26ff92db5755f0b649180b00c @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4468: 1e60f1499e5b71b6d5a747189d7c28f57359a87f @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8970/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for RFT gem_mmap_gtt/gdg: 2 tile rows
== Series Details == Series: RFT gem_mmap_gtt/gdg: 2 tile rows URL : https://patchwork.freedesktop.org/series/42990/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4163_full -> Patchwork_8971_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_8971_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_8971_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/42990/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_8971_full: === IGT changes === Warnings igt@gem_exec_schedule@deep-bsd2: shard-kbl: SKIP -> PASS +2 igt@kms_force_connector_basic@force-connector-state: shard-snb: PASS -> SKIP igt@kms_properties@plane-properties-legacy: shard-snb: SKIP -> PASS +4 igt@pm_rc6_residency@rc6-accuracy: shard-kbl: PASS -> SKIP == Known issues == Here are the changes found in Patchwork_8971_full that come from known issues: === IGT changes === Issues hit igt@kms_flip@2x-flip-vs-modeset: shard-hsw: PASS -> DMESG-WARN (fdo#102614) igt@kms_flip@flip-vs-wf_vblank-interruptible: shard-glk: PASS -> FAIL (fdo#100368) igt@kms_flip@plain-flip-fb-recreate-interruptible: shard-hsw: PASS -> FAIL (fdo#100368) igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render: shard-kbl: PASS -> DMESG-WARN (fdo#103558, fdo#105602, fdo#103313) +6 igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen: shard-kbl: PASS -> DMESG-WARN (fdo#103558, fdo#105602) +2 igt@kms_pipe_crc_basic@read-crc-pipe-b: shard-apl: PASS -> FAIL (fdo#104724) igt@pm_rpm@dpms-mode-unset-non-lpsp: shard-kbl: PASS -> DMESG-WARN (fdo#103558, fdo#103313) Possible fixes igt@kms_flip@absolute-wf_vblank-interruptible: shard-glk: FAIL (fdo#106087) -> PASS igt@kms_flip@flip-vs-expired-vblank-interruptible: shard-apl: FAIL (fdo#105363, fdo#102887) -> PASS shard-glk: FAIL (fdo#102887) -> PASS igt@kms_flip@wf_vblank-ts-check-interruptible: shard-glk: FAIL (fdo#100368) -> PASS shard-apl: FAIL (fdo#100368) -> PASS igt@kms_setmode@basic: shard-kbl: FAIL (fdo#99912) -> PASS fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614 fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887 fdo#103313 https://bugs.freedesktop.org/show_bug.cgi?id=103313 fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558 fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602 fdo#106087 https://bugs.freedesktop.org/show_bug.cgi?id=106087 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (9 -> 9) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4163 -> Patchwork_8971 CI_DRM_4163: 8e1dab6e913be7d014eb9bc355ec65b6b56dcd56 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4468: 548a894dc904c4628522dbbc77cb179a4c965ebc @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_8971: 4c164840dbb9075ee94e40f7e7073fca63da46c4 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4468: 1e60f1499e5b71b6d5a747189d7c28f57359a87f @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8971/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/9] trace.pl: Improve readability of graphical timeline representation
From: Tvrtko Ursulin We add stripes for different stages of request execution so it is easier to follow one context in the multi-colour mode. Vertical stripe pattern indicates pipeline "blockages" - requests waiting for dependencies before they are runnable. Diagonal stripes indicate runnable requests waiting for GPU time. Horizontal strips are requests executing on the GPU. Signed-off-by: Tvrtko Ursulin --- scripts/media-bench.pl | 2 +- scripts/trace.pl | 29 +++-- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/scripts/media-bench.pl b/scripts/media-bench.pl index 78f45199e95d..c07555b0dc74 100755 --- a/scripts/media-bench.pl +++ b/scripts/media-bench.pl @@ -207,7 +207,7 @@ sub trace_workload show_cmd($cmd); system($cmd); - $cmd = "perf script | $tracepl --html -x ctxsave -s --squash-ctx-id "; + $cmd = "perf script | $tracepl --html -x ctxsave -s -c --squash-ctx-id "; $cmd .= join ' ', map("-i $_", @skip_engine); $cmd .= " > ${file}.html"; show_cmd($cmd); diff --git a/scripts/trace.pl b/scripts/trace.pl index c8182a8ea86d..7f181a3fa2f5 100755 --- a/scripts/trace.pl +++ b/scripts/trace.pl @@ -743,9 +743,9 @@ foreach my $key (keys %reqwait) { say sprintf('GPU: %.2f%% idle, %.2f%% busy', $flat_busy{'gpu-idle'}, $flat_busy{'gpu-busy'}) unless $html; -my $queued_colour = $colour_contexts ? 'multi-colour light' : 'blue'; -my $runnable_colour = $colour_contexts ? 'multi-colour dark' : 'grey'; -my $execute_colour = $colour_contexts ? 'multi-colour' : 'pink'; +my $queued_colour = $colour_contexts ? 'multi-colour light, vertical stripes' : 'blue'; +my $runnable_colour = $colour_contexts ? 'multi-colour dark, angled stripes' : 'grey'; +my $execute_colour = $colour_contexts ? 'multi-colour, horizontal stripes' : 'pink'; print < @@ -896,7 +896,19 @@ sub ctx_colour $val = int(360 / ($max_ctx - $min_ctx + 1)) * ($ctx - $min_ctx); - return "hsl($val, $s%, $l%);"; + return "hsl($val, $s%, $l%)"; +} + +sub box_style +{ + my ($ctx, $deg, $s, $l) = @_; + + return 'color: black; background: repeating-linear-gradient(' . + $deg . 'deg, ' . + ctx_colour($ctx, $s, $l) . ', ' . + ctx_colour($ctx, $s, $l) . ' 10px, ' . + ctx_colour($ctx, $s, int($l * 0.90)) . ' 10px, ' . + ctx_colour($ctx, $s, int($l * 0.90)) . ' 20px);'; } my $i = 0; @@ -913,8 +925,7 @@ foreach my $key (sort sortQueue keys %db) { # submit to execute unless (exists $skip_box{'queue'}) { $skey = 2 * $max_seqno * $ctx + 2 * $seqno; - $style = 'color: black; background-color: ' . -ctx_colour($ctx, 35, 85); + $style = box_style($ctx, 90, 35, 85); $content = "$name$db{$key}->{'submit-delay'}us ($db{$key}->{'execute-delay'}us)"; $startend = 'start: \'' . ts($queue) . '\', end: \'' . ts($submit) . '\''; print "\t{id: $i, key: $skey, $type group: $group, subgroup: 1, subgroupOrder: 1, content: '$content', $startend, style: \'$style\'},\n"; @@ -924,8 +935,7 @@ foreach my $key (sort sortQueue keys %db) { # execute to start unless (exists $skip_box{'ready'}) { $skey = 2 * $max_seqno * $ctx + 2 * $seqno + 1; - $style = 'color: black; background-color: ' . -ctx_colour($ctx, 35, 45); + $style = box_style($ctx, 45, 35, 45); $content = "$name$db{$key}->{'execute-delay'}us"; $startend = 'start: \'' . ts($submit) . '\', end: \'' . ts($start) . '\''; print "\t{id: $i, key: $skey, $type group: $group, subgroup: 1, subgroupOrder: 2, content: '$content', $startend, style: \'$style\'},\n"; @@ -938,8 +948,7 @@ foreach my $key (sort sortQueue keys %db) { if (exists $db{$key}->{'incomplete'}) { $style = 'color: white; background-color: red;'; } else { - $style = 'color: black; background-color: ' . - ctx_colour($ctx, 80, 65); + $style = box_style($ctx, 0, 80, 65); } $content = "$name $db{$key}->{'port'}"; $content .= ' ??? ' if exists $db{$key}->{'incomplete'}; -- 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 1/9] trace.pl: Add support for colouring context execution
From: Tvrtko Ursulin Add the command line switch which uses different colours for different context execution boxes. v2: * Use HSL to simplify color generation. (Lionel) * Colour other boxes in the same colour but different shade so it is easier to follow the timeline. Signed-off-by: Tvrtko Ursulin Cc: John Harrison Reviewed-by: Lionel Landwerlin # v1 --- scripts/trace.pl | 54 -- 1 file changed, 48 insertions(+), 6 deletions(-) diff --git a/scripts/trace.pl b/scripts/trace.pl index 27b39efcebbd..c8182a8ea86d 100755 --- a/scripts/trace.pl +++ b/scripts/trace.pl @@ -40,6 +40,7 @@ my $trace = 0; my $avg_delay_stats = 0; my $squash_context_id = 0; my $gpu_timeline = 0; +my $colour_contexts = 0; my @args; @@ -110,6 +111,8 @@ Usage: --squash-ctx-id Squash context id by substracting engine id from ctx id. --gpu-timeline Draw overall GPU busy timeline. + --colour-contexts / -c Use different colours for different + context execution boxes. ENDHELP exit 0; @@ -279,6 +282,20 @@ sub arg_skip_box return @_; } +sub arg_colour_contexts +{ + return unless scalar(@_); + + if ($_[0] eq '--colour-contexts' or + $_[0] eq '--color-contexts' or + $_[0] eq '-c') { + shift @_; + $colour_contexts = 1; + } + + return @_; +} + @args = @ARGV; while (@args) { my $left = scalar(@args); @@ -294,6 +311,7 @@ while (@args) { @args = arg_split_requests(@args); @args = arg_ignore_ring(@args); @args = arg_skip_box(@args); + @args = arg_colour_contexts(@args); last if $left == scalar(@args); } @@ -581,6 +599,7 @@ foreach my $key (@sorted_keys) { my $last_ts = 0; my $first_ts; +my ($min_ctx, $max_ctx); foreach my $key (@sorted_keys) { my $ring = $db{$key}->{'ring'}; @@ -590,6 +609,10 @@ foreach my $key (@sorted_keys) { $first_ts = $db{$key}->{'queue'} if not defined $first_ts or $db{$key}->{'queue'} < $first_ts; $last_ts = $end if $end > $last_ts; + $min_ctx = $db{$key}->{'ctx'} if not defined $min_ctx or +$db{$key}->{'ctx'} < $min_ctx; + $max_ctx = $db{$key}->{'ctx'} if not defined $max_ctx or +$db{$key}->{'ctx'} > $max_ctx; $db{$key}->{'context-complete-delay'} = $end - $notify; $db{$key}->{'execute-delay'} = $start - $db{$key}->{'submit'}; @@ -720,6 +743,10 @@ foreach my $key (keys %reqwait) { say sprintf('GPU: %.2f%% idle, %.2f%% busy', $flat_busy{'gpu-idle'}, $flat_busy{'gpu-busy'}) unless $html; +my $queued_colour = $colour_contexts ? 'multi-colour light' : 'blue'; +my $runnable_colour = $colour_contexts ? 'multi-colour dark' : 'grey'; +my $execute_colour = $colour_contexts ? 'multi-colour' : 'pink'; + print < @@ -740,9 +767,9 @@ print{'name'}, $db{$key}->{'ctx'}, $db{$key}->{'seqno'}); @@ -874,7 +913,8 @@ foreach my $key (sort sortQueue keys %db) { # submit to execute unless (exists $skip_box{'queue'}) { $skey = 2 * $max_seqno * $ctx + 2 * $seqno; - $style = 'color: black; background-color: lightblue;'; + $style = 'color: black; background-color: ' . +ctx_colour($ctx, 35, 85); $content = "$name$db{$key}->{'submit-delay'}us ($db{$key}->{'execute-delay'}us)"; $startend = 'start: \'' . ts($queue) . '\', end: \'' . ts($submit) . '\''; print "\t{id: $i, key: $skey, $type group: $group, subgroup: 1, subgroupOrder: 1, content: '$content', $startend, style: \'$style\'},\n"; @@ -884,7 +924,8 @@ foreach my $key (sort sortQueue keys %db) { # execute to start unless (exists $skip_box{'ready'}) { $skey = 2 * $max_seqno * $ctx + 2 * $seqno + 1; - $style = 'color: black; background-color: lightgrey;'; + $style = 'c
[Intel-gfx] [PATCH i-g-t 4/9] trace.pl: Fix engine busy accounting in split mode
From: Tvrtko Ursulin In split mode all requests have to be added up since they were previously re-arranged so there is no overlap. Signed-off-by: Tvrtko Ursulin Cc: John Harrison --- scripts/trace.pl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/trace.pl b/scripts/trace.pl index fae94d5044ef..696c18e8b1cf 100755 --- a/scripts/trace.pl +++ b/scripts/trace.pl @@ -601,7 +601,8 @@ foreach my $key (@sorted_keys) { $db{$key}->{'submit-delay'} = $db{$key}->{'submit'} - $db{$key}->{'queue'}; $db{$key}->{'duration'} = $notify - $start; - $running{$ring} += $end - $start unless exists $db{$key}->{'no-end'}; + $running{$ring} += $end - $start if $correct_durations or + not exists $db{$key}->{'no-end'}; $runnable{$ring} += $db{$key}->{'execute-delay'}; $queued{$ring} += $start - $db{$key}->{'execute-delay'} - $db{$key}->{'queue'}; -- 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 3/9] trace.pl: Remove context squashing option
From: Tvrtko Ursulin Timeline id allocation order is not tied with engine ids any more. Remove the option which assumed that was the case in attempt to provide more readable timeline. Signed-off-by: Tvrtko Ursulin --- scripts/media-bench.pl | 2 +- scripts/trace.pl | 18 -- 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/scripts/media-bench.pl b/scripts/media-bench.pl index c07555b0dc74..375844d9cdf6 100755 --- a/scripts/media-bench.pl +++ b/scripts/media-bench.pl @@ -207,7 +207,7 @@ sub trace_workload show_cmd($cmd); system($cmd); - $cmd = "perf script | $tracepl --html -x ctxsave -s -c --squash-ctx-id "; + $cmd = "perf script | $tracepl --html -x ctxsave -s -c "; $cmd .= join ' ', map("-i $_", @skip_engine); $cmd .= " > ${file}.html"; show_cmd($cmd); diff --git a/scripts/trace.pl b/scripts/trace.pl index 7f181a3fa2f5..fae94d5044ef 100755 --- a/scripts/trace.pl +++ b/scripts/trace.pl @@ -38,7 +38,6 @@ my %skip_box; my $html = 0; my $trace = 0; my $avg_delay_stats = 0; -my $squash_context_id = 0; my $gpu_timeline = 0; my $colour_contexts = 0; @@ -108,8 +107,6 @@ Usage: --html Generate HTML output. --trace cmd Trace the following command. --avg-delay-statsPrint average delay stats. - --squash-ctx-id Squash context id by substracting engine - id from ctx id. --gpu-timeline Draw overall GPU busy timeline. --colour-contexts / -c Use different colours for different context execution boxes. @@ -145,18 +142,6 @@ sub arg_avg_delay_stats return @_; } -sub arg_squash_ctx_id -{ - return unless scalar(@_); - - if ($_[0] eq '--squash-ctx-id') { - shift @_; - $squash_context_id = 1; - } - - return @_; -} - sub arg_gpu_timeline { return unless scalar(@_); @@ -303,7 +288,6 @@ while (@args) { @args = arg_help(@args); @args = arg_html(@args); @args = arg_avg_delay_stats(@args); - @args = arg_squash_ctx_id(@args); @args = arg_gpu_timeline(@args); @args = arg_trace(@args); @args = arg_max_items(@args); @@ -338,8 +322,6 @@ sub sanitize_ctx { my ($ctx, $ring) = @_; - $ctx = $ctx - $ring if $squash_context_id; - if (exists $ctxdb{$ctx}) { return $ctx . '.' . $ctxdb{$ctx}; } else { -- 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 8/9] trace.pl: Basic preemption support
From: Tvrtko Ursulin Just forget about earlier request_in events. Signed-off-by: Tvrtko Ursulin --- scripts/trace.pl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/trace.pl b/scripts/trace.pl index c795406ae785..92431974296a 100755 --- a/scripts/trace.pl +++ b/scripts/trace.pl @@ -426,7 +426,9 @@ while (<>) { } elsif ($tp_name eq 'i915:i915_request_in:') { my %req; - die if exists $db{$key}; + # preemption + delete $db{$key} if exists $db{$key}; + die unless exists $queue{$key}; die unless exists $submit{$key}; -- 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 9/9] trace.pl: Fix request split mode
From: Tvrtko Ursulin Request split mode had several bugs, both in the original version and also after the recent refactorings. One big one was that it wasn't considering different submit ports as a reason to split execution, and also that it was too time based instead of looking at relevant timelines. In this refactoring we address the former by using the engine timelines introduced in the previous patch. Secondary port submissions are moved to follow the preceding submission as a first step in the correction process. In the second step, we add context timelines and use then in a similar fashion to separate start and end time of coalesced requests. For each coalesced request we know its boundaries by looking at the engine timeline (via global seqnos), and we know the previous request it should only start after, by looking at the context timeline. v2: * Remove some dead code. * Fix !port0 shifting logic. v3: * Refactor for less list walking as with incomplete handling. v4: * Database of context timelines should not contain duplicates! (Converted from array into a hash.) v5: * Avoid over-accounting runnable time for a coalesced group by recording the time first request entered the GPU and ending the execute delay at that point for the whole group. Signed-off-by: Tvrtko Ursulin Cc: John Harrison --- scripts/trace.pl | 140 +++ 1 file changed, 109 insertions(+), 31 deletions(-) diff --git a/scripts/trace.pl b/scripts/trace.pl index 92431974296a..e1fc2c7366d2 100755 --- a/scripts/trace.pl +++ b/scripts/trace.pl @@ -27,7 +27,7 @@ use warnings; use 5.010; my $gid = 0; -my (%db, %queue, %submit, %notify, %rings, %ctxdb, %ringmap, %reqwait); +my (%db, %queue, %submit, %notify, %rings, %ctxdb, %ringmap, %reqwait, %ctxtimelines); my @freqs; my $max_items = 3000; @@ -436,6 +436,7 @@ while (<>) { $req{'ring'} = $ring; $req{'seqno'} = $seqno; $req{'ctx'} = $ctx; + $ctxtimelines{$ctx . '/' . $ring} = 1; $req{'name'} = $ctx . '/' . $seqno; $req{'global'} = $tp{'global'}; $req{'port'} = $tp{'port'}; @@ -591,41 +592,113 @@ sub sortStart { return $val; } -my @sorted_keys = sort sortStart keys %db; -my $re_sort = 0; +my $re_sort = 1; +my @sorted_keys; -die "Database changed size?!" unless scalar(@sorted_keys) == $key_count; +sub maybe_sort_keys +{ + if ($re_sort) { + @sorted_keys = sort sortStart keys %db; + $re_sort = 0; + die "Database changed size?!" unless scalar(@sorted_keys) == +$key_count; + } +} -foreach my $key (@sorted_keys) { - my $ring = $db{$key}->{'ring'}; - my $end = $db{$key}->{'end'}; +maybe_sort_keys(); + +my %ctx_timelines; + +sub sortContext { + my $as = $db{$a}->{'seqno'}; + my $bs = $db{$b}->{'seqno'}; + my $val; + + $val = $as <=> $bs; + + die if $val == 0; + + return $val; +} + +sub get_ctx_timeline { + my ($ctx, $ring, $key) = @_; + my @timeline; + + return $ctx_timelines{$key} if exists $ctx_timelines{$key}; + + @timeline = grep { $db{$_}->{'ring'} == $ring and + $db{$_}->{'ctx'} == $ctx } @sorted_keys; + # FIXME seqno restart + @timeline = sort sortContext @timeline; + + $ctx_timelines{$key} = \@timeline; + + return \@timeline; +} + +# Split out merged batches if requested. +if ($correct_durations) { + # Shift !port0 requests start time to after the previous context on the + # same timeline has finished. + foreach my $gid (sort keys %rings) { + my $ring = $ringmap{$rings{$gid}}; + my $timeline = get_engine_timeline($ring); + my $complete; + + foreach my $pos (0..$#{$timeline}) { + my $key = @{$timeline}[$pos]; + my $prev = $complete; + my $pkey; + + $complete = $key unless exists $db{$key}->{'no-end'}; + $pkey = $complete; + + next if $db{$key}->{'port'} == 0; + + $pkey = $prev if $complete eq $key; + + die unless defined $pkey; + + $db{$key}->{'start'} = $db{$pkey}->{'end'}; + $db{$key}->{'start'} = $db{$pkey}->{'notify'} if $db{$key}->{'start'} > $db{$key}->{'end'}; + + die if $db{$key}->{'start'} > $db{$key}->{'end'}; - # correct duration of merged batches - if ($correct_durations and exists $db{$key}->{'no-end'}) { - my $ctx = $db{$key}->{'ctx'}; - my $seqno = $db{$key}->{'seqno'}; - my $start = $db{$key}->{'start'}; - my $next_key; - my $i = 1; - - do { -
[Intel-gfx] [PATCH i-g-t 7/9] trace.pl: Fix incomplete request handling
From: Tvrtko Ursulin Incomplete requests (no notify, no context complete) have to be corrected by looking at the engine timeline, and not the sorted-by-start-time view as was previously used. Per-engine timelines are generated on demand and cached for later use. v2: Find end of current context on the engine timeline instead of just using the next request for adjusting the incomplete start time. v3: Improve scaling with large datasets by only walking each engine timeline once and some caching. (John Harrison) v4: * Fix logic fail from v3. * Refactor the code a bit to separate the stages better. * Do not account batches with unknown duration in avg stats. * Handle two user interrupts with the same seqno. * Handle user interrupt arriving after request_out. Signed-off-by: Tvrtko Ursulin Cc: John Harrison --- scripts/trace.pl | 163 --- 1 file changed, 108 insertions(+), 55 deletions(-) diff --git a/scripts/trace.pl b/scripts/trace.pl index a10de11de01a..c795406ae785 100755 --- a/scripts/trace.pl +++ b/scripts/trace.pl @@ -450,16 +450,11 @@ while (<>) { die if exists $db{$key}->{'end'}; $db{$key}->{'end'} = $time; - if (exists $notify{$gkey}) { - $db{$key}->{'notify'} = $notify{$gkey}; - } else { - # No notify so far. Maybe it will arrive later which - # will be handled in the sanitation loop below. - $db{$key}->{'notify'} = $db{$key}->{'end'}; - $db{$key}->{'no-notify'} = 1; - } + $db{$key}->{'notify'} = $notify{$gkey} if exists $notify{$gkey}; } elsif ($tp_name eq 'i915:intel_engine_notify:') { - $notify{global_key($ring, $seqno)} = $time; + my $gkey = global_key($ring, $seqno); + + $notify{$gkey} = $time unless exists $notify{$gkey}; } elsif ($tp_name eq 'i915:intel_gpu_freq_change:') { push @freqs, [$prev_freq_ts, $time, $prev_freq] if $prev_freq; $prev_freq_ts = $time; @@ -472,66 +467,116 @@ while (<>) { my $max_seqno = 0; foreach my $key (keys %db) { my $gkey = global_key($db{$key}->{'ring'}, $db{$key}->{'global'}); - my $notify = $notify{$gkey}; die unless exists $db{$key}->{'start'}; $max_seqno = $db{$key}->{'seqno'} if $db{$key}->{'seqno'} > $max_seqno; - unless (exists $db{$key}->{'end'}) { - # Context complete not received. - $db{$key}->{'no-end'} = 1; + # Notify arrived after context complete? + $db{$key}->{'notify'} = $notify{$gkey} if not exists $db{$key}->{'notify'} + and exists $notify{$gkey}; - if (defined($notify)) { - # No context complete due req merging - use notify. - $db{$key}->{'notify'} = $notify; - $db{$key}->{'end'} = $notify; - } else { - # No notify and no context complete - give up for now. - $db{$key}->{'incomplete'} = 1; - } - } else { - # Notify arrived after context complete. - if (exists $db{$key}->{'no-notify'} and defined($notify)) { - delete $db{$key}->{'no-notify'}; - $db{$key}->{'notify'} = $notify; - } + # No notify but we have end? + $db{$key}->{'notify'} = $db{$key}->{'end'} if exists $db{$key}->{'end'} and + not exists $db{$key}->{'notify'}; + + # If user interrupt arrived out of order push it back to be no later + # than request out. + if (exists $db{$key}->{'end'} and exists $db{$key}->{'notify'} and + $db{$key}->{'notify'} > $db{$key}->{'end'}) { + $db{$key}->{'notify'} = $db{$key}->{'end'}; } } -# Fix up incompletes my $key_count = scalar(keys %db); -foreach my $key (keys %db) { - next unless exists $db{$key}->{'incomplete'}; - # End the incomplete batch at the time next one starts - my $ring = $db{$key}->{'ring'}; - my $ctx = $db{$key}->{'ctx'}; - my $seqno = $db{$key}->{'seqno'}; - my $next_key; - my $i = 1; - my $end; - - do { - $next_key = db_key($ring, $ctx, $seqno + $i); - $i++; - } until ((exists $db{$next_key} and not exists $db{$next_key}->{'incomplete'}) -or $i > $key_count); # ugly stop hack +my %engine_timelines; - if (exists $db{$next_key}) { - $end = $db{$next_key}->{'end'}; - } else { - # No info at all, fake it: - $end = $db{$key}->{'start'} + 999; - } +sub sortEngine { + my $as = $db{$a}->{'global'}; + my $bs = $db{$b}->
[Intel-gfx] [PATCH i-g-t 6/9] trace.pl: Context save only applies to last request of a bunch
From: Tvrtko Ursulin Skip accounting the context save time for anything but the last request of the coalesced bunch, and also skip drawing those boxes on the timeline. Signed-off-by: Tvrtko Ursulin --- scripts/trace.pl | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/scripts/trace.pl b/scripts/trace.pl index d92b338ba507..a10de11de01a 100755 --- a/scripts/trace.pl +++ b/scripts/trace.pl @@ -597,7 +597,11 @@ foreach my $key (@sorted_keys) { $max_ctx = $db{$key}->{'ctx'} if not defined $max_ctx or $db{$key}->{'ctx'} > $max_ctx; - $db{$key}->{'context-complete-delay'} = $end - $notify; + unless (exists $db{$key}->{'no-end'}) { + $db{$key}->{'context-complete-delay'} = $end - $notify; + } else { + $db{$key}->{'context-complete-delay'} = 0; + } $db{$key}->{'execute-delay'} = $start - $db{$key}->{'submit'}; $db{$key}->{'submit-delay'} = $db{$key}->{'submit'} - $db{$key}->{'queue'}; $db{$key}->{'duration'} = $notify - $start; @@ -614,7 +618,7 @@ foreach my $key (@sorted_keys) { $submit_avg{$ring} += $db{$key}->{'submit-delay'}; $execute_avg{$ring} += $db{$key}->{'execute-delay'}; - $ctxsave_avg{$ring} += $end - $notify; + $ctxsave_avg{$ring} += $db{$key}->{'context-complete-delay'}; } foreach my $ring (sort keys %batch_avg) { @@ -949,7 +953,8 @@ foreach my $key (sort sortQueue keys %db) { # user interrupt to context complete $duration = $end - $notify; - unless (exists $skip_box{'ctxsave'} or $duration < $min_duration) { + unless (exists $skip_box{'ctxsave'} or $duration < $min_duration or + exists $db{$key}->{'no-end'}) { $skey = -2 * $max_seqno * $ctx - 2 * $seqno; $style = 'color: black; background-color: orange;'; my $ctxsave = $db{$key}->{'end'} - $db{$key}->{'notify'}; -- 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 5/9] trace.pl: Skip drawing very short boxes
From: Tvrtko Ursulin Few micro-second long boxes cannot be displayed by the visualizer in a meaningful way so just burden the browser and the human looking at the noise. Sp skip drawing anything shorter and 10us. Signed-off-by: Tvrtko Ursulin --- scripts/trace.pl | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/scripts/trace.pl b/scripts/trace.pl index 696c18e8b1cf..d92b338ba507 100755 --- a/scripts/trace.pl +++ b/scripts/trace.pl @@ -40,6 +40,7 @@ my $trace = 0; my $avg_delay_stats = 0; my $gpu_timeline = 0; my $colour_contexts = 0; +my $min_duration = 10; # us my @args; @@ -899,14 +900,15 @@ foreach my $key (sort sortQueue keys %db) { my ($name, $ctx, $seqno) = ($db{$key}->{'name'}, $db{$key}->{'ctx'}, $db{$key}->{'seqno'}); my ($queue, $start, $notify, $end) = ($db{$key}->{'queue'}, $db{$key}->{'start'}, $db{$key}->{'notify'}, $db{$key}->{'end'}); my $submit = $queue + $db{$key}->{'submit-delay'}; - my ($content, $style); + my ($content, $style, $duration); my $group = $engine_start_id + $rings{$db{$key}->{'ring'}}; my $type = ' type: \'range\','; my $startend; my $skey; # submit to execute - unless (exists $skip_box{'queue'}) { + $duration = $submit - $queue; + unless (exists $skip_box{'queue'} or $duration < $min_duration) { $skey = 2 * $max_seqno * $ctx + 2 * $seqno; $style = box_style($ctx, 90, 35, 85); $content = "$name$db{$key}->{'submit-delay'}us ($db{$key}->{'execute-delay'}us)"; @@ -916,7 +918,8 @@ foreach my $key (sort sortQueue keys %db) { } # execute to start - unless (exists $skip_box{'ready'}) { + $duration = $start - $submit; + unless (exists $skip_box{'ready'} or $duration < $min_duration) { $skey = 2 * $max_seqno * $ctx + 2 * $seqno + 1; $style = box_style($ctx, 45, 35, 45); $content = "$name$db{$key}->{'execute-delay'}us"; @@ -926,7 +929,8 @@ foreach my $key (sort sortQueue keys %db) { } # start to user interrupt - unless (exists $skip_box{'execute'}) { + $duration = $notify - $start; + unless (exists $skip_box{'execute'} or $duration < $min_duration) { $skey = -2 * $max_seqno * $ctx - 2 * $seqno - 1; if (exists $db{$key}->{'incomplete'}) { $style = 'color: white; background-color: red;'; @@ -944,7 +948,8 @@ foreach my $key (sort sortQueue keys %db) { } # user interrupt to context complete - unless (exists $skip_box{'ctxsave'}) { + $duration = $end - $notify; + unless (exists $skip_box{'ctxsave'} or $duration < $min_duration) { $skey = -2 * $max_seqno * $ctx - 2 * $seqno; $style = 'color: black; background-color: orange;'; my $ctxsave = $db{$key}->{'end'} - $db{$key}->{'notify'}; -- 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] tests/gem_exec_latency: New subtests for checking submission from RT tasks
From: Tvrtko Ursulin We want to make sure RT tasks which use a lot of CPU times can submit batch buffers with roughly the same latency (and certainly not worse) compared to normal tasks. v2: Add tests to run across all engines simultaneously to encourage ksoftirqd to kick in even more often. Signed-off-by: Tvrtko Ursulin Reviewed-by: Chris Wilson #v1 Signed-off-by: Chris Wilson --- tests/gem_exec_latency.c | 206 +++ 1 file changed, 206 insertions(+) diff --git a/tests/gem_exec_latency.c b/tests/gem_exec_latency.c index 9498c0921..bd036615a 100644 --- a/tests/gem_exec_latency.c +++ b/tests/gem_exec_latency.c @@ -36,11 +36,15 @@ #include #include #include +#include #include "drm.h" #include "igt_sysfs.h" #include "igt_vgem.h" +#include "igt_dummyload.h" +#include "igt_stats.h" + #include "i915/gem_ring.h" #define LOCAL_I915_EXEC_NO_RELOC (1<<11) @@ -351,6 +355,188 @@ static void latency_from_ring(int fd, } } +static void __rearm_spin_batch(igt_spin_t *spin) +{ + const uint32_t mi_arb_chk = 0x5 << 23; + + *spin->batch = mi_arb_chk; + *spin->running = 0; + __sync_synchronize(); +} + +static void +__submit_spin_batch(int fd, igt_spin_t *spin, unsigned int flags) +{ + struct drm_i915_gem_execbuffer2 eb = spin->execbuf; + + eb.flags &= ~(0x3f | I915_EXEC_BSD_MASK); + eb.flags |= flags | I915_EXEC_NO_RELOC; + + gem_execbuf(fd, &eb); +} + +struct rt_pkt { + struct igt_mean mean; + double min, max; +}; + +static bool __spin_wait(int fd, igt_spin_t *spin) +{ + while (!READ_ONCE(*spin->running)) { + if (!gem_bo_busy(fd, spin->handle)) + return false; + } + + return true; +} + +/* + * Test whether RT thread which hogs the CPU a lot can submit work with + * reasonable latency. + */ +static void +rthog_latency_on_ring(int fd, unsigned int engine, const char *name, unsigned int flags) +#define RTIDLE 0x1 +{ + const char *passname[] = { "warmup", "normal", "rt" }; + struct rt_pkt *results; + unsigned int engines[16]; + const char *names[16]; + unsigned int nengine; + int ret; + + results = mmap(NULL, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0); + igt_assert(results != MAP_FAILED); + + nengine = 0; + if (engine == ALL_ENGINES) { + for_each_physical_engine(fd, engine) { + if (!gem_can_store_dword(fd, engine)) + continue; + + engines[nengine] = engine; + names[nengine] = e__->name; + nengine++; + } + igt_require(nengine > 1); + } else { + igt_require(gem_can_store_dword(fd, engine)); + engines[nengine] = engine; + names[nengine] = name; + nengine++; + } + + igt_fork(child, nengine) { + unsigned int pass = 0; /* Three passes: warmup, normal, rt. */ + + engine = engines[child]; + do { + struct igt_mean mean; + double min = HUGE_VAL; + double max = -HUGE_VAL; + igt_spin_t *spin; + + igt_mean_init(&mean); + + if (pass == 2) { + struct sched_param rt = + { .sched_priority = 99 }; + + ret = sched_setscheduler(0, +SCHED_FIFO | SCHED_RESET_ON_FORK, +&rt); + if (ret) { + igt_warn("Failed to set scheduling policy!\n"); + break; + } + } + + spin = __igt_spin_batch_new_poll(fd, 0, engine); + if (!spin) { + igt_warn("Failed to create spinner! (%s)\n", +passname[pass]); + break; + } + igt_spin_busywait_until_running(spin); + + igt_until_timeout(pass > 0 ? 5 : 2) { + struct timespec ts = { }; + double t; + + igt_spin_batch_end(spin); + gem_sync(fd, spin->handle); + if (flags & RTIDLE) + usleep(250); + + /* +* If we are oversubscribed (more RT hogs than +* cpus) give the others a change to run; +* otherwise, they will inter
Re: [Intel-gfx] [PATCH 4/5] drm/i915/execlists: Direct submission from irq handler
Quoting Chris Wilson (2018-05-09 15:28:00) > Continuing the theme of bypassing ksoftirqd latency, also first try to > directly submit from the CS interrupt handler to clear the ELSP and > queue the next. > > In the past, we have been hesitant to do this as the context switch > processing has been quite heavy, requiring forcewaked mmio. However, as > we now can read the GPU state from the cacheable HWSP, it is relatively > cheap! > > v2: Explain why we test_bit(IRQ_EXECLIST) after doing notify_ring (it's > because the notify_ring() may itself trigger direct submission clearing > the bit) > > Suggested-by: Tvrtko Ursulin > Signed-off-by: Chris Wilson > Cc: Tvrtko Ursulin Tvrtko has some extra tests for igt/gem_exec_latency that show the impact of triggering ksoftirqd has on submission latency. It's not pretty. Fortunately, this pair of patches makes a huge difference. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] gpu: drm: i915: Change return type to vm_fault_t
On Wed, Apr 18, 2018 at 12:32 AM, Souptick Joarder wrote: > Use new return type vm_fault_t for fault handler. For > now, this is just documenting that the function returns > a VM_FAULT value rather than an errno. Once all instances > are converted, vm_fault_t will become a distinct type. > > Reference id -> 1c8f422059ae ("mm: change return type to > vm_fault_t") > > Fixed one checkpatch.pl warning inside WARN_ONCE. > > Signed-off-by: Souptick Joarder > --- > drivers/gpu/drm/i915/i915_drv.h | 3 ++- > drivers/gpu/drm/i915/i915_gem.c | 37 +++-- > 2 files changed, 21 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index a42deeb..95b0d50 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -51,6 +51,7 @@ > #include > #include > #include > +#include > > #include "i915_params.h" > #include "i915_reg.h" > @@ -3363,7 +3364,7 @@ int i915_gem_wait_for_idle(struct drm_i915_private > *dev_priv, >unsigned int flags); > int __must_check i915_gem_suspend(struct drm_i915_private *dev_priv); > void i915_gem_resume(struct drm_i915_private *dev_priv); > -int i915_gem_fault(struct vm_fault *vmf); > +vm_fault_t i915_gem_fault(struct vm_fault *vmf); > int i915_gem_object_wait(struct drm_i915_gem_object *obj, > unsigned int flags, > long timeout, > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index dd89abd..61816e8 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -1882,7 +1882,7 @@ int i915_gem_mmap_gtt_version(void) > * The current feature set supported by i915_gem_fault() and thus GTT mmaps > * is exposed via I915_PARAM_MMAP_GTT_VERSION (see > i915_gem_mmap_gtt_version). > */ > -int i915_gem_fault(struct vm_fault *vmf) > +vm_fault_t i915_gem_fault(struct vm_fault *vmf) > { > #define MIN_CHUNK_PAGES ((1 << 20) >> PAGE_SHIFT) /* 1 MiB */ > struct vm_area_struct *area = vmf->vma; > @@ -1894,7 +1894,8 @@ int i915_gem_fault(struct vm_fault *vmf) > struct i915_vma *vma; > pgoff_t page_offset; > unsigned int flags; > - int ret; > + int error; > + vm_fault_t ret; > > /* We don't use vmf->pgoff since that has the fake offset */ > page_offset = (vmf->address - area->vm_start) >> PAGE_SHIFT; > @@ -1906,26 +1907,26 @@ int i915_gem_fault(struct vm_fault *vmf) > * repeat the flush holding the lock in the normal manner to catch > cases > * where we are gazumped. > */ > - ret = i915_gem_object_wait(obj, > + error = i915_gem_object_wait(obj, >I915_WAIT_INTERRUPTIBLE, >MAX_SCHEDULE_TIMEOUT, >NULL); > - if (ret) > + if (error) > goto err; > > - ret = i915_gem_object_pin_pages(obj); > - if (ret) > + error = i915_gem_object_pin_pages(obj); > + if (error) > goto err; > > intel_runtime_pm_get(dev_priv); > > - ret = i915_mutex_lock_interruptible(dev); > - if (ret) > + error = i915_mutex_lock_interruptible(dev); > + if (error) > goto err_rpm; > > /* Access to snoopable pages through the GTT is incoherent. */ > if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(dev_priv)) { > - ret = -EFAULT; > + error = -EFAULT; > goto err_unlock; > } > > @@ -1952,25 +1953,25 @@ int i915_gem_fault(struct vm_fault *vmf) > vma = i915_gem_object_ggtt_pin(obj, &view, 0, 0, > PIN_MAPPABLE); > } > if (IS_ERR(vma)) { > - ret = PTR_ERR(vma); > + error = PTR_ERR(vma); > goto err_unlock; > } > > - ret = i915_gem_object_set_to_gtt_domain(obj, write); > - if (ret) > + error = i915_gem_object_set_to_gtt_domain(obj, write); > + if (error) > goto err_unpin; > > - ret = i915_vma_pin_fence(vma); > - if (ret) > + error = i915_vma_pin_fence(vma); > + if (error) > goto err_unpin; > > /* Finally, remap it using the new GTT offset */ > - ret = remap_io_mapping(area, > + error = remap_io_mapping(area, >area->vm_start + > (vma->ggtt_view.partial.offset << PAGE_SHIFT), >(ggtt->gmadr.start + vma->node.start) >> > PAGE_SHIFT, >min_t(u64, vma->size, area->vm_end - > area->vm_start), >&ggtt->iomap); > - if (ret) > + if (error) > goto err_fence; > > /* Mark as being mmapped into userspace for later revocation */ > @@ -1991,7 +1992,7 @@ int i915
Re: [Intel-gfx] [PATCH v6 10/14] drm/i915/kvmgt: Support setting dma map for huge pages
On 8 May 2018 at 10:05, wrote: > From: Changbin Du > > To support huge gtt, we need to support huge pages in kvmgt first. > This patch adds a 'size' param to the intel_gvt_mpt::dma_map_guest_page > API and implements it in kvmgt. > > v2: rebase. > > Signed-off-by: Changbin Du > --- > drivers/gpu/drm/i915/gvt/gtt.c | 6 +- > drivers/gpu/drm/i915/gvt/hypercall.h | 2 +- > drivers/gpu/drm/i915/gvt/kvmgt.c | 130 > +-- > drivers/gpu/drm/i915/gvt/mpt.h | 7 +- > 4 files changed, 101 insertions(+), 44 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c > index 2f13464..ffeecda 100644 > --- a/drivers/gpu/drm/i915/gvt/gtt.c > +++ b/drivers/gpu/drm/i915/gvt/gtt.c > @@ -1104,7 +1104,7 @@ static int split_64KB_gtt_entry(struct intel_vgpu *vgpu, > > for (i = 0; i < GTT_64K_PTE_STRIDE; i++) { > ret = intel_gvt_hypervisor_dma_map_guest_page(vgpu, > - start_gfn + i, &dma_addr); > + start_gfn + i, PAGE_SIZE, &dma_addr); > if (ret) > return ret; > > @@ -1150,7 +1150,7 @@ static int ppgtt_populate_shadow_entry(struct > intel_vgpu *vgpu, > }; > > /* direct shadow */ > - ret = intel_gvt_hypervisor_dma_map_guest_page(vgpu, gfn, &dma_addr); > + ret = intel_gvt_hypervisor_dma_map_guest_page(vgpu, gfn, PAGE_SIZE, > &dma_addr); > if (ret) > return -ENXIO; > > @@ -2078,7 +2078,7 @@ static int emulate_ggtt_mmio_write(struct intel_vgpu > *vgpu, unsigned int off, > } > > ret = intel_gvt_hypervisor_dma_map_guest_page(vgpu, gfn, > - &dma_addr); > + PAGE_SIZE, &dma_addr); > if (ret) { > gvt_vgpu_err("fail to populate guest ggtt entry\n"); > /* guest driver may read/write the entry when partial > diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h > b/drivers/gpu/drm/i915/gvt/hypercall.h > index f6dd9f7..5af11cf 100644 > --- a/drivers/gpu/drm/i915/gvt/hypercall.h > +++ b/drivers/gpu/drm/i915/gvt/hypercall.h > @@ -53,7 +53,7 @@ struct intel_gvt_mpt { > unsigned long (*gfn_to_mfn)(unsigned long handle, unsigned long gfn); > > int (*dma_map_guest_page)(unsigned long handle, unsigned long gfn, > - dma_addr_t *dma_addr); > + unsigned long size, dma_addr_t *dma_addr); > void (*dma_unmap_guest_page)(unsigned long handle, dma_addr_t > dma_addr); > > int (*map_gfn_to_mfn)(unsigned long handle, unsigned long gfn, > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c > b/drivers/gpu/drm/i915/gvt/kvmgt.c > index df4e4a0..4d2f53a 100644 > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c > @@ -94,6 +94,7 @@ struct gvt_dma { > struct rb_node dma_addr_node; > gfn_t gfn; > dma_addr_t dma_addr; > + unsigned long size; > struct kref ref; > }; > > @@ -106,51 +107,103 @@ static int kvmgt_guest_init(struct mdev_device *mdev); > static void intel_vgpu_release_work(struct work_struct *work); > static bool kvmgt_guest_exit(struct kvmgt_guest_info *info); > > -static int gvt_dma_map_page(struct intel_vgpu *vgpu, unsigned long gfn, > - dma_addr_t *dma_addr) > +static void gvt_unpin_guest_page(struct intel_vgpu *vgpu, unsigned long gfn, > + unsigned long size) > { > - struct device *dev = &vgpu->gvt->dev_priv->drm.pdev->dev; > - struct page *page; > - unsigned long pfn; > + int total_pages; > + int npage; > int ret; > > - /* Pin the page first. */ > - ret = vfio_pin_pages(mdev_dev(vgpu->vdev.mdev), &gfn, 1, > -IOMMU_READ | IOMMU_WRITE, &pfn); > - if (ret != 1) { > - gvt_vgpu_err("vfio_pin_pages failed for gfn 0x%lx: %d\n", > -gfn, ret); > - return -EINVAL; > + total_pages = roundup(size, PAGE_SIZE) / PAGE_SIZE; > + > + for (npage = 0; npage < total_pages; npage++) { > + unsigned long cur_gfn = gfn + npage; > + > + ret = vfio_unpin_pages(mdev_dev(vgpu->vdev.mdev), &cur_gfn, > 1); > + WARN_ON(ret != 1); > } > +} > > - if (!pfn_valid(pfn)) { > - gvt_vgpu_err("pfn 0x%lx is not mem backed\n", pfn); > - vfio_unpin_pages(mdev_dev(vgpu->vdev.mdev), &gfn, 1); > - return -EINVAL; > +/* Pin a normal or compound guest page for dma. */ > +static int gvt_pin_guest_page(struct intel_vgpu *vgpu, unsigned long gfn, > + unsigned long size, struct page **page) > +{ > + unsigned long base_pfn = 0; > + int total_pages; >
[Intel-gfx] [PATCH] drm/i915/guc: Don't store runtime GuC log level in modparam
From: Piotr Piórkowski Currently we are using modparam as placeholder for GuC log level. Stop doing this and keep runtime GuC level in intel_guc_log struct. Signed-off-by: Piotr Piórkowski Cc: Michal Wajdeczko Cc: Michał Winiarski Cc: Joonas Lahtinen Cc: Chris Wilson --- drivers/gpu/drm/i915/intel_guc.c | 8 +++- drivers/gpu/drm/i915/intel_guc_log.c | 17 - drivers/gpu/drm/i915/intel_guc_log.h | 3 ++- drivers/gpu/drm/i915/intel_uc.c | 2 +- 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c index 116f4ccf1bbd..4b489b67663e 100644 --- a/drivers/gpu/drm/i915/intel_guc.c +++ b/drivers/gpu/drm/i915/intel_guc.c @@ -203,13 +203,11 @@ void intel_guc_fini(struct intel_guc *guc) guc_shared_data_destroy(guc); } -static u32 get_log_control_flags(void) +static u32 get_log_control_flags(struct intel_guc_log *log) { - u32 level = i915_modparams.guc_log_level; + u32 level = log->level; u32 flags = 0; - GEM_BUG_ON(level < 0); - if (!GUC_LOG_LEVEL_IS_ENABLED(level)) flags |= GUC_LOG_DEFAULT_DISABLED; @@ -250,7 +248,7 @@ void intel_guc_init_params(struct intel_guc *guc) params[GUC_CTL_LOG_PARAMS] = guc->log.flags; - params[GUC_CTL_DEBUG] = get_log_control_flags(); + params[GUC_CTL_DEBUG] = get_log_control_flags(&guc->log); /* If GuC submission is enabled, set up additional parameters here */ if (USES_GUC_SUBMISSION(dev_priv)) { diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c index 401e1704d61e..b3819680070a 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.c +++ b/drivers/gpu/drm/i915/intel_guc_log.c @@ -475,11 +475,13 @@ int intel_guc_log_create(struct intel_guc_log *log) offset = intel_guc_ggtt_offset(guc, vma) >> PAGE_SHIFT; log->flags = (offset << GUC_LOG_BUF_ADDR_SHIFT) | flags; + GEM_BUG_ON(i915_modparams.guc_log_level < 0); + log->level = i915_modparams.guc_log_level; + return 0; err: - /* logging will be off */ - i915_modparams.guc_log_level = 0; + DRM_DEBUG_DRIVER("Failed to allocate GuC log buffer. %d\n", ret); return ret; } @@ -488,12 +490,10 @@ void intel_guc_log_destroy(struct intel_guc_log *log) i915_vma_unpin_and_release(&log->vma); } -int intel_guc_log_level_get(struct intel_guc_log *log) +u32 intel_guc_log_level_get(struct intel_guc_log *log) { GEM_BUG_ON(!log->vma); - GEM_BUG_ON(i915_modparams.guc_log_level < 0); - - return i915_modparams.guc_log_level; + return log->level; } int intel_guc_log_level_set(struct intel_guc_log *log, u64 val) @@ -504,7 +504,6 @@ int intel_guc_log_level_set(struct intel_guc_log *log, u64 val) BUILD_BUG_ON(GUC_LOG_VERBOSITY_MIN != 0); GEM_BUG_ON(!log->vma); - GEM_BUG_ON(i915_modparams.guc_log_level < 0); /* * GuC is recognizing log levels starting from 0 to max, we're using 0 @@ -515,7 +514,7 @@ int intel_guc_log_level_set(struct intel_guc_log *log, u64 val) mutex_lock(&dev_priv->drm.struct_mutex); - if (i915_modparams.guc_log_level == val) { + if (log->level == val) { ret = 0; goto out_unlock; } @@ -530,7 +529,7 @@ int intel_guc_log_level_set(struct intel_guc_log *log, u64 val) goto out_unlock; } - i915_modparams.guc_log_level = val; + log->level = val; out_unlock: mutex_unlock(&dev_priv->drm.struct_mutex); diff --git a/drivers/gpu/drm/i915/intel_guc_log.h b/drivers/gpu/drm/i915/intel_guc_log.h index fa80535a6f9d..db21241588e6 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.h +++ b/drivers/gpu/drm/i915/intel_guc_log.h @@ -59,6 +59,7 @@ struct intel_guc; struct intel_guc_log { u32 flags; + u32 level; struct i915_vma *vma; struct { void *buf_addr; @@ -80,7 +81,7 @@ void intel_guc_log_init_early(struct intel_guc_log *log); int intel_guc_log_create(struct intel_guc_log *log); void intel_guc_log_destroy(struct intel_guc_log *log); -int intel_guc_log_level_get(struct intel_guc_log *log); +u32 intel_guc_log_level_get(struct intel_guc_log *log); int intel_guc_log_level_set(struct intel_guc_log *log, u64 control_val); bool intel_guc_log_relay_enabled(const struct intel_guc_log *log); int intel_guc_log_relay_open(struct intel_guc_log *log); diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 1cffaf7b5dbe..897eaad09c7d 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -208,7 +208,7 @@ void intel_uc_init_mmio(struct drm_i915_private *dev_priv) static void guc_capture_load_err_log(struct intel_guc *guc) { - if (!guc->log.vma || !i915_modparams.guc_log_level) + if (!guc->log.vma || !guc->log.level) retur
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/guc: Don't store runtime GuC log level in modparam
== Series Details == Series: drm/i915/guc: Don't store runtime GuC log level in modparam URL : https://patchwork.freedesktop.org/series/43013/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4163 -> Patchwork_8973 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/43013/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_8973 that come from known issues: === IGT changes === Issues hit igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: fi-cnl-psr: PASS -> DMESG-WARN (fdo#104951) Possible fixes igt@drv_module_reload@basic-reload-inject: fi-glk-j4005: DMESG-WARN (fdo#106248) -> PASS igt@gem_mmap_gtt@basic-small-bo-tiledx: fi-gdg-551: FAIL (fdo#102575) -> PASS igt@kms_flip@basic-flip-vs-wf_vblank: fi-hsw-4770r: FAIL (fdo#100368) -> PASS fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575 fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951 fdo#106248 https://bugs.freedesktop.org/show_bug.cgi?id=106248 == Participating hosts (41 -> 36) == Missing(5): fi-byt-j1900 fi-byt-squawks fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq == Build changes == * Linux: CI_DRM_4163 -> Patchwork_8973 CI_DRM_4163: 8e1dab6e913be7d014eb9bc355ec65b6b56dcd56 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4468: 548a894dc904c4628522dbbc77cb179a4c965ebc @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_8973: b1792bf8c0170919171d3ef49a254b961794321d @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4468: 1e60f1499e5b71b6d5a747189d7c28f57359a87f @ git://anongit.freedesktop.org/piglit == Linux commits == b1792bf8c017 drm/i915/guc: Don't store runtime GuC log level in modparam == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8973/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 04/14] drm/i915/gvt: Detect 64K gtt entry by IPS bit of PDE
On 8 May 2018 at 10:05, wrote: > From: Changbin Du > > This change help us detect the real entry type per PSE and IPS setting. > For 64K entry, we also need to check reg GEN8_GAMW_ECO_DEV_RW_IA. > > Signed-off-by: Changbin Du > --- > drivers/gpu/drm/i915/gvt/gtt.c | 68 > +- > drivers/gpu/drm/i915/gvt/gtt.h | 2 ++ > 2 files changed, 49 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c > index cd2a227..7a80518 100644 > --- a/drivers/gpu/drm/i915/gvt/gtt.c > +++ b/drivers/gpu/drm/i915/gvt/gtt.c > @@ -384,20 +384,7 @@ static void gen8_gtt_set_pfn(struct intel_gvt_gtt_entry > *e, unsigned long pfn) > > static bool gen8_gtt_test_pse(struct intel_gvt_gtt_entry *e) > { > - /* Entry doesn't have PSE bit. */ > - if (get_pse_type(e->type) == GTT_TYPE_INVALID) > - return false; > - > - e->type = get_entry_type(e->type); > - if (!(e->val64 & _PAGE_PSE)) > - return false; > - > - /* We don't support 64K entry yet, will remove this later. */ > - if (get_pse_type(e->type) == GTT_TYPE_PPGTT_PTE_64K_ENTRY) > - return false; > - > - e->type = get_pse_type(e->type); > - return true; > + return !!(e->val64 & _PAGE_PSE); > } > > static bool gen8_gtt_test_ips(struct intel_gvt_gtt_entry *e) > @@ -487,6 +474,27 @@ static struct intel_gvt_gtt_gma_ops gen8_gtt_gma_ops = { > .gma_to_pml4_index = gen8_gma_to_pml4_index, > }; > > +/* Update entry type per pse and ips bit. */ > +static void update_entry_type_for_real(struct intel_gvt_gtt_pte_ops *pte_ops, > + struct intel_gvt_gtt_entry *entry, bool ips) > +{ > + switch (entry->type) { > + case GTT_TYPE_PPGTT_PDE_ENTRY: > + case GTT_TYPE_PPGTT_PDP_ENTRY: > + if (pte_ops->test_pse(entry)) > + entry->type = get_pse_type(entry->type); > + break; > + case GTT_TYPE_PPGTT_PTE_4K_ENTRY: > + if (ips) > + entry->type = get_pse_type(entry->type); > + break; > + default: > + GEM_BUG_ON(!gtt_type_is_entry(entry->type)); > + } > + > + GEM_BUG_ON(entry->type == GTT_TYPE_INVALID); > +} > + > /* > * MM helpers. > */ > @@ -502,8 +510,7 @@ static void _ppgtt_get_root_entry(struct intel_vgpu_mm > *mm, > pte_ops->get_entry(guest ? mm->ppgtt_mm.guest_pdps : >mm->ppgtt_mm.shadow_pdps, >entry, index, false, 0, mm->vgpu); > - > - pte_ops->test_pse(entry); > + update_entry_type_for_real(pte_ops, entry, false); > } > > static inline void ppgtt_get_guest_root_entry(struct intel_vgpu_mm *mm, > @@ -608,7 +615,8 @@ static inline int ppgtt_spt_get_entry( > if (ret) > return ret; > > - ops->test_pse(e); > + update_entry_type_for_real(ops, e, guest ? > + spt->guest_page.pde_ips : false); > > gvt_vdbg_mm("read ppgtt entry, spt type %d, entry type %d, index %lu, > value %llx\n", > type, e->type, index, e->val64); > @@ -752,7 +760,8 @@ static inline struct intel_vgpu_ppgtt_spt > *intel_vgpu_find_spt_by_mfn( > static int reclaim_one_ppgtt_mm(struct intel_gvt *gvt); > > static struct intel_vgpu_ppgtt_spt *ppgtt_alloc_spt( > - struct intel_vgpu *vgpu, int type, unsigned long gfn) > + struct intel_vgpu *vgpu, int type, unsigned long gfn, > + bool guest_pde_ips) > { > struct device *kdev = &vgpu->gvt->dev_priv->drm.pdev->dev; > struct intel_vgpu_ppgtt_spt *spt = NULL; > @@ -792,6 +801,7 @@ static struct intel_vgpu_ppgtt_spt *ppgtt_alloc_spt( > */ > spt->guest_page.type = type; > spt->guest_page.gfn = gfn; > + spt->guest_page.pde_ips = guest_pde_ips; > > ret = intel_vgpu_register_page_track(vgpu, spt->guest_page.gfn, > ppgtt_write_protection_handler, spt); > @@ -934,6 +944,20 @@ static int ppgtt_invalidate_spt(struct > intel_vgpu_ppgtt_spt *spt) > return ret; > } > > +static bool vgpu_ips_enabled(struct intel_vgpu *vgpu) > +{ > + if (INTEL_GEN(vgpu->gvt->dev_priv) == 9) { > + u32 ips = vgpu_vreg_t(vgpu, GEN8_GAMW_ECO_DEV_RW_IA) & > + GAMW_ECO_ENABLE_64K_IPS_FIELD; > + > + return ips == GAMW_ECO_ENABLE_64K_IPS_FIELD; > + } else if (INTEL_GEN(vgpu->gvt->dev_priv) >= 10) { > + /* 64K paging only controlled by IPS bit in PTE now. */ > + return true; AFAIK the funny business with having to enable the IPS bit through mmio is also needed on GEN10. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] drm/i915: Wrap tasklet_struct for abuse
On 09/05/2018 15:27, Chris Wilson wrote: In the next few patches, we want to abuse tasklet to avoid ksoftirqd latency along critical paths. To make that abuse easily to swallow, first coat the tasklet in a little syntactic sugar. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_gem.c | 4 +- drivers/gpu/drm/i915/i915_irq.c | 2 +- drivers/gpu/drm/i915/i915_tasklet.h | 78 + drivers/gpu/drm/i915/intel_engine_cs.c | 11 ++- drivers/gpu/drm/i915/intel_guc_submission.c | 8 +-- drivers/gpu/drm/i915/intel_lrc.c| 18 ++--- drivers/gpu/drm/i915/intel_ringbuffer.h | 3 +- 7 files changed, 102 insertions(+), 22 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_tasklet.h diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 63c96c2b8fcf..59e04387a27c 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3036,7 +3036,7 @@ i915_gem_reset_prepare_engine(struct intel_engine_cs *engine) * Turning off the execlists->tasklet until the reset is over * prevents the race. */ - tasklet_disable(&engine->execlists.tasklet); + i915_tasklet_lock(&engine->execlists.tasklet); /* * We're using worker to queue preemption requests from the tasklet in @@ -3256,7 +3256,7 @@ void i915_gem_reset(struct drm_i915_private *dev_priv, void i915_gem_reset_finish_engine(struct intel_engine_cs *engine) { - tasklet_enable(&engine->execlists.tasklet); + i915_tasklet_unlock(&engine->execlists.tasklet); kthread_unpark(engine->breadcrumbs.signaler); intel_uncore_forcewake_put(engine->i915, FORCEWAKE_ALL); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index f9bc3aaa90d0..f8aff5a5aa83 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1477,7 +1477,7 @@ gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir) } if (tasklet) - tasklet_hi_schedule(&execlists->tasklet); + i915_tasklet_schedule(&execlists->tasklet); } static void gen8_gt_irq_ack(struct drm_i915_private *i915, diff --git a/drivers/gpu/drm/i915/i915_tasklet.h b/drivers/gpu/drm/i915/i915_tasklet.h new file mode 100644 index ..42b002b88edb --- /dev/null +++ b/drivers/gpu/drm/i915/i915_tasklet.h @@ -0,0 +1,78 @@ +/* + * SPDX-License-Identifier: GPL-2.0 + * + * Copyright © 2018 Intel Corporation + */ + +#ifndef _I915_TASKLET_H_ +#define _I915_TASKLET_H_ + +#include +#include + +/** + * struct i915_tasklet - wrapper around tasklet_struct + * + * We want to abuse tasklets slightly, such as calling them directly. In some + * cases, this requires some auxiliary tracking so subclass the tasklet_struct + * so that we have a central place and helpers. + */ +struct i915_tasklet { + struct tasklet_struct base; +}; + +static inline void i915_tasklet_init(struct i915_tasklet *t, +void (*func)(unsigned long), +unsigned long data) +{ + tasklet_init(&t->base, func, data); +} + +static inline bool i915_tasklet_is_scheduled(const struct i915_tasklet *t) +{ + return test_bit(TASKLET_STATE_SCHED, &t->base.state); +} + +static inline bool i915_tasklet_is_running(const struct i915_tasklet *t) +{ + return test_bit(TASKLET_STATE_RUN, &t->base.state); +} + +static inline bool i915_tasklet_is_enabled(const struct i915_tasklet *t) +{ + return likely(!atomic_read(&t->base.count)); +} + +static inline void i915_tasklet_schedule(struct i915_tasklet *t) +{ + tasklet_hi_schedule(&t->base); +} + +static inline void i915_tasklet_flush(struct i915_tasklet *t) +{ + tasklet_kill(&t->base); +} + +static inline void i915_tasklet_lock(struct i915_tasklet *t) +{ + tasklet_disable(&t->base); +} + +static inline void i915_tasklet_unlock(struct i915_tasklet *t) +{ + tasklet_enable(&t->base); +} I agree keeping the original naming for kill/enable/disable would be better. + +static inline void i915_tasklet_set_func(struct i915_tasklet *t, +void (*func)(unsigned long data), +unsigned long data) +{ + i915_tasklet_lock(t); + + t->base.func = func; + t->base.data = data; + + i915_tasklet_unlock(t); I kind of remember something about issues we had with placing tasklet_disable placed in some contexts. Hm.. if only I could recall the details. It's probably fine. I cannot come up with ideas on how to protect against that. GEM_BUG_ON(in_irq() || in_softirq())? Too far fetched probably. +} + +#endif /* _I915_TASKLET_H_ */ diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 70325e0824e3..3c8a0c3245f3 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/guc: Don't store runtime GuC log level in modparam
== Series Details == Series: drm/i915/guc: Don't store runtime GuC log level in modparam URL : https://patchwork.freedesktop.org/series/43013/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4163_full -> Patchwork_8973_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_8973_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_8973_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/43013/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_8973_full: === IGT changes === Warnings igt@gem_exec_schedule@deep-bsd2: shard-kbl: SKIP -> PASS +3 igt@gem_mocs_settings@mocs-rc6-bsd1: shard-kbl: PASS -> SKIP igt@kms_force_connector_basic@force-connector-state: shard-snb: PASS -> SKIP igt@kms_properties@plane-properties-legacy: shard-snb: SKIP -> PASS +4 == Known issues == Here are the changes found in Patchwork_8973_full that come from known issues: === IGT changes === Issues hit igt@kms_cursor_crc@cursor-64x64-suspend: shard-apl: PASS -> FAIL (fdo#103191, fdo#103232, fdo#104724, fdo#104645) igt@kms_cursor_legacy@flip-vs-cursor-toggle: shard-hsw: PASS -> FAIL (fdo#102670) igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible: shard-apl: PASS -> FAIL (fdo#100368) igt@kms_flip@plain-flip-fb-recreate-interruptible: shard-glk: PASS -> FAIL (fdo#100368) +2 Possible fixes igt@kms_flip@absolute-wf_vblank-interruptible: shard-glk: FAIL (fdo#106087) -> PASS igt@kms_flip@flip-vs-expired-vblank-interruptible: shard-apl: FAIL (fdo#105363, fdo#102887) -> PASS igt@kms_flip@wf_vblank-ts-check-interruptible: shard-glk: FAIL (fdo#100368) -> PASS shard-apl: FAIL (fdo#100368) -> PASS igt@kms_setmode@basic: shard-kbl: FAIL (fdo#99912) -> PASS fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670 fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887 fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232 fdo#104645 https://bugs.freedesktop.org/show_bug.cgi?id=104645 fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#106087 https://bugs.freedesktop.org/show_bug.cgi?id=106087 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (9 -> 9) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4163 -> Patchwork_8973 CI_DRM_4163: 8e1dab6e913be7d014eb9bc355ec65b6b56dcd56 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4468: 548a894dc904c4628522dbbc77cb179a4c965ebc @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_8973: b1792bf8c0170919171d3ef49a254b961794321d @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4468: 1e60f1499e5b71b6d5a747189d7c28f57359a87f @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8973/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] drm/i915: Wrap tasklet_struct for abuse
Quoting Tvrtko Ursulin (2018-05-10 16:49:03) > > On 09/05/2018 15:27, Chris Wilson wrote: > > In the next few patches, we want to abuse tasklet to avoid ksoftirqd > > latency along critical paths. To make that abuse easily to swallow, > > first coat the tasklet in a little syntactic sugar. > > > > Signed-off-by: Chris Wilson > > Cc: Tvrtko Ursulin > > --- > > +static inline void i915_tasklet_unlock(struct i915_tasklet *t) > > +{ > > + tasklet_enable(&t->base); > > +} > > I agree keeping the original naming for kill/enable/disable would be better. > > > + > > +static inline void i915_tasklet_set_func(struct i915_tasklet *t, > > + void (*func)(unsigned long data), > > + unsigned long data) > > +{ > > + i915_tasklet_lock(t); > > + > > + t->base.func = func; > > + t->base.data = data; > > + > > + i915_tasklet_unlock(t); > > I kind of remember something about issues we had with placing > tasklet_disable placed in some contexts. Hm.. if only I could recall the > details. It's probably fine. I cannot come up with ideas on how to > protect against that. GEM_BUG_ON(in_irq() || in_softirq())? Too far > fetched probably. When we get it wrong, CI complains with a lock up. It cannot be used in atomic context as it uses yield() to kick the tasklet (as it may be on the local cpu). > > @@ -2209,7 +2208,10 @@ static void execlists_set_default_submission(struct > > intel_engine_cs *engine) > > engine->submit_request = execlists_submit_request; > > engine->cancel_requests = execlists_cancel_requests; > > engine->schedule = execlists_schedule; > > - engine->execlists.tasklet.func = execlists_submission_tasklet; > > + > > + i915_tasklet_set_func(&engine->execlists.tasklet, > > + execlists_submission_tasklet, > > + (unsigned long)engine); > > Or eliminate the above dilemma by removing the data parameter from > i915_tasklet_set_func since it looks it is not needed at the moment? It doesn't eliminate the dilemma, updating the func itself raises the question of what if the tasklet is running at that time, what was the caller thinking would happen? I expect tasklets will start passing themselves to the func and eliminate the data parameters themselves at some point in the mid term. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/5] drm/i915/execlists: Direct submit onto idle engines
On 09/05/2018 15:27, Chris Wilson wrote: Bypass using the tasklet to submit the first request to HW, as the tasklet may be deferred unto ksoftirqd and at a minimum will add in excess of 10us (and maybe tens of milliseconds) to our execution latency. This latency reduction is most notable when execution flows between engines. v2: Beware handling preemption completion from the direct submit path as well. v3: Make the abuse clear and track our extra state inside i915_tasklet. Suggested-by: Tvrtko Ursulin Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_tasklet.h | 24 +++ drivers/gpu/drm/i915/intel_guc_submission.c | 10 ++- drivers/gpu/drm/i915/intel_lrc.c| 71 + 3 files changed, 89 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_tasklet.h b/drivers/gpu/drm/i915/i915_tasklet.h index 42b002b88edb..99e2fa2241ba 100644 --- a/drivers/gpu/drm/i915/i915_tasklet.h +++ b/drivers/gpu/drm/i915/i915_tasklet.h @@ -8,8 +8,11 @@ #define _I915_TASKLET_H_ #include +#include #include +#include "i915_gem.h" + /** * struct i915_tasklet - wrapper around tasklet_struct * @@ -19,6 +22,8 @@ */ struct i915_tasklet { struct tasklet_struct base; + unsigned long flags; +#define I915_TASKLET_DIRECT_SUBMIT BIT(0) I would suggest a more generic name for the bit since i915_tasklet is generic-ish. For instance simply I915_TASKLET_DIRECT would signify the callback has been invoked directly and not (necessarily) from softirq context. Then it is for each user to know what that means for them specifically. }; static inline void i915_tasklet_init(struct i915_tasklet *t, @@ -43,6 +48,14 @@ static inline bool i915_tasklet_is_enabled(const struct i915_tasklet *t) return likely(!atomic_read(&t->base.count)); } +static inline bool i915_tasklet_is_direct_submit(const struct i915_tasklet *t) +{ + /* Only legal to be checked from inside the tasklet. */ + GEM_BUG_ON(!i915_tasklet_is_running(t)); + + return t->flags & I915_TASKLET_DIRECT_SUBMIT; +} Or maybe i915_tasklet_direct_invocation? + static inline void i915_tasklet_schedule(struct i915_tasklet *t) { tasklet_hi_schedule(&t->base); @@ -75,4 +88,15 @@ static inline void i915_tasklet_set_func(struct i915_tasklet *t, i915_tasklet_unlock(t); } +static inline void __i915_tasklet_run(const struct i915_tasklet *t) +{ + t->base.func(t->base.data); +} + +static inline void i915_tasklet_run(const struct i915_tasklet *t) +{ + GEM_BUG_ON(!i915_tasklet_is_running(t)); + __i915_tasklet_run(t); +} + #endif /* _I915_TASKLET_H_ */ diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index a7afc976c3b9..f2ded1796523 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -754,14 +754,18 @@ static bool __guc_dequeue(struct intel_engine_cs *engine) static void guc_dequeue(struct intel_engine_cs *engine) { - unsigned long flags; + unsigned long uninitialized_var(flags); bool submit; local_irq_save(flags); - spin_lock(&engine->timeline.lock); + if (!i915_tasklet_is_direct_submit(&engine->execlists.tasklet)) + spin_lock(&engine->timeline.lock); + submit = __guc_dequeue(engine); - spin_unlock(&engine->timeline.lock); + + if (!i915_tasklet_is_direct_submit(&engine->execlists.tasklet)) + spin_unlock(&engine->timeline.lock); if (submit) guc_submit(engine); diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 539fa03d7600..09fded9d409f 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -356,13 +356,15 @@ execlists_unwind_incomplete_requests(struct intel_engine_execlists *execlists) { struct intel_engine_cs *engine = container_of(execlists, typeof(*engine), execlists); - unsigned long flags; + unsigned long uninitialized_var(flags); - spin_lock_irqsave(&engine->timeline.lock, flags); + if (!i915_tasklet_is_direct_submit(&execlists->tasklet)) + spin_lock_irqsave(&engine->timeline.lock, flags); __unwind_incomplete_requests(engine); - spin_unlock_irqrestore(&engine->timeline.lock, flags); + if (!i915_tasklet_is_direct_submit(&execlists->tasklet)) + spin_unlock_irqrestore(&engine->timeline.lock, flags); } static inline void @@ -601,6 +603,8 @@ static bool __execlists_dequeue(struct intel_engine_cs *engine) */ GEM_BUG_ON(!execlists_is_active(execlists, EXECLISTS_ACTIVE_USER)); + GEM_BUG_ON(execlists_is_active(execlists, + EXECLISTS_ACTIVE_PREEMPT));
Re: [Intel-gfx] [PATCH 2/5] drm/i915: Wrap tasklet_struct for abuse
On 10/05/2018 17:03, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-05-10 16:49:03) On 09/05/2018 15:27, Chris Wilson wrote: In the next few patches, we want to abuse tasklet to avoid ksoftirqd latency along critical paths. To make that abuse easily to swallow, first coat the tasklet in a little syntactic sugar. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- +static inline void i915_tasklet_unlock(struct i915_tasklet *t) +{ + tasklet_enable(&t->base); +} I agree keeping the original naming for kill/enable/disable would be better. + +static inline void i915_tasklet_set_func(struct i915_tasklet *t, + void (*func)(unsigned long data), + unsigned long data) +{ + i915_tasklet_lock(t); + + t->base.func = func; + t->base.data = data; + + i915_tasklet_unlock(t); I kind of remember something about issues we had with placing tasklet_disable placed in some contexts. Hm.. if only I could recall the details. It's probably fine. I cannot come up with ideas on how to protect against that. GEM_BUG_ON(in_irq() || in_softirq())? Too far fetched probably. When we get it wrong, CI complains with a lock up. It cannot be used in atomic context as it uses yield() to kick the tasklet (as it may be on the local cpu). @@ -2209,7 +2208,10 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine) engine->submit_request = execlists_submit_request; engine->cancel_requests = execlists_cancel_requests; engine->schedule = execlists_schedule; - engine->execlists.tasklet.func = execlists_submission_tasklet; + + i915_tasklet_set_func(&engine->execlists.tasklet, + execlists_submission_tasklet, + (unsigned long)engine); Or eliminate the above dilemma by removing the data parameter from i915_tasklet_set_func since it looks it is not needed at the moment? It doesn't eliminate the dilemma, updating the func itself raises the question of what if the tasklet is running at that time, what was the caller thinking would happen? Well same as it can do now so you could drop tasklet_disable/enable then. At least I assumed the purpose is for atomicity of func/data updates. I expect tasklets will start passing themselves to the func and eliminate the data parameters themselves at some point in the mid term. Needs to be tasklet_disable_nosync then, or it will hang. But then it loses its purpose anyway. So I am confused. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] drm/i915: Wrap tasklet_struct for abuse
Quoting Tvrtko Ursulin (2018-05-10 17:15:46) > > On 10/05/2018 17:03, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-05-10 16:49:03) > >> > >> On 09/05/2018 15:27, Chris Wilson wrote: > >>> In the next few patches, we want to abuse tasklet to avoid ksoftirqd > >>> latency along critical paths. To make that abuse easily to swallow, > >>> first coat the tasklet in a little syntactic sugar. > >>> > >>> Signed-off-by: Chris Wilson > >>> Cc: Tvrtko Ursulin > >>> --- > >>> +static inline void i915_tasklet_unlock(struct i915_tasklet *t) > >>> +{ > >>> + tasklet_enable(&t->base); > >>> +} > >> > >> I agree keeping the original naming for kill/enable/disable would be > >> better. > >> > >>> + > >>> +static inline void i915_tasklet_set_func(struct i915_tasklet *t, > >>> + void (*func)(unsigned long data), > >>> + unsigned long data) > >>> +{ > >>> + i915_tasklet_lock(t); > >>> + > >>> + t->base.func = func; > >>> + t->base.data = data; > >>> + > >>> + i915_tasklet_unlock(t); > >> > >> I kind of remember something about issues we had with placing > >> tasklet_disable placed in some contexts. Hm.. if only I could recall the > >> details. It's probably fine. I cannot come up with ideas on how to > >> protect against that. GEM_BUG_ON(in_irq() || in_softirq())? Too far > >> fetched probably. > > > > When we get it wrong, CI complains with a lock up. It cannot be used in > > atomic context as it uses yield() to kick the tasklet (as it may be on > > the local cpu). > > > >>> @@ -2209,7 +2208,10 @@ static void > >>> execlists_set_default_submission(struct intel_engine_cs *engine) > >>>engine->submit_request = execlists_submit_request; > >>>engine->cancel_requests = execlists_cancel_requests; > >>>engine->schedule = execlists_schedule; > >>> - engine->execlists.tasklet.func = execlists_submission_tasklet; > >>> + > >>> + i915_tasklet_set_func(&engine->execlists.tasklet, > >>> + execlists_submission_tasklet, > >>> + (unsigned long)engine); > >> > >> Or eliminate the above dilemma by removing the data parameter from > >> i915_tasklet_set_func since it looks it is not needed at the moment? > > > > It doesn't eliminate the dilemma, updating the func itself raises the > > question of what if the tasklet is running at that time, what was the > > caller thinking would happen? > > Well same as it can do now so you could drop tasklet_disable/enable > then. At least I assumed the purpose is for atomicity of func/data updates. Sure, it mostly there to catch using it from silly circumstances where the tasklet may be being called as the change occurs. Should never happen, so I bet it does. > > I expect tasklets will start passing themselves to the func and > > eliminate the data parameters themselves at some point in the mid term. > > Needs to be tasklet_disable_nosync then, or it will hang. But then it > loses its purpose anyway. So I am confused. I was just referring to my hope that the unsigned long data parameter will be moved to the great dustbin of history. If timers can make the leap, so can tasklets! -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/5] drm/i915/execlists: Direct submit onto idle engines
Quoting Tvrtko Ursulin (2018-05-10 17:09:14) > > On 09/05/2018 15:27, Chris Wilson wrote: > > Bypass using the tasklet to submit the first request to HW, as the > > tasklet may be deferred unto ksoftirqd and at a minimum will add in > > excess of 10us (and maybe tens of milliseconds) to our execution > > latency. This latency reduction is most notable when execution flows > > between engines. > > > > v2: Beware handling preemption completion from the direct submit path as > > well. > > v3: Make the abuse clear and track our extra state inside i915_tasklet. > > > > Suggested-by: Tvrtko Ursulin > > Signed-off-by: Chris Wilson > > Cc: Tvrtko Ursulin > > --- > > drivers/gpu/drm/i915/i915_tasklet.h | 24 +++ > > drivers/gpu/drm/i915/intel_guc_submission.c | 10 ++- > > drivers/gpu/drm/i915/intel_lrc.c| 71 + > > 3 files changed, 89 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_tasklet.h > > b/drivers/gpu/drm/i915/i915_tasklet.h > > index 42b002b88edb..99e2fa2241ba 100644 > > --- a/drivers/gpu/drm/i915/i915_tasklet.h > > +++ b/drivers/gpu/drm/i915/i915_tasklet.h > > @@ -8,8 +8,11 @@ > > #define _I915_TASKLET_H_ > > > > #include > > +#include > > #include > > > > +#include "i915_gem.h" > > + > > /** > >* struct i915_tasklet - wrapper around tasklet_struct > >* > > @@ -19,6 +22,8 @@ > >*/ > > struct i915_tasklet { > > struct tasklet_struct base; > > + unsigned long flags; > > +#define I915_TASKLET_DIRECT_SUBMIT BIT(0) > > I would suggest a more generic name for the bit since i915_tasklet is > generic-ish. For instance simply I915_TASKLET_DIRECT would signify the > callback has been invoked directly and not (necessarily) from softirq > context. Then it is for each user to know what that means for them > specifically. Problem is we have two direct invocations, only one is special. It really wants to be something like I915_TASKLET_ENGINE_IS_LOCKED - you can see why I didn't propose that. > > -static void __submit_queue(struct intel_engine_cs *engine, int prio) > > +static void __wakeup_queue(struct intel_engine_cs *engine, int prio) > > { > > engine->execlists.queue_priority = prio; > > +} > > Why is this called wakeup? Plans to add something in it later? Yes. It's called wakeup because it's setting the value that the dequeue wakes up at. First name was kick_queue, but it doesn't kick either. The later side-effect involves controlling timers. __restart_queue()? > > +static void __schedule_queue(struct intel_engine_cs *engine) > > +{ > > i915_tasklet_schedule(&engine->execlists.tasklet); > > } > > > > +static bool __direct_submit(struct intel_engine_execlists *const execlists) > > +{ > > + struct i915_tasklet * const t = &execlists->tasklet; > > + > > + if (!tasklet_trylock(&t->base)) > > + return false; > > + > > + t->flags |= I915_TASKLET_DIRECT_SUBMIT; > > + i915_tasklet_run(t); > > + t->flags &= ~I915_TASKLET_DIRECT_SUBMIT; > > + > > + tasklet_unlock(&t->base); > > Feels like this whole sequence belongs to i915_tasklet since it touches > the internals. Maybe i915_tasklet_try_run, or i915_tasklet_run_or_schedule? Keep reading the series and you'll see just why this is so special and confined to execlists. > > + return true; > > +} > > + > > +static void __submit_queue(struct intel_engine_cs *engine) > > +{ > > + struct intel_engine_execlists * const execlists = &engine->execlists; > > + > > + GEM_BUG_ON(!engine->i915->gt.awake); > > + > > + /* If inside GPU reset, the tasklet will be queued later. */ > > + if (!i915_tasklet_is_enabled(&execlists->tasklet)) > > + return; > > + > > + /* Directly submit the first request to reduce the initial latency */ > > + if (port_isset(execlists->port) || !__direct_submit(execlists)) > > + __schedule_queue(engine); > > Hmm a bit evil to maybe invoke in the condition. Would it be acceptable to: > > if (!port_isset(...)) > i915_tasklet_run_or_schedule(...); > else > i915_tasklet_schedule(...); > > It's not ideal but maybe a bit better. Beauty is in the eye of the beholder, and that ain't beautiful :) -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] drm/i915: Wrap tasklet_struct for abuse
On 10/05/2018 17:19, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-05-10 17:15:46) On 10/05/2018 17:03, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-05-10 16:49:03) On 09/05/2018 15:27, Chris Wilson wrote: In the next few patches, we want to abuse tasklet to avoid ksoftirqd latency along critical paths. To make that abuse easily to swallow, first coat the tasklet in a little syntactic sugar. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- +static inline void i915_tasklet_unlock(struct i915_tasklet *t) +{ + tasklet_enable(&t->base); +} I agree keeping the original naming for kill/enable/disable would be better. + +static inline void i915_tasklet_set_func(struct i915_tasklet *t, + void (*func)(unsigned long data), + unsigned long data) +{ + i915_tasklet_lock(t); + + t->base.func = func; + t->base.data = data; + + i915_tasklet_unlock(t); I kind of remember something about issues we had with placing tasklet_disable placed in some contexts. Hm.. if only I could recall the details. It's probably fine. I cannot come up with ideas on how to protect against that. GEM_BUG_ON(in_irq() || in_softirq())? Too far fetched probably. When we get it wrong, CI complains with a lock up. It cannot be used in atomic context as it uses yield() to kick the tasklet (as it may be on the local cpu). @@ -2209,7 +2208,10 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine) engine->submit_request = execlists_submit_request; engine->cancel_requests = execlists_cancel_requests; engine->schedule = execlists_schedule; - engine->execlists.tasklet.func = execlists_submission_tasklet; + + i915_tasklet_set_func(&engine->execlists.tasklet, + execlists_submission_tasklet, + (unsigned long)engine); Or eliminate the above dilemma by removing the data parameter from i915_tasklet_set_func since it looks it is not needed at the moment? It doesn't eliminate the dilemma, updating the func itself raises the question of what if the tasklet is running at that time, what was the caller thinking would happen? Well same as it can do now so you could drop tasklet_disable/enable then. At least I assumed the purpose is for atomicity of func/data updates. Sure, it mostly there to catch using it from silly circumstances where the tasklet may be being called as the change occurs. Should never happen, so I bet it does. You could do similar by asserting tasklet is either disabled, or at least not running. I expect tasklets will start passing themselves to the func and eliminate the data parameters themselves at some point in the mid term. Needs to be tasklet_disable_nosync then, or it will hang. But then it loses its purpose anyway. So I am confused. I was just referring to my hope that the unsigned long data parameter will be moved to the great dustbin of history. If timers can make the leap, so can tasklets! I just don't see the point in adding the second parameter now. I mean it's not a big deal, but I don't see it useful. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/5] drm/i915/execlists: Direct submit onto idle engines
On 10/05/2018 17:25, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-05-10 17:09:14) On 09/05/2018 15:27, Chris Wilson wrote: Bypass using the tasklet to submit the first request to HW, as the tasklet may be deferred unto ksoftirqd and at a minimum will add in excess of 10us (and maybe tens of milliseconds) to our execution latency. This latency reduction is most notable when execution flows between engines. v2: Beware handling preemption completion from the direct submit path as well. v3: Make the abuse clear and track our extra state inside i915_tasklet. Suggested-by: Tvrtko Ursulin Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_tasklet.h | 24 +++ drivers/gpu/drm/i915/intel_guc_submission.c | 10 ++- drivers/gpu/drm/i915/intel_lrc.c| 71 + 3 files changed, 89 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_tasklet.h b/drivers/gpu/drm/i915/i915_tasklet.h index 42b002b88edb..99e2fa2241ba 100644 --- a/drivers/gpu/drm/i915/i915_tasklet.h +++ b/drivers/gpu/drm/i915/i915_tasklet.h @@ -8,8 +8,11 @@ #define _I915_TASKLET_H_ #include +#include #include +#include "i915_gem.h" + /** * struct i915_tasklet - wrapper around tasklet_struct * @@ -19,6 +22,8 @@ */ struct i915_tasklet { struct tasklet_struct base; + unsigned long flags; +#define I915_TASKLET_DIRECT_SUBMIT BIT(0) I would suggest a more generic name for the bit since i915_tasklet is generic-ish. For instance simply I915_TASKLET_DIRECT would signify the callback has been invoked directly and not (necessarily) from softirq context. Then it is for each user to know what that means for them specifically. Problem is we have two direct invocations, only one is special. It really wants to be something like I915_TASKLET_ENGINE_IS_LOCKED - you can see why I didn't propose that. TBC... -static void __submit_queue(struct intel_engine_cs *engine, int prio) +static void __wakeup_queue(struct intel_engine_cs *engine, int prio) { engine->execlists.queue_priority = prio; +} Why is this called wakeup? Plans to add something in it later? Yes. It's called wakeup because it's setting the value that the dequeue wakes up at. First name was kick_queue, but it doesn't kick either. The later side-effect involves controlling timers. __restart_queue()? __update_queue_priority? :) +static void __schedule_queue(struct intel_engine_cs *engine) +{ i915_tasklet_schedule(&engine->execlists.tasklet); } +static bool __direct_submit(struct intel_engine_execlists *const execlists) +{ + struct i915_tasklet * const t = &execlists->tasklet; + + if (!tasklet_trylock(&t->base)) + return false; + + t->flags |= I915_TASKLET_DIRECT_SUBMIT; + i915_tasklet_run(t); + t->flags &= ~I915_TASKLET_DIRECT_SUBMIT; + + tasklet_unlock(&t->base); Feels like this whole sequence belongs to i915_tasklet since it touches the internals. Maybe i915_tasklet_try_run, or i915_tasklet_run_or_schedule? Keep reading the series and you'll see just why this is so special and confined to execlists. ... TBC here. Having peeked ahead, it feels a bit not generic enough as it is, a bit too hacky. Would it work to pass context together with the invocation. Like: i915_tasklet_try(..., I915_TASKLET_SUBMIT_IDLE); i915_tasklet_try(..., I915_TASKLET_SUBMIT_IRQ); i915_tasklet.flags field namespace would then be owned by the caller completely. And the tasklet func itself would have more context on what to do. Following form that, i915_tasklet_run_or_schedule(.., flags). bool i915_taskle_try(tasklet, flags) { if (!trylock) return false; t->flags |= flags; i915_tasklet_run(...); t->flags &= ~flags; tasklet_unlock(...); return true; } void i915_tasklet_run_or_schedule(..., flags) { if (!i915_tasklet_try(..., flags)) i915_tasklet_schedule(...); } ? Leaves a question of a tasklet_is_enabled check in your tasklet_try, which I don't quite get since that check wasn't there before. So why it is needed? + return true; +} + +static void __submit_queue(struct intel_engine_cs *engine) +{ + struct intel_engine_execlists * const execlists = &engine->execlists; + + GEM_BUG_ON(!engine->i915->gt.awake); + + /* If inside GPU reset, the tasklet will be queued later. */ + if (!i915_tasklet_is_enabled(&execlists->tasklet)) + return; + + /* Directly submit the first request to reduce the initial latency */ + if (port_isset(execlists->port) || !__direct_submit(execlists)) + __schedule_queue(engine); Hmm a bit evil to maybe invoke in the condition. Would it be acceptable to: if (!port_isset(...)) i915_tasklet_run_or_schedule(...); else i915_tasklet_schedule(...); It's not ideal but maybe a bit better. Beauty is in the eye of t
Re: [Intel-gfx] [PATCH 3/5] drm/i915/execlists: Direct submit onto idle engines
Quoting Tvrtko Ursulin (2018-05-10 18:26:31) > > On 10/05/2018 17:25, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-05-10 17:09:14) > >> > >> On 09/05/2018 15:27, Chris Wilson wrote: > >>> Bypass using the tasklet to submit the first request to HW, as the > >>> tasklet may be deferred unto ksoftirqd and at a minimum will add in > >>> excess of 10us (and maybe tens of milliseconds) to our execution > >>> latency. This latency reduction is most notable when execution flows > >>> between engines. > >>> > >>> v2: Beware handling preemption completion from the direct submit path as > >>> well. > >>> v3: Make the abuse clear and track our extra state inside i915_tasklet. > >>> > >>> Suggested-by: Tvrtko Ursulin > >>> Signed-off-by: Chris Wilson > >>> Cc: Tvrtko Ursulin > >>> --- > >>>drivers/gpu/drm/i915/i915_tasklet.h | 24 +++ > >>>drivers/gpu/drm/i915/intel_guc_submission.c | 10 ++- > >>>drivers/gpu/drm/i915/intel_lrc.c| 71 + > >>>3 files changed, 89 insertions(+), 16 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/i915/i915_tasklet.h > >>> b/drivers/gpu/drm/i915/i915_tasklet.h > >>> index 42b002b88edb..99e2fa2241ba 100644 > >>> --- a/drivers/gpu/drm/i915/i915_tasklet.h > >>> +++ b/drivers/gpu/drm/i915/i915_tasklet.h > >>> @@ -8,8 +8,11 @@ > >>>#define _I915_TASKLET_H_ > >>> > >>>#include > >>> +#include > >>>#include > >>> > >>> +#include "i915_gem.h" > >>> + > >>>/** > >>> * struct i915_tasklet - wrapper around tasklet_struct > >>> * > >>> @@ -19,6 +22,8 @@ > >>> */ > >>>struct i915_tasklet { > >>>struct tasklet_struct base; > >>> + unsigned long flags; > >>> +#define I915_TASKLET_DIRECT_SUBMIT BIT(0) > >> > >> I would suggest a more generic name for the bit since i915_tasklet is > >> generic-ish. For instance simply I915_TASKLET_DIRECT would signify the > >> callback has been invoked directly and not (necessarily) from softirq > >> context. Then it is for each user to know what that means for them > >> specifically. > > > > Problem is we have two direct invocations, only one is special. It > > really wants to be something like I915_TASKLET_ENGINE_IS_LOCKED - you can > > see why I didn't propose that. > > TBC... > > >>> -static void __submit_queue(struct intel_engine_cs *engine, int prio) > >>> +static void __wakeup_queue(struct intel_engine_cs *engine, int prio) > >>>{ > >>>engine->execlists.queue_priority = prio; > >>> +} > >> > >> Why is this called wakeup? Plans to add something in it later? > > > > Yes. It's called wakeup because it's setting the value that the dequeue > > wakes up at. First name was kick_queue, but it doesn't kick either. > > > > The later side-effect involves controlling timers. > > > > __restart_queue()? > > __update_queue_priority? :) It doesn't just update the priority... Now a choice between restart_queue and update_queue. > >>> +static void __schedule_queue(struct intel_engine_cs *engine) > >>> +{ > >>>i915_tasklet_schedule(&engine->execlists.tasklet); > >>>} > >>> > >>> +static bool __direct_submit(struct intel_engine_execlists *const > >>> execlists) > >>> +{ > >>> + struct i915_tasklet * const t = &execlists->tasklet; > >>> + > >>> + if (!tasklet_trylock(&t->base)) > >>> + return false; > >>> + > >>> + t->flags |= I915_TASKLET_DIRECT_SUBMIT; > >>> + i915_tasklet_run(t); > >>> + t->flags &= ~I915_TASKLET_DIRECT_SUBMIT; > >>> + > >>> + tasklet_unlock(&t->base); > >> > >> Feels like this whole sequence belongs to i915_tasklet since it touches > >> the internals. Maybe i915_tasklet_try_run, or i915_tasklet_run_or_schedule? > > > > Keep reading the series and you'll see just why this is so special and > > confined to execlists. > > ... TBC here. > > Having peeked ahead, it feels a bit not generic enough as it is, a bit > too hacky. > > Would it work to pass context together with the invocation. Like: > > i915_tasklet_try(..., I915_TASKLET_SUBMIT_IDLE); > i915_tasklet_try(..., I915_TASKLET_SUBMIT_IRQ); > > i915_tasklet.flags field namespace would then be owned by the caller > completely. And the tasklet func itself would have more context on what > to do. That doesn't apply very well to the use case either. It's not the tasklet being called from irq/process that's significant but whether we are calling it with the engine/data locked. I keep wanting to use LOCKED, but that has no meaning to the tasklet, and tasklet_trylock means something entirely different. > Following form that, i915_tasklet_run_or_schedule(.., flags). > > bool i915_taskle_try(tasklet, flags) > { > if (!trylock) > return false; > > t->flags |= flags; > i915_tasklet_run(...); > t->flags &= ~flags; > > tasklet_unlock(...); > > return true; > } > > > void i915_tasklet_run_or_schedule(..., flags) > { > if (!i915_tasklet_try(.
[Intel-gfx] [PATCH] drm/i915/selftests: scrub 64K
We write out all PTEs when operating in 64K mode, which is acceptable given the assertion that the hw only cares about every 16th PTE and so will ignore everything else. However this may hide potential issues, for example the hw could be sneakily operating in 4K mode and we would be none the wiser, so make sure this doesn't escape us in the selftests. Signed-off-by: Matthew Auld Cc: Chris Wilson Cc: Changbin Du --- drivers/gpu/drm/i915/i915_gem_gtt.c | 20 drivers/gpu/drm/i915/i915_gem_gtt.h | 1 + drivers/gpu/drm/i915/selftests/huge_pages.c | 3 +++ 3 files changed, 24 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index c879bfd9294f..5deef6044944 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1161,6 +1161,26 @@ static void gen8_ppgtt_insert_huge_entries(struct i915_vma *vma, vaddr[idx.pde] |= GEN8_PDE_IPS_64K; kunmap_atomic(vaddr); page_size = I915_GTT_PAGE_SIZE_64K; + + /* +* For simplicity we choose to write out all PTEs when +* operating in 64K mode, which is acceptable given the +* assertion that the hw only cares about every 16th PTE +* and so will ignore everything else. However this may +* hide potential issues, so make sure this doesn't +* escape us in the selftests. +*/ + if (I915_SELFTEST_ONLY(vma->vm->scrub_64K)) { + u16 i; + + encode = pte_encode | vma->vm->scratch_page.daddr; + vaddr = kmap_atomic_px(pd->page_table[idx.pde]); + + for (i = 1; i < index; i += 16) + memset64(vaddr + i, encode, 15); + + kunmap_atomic(vaddr); + } } vma->page_sizes.gtt |= page_size; diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index 1db0dedb4059..aec4f73574f4 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -342,6 +342,7 @@ struct i915_address_space { void (*clear_pages)(struct i915_vma *vma); I915_SELFTEST_DECLARE(struct fault_attr fault_attr); + I915_SELFTEST_DECLARE(bool scrub_64K); }; #define i915_is_ggtt(V) (!(V)->file) diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c index d7c8ef8e6764..91c72911be3c 100644 --- a/drivers/gpu/drm/i915/selftests/huge_pages.c +++ b/drivers/gpu/drm/i915/selftests/huge_pages.c @@ -1757,6 +1757,9 @@ int i915_gem_huge_page_live_selftests(struct drm_i915_private *dev_priv) goto out_unlock; } + if (ctx->ppgtt) + ctx->ppgtt->base.scrub_64K = true; + err = i915_subtests(tests, ctx); out_unlock: -- 2.17.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/selftests: scrub 64K
Quoting Matthew Auld (2018-05-10 20:00:06) > We write out all PTEs when operating in 64K mode, which is acceptable > given the assertion that the hw only cares about every 16th PTE and so > will ignore everything else. However this may hide potential issues, > for example the hw could be sneakily operating in 4K mode and we would > be none the wiser, so make sure this doesn't escape us in the selftests. For selftesting, don't you want to do the opposite and set the ignored addresses to something that will explode, and not something safe? -Chris ___ 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/selftests: scrub 64K
== Series Details == Series: drm/i915/selftests: scrub 64K URL : https://patchwork.freedesktop.org/series/43023/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4164 -> Patchwork_8974 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/43023/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_8974 that come from known issues: === IGT changes === Possible fixes igt@gem_mmap_gtt@basic-small-bo-tiledx: fi-gdg-551: FAIL (fdo#102575) -> PASS igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-cnl-psr: DMESG-WARN (fdo#104951) -> PASS fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575 fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951 == Participating hosts (40 -> 37) == Additional (1): fi-byt-j1900 Missing(4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-skl-6700hq == Build changes == * Linux: CI_DRM_4164 -> Patchwork_8974 CI_DRM_4164: a44969bdb6d69244a063eac7f76ea46353960409 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4475: 35f08c12aa216d5b62a5b9984b575cee6905098f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_8974: 3877d1bf8190cbeb5c1c163dfa0127cfb96a878c @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4475: 3ba0657bff4216d1ec7179935590261855f1651e @ git://anongit.freedesktop.org/piglit == Linux commits == 3877d1bf8190 drm/i915/selftests: scrub 64K == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8974/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/gen9: Add WaClearHIZ_WM_CHICKEN3 for bxt and glk
Factor in clear values wherever required while updating destination min/max. References: HSDES#160184 Signed-off-by: Michel Thierry Cc: mesa-...@lists.freedesktop.org Cc: Mika Kuoppala Cc: Oscar Mateo --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_workarounds.c | 4 2 files changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 085928c9005e..ee11f6ed217a 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7251,6 +7251,9 @@ enum { #define SLICE_ECO_CHICKEN0 _MMIO(0x7308) #define PIXEL_MASK_CAMMING_DISABLE (1 << 14) +#define GEN9_WM_CHICKEN3 _MMIO(0x5588) +#define GEN9_FACTOR_IN_CLR_VAL_HIZ (1 << 9) + /* WaCatErrorRejectionIssue */ #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG _MMIO(0x9030) #define GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB (1<<11) diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c index ec9d340fcb00..054e1dee7899 100644 --- a/drivers/gpu/drm/i915/intel_workarounds.c +++ b/drivers/gpu/drm/i915/intel_workarounds.c @@ -270,6 +270,10 @@ static int gen9_ctx_workarounds_init(struct drm_i915_private *dev_priv) GEN9_PREEMPT_GPGPU_LEVEL_MASK, GEN9_PREEMPT_GPGPU_COMMAND_LEVEL); + /* WaClearHIZ_WM_CHICKEN3:bxt,glk */ + if (IS_GEN9_LP(dev_priv)) + WA_SET_BIT_MASKED(GEN9_WM_CHICKEN3, GEN9_FACTOR_IN_CLR_VAL_HIZ); + return 0; } -- 2.17.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v12 03/17] drm/i915/guc/slpc: Lay out SLPC init/enable/disable/fini helpers
On Fri, 30 Mar 2018 10:31:48 +0200, Sagar Arun Kamble wrote: SLPC operates based on parameters setup in shared data between i915 and GuC SLPC. This is to be created/initialized in intel_guc_slpc_init. From there onwards i915 can control the SLPC operations by enabling, disabling complete SLPC or changing SLPC parameters. During cleanup, SLPC shared data has to be freed. v1: Return void instead of ignored error code. Replace HAS_SLPC() use with intel_slpc_enabled()/ intel_slpc_active() (Paulo) Enable/disable RC6 in SLPC flows (Sagar) Fix for renaming gen9_disable_rps to gen9_disable_rc6 in "drm/i915/bxt: Explicitly clear the Turbo control register" Defer RC6 and SLPC enabling to intel_gen6_powersave_work. (Sagar) Performance drop with SLPC was happening as ring frequency table was not programmed when SLPC was enabled. This patch programs ring frequency table with SLPC. Initial reset of SLPC is based on kernel parameter as planning to add slpc state in intel_slpc_active. Cleanup is also based on kernel parameter as SLPC gets disabled in disable/suspend.(Sagar) v2: Usage of INTEL_GEN instead of INTEL_INFO->gen (David) Checkpatch update. v3: Rebase v4: Removed reset functions to comply with *_gt_powersave routines. (Sagar) v5: Removed intel_slpc_active. Relying on slpc.active for control flows that are based on SLPC active status in GuC. State setup/cleanup needed for SLPC is handled using kernel parameter i915.enable_slpc. Moved SLPC init and enabling to GuC enable path as SLPC in GuC can start doing the setup post GuC init. Commit message update. (Sagar) v6: Rearranged function definitions. v7: Makefile rearrangement. Reducing usage of i915.enable_slpc and relying mostly on rps.rps_enabled to bypass Host RPS flows. Commit message update. v8: Changed parameters for SLPC functions to struct intel_slpc*. v9: Reinstated intel_slpc_active and intel_slpc_enabled as they are more meaningful. v10: Rebase changes due to creation of intel_guc.h. Updates in intel_guc_cleanup w.r.t slpc cleanup. v11: s/intel_slpc/intel_guc_slpc. Adjusted place for slpc struct inside guc struct. (Michal Wajdeczko) Updated comment about intel_slpc_enable as we plan to not defer the SLPC status check on enabling later and will have to wait for SLPC status as part of intel_slpc_enable itself. Prepared guc_slpc_initialized and guc_slpc_enabled to track state of SLPC initialization and enabling. v12: s/guc_slpc_cleanup/guc_slpc_fini. Updated SLPC flows w.r.t uC flows. Signed-off-by: Tom O'Rourke Signed-off-by: Sagar Arun Kamble Cc: Chris Wilson Cc: Joonas Lahtinen Cc: Radoslaw Szwichtenberg Cc: Michal Wajdeczko Cc: Sujaritha Sundaresan Cc: Jeff McGee --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/intel_guc.h | 2 ++ drivers/gpu/drm/i915/intel_guc_slpc.c | 25 + drivers/gpu/drm/i915/intel_guc_slpc.h | 17 + drivers/gpu/drm/i915/intel_uc.c | 30 +- 5 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/intel_guc_slpc.c create mode 100644 drivers/gpu/drm/i915/intel_guc_slpc.h diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 0c79c19..499cb89 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -90,6 +90,7 @@ i915-y += intel_uc.o \ intel_guc_ct.o \ intel_guc_fw.o \ intel_guc_log.o \ + intel_guc_slpc.o \ intel_guc_submission.o \ intel_huc.o \ intel_huc_fw.o diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h index f1265e1..2d2451a 100644 --- a/drivers/gpu/drm/i915/intel_guc.h +++ b/drivers/gpu/drm/i915/intel_guc.h @@ -31,6 +31,7 @@ #include "intel_guc_ct.h" #include "intel_guc_log.h" #include "intel_guc_reg.h" +#include "intel_guc_slpc.h" #include "intel_uc_fw.h" #include "i915_vma.h" @@ -48,6 +49,7 @@ struct intel_guc { struct intel_uc_fw fw; struct intel_guc_log log; struct intel_guc_ct ct; + struct intel_guc_slpc slpc; /* Offset where Non-WOPCM memory starts. */ u32 ggtt_pin_bias; diff --git a/drivers/gpu/drm/i915/intel_guc_slpc.c b/drivers/gpu/drm/i915/intel_guc_slpc.c new file mode 100644 index 000..63f100c --- /dev/null +++ b/drivers/gpu/drm/i915/intel_guc_slpc.c @@ -0,0 +1,25 @@ +/* + * SPDX-License-Identifier: MIT + * + * Copyright © 2015-2018 Intel Corporation + */ + +#include "intel_guc_slpc.h" + +int intel_guc_slpc_init(struct intel_guc_slpc *slpc) +{ + return 0; +} + +int intel_guc_slpc_enable(struct intel_guc_slpc *slpc) +{ + return 0; +} + +void intel_guc_slpc_disable(struct intel_guc_slpc *slpc) +{ +} + +void intel_guc_slpc_fini(struct intel_guc_slpc *slpc) +{ +} diff --git a/drivers/gpu/drm/i915/intel_guc_slp
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gen9: Add WaClearHIZ_WM_CHICKEN3 for bxt and glk
== Series Details == Series: drm/i915/gen9: Add WaClearHIZ_WM_CHICKEN3 for bxt and glk URL : https://patchwork.freedesktop.org/series/43024/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4164 -> Patchwork_8975 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/43024/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_8975 that come from known issues: === IGT changes === Possible fixes igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-cnl-psr: DMESG-WARN (fdo#104951) -> PASS fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951 == Participating hosts (40 -> 36) == Missing(4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-skl-6700hq == Build changes == * Linux: CI_DRM_4164 -> Patchwork_8975 CI_DRM_4164: a44969bdb6d69244a063eac7f76ea46353960409 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4475: 35f08c12aa216d5b62a5b9984b575cee6905098f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_8975: 381f5f305fccb06be385915c40ed40c0feacdb49 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4475: 3ba0657bff4216d1ec7179935590261855f1651e @ git://anongit.freedesktop.org/piglit == Linux commits == 381f5f305fcc drm/i915/gen9: Add WaClearHIZ_WM_CHICKEN3 for bxt and glk == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8975/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v12 06/17] drm/i915/guc/slpc: Allocate/initialize/release SLPC shared data
On Fri, 30 Mar 2018 10:31:51 +0200, Sagar Arun Kamble wrote: Populate SLPC shared data with required default values for slice count, power source/plan, IA perf MSRs. v1: Update for SLPC interface version 2015.2.4. intel_slpc_active() returns 1 if slpc initialized (Paulo) change default host_os to "Windows" Spelling fixes (Sagar and Nick Hoath). Added WARN for checking if upper 32bits of GTT offset of shared object are zero. (Chris) Updated commit message and moved POWER_PLAN and POWER_SOURCE defn. from later patch. (Akash) Add struct_mutex locking while allocating/releasing slpc shared object. This was caught by CI BAT. Adding SLPC state variable to determine if it is active as it not just dependent on shared data setup. Rebase with guc_allocate_vma related changes. v2: WARN_ON for platform_sku validity and space changes.(David) Checkpatch update. v3: Fixing WARNING in igt@drv_module_reload_basic found in trybot BAT with SLPC Enabled. v4: Updated support for GuC v9. s/slice_total/hweight8(slice_mask)/(Dave). v5: SLPC vma map changes and removed explicit type conversions.(Chris). s/freq_unslice_max|min/unslice__max|min_freq. v6: Commit message update. s/msr_value/val for reuse later. v7: Set default values for tasks and min frequency parameters. Moved init with allocation of data so that post GuC load earlier params persist. v8: Added check for SLPC status during cleanup of shared data. SLPC disabling is asynchronous and should complete within 10us. v9: Enabling Balancer task in SLPC. v10: Rebase. v11: Rebase. Added lock specific to SLPC. Signed-off-by: Tom O'Rourke Signed-off-by: Sagar Arun Kamble Cc: Chris Wilson Cc: Joonas Lahtinen Cc: Radoslaw Szwichtenberg Cc: Michal Wajdeczko Cc: Sujaritha Sundaresan Cc: Jeff McGee --- drivers/gpu/drm/i915/i915_drv.h | 5 + drivers/gpu/drm/i915/intel_guc_slpc.c | 204 ++ drivers/gpu/drm/i915/intel_guc_slpc.h | 3 + 3 files changed, 212 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 5176801..d17e778 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2416,6 +2416,11 @@ intel_info(const struct drm_i915_private *dev_priv) #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support) +#define IS_ULX_SKU(dev_priv) (IS_SKL_ULX(dev_priv) || IS_KBL_ULX(dev_priv)) +#define IS_ULT_SKU(dev_priv) (IS_SKL_ULT(dev_priv) || \ +IS_KBL_ULT(dev_priv) || \ +IS_CFL_ULT(dev_priv)) + #define SKL_REVID_A0 0x0 #define SKL_REVID_B0 0x1 #define SKL_REVID_C0 0x2 diff --git a/drivers/gpu/drm/i915/intel_guc_slpc.c b/drivers/gpu/drm/i915/intel_guc_slpc.c index 63f100c..974a3c0 100644 --- a/drivers/gpu/drm/i915/intel_guc_slpc.c +++ b/drivers/gpu/drm/i915/intel_guc_slpc.c @@ -4,10 +4,203 @@ * Copyright © 2015-2018 Intel Corporation */ +#include + +#include "i915_drv.h" #include "intel_guc_slpc.h" +static inline struct intel_guc *slpc_to_guc(struct intel_guc_slpc *slpc) +{ + return container_of(slpc, struct intel_guc, slpc); +} + +static unsigned int slpc_get_platform_sku(struct drm_i915_private *dev_priv) If you want to use 'slpc_' prefix for your functions, then always pass struct intel_guc_slpc *slpc as first param +{ + enum slpc_platform_sku platform_sku; + + if (IS_ULX_SKU(dev_priv)) + platform_sku = SLPC_PLATFORM_SKU_ULX; + else if (IS_ULT_SKU(dev_priv)) + platform_sku = SLPC_PLATFORM_SKU_ULT; + else + platform_sku = SLPC_PLATFORM_SKU_DT; Do we really need to pass this to SLPC/GuC? + + WARN_ON(platform_sku > 0xFF); Maybe just define this function to return u16 ? + + return platform_sku; +} + +static unsigned int slpc_get_slice_count(struct drm_i915_private *dev_priv) Is this really slpc specific function ? Maybe this can be added as inline from intel_device_info.h ? +{ + unsigned int slice_count = 1; + + if (IS_SKYLAKE(dev_priv)) + slice_count = hweight8(INTEL_INFO(dev_priv)->sseu.slice_mask); + + return slice_count; +} + +static void slpc_mem_set_param(struct slpc_shared_data *data, + u32 id, + u32 value) +{ + data->override_params_set_bits[id >> 5] |= (1 << (id % 32)); + data->override_params_values[id] = value; +} + +static void slpc_mem_unset_param(struct slpc_shared_data *data, +u32 id) +{ + data->override_params_set_bits[id >> 5] &= (~(1 << (id % 32))); + data->override_params_values[id] = 0; +} + +static int slpc_mem_task_control(struct slpc_shared_data *data, +u64 val, u32 enable_id, u32 disable_id) +{ + int ret = 0; + + if (val == SLPC_PARAM_TASK_DEFAULT)
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/selftests: scrub 64K
== Series Details == Series: drm/i915/selftests: scrub 64K URL : https://patchwork.freedesktop.org/series/43023/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4164_full -> Patchwork_8974_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_8974_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_8974_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/43023/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_8974_full: === IGT changes === Warnings igt@gem_mocs_settings@mocs-rc6-dirty-render: shard-kbl: PASS -> SKIP +1 igt@gem_pwrite@big-cpu-random: shard-glk: SKIP -> PASS == Known issues == Here are the changes found in Patchwork_8974_full that come from known issues: === IGT changes === Issues hit igt@gem_ppgtt@blt-vs-render-ctxn: shard-kbl: PASS -> INCOMPLETE (fdo#103665, fdo#106023) igt@kms_flip@flip-vs-expired-vblank: shard-glk: PASS -> FAIL (fdo#105363, fdo#102887) igt@kms_flip@modeset-vs-vblank-race-interruptible: shard-hsw: PASS -> FAIL (fdo#103060) igt@kms_flip@plain-flip-ts-check-interruptible: shard-glk: PASS -> FAIL (fdo#100368) +1 igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-wc: shard-hsw: PASS -> DMESG-WARN (fdo#102614) Possible fixes igt@kms_flip@flip-vs-expired-vblank-interruptible: shard-glk: FAIL (fdo#105363) -> PASS igt@kms_flip@plain-flip-ts-check: shard-glk: FAIL (fdo#100368) -> PASS igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-mmap-gtt: shard-hsw: DMESG-FAIL (fdo#103167, fdo#104724) -> PASS fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614 fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887 fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023 == Participating hosts (9 -> 9) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4164 -> Patchwork_8974 CI_DRM_4164: a44969bdb6d69244a063eac7f76ea46353960409 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4475: 35f08c12aa216d5b62a5b9984b575cee6905098f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_8974: 3877d1bf8190cbeb5c1c163dfa0127cfb96a878c @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4475: 3ba0657bff4216d1ec7179935590261855f1651e @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8974/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/selftests: scrub 64K
On 10 May 2018 at 20:56, Chris Wilson wrote: > Quoting Matthew Auld (2018-05-10 20:00:06) >> We write out all PTEs when operating in 64K mode, which is acceptable >> given the assertion that the hw only cares about every 16th PTE and so >> will ignore everything else. However this may hide potential issues, >> for example the hw could be sneakily operating in 4K mode and we would >> be none the wiser, so make sure this doesn't escape us in the selftests. > > For selftesting, don't you want to do the opposite and set the ignored > addresses to something that will explode, and not something safe? The tests will fail, when we do write_huge some of our writes will land in scratch, and cpu_check will throw a fit. > -Chris > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/icl: Read the correct Gen11 interrupt registers
Stop reading some now deprecated interrupt registers in both debugfs and error state. Instead, read the new equivalents in the Gen11 interrupt repartitioning scheme. Note that the equivalent to the PM ISR & IIR cannot be read without affecting the current state of the system, so I've opted for leaving them out. See gen11_service_one_iir() for more info. v2: else if !!! (Paulo) v3: another else if (Vinay) v4: - Rebased - Renamed patch - Improved the ordering of GENs - Improved the printing of per-GEN info Suggested-by: Paulo Zanoni Signed-off-by: Oscar Mateo Cc: Tvrtko Ursulin Cc: Daniele Ceraolo Spurio Cc: Sagar Arun Kamble Cc: Vinay Belgaumkar --- drivers/gpu/drm/i915/i915_debugfs.c | 28 ++-- drivers/gpu/drm/i915/i915_gpu_error.c | 11 ++- drivers/gpu/drm/i915/i915_gpu_error.h | 2 +- 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index d663a9e0..8a7030a 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1170,19 +1170,22 @@ static int i915_frequency_info(struct seq_file *m, void *unused) intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); - if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv)) { - pm_ier = I915_READ(GEN6_PMIER); - pm_imr = I915_READ(GEN6_PMIMR); - pm_isr = I915_READ(GEN6_PMISR); - pm_iir = I915_READ(GEN6_PMIIR); - pm_mask = I915_READ(GEN6_PMINTRMSK); - } else { + if (INTEL_GEN(dev_priv) >= 11) { + pm_ier = I915_READ(GEN11_GPM_WGBOXPERF_INTR_ENABLE); + pm_imr = I915_READ(GEN11_GPM_WGBOXPERF_INTR_MASK); + } else if (INTEL_GEN(dev_priv) >= 8) { pm_ier = I915_READ(GEN8_GT_IER(2)); pm_imr = I915_READ(GEN8_GT_IMR(2)); pm_isr = I915_READ(GEN8_GT_ISR(2)); pm_iir = I915_READ(GEN8_GT_IIR(2)); - pm_mask = I915_READ(GEN6_PMINTRMSK); + } else { + pm_ier = I915_READ(GEN6_PMIER); + pm_imr = I915_READ(GEN6_PMIMR); + pm_isr = I915_READ(GEN6_PMISR); + pm_iir = I915_READ(GEN6_PMIIR); } + pm_mask = I915_READ(GEN6_PMINTRMSK); + seq_printf(m, "Video Turbo Mode: %s\n", yesno(rpmodectl & GEN6_RP_MEDIA_TURBO)); seq_printf(m, "HW control enabled: %s\n", @@ -1190,8 +1193,13 @@ static int i915_frequency_info(struct seq_file *m, void *unused) seq_printf(m, "SW control enabled: %s\n", yesno((rpmodectl & GEN6_RP_MEDIA_MODE_MASK) == GEN6_RP_MEDIA_SW_MODE)); - seq_printf(m, "PM IER=0x%08x IMR=0x%08x ISR=0x%08x IIR=0x%08x, MASK=0x%08x\n", - pm_ier, pm_imr, pm_isr, pm_iir, pm_mask); + + seq_printf(m, "PM IER=0x%08x IMR=0x%08x, MASK=0x%08x\n", + pm_ier, pm_imr, pm_mask); + if (INTEL_GEN(dev_priv) < 11) { + seq_printf(m, "PM ISR=0x%08x IIR=0x%08x\n", + pm_isr, pm_iir); + } seq_printf(m, "pm_intrmsk_mbz: 0x%08x\n", rps->pm_intrmsk_mbz); seq_printf(m, "GT_PERF_STATUS: 0x%08x\n", gt_perf_status); diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index b98cd44..d9f2f69 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1675,7 +1675,16 @@ static void capture_reg_state(struct i915_gpu_state *error) } /* 4: Everything else */ - if (INTEL_GEN(dev_priv) >= 8) { + if (INTEL_GEN(dev_priv) >= 11) { + error->ier = I915_READ(GEN8_DE_MISC_IER); + error->gtier[0] = I915_READ(GEN11_RENDER_COPY_INTR_ENABLE); + error->gtier[1] = I915_READ(GEN11_VCS_VECS_INTR_ENABLE); + error->gtier[2] = I915_READ(GEN11_GUC_SG_INTR_ENABLE); + error->gtier[3] = I915_READ(GEN11_GPM_WGBOXPERF_INTR_ENABLE); + error->gtier[4] = I915_READ(GEN11_CRYPTO_RSVD_INTR_ENABLE); + error->gtier[5] = I915_READ(GEN11_GUNIT_CSME_INTR_ENABLE); + error->ngtier = 6; + } else if (INTEL_GEN(dev_priv) >= 8) { error->ier = I915_READ(GEN8_DE_MISC_IER); for (i = 0; i < 4; i++) error->gtier[i] = I915_READ(GEN8_GT_IER(i)); diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h b/drivers/gpu/drm/i915/i915_gpu_error.h index dac0f8c..58910f1 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.h +++ b/drivers/
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/icl: Read the correct Gen11 interrupt registers
== Series Details == Series: drm/i915/icl: Read the correct Gen11 interrupt registers URL : https://patchwork.freedesktop.org/series/43027/ State : failure == Summary == CHK include/config/kernel.release CHK include/generated/uapi/linux/version.h CHK include/generated/utsrelease.h CHK include/generated/bounds.h CHK include/generated/timeconst.h CHK include/generated/asm-offsets.h CALLscripts/checksyscalls.sh DESCEND objtool CHK scripts/mod/devicetable-offsets.h CHK include/generated/compile.h CHK kernel/config_data.h CC [M] drivers/gpu/drm/i915/i915_debugfs.o drivers/gpu/drm/i915/i915_debugfs.c: In function ‘i915_frequency_info’: drivers/gpu/drm/i915/i915_debugfs.c:1192:4: error: ‘pm_iir’ may be used uninitialized in this function [-Werror=maybe-uninitialized] seq_printf(m, "PM ISR=0x%08x IIR=0x%08x\n", ^~~ pm_isr, pm_iir); ~~~ drivers/gpu/drm/i915/i915_debugfs.c:1192:4: error: ‘pm_isr’ may be used uninitialized in this function [-Werror=maybe-uninitialized] cc1: all warnings being treated as errors scripts/Makefile.build:312: recipe for target 'drivers/gpu/drm/i915/i915_debugfs.o' failed make[4]: *** [drivers/gpu/drm/i915/i915_debugfs.o] Error 1 scripts/Makefile.build:559: recipe for target 'drivers/gpu/drm/i915' failed make[3]: *** [drivers/gpu/drm/i915] Error 2 scripts/Makefile.build:559: recipe for target 'drivers/gpu/drm' failed make[2]: *** [drivers/gpu/drm] Error 2 scripts/Makefile.build:559: recipe for target 'drivers/gpu' failed make[1]: *** [drivers/gpu] Error 2 Makefile:1060: recipe for target 'drivers' failed make: *** [drivers] Error 2 ___ 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/gen9: Add WaClearHIZ_WM_CHICKEN3 for bxt and glk
== Series Details == Series: drm/i915/gen9: Add WaClearHIZ_WM_CHICKEN3 for bxt and glk URL : https://patchwork.freedesktop.org/series/43024/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4164_full -> Patchwork_8975_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_8975_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_8975_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/43024/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_8975_full: === IGT changes === Warnings igt@gem_pwrite@big-cpu-random: shard-glk: SKIP -> PASS igt@kms_atomic@plane_primary_legacy: shard-snb: PASS -> SKIP == Known issues == Here are the changes found in Patchwork_8975_full that come from known issues: === IGT changes === Issues hit igt@gem_ppgtt@blt-vs-render-ctxn: shard-kbl: PASS -> INCOMPLETE (fdo#103665, fdo#106023) igt@kms_cursor_crc@cursor-128x128-suspend: shard-snb: PASS -> DMESG-WARN (fdo#102365) igt@kms_flip@2x-modeset-vs-vblank-race-interruptible: shard-hsw: PASS -> DMESG-WARN (fdo#102614) igt@kms_flip@flip-vs-expired-vblank: shard-glk: PASS -> FAIL (fdo#105363) igt@kms_flip@flip-vs-wf_vblank-interruptible: shard-apl: PASS -> FAIL (fdo#105312) igt@kms_flip@plain-flip-ts-check-interruptible: shard-glk: PASS -> FAIL (fdo#100368) +2 Possible fixes igt@kms_flip@flip-vs-expired-vblank-interruptible: shard-glk: FAIL (fdo#105363) -> PASS igt@kms_flip@plain-flip-ts-check: shard-glk: FAIL (fdo#100368) -> PASS igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-mmap-gtt: shard-hsw: DMESG-FAIL (fdo#104724, fdo#103167) -> PASS igt@kms_setmode@basic: shard-hsw: FAIL (fdo#99912) -> PASS fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#102365 https://bugs.freedesktop.org/show_bug.cgi?id=102365 fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724 fdo#105312 https://bugs.freedesktop.org/show_bug.cgi?id=105312 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (9 -> 9) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4164 -> Patchwork_8975 CI_DRM_4164: a44969bdb6d69244a063eac7f76ea46353960409 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4475: 35f08c12aa216d5b62a5b9984b575cee6905098f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_8975: 381f5f305fccb06be385915c40ed40c0feacdb49 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4475: 3ba0657bff4216d1ec7179935590261855f1651e @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8975/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/icl: Read the correct Gen11 interrupt registers
Stop reading some now deprecated interrupt registers in both debugfs and error state. Instead, read the new equivalents in the Gen11 interrupt repartitioning scheme. Note that the equivalent to the PM ISR & IIR cannot be read without affecting the current state of the system, so I've opted for leaving them out. See gen11_service_one_iir() for more info. v2: else if !!! (Paulo) v3: another else if (Vinay) v4: - Rebased - Renamed patch - Improved the ordering of GENs - Improved the printing of per-GEN info v5: Avoid maybe-unitialized & add comment explaining the lack of PM ISR & IIR Suggested-by: Paulo Zanoni Signed-off-by: Oscar Mateo Cc: Tvrtko Ursulin Cc: Daniele Ceraolo Spurio Cc: Sagar Arun Kamble Cc: Vinay Belgaumkar --- drivers/gpu/drm/i915/i915_debugfs.c | 34 -- drivers/gpu/drm/i915/i915_gpu_error.c | 11 ++- drivers/gpu/drm/i915/i915_gpu_error.h | 2 +- 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index d663a9e0..d992dd2 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1170,19 +1170,28 @@ static int i915_frequency_info(struct seq_file *m, void *unused) intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); - if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv)) { - pm_ier = I915_READ(GEN6_PMIER); - pm_imr = I915_READ(GEN6_PMIMR); - pm_isr = I915_READ(GEN6_PMISR); - pm_iir = I915_READ(GEN6_PMIIR); - pm_mask = I915_READ(GEN6_PMINTRMSK); - } else { + if (INTEL_GEN(dev_priv) >= 11) { + pm_ier = I915_READ(GEN11_GPM_WGBOXPERF_INTR_ENABLE); + pm_imr = I915_READ(GEN11_GPM_WGBOXPERF_INTR_MASK); + /* +* The equivalent to the PM ISR & IIR cannot be read +* without affecting the current state of the system +*/ + pm_isr = 0; + pm_iir = 0; + } else if (INTEL_GEN(dev_priv) >= 8) { pm_ier = I915_READ(GEN8_GT_IER(2)); pm_imr = I915_READ(GEN8_GT_IMR(2)); pm_isr = I915_READ(GEN8_GT_ISR(2)); pm_iir = I915_READ(GEN8_GT_IIR(2)); - pm_mask = I915_READ(GEN6_PMINTRMSK); + } else { + pm_ier = I915_READ(GEN6_PMIER); + pm_imr = I915_READ(GEN6_PMIMR); + pm_isr = I915_READ(GEN6_PMISR); + pm_iir = I915_READ(GEN6_PMIIR); } + pm_mask = I915_READ(GEN6_PMINTRMSK); + seq_printf(m, "Video Turbo Mode: %s\n", yesno(rpmodectl & GEN6_RP_MEDIA_TURBO)); seq_printf(m, "HW control enabled: %s\n", @@ -1190,8 +1199,13 @@ static int i915_frequency_info(struct seq_file *m, void *unused) seq_printf(m, "SW control enabled: %s\n", yesno((rpmodectl & GEN6_RP_MEDIA_MODE_MASK) == GEN6_RP_MEDIA_SW_MODE)); - seq_printf(m, "PM IER=0x%08x IMR=0x%08x ISR=0x%08x IIR=0x%08x, MASK=0x%08x\n", - pm_ier, pm_imr, pm_isr, pm_iir, pm_mask); + + seq_printf(m, "PM IER=0x%08x IMR=0x%08x, MASK=0x%08x\n", + pm_ier, pm_imr, pm_mask); + if (INTEL_GEN(dev_priv) < 11) { + seq_printf(m, "PM ISR=0x%08x IIR=0x%08x\n", + pm_isr, pm_iir); + } seq_printf(m, "pm_intrmsk_mbz: 0x%08x\n", rps->pm_intrmsk_mbz); seq_printf(m, "GT_PERF_STATUS: 0x%08x\n", gt_perf_status); diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index b98cd44..d9f2f69 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1675,7 +1675,16 @@ static void capture_reg_state(struct i915_gpu_state *error) } /* 4: Everything else */ - if (INTEL_GEN(dev_priv) >= 8) { + if (INTEL_GEN(dev_priv) >= 11) { + error->ier = I915_READ(GEN8_DE_MISC_IER); + error->gtier[0] = I915_READ(GEN11_RENDER_COPY_INTR_ENABLE); + error->gtier[1] = I915_READ(GEN11_VCS_VECS_INTR_ENABLE); + error->gtier[2] = I915_READ(GEN11_GUC_SG_INTR_ENABLE); + error->gtier[3] = I915_READ(GEN11_GPM_WGBOXPERF_INTR_ENABLE); + error->gtier[4] = I915_READ(GEN11_CRYPTO_RSVD_INTR_ENABLE); + error->gtier[5] = I915_READ(GEN11_GUNIT_CSME_INTR_ENABLE); + error->ngtier = 6; + } else if (INTEL_
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/icl: Read the correct Gen11 interrupt registers (rev2)
== Series Details == Series: drm/i915/icl: Read the correct Gen11 interrupt registers (rev2) URL : https://patchwork.freedesktop.org/series/43027/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4164 -> Patchwork_8977 = == Summary - WARNING == Minor unknown changes coming with Patchwork_8977 need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_8977, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/43027/revisions/2/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_8977: === IGT changes === Warnings igt@gem_exec_gttfill@basic: fi-pnv-d510:PASS -> SKIP == Known issues == Here are the changes found in Patchwork_8977 that come from known issues: === IGT changes === Issues hit igt@kms_chamelium@dp-edid-read: fi-kbl-7500u: PASS -> FAIL (fdo#103841) Possible fixes igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-bxt-dsi: INCOMPLETE (fdo#103927) -> PASS fi-cnl-psr: DMESG-WARN (fdo#104951) -> PASS fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951 == Participating hosts (40 -> 37) == Additional (1): fi-byt-j1900 Missing(4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-skl-6700hq == Build changes == * Linux: CI_DRM_4164 -> Patchwork_8977 CI_DRM_4164: a44969bdb6d69244a063eac7f76ea46353960409 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4475: 35f08c12aa216d5b62a5b9984b575cee6905098f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_8977: fe3ca93631ddf6a4be3cd9753bb083ace33dbf26 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4475: 3ba0657bff4216d1ec7179935590261855f1651e @ git://anongit.freedesktop.org/piglit == Linux commits == fe3ca93631dd drm/i915/icl: Read the correct Gen11 interrupt registers == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8977/issues.html ___ 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/icl: Read the correct Gen11 interrupt registers (rev2)
== Series Details == Series: drm/i915/icl: Read the correct Gen11 interrupt registers (rev2) URL : https://patchwork.freedesktop.org/series/43027/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4164_full -> Patchwork_8977_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_8977_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_8977_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/43027/revisions/2/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_8977_full: === IGT changes === Warnings igt@gem_exec_schedule@deep-bsd1: shard-kbl: PASS -> SKIP igt@gem_pwrite@big-cpu-random: shard-apl: SKIP -> PASS == Known issues == Here are the changes found in Patchwork_8977_full that come from known issues: === IGT changes === Issues hit igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible: shard-glk: PASS -> FAIL (fdo#100368) igt@kms_flip@flip-vs-expired-vblank: shard-glk: PASS -> FAIL (fdo#105707) igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-render: shard-apl: PASS -> FAIL (fdo#104724, fdo#103167) igt@kms_plane@plane-panning-top-left-pipe-c-planes: shard-kbl: PASS -> DMESG-WARN (fdo#105602, fdo#103558) +9 Possible fixes igt@kms_flip@flip-vs-expired-vblank-interruptible: shard-glk: FAIL (fdo#105363) -> PASS igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-mmap-gtt: shard-hsw: DMESG-FAIL (fdo#104724, fdo#103167) -> PASS fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558 fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602 fdo#105707 https://bugs.freedesktop.org/show_bug.cgi?id=105707 == Participating hosts (9 -> 9) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4164 -> Patchwork_8977 CI_DRM_4164: a44969bdb6d69244a063eac7f76ea46353960409 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4475: 35f08c12aa216d5b62a5b9984b575cee6905098f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_8977: fe3ca93631ddf6a4be3cd9753bb083ace33dbf26 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4475: 3ba0657bff4216d1ec7179935590261855f1651e @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8977/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/psr: Fix missed entry in PSR setup time table.
Entry corresponding to 220 us setup time was missing. I am not aware of any specific bug this fixes, but this could potentially result in enabling PSR on a panel with a higher setup time requirement than supported by the hardware. I verified the value is present in eDP spec versions 1.3, 1.4 and 1.4a. Fixes: 6608804b3d7f ("drm/dp: Add drm_dp_psr_setup_time()") Cc: sta...@vger.kernel.org Cc: Ville Syrjälä Cc: Jose Roberto de Souza Signed-off-by: Dhinakaran Pandiyan --- drivers/gpu/drm/drm_dp_helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 36c7609a4bd5..a7ba602a43a8 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -1159,6 +1159,7 @@ int drm_dp_psr_setup_time(const u8 psr_cap[EDP_PSR_RECEIVER_CAP_SIZE]) static const u16 psr_setup_time_us[] = { PSR_SETUP_TIME(330), PSR_SETUP_TIME(275), + PSR_SETUP_TIME(220), PSR_SETUP_TIME(165), PSR_SETUP_TIME(110), PSR_SETUP_TIME(55), -- 2.14.1 ___ 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/psr: Fix missed entry in PSR setup time table.
== Series Details == Series: drm/psr: Fix missed entry in PSR setup time table. URL : https://patchwork.freedesktop.org/series/43032/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4164 -> Patchwork_8978 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/43032/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_8978 that come from known issues: === IGT changes === Issues hit igt@gem_exec_suspend@basic-s4-devices: fi-kbl-7500u: PASS -> DMESG-WARN (fdo#105128) Possible fixes igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-cnl-psr: DMESG-WARN (fdo#104951) -> PASS fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951 fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128 == Participating hosts (40 -> 36) == Missing(4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-skl-6700hq == Build changes == * Linux: CI_DRM_4164 -> Patchwork_8978 CI_DRM_4164: a44969bdb6d69244a063eac7f76ea46353960409 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4475: 35f08c12aa216d5b62a5b9984b575cee6905098f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_8978: a0ba0ff180cedd1e5edcf9dd424a64fb9ce0cf78 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4475: 3ba0657bff4216d1ec7179935590261855f1651e @ git://anongit.freedesktop.org/piglit == Linux commits == a0ba0ff180ce drm/psr: Fix missed entry in PSR setup time table. == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8978/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/psr: Fix missed entry in PSR setup time table.
== Series Details == Series: drm/psr: Fix missed entry in PSR setup time table. URL : https://patchwork.freedesktop.org/series/43032/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4164_full -> Patchwork_8978_full = == Summary - FAILURE == Serious unknown changes coming with Patchwork_8978_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_8978_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/43032/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_8978_full: === IGT changes === Possible regressions igt@gem_exec_schedule@preempt-queue-contexts-chain-bsd1: shard-kbl: PASS -> DMESG-WARN Warnings igt@gem_exec_schedule@deep-bsd1: shard-kbl: PASS -> SKIP +1 igt@gem_pwrite@big-cpu-random: shard-apl: SKIP -> PASS == Known issues == Here are the changes found in Patchwork_8978_full that come from known issues: === IGT changes === Issues hit igt@kms_flip@wf_vblank-ts-check-interruptible: shard-glk: PASS -> FAIL (fdo#100368) igt@kms_rotation_crc@sprite-rotation-180: shard-apl: PASS -> DMESG-WARN (fdo#105127) igt@kms_vblank@pipe-b-accuracy-idle: shard-hsw: PASS -> FAIL (fdo#102583) Possible fixes igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-mmap-gtt: shard-hsw: DMESG-FAIL (fdo#104724, fdo#103167) -> PASS fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#102583 https://bugs.freedesktop.org/show_bug.cgi?id=102583 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724 fdo#105127 https://bugs.freedesktop.org/show_bug.cgi?id=105127 == Participating hosts (9 -> 9) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4164 -> Patchwork_8978 CI_DRM_4164: a44969bdb6d69244a063eac7f76ea46353960409 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4475: 35f08c12aa216d5b62a5b9984b575cee6905098f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_8978: a0ba0ff180cedd1e5edcf9dd424a64fb9ce0cf78 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4475: 3ba0657bff4216d1ec7179935590261855f1651e @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8978/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 04/14] drm/i915/gvt: Detect 64K gtt entry by IPS bit of PDE
On 2018.05.10 16:17:35 +0100, Matthew Auld wrote: > > @@ -934,6 +944,20 @@ static int ppgtt_invalidate_spt(struct > > intel_vgpu_ppgtt_spt *spt) > > return ret; > > } > > > > +static bool vgpu_ips_enabled(struct intel_vgpu *vgpu) > > +{ > > + if (INTEL_GEN(vgpu->gvt->dev_priv) == 9) { > > + u32 ips = vgpu_vreg_t(vgpu, GEN8_GAMW_ECO_DEV_RW_IA) & > > + GAMW_ECO_ENABLE_64K_IPS_FIELD; > > + > > + return ips == GAMW_ECO_ENABLE_64K_IPS_FIELD; > > + } else if (INTEL_GEN(vgpu->gvt->dev_priv) >= 10) { > > + /* 64K paging only controlled by IPS bit in PTE now. */ > > + return true; > > AFAIK the funny business with having to enable the IPS bit through > mmio is also needed on GEN10. yeah, looks it only won't be needed from Gen11. Thanks for review! Will update that. -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/selftests: scrub 64K
Quoting Matthew Auld (2018-05-10 22:12:13) > On 10 May 2018 at 20:56, Chris Wilson wrote: > > Quoting Matthew Auld (2018-05-10 20:00:06) > >> We write out all PTEs when operating in 64K mode, which is acceptable > >> given the assertion that the hw only cares about every 16th PTE and so > >> will ignore everything else. However this may hide potential issues, > >> for example the hw could be sneakily operating in 4K mode and we would > >> be none the wiser, so make sure this doesn't escape us in the selftests. > > > > For selftesting, don't you want to do the opposite and set the ignored > > addresses to something that will explode, and not something safe? > > The tests will fail, when we do write_huge some of our writes will > land in scratch, and cpu_check will throw a fit. From reading the comment I was mightily confused, the commitmsg is better for it included an extra clause of explanation. /* * We write all 4K page entries, even when using 64K pages. In order * to verify that the HW isn't cheating by using the 4K PTE instead * of the 64K PTE, we want to remove all the surplus entries. If the HW * skipped the 64K PTE, it will read/write into the scratch page instead - * which we detect as missing results during selftests. */ -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx