Re: [Intel-gfx] [PATCH] firmware/dmc/icl: load v1.07 on icelake.
On 08/02/2018 08:11 AM, Rodrigo Vivi wrote: On Wed, Aug 01, 2018 at 05:30:49PM -0700, Paulo Zanoni wrote: Em Qua, 2018-08-01 às 17:07 -0700, Anusha Srivatsa escreveu: Add Support to load DMC on Icelake. While at it, also add support to load the firmware during system resume. v2: load firmware during system resume.(Imre) Just to make it clear: did we test this on actual machines before submitting or are we entirely relying on the CI results? I'm not sure the CI is running enough tests to validate this patch with confidence, we'll probably need to do some manual testing here. At some point I believe it was agreed that CI would test this and get the new firmware automatically from the cover-letter. The problem is that I don't see any cover-letter so I'm afraid it is not running with the new firmware. Tomi? The requests can be checked from patchwork REST API: https://patchwork.freedesktop.org/api/1.0/projects/intel-gfx/events/?page=1&name=pull-request-new There has been a pull request for new firmware, but it couldn't be acted. CI can't connect to SSH repository, because those generally need an account. This has been tried and tested before. Better way is to use git://, http:// or https:// URLs, as the pull shouldn't need any special permissions. This time I have manually pulled git://anongit.freedesktop.org/drm/drm-firmware/master but, if you want to make this automatic, the pull requests shouldn't be from repositories with obligatory login. Another note: known firmware pull repositories are now freedesktop.org git://anongit.freedesktop.org/drm/drm-firmware (fetch) g_anushasr git://github.com/anushasr/linux-firmware.git (fetch) h_anushasr https://github.com/anushasr/linux-firmware.git (fetch) kernel.org git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware (fetch) If you know that in future there might be pull requests from another repository, please inform me about that in advance. Tomi Also I believe in case it has the cover letter it should run the full CI on the machine or at least stash it and run on the weekend or whenever we run the full on all machines and then report back again. Possible? Martin? Cc: Imre Deak Cc: Rodrigo Vivi Cc: Paulo Zanoni Signed-off-by: Anusha Srivatsa --- drivers/gpu/drm/i915/intel_csr.c| 7 +++ drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++ 2 files changed, 10 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index cf9b600..393d419 100644 --- a/drivers/gpu/drm/i915/intel_csr.c +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -34,6 +34,9 @@ * low-power state and comes back to normal. */ +#define I915_CSR_ICL "i915/icl_dmc_ver1_07.bin" +#define ICL_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) + #define I915_CSR_GLK "i915/glk_dmc_ver1_04.bin" MODULE_FIRMWARE(I915_CSR_GLK); #define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 4) @@ -301,6 +304,8 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, if (csr->fw_path == i915_modparams.dmc_firmware_path) { /* Bypass version check for firmware override. */ required_version = csr->version; + } else if (IS_ICELAKE(dev_priv)) { + required_version = ICL_CSR_VERSION_REQUIRED; } else if (IS_CANNONLAKE(dev_priv)) { required_version = CNL_CSR_VERSION_REQUIRED; } else if (IS_GEMINILAKE(dev_priv)) { @@ -458,6 +463,8 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) if (i915_modparams.dmc_firmware_path) csr->fw_path = i915_modparams.dmc_firmware_path; + else if (IS_ICELAKE(dev_priv)) + csr->fw_path = I915_CSR_ICL; else if (IS_CANNONLAKE(dev_priv)) csr->fw_path = I915_CSR_CNL; else if (IS_GEMINILAKE(dev_priv)) diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index cf89141..77c0986 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -3372,6 +3372,9 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv, /* 7. Setup MBUS. */ icl_mbus_init(dev_priv); + + if (resume && dev_priv->csr.dmc_payload) + intel_csr_load_program(dev_priv); } static void icl_display_core_uninit(struct drm_i915_private *dev_priv) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx Tomi -- Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915: Drop stray clearing of rps->last_adj (rev2)
== Series Details == Series: series starting with [1/2] drm/i915: Drop stray clearing of rps->last_adj (rev2) URL : https://patchwork.freedesktop.org/series/47554/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4605_full -> Patchwork_9838_full = == Summary - FAILURE == Serious unknown changes coming with Patchwork_9838_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_9838_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_9838_full: === Piglit changes === Possible regressions spec@ext_texture_array@fbo-depth-array stencil-draw: pig-glk-j5005: NOTRUN -> INCOMPLETE +3 == Known issues == Here are the changes found in Patchwork_9838_full that come from known issues: === IGT changes === Issues hit igt@gem_eio@throttle: shard-kbl: PASS -> INCOMPLETE (fdo#103665) +5 igt@gem_exec_schedule@preempt-queue-contexts-vebox: shard-apl: PASS -> INCOMPLETE (fdo#103927) +2 igt@gem_userptr_blits@create-destroy-sync: shard-snb: PASS -> INCOMPLETE (fdo#105411) Possible fixes igt@pm_rps@min-max-config-loaded: shard-apl: FAIL (fdo#102250) -> PASS shard-glk: FAIL -> PASS fdo#102250 https://bugs.freedesktop.org/show_bug.cgi?id=102250 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 == Participating hosts (5 -> 6) == Additional (1): pig-glk-j5005 == Build changes == * Linux: CI_DRM_4605 -> Patchwork_9838 CI_DRM_4605: 50098198da758bdd54245d511f4f97236c296c72 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4582: 263ca16e4d8909f475d32a28fc0e5972bac214fb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9838: ccc958dd42f806e8a6a4b700ec69a3e4ba12994b @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9838/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for firmware/dmc/icl: load v1.07 on icelake. (rev2)
== Series Details == Series: firmware/dmc/icl: load v1.07 on icelake. (rev2) URL : https://patchwork.freedesktop.org/series/47450/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4605 -> Patchwork_9839 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/47450/revisions/2/mbox/ == Known issues == Here are the changes found in Patchwork_9839 that come from known issues: === IGT changes === Issues hit igt@debugfs_test@read_all_entries: {fi-icl-u}: NOTRUN -> DMESG-WARN (fdo#107396) igt@drv_selftest@live_guc: fi-skl-guc: NOTRUN -> DMESG-WARN (fdo#107258, fdo#107175) igt@drv_selftest@live_hangcheck: {fi-icl-u}: NOTRUN -> INCOMPLETE (fdo#107399) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: {fi-icl-u}: NOTRUN -> DMESG-WARN (fdo#107382) +4 {igt@kms_psr@primary_page_flip}: {fi-icl-u}: NOTRUN -> FAIL (fdo#107383) +3 Possible fixes igt@drv_selftest@live_workarounds: {fi-bdw-samus}: DMESG-FAIL (fdo#107292) -> PASS igt@prime_vgem@basic-fence-flip: fi-ilk-650: FAIL (fdo#104008) -> PASS {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008 fdo#107175 https://bugs.freedesktop.org/show_bug.cgi?id=107175 fdo#107258 https://bugs.freedesktop.org/show_bug.cgi?id=107258 fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292 fdo#107382 https://bugs.freedesktop.org/show_bug.cgi?id=107382 fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383 fdo#107396 https://bugs.freedesktop.org/show_bug.cgi?id=107396 fdo#107399 https://bugs.freedesktop.org/show_bug.cgi?id=107399 == Participating hosts (50 -> 45) == Additional (2): fi-skl-guc fi-icl-u Missing(7): fi-ilk-m540 fi-bxt-dsi fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper == Build changes == * Linux: CI_DRM_4605 -> Patchwork_9839 CI_DRM_4605: 50098198da758bdd54245d511f4f97236c296c72 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4582: 263ca16e4d8909f475d32a28fc0e5972bac214fb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9839: 56d1d82c3d1f028586fce8d75dfc9040788d550d @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 56d1d82c3d1f firmware/dmc/icl: load v1.07 on icelake. == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9839/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] firmware/dmc/icl: load v1.07 on icelake.
On 08/02/2018 10:15 AM, Tomi Sarvela wrote: On 08/02/2018 08:11 AM, Rodrigo Vivi wrote: On Wed, Aug 01, 2018 at 05:30:49PM -0700, Paulo Zanoni wrote: Em Qua, 2018-08-01 às 17:07 -0700, Anusha Srivatsa escreveu: Add Support to load DMC on Icelake. While at it, also add support to load the firmware during system resume. v2: load firmware during system resume.(Imre) Just to make it clear: did we test this on actual machines before submitting or are we entirely relying on the CI results? I'm not sure the CI is running enough tests to validate this patch with confidence, we'll probably need to do some manual testing here. At some point I believe it was agreed that CI would test this and get the new firmware automatically from the cover-letter. The problem is that I don't see any cover-letter so I'm afraid it is not running with the new firmware. Tomi? The requests can be checked from patchwork REST API: https://patchwork.freedesktop.org/api/1.0/projects/intel-gfx/events/?page=1&name=pull-request-new There has been a pull request for new firmware, but it couldn't be acted. CI can't connect to SSH repository, because those generally need an account. This has been tried and tested before. Better way is to use git://, http:// or https:// URLs, as the pull shouldn't need any special permissions. This time I have manually pulled git://anongit.freedesktop.org/drm/drm-firmware/master but, if you want to make this automatic, the pull requests shouldn't be from repositories with obligatory login. Another note: known firmware pull repositories are now freedesktop.org git://anongit.freedesktop.org/drm/drm-firmware (fetch) g_anushasr git://github.com/anushasr/linux-firmware.git (fetch) h_anushasr https://github.com/anushasr/linux-firmware.git (fetch) kernel.org git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware (fetch) If you know that in future there might be pull requests from another repository, please inform me about that in advance. The results from the re-tested patchset are back, with dmesgs: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9839/fi-icl-u/boot0.log https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9839/fi-icl-u/dmesg0.log Testrun hung in igt@drv_selftest@live_hangcheck, which is standard place for ICL to rest in peace. https://intel-gfx-ci.01.org/tree/drm-tip/fi-icl-u.html Tomi Also I believe in case it has the cover letter it should run the full CI on the machine or at least stash it and run on the weekend or whenever we run the full on all machines and then report back again. Possible? Martin? Cc: Imre Deak Cc: Rodrigo Vivi Cc: Paulo Zanoni Signed-off-by: Anusha Srivatsa --- drivers/gpu/drm/i915/intel_csr.c | 7 +++ drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++ 2 files changed, 10 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index cf9b600..393d419 100644 --- a/drivers/gpu/drm/i915/intel_csr.c +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -34,6 +34,9 @@ * low-power state and comes back to normal. */ +#define I915_CSR_ICL "i915/icl_dmc_ver1_07.bin" +#define ICL_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) + #define I915_CSR_GLK "i915/glk_dmc_ver1_04.bin" MODULE_FIRMWARE(I915_CSR_GLK); #define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 4) @@ -301,6 +304,8 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, if (csr->fw_path == i915_modparams.dmc_firmware_path) { /* Bypass version check for firmware override. */ required_version = csr->version; + } else if (IS_ICELAKE(dev_priv)) { + required_version = ICL_CSR_VERSION_REQUIRED; } else if (IS_CANNONLAKE(dev_priv)) { required_version = CNL_CSR_VERSION_REQUIRED; } else if (IS_GEMINILAKE(dev_priv)) { @@ -458,6 +463,8 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) if (i915_modparams.dmc_firmware_path) csr->fw_path = i915_modparams.dmc_firmware_path; + else if (IS_ICELAKE(dev_priv)) + csr->fw_path = I915_CSR_ICL; else if (IS_CANNONLAKE(dev_priv)) csr->fw_path = I915_CSR_CNL; else if (IS_GEMINILAKE(dev_priv)) diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index cf89141..77c0986 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -3372,6 +3372,9 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv, /* 7. Setup MBUS. */ icl_mbus_init(dev_priv); + + if (resume && dev_priv->csr.dmc_payload) + intel_csr_load_program(dev_priv); } static void icl_display_core_uninit(struct drm_i915_private *dev_priv) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx Tomi Tomi -- Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7,
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Drop stray clearing of rps->last_adj (rev2)
== Series Details == Series: series starting with [1/2] drm/i915: Drop stray clearing of rps->last_adj (rev2) URL : https://patchwork.freedesktop.org/series/47554/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4605 -> Patchwork_9840 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/47554/revisions/2/mbox/ == Known issues == Here are the changes found in Patchwork_9840 that come from known issues: === IGT changes === Issues hit igt@debugfs_test@read_all_entries: {fi-icl-u}: NOTRUN -> DMESG-WARN (fdo#107396) igt@drv_selftest@live_guc: fi-skl-guc: NOTRUN -> DMESG-WARN (fdo#107258, fdo#107175) igt@drv_selftest@live_hangcheck: {fi-icl-u}: NOTRUN -> INCOMPLETE (fdo#107399) igt@drv_selftest@live_workarounds: fi-cnl-psr: PASS -> DMESG-FAIL (fdo#107292) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: {fi-icl-u}: NOTRUN -> DMESG-WARN (fdo#107382) +4 {igt@kms_psr@primary_page_flip}: {fi-icl-u}: NOTRUN -> FAIL (fdo#107383) +3 Possible fixes igt@drv_selftest@live_workarounds: {fi-bdw-samus}: DMESG-FAIL (fdo#107292) -> PASS {igt@kms_psr@primary_mmap_gtt}: fi-cnl-psr: DMESG-WARN (fdo#107372) -> PASS igt@prime_vgem@basic-fence-flip: fi-ilk-650: FAIL (fdo#104008) -> PASS {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008 fdo#107175 https://bugs.freedesktop.org/show_bug.cgi?id=107175 fdo#107258 https://bugs.freedesktop.org/show_bug.cgi?id=107258 fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292 fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372 fdo#107382 https://bugs.freedesktop.org/show_bug.cgi?id=107382 fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383 fdo#107396 https://bugs.freedesktop.org/show_bug.cgi?id=107396 fdo#107399 https://bugs.freedesktop.org/show_bug.cgi?id=107399 == Participating hosts (50 -> 46) == Additional (2): fi-skl-guc fi-icl-u Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper == Build changes == * Linux: CI_DRM_4605 -> Patchwork_9840 CI_DRM_4605: 50098198da758bdd54245d511f4f97236c296c72 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4582: 263ca16e4d8909f475d32a28fc0e5972bac214fb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9840: 698d0f043aef58678566c46bfc1a2238e5a5ff30 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 698d0f043aef drm/i915: Dampen RPS slow start c7ae4f0e91be drm/i915: Drop stray clearing of rps->last_adj == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9840/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 firmware/dmc/icl: load v1.07 on icelake. (rev2)
== Series Details == Series: firmware/dmc/icl: load v1.07 on icelake. (rev2) URL : https://patchwork.freedesktop.org/series/47450/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4605_full -> Patchwork_9839_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_9839_full that come from known issues: === IGT changes === Issues hit igt@drv_suspend@shrink: shard-apl: PASS -> INCOMPLETE (fdo#103927, fdo#106886) igt@gem_userptr_blits@create-destroy-sync: shard-snb: PASS -> INCOMPLETE (fdo#105411) igt@kms_flip@2x-flip-vs-expired-vblank-interruptible: shard-glk: PASS -> FAIL (fdo#105363) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: shard-kbl: PASS -> INCOMPLETE (fdo#103665) Possible fixes igt@kms_setmode@basic: shard-apl: FAIL (fdo#99912) -> PASS shard-kbl: FAIL (fdo#99912) -> PASS fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4605 -> Patchwork_9839 CI_DRM_4605: 50098198da758bdd54245d511f4f97236c296c72 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4582: 263ca16e4d8909f475d32a28fc0e5972bac214fb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9839: 56d1d82c3d1f028586fce8d75dfc9040788d550d @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9839/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] drm/i915: Don't reset on preemptible workloads
On Wed, 2018-08-01 at 15:22 +0100, Chris Wilson wrote: > Quoting Jakub Bartmiński (2018-08-01 14:56:11) > > [...] > > + /* We have already tried preempting, but the hardware did > > not react */ > > + if (engine->hangcheck.try_preempt) > > + return false; > > + > > + active_request = i915_gem_find_active_request(engine); > > No ownership here of rq. You either need a new function to return a > reference, or careful application of rcu. This is a fair point, I'm assuming without taking ownership here we may end up with a nasty race? > > [...] > > + /* > > +* If the workload is preemptible but its context > > was closed > > +* we force the engine to skip its execution > > instead. > > +*/ > > + if (i915_gem_context_is_closed(active_context)) > > + skip_seqno = true; > > Nope. Even if the context is closed, its results may be depended upon > by others; both GPU and userspace. While there may be dependencies, we currently break these dependencies anyway by resetting the entire engine, don't we? Couldn't we signal an error for the skipped requests as if we reseted the engine and continue on with the execution of the remaining requests? If we don't reset/skip we are left with a request that potentially spins forever, with blocked dependent requests, and since it's basically the halting problem we have to draw the line somewhere. > [...] > So the gist of this is to excuse lowest priority user contexts from > hangcheck, do we want to go one step further and ask userspace to opt > in explicitly? Making it explicit opt-in was the idea here, doing it by setting the lowest priority was just a rough way of accomplishing that. > Though the argument that as long as its preemptible (and isolated) it > is not acting as DoS so we don't need to enforce a timeout. That also > suggests that we need not reset until the request is blocking others > or userspace. So if I'm understanding correctly you are suggesting that if we have preemption enabled we could drop the hangcheck entirely, and just reset if preemption fails? > As discussed that seems reasonable, but I think this does need to > actually trigger preemption. > -Chris I'm not sure what you mean right here by "actually trigger preemption", since this patch does trigger the preemption, could you clarify? As I've mentioned before, one other problem with this patch is that we can't have more than one "background" request working before the first one finishes. While this could be solved at hangcheck by putting the background requests in the back of the background-priority queue instead of at the front during preemption, this breaks if the background requests are dependent on each other. These dependencies could be resolved in the tasklet (only in the case hangcheck was called so it wouldn't slow us down too much, they could also be resolved in the hangcheck itself but the timeline could change in between the hangcheck and the tasklet, which would make doing it correctly more complicated I think). - Jakub smime.p7s Description: S/MIME cryptographic signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] drm/i915: Don't reset on preemptible workloads
Quoting Bartminski, Jakub (2018-08-02 09:56:10) > On Wed, 2018-08-01 at 15:22 +0100, Chris Wilson wrote: > > Quoting Jakub Bartmiński (2018-08-01 14:56:11) > > > [...] > > > + /* We have already tried preempting, but the hardware did > > > not react */ > > > + if (engine->hangcheck.try_preempt) > > > + return false; > > > + > > > + active_request = i915_gem_find_active_request(engine); > > > > No ownership here of rq. You either need a new function to return a > > reference, or careful application of rcu. > > This is a fair point, I'm assuming without taking ownership here we may > end up with a nasty race? > > > > [...] > > > + /* > > > +* If the workload is preemptible but its context > > > was closed > > > +* we force the engine to skip its execution > > > instead. > > > +*/ > > > + if (i915_gem_context_is_closed(active_context)) > > > + skip_seqno = true; > > > > Nope. Even if the context is closed, its results may be depended upon > > by others; both GPU and userspace. > > While there may be dependencies, we currently break these dependencies > anyway by resetting the entire engine, don't we? Couldn't we signal an > error for the skipped requests as if we reseted the engine and continue > on with the execution of the remaining requests? That's what we do currently, but we don't bother propagating it past the immediate request. I've played with that in the past, especially important for propagating fatal errors from async workers. > If we don't reset/skip > we are left with a request that potentially spins forever, with blocked > dependent requests, and since it's basically the halting problem we > have to draw the line somewhere. Exactly. Why bother as we could just reset the engine within a few microseconds. > > [...] > > So the gist of this is to excuse lowest priority user contexts from > > hangcheck, do we want to go one step further and ask userspace to opt > > in explicitly? > > Making it explicit opt-in was the idea here, doing it by setting the > lowest priority was just a rough way of accomplishing that. > > > Though the argument that as long as its preemptible (and isolated) it > > is not acting as DoS so we don't need to enforce a timeout. That also > > suggests that we need not reset until the request is blocking others > > or userspace. > > So if I'm understanding correctly you are suggesting that if we have > preemption enabled we could drop the hangcheck entirely, and just reset > if preemption fails? Yes, but we still need the reset if we are stuck at the head of a chain. > > As discussed that seems reasonable, but I think this does need to > > actually trigger preemption. > > -Chris > > I'm not sure what you mean right here by "actually trigger preemption", > since this patch does trigger the preemption, could you clarify? I glossed over the direct call as that is not the way it should be done :-p > As I've mentioned before, one other problem with this patch is that we > can't have more than one "background" request working before the first > one finishes. While this could be solved at hangcheck by putting the > background requests in the back of the background-priority queue > instead of at the front during preemption, this breaks if the > background requests are dependent on each other. These dependencies > could be resolved in the tasklet (only in the case hangcheck was called > so it wouldn't slow us down too much, they could also be resolved in > the hangcheck itself but the timeline could change in between the > hangcheck and the tasklet, which would make doing it correctly more > complicated I think). I think the very simple rule that you reset if there are any dependencies and demote otherwise covers that. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Drop stray clearing of rps->last_adj (rev2)
== Series Details == Series: series starting with [1/2] drm/i915: Drop stray clearing of rps->last_adj (rev2) URL : https://patchwork.freedesktop.org/series/47554/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4605_full -> Patchwork_9840_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_9840_full that come from known issues: === IGT changes === Issues hit igt@drv_suspend@shrink: shard-apl: PASS -> INCOMPLETE (fdo#106886, fdo#103927) igt@gem_sync@basic-store-all: shard-apl: PASS -> INCOMPLETE (fdo#103927) +4 igt@gem_userptr_blits@create-destroy-sync: shard-snb: PASS -> INCOMPLETE (fdo#105411) igt@kms_flip@2x-flip-vs-expired-vblank: shard-glk: PASS -> FAIL (fdo#105363) igt@kms_vblank@pipe-a-wait-forked-busy: shard-kbl: PASS -> INCOMPLETE (fdo#103665) +4 Possible fixes igt@pm_rps@min-max-config-loaded: shard-apl: FAIL (fdo#102250) -> PASS shard-glk: FAIL -> PASS fdo#102250 https://bugs.freedesktop.org/show_bug.cgi?id=102250 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4605 -> Patchwork_9840 CI_DRM_4605: 50098198da758bdd54245d511f4f97236c296c72 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4582: 263ca16e4d8909f475d32a28fc0e5972bac214fb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9840: 698d0f043aef58678566c46bfc1a2238e5a5ff30 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9840/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v9 7/7] drm/i915: add a sysfs entry to let users set sseu configs
[Picking this point in the thread to reply on some points mentioned by folks in the whole thread.] I don't remember if any patches from Lionel's series actually had r-b's, but a few people including myself have certainly been reviewing them. If I had applied the final r-b I wouldn't have made much difference due lack of userspace and disagreement on the DoS mitigation story. So to say effectively put your money where your mouth is and review is not entirely fair. Suggestion to add a master sysfs switch was to alleviate the DoS concerns because dynamic switching has a cost towards all GPU clients, not that it just has potential to slow down media. Suggestion was also that this switch becomes immutable and defaults to "allow" on Gen11 onwards. I preferred sysfs versus a modparam since it makes testing (Both for developers and users (what config works better for my use case?) easier.) I am fine with the suggestion to drive the Gen11 part first, which removes the need for any of this. Patch series is already (AFAIR) nicely split so only the last patch needs to be dropped. If at some point we decide to revisit the Gen8/9 story, we can evaluate/measure whether any master switch is actually warranted. I think Lionel did some micro-benchmarking which showed impact is not so severe, so perhaps for real-world use cases it would be even less. I can re-read the public patches and review them, or perhaps even adopt them if they have been orphaned. Have to check with Francesco first before I commit to the latter. On the uAPI front interface looked fine to me. One recent interesting development are Mesa patches posted to mesa-dev (https://lists.freedesktop.org/archives/mesa-dev/2018-July/200607.html) which add EGL_CONTEXT_load_type extension (low/medium/high). This would need some sort of mapping between low/medium/high to more specific settings but still seems okay to me. This may bring another (open source?) user for the patches. Which Gen's they are interested in is also a question. Regards, Tvrtko On 24/07/2018 22:50, Bloomfield, Jon wrote: Gratuitous top posting to re-kick the thread. For Gen11 we can't have an on/off switch anyway (media simply won't run with an oncompatible slice config), so let's agree on an api to allow userland to select the slice configuration for its contexts, for Gen11 only. I'd prefer a simple {MediaConfig/GeneralConfig} type context param but I really don't care that much. For gen9/10 it's arguable whether we need this at all. The effect on media workloads varies, but I'm guessing normal users (outside a transcode type environment) will never know they're missing anything. Either way, we can continue discussing while we progress the gen11 solution. Jon -Original Message- From: Intel-gfx On Behalf Of Bloomfield, Jon Sent: Wednesday, July 18, 2018 9:44 AM To: Tvrtko Ursulin ; Joonas Lahtinen ; Chris Wilson ; Landwerlin, Lionel G ; intel- g...@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v9 7/7] drm/i915: add a sysfs entry to let users set sseu configs -Original Message- From: Intel-gfx On Behalf Of Tvrtko Ursulin Sent: Thursday, June 14, 2018 1:29 AM To: Joonas Lahtinen ; Chris Wilson ; Landwerlin, Lionel G ; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v9 7/7] drm/i915: add a sysfs entry to let users set sseu configs On 13/06/2018 13:49, Joonas Lahtinen wrote: Quoting Tvrtko Ursulin (2018-06-12 15:02:07) On 12/06/2018 11:52, Lionel Landwerlin wrote: On 12/06/18 11:37, Chris Wilson wrote: Quoting Lionel Landwerlin (2018-06-12 11:33:34) On 12/06/18 10:20, Joonas Lahtinen wrote: Quoting Chris Wilson (2018-06-11 18:02:37) Quoting Lionel Landwerlin (2018-06-11 14:46:07) On 11/06/18 13:10, Tvrtko Ursulin wrote: On 30/05/2018 15:33, Lionel Landwerlin wrote: There are concerns about denial of service around the per context sseu configuration capability. In a previous commit introducing the capability we allowed it only for capable users. This changes adds a new debugfs entry to let any user configure its own context powergating setup. As far as I understood it, Joonas' concerns here are: 1) That in the containers use case individual containers wouldn't be able to turn on the sysfs toggle for them. 2) That also in the containers use case if box admin turns on the feature, some containers would potentially start negatively affecting the others (via the accumulated cost of slice re-configuration on context switching). I am not familiar with typical container setups to be authoritative here, but intuitively I find it reasonable that a low-level hardware switch like this would be under the control of a master domain administrator. ("If you are installing our product in the container environment, make sure your system administrator enables this hardware feature.", "Note to system administrators: Enabling this features may negatively affect the performance of other con
[Intel-gfx] [PATCH 3/4] drm/i915: Clear all residual RPS events on disabling interrupts
Make sure that the RPS IIR is completely clear on disabling so we should not get any more interrupts after idling. Since the IIR is shared with the guc, we have to be careful to only clobber RPS events. Signed-off-by: Chris Wilson Cc: Mika Kuoppala --- drivers/gpu/drm/i915/i915_irq.c | 8 +--- drivers/gpu/drm/i915/i915_reg.h | 6 -- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index e37e3ec22a79..8084e35b25c5 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -478,7 +478,7 @@ void gen11_reset_rps_interrupts(struct drm_i915_private *dev_priv) void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv) { spin_lock_irq(&dev_priv->irq_lock); - gen6_reset_pm_iir(dev_priv, dev_priv->pm_rps_events); + gen6_reset_pm_iir(dev_priv, GEN6_PM_RPS_EVENTS); dev_priv->gt_pm.rps.pm_iir = 0; spin_unlock_irq(&dev_priv->irq_lock); } @@ -516,7 +516,7 @@ void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv) I915_WRITE(GEN6_PMINTRMSK, gen6_sanitize_rps_pm_mask(dev_priv, ~0u)); - gen6_disable_pm_irq(dev_priv, dev_priv->pm_rps_events); + gen6_disable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS); spin_unlock_irq(&dev_priv->irq_lock); synchronize_irq(dev_priv->drm.irq); @@ -4778,7 +4778,9 @@ void intel_irq_init(struct drm_i915_private *dev_priv) /* WaGsvRC0ResidencyMethod:vlv */ dev_priv->pm_rps_events = GEN6_PM_RP_UP_EI_EXPIRED; else - dev_priv->pm_rps_events = GEN6_PM_RPS_EVENTS; + dev_priv->pm_rps_events = (GEN6_PM_RP_UP_THRESHOLD | + GEN6_PM_RP_DOWN_THRESHOLD | + GEN6_PM_RP_DOWN_TIMEOUT); rps->pm_intrmsk_mbz = 0; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index e0f5999fff07..4b656f31fde9 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8582,8 +8582,10 @@ enum { #define GEN6_PM_RP_DOWN_THRESHOLD (1 << 4) #define GEN6_PM_RP_UP_EI_EXPIRED (1 << 2) #define GEN6_PM_RP_DOWN_EI_EXPIRED(1 << 1) -#define GEN6_PM_RPS_EVENTS(GEN6_PM_RP_UP_THRESHOLD | \ -GEN6_PM_RP_DOWN_THRESHOLD | \ +#define GEN6_PM_RPS_EVENTS(GEN6_PM_RP_UP_EI_EXPIRED | \ +GEN6_PM_RP_UP_THRESHOLD| \ +GEN6_PM_RP_DOWN_EI_EXPIRED | \ +GEN6_PM_RP_DOWN_THRESHOLD | \ GEN6_PM_RP_DOWN_TIMEOUT) #define GEN7_GT_SCRATCH(i) _MMIO(0x4F100 + (i) * 4) -- 2.18.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: Dampen RPS slow start
Currently, we note congestion for the slow start ramping up of RPS only when we overshoot the target workload and have to reverse direction for our reclocking. That is, if we have a period where the current GPU frequency is enough to sustain the workload within our target utilisation, we should not trigger any RPS EI interrupts, and then may continue again with the previous last_adj after multiple periods causing us to dramatically overreact. To prevent us not noticing a period where the system is behaving correctly, we can schedule an extra interrupt that will not be associated with either an up or down event causing to reset last_adj back to zero, cancelling the slow start due to the congestion. v2: Separate up/down EI v3: Reset rps events upon enabling Signed-off-by: Chris Wilson Cc: Mika Kuoppala --- drivers/gpu/drm/i915/i915_irq.c | 43 + drivers/gpu/drm/i915/intel_pm.c | 14 --- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 8084e35b25c5..69919a97ec2e 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -493,6 +493,14 @@ void gen6_enable_rps_interrupts(struct drm_i915_private *dev_priv) spin_lock_irq(&dev_priv->irq_lock); WARN_ON_ONCE(rps->pm_iir); + if (IS_VALLEYVIEW(dev_priv)) + /* WaGsvRC0ResidencyMethod:vlv */ + dev_priv->pm_rps_events = GEN6_PM_RP_UP_EI_EXPIRED; + else + dev_priv->pm_rps_events = (GEN6_PM_RP_UP_THRESHOLD | + GEN6_PM_RP_DOWN_THRESHOLD | + GEN6_PM_RP_DOWN_TIMEOUT); + if (INTEL_GEN(dev_priv) >= 11) WARN_ON_ONCE(gen11_reset_one_iir(dev_priv, 0, GEN11_GTPM)); else @@ -1298,7 +1306,13 @@ static void gen6_pm_rps_work(struct work_struct *work) mutex_lock(&dev_priv->pcu_lock); - pm_iir |= vlv_wa_c0_ei(dev_priv, pm_iir); + dev_priv->pm_rps_events &= + ~(GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_UP_EI_EXPIRED); + + if (IS_VALLEYVIEW(dev_priv)) { + dev_priv->pm_rps_events |= GEN6_PM_RP_UP_EI_EXPIRED; + pm_iir |= vlv_wa_c0_ei(dev_priv, pm_iir); + } adj = rps->last_adj; new_delay = rps->cur_freq; @@ -1310,10 +1324,12 @@ static void gen6_pm_rps_work(struct work_struct *work) new_delay = rps->boost_freq; adj = 0; } else if (pm_iir & GEN6_PM_RP_UP_THRESHOLD) { - if (adj > 0) + if (adj > 0) { + dev_priv->pm_rps_events |= GEN6_PM_RP_UP_EI_EXPIRED; adj *= 2; - else /* CHV needs even encode values */ + } else { /* CHV needs even encode values */ adj = IS_CHERRYVIEW(dev_priv) ? 2 : 1; + } if (new_delay >= rps->max_freq_softlimit) adj = 0; @@ -1326,15 +1342,21 @@ static void gen6_pm_rps_work(struct work_struct *work) new_delay = rps->min_freq_softlimit; adj = 0; } else if (pm_iir & GEN6_PM_RP_DOWN_THRESHOLD) { - if (adj < 0) + if (adj < 0) { + dev_priv->pm_rps_events |= GEN6_PM_RP_DOWN_EI_EXPIRED; adj *= 2; - else /* CHV needs even encode values */ + } else { /* CHV needs even encode values */ adj = IS_CHERRYVIEW(dev_priv) ? -2 : -1; + } if (new_delay <= rps->min_freq_softlimit) adj = 0; - } else { /* unknown event */ + } else if (pm_iir & GEN6_PM_RP_UP_THRESHOLD && adj > 0) { + adj = 0; + } else if (pm_iir & GEN6_PM_RP_DOWN_THRESHOLD && adj < 0) { adj = 0; + } else { + /* unknown event */ } rps->last_adj = adj; @@ -4773,15 +4795,6 @@ void intel_irq_init(struct drm_i915_private *dev_priv) if (HAS_GUC_SCHED(dev_priv)) dev_priv->pm_guc_events = GEN9_GUC_TO_HOST_INT_EVENT; - /* Let's track the enabled rps events */ - if (IS_VALLEYVIEW(dev_priv)) - /* WaGsvRC0ResidencyMethod:vlv */ - dev_priv->pm_rps_events = GEN6_PM_RP_UP_EI_EXPIRED; - else - dev_priv->pm_rps_events = (GEN6_PM_RP_UP_THRESHOLD | - GEN6_PM_RP_DOWN_THRESHOLD | - GEN6_PM_RP_DOWN_TIMEOUT); - rps->pm_intrmsk_mbz = 0; /* diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index f90a3c7f1c40..d71a498ee3a1 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -6397,10 +6397,16 @@ static u32 gen6_rps_pm_mask(struct drm_i915_private *dev_p
[Intel-gfx] [PATCH 2/4] drm/i915: Unconditionally clear the pm/guc GT IIR upon acking
Having stored the IIR for action, we should always clear it. Signed-off-by: Chris Wilson Cc: Mika Kuoppala --- drivers/gpu/drm/i915/i915_irq.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 90628a47ae17..e37e3ec22a79 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1534,11 +1534,8 @@ static void gen8_gt_irq_ack(struct drm_i915_private *i915, if (master_ctl & (GEN8_GT_PM_IRQ | GEN8_GT_GUC_IRQ)) { gt_iir[2] = raw_reg_read(regs, GEN8_GT_IIR(2)); - if (likely(gt_iir[2] & (i915->pm_rps_events | - i915->pm_guc_events))) - raw_reg_write(regs, GEN8_GT_IIR(2), - gt_iir[2] & (i915->pm_rps_events | - i915->pm_guc_events)); + if (likely(gt_iir[2])) + raw_reg_write(regs, GEN8_GT_IIR(2), gt_iir[2]); } if (master_ctl & GEN8_GT_VECS_IRQ) { -- 2.18.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915: Drop stray clearing of rps->last_adj
We used to reset last_adj to 0 on crossing a power domain boundary, to slow down our rate of change. However, commit 60548c554be2 ("drm/i915: Interactive RPS mode") accidentally caused it to be reset on every frequency update, nerfing the fast response granted by the slow start algorithm. Fixes: 60548c554be2 ("drm/i915: Interactive RPS mode") Testcase: igt/pm_rps/mix-max-config-loaded Signed-off-by: Chris Wilson Cc: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_pm.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 2531eb75bdce..f90a3c7f1c40 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -6371,7 +6371,6 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) new_power = HIGH_POWER; rps_set_power(dev_priv, new_power); mutex_unlock(&rps->power.mutex); - rps->last_adj = 0; } void intel_rps_mark_interactive(struct drm_i915_private *i915, bool interactive) -- 2.18.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 21/23] drm/i915/icl: Add Display Stream Splitter control registers
On 7/31/2018 7:43 AM, Manasi Navare wrote: From: "Srivatsa, Anusha" Add defines for DSS_CTL registers. These registers specify the big joiner, splitter, overlap pixels and info regarding display stream compression enabled on left or right branch. v2: - Add define to conditionally check the buffer target depth (James Ausmus) Suggested-by: Madhav Chauhan Cc: Madhav Chauhan Cc: Manasi Navare Cc: Rodrigo Vivi Signed-off-by: Anusha Srivatsa --- drivers/gpu/drm/i915/i915_reg.h | 33 + 1 file changed, 33 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index b8e41db..0ae38b6 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7792,6 +7792,39 @@ enum { #define RC_MAX_QP_SHIFT 5 #define RC_MIN_QP_SHIFT 0 +/* Display Stream Splitter Control */ +#define DSS_CTL1 _MMIO(0x67400) +#define SPLITTER_ENABLE (1 << 31) +#define JOINER_ENABLE (1 << 30) +#define DUAL_LINK_MODE_INTERLEAVE (1 << 24) +#define DUAL_LINK_MODE_FRONTBACK (0 << 24) +#define OVERLAP_PIXELS_MASK (0xf << 16) +#define OVERLAP_PIXELS(pixels)((pixels) << 16) +#define LEFT_DL_BUF_TARGET_DEPTH_MASK (0xfff << 0) +#define LEFT_DL_BUF_TARGET_DEPTH(pixels) ((pixels) << 0) +#define MAX_DL_BUFFER_TARGET_DEPTH0x5A0 Please use lower case for hex values. With this fix, Reviewed-by: Madhav Chauhan Regards, Madhav + +#define DSS_CTL2 _MMIO(0x67404) +#define LEFT_BRANCH_VDSC_ENABLE (1 << 31) +#define RIGHT_BRANCH_VDSC_ENABLE (1 << 15) +#define RIGHT_DL_BUF_TARGET_DEPTH_MASK(0xfff << 0) +#define RIGHT_DL_BUF_TARGET_DEPTH(pixels) ((pixels) << 0) + +#define _ICL_PIPE_DSS_CTL1_PB 0x78200 +#define _ICL_PIPE_DSS_CTL1_PC 0x78400 +#define ICL_PIPE_DSS_CTL1(pipe)_MMIO_PIPE((pipe) - PIPE_B, \ + _ICL_PIPE_DSS_CTL1_PB, \ + _ICL_PIPE_DSS_CTL1_PC) +#define BIG_JOINER_ENABLE (1 << 29) +#define MASTER_BIG_JOINER_ENABLE (1 << 28) +#define VGA_CENTERING_ENABLE (1 << 27) + +#define _ICL_PIPE_DSS_CTL2_PB 0x78204 +#define _ICL_PIPE_DSS_CTL2_PC 0x78404 +#define ICL_PIPE_DSS_CTL2(pipe)_MMIO_PIPE((pipe) - PIPE_B, \ + _ICL_PIPE_DSS_CTL2_PB, \ + _ICL_PIPE_DSS_CTL2_PC) + #define DSCA_RC_RANGE_PARAMETERS_1_MMIO(0x6B248) #define DSCA_RC_RANGE_PARAMETERS_1_UDW_MMIO(0x6B248 + 4) #define DSCC_RC_RANGE_PARAMETERS_1_MMIO(0x6BA48) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/4] trace.pl: Context save only applies to last request of a bunch
On 27/07/2018 22:37, John Harrison wrote: On 7/19/2018 2:35 AM, Tvrtko Ursulin wrote: 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 | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) Not sure if you are not getting some of my replies? I have definitely sent an R-B to this patch at least twice already. Maybe they are getting caught by some kind of mailing list filter? I know your patches appear randomly in one of three mailboxes for me - inbox, IGT or IntelGFX according to which rule happens to take precedence on any given second :(. Anyway... Reviewed-by: John Harrison Thanks! I looked backwards a bit and it seems (from my end at least!) that on previous two occasions you did a reply instead of reply to all so I missed it. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 3/4] trace.pl: Bring back timeline stacking
On 27/07/2018 22:47, John Harrison wrote: On 7/19/2018 2:36 AM, Tvrtko Ursulin wrote: From: Tvrtko Ursulin Bring back the button which expands/stacks overlapping timeline boxes. We default to no stacking, but sometimes expanding the view can be useful, especially with deep request pipelines. Signed-off-by: Tvrtko Ursulin --- scripts/trace.pl | 7 +++ 1 file changed, 7 insertions(+) diff --git a/scripts/trace.pl b/scripts/trace.pl index 59f6d32dc3c8..1924333e12b6 100755 --- a/scripts/trace.pl +++ b/scripts/trace.pl @@ -935,6 +935,7 @@ Boxes are in format 'ctx-id/seqno'. Use Ctrl+scroll-action to zoom-in/out and scroll-action or dragging to move around the timeline. +Toggle overlap stacking @@ -1284,6 +1285,12 @@ print < // Create a Timeline var timeline = new vis.Timeline(container, items, groups, options); + + function toggleStacking() { + options.stack = !options.stack; + options.stackSubgroups = !options.stackSubgroups; + timeline.setOptions(options); + } ENDHTML print < Reviewed-by: John Harrison Pushed up to this one, thanks! (BTW this reply even patchwork did not see. But it saw your replies to 1 & 2.) Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 4/4] trace.pl: Fix frequency timeline
On 27/07/2018 23:17, John Harrison wrote: On 7/19/2018 2:36 AM, Tvrtko Ursulin wrote: From: Tvrtko Ursulin Frequency timeline needs to be finished with an entry spanning to the end of known time so that the last known frequency is displayed. Signed-off-by: Tvrtko Ursulin --- scripts/trace.pl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/trace.pl b/scripts/trace.pl index 1924333e12b6..2976cfdf585a 100755 --- a/scripts/trace.pl +++ b/scripts/trace.pl @@ -1201,6 +1201,8 @@ foreach my $key (sort sortQueue keys %db) { last if $i > $max_items; } +push @freqs, [$prev_freq_ts, $last_ts, $prev_freq] if $prev_freq; + foreach my $item (@freqs) { my ($start, $end, $freq) = @$item; my $startend; This does not appear to do anything for me. At least not with any of my trace files. I get exactly the same output with or without the change. What situation is it meant to fix? It fixes the frequency box ending at the last intel_gpu_freq_change timestamp instead of at the end of the displayed timeline. Note that I get the frequency line abbreviated to the size of the request trace. Not sure if that is intentional or not. E.g. with the following trace data the frequency bar starts with 300 at a time of 728us not 706us. Likewise, it ends at 389838us not 392227us: gem_exec_trace 1316 [002] 856981.389706: i915:intel_gpu_freq_change: new_freq=300 gem_exec_trace 1316 [002] 856981.389728:i915:i915_request_add: dev=0, engine=0:0, hw_id=2, ctx=847, seqno=1, global=0 gem_exec_trace 1316 [002] 856981.389732: i915:i915_request_submit: dev=0, engine=0:0, hw_id=2, ctx=847, seqno=1, global=0 gem_exec_trace 1316 [002] 856981.389739: i915:i915_request_in: dev=0, engine=0:0, hw_id=2, ctx=847, seqno=1, prio=0, global=1, port=0 swapper 0 [002] 856981.389838:i915:i915_request_out: dev=0, engine=0:0, hw_id=2, ctx=847, seqno=1, global=1, completed?=1 kworker/u8:1 1246 [001] 856981.392227: i915:intel_gpu_freq_change: new_freq=300 For this I have no explanation. All processing is happening inside the freq change tracepoint so I don't understand how it could get one belonging to a different event. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tests/kms_pipe_crc_basic: expect setting bad source to fail
Op 02-07-18 om 13:27 schreef Maarten Lankhorst: > Op 02-07-18 om 13:16 schreef Mahesh Kumar: >> Now crc-core framework verifies the source string passed by the user. >> So setting bad-source will fail. Expect file write to fail in bad-source >> subtest of kms_pipe_crc_basic. >> >> Signed-off-by: Mahesh Kumar >> --- >> tests/kms_pipe_crc_basic.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c >> index 235fdc38..2d4dfee8 100644 >> --- a/tests/kms_pipe_crc_basic.c >> +++ b/tests/kms_pipe_crc_basic.c >> @@ -48,8 +48,7 @@ static struct { >> >> static void test_bad_source(data_t *data) >> { >> -igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")); >> -igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1); >> +igt_assert(!igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")); >> } >> >> #define N_CRCS 3 > New behavior makes more sense. > > Reviewed-by: Maarten Lankhorst > > Do you have igt commit rights? > Any objections if we change this to allow both behaviors? diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c index 235fdc386ba2..91909fa42f2f 100644 --- a/tests/kms_pipe_crc_basic.c +++ b/tests/kms_pipe_crc_basic.c @@ -48,8 +48,11 @@ static struct { static void test_bad_source(data_t *data) { - igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")); - igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1); + errno = 0; + if (igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")) + igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1); + else + igt_assert_eq(errno, EINVAL); } #define N_CRCS 3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tests/kms_pipe_crc_basic: expect setting bad source to fail
Op 02-08-18 om 12:42 schreef Maarten Lankhorst: > Op 02-07-18 om 13:27 schreef Maarten Lankhorst: >> Op 02-07-18 om 13:16 schreef Mahesh Kumar: >>> Now crc-core framework verifies the source string passed by the user. >>> So setting bad-source will fail. Expect file write to fail in bad-source >>> subtest of kms_pipe_crc_basic. >>> >>> Signed-off-by: Mahesh Kumar >>> --- >>> tests/kms_pipe_crc_basic.c | 3 +-- >>> 1 file changed, 1 insertion(+), 2 deletions(-) >>> >>> diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c >>> index 235fdc38..2d4dfee8 100644 >>> --- a/tests/kms_pipe_crc_basic.c >>> +++ b/tests/kms_pipe_crc_basic.c >>> @@ -48,8 +48,7 @@ static struct { >>> >>> static void test_bad_source(data_t *data) >>> { >>> - igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")); >>> - igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1); >>> + igt_assert(!igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")); >>> } >>> >>> #define N_CRCS 3 >> New behavior makes more sense. >> >> Reviewed-by: Maarten Lankhorst >> >> Do you have igt commit rights? >> > Any objections if we change this to allow both behaviors? > > diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c > index 235fdc386ba2..91909fa42f2f 100644 > --- a/tests/kms_pipe_crc_basic.c > +++ b/tests/kms_pipe_crc_basic.c > @@ -48,8 +48,11 @@ static struct { > > static void test_bad_source(data_t *data) > { > - igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")); > - igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1); > + errno = 0; > + if (igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")) > + igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) > == -1); > + else > + igt_assert_eq(errno, EINVAL); > } > > #define N_CRCS 3 > Hm without the else, errno should be EINVAL in any case.. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: Drop stray clearing of rps->last_adj
== Series Details == Series: series starting with [1/4] drm/i915: Drop stray clearing of rps->last_adj URL : https://patchwork.freedesktop.org/series/47600/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4605 -> Patchwork_9841 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/47600/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_9841 that come from known issues: === IGT changes === Issues hit igt@drv_selftest@live_guc: fi-skl-guc: NOTRUN -> DMESG-WARN (fdo#107258, fdo#107175) igt@drv_selftest@live_workarounds: {fi-cfl-8109u}: PASS -> DMESG-FAIL (fdo#107292) fi-cnl-psr: PASS -> DMESG-FAIL (fdo#107292) Possible fixes igt@drv_selftest@live_workarounds: {fi-bdw-samus}: DMESG-FAIL (fdo#107292) -> PASS igt@prime_vgem@basic-fence-flip: fi-ilk-650: FAIL (fdo#104008) -> PASS Warnings {igt@kms_psr@primary_page_flip}: fi-cnl-psr: DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372) {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008 fdo#107175 https://bugs.freedesktop.org/show_bug.cgi?id=107175 fdo#107258 https://bugs.freedesktop.org/show_bug.cgi?id=107258 fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292 fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372 == Participating hosts (50 -> 44) == Additional (1): fi-skl-guc Missing(7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-kbl-7560u fi-byt-clapper == Build changes == * Linux: CI_DRM_4605 -> Patchwork_9841 CI_DRM_4605: 50098198da758bdd54245d511f4f97236c296c72 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4582: 263ca16e4d8909f475d32a28fc0e5972bac214fb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9841: 6508fa1fc8c9799bceee3853bc7025c254c11afb @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 6508fa1fc8c9 drm/i915: Dampen RPS slow start 3e13f77aebce drm/i915: Clear all residual RPS events on disabling interrupts b66d6b661fde drm/i915: Unconditionally clear the pm/guc GT IIR upon acking 8618cf85463b drm/i915: Drop stray clearing of rps->last_adj == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9841/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tests/kms_pipe_crc_basic: expect setting bad source to fail
Hi, On 8/2/2018 4:13 PM, Maarten Lankhorst wrote: Op 02-08-18 om 12:42 schreef Maarten Lankhorst: Op 02-07-18 om 13:27 schreef Maarten Lankhorst: Op 02-07-18 om 13:16 schreef Mahesh Kumar: Now crc-core framework verifies the source string passed by the user. So setting bad-source will fail. Expect file write to fail in bad-source subtest of kms_pipe_crc_basic. Signed-off-by: Mahesh Kumar --- tests/kms_pipe_crc_basic.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c index 235fdc38..2d4dfee8 100644 --- a/tests/kms_pipe_crc_basic.c +++ b/tests/kms_pipe_crc_basic.c @@ -48,8 +48,7 @@ static struct { static void test_bad_source(data_t *data) { - igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")); - igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1); + igt_assert(!igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")); } #define N_CRCS 3 New behavior makes more sense. Reviewed-by: Maarten Lankhorst Do you have igt commit rights? Any objections if we change this to allow both behaviors? diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c index 235fdc386ba2..91909fa42f2f 100644 --- a/tests/kms_pipe_crc_basic.c +++ b/tests/kms_pipe_crc_basic.c @@ -48,8 +48,11 @@ static struct { static void test_bad_source(data_t *data) { - igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")); - igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1); + errno = 0; + if (igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")) + igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1); + else + igt_assert_eq(errno, EINVAL); } #define N_CRCS 3 Hm without the else, errno should be EINVAL in any case.. agree, with this change Reviewed-by: Mahesh Kumar ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] drm-misc-next-fixes
Hi Dave, Two fixes for 4.19 here. For an oops on the DP CEC code and a memory leak on the vkms driver. Please pull. Regards, Gustavo drm-misc-next-fixes-2018-08-02: Fixes an oops on the DP CEC code and a memory leak on the vkms driver. The following changes since commit 6d52aacd92c60331ec8c3117522f4301b5195e28: Merge branch 'drm-next-4.19' of git://people.freedesktop.org/~agd5f/linux into drm-next (2018-07-27 12:31:48 +1000) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2018-08-02 for you to fetch changes up to 2ead1be54b22ccdc93d3030172993e363128f1b4: drm/vkms: Fix connector leak at the module removal (2018-07-28 16:09:39 -0300) Fixes an oops on the DP CEC code and a memory leak on the vkms driver. Hans Verkuil (1): drm_dp_cec.c: fix formatting typo: %pdH -> %phD Rodrigo Siqueira (1): drm/vkms: Fix connector leak at the module removal drivers/gpu/drm/drm_dp_cec.c| 2 +- drivers/gpu/drm/vkms/vkms_drv.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/4] drm/i915: Drop stray clearing of rps->last_adj
== Series Details == Series: series starting with [1/4] drm/i915: Drop stray clearing of rps->last_adj URL : https://patchwork.freedesktop.org/series/47600/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4605_full -> Patchwork_9841_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_9841_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_9841_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_9841_full: === IGT changes === Warnings igt@kms_frontbuffer_tracking@fbc-1p-rte: shard-snb: PASS -> SKIP +2 == Known issues == Here are the changes found in Patchwork_9841_full that come from known issues: === IGT changes === Issues hit igt@gem_userptr_blits@create-destroy-sync: shard-snb: PASS -> INCOMPLETE (fdo#105411) igt@kms_flip@modeset-vs-vblank-race: shard-glk: PASS -> FAIL (fdo#103060) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: shard-kbl: PASS -> INCOMPLETE (fdo#103665) Possible fixes igt@kms_setmode@basic: shard-apl: FAIL (fdo#99912) -> PASS igt@pm_rps@min-max-config-loaded: shard-apl: FAIL (fdo#102250) -> PASS shard-glk: FAIL -> PASS fdo#102250 https://bugs.freedesktop.org/show_bug.cgi?id=102250 fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4605 -> Patchwork_9841 CI_DRM_4605: 50098198da758bdd54245d511f4f97236c296c72 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4582: 263ca16e4d8909f475d32a28fc0e5972bac214fb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9841: 6508fa1fc8c9799bceee3853bc7025c254c11afb @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9841/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for tests/kms_pipe_crc_basic: expect setting bad source to fail (rev2)
== Series Details == Series: tests/kms_pipe_crc_basic: expect setting bad source to fail (rev2) URL : https://patchwork.freedesktop.org/series/45768/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4605 -> IGTPW_1677 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/45768/revisions/2/mbox/ == Known issues == Here are the changes found in IGTPW_1677 that come from known issues: === IGT changes === Issues hit igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: fi-snb-2520m: PASS -> INCOMPLETE (fdo#103713) Possible fixes igt@prime_vgem@basic-fence-flip: fi-ilk-650: FAIL (fdo#104008) -> PASS fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008 == Participating hosts (50 -> 43) == Additional (1): fi-skl-guc Missing(8): fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-kbl-7560u fi-byt-clapper == Build changes == * IGT: IGT_4582 -> IGTPW_1677 CI_DRM_4605: 50098198da758bdd54245d511f4f97236c296c72 @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_1677: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1677/ IGT_4582: 263ca16e4d8909f475d32a28fc0e5972bac214fb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1677/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/10] drm/i915/vlv: Use power well CTL IDX instead of ID
On Wed, Aug 01, 2018 at 03:15:40PM -0700, Paulo Zanoni wrote: > Em Sex, 2018-07-20 às 17:14 +0300, Imre Deak escreveu: > > Atm, we determine the control/status flag offsets within the PUNIT > > control/status registers based on the power well's ID. Since the > > power > > well ID enum is global across all platforms, the associated macros to > > get the flag offsets involves some magic. This makes checking the > > register/bit definitions against the specification more difficult > > than > > necessary. Also the values in the power well ID enum must stay fixed, > > making code maintenance of the enum cumbersome. > > > > To solve the above define the control/status flag indices right after > > the corresponding registers and use these to derive the > > control/status > > flag values by storing the indices in the i915_power_well_desc > > struct. > > > > Initializing anonymous unions requires - even named - initializers to > > be in order of the struct declaration, hence the reordering of the > > .id > > fields. > > My C-fu is not as strong as I thought it was. After some playing with > this it seems the only requirement is to initialize the enum exactly > after .id. Ok. Hm, right the only requirement is that the field right before the anonymous initializer is explicitly intialized and it's initialized in the order the fields are listed in the struct declaration. The init order of the other fields don't seem to play a role in this. I'll correct this in the commit message. > > But then, since we're reordering anyway, shouldn't we also move .ops > down when relevant, and keep the ordering "perfect" for every member? My thinking was to keep the fields in the initializers in the beginning that need to be provided for all power wells, while keep the anonymous union initializer at the end since it's either power well specific, or doesn't need to be provided at all. So, we could consider to move .ops accordingly in the struct declaration earlier, even saving some space due to alignment. Will think about it as a follow-up. > > Anyway, the patch does what it says, so with or without the new color: > Reviewed-by: Paulo Zanoni > > > > > Cc: Ville Syrjala > > Cc: Paulo Zanoni > > Cc: Jani Nikula > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/i915/i915_drv.h | 7 + > > drivers/gpu/drm/i915/i915_reg.h | 22 ++ > > drivers/gpu/drm/i915/intel_runtime_pm.c | 52 > > - > > 3 files changed, 62 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 3ae200a9e8f1..d31a8ef05d18 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -874,6 +874,13 @@ struct i915_power_well_desc { > > */ > > union { > > struct { > > + /* > > +* request/status flag index in the PUNIT > > power well > > +* control/status registers. > > +*/ > > + u8 idx; > > + } vlv; > > + struct { > > enum dpio_phy phy; > > } bxt; > > struct { > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index 8af945d8a995..f76bb4f3c944 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -1144,11 +1144,23 @@ enum i915_power_well_id { > > > > #define PUNIT_REG_PWRGT_CTRL 0x60 > > #define PUNIT_REG_PWRGT_STATUS 0x61 > > -#define PUNIT_PWRGT_MASK(power_well) (3 << > > ((power_well) * 2)) > > -#define PUNIT_PWRGT_PWR_ON(power_well) (0 << ((power_well) > > * 2)) > > -#define PUNIT_PWRGT_CLK_GATE(power_well) (1 << > > ((power_well) * 2)) > > -#define PUNIT_PWRGT_RESET(power_well)(2 << > > ((power_well) * 2)) > > -#define PUNIT_PWRGT_PWR_GATE(power_well) (3 << > > ((power_well) * 2)) > > +#define PUNIT_PWRGT_MASK(pw_idx) (3 << ((pw_idx) * > > 2)) > > +#define PUNIT_PWRGT_PWR_ON(pw_idx) (0 << ((pw_idx) > > * 2)) > > +#define PUNIT_PWRGT_CLK_GATE(pw_idx) (1 << > > ((pw_idx) * 2)) > > +#define PUNIT_PWRGT_RESET(pw_idx)(2 << ((pw_idx) * > > 2)) > > +#define PUNIT_PWRGT_PWR_GATE(pw_idx) (3 << > > ((pw_idx) * 2)) > > + > > +#define PUNIT_PWGT_IDX_RENDER 0 > > +#define PUNIT_PWGT_IDX_MEDIA 1 > > +#define PUNIT_PWGT_IDX_DISP2D 3 > > +#define PUNIT_PWGT_IDX_DPIO_CMN_BC 5 > > +#define PUNIT_PWGT_IDX_DPIO_TX_B_LANES_01 6 > > +#define PUNIT_PWGT_IDX_DPIO_TX_B_LANES_23 7 > > +#define PUNIT_PWGT_IDX_DPIO_TX_C_LANES_01 8 > > +#define PUNIT_PWGT_IDX_DPIO_TX_C_LANES_23 9 > > +#define PUNIT_PWGT_IDX_DPIO_RX010 > > +#define PUNIT_PWGT_IDX_DPIO_RX111 > > +#define PUNIT_PWG
Re: [Intel-gfx] [PATCH 04/10] drm/i915: Constify power well descriptors
On Wed, Aug 01, 2018 at 02:39:31PM -0700, Paulo Zanoni wrote: > Em Sex, 2018-07-20 às 17:14 +0300, Imre Deak escreveu: > > It makes sense to keep unchanging data const. Extract such fields > > from > > the i915_power_well struct into a new i915_power_well_desc struct > > that > > we initialize during compile time. For the rest of the dynamic > > fields allocate an array of i915_power_well objects in i915 dev_priv, > > and link to each of these objects their corresponding > > i915_power_well_desc object. > > > > Cc: Ville Syrjala > > Cc: Paulo Zanoni > > Cc: Jani Nikula > > Signed-off-by: Imre Deak > > Quite a few issues pointed by checkpatch for this patch, please take a > look at them. > > More below: > > > --- > > drivers/gpu/drm/i915/i915_debugfs.c | 4 +- > > drivers/gpu/drm/i915/i915_drv.c | 8 +- > > drivers/gpu/drm/i915/i915_drv.h | 14 ++- > > drivers/gpu/drm/i915/intel_display.h| 4 +- > > drivers/gpu/drm/i915/intel_drv.h| 1 + > > drivers/gpu/drm/i915/intel_hdcp.c | 6 +- > > drivers/gpu/drm/i915/intel_runtime_pm.c | 204 +++--- > > -- > > 7 files changed, 144 insertions(+), 97 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > b/drivers/gpu/drm/i915/i915_debugfs.c > > index b3aefd623557..eb284cac8fda 100644 > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > @@ -2833,10 +2833,10 @@ static int i915_power_domain_info(struct > > seq_file *m, void *unused) > > enum intel_display_power_domain power_domain; > > > > power_well = &power_domains->power_wells[i]; > > - seq_printf(m, "%-25s %d\n", power_well->name, > > + seq_printf(m, "%-25s %d\n", power_well->desc->name, > >power_well->count); > > > > - for_each_power_domain(power_domain, power_well- > > >domains) > > + for_each_power_domain(power_domain, power_well- > > >desc->domains) > > seq_printf(m, " %-23s %d\n", > > intel_display_power_domain_str(powe > > r_domain), > > power_domains- > > >domain_use_count[power_domain]); > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c > > index 3c984530fef9..5743db4500fb 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -922,7 +922,9 @@ static int i915_driver_init_early(struct > > drm_i915_private *dev_priv, > > intel_uc_init_early(dev_priv); > > intel_pm_setup(dev_priv); > > intel_init_dpio(dev_priv); > > - intel_power_domains_init(dev_priv); > > + ret = intel_power_domains_init(dev_priv); > > + if (ret < 0) > > + goto err_uc; > > intel_irq_init(dev_priv); > > intel_hangcheck_init(dev_priv); > > intel_init_display_hooks(dev_priv); > > @@ -934,6 +936,9 @@ static int i915_driver_init_early(struct > > drm_i915_private *dev_priv, > > > > return 0; > > > > +err_uc: > > + intel_uc_cleanup_early(dev_priv); > > Please leave the guc fixes for a different patch, regardless of how > innocent they look. Well, at least I didn't intend to fix guc. intel_uc_cleanup_early() is already called properly from i915_driver_cleanup_early(), not adding the call here would introduce a new problem if intel_power_domains_init() failed. > > Everything else looks good! > > Thanks, > Paulo > > > + i915_gem_cleanup_early(dev_priv); > > err_workqueues: > > i915_workqueues_cleanup(dev_priv); > > err_engines: > > @@ -948,6 +953,7 @@ static int i915_driver_init_early(struct > > drm_i915_private *dev_priv, > > static void i915_driver_cleanup_early(struct drm_i915_private > > *dev_priv) > > { > > intel_irq_fini(dev_priv); > > + intel_power_domains_cleanup(dev_priv); > > intel_uc_cleanup_early(dev_priv); > > i915_gem_cleanup_early(dev_priv); > > i915_workqueues_cleanup(dev_priv); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 4fb937399440..3ae200a9e8f1 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -862,13 +862,9 @@ struct i915_power_well_ops { > > }; > > > > /* Power well structure for haswell */ > > -struct i915_power_well { > > +struct i915_power_well_desc { > > const char *name; > > bool always_on; > > - /* power well enable/disable usage count */ > > - int count; > > - /* cached hw enabled state */ > > - bool hw_enabled; > > u64 domains; > > /* unique identifier for this power well */ > > enum i915_power_well_id id; > > @@ -891,6 +887,14 @@ struct i915_power_well { > > const struct i915_power_well_ops *ops; > > }; > > > > +struct i915_power_well { > > + const struct i915_power_well_desc *desc; > > + /* power well enable/disable usage count */ > > + int count; > > + /* cached hw enabled state */ > > + bool hw_enabled
Re: [Intel-gfx] [RFC] drm/i915: Don't reset on preemptible workloads
On Thu, 2018-08-02 at 10:14 +0100, Chris Wilson wrote: > Quoting Bartminski, Jakub (2018-08-02 09:56:10) > > On Wed, 2018-08-01 at 15:22 +0100, Chris Wilson wrote: > > > Quoting Jakub Bartmiński (2018-08-01 14:56:11) > > > > [...] > > > Though the argument that as long as its preemptible (and > > > isolated) it > > > is not acting as DoS so we don't need to enforce a timeout. That > > > also > > > suggests that we need not reset until the request is blocking > > > others > > > or userspace. > > > > So if I'm understanding correctly you are suggesting that if we > > have > > preemption enabled we could drop the hangcheck entirely, and just > > reset > > if preemption fails? > > Yes, but we still need the reset if we are stuck at the head of a > chain. I see a potential problem with that approach. If we have a request that is actually hung and introduce a higher-priority request, when trying to preempt the hung request we experience some considerable latency since we need to do the hangcheck at that time, whereas if we were hangchecking regularly it would have been declared hung before that. > [...] > > As I've mentioned before, one other problem with this patch is that > > we > > can't have more than one "background" request working before the > > first > > one finishes. While this could be solved at hangcheck by putting > > the > > background requests in the back of the background-priority queue > > instead of at the front during preemption, this breaks if the > > background requests are dependent on each other. These dependencies > > could be resolved in the tasklet (only in the case hangcheck was > > called > > so it wouldn't slow us down too much, they could also be resolved > > in > > the hangcheck itself but the timeline could change in between the > > hangcheck and the tasklet, which would make doing it correctly more > > complicated I think). > > I think the very simple rule that you reset if there are any > dependencies and demote otherwise covers that. > -Chris So from what I understand, what you meant by that is that you would scratch the opt-in and just demote the priority by some constant on the hangcheck, handle the change in the tasklet, possibly preempting, and if so then wait until the next hangcheck to see if the preempt worked (and if not then reset the engine)? While this is an elegant solution, if we want to allow (do we want to?) multiple "background" requests working interweaved with each other this can still lead to starvation. Imagine the following scenario: we leave a "background" request working for a long enough time that its priority is decreased substantially. If we now introduce a second "background" request with the same starting priority, it will starve the first one for as long as it takes for it to be demoted to the first's current priority. We could avoid demoting if the request is the only one of that priority, but then the same starvation happens if we have two starting requests instead of one, and introduce the third one after some time. I think one way to fix this could be to demote not by a constant, but to a bottom ("background") queue, and then maybe rotate the bottom queue on hangcheck (if that's possible, we should have no dependencies there with what you proposed) to let multiple background requests work interweaved. What is your opinion on that approach? smime.p7s Description: S/MIME cryptographic signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tests/kms_pipe_crc_basic: expect setting bad source to fail
Quoting Maarten Lankhorst (2018-08-02 11:42:32) > Op 02-07-18 om 13:27 schreef Maarten Lankhorst: > > Op 02-07-18 om 13:16 schreef Mahesh Kumar: > >> Now crc-core framework verifies the source string passed by the user. > >> So setting bad-source will fail. Expect file write to fail in bad-source > >> subtest of kms_pipe_crc_basic. > >> > >> Signed-off-by: Mahesh Kumar > >> --- > >> tests/kms_pipe_crc_basic.c | 3 +-- > >> 1 file changed, 1 insertion(+), 2 deletions(-) > >> > >> diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c > >> index 235fdc38..2d4dfee8 100644 > >> --- a/tests/kms_pipe_crc_basic.c > >> +++ b/tests/kms_pipe_crc_basic.c > >> @@ -48,8 +48,7 @@ static struct { > >> > >> static void test_bad_source(data_t *data) > >> { > >> -igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")); > >> -igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1); > >> +igt_assert(!igt_sysfs_set(data->debugfs, "crtc-0/crc/control", > >> "foo")); > >> } > >> > >> #define N_CRCS 3 > > New behavior makes more sense. > > > > Reviewed-by: Maarten Lankhorst > > > > Do you have igt commit rights? > > > Any objections if we change this to allow both behaviors? > > diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c > index 235fdc386ba2..91909fa42f2f 100644 > --- a/tests/kms_pipe_crc_basic.c > +++ b/tests/kms_pipe_crc_basic.c > @@ -48,8 +48,11 @@ static struct { > > static void test_bad_source(data_t *data) > { > - igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")); > - igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1); > + errno = 0; > + if (igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")) > + igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) > == -1); > + else > + igt_assert_eq(errno, EINVAL); Current errno is EIO https://intel-gfx-ci.01.org/tree/drm-tip/IGT_4585/fi-bsw-n3050/igt@kms_pipe_crc_ba...@bad-source.html https://intel-gfx-ci.01.org/tree/drm-tip/IGT_4585/fi-kbl-x1275/igt@kms_pipe_crc_ba...@bad-source.html https://intel-gfx-ci.01.org/tree/drm-tip/IGT_4585/fi-kbl-guc/igt@kms_pipe_crc_ba...@bad-source.html -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for Improve crc-core driver interface (rev9)
== Series Details == Series: Improve crc-core driver interface (rev9) URL : https://patchwork.freedesktop.org/series/45246/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4606 -> Patchwork_9842 = == Summary - FAILURE == Serious unknown changes coming with Patchwork_9842 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_9842, 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/45246/revisions/9/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_9842: === IGT changes === Possible regressions igt@kms_pipe_crc_basic@bad-source: {fi-cfl-8109u}: PASS -> FAIL fi-skl-6770hq: PASS -> FAIL +6 fi-bwr-2160:PASS -> FAIL +4 fi-hsw-4770r: PASS -> FAIL +6 fi-glk-j4005: PASS -> FAIL fi-cfl-8700k: PASS -> FAIL fi-cfl-guc: PASS -> FAIL fi-cnl-psr: PASS -> FAIL fi-kbl-x1275: PASS -> FAIL fi-kbl-guc: PASS -> FAIL fi-glk-dsi: PASS -> FAIL {fi-bsw-kefka}: PASS -> FAIL fi-bsw-n3050: PASS -> FAIL fi-cfl-s3: PASS -> FAIL igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a: fi-ivb-3520m: PASS -> FAIL +6 fi-skl-6700hq: PASS -> FAIL +6 fi-skl-guc: PASS -> FAIL +6 fi-blb-e6850: PASS -> FAIL +4 fi-byt-j1900: PASS -> FAIL +4 igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence: fi-ilk-650: PASS -> FAIL +4 fi-elk-e7500: PASS -> FAIL +4 fi-byt-n2820: PASS -> FAIL +4 fi-snb-2520m: PASS -> FAIL +4 igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b: fi-bdw-5557u: PASS -> FAIL +6 fi-pnv-d510:PASS -> FAIL +4 fi-skl-6600u: PASS -> FAIL +6 fi-bxt-dsi: PASS -> FAIL +6 fi-hsw-4770:PASS -> FAIL +6 fi-ivb-3770:PASS -> FAIL +6 igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence: {fi-bdw-samus}: PASS -> FAIL +6 fi-hsw-peppy: PASS -> FAIL +6 fi-bdw-gvtdvm: PASS -> FAIL +6 fi-gdg-551: PASS -> FAIL +4 fi-kbl-7500u: PASS -> FAIL +6 fi-snb-2600:PASS -> FAIL +4 igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c: fi-kbl-7567u: PASS -> FAIL +6 fi-skl-6260u: PASS -> FAIL +6 fi-skl-6700k2: PASS -> FAIL +6 fi-skl-gvtdvm: PASS -> FAIL +6 {fi-skl-iommu}: PASS -> FAIL +6 fi-bxt-j4205: PASS -> FAIL +6 fi-kbl-7560u: PASS -> FAIL +6 fi-whl-u: PASS -> FAIL +6 fi-kbl-r: PASS -> FAIL +6 == Known issues == Here are the changes found in Patchwork_9842 that come from known issues: === IGT changes === Issues hit igt@drv_selftest@live_coherency: fi-gdg-551: PASS -> DMESG-FAIL (fdo#107164) igt@drv_selftest@live_hangcheck: fi-kbl-guc: PASS -> DMESG-FAIL (fdo#106947) igt@drv_selftest@live_workarounds: {fi-bsw-kefka}: PASS -> DMESG-FAIL (fdo#107292) igt@kms_frontbuffer_tracking@basic: fi-hsw-peppy: PASS -> DMESG-FAIL (fdo#102614) igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a: fi-cfl-s3: PASS -> FAIL (fdo#103481) +5 igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b: {fi-bsw-kefka}: PASS -> FAIL (fdo#106211) +3 {fi-cfl-8109u}: PASS -> FAIL (fdo#103481) +5 fi-cfl-guc: PASS -> FAIL (fdo#103481) +5 igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence: fi-cnl-psr: PASS -> FAIL (fdo#106211) +5 igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c: fi-glk-j4005: PASS -> FAIL (fdo#106211) +5 fi-cfl-8700k: PASS -> FAIL (fdo#103481) +5 fi-bsw-n3050: PASS -> FAIL (fdo#106211) +1 igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c-frame-sequence: fi-glk-dsi: PASS -> FAIL (fdo#106211) +5 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-bxt-dsi: PASS -> INCOMPLETE (fdo#103927) {igt@kms_psr@primary_mmap_gtt}: fi-cnl-psr: PASS -> DMESG-WARN (fdo#107372) Possible fixes igt@drv_selftest@live_workarounds: fi-cnl-psr: DMESG-FAIL (fdo#107292) -> PASS igt@kms_flip@basic-flip-vs-wf_vblank: fi-bsw-n3050: FAIL (fdo#100368) -> PASS Warnings {igt@kms_psr@primary_page_flip}: fi-cnl-psr: DMESG-WARN (fdo#107372) -> DMESG-FAIL (fdo#107372) {name}: This element is suppressed. This means it is ignored when computing the status of the difference (S
[Intel-gfx] ✓ Fi.CI.IGT: success for tests/kms_pipe_crc_basic: expect setting bad source to fail (rev2)
== Series Details == Series: tests/kms_pipe_crc_basic: expect setting bad source to fail (rev2) URL : https://patchwork.freedesktop.org/series/45768/ State : success == Summary == = CI Bug Log - changes from IGT_4582_full -> IGTPW_1677_full = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/45768/revisions/2/mbox/ == Known issues == Here are the changes found in IGTPW_1677_full that come from known issues: === IGT changes === Issues hit igt@drv_suspend@shrink: shard-snb: PASS -> INCOMPLETE (fdo#106886, fdo#105411) igt@gem_sync@basic-store-each: shard-snb: PASS -> INCOMPLETE (fdo#105411) igt@kms_cursor_crc@cursor-256x256-suspend: shard-glk: PASS -> FAIL (fdo#103375) igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic: shard-glk: PASS -> FAIL (fdo#106765) igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic: shard-glk: NOTRUN -> FAIL (fdo#106509) igt@kms_rotation_crc@sprite-rotation-180: shard-hsw: PASS -> FAIL (fdo#103925) igt@kms_setmode@basic: shard-apl: PASS -> FAIL (fdo#99912) shard-kbl: PASS -> FAIL (fdo#99912) igt@perf@polling: shard-hsw: PASS -> FAIL (fdo#102252) Possible fixes igt@gem_userptr_blits@create-destroy-sync: shard-snb: INCOMPLETE (fdo#105411) -> PASS igt@kms_available_modes_crc@available_mode_test_crc: shard-snb: FAIL (fdo#106641) -> PASS igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: shard-kbl: INCOMPLETE (fdo#103665) -> PASS igt@kms_rotation_crc@sprite-rotation-180: shard-snb: FAIL (fdo#103925) -> PASS igt@pm_rpm@drm-resources-equal: shard-kbl: FAIL (fdo#106539) -> PASS shard-hsw: FAIL (fdo#106539) -> PASS shard-glk: FAIL (fdo#106539) -> PASS shard-apl: FAIL (fdo#106539) -> PASS igt@testdisplay: shard-glk: INCOMPLETE (fdo#103359, k.org#198133, fdo#107093) -> PASS fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359 fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509 fdo#106539 https://bugs.freedesktop.org/show_bug.cgi?id=106539 fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641 fdo#106765 https://bugs.freedesktop.org/show_bug.cgi?id=106765 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 fdo#107093 https://bugs.freedesktop.org/show_bug.cgi?id=107093 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * IGT: IGT_4582 -> IGTPW_1677 * Linux: CI_DRM_4600 -> CI_DRM_4605 CI_DRM_4600: 308427119c70d0aaa90433b05969a0317165b122 @ git://anongit.freedesktop.org/gfx-ci/linux CI_DRM_4605: 50098198da758bdd54245d511f4f97236c296c72 @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_1677: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1677/ IGT_4582: 263ca16e4d8909f475d32a28fc0e5972bac214fb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1677/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tests/kms_pipe_crc_basic: expect setting bad source to fail
Op 02-08-18 om 15:03 schreef Chris Wilson: > Quoting Maarten Lankhorst (2018-08-02 11:42:32) >> Op 02-07-18 om 13:27 schreef Maarten Lankhorst: >>> Op 02-07-18 om 13:16 schreef Mahesh Kumar: Now crc-core framework verifies the source string passed by the user. So setting bad-source will fail. Expect file write to fail in bad-source subtest of kms_pipe_crc_basic. Signed-off-by: Mahesh Kumar --- tests/kms_pipe_crc_basic.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c index 235fdc38..2d4dfee8 100644 --- a/tests/kms_pipe_crc_basic.c +++ b/tests/kms_pipe_crc_basic.c @@ -48,8 +48,7 @@ static struct { static void test_bad_source(data_t *data) { -igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")); -igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1); +igt_assert(!igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")); } #define N_CRCS 3 >>> New behavior makes more sense. >>> >>> Reviewed-by: Maarten Lankhorst >>> >>> Do you have igt commit rights? >>> >> Any objections if we change this to allow both behaviors? >> >> diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c >> index 235fdc386ba2..91909fa42f2f 100644 >> --- a/tests/kms_pipe_crc_basic.c >> +++ b/tests/kms_pipe_crc_basic.c >> @@ -48,8 +48,11 @@ static struct { >> >> static void test_bad_source(data_t *data) >> { >> - igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", >> "foo")); >> - igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1); >> + errno = 0; >> + if (igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")) >> + igt_assert(openat(data->debugfs, "crtc-0/crc/data", >> O_WRONLY) == -1); >> + else >> + igt_assert_eq(errno, EINVAL); > Current errno is EIO > https://intel-gfx-ci.01.org/tree/drm-tip/IGT_4585/fi-bsw-n3050/igt@kms_pipe_crc_ba...@bad-source.html > https://intel-gfx-ci.01.org/tree/drm-tip/IGT_4585/fi-kbl-x1275/igt@kms_pipe_crc_ba...@bad-source.html > https://intel-gfx-ci.01.org/tree/drm-tip/IGT_4585/fi-kbl-guc/igt@kms_pipe_crc_ba...@bad-source.html > -Chris Ugh, my bad, messed up. https://patchwork.freedesktop.org/patch/242410/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/lpe: Mark LPE audio as "no callbacks"
The LPE audio is a child device of i915, it is powered up and down alongside the igfx and presents no independent runtime interface. This aptly fulfils the description of a "No-Callback" Device, so mark it thus. Fixes: 183c00350ccd ("drm/i915: Fix runtime PM for LPE audio") Testcase: igt/pm_rpm/basic-pci-d3-state Testcase: igt/pm_rpm/basic-rte Signed-off-by: Chris Wilson Cc: Takashi Iwai Cc: Pierre-Louis Bossart Cc: Ville Syrjälä Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/intel_lpe_audio.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 6269750e2b54..430732720e65 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -126,9 +126,7 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv) return platdev; } - pm_runtime_forbid(&platdev->dev); - pm_runtime_set_active(&platdev->dev); - pm_runtime_enable(&platdev->dev); + pm_runtime_no_callbacks(&platdev->dev); return platdev; } -- 2.18.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Expose the busyspin durations for i915_wait_request
On 28/07/2018 17:46, Chris Wilson wrote: An interesting discussion regarding "hybrid interrupt polling" for NVMe came to the conclusion that the ideal busyspin before sleeping was half of the expected request latency (and better if it was already halfway through that request). This suggested that we too should look again at our tradeoff between spinning and waiting. Currently, our spin simply tries to hide the cost of enabling the interrupt, which is good to avoid penalising nop requests (i.e. test throughput) and not much else. Studying real world workloads suggests that a spin of upto 500us can dramatically boost performance, but the suggestion is that this is not from avoiding interrupt latency per-se, but from secondary effects of sleeping such as allowing the CPU reduce cstate and context switch away. In a truly hybrid interrupt polling scheme, we would aim to sleep until just before the request completed and then wake up in advance of the interrupt and do a quick poll to handle completion. This is tricky for ourselves at the moment as we are not recording request times, and since we allow preemption, our requests are not on as a nicely ordered timeline as IO. However, the idea is interesting, for it will certainly help us decide when busyspinning is worthwhile. v2: Expose the spin setting via Kconfig options for easier adjustment and testing. v3: Don't get caught sneaking in a change to the busyspin parameters. v4: Explain more about the "hybrid interrupt polling" scheme that we want to migrate towards. Suggested-by: Sagar Kamble References: http://events.linuxfoundation.org/sites/events/files/slides/lemoal-nvme-polling-vault-2017-final_0.pdf Signed-off-by: Chris Wilson Cc: Sagar Kamble Cc: Eero Tamminen Cc: Tvrtko Ursulin Cc: Ben Widawsky Cc: Joonas Lahtinen Cc: Michał Winiarski Reviewed-by: Sagar Kamble --- drivers/gpu/drm/i915/Kconfig | 6 + drivers/gpu/drm/i915/Kconfig.profile | 26 +++ drivers/gpu/drm/i915/i915_request.c | 39 +--- 3 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 drivers/gpu/drm/i915/Kconfig.profile diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 5c607f2c707b..387613f29cb0 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -132,3 +132,9 @@ depends on DRM_I915 depends on EXPERT source drivers/gpu/drm/i915/Kconfig.debug endmenu + +menu "drm/i915 Profile Guided Optimisation" Or something like a more generic drm/i915 Manual Tuning Options so it sounds less like an automated thing where you can feed the results of something straight in? + visible if EXPERT + depends on DRM_I915 + source drivers/gpu/drm/i915/Kconfig.profile +endmenu diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile new file mode 100644 index ..8a230eeb98df --- /dev/null +++ b/drivers/gpu/drm/i915/Kconfig.profile @@ -0,0 +1,26 @@ +config DRM_I915_SPIN_REQUEST_IRQ + int + default 5 # microseconds + help + Before sleeping waiting for a request (GPU operation) to complete, + we may spend some time polling for its completion. As the IRQ may + take a non-negligible time to setup, we do a short spin first to + check if the request will complete in the time it would have taken + us to enable the interrupt. + + May be 0 to disable the initial spin. In practice, we estimate + the cost of enabling the interrupt (if currently disabled) to be + a few microseconds. + +config DRM_I915_SPIN_REQUEST_CS + int + default 2 # microseconds + help + After sleeping for a request (GPU operation) to complete, we will + be woken up on the completion of every request prior to the one + being waited on. For very short requests, going back to sleep and + be woken up again may add considerably to the wakeup latency. To + avoid incurring extra latency from the scheduler, we may choose to + spin prior to sleeping again. + + May be 0 to disable spinning after being woken. diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 5c2c93cbab12..ca25c53f8daa 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1336,8 +1336,32 @@ long i915_request_wait(struct i915_request *rq, GEM_BUG_ON(!intel_wait_has_seqno(&wait)); GEM_BUG_ON(!i915_sw_fence_signaled(&rq->submit)); - /* Optimistic short spin before touching IRQs */ - if (__i915_spin_request(rq, wait.seqno, state, 5)) + /* +* Optimistic spin before touching IRQs. +* +* We may use a rather large value here to offset the penalty of +* switching away from the active task. Frequently, the client will +* wait upon an old swapbuffer to throttle itself to remain within a +
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Expose the busyspin durations for i915_wait_request
Quoting Tvrtko Ursulin (2018-08-02 15:15:35) > > On 28/07/2018 17:46, Chris Wilson wrote: > > An interesting discussion regarding "hybrid interrupt polling" for NVMe > > came to the conclusion that the ideal busyspin before sleeping was half > > of the expected request latency (and better if it was already halfway > > through that request). This suggested that we too should look again at > > our tradeoff between spinning and waiting. Currently, our spin simply > > tries to hide the cost of enabling the interrupt, which is good to avoid > > penalising nop requests (i.e. test throughput) and not much else. > > Studying real world workloads suggests that a spin of upto 500us can > > dramatically boost performance, but the suggestion is that this is not > > from avoiding interrupt latency per-se, but from secondary effects of > > sleeping such as allowing the CPU reduce cstate and context switch away. > > > > In a truly hybrid interrupt polling scheme, we would aim to sleep until > > just before the request completed and then wake up in advance of the > > interrupt and do a quick poll to handle completion. This is tricky for > > ourselves at the moment as we are not recording request times, and since > > we allow preemption, our requests are not on as a nicely ordered > > timeline as IO. However, the idea is interesting, for it will certainly > > help us decide when busyspinning is worthwhile. > > > > v2: Expose the spin setting via Kconfig options for easier adjustment > > and testing. > > v3: Don't get caught sneaking in a change to the busyspin parameters. > > v4: Explain more about the "hybrid interrupt polling" scheme that we > > want to migrate towards. > > > > Suggested-by: Sagar Kamble > > References: > > http://events.linuxfoundation.org/sites/events/files/slides/lemoal-nvme-polling-vault-2017-final_0.pdf > > Signed-off-by: Chris Wilson > > Cc: Sagar Kamble > > Cc: Eero Tamminen > > Cc: Tvrtko Ursulin > > Cc: Ben Widawsky > > Cc: Joonas Lahtinen > > Cc: Michał Winiarski > > Reviewed-by: Sagar Kamble > > --- > > drivers/gpu/drm/i915/Kconfig | 6 + > > drivers/gpu/drm/i915/Kconfig.profile | 26 +++ > > drivers/gpu/drm/i915/i915_request.c | 39 +--- > > 3 files changed, 67 insertions(+), 4 deletions(-) > > create mode 100644 drivers/gpu/drm/i915/Kconfig.profile > > > > diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig > > index 5c607f2c707b..387613f29cb0 100644 > > --- a/drivers/gpu/drm/i915/Kconfig > > +++ b/drivers/gpu/drm/i915/Kconfig > > @@ -132,3 +132,9 @@ depends on DRM_I915 > > depends on EXPERT > > source drivers/gpu/drm/i915/Kconfig.debug > > endmenu > > + > > +menu "drm/i915 Profile Guided Optimisation" > > Or something like a more generic drm/i915 Manual Tuning Options so it > sounds less like an automated thing where you can feed the results of > something straight in? Hah, good called. PGO was taken right out of the gcc manual, so yeah it's a bit misleading. > Reviewed-by: Tvrtko Ursulin > > We could add PMU metrics to count hit/missed busy spins so profile > guided bit becomes more direct? Sensible. I wonder though how much we can do already with kprobes? Another one I think I'd like here is the DMA_LATENCY qos parameter. If you haven't seen https://patchwork.freedesktop.org/patch/241571/ that's worth throwing against media-bench. -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: Expose idle delays to Kconfig
On 28/07/2018 17:46, Chris Wilson wrote: We want to expose the parameters for controlling how long it takes for us to notice and park the GPU after a stream of requests in order to try and tune the optimal power-efficiency vs latency of a mostly idle system. Who do we expect to tweak these and what kind of gains have they reported? v2: retire_work has two scheduling points, update them both Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/Kconfig.profile | 23 +++ drivers/gpu/drm/i915/i915_gem.c | 8 +--- drivers/gpu/drm/i915/i915_gem.h | 13 + 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile index 8a230eeb98df..63cb744d920d 100644 --- a/drivers/gpu/drm/i915/Kconfig.profile +++ b/drivers/gpu/drm/i915/Kconfig.profile @@ -24,3 +24,26 @@ config DRM_I915_SPIN_REQUEST_CS spin prior to sleeping again. May be 0 to disable spinning after being woken. + +config DRM_I915_GEM_RETIRE_DELAY + int + default 1000 # milliseconds + help + We maintain a background job whose purpose is to keep cleaning up + after userspace, and may be the first to spot an idle system. This "user space" since this is help text? + parameter determines the interval between execution of the worker. + + To reduce the number of timer wakeups, large delays (of greater than + a second) are rounded to the next walltime second to allow coalescing Wake ups, wall time or with dashes? + of multiple system timers into a single wakeup. Do you perhaps want to omit the bit about rounding to next wall time second and just say it may not be exact, so to reserve some internal freedom? + +config DRM_I915_GEM_PARK_DELAY + int + default 100 # milliseconds + help + Before parking the engines and the GPU after the final request is + retired, we may wait for a small delay to reduce the frequecy of typo in frequency + having to park/unpark and so the latency in executing a new request. + + May be 0 to immediately start parking the engines after the last + request. I'd add so it says "the last request is retired." so there is even more hint that it is not after the request has actually completed but when we went to retire it. diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 460f256114f7..5b538a92631d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -192,7 +192,9 @@ void i915_gem_park(struct drm_i915_private *i915) return; /* Defer the actual call to __i915_gem_park() to prevent ping-pongs */ - mod_delayed_work(i915->wq, &i915->gt.idle_work, msecs_to_jiffies(100)); + mod_delayed_work(i915->wq, +&i915->gt.idle_work, +msecs_to_jiffies(CONFIG_DRM_I915_GEM_PARK_DELAY)); } void i915_gem_unpark(struct drm_i915_private *i915) @@ -236,7 +238,7 @@ void i915_gem_unpark(struct drm_i915_private *i915) queue_delayed_work(i915->wq, &i915->gt.retire_work, - round_jiffies_up_relative(HZ)); + i915_gem_retire_delay()); } int @@ -3466,7 +3468,7 @@ i915_gem_retire_work_handler(struct work_struct *work) if (READ_ONCE(dev_priv->gt.awake)) queue_delayed_work(dev_priv->wq, &dev_priv->gt.retire_work, - round_jiffies_up_relative(HZ)); + i915_gem_retire_delay()); } static void shrink_caches(struct drm_i915_private *i915) diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h index e46592956872..c6b4971435dc 100644 --- a/drivers/gpu/drm/i915/i915_gem.h +++ b/drivers/gpu/drm/i915/i915_gem.h @@ -27,6 +27,8 @@ #include #include +#include +#include struct drm_i915_private; @@ -76,6 +78,17 @@ struct drm_i915_private; void i915_gem_park(struct drm_i915_private *i915); void i915_gem_unpark(struct drm_i915_private *i915); +static inline unsigned long i915_gem_retire_delay(void) +{ + const unsigned long delay = + msecs_to_jiffies(CONFIG_DRM_I915_GEM_RETIRE_DELAY); + + if (CONFIG_DRM_I915_GEM_RETIRE_DELAY >= 1000) + return round_jiffies_up_relative(delay); + else + return delay; +} + static inline void __tasklet_disable_sync_once(struct tasklet_struct *t) { if (atomic_inc_return(&t->count) == 1) Leave it to you to pick or not any of the tidies I suggested. Either way: Reviewed-by: Tvrtko Ursulin Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listin
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/lpe: Mark LPE audio as "no callbacks"
== Series Details == Series: drm/i915/lpe: Mark LPE audio as "no callbacks" URL : https://patchwork.freedesktop.org/series/47620/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4606 -> Patchwork_9843 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/47620/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_9843 that come from known issues: === IGT changes === Issues hit igt@debugfs_test@read_all_entries: {fi-icl-u}: NOTRUN -> DMESG-WARN (fdo#107396) igt@drv_selftest@live_hangcheck: {fi-icl-u}: NOTRUN -> INCOMPLETE (fdo#107399) igt@kms_pipe_crc_basic@read-crc-pipe-b: {fi-byt-clapper}: PASS -> FAIL (fdo#107362) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: {fi-icl-u}: NOTRUN -> DMESG-WARN (fdo#107382) +4 {igt@kms_psr@primary_mmap_gtt}: fi-cnl-psr: PASS -> DMESG-WARN (fdo#107372) {igt@kms_psr@primary_page_flip}: {fi-icl-u}: NOTRUN -> FAIL (fdo#107383) +3 Possible fixes igt@drv_selftest@live_workarounds: fi-cnl-psr: DMESG-FAIL (fdo#107292) -> PASS igt@kms_chamelium@dp-edid-read: fi-kbl-7500u: FAIL (fdo#103841) -> PASS igt@kms_flip@basic-flip-vs-wf_vblank: fi-bsw-n3050: FAIL (fdo#100368) -> PASS igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence: {fi-byt-clapper}: FAIL (fdo#103191, fdo#107362) -> PASS igt@pm_rpm@basic-rte: {fi-byt-clapper}: FAIL (fdo#107357) -> PASS +1 {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#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841 fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292 fdo#107357 https://bugs.freedesktop.org/show_bug.cgi?id=107357 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372 fdo#107382 https://bugs.freedesktop.org/show_bug.cgi?id=107382 fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383 fdo#107396 https://bugs.freedesktop.org/show_bug.cgi?id=107396 fdo#107399 https://bugs.freedesktop.org/show_bug.cgi?id=107399 == Participating hosts (51 -> 47) == Additional (1): fi-icl-u Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * Linux: CI_DRM_4606 -> Patchwork_9843 CI_DRM_4606: 603c0696f3c56d3f1e47c283de897448473c9041 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4584: 33f47ff4d64bd3996995dc5493deee26294e3aa3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9843: dc9bcff3a83520c5b739937f4891af4264463d5c @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == dc9bcff3a835 drm/i915/lpe: Mark LPE audio as "no callbacks" == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9843/issues.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: Expose idle delays to Kconfig
Quoting Tvrtko Ursulin (2018-08-02 15:33:33) > > On 28/07/2018 17:46, Chris Wilson wrote: > > We want to expose the parameters for controlling how long it takes for > > us to notice and park the GPU after a stream of requests in order to try > > and tune the optimal power-efficiency vs latency of a mostly idle system. > > Who do we expect to tweak these and what kind of gains have they reported? These are more targeted at S0ix. Which is a can of worms that I am surprised hasn't been opened yet. Could it be that it all works better than expected? Or more likely they just have been talking to us. So these are definitely more optimistically opportunistic than practical. > > v2: retire_work has two scheduling points, update them both > > > > Signed-off-by: Chris Wilson > > Cc: Tvrtko Ursulin > > --- > > drivers/gpu/drm/i915/Kconfig.profile | 23 +++ > > drivers/gpu/drm/i915/i915_gem.c | 8 +--- > > drivers/gpu/drm/i915/i915_gem.h | 13 + > > 3 files changed, 41 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/Kconfig.profile > > b/drivers/gpu/drm/i915/Kconfig.profile > > index 8a230eeb98df..63cb744d920d 100644 > > --- a/drivers/gpu/drm/i915/Kconfig.profile > > +++ b/drivers/gpu/drm/i915/Kconfig.profile > > @@ -24,3 +24,26 @@ config DRM_I915_SPIN_REQUEST_CS > > spin prior to sleeping again. > > > > May be 0 to disable spinning after being woken. > > + > > +config DRM_I915_GEM_RETIRE_DELAY > > + int > > + default 1000 # milliseconds > > + help > > + We maintain a background job whose purpose is to keep cleaning up > > + after userspace, and may be the first to spot an idle system. This > > "user space" since this is help text? > > > + parameter determines the interval between execution of the worker. > > + > > + To reduce the number of timer wakeups, large delays (of greater than > > + a second) are rounded to the next walltime second to allow > > coalescing > > Wake ups, wall time or with dashes? > > > + of multiple system timers into a single wakeup. > > Do you perhaps want to omit the bit about rounding to next wall time > second and just say it may not be exact, so to reserve some internal > freedom? Sure, but it's help text, even less accurate than comments ;) I think it's prudent to say that it is inexact. > > > + > > +config DRM_I915_GEM_PARK_DELAY > > + int > > + default 100 # milliseconds > > + help > > + Before parking the engines and the GPU after the final request is > > + retired, we may wait for a small delay to reduce the frequecy of > > typo in frequency > > > + having to park/unpark and so the latency in executing a new request. > > + > > + May be 0 to immediately start parking the engines after the last > > + request. > > I'd add so it says "the last request is retired." so there is even more > hint that it is not after the request has actually completed but when we > went to retire it. Ok. -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: Increase busyspin limit before a context-switch
On 28/07/2018 17:46, Chris Wilson wrote: Looking at the distribution of i915_wait_request for a set of GL What was the set? benchmarks, we see: broadwell# python bcc/tools/funclatency.py -u i915_wait_request usecs : count distribution 0 -> 1 : 29184|| 2 -> 3 : 5767 |*** | 4 -> 7 : 3000 || 8 -> 15 : 491 || 16 -> 31 : 140 || 32 -> 63 : 203 || 64 -> 127: 543 || 128 -> 255: 881 |* | 256 -> 511: 1209 |* | 512 -> 1023 : 1739 |** | 1024 -> 2047 : 22855|*** | 2048 -> 4095 : 1725 |** | 4096 -> 8191 : 5813 |*** | 8192 -> 16383 : 5348 |*** | 16384 -> 32767 : 1000 |* | 32768 -> 65535 : 4400 |** | 65536 -> 131071 : 296 || 131072 -> 262143 : 225 || 262144 -> 524287 : 4|| 524288 -> 1048575: 1|| 1048576 -> 2097151: 1|| 2097152 -> 4194303: 1|| broxton# python bcc/tools/funclatency.py -u i915_wait_request usecs : count distribution 0 -> 1 : 5523 |* | 2 -> 3 : 1340 |* | 4 -> 7 : 2100 |** | 8 -> 15 : 755 |* | 16 -> 31 : 211 |* | 32 -> 63 : 53 || 64 -> 127: 71 || 128 -> 255: 113 || 256 -> 511: 262 |* | 512 -> 1023 : 358 |** | 1024 -> 2047 : 1105 |*** | 2048 -> 4095 : 848 |* | 4096 -> 8191 : 1295 || 8192 -> 16383 : 5894 || 16384 -> 32767 : 4270 || 32768 -> 65535 : 5622 |** | 65536 -> 131071 : 306 |** | 131072 -> 262143 : 50 || 262144 -> 524287 : 76 || 524288 -> 1048575: 34 || 1048576 -> 2097151: 0|| 2097152 -> 4194303: 1|| Picking 20us for the context-switch busyspin has the dual advantage of catching most frequent short waits while avoiding the cost of a context switch. 20us is a typical latency of 2 context-switches, i.e. the cost of taking the sleep, without the secondary effects of cache flushing. Signed-off-by: Chris Wilson Cc: Sagar Kamble Cc: Eero Tamminen Cc: Tvrtko Ursulin Cc: Ben Widawsky Cc: Joonas Lahtinen Cc: Michał Winiarski --- drivers/gpu/drm/i915/Kconfig.profile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile index 63cb744d920d..de394dea4a14 100644 --- a/drivers/gpu/drm/i915/Kconfig.profile +++ b/drivers/gpu/drm/i915/Kconfig.profile @@ -14,7 +14,7 @@ config DRM_I915_SPIN_REQUEST_IRQ config DRM_I915_SPIN_REQUEST_CS int - default 2 # microseconds + default 20 # microseconds help After sleeping for a request (GPU operation) to complete, we will be woken up on the completion of every request prior to the one I'd be more tempted to pick 10us given the histograms. It would avoid wasting cycles on Broadwell and keep
Re: [Intel-gfx] [PATCH 3/5] drm/i915: Increase busyspin limit before a context-switch
On 02/08/2018 15:40, Tvrtko Ursulin wrote: On 28/07/2018 17:46, Chris Wilson wrote: Looking at the distribution of i915_wait_request for a set of GL What was the set? benchmarks, we see: broadwell# python bcc/tools/funclatency.py -u i915_wait_request usecs : count distribution 0 -> 1 : 29184 || 2 -> 3 : 5767 |*** | 4 -> 7 : 3000 | | 8 -> 15 : 491 | | 16 -> 31 : 140 | | 32 -> 63 : 203 | | 64 -> 127 : 543 | | 128 -> 255 : 881 |* | 256 -> 511 : 1209 |* | 512 -> 1023 : 1739 |** | 1024 -> 2047 : 22855 |*** | 2048 -> 4095 : 1725 |** | 4096 -> 8191 : 5813 |*** | 8192 -> 16383 : 5348 |*** | 16384 -> 32767 : 1000 |* | 32768 -> 65535 : 4400 |** | 65536 -> 131071 : 296 | | 131072 -> 262143 : 225 | | 262144 -> 524287 : 4 | | 524288 -> 1048575 : 1 | | 1048576 -> 2097151 : 1 | | 2097152 -> 4194303 : 1 | | broxton# python bcc/tools/funclatency.py -u i915_wait_request usecs : count distribution 0 -> 1 : 5523 |* | 2 -> 3 : 1340 |* | 4 -> 7 : 2100 |** | 8 -> 15 : 755 |* | 16 -> 31 : 211 |* | 32 -> 63 : 53 | | 64 -> 127 : 71 | | 128 -> 255 : 113 | | 256 -> 511 : 262 |* | 512 -> 1023 : 358 |** | 1024 -> 2047 : 1105 |*** | 2048 -> 4095 : 848 |* | 4096 -> 8191 : 1295 | | 8192 -> 16383 : 5894 || 16384 -> 32767 : 4270 | | 32768 -> 65535 : 5622 |** | 65536 -> 131071 : 306 |** | 131072 -> 262143 : 50 | | 262144 -> 524287 : 76 | | 524288 -> 1048575 : 34 | | 1048576 -> 2097151 : 0 | | 2097152 -> 4194303 : 1 | | Picking 20us for the context-switch busyspin has the dual advantage of catching most frequent short waits while avoiding the cost of a context switch. 20us is a typical latency of 2 context-switches, i.e. the cost of taking the sleep, without the secondary effects of cache flushing. Signed-off-by: Chris Wilson Cc: Sagar Kamble Cc: Eero Tamminen Cc: Tvrtko Ursulin Cc: Ben Widawsky Cc: Joonas Lahtinen Cc: Michał Winiarski --- drivers/gpu/drm/i915/Kconfig.profile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile index 63cb744d920d..de394dea4a14 100644 --- a/drivers/gpu/drm/i915/Kconfig.profile +++ b/drivers/gpu/drm/i915/Kconfig.profile @@ -14,7 +14,7 @@ config DRM_I915_SPIN_REQUEST_IRQ config DRM_I915_SPIN_REQUEST_CS int - default 2 # microseconds + default 20 # microseconds help After sleeping for a request (GPU operation) to complete, we will be woken up on the completion of every request prior to the one I'd be more tempted to pick 10us g
Re: [Intel-gfx] [PATCH 3/5] drm/i915: Increase busyspin limit before a context-switch
Quoting Tvrtko Ursulin (2018-08-02 15:40:27) > > On 28/07/2018 17:46, Chris Wilson wrote: > > Looking at the distribution of i915_wait_request for a set of GL > > What was the set? > > > benchmarks, we see: > > > > broadwell# python bcc/tools/funclatency.py -u i915_wait_request > > usecs : count distribution > > 0 -> 1 : 29184 > > || > > 2 -> 3 : 5767 |*** > > | > > 4 -> 7 : 3000 | > > | > > 8 -> 15 : 491 | > > | > >16 -> 31 : 140 | > > | > >32 -> 63 : 203 | > > | > >64 -> 127: 543 | > > | > > 128 -> 255: 881 |* > > | > > 256 -> 511: 1209 |* > > | > > 512 -> 1023 : 1739 |** > > | > > 1024 -> 2047 : 22855|*** > > | > > 2048 -> 4095 : 1725 |** > > | > > 4096 -> 8191 : 5813 |*** > > | > > 8192 -> 16383 : 5348 |*** > > | > > 16384 -> 32767 : 1000 |* > > | > > 32768 -> 65535 : 4400 |** > > | > > 65536 -> 131071 : 296 | > > | > >131072 -> 262143 : 225 | > > | > >262144 -> 524287 : 4| > > | > >524288 -> 1048575: 1| > > | > > 1048576 -> 2097151: 1| > > | > > 2097152 -> 4194303: 1| > > | > > > > broxton# python bcc/tools/funclatency.py -u i915_wait_request > > usecs : count distribution > > 0 -> 1 : 5523 |* > > | > > 2 -> 3 : 1340 |* > > | > > 4 -> 7 : 2100 |** > > | > > 8 -> 15 : 755 |* > > | > >16 -> 31 : 211 |* > > | > >32 -> 63 : 53 | > > | > >64 -> 127: 71 | > > | > > 128 -> 255: 113 | > > | > > 256 -> 511: 262 |* > > | > > 512 -> 1023 : 358 |** > > | > > 1024 -> 2047 : 1105 |*** > > | > > 2048 -> 4095 : 848 |* > > | > > 4096 -> 8191 : 1295 | > > | > > 8192 -> 16383 : 5894 > > || > > 16384 -> 32767 : 4270 | > > | > > 32768 -> 65535 : 5622 |** > > | > > 65536 -> 131071 : 306 |** > > | > >131072 -> 262143 : 50 | > > | > >262144 -> 524287 : 76 | > > | > >524288 -> 1048575: 34 | > > | > > 1048576 -> 2097151: 0| > > | > > 2097152 -> 4194303: 1| > > | > > > > Picking 20us for the context-switch busyspin has the dual advantage of > > catching most frequent short waits while avoiding the cost of a context > > switch. 20us is a typical latency of 2 context-switches, i.e. the cost > > of taking the sleep, without the secondary effects of cache flushing. > > > > Signed-off-by: Chris Wilson > > Cc: Sagar Kamble > > Cc: Eero Tamminen > > Cc: Tvrtko Ursulin > > Cc: Ben Widawsky > > Cc: Joonas Lahtinen > > Cc: Michał Winiarski > > --- > > drivers/gpu/drm/i915/Kconfig.profile | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/Kconfig.profile > > b/drivers/gpu/drm/i915/Kconfig.profile > > index 63c
Re: [Intel-gfx] [PATCH 4/5] drm/i915: Increase initial busyspin limit
On 28/07/2018 17:46, Chris Wilson wrote: Here we bump the busyspin for the current request to avoid sleeping and the cost of both idling and downclocking the CPU. Signed-off-by: Chris Wilson Cc: Sagar Kamble Cc: Eero Tamminen Cc: Francisco Jerez Cc: Tvrtko Ursulin Cc: Ben Widawsky Cc: Joonas Lahtinen Cc: Michał Winiarski --- drivers/gpu/drm/i915/Kconfig.profile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile index de394dea4a14..54e0ba4051e7 100644 --- a/drivers/gpu/drm/i915/Kconfig.profile +++ b/drivers/gpu/drm/i915/Kconfig.profile @@ -1,6 +1,6 @@ config DRM_I915_SPIN_REQUEST_IRQ int - default 5 # microseconds + default 250 # microseconds help Before sleeping waiting for a request (GPU operation) to complete, we may spend some time polling for its completion. As the IRQ may But the histograms from previous patch do not suggest 250us busy spin gets us into interesting territory. And we have to know the distribution between IRQ and CS spins. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/lpe: Mark LPE audio as "no callbacks"
Quoting Patchwork (2018-08-02 15:38:35) > Possible fixes > > igt@pm_rpm@basic-rte: > {fi-byt-clapper}: FAIL (fdo#107357) -> PASS +1 \o/ -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/5] drm/i915: Increase initial busyspin limit
Quoting Tvrtko Ursulin (2018-08-02 15:47:37) > > On 28/07/2018 17:46, Chris Wilson wrote: > > Here we bump the busyspin for the current request to avoid sleeping and > > the cost of both idling and downclocking the CPU. > > > > Signed-off-by: Chris Wilson > > Cc: Sagar Kamble > > Cc: Eero Tamminen > > Cc: Francisco Jerez > > Cc: Tvrtko Ursulin > > Cc: Ben Widawsky > > Cc: Joonas Lahtinen > > Cc: Michał Winiarski > > --- > > drivers/gpu/drm/i915/Kconfig.profile | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/Kconfig.profile > > b/drivers/gpu/drm/i915/Kconfig.profile > > index de394dea4a14..54e0ba4051e7 100644 > > --- a/drivers/gpu/drm/i915/Kconfig.profile > > +++ b/drivers/gpu/drm/i915/Kconfig.profile > > @@ -1,6 +1,6 @@ > > config DRM_I915_SPIN_REQUEST_IRQ > > int > > - default 5 # microseconds > > + default 250 # microseconds > > help > > Before sleeping waiting for a request (GPU operation) to complete, > > we may spend some time polling for its completion. As the IRQ may > > > > But the histograms from previous patch do not suggest 250us busy spin > gets us into interesting territory. And we have to know the distribution > between IRQ and CS spins. This is for a different problem than presented in those histograms. This is to hide the reclocking that occurred if we sleep. Previous justification for the initial spin was to try and hide the irq setup costs, this is to try and hide the sleep costs. The purpose of this patch was just to dip the toe into the water of what is possible and solicit feedback since we are still waiting for the panacea patches to improve cpufreq. (Why it had such a lackluster justification.) -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Do not use iowait while waiting for the GPU
On 28/07/2018 17:46, Chris Wilson wrote: A recent trend for cpufreq is to boost the CPU frequencies for iowaiters, in particularly to benefit high frequency I/O. We do the same and boost the GPU clocks to try and minimise time spent waiting for the GPU. However, as the igfx and CPU share the same TDP, boosting the CPU frequency will result in the GPU being throttled and its frequency being reduced. Thus declaring iowait negatively impacts on GPU throughput. I see it has been discussed in another thread. My view here is that I do not know. But fiddling with this on our side does feel questionable. If IO wait is waiting for a device then it is correct. If it interacts badly with cpufreq in specifics of shared TDP then I feel it should be solved elsewhere (cpufreq). I don't have a lot of ideas how though. GPU usage has been tried by someone in the past but I don't know how that went. Regards, Tvrtko v2: Both sleeps! Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107410 References: 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup") Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Joonas Lahtinen Cc: Eero Tamminen Cc: Francisco Jerez --- drivers/gpu/drm/i915/i915_request.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index ca25c53f8daa..694269e5b761 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1330,7 +1330,7 @@ long i915_request_wait(struct i915_request *rq, goto complete; } - timeout = io_schedule_timeout(timeout); + timeout = schedule_timeout(timeout); } while (1); GEM_BUG_ON(!intel_wait_has_seqno(&wait)); @@ -1387,7 +1387,7 @@ long i915_request_wait(struct i915_request *rq, break; } - timeout = io_schedule_timeout(timeout); + timeout = schedule_timeout(timeout); if (intel_wait_complete(&wait) && intel_wait_check_request(&wait, rq)) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCHv3] lib/ratelimit: Lockless ratelimiting
Hi Steven, Thanks for your reply, On Wed, 2018-08-01 at 21:48 -0400, Steven Rostedt wrote: > I'm just catching up from my vacation. What about making rs->missed > into an atomic, and have: > > if (!raw_spin_trylock_irqsave(&rs->lock, flags)) { > atomic_inc(&rs->missed); > return 0; > } > > ? Uhm. Do you mean as a preparation patch to split this on two patches? Because it will not solve the issue where one CPU has taken rs->lock, and is updating rs->printed, checking burst and whatnot; while the second CPU will loose the message which was even *under* burst limit. I.e.: there are enough of printk_ratelimit() users in tree and a message from one can be suppressed, while shouldn't. > You would also need to do: > > if (time_is_before_jiffies(rs->begin + rs->interval)) { > int missed = atomic_xchg(&rs->missed, 0); > if (missed) { > > So that you don't have a race between checking rs->missed and setting > it > to zero. -- Thanks, Dmitry ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915: Drop stray clearing of rps->last_adj
Chris Wilson writes: > We used to reset last_adj to 0 on crossing a power domain boundary, to > slow down our rate of change. However, commit 60548c554be2 ("drm/i915: > Interactive RPS mode") accidentally caused it to be reset on every > frequency update, nerfing the fast response granted by the slow start > algorithm. > > Fixes: 60548c554be2 ("drm/i915: Interactive RPS mode") > Testcase: igt/pm_rps/mix-max-config-loaded > Signed-off-by: Chris Wilson > Cc: Joonas Lahtinen Reviewed-by: Mika Kuoppala > --- > drivers/gpu/drm/i915/intel_pm.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 2531eb75bdce..f90a3c7f1c40 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -6371,7 +6371,6 @@ static void gen6_set_rps_thresholds(struct > drm_i915_private *dev_priv, u8 val) > new_power = HIGH_POWER; > rps_set_power(dev_priv, new_power); > mutex_unlock(&rps->power.mutex); > - rps->last_adj = 0; > } > > void intel_rps_mark_interactive(struct drm_i915_private *i915, bool > interactive) > -- > 2.18.0 > > ___ > 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] ✗ Fi.CI.IGT: failure for drm/i915/lpe: Mark LPE audio as "no callbacks"
== Series Details == Series: drm/i915/lpe: Mark LPE audio as "no callbacks" URL : https://patchwork.freedesktop.org/series/47620/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4606_full -> Patchwork_9843_full = == Summary - FAILURE == Serious unknown changes coming with Patchwork_9843_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_9843_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_9843_full: === IGT changes === Possible regressions igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt: shard-snb: NOTRUN -> FAIL == Known issues == Here are the changes found in Patchwork_9843_full that come from known issues: === IGT changes === Issues hit igt@gem_mmap_gtt@basic-wc: shard-snb: NOTRUN -> INCOMPLETE (fdo#105411) Possible fixes igt@perf_pmu@busy-vcs0: shard-snb: INCOMPLETE (fdo#105411) -> SKIP Warnings igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-wc: shard-snb: DMESG-WARN -> DMESG-FAIL (fdo#103167) fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4606 -> Patchwork_9843 CI_DRM_4606: 603c0696f3c56d3f1e47c283de897448473c9041 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4584: 33f47ff4d64bd3996995dc5493deee26294e3aa3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9843: dc9bcff3a83520c5b739937f4891af4264463d5c @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9843/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v9 7/7] drm/i915: add a sysfs entry to let users set sseu configs
On Thu, 2018-08-02 at 11:00 +0100, Tvrtko Ursulin wrote: > [Picking this point in the thread to reply on some points mentioned > by > folks in the whole thread.] > > I don't remember if any patches from Lionel's series actually had r- > b's, > but a few people including myself have certainly been reviewing them. > If > I had applied the final r-b I wouldn't have made much difference due > lack of userspace and disagreement on the DoS mitigation story. So > to > say effectively put your money where your mouth is and review is not > entirely fair. > > Suggestion to add a master sysfs switch was to alleviate the DoS > concerns because dynamic switching has a cost towards all GPU > clients, > not that it just has potential to slow down media. > > Suggestion was also that this switch becomes immutable and defaults > to > "allow" on Gen11 onwards. > > I preferred sysfs versus a modparam since it makes testing (Both for > developers and users (what config works better for my use case?) > easier.) > > I am fine with the suggestion to drive the Gen11 part first, which > removes the need for any of this. > > Patch series is already (AFAIR) nicely split so only the last patch > needs to be dropped. > > If at some point we decide to revisit the Gen8/9 story, we can > evaluate/measure whether any master switch is actually warranted. I > think Lionel did some micro-benchmarking which showed impact is not > so > severe, so perhaps for real-world use cases it would be even less. > > I can re-read the public patches and review them, or perhaps even > adopt > them if they have been orphaned. Have to check with Francesco first > before I commit to the latter. Thank you for volunteering:). I talked with Lionel about that and he thinks he may be able to do a respin next week. So, please, check with him also to avoid double work. > > On the uAPI front interface looked fine to me. > > One recent interesting development are Mesa patches posted to mesa- > dev > (https://lists.freedesktop.org/archives/mesa-dev/2018-July/200607.htm > l) > which add EGL_CONTEXT_load_type extension (low/medium/high). This > would > need some sort of mapping between low/medium/high to more specific > settings but still seems okay to me. > > This may bring another (open source?) user for the patches. Which > Gen's > they are interested in is also a question. > > Regards, > > Tvrtko > > On 24/07/2018 22:50, Bloomfield, Jon wrote: > > Gratuitous top posting to re-kick the thread. > > > > For Gen11 we can't have an on/off switch anyway (media simply won't > > run > > with an oncompatible slice config), so let's agree on an api to > > allow userland > > to select the slice configuration for its contexts, for Gen11 only. > > I'd prefer a > > simple {MediaConfig/GeneralConfig} type context param but I really > > don't > > care that much. > > > > For gen9/10 it's arguable whether we need this at all. The effect > > on media > > workloads varies, but I'm guessing normal users (outside a > > transcode > > type environment) will never know they're missing anything. Either > > way, > > we can continue discussing while we progress the gen11 solution. > > > > Jon > > > > > -Original Message- > > > From: Intel-gfx On > > > Behalf Of > > > Bloomfield, Jon > > > Sent: Wednesday, July 18, 2018 9:44 AM > > > To: Tvrtko Ursulin ; Joonas > > > Lahtinen > > > ; Chris Wilson > > on.co.uk>; > > > Landwerlin, Lionel G ; intel- > > > g...@lists.freedesktop.org > > > Subject: Re: [Intel-gfx] [PATCH v9 7/7] drm/i915: add a sysfs > > > entry to let users > > > set sseu configs > > > > > > > -Original Message- > > > > From: Intel-gfx On > > > > Behalf Of > > > > Tvrtko Ursulin > > > > Sent: Thursday, June 14, 2018 1:29 AM > > > > To: Joonas Lahtinen ; Chris > > > > Wilson > > > > ; Landwerlin, Lionel G > > > > ; intel-...@lists.freedesktop.or > > > > g > > > > Subject: Re: [Intel-gfx] [PATCH v9 7/7] drm/i915: add a sysfs > > > > entry to let > > > > > > users > > > > set sseu configs > > > > > > > > > > > > On 13/06/2018 13:49, Joonas Lahtinen wrote: > > > > > Quoting Tvrtko Ursulin (2018-06-12 15:02:07) > > > > > > > > > > > > On 12/06/2018 11:52, Lionel Landwerlin wrote: > > > > > > > On 12/06/18 11:37, Chris Wilson wrote: > > > > > > > > Quoting Lionel Landwerlin (2018-06-12 11:33:34) > > > > > > > > > On 12/06/18 10:20, Joonas Lahtinen wrote: > > > > > > > > > > Quoting Chris Wilson (2018-06-11 18:02:37) > > > > > > > > > > > Quoting Lionel Landwerlin (2018-06-11 14:46:07) > > > > > > > > > > > > On 11/06/18 13:10, Tvrtko Ursulin wrote: > > > > > > > > > > > > > On 30/05/2018 15:33, Lionel Landwerlin wrote: > > > > > > > > > > > > > > There are concerns about denial of service > > > > > > > > > > > > > > around the per > > > > > > > > > > > > > > context sseu > > > > > > > > > > > > > > configuration capability. In a previous > > > > > > > > > > > > > > commit introducing the > > > > > > > > > > > > > > capabilit
Re: [Intel-gfx] [PATCH 4/5] drm/i915: Increase initial busyspin limit
On Thu, 2018-08-02 at 15:54 +0100, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2018-08-02 15:47:37) > > > > On 28/07/2018 17:46, Chris Wilson wrote: > > > Here we bump the busyspin for the current request to avoid > > > sleeping and > > > the cost of both idling and downclocking the CPU. > > > > > > Signed-off-by: Chris Wilson > > > Cc: Sagar Kamble > > > Cc: Eero Tamminen > > > Cc: Francisco Jerez > > > Cc: Tvrtko Ursulin > > > Cc: Ben Widawsky > > > Cc: Joonas Lahtinen > > > Cc: Michał Winiarski > > > --- > > > drivers/gpu/drm/i915/Kconfig.profile | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/Kconfig.profile > > > b/drivers/gpu/drm/i915/Kconfig.profile > > > index de394dea4a14..54e0ba4051e7 100644 > > > --- a/drivers/gpu/drm/i915/Kconfig.profile > > > +++ b/drivers/gpu/drm/i915/Kconfig.profile > > > @@ -1,6 +1,6 @@ > > > config DRM_I915_SPIN_REQUEST_IRQ > > > int > > > - default 5 # microseconds > > > + default 250 # microseconds > > > help > > > Before sleeping waiting for a request (GPU operation) to > > > complete, > > > we may spend some time polling for its completion. As the > > > IRQ may > > > > > > > But the histograms from previous patch do not suggest 250us busy > > spin > > gets us into interesting territory. And we have to know the > > distribution > > between IRQ and CS spins. > > This is for a different problem than presented in those histograms. > This > is to hide the reclocking that occurred if we sleep. > > Previous justification for the initial spin was to try and hide the > irq > setup costs, this is to try and hide the sleep costs. > > The purpose of this patch was just to dip the toe into the water of > what > is possible and solicit feedback since we are still waiting for the > panacea patches to improve cpufreq. (Why it had such a lackluster > justification.) This patch somewhat resembles with the issue we just met in userspace for the opencl and media. Long story short there was CPU% regression between 2 opencl releases root caused to the following. OpenCL rewrote the code how they wait for the task completion. Essentially they have an optimistic spin implemented on the userspace side and logic their is quite complex: they commit to different duration of the spin depending on the scenario. And they had some pretty long spin, like 10ms. This is a killer thing if opencl is coupled with media workloads, especially when media is dominant. That's because usual execution time of media batches is pretty long, few or dozen milliseconds is quite typical. As a result opencl workload is really being submitted and executed with significant delay, thus optimistic spin gives up and it comes with the CPU% cost. This story was discussed here: https://github.com/intel/comp ute-runtime/commit/d53e1c3979f6d822f55645bfcd753be1658f53ca#comments. I was actually surprised why they don't rely on i915 wait function. They responded in was: "I would gladly always use bo_wait ioctl, but it has big overhead for short workloads." Thus the question: do we have an opportunity to improve? Can we have some dynamic way to configure spinning for the particular requests/contexts? /if I understand correctly current patch it introduces kernel config-level setting rather than some dynamic setting/ > -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
Re: [Intel-gfx] [PATCH 4/5] drm/i915: Increase initial busyspin limit
Quoting Rogozhkin, Dmitry V (2018-08-02 20:37:36) > I was actually surprised why they don't rely on i915 wait function. > They responded in was: "I would gladly always use bo_wait ioctl, but it > has big overhead for short workloads." Thus the question: do we have an > opportunity to improve? Can we have some dynamic way to configure > spinning for the particular requests/contexts? /if I understand > correctly current patch it introduces kernel config-level setting > rather than some dynamic setting/ Better yet, tell us about the problem with the overhead! The latency from batch to client (measuring RCS timestamp in batch to RCS timestamp in userspace) is 10us (bdw and the like), under controlled conditions ;) Hmm, that does drop a bit for a RT client which does also suggest that cpufreq is a factor there. A roundtrip through the wait ioctl doing nothing is 100ns; minimum wait due to irq setup is around 7us (spin then mmio before detecting completion immediately afterwards). If your measurements are orders of magnitude worse that too is enlightening. To justify a 10ms spin is that sleeping (hitting the scheduler) is unacceptable, which seems to be a widerscale problem. But that I recommend we look at https://patchwork.freedesktop.org/patch/241571/ to see if something like that helps us in the short term. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915: Drop stray clearing of rps->last_adj
Quoting Mika Kuoppala (2018-08-02 16:30:24) > Chris Wilson writes: > > > We used to reset last_adj to 0 on crossing a power domain boundary, to > > slow down our rate of change. However, commit 60548c554be2 ("drm/i915: > > Interactive RPS mode") accidentally caused it to be reset on every > > frequency update, nerfing the fast response granted by the slow start > > algorithm. > > > > Fixes: 60548c554be2 ("drm/i915: Interactive RPS mode") > > Testcase: igt/pm_rps/mix-max-config-loaded > > Signed-off-by: Chris Wilson > > Cc: Joonas Lahtinen > > Reviewed-by: Mika Kuoppala Plonked this one as fixes the regression. 2&3 are nice cleanups, 4 seems like an unlikely boundary case, but I suspect kodi might be a good example. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/10] drm/i915/ddi: Use power well CTL IDX instead of ID
Em Sex, 2018-07-20 às 17:15 +0300, Imre Deak escreveu: > Similarly to the previous patch use a separate request/status HW flag > index defined right after the corresponding control registers instead > of > depending for this on the power well IDs. Since the set of > control/status registers varies among the different power wells (on a > single platform), also add a new i915_power_well_registers struct > that > we populate and assign to each DDI power well as needed. > > Also clarify a bit the code comment describing the function and > layout > of the control registers. > > This also fixes a problem on ICL, where we incorrectly read the KVMR > control register in hsw_power_well_requesters() even for DDI and AUX > power wells. > > Cc: Ville Syrjala > Cc: Paulo Zanoni > Cc: Jani Nikula > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/gvt/handlers.c | 30 +--- > drivers/gpu/drm/i915/i915_drv.h | 13 ++ > drivers/gpu/drm/i915/i915_reg.h | 126 - > drivers/gpu/drm/i915/intel_display.c| 5 +- > drivers/gpu/drm/i915/intel_runtime_pm.c | 302 > ++-- > 5 files changed, 359 insertions(+), 117 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c > b/drivers/gpu/drm/i915/gvt/handlers.c > index 7a58ca555197..79e748569d31 100644 > --- a/drivers/gpu/drm/i915/gvt/handlers.c > +++ b/drivers/gpu/drm/i915/gvt/handlers.c > @@ -1287,12 +1287,12 @@ static int power_well_ctl_mmio_write(struct > intel_vgpu *vgpu, > { > write_vreg(vgpu, offset, p_data, bytes); > > - if (vgpu_vreg(vgpu, offset) & > HSW_PWR_WELL_CTL_REQ(HSW_DISP_PW_GLOBAL)) > + if (vgpu_vreg(vgpu, offset) & > HSW_PWR_WELL_CTL_REQ(HSW_PW_CTL_IDX_GLOBAL)) > vgpu_vreg(vgpu, offset) |= > - HSW_PWR_WELL_CTL_STATE(HSW_DISP_PW_GLOBAL); > + HSW_PWR_WELL_CTL_STATE(HSW_PW_CTL_IDX_GLOBAL > ); > else > vgpu_vreg(vgpu, offset) &= > - ~HSW_PWR_WELL_CTL_STATE(HSW_DISP_PW_GLOBAL); > + ~HSW_PWR_WELL_CTL_STATE(HSW_PW_CTL_IDX_GLOBA > L); > return 0; > } > > @@ -2443,17 +2443,10 @@ static int init_generic_mmio_info(struct > intel_gvt *gvt) > MMIO_D(GEN6_RC6p_THRESHOLD, D_ALL); > MMIO_D(GEN6_RC6pp_THRESHOLD, D_ALL); > MMIO_D(GEN6_PMINTRMSK, D_ALL); > - /* > - * Use an arbitrary power well controlled by the > PWR_WELL_CTL > - * register. > - */ > - MMIO_DH(HSW_PWR_WELL_CTL_BIOS(HSW_DISP_PW_GLOBAL), D_BDW, > NULL, > - power_well_ctl_mmio_write); > - MMIO_DH(HSW_PWR_WELL_CTL_DRIVER(HSW_DISP_PW_GLOBAL), D_BDW, > NULL, > - power_well_ctl_mmio_write); > - MMIO_DH(HSW_PWR_WELL_CTL_KVMR, D_BDW, NULL, > power_well_ctl_mmio_write); > - MMIO_DH(HSW_PWR_WELL_CTL_DEBUG(HSW_DISP_PW_GLOBAL), D_BDW, > NULL, > - power_well_ctl_mmio_write); > + MMIO_DH(HSW_PWR_WELL_CTL1, D_BDW, NULL, > power_well_ctl_mmio_write); > + MMIO_DH(HSW_PWR_WELL_CTL2, D_BDW, NULL, > power_well_ctl_mmio_write); > + MMIO_DH(HSW_PWR_WELL_CTL3, D_BDW, NULL, > power_well_ctl_mmio_write); > + MMIO_DH(HSW_PWR_WELL_CTL4, D_BDW, NULL, > power_well_ctl_mmio_write); > MMIO_DH(HSW_PWR_WELL_CTL5, D_BDW, NULL, > power_well_ctl_mmio_write); > MMIO_DH(HSW_PWR_WELL_CTL6, D_BDW, NULL, > power_well_ctl_mmio_write); > > @@ -2804,13 +2797,8 @@ static int init_skl_mmio_info(struct intel_gvt > *gvt) > MMIO_F(_MMIO(_DPD_AUX_CH_CTL), 6 * 4, 0, 0, 0, D_SKL_PLUS, > NULL, > dp_aux_ch_ctl_mmio_w > rite); > > - /* > - * Use an arbitrary power well controlled by the > PWR_WELL_CTL > - * register. > - */ > - MMIO_D(HSW_PWR_WELL_CTL_BIOS(SKL_DISP_PW_MISC_IO), > D_SKL_PLUS); > - MMIO_DH(HSW_PWR_WELL_CTL_DRIVER(SKL_DISP_PW_MISC_IO), > D_SKL_PLUS, NULL, > - skl_power_well_ctl_write); > + MMIO_D(HSW_PWR_WELL_CTL1, D_SKL_PLUS); > + MMIO_DH(HSW_PWR_WELL_CTL2, D_SKL_PLUS, NULL, > skl_power_well_ctl_write); > > MMIO_D(_MMIO(0xa210), D_SKL_PLUS); > MMIO_D(GEN9_MEDIA_PG_IDLE_HYSTERESIS, D_SKL_PLUS); > diff --git a/drivers/gpu/drm/i915/i915_drv.h > b/drivers/gpu/drm/i915/i915_drv.h > index d31a8ef05d18..d73ce0a7b8f7 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -861,6 +861,13 @@ struct i915_power_well_ops { > struct i915_power_well *power_well); > }; > > +struct i915_power_well_regs { > + i915_reg_t bios; > + i915_reg_t driver; > + i915_reg_t kvmr; > + i915_reg_t debug; > +}; > + > /* Power well structure for haswell */ > struct i915_power_well_desc { > const char *name; > @@ -884,6 +891,12 @@ struct i915_power_well_desc { > enum dpio_phy phy; > } bxt; > struct { > + const struct i915_power_well_regs *regs; > + /* > +
Re: [Intel-gfx] [PATCH] firmware/dmc/icl: load v1.07 on icelake.
>-Original Message- >From: Sarvela, Tomi P >Sent: Thursday, August 2, 2018 12:51 AM >To: Vivi, Rodrigo ; Zanoni, Paulo R > >Cc: Srivatsa, Anusha ; intel- >g...@lists.freedesktop.org; Martin Peres >Subject: Re: [Intel-gfx] [PATCH] firmware/dmc/icl: load v1.07 on icelake. > >On 08/02/2018 10:15 AM, Tomi Sarvela wrote: >> On 08/02/2018 08:11 AM, Rodrigo Vivi wrote: >>> On Wed, Aug 01, 2018 at 05:30:49PM -0700, Paulo Zanoni wrote: Em Qua, 2018-08-01 às 17:07 -0700, Anusha Srivatsa escreveu: > Add Support to load DMC on Icelake. > > While at it, also add support to load the firmware during system > resume. > > v2: load firmware during system resume.(Imre) Just to make it clear: did we test this on actual machines before submitting or are we entirely relying on the CI results? I'm not sure the CI is running enough tests to validate this patch with confidence, we'll probably need to do some manual testing here. >>> >>> At some point I believe it was agreed that CI would test this and get >>> the new firmware automatically from the cover-letter. >>> >>> The problem is that I don't see any cover-letter so I'm afraid it is >>> not running with the new firmware. >>> >>> Tomi? >> >> The requests can be checked from patchwork REST API: >> >> https://patchwork.freedesktop.org/api/1.0/projects/intel-gfx/events/?p >> age=1&name=pull-request-new >> >> >> There has been a pull request for new firmware, but it couldn't be >> acted. CI can't connect to SSH repository, because those generally >> need an account. This has been tried and tested before. Better way is >> to use git://, http:// or https:// URLs, as the pull shouldn't need >> any special permissions. >> >> This time I have manually pulled >> git://anongit.freedesktop.org/drm/drm-firmware/master but, if you want >> to make this automatic, the pull requests shouldn't be from >> repositories with obligatory login. >> >> Another note: known firmware pull repositories are now >> >> freedesktop.org git://anongit.freedesktop.org/drm/drm-firmware >> (fetch) g_anushasr git://github.com/anushasr/linux-firmware.git >> (fetch) h_anushasr https://github.com/anushasr/linux-firmware.git >> (fetch) kernel.org >> git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware >> (fetch) >> >> If you know that in future there might be pull requests from another >> repository, please inform me about that in advance. > >The results from the re-tested patchset are back, with dmesgs: > >https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9839/fi-icl-u/boot0.log > >https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9839/fi-icl-u/dmesg0.log > >Testrun hung in igt@drv_selftest@live_hangcheck, which is standard place for >ICL >to rest in peace. > >https://intel-gfx-ci.01.org/tree/drm-tip/fi-icl-u.html > >Tomi > > > >> >>> >>> Also I believe in case it has the cover letter it should run the full >>> CI on the machine or at least stash it and run on the weekend or >>> whenever we run the full on all machines and then report back again. >>> Possible? >>> Martin? For Firmware patches, it would be nice to have the whole IGT running with the patch ofcourse. Martin, Tomi is this possible? Anusha > > Cc: Imre Deak > Cc: Rodrigo Vivi > Cc: Paulo Zanoni > Signed-off-by: Anusha Srivatsa > --- > drivers/gpu/drm/i915/intel_csr.c | 7 +++ > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++ > 2 files changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_csr.c > b/drivers/gpu/drm/i915/intel_csr.c > index cf9b600..393d419 100644 > --- a/drivers/gpu/drm/i915/intel_csr.c > +++ b/drivers/gpu/drm/i915/intel_csr.c > @@ -34,6 +34,9 @@ > * low-power state and comes back to normal. > */ > +#define I915_CSR_ICL "i915/icl_dmc_ver1_07.bin" > +#define ICL_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) > + > #define I915_CSR_GLK "i915/glk_dmc_ver1_04.bin" > MODULE_FIRMWARE(I915_CSR_GLK); > #define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 4) @@ -301,6 > +304,8 @@ static uint32_t *parse_csr_fw(struct drm_i915_private > *dev_priv, > if (csr->fw_path == i915_modparams.dmc_firmware_path) { > /* Bypass version check for firmware override. */ > required_version = csr->version; > + } else if (IS_ICELAKE(dev_priv)) { > + required_version = ICL_CSR_VERSION_REQUIRED; > } else if (IS_CANNONLAKE(dev_priv)) { > required_version = CNL_CSR_VERSION_REQUIRED; > } else if (IS_GEMINILAKE(dev_priv)) { @@ -458,6 +463,8 @@ > void intel_csr_ucode_init(struct drm_i915_private > *dev_priv) > if (i915_modparams.dmc_firmware_path) > csr->fw_path = i915_modparams.dmc_firmware_path; > + else if (IS_ICELAKE(dev_priv)) > + csr->fw_path = I915_CSR_ICL; > else if (IS_C
Re: [Intel-gfx] [PATCH 07/10] drm/i915: Remove redundant power well IDs
Em Sex, 2018-07-20 às 17:15 +0300, Imre Deak escreveu: > Now that we removed dependence on the power well IDs to determine the > control register and request/status flag offsets the only purpose of > power well IDs is to look up power wells directly bypassing the power > domains framework. However this direct lookup isn't needed for most > of > the exisiting power wells and hopefully won't be needed for any new > power wells in the future. To make maintenance of the power well ID > enum > easier, don't require a unique ID for each power well, only if it's > necessary. Remove the IDs becoming redundant this way and assign to > all > the corresponding power wells a new DISP_PW_ID_NONE ID. > > After the previous two patches the IDs don't need to have a fixed > value, > so remove the explicit initializers and adjust the enum's code > comment > accordingly. I would probably have kept every enum, but let's proceed with your colors. More below: > > Cc: Ville Syrjala > Cc: Paulo Zanoni > Cc: Jani Nikula > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/i915_reg.h | 118 > - > drivers/gpu/drm/i915/intel_runtime_pm.c | 129 > > 2 files changed, 79 insertions(+), 168 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > index b7022fb8d524..9b3635009826 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1029,117 +1029,25 @@ static inline bool > i915_mmio_reg_valid(i915_reg_t reg) > /* > * i915_power_well_id: > * > - * Platform specific IDs used to look up power wells and - except > for custom > - * power wells - to define request/status register flag bit > positions. As such > - * the set of IDs on a given platform must be unique and except for > custom > - * power wells their value must stay fixed. > + * IDs used to look up power wells. Power wells accessed directly > bypassing > + * the power domains framework must be assigned a unique ID. The > rest of power > + * wells must be assigned DISP_PW_ID_NONE. > */ > enum i915_power_well_id { > - /* > - * I830 > - * - custom power well > - */ > - I830_DISP_PW_PIPES = 0, > - > - /* > - * VLV/CHV > - * - PUNIT_REG_PWRGT_CTRL (bit: id*2), > - *PUNIT_REG_PWRGT_STATUS (bit: id*2) (PUNIT HAS v0.8) > - */ > - PUNIT_POWER_WELL_RENDER = 0, > - PUNIT_POWER_WELL_MEDIA = 1, > - PUNIT_POWER_WELL_DISP2D = 3, > - PUNIT_POWER_WELL_DPIO_CMN_BC= 5, > - PUNIT_POWER_WELL_DPIO_TX_B_LANES_01 = 6, > - PUNIT_POWER_WELL_DPIO_TX_B_LANES_23 = 7, > - PUNIT_POWER_WELL_DPIO_TX_C_LANES_01 = 8, > - PUNIT_POWER_WELL_DPIO_TX_C_LANES_23 = 9, > - PUNIT_POWER_WELL_DPIO_RX0 = 10, > - PUNIT_POWER_WELL_DPIO_RX1 = 11, > - PUNIT_POWER_WELL_DPIO_CMN_D = 12, > - /* - custom power well */ > - CHV_DISP_PW_PIPE_A, /* 13 */ > - > - /* > - * HSW/BDW > - * - _HSW_PWR_WELL_CTL1-4 (status bit: id*2, req bit: > id*2+1) > - */ > - HSW_DISP_PW_GLOBAL = 15, > - > - /* > - * GEN9+ > - * - _HSW_PWR_WELL_CTL1-4 (status bit: id*2, req bit: > id*2+1) > - */ > - SKL_DISP_PW_MISC_IO = 0, > - SKL_DISP_PW_DDI_A_E, > - GLK_DISP_PW_DDI_A = SKL_DISP_PW_DDI_A_E, > - CNL_DISP_PW_DDI_A = SKL_DISP_PW_DDI_A_E, > - SKL_DISP_PW_DDI_B, > - SKL_DISP_PW_DDI_C, > - SKL_DISP_PW_DDI_D, > - CNL_DISP_PW_DDI_F = 6, > - > - GLK_DISP_PW_AUX_A = 8, > - GLK_DISP_PW_AUX_B, > - GLK_DISP_PW_AUX_C, > - CNL_DISP_PW_AUX_A = GLK_DISP_PW_AUX_A, > - CNL_DISP_PW_AUX_B = GLK_DISP_PW_AUX_B, > - CNL_DISP_PW_AUX_C = GLK_DISP_PW_AUX_C, > - CNL_DISP_PW_AUX_D, > - CNL_DISP_PW_AUX_F, > - > - SKL_DISP_PW_1 = 14, > + DISP_PW_ID_NONE, > + > + PUNIT_POWER_WELL_DISP2D, > + PUNIT_POWER_WELL_DPIO_CMN_BC, > + PUNIT_POWER_WELL_DPIO_CMN_D, > + HSW_DISP_PW_GLOBAL, Looking up this one (from intel_hdcp.c) will fail to find any power wells because they got changed to DISP_PW_ID_NONE. > + SKL_DISP_PW_MISC_IO, > + SKL_DISP_PW_1, > SKL_DISP_PW_2, > - > - /* - custom power wells */ > BXT_DPIO_CMN_A, > BXT_DPIO_CMN_BC, > - GLK_DPIO_CMN_C, /* 18 */ > - > - /* > - * GEN11+ > - * - _HSW_PWR_WELL_CTL1-4 > - *(status bit: (id&15)*2, req bit:(id&15)*2+1) > - */ > - ICL_DISP_PW_1 = 0, > + GLK_DPIO_CMN_C, > + ICL_DISP_PW_1, > ICL_DISP_PW_2, Either we kill ICL_DISP_PW_2 or we don't switch its power wells to DISP_PW_ID_NONE. I suppose killing ICL_DISP_PW_2 is the direction we're moving to. > - ICL_DISP_PW_3, > - ICL_DISP_PW_4, > - > - /* > - * - _HSW_PWR_WELL_CTL_AUX1/2/4 > - *(status bit: (id&15)*2, req bit:(id&15)*2+1) > -
Re: [Intel-gfx] [PATCH 08/10] drm/i915: Make power well ID names more uniform
Em Sex, 2018-07-20 às 17:15 +0300, Imre Deak escreveu: > The format for the ID names is _DISP_PW_* so rename the IDs > not following this accordingly. Leave BXT_DPIO_CMN_BC as-is since > we'll > change that to use another existing ID in the next patch. > > Cc: Ville Syrjala > Cc: Paulo Zanoni Reviewed-by: Paulo Zanoni > Cc: Jani Nikula > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/i915_reg.h | 10 > drivers/gpu/drm/i915/intel_runtime_pm.c | 44 --- > -- > 2 files changed, 27 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > index 9b3635009826..b6076f712db5 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1036,16 +1036,16 @@ static inline bool > i915_mmio_reg_valid(i915_reg_t reg) > enum i915_power_well_id { > DISP_PW_ID_NONE, > > - PUNIT_POWER_WELL_DISP2D, > - PUNIT_POWER_WELL_DPIO_CMN_BC, > - PUNIT_POWER_WELL_DPIO_CMN_D, > + VLV_DISP_PW_DISP2D, > + BXT_DISP_PW_DPIO_CMN_A, > + VLV_DISP_PW_DPIO_CMN_BC, > + GLK_DISP_PW_DPIO_CMN_C, > + CHV_DISP_PW_DPIO_CMN_D, > HSW_DISP_PW_GLOBAL, > SKL_DISP_PW_MISC_IO, > SKL_DISP_PW_1, > SKL_DISP_PW_2, > - BXT_DPIO_CMN_A, > BXT_DPIO_CMN_BC, > - GLK_DPIO_CMN_C, > ICL_DISP_PW_1, > ICL_DISP_PW_2, > }; > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 792394d20f62..56161d0dc3ca 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -768,7 +768,7 @@ static void bxt_verify_ddi_phy_power_wells(struct > drm_i915_private *dev_priv) > { > struct i915_power_well *power_well; > > - power_well = lookup_power_well(dev_priv, BXT_DPIO_CMN_A); > + power_well = lookup_power_well(dev_priv, > BXT_DISP_PW_DPIO_CMN_A); > if (power_well->count > 0) > bxt_ddi_phy_verify_state(dev_priv, power_well->desc- > >bxt.phy); > > @@ -777,7 +777,7 @@ static void bxt_verify_ddi_phy_power_wells(struct > drm_i915_private *dev_priv) > bxt_ddi_phy_verify_state(dev_priv, power_well->desc- > >bxt.phy); > > if (IS_GEMINILAKE(dev_priv)) { > - power_well = lookup_power_well(dev_priv, > GLK_DPIO_CMN_C); > + power_well = lookup_power_well(dev_priv, > GLK_DISP_PW_DPIO_CMN_C); > if (power_well->count > 0) > bxt_ddi_phy_verify_state(dev_priv, >power_well->desc- > >bxt.phy); > @@ -1129,9 +1129,9 @@ lookup_power_well(struct drm_i915_private > *dev_priv, > static void assert_chv_phy_status(struct drm_i915_private *dev_priv) > { > struct i915_power_well *cmn_bc = > - lookup_power_well(dev_priv, > PUNIT_POWER_WELL_DPIO_CMN_BC); > + lookup_power_well(dev_priv, > VLV_DISP_PW_DPIO_CMN_BC); > struct i915_power_well *cmn_d = > - lookup_power_well(dev_priv, > PUNIT_POWER_WELL_DPIO_CMN_D); > + lookup_power_well(dev_priv, CHV_DISP_PW_DPIO_CMN_D); > u32 phy_control = dev_priv->chv_phy_control; > u32 phy_status = 0; > u32 phy_status_mask = 0x; > @@ -1241,10 +1241,10 @@ static void > chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv, > enum pipe pipe; > uint32_t tmp; > > - WARN_ON_ONCE(power_well->desc->id != > PUNIT_POWER_WELL_DPIO_CMN_BC && > - power_well->desc->id != > PUNIT_POWER_WELL_DPIO_CMN_D); > + WARN_ON_ONCE(power_well->desc->id != VLV_DISP_PW_DPIO_CMN_BC > && > + power_well->desc->id != > CHV_DISP_PW_DPIO_CMN_D); > > - if (power_well->desc->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { > + if (power_well->desc->id == VLV_DISP_PW_DPIO_CMN_BC) { > pipe = PIPE_A; > phy = DPIO_PHY0; > } else { > @@ -1272,7 +1272,7 @@ static void > chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv, > DPIO_SUS_CLK_CONFIG_GATE_CLKREQ; > vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW28, tmp); > > - if (power_well->desc->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { > + if (power_well->desc->id == VLV_DISP_PW_DPIO_CMN_BC) { > tmp = vlv_dpio_read(dev_priv, pipe, > _CHV_CMN_DW6_CH1); > tmp |= DPIO_DYNPWRDOWNEN_CH1; > vlv_dpio_write(dev_priv, pipe, _CHV_CMN_DW6_CH1, > tmp); > @@ -1303,10 +1303,10 @@ static void > chv_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv, > { > enum dpio_phy phy; > > - WARN_ON_ONCE(power_well->desc->id != > PUNIT_POWER_WELL_DPIO_CMN_BC && > - power_well->desc->id != > PUNIT_POWER_WELL_DPIO_CMN_D); > + WARN_ON_ONCE(power_well->desc->id != VLV_DISP_PW_DPIO_CMN_BC > && > + power_well->desc->id != > CHV_DISP_PW_DPIO_CMN_D); > > - if (power_well->desc->id == PUNIT
Re: [Intel-gfx] [PATCH 09/10] drm/i915: Use existing power well IDs where possible
Em Sex, 2018-07-20 às 17:15 +0300, Imre Deak escreveu: > There is no need for separate IDs for power wells on a new platform > with > the same functionality as an other power well on a previous platform, > we > can just reuse the ID from the previous platform. This is only > possible > after the previous patches where we removed dependence on the actual > enum values. > > Cc: Ville Syrjala > Cc: Paulo Zanoni > Cc: Jani Nikula > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/i915_reg.h | 3 --- > drivers/gpu/drm/i915/intel_runtime_pm.c | 12 ++-- > 2 files changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > index b6076f712db5..19b4eac1cc8a 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1045,9 +1045,6 @@ enum i915_power_well_id { > SKL_DISP_PW_MISC_IO, > SKL_DISP_PW_1, > SKL_DISP_PW_2, > - BXT_DPIO_CMN_BC, > - ICL_DISP_PW_1, > - ICL_DISP_PW_2, I mentioned on patch 7 about killing ICL_DISP_PW_2. Here instead of reusing another ID for it (as the commit title implies) you just kill it :). Please do it on patch 7 for better organization. With that: Reviewed-by: Paulo Zanoni > }; > > #define PUNIT_REG_PWRGT_CTRL 0x60 > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 56161d0dc3ca..b7acf54d8a72 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -772,7 +772,7 @@ static void bxt_verify_ddi_phy_power_wells(struct > drm_i915_private *dev_priv) > if (power_well->count > 0) > bxt_ddi_phy_verify_state(dev_priv, power_well->desc- > >bxt.phy); > > - power_well = lookup_power_well(dev_priv, BXT_DPIO_CMN_BC); > + power_well = lookup_power_well(dev_priv, > VLV_DISP_PW_DPIO_CMN_BC); > if (power_well->count > 0) > bxt_ddi_phy_verify_state(dev_priv, power_well->desc- > >bxt.phy); > > @@ -2456,7 +2456,7 @@ static const struct i915_power_well_desc > bxt_power_wells[] = { > .name = "dpio-common-bc", > .domains = BXT_DPIO_CMN_BC_POWER_DOMAINS, > .ops = &bxt_dpio_cmn_power_well_ops, > - .id = BXT_DPIO_CMN_BC, > + .id = VLV_DISP_PW_DPIO_CMN_BC, > { > .bxt.phy = DPIO_PHY0, > }, > @@ -2515,7 +2515,7 @@ static const struct i915_power_well_desc > glk_power_wells[] = { > .name = "dpio-common-b", > .domains = GLK_DPIO_CMN_B_POWER_DOMAINS, > .ops = &bxt_dpio_cmn_power_well_ops, > - .id = BXT_DPIO_CMN_BC, > + .id = VLV_DISP_PW_DPIO_CMN_BC, > { > .bxt.phy = DPIO_PHY0, > }, > @@ -2764,7 +2764,7 @@ static const struct i915_power_well_desc > icl_power_wells[] = { > /* Handled by the DMC firmware */ > .domains = 0, > .ops = &hsw_power_well_ops, > - .id = ICL_DISP_PW_1, > + .id = SKL_DISP_PW_1, > { > .hsw.regs = &hsw_power_well_regs, > .hsw.idx = ICL_PW_CTL_IDX_PW_1, > @@ -3584,7 +3584,7 @@ static void icl_display_core_init(struct > drm_i915_private *dev_priv, >*The AUX IO power wells will be enabled on demand. >*/ > mutex_lock(&power_domains->lock); > - well = lookup_power_well(dev_priv, ICL_DISP_PW_1); > + well = lookup_power_well(dev_priv, SKL_DISP_PW_1); > intel_power_well_enable(dev_priv, well); > mutex_unlock(&power_domains->lock); > > @@ -3625,7 +3625,7 @@ static void icl_display_core_uninit(struct > drm_i915_private *dev_priv) >*disabled at this point. >*/ > mutex_lock(&power_domains->lock); > - well = lookup_power_well(dev_priv, ICL_DISP_PW_1); > + well = lookup_power_well(dev_priv, SKL_DISP_PW_1); > intel_power_well_disable(dev_priv, well); > mutex_unlock(&power_domains->lock); > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/10] drm/i915/icl: Add missing power gate enums
Em Sex, 2018-07-20 às 17:15 +0300, Imre Deak escreveu: > On ICL there are 5 fused power gates, so add the two missing ones for > clarity. > > Cc: Ville Syrjala > Cc: Paulo Zanoni Reviewed-by: Paulo Zanoni > Cc: Jani Nikula > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/i915_reg.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > index 19b4eac1cc8a..7b6fba25614e 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -8830,6 +8830,8 @@ enum skl_power_gate { > SKL_PG0, > SKL_PG1, > SKL_PG2, > + ICL_PG3, > + ICL_PG4, > }; > > #define SKL_FUSE_STATUS _MMIO(0x42000 > ) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] docs: fix typo sharding->sharing
I was grepping for shard as the tests run on CI, but the only occurrence was this one which seems to be a typo since it's about prime tests. Fixes: 76bce773 ("docs: Update documentation generation with missing entries") Signed-off-by: Lucas De Marchi --- docs/reference/igt-gpu-tools/igt_test_programs.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/igt-gpu-tools/igt_test_programs.xml b/docs/reference/igt-gpu-tools/igt_test_programs.xml index ec05d53e..95c4653e 100644 --- a/docs/reference/igt-gpu-tools/igt_test_programs.xml +++ b/docs/reference/igt-gpu-tools/igt_test_programs.xml @@ -229,7 +229,7 @@ Prime Tests - Tests for buffer sharding + Tests for buffer sharing -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] firmware/dmc/icl: load v1.07 on icelake.
>-Original Message- >From: Yadav, Jyoti R >Sent: Wednesday, August 1, 2018 10:34 PM >To: Srivatsa, Anusha ; intel- >g...@lists.freedesktop.org; Zanoni, Paulo R >Cc: Sarvela, Tomi P ; Vivi, Rodrigo >; Zanoni, Paulo R ; intel- >g...@lists.freedesktop.org >Subject: Re: [Intel-gfx] [PATCH] firmware/dmc/icl: load v1.07 on icelake. > >Hi Anusha, > >I think we should also add "HAS_CSR" capability, which is being exercised >inside >intel_csr_ucode_init() path. Yes you are right :) Thanks for pointing it. >For ICL, inside intel_device_info structure we should add has_csr = 1, >otherwise >below check will fail and function will return from there itself. > >if (!HAS_CSR(dev_priv)) > return; That is what is happening according to the logs. Rodrigo, Paulo : The has_csr = 1 should be a separate patch though. This patch should just try and load the blob and has_csr patch has to enable csr on ICL. Anusha >Regards >Jyoti >On 8/2/2018 10:41 AM, Rodrigo Vivi wrote: >> On Wed, Aug 01, 2018 at 05:30:49PM -0700, Paulo Zanoni wrote: >>> Em Qua, 2018-08-01 às 17:07 -0700, Anusha Srivatsa escreveu: Add Support to load DMC on Icelake. While at it, also add support to load the firmware during system resume. v2: load firmware during system resume.(Imre) >>> Just to make it clear: did we test this on actual machines before >>> submitting or are we entirely relying on the CI results? >>> >>> I'm not sure the CI is running enough tests to validate this patch >>> with confidence, we'll probably need to do some manual testing here. >> At some point I believe it was agreed that CI would test this and get >> the new firmware automatically from the cover-letter. >> >> The problem is that I don't see any cover-letter so I'm afraid it is >> not running with the new firmware. >> >> Tomi? >> >> Also I believe in case it has the cover letter it should run the full >> CI on the machine or at least stash it and run on the weekend or >> whenever we run the full on all machines and then report back again. >> Possible? >> >> Martin? >> Cc: Imre Deak Cc: Rodrigo Vivi Cc: Paulo Zanoni Signed-off-by: Anusha Srivatsa --- drivers/gpu/drm/i915/intel_csr.c| 7 +++ drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++ 2 files changed, 10 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index cf9b600..393d419 100644 --- a/drivers/gpu/drm/i915/intel_csr.c +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -34,6 +34,9 @@ * low-power state and comes back to normal. */ +#define I915_CSR_ICL "i915/icl_dmc_ver1_07.bin" +#define ICL_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) + #define I915_CSR_GLK "i915/glk_dmc_ver1_04.bin" MODULE_FIRMWARE(I915_CSR_GLK); #define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 4) @@ -301,6 +304,8 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, if (csr->fw_path == i915_modparams.dmc_firmware_path) { /* Bypass version check for firmware override. */ required_version = csr->version; + } else if (IS_ICELAKE(dev_priv)) { + required_version = ICL_CSR_VERSION_REQUIRED; } else if (IS_CANNONLAKE(dev_priv)) { required_version = CNL_CSR_VERSION_REQUIRED; } else if (IS_GEMINILAKE(dev_priv)) { @@ -458,6 +463,8 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) if (i915_modparams.dmc_firmware_path) csr->fw_path = i915_modparams.dmc_firmware_path; + else if (IS_ICELAKE(dev_priv)) + csr->fw_path = I915_CSR_ICL; else if (IS_CANNONLAKE(dev_priv)) csr->fw_path = I915_CSR_CNL; else if (IS_GEMINILAKE(dev_priv)) diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index cf89141..77c0986 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -3372,6 +3372,9 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv, /* 7. Setup MBUS. */ icl_mbus_init(dev_priv); + + if (resume && dev_priv->csr.dmc_payload) + intel_csr_load_program(dev_priv); } static void icl_display_core_uninit(struct drm_i915_private *dev_priv) >>> ___ >>> 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 mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinf
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [v3,1/2] drm/i915/icl: move has_resource_streamer to GEN11_FEATURES
== Series Details == Series: series starting with [v3,1/2] drm/i915/icl: move has_resource_streamer to GEN11_FEATURES URL : https://patchwork.freedesktop.org/series/46884/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4608 -> Patchwork_9844 = == Summary - FAILURE == Serious unknown changes coming with Patchwork_9844 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_9844, 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/46884/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_9844: === IGT changes === Possible regressions igt@drv_module_reload@basic-reload-inject: fi-bxt-j4205: PASS -> DMESG-WARN == Known issues == Here are the changes found in Patchwork_9844 that come from known issues: === IGT changes === Issues hit igt@drv_selftest@live_hangcheck: fi-skl-guc: PASS -> DMESG-FAIL (fdo#107174) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-bxt-dsi: PASS -> INCOMPLETE (fdo#103927) Possible fixes igt@drv_selftest@live_hangcheck: fi-bdw-5557u: DMESG-FAIL (fdo#106560) -> PASS igt@drv_selftest@live_workarounds: {fi-cfl-8109u}: DMESG-FAIL (fdo#107292) -> PASS {igt@kms_psr@primary_mmap_gtt}: fi-cnl-psr: DMESG-WARN (fdo#107372) -> PASS {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560 fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174 fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292 fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372 == Participating hosts (52 -> 46) == Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-u == Build changes == * Linux: CI_DRM_4608 -> Patchwork_9844 CI_DRM_4608: 9d129b43738d0b604a787e54e041973ac7a7c922 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4586: 57caaf440520e397403d898e1d3f1d65ef7b79e2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9844: 07debe9ce5e89b7446cc2c5ef66c9f34154fbca5 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 07debe9ce5e8 drm/i915: kill resource streamer 47aa5c7d064f drm/i915/icl: move has_resource_streamer to GEN11_FEATURES == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9844/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for docs: fix typo sharding->sharing
== Series Details == Series: docs: fix typo sharding->sharing URL : https://patchwork.freedesktop.org/series/47632/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4606 -> IGTPW_1679 = == Summary - WARNING == Minor unknown changes coming with IGTPW_1679 need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in IGTPW_1679, 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/47632/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in IGTPW_1679: === IGT changes === Warnings igt@kms_pipe_crc_basic@bad-source: fi-kbl-x1275: PASS -> SKIP fi-kbl-guc: PASS -> SKIP fi-bsw-n3050: PASS -> SKIP == Known issues == Here are the changes found in IGTPW_1679 that come from known issues: === IGT changes === Issues hit igt@drv_selftest@live_hangcheck: fi-skl-guc: PASS -> DMESG-FAIL (fdo#107174) fi-glk-dsi: PASS -> DMESG-FAIL (fdo#106560, fdo#106947) igt@drv_selftest@live_requests: fi-bsw-n3050: PASS -> INCOMPLETE (fdo#105876) igt@drv_selftest@live_workarounds: fi-bsw-n3050: PASS -> DMESG-FAIL (fdo#107292) {igt@kms_psr@primary_mmap_gtt}: fi-cnl-psr: PASS -> DMESG-WARN (fdo#107372) Possible fixes igt@drv_selftest@live_workarounds: fi-cnl-psr: DMESG-FAIL (fdo#107292) -> PASS igt@kms_chamelium@dp-edid-read: fi-kbl-7500u: FAIL (fdo#103841) -> PASS igt@kms_flip@basic-flip-vs-wf_vblank: fi-bsw-n3050: FAIL (fdo#100368) -> PASS igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence: {fi-byt-clapper}: FAIL (fdo#107362, fdo#103191) -> 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#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841 fdo#105876 https://bugs.freedesktop.org/show_bug.cgi?id=105876 fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560 fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947 fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174 fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372 == Participating hosts (51 -> 46) == Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * IGT: IGT_4584 -> IGTPW_1679 CI_DRM_4606: 603c0696f3c56d3f1e47c283de897448473c9041 @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_1679: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1679/ IGT_4584: 33f47ff4d64bd3996995dc5493deee26294e3aa3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1679/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/10] drm/i915: Constify power well descriptors
Em Qui, 2018-08-02 às 15:03 +0300, Imre Deak escreveu: > On Wed, Aug 01, 2018 at 02:39:31PM -0700, Paulo Zanoni wrote: > > Em Sex, 2018-07-20 às 17:14 +0300, Imre Deak escreveu: > > > It makes sense to keep unchanging data const. Extract such fields > > > from > > > the i915_power_well struct into a new i915_power_well_desc struct > > > that > > > we initialize during compile time. For the rest of the dynamic > > > fields allocate an array of i915_power_well objects in i915 > > > dev_priv, > > > and link to each of these objects their corresponding > > > i915_power_well_desc object. > > > > > > Cc: Ville Syrjala > > > Cc: Paulo Zanoni > > > Cc: Jani Nikula > > > Signed-off-by: Imre Deak > > > > Quite a few issues pointed by checkpatch for this patch, please > > take a > > look at them. > > > > More below: > > > > > --- > > > drivers/gpu/drm/i915/i915_debugfs.c | 4 +- > > > drivers/gpu/drm/i915/i915_drv.c | 8 +- > > > drivers/gpu/drm/i915/i915_drv.h | 14 ++- > > > drivers/gpu/drm/i915/intel_display.h| 4 +- > > > drivers/gpu/drm/i915/intel_drv.h| 1 + > > > drivers/gpu/drm/i915/intel_hdcp.c | 6 +- > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 204 > > > +++--- > > > -- > > > 7 files changed, 144 insertions(+), 97 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > > b/drivers/gpu/drm/i915/i915_debugfs.c > > > index b3aefd623557..eb284cac8fda 100644 > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > > @@ -2833,10 +2833,10 @@ static int i915_power_domain_info(struct > > > seq_file *m, void *unused) > > > enum intel_display_power_domain power_domain; > > > > > > power_well = &power_domains->power_wells[i]; > > > - seq_printf(m, "%-25s %d\n", power_well->name, > > > + seq_printf(m, "%-25s %d\n", power_well->desc- > > > >name, > > > power_well->count); > > > > > > - for_each_power_domain(power_domain, power_well- > > > > domains) > > > > > > + for_each_power_domain(power_domain, power_well- > > > > desc->domains) > > > > > > seq_printf(m, " %-23s %d\n", > > >intel_display_power_domain_str( > > > powe > > > r_domain), > > >power_domains- > > > > domain_use_count[power_domain]); > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > b/drivers/gpu/drm/i915/i915_drv.c > > > index 3c984530fef9..5743db4500fb 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -922,7 +922,9 @@ static int i915_driver_init_early(struct > > > drm_i915_private *dev_priv, > > > intel_uc_init_early(dev_priv); > > > intel_pm_setup(dev_priv); > > > intel_init_dpio(dev_priv); > > > - intel_power_domains_init(dev_priv); > > > + ret = intel_power_domains_init(dev_priv); > > > + if (ret < 0) > > > + goto err_uc; > > > intel_irq_init(dev_priv); > > > intel_hangcheck_init(dev_priv); > > > intel_init_display_hooks(dev_priv); > > > @@ -934,6 +936,9 @@ static int i915_driver_init_early(struct > > > drm_i915_private *dev_priv, > > > > > > return 0; > > > > > > +err_uc: > > > + intel_uc_cleanup_early(dev_priv); > > > > Please leave the guc fixes for a different patch, regardless of how > > innocent they look. > > Well, at least I didn't intend to fix guc. intel_uc_cleanup_early() > is > already called properly from i915_driver_cleanup_early(), not adding > the > call here would introduce a new problem if intel_power_domains_init() > failed. Ooops, I failed to realize we didn't have the guc cleanup call originally since there was no way to return non-zero after it. You're right. So with the checkpatch issues fixed: Reviewed-by: Paulo Zanoni > > > > > Everything else looks good! > > > > Thanks, > > Paulo > > > > > + i915_gem_cleanup_early(dev_priv); > > > err_workqueues: > > > i915_workqueues_cleanup(dev_priv); > > > err_engines: > > > @@ -948,6 +953,7 @@ static int i915_driver_init_early(struct > > > drm_i915_private *dev_priv, > > > static void i915_driver_cleanup_early(struct drm_i915_private > > > *dev_priv) > > > { > > > intel_irq_fini(dev_priv); > > > + intel_power_domains_cleanup(dev_priv); > > > intel_uc_cleanup_early(dev_priv); > > > i915_gem_cleanup_early(dev_priv); > > > i915_workqueues_cleanup(dev_priv); > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > b/drivers/gpu/drm/i915/i915_drv.h > > > index 4fb937399440..3ae200a9e8f1 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -862,13 +862,9 @@ struct i915_power_well_ops { > > > }; > > > > > > /* Power well structure for haswell */ > > > -struct i915_power_well { > > > +struct i915_power_well_desc { > > > const char *name; > > > bool always_on; > > > - /* power well enable/disable usage count */ > > > - int
Re: [Intel-gfx] [PATCH 01/10] drm/i915/icl: Fix power well anonymous union initializers
On Fri, Jul 20, 2018 at 05:14:55PM +0300, Imre Deak wrote: > Similarly to > 0a445945be6d ("drm/i915: Work around GCC anonymous union initialization bug") > we need to initialize anonymous unions inside extra braces to work > around a GCC4.4 build error. Aren't we jumping to gcc 4.5 as minimum version? Or was it 4.6/4.8? Lucas De Marchi > > Cc: Chris Wilson > Cc: Ville Syrjala > Cc: Paulo Zanoni > Cc: Jani Nikula > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/intel_runtime_pm.c | 22 +++--- > 1 file changed, 15 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 6b5aa3b074ec..1a87176a85c1 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -2620,14 +2620,18 @@ static struct i915_power_well icl_power_wells[] = { > .domains = 0, > .ops = &hsw_power_well_ops, > .id = ICL_DISP_PW_1, > - .hsw.has_fuses = true, > + { > + .hsw.has_fuses = true, > + }, > }, > { > .name = "power well 2", > .domains = ICL_PW_2_POWER_DOMAINS, > .ops = &hsw_power_well_ops, > .id = ICL_DISP_PW_2, > - .hsw.has_fuses = true, > + { > + .hsw.has_fuses = true, > + }, > }, > { > .name = "DC off", > @@ -2640,9 +2644,11 @@ static struct i915_power_well icl_power_wells[] = { > .domains = ICL_PW_3_POWER_DOMAINS, > .ops = &hsw_power_well_ops, > .id = ICL_DISP_PW_3, > - .hsw.irq_pipe_mask = BIT(PIPE_B), > - .hsw.has_vga = true, > - .hsw.has_fuses = true, > + { > + .hsw.irq_pipe_mask = BIT(PIPE_B), > + .hsw.has_vga = true, > + .hsw.has_fuses = true, > + }, > }, > { > .name = "DDI A IO", > @@ -2745,8 +2751,10 @@ static struct i915_power_well icl_power_wells[] = { > .domains = ICL_PW_4_POWER_DOMAINS, > .ops = &hsw_power_well_ops, > .id = ICL_DISP_PW_4, > - .hsw.has_fuses = true, > - .hsw.irq_pipe_mask = BIT(PIPE_C), > + { > + .hsw.has_fuses = true, > + .hsw.irq_pipe_mask = BIT(PIPE_C), > + }, > }, > }; > > -- > 2.13.2 > > ___ > 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] ✓ Fi.CI.IGT: success for docs: fix typo sharding->sharing
== Series Details == Series: docs: fix typo sharding->sharing URL : https://patchwork.freedesktop.org/series/47632/ State : success == Summary == = CI Bug Log - changes from IGT_4584_full -> IGTPW_1679_full = == Summary - WARNING == Minor unknown changes coming with IGTPW_1679_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in IGTPW_1679_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/47632/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in IGTPW_1679_full: === IGT changes === Warnings igt@prime_busy@before-blt: shard-snb: SKIP -> PASS == Known issues == Here are the changes found in IGTPW_1679_full that come from known issues: === IGT changes === Issues hit igt@kms_flip@flip-vs-expired-vblank: shard-glk: PASS -> FAIL (fdo#105363, fdo#102887) igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt: shard-glk: PASS -> FAIL (fdo#103167) igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-plflip-blt: shard-snb: PASS -> INCOMPLETE (fdo#105411) igt@kms_setmode@basic: shard-apl: PASS -> FAIL (fdo#99912) igt@pm_rpm@system-suspend: shard-kbl: PASS -> INCOMPLETE (fdo#103665) Possible fixes igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-wc: shard-snb: DMESG-WARN -> PASS igt@kms_rotation_crc@primary-rotation-180: shard-snb: FAIL (fdo#103925) -> PASS igt@kms_setmode@basic: shard-kbl: FAIL (fdo#99912) -> PASS igt@perf_pmu@busy-vcs0: shard-snb: INCOMPLETE (fdo#105411) -> PASS fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * IGT: IGT_4584 -> IGTPW_1679 * Linux: CI_DRM_4605 -> CI_DRM_4606 CI_DRM_4605: 50098198da758bdd54245d511f4f97236c296c72 @ git://anongit.freedesktop.org/gfx-ci/linux CI_DRM_4606: 603c0696f3c56d3f1e47c283de897448473c9041 @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_1679: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1679/ IGT_4584: 33f47ff4d64bd3996995dc5493deee26294e3aa3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1679/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/10] drm/i915/icl: Fix power well anonymous union initializers
Em Qui, 2018-08-02 às 16:17 -0700, Lucas De Marchi escreveu: > On Fri, Jul 20, 2018 at 05:14:55PM +0300, Imre Deak wrote: > > Similarly to > > 0a445945be6d ("drm/i915: Work around GCC anonymous union > > initialization bug") > > we need to initialize anonymous unions inside extra braces to work > > around a GCC4.4 build error. > > Aren't we jumping to gcc 4.5 as minimum version? Or was it 4.6/4.8? https://cgit.freedesktop.org/drm-tip/tree/Documentation/process/changes .rst#n32 3.2 is still the theoretical minimum for us. > > Lucas De Marchi > > > > > Cc: Chris Wilson > > Cc: Ville Syrjala > > Cc: Paulo Zanoni > > Cc: Jani Nikula > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/i915/intel_runtime_pm.c | 22 +++ > > --- > > 1 file changed, 15 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index 6b5aa3b074ec..1a87176a85c1 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -2620,14 +2620,18 @@ static struct i915_power_well > > icl_power_wells[] = { > > .domains = 0, > > .ops = &hsw_power_well_ops, > > .id = ICL_DISP_PW_1, > > - .hsw.has_fuses = true, > > + { > > + .hsw.has_fuses = true, > > + }, > > }, > > { > > .name = "power well 2", > > .domains = ICL_PW_2_POWER_DOMAINS, > > .ops = &hsw_power_well_ops, > > .id = ICL_DISP_PW_2, > > - .hsw.has_fuses = true, > > + { > > + .hsw.has_fuses = true, > > + }, > > }, > > { > > .name = "DC off", > > @@ -2640,9 +2644,11 @@ static struct i915_power_well > > icl_power_wells[] = { > > .domains = ICL_PW_3_POWER_DOMAINS, > > .ops = &hsw_power_well_ops, > > .id = ICL_DISP_PW_3, > > - .hsw.irq_pipe_mask = BIT(PIPE_B), > > - .hsw.has_vga = true, > > - .hsw.has_fuses = true, > > + { > > + .hsw.irq_pipe_mask = BIT(PIPE_B), > > + .hsw.has_vga = true, > > + .hsw.has_fuses = true, > > + }, > > }, > > { > > .name = "DDI A IO", > > @@ -2745,8 +2751,10 @@ static struct i915_power_well > > icl_power_wells[] = { > > .domains = ICL_PW_4_POWER_DOMAINS, > > .ops = &hsw_power_well_ops, > > .id = ICL_DISP_PW_4, > > - .hsw.has_fuses = true, > > - .hsw.irq_pipe_mask = BIT(PIPE_C), > > + { > > + .hsw.has_fuses = true, > > + .hsw.irq_pipe_mask = BIT(PIPE_C), > > + }, > > }, > > }; > > > > -- > > 2.13.2 > > > > ___ > > 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
Re: [Intel-gfx] [PATCH] drm/i915/gvt: fix memory leak in intel_vgpu_ioctl()
On 2018.08.03 08:41:19 +0800, Yi Wang wrote: > The 'sparse' variable may leak when return in function > intel_vgpu_ioctl(), and this patch fixes this. > > Signed-off-by: Yi Wang > Reviewed-by: Jiang Biao > --- Looks fine to me, will queue this up. Thanks for the patch! > drivers/gpu/drm/i915/gvt/kvmgt.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c > b/drivers/gpu/drm/i915/gvt/kvmgt.c > index df4e4a0..6a6f199 100644 > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c > @@ -1200,6 +1200,7 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, > unsigned int cmd, > return ret; > break; > default: > + kfree(sparse); > return -EINVAL; > } > } > @@ -1215,6 +1216,7 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, > unsigned int cmd, > sizeof(info), caps.buf, > caps.size)) { > kfree(caps.buf); > + kfree(sparse); > return -EFAULT; > } > info.cap_offset = sizeof(info); > @@ -1223,6 +1225,7 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, > unsigned int cmd, > kfree(caps.buf); > } > > + kfree(sparse); > return copy_to_user((void __user *)arg, &info, minsz) ? > -EFAULT : 0; > } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) { > -- > 1.8.3.1 > -- 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
[Intel-gfx] [PATCH] drm/i915/kvmgt: Fix potential Spectre v1
info.index can be indirectly controlled by user-space, hence leading to a potential exploitation of the Spectre variant 1 vulnerability. This issue was detected with the help of Smatch: drivers/gpu/drm/i915/gvt/kvmgt.c:1232 intel_vgpu_ioctl() warn: potential spectre issue 'vgpu->vdev.region' [r] Fix this by sanitizing info.index before indirectly using it to index vgpu->vdev.region Notice that given that speculation windows are large, the policy is to kill the speculation on the first load and not worry if it can be completed with a dependent load/store [1]. [1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2 Cc: sta...@vger.kernel.org Signed-off-by: Gustavo A. R. Silva --- drivers/gpu/drm/i915/gvt/kvmgt.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 4d2f53a..b703f20 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -43,6 +43,8 @@ #include #include +#include + #include "i915_drv.h" #include "gvt.h" @@ -1139,7 +1141,8 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd, } else if (cmd == VFIO_DEVICE_GET_REGION_INFO) { struct vfio_region_info info; struct vfio_info_cap caps = { .buf = NULL, .size = 0 }; - int i, ret; + unsigned int i; + int ret; struct vfio_region_info_cap_sparse_mmap *sparse = NULL; size_t size; int nr_areas = 1; @@ -1224,6 +1227,10 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd, if (info.index >= VFIO_PCI_NUM_REGIONS + vgpu->vdev.num_regions) return -EINVAL; + info.index = + array_index_nospec(info.index, + VFIO_PCI_NUM_REGIONS + + vgpu->vdev.num_regions); i = info.index - VFIO_PCI_NUM_REGIONS; -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/kvmgt: Fix potential Spectre v1
== Series Details == Series: drm/i915/kvmgt: Fix potential Spectre v1 URL : https://patchwork.freedesktop.org/series/47640/ State : warning == Summary == $ dim checkpatch origin/drm-tip 0ea70a869370 drm/i915/kvmgt: Fix potential Spectre v1 -:55: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis #55: FILE: drivers/gpu/drm/i915/gvt/kvmgt.c:1232: + array_index_nospec(info.index, + VFIO_PCI_NUM_REGIONS + total: 0 errors, 0 warnings, 1 checks, 27 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 drm/i915/kvmgt: Fix potential Spectre v1
== Series Details == Series: drm/i915/kvmgt: Fix potential Spectre v1 URL : https://patchwork.freedesktop.org/series/47640/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4608 -> Patchwork_9845 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/47640/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_9845 that come from known issues: === IGT changes === Issues hit igt@drv_module_reload@basic-reload-inject: fi-hsw-4770r: PASS -> DMESG-WARN (fdo#107425) igt@drv_selftest@live_workarounds: {fi-bsw-kefka}: PASS -> DMESG-FAIL (fdo#107292) fi-skl-6700k2: PASS -> DMESG-FAIL (fdo#107292) igt@kms_frontbuffer_tracking@basic: fi-hsw-peppy: PASS -> DMESG-FAIL (fdo#102614, fdo#106103) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: fi-cnl-psr: PASS -> DMESG-WARN (fdo#104951) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-bxt-dsi: PASS -> INCOMPLETE (fdo#103927) Possible fixes igt@drv_selftest@live_hangcheck: fi-bdw-5557u: DMESG-FAIL (fdo#106560) -> PASS Warnings {igt@kms_psr@primary_page_flip}: fi-cnl-psr: DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372) {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951 fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103 fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560 fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292 fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372 fdo#107425 https://bugs.freedesktop.org/show_bug.cgi?id=107425 == Participating hosts (52 -> 46) == Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-u == Build changes == * Linux: CI_DRM_4608 -> Patchwork_9845 CI_DRM_4608: 9d129b43738d0b604a787e54e041973ac7a7c922 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4586: 57caaf440520e397403d898e1d3f1d65ef7b79e2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9845: 0ea70a8693707d7c8195c331511c4d01e5500153 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 0ea70a869370 drm/i915/kvmgt: Fix potential Spectre v1 == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9845/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/kvmgt: Fix potential Spectre v1
== Series Details == Series: drm/i915/kvmgt: Fix potential Spectre v1 URL : https://patchwork.freedesktop.org/series/47640/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4608_full -> Patchwork_9845_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_9845_full that come from known issues: === IGT changes === Issues hit igt@drv_suspend@shrink: shard-kbl: PASS -> FAIL (fdo#106886) igt@gem_ppgtt@blt-vs-render-ctxn: shard-kbl: PASS -> INCOMPLETE (fdo#106023, fdo#103665) Possible fixes igt@drv_selftest@live_hangcheck: shard-kbl: DMESG-FAIL (fdo#106560, fdo#106947) -> PASS igt@drv_suspend@shrink: shard-snb: INCOMPLETE (fdo#105411, fdo#106886) -> PASS shard-hsw: INCOMPLETE (fdo#103540, fdo#106886) -> PASS igt@gem_ppgtt@blt-vs-render-ctx0: shard-kbl: INCOMPLETE (fdo#106023, fdo#103665) -> PASS igt@kms_flip@flip-vs-expired-vblank: shard-glk: FAIL (fdo#105363, fdo#102887) -> PASS igt@kms_setmode@basic: shard-kbl: FAIL (fdo#99912) -> PASS fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887 fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023 fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4608 -> Patchwork_9845 CI_DRM_4608: 9d129b43738d0b604a787e54e041973ac7a7c922 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4586: 57caaf440520e397403d898e1d3f1d65ef7b79e2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9845: 0ea70a8693707d7c8195c331511c4d01e5500153 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9845/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx