[Intel-gfx] ✗ Ro.CI.BAT: failure for drm/i915/ilk: Wait one vblank before enabling audio
== Series Details == Series: drm/i915/ilk: Wait one vblank before enabling audio URL : https://patchwork.freedesktop.org/series/7488/ State : failure == Summary == Series 7488v1 drm/i915/ilk: Wait one vblank before enabling audio http://patchwork.freedesktop.org/api/1.0/series/7488/revisions/1/mbox Test drv_module_reload_basic: pass -> DMESG-WARN (fi-byt-n2820) Test gem_exec_gttfill: Subgroup basic: skip -> PASS (fi-hsw-i7-4770k) skip -> PASS (fi-hsw-i7-4770r) skip -> FAIL (fi-skl-i7-6700k) Test gem_exec_suspend: Subgroup basic-s3: pass -> DMESG-WARN (fi-byt-n2820) Test kms_flip: Subgroup basic-flip-vs-dpms: pass -> DMESG-WARN (fi-byt-n2820) Subgroup basic-flip-vs-modeset: pass -> DMESG-WARN (fi-byt-n2820) Subgroup basic-flip-vs-wf_vblank: pass -> DMESG-WARN (fi-byt-n2820) Subgroup basic-plain-flip: pass -> DMESG-WARN (fi-byt-n2820) Test kms_frontbuffer_tracking: Subgroup basic: pass -> DMESG-WARN (fi-byt-n2820) Test kms_pipe_crc_basic: Subgroup hang-read-crc-pipe-a: pass -> DMESG-WARN (fi-byt-n2820) Subgroup hang-read-crc-pipe-b: pass -> DMESG-WARN (fi-byt-n2820) Subgroup nonblocking-crc-pipe-a: pass -> DMESG-WARN (fi-byt-n2820) Subgroup nonblocking-crc-pipe-a-frame-sequence: pass -> DMESG-WARN (fi-byt-n2820) Subgroup nonblocking-crc-pipe-b: pass -> DMESG-WARN (fi-byt-n2820) Subgroup nonblocking-crc-pipe-b-frame-sequence: pass -> DMESG-WARN (fi-byt-n2820) Subgroup read-crc-pipe-a: pass -> DMESG-WARN (fi-byt-n2820) Subgroup read-crc-pipe-a-frame-sequence: pass -> DMESG-WARN (fi-byt-n2820) Subgroup read-crc-pipe-b: pass -> DMESG-WARN (fi-byt-n2820) Subgroup read-crc-pipe-b-frame-sequence: pass -> DMESG-WARN (fi-byt-n2820) Subgroup suspend-read-crc-pipe-a: pass -> DMESG-WARN (fi-byt-n2820) Subgroup suspend-read-crc-pipe-b: pass -> DMESG-WARN (fi-byt-n2820) Test kms_sink_crc_basic: pass -> SKIP (ro-skl-i7-6700hq) Test pm_rpm: Subgroup basic-pci-d3-state: pass -> DMESG-WARN (fi-byt-n2820) Subgroup basic-rte: pass -> DMESG-WARN (fi-byt-n2820) fi-byt-n2820 total:209 pass:148 dwarn:21 dfail:0 fail:2 skip:38 fi-hsw-i7-4770k total:209 pass:190 dwarn:0 dfail:0 fail:0 skip:19 fi-hsw-i7-4770r total:209 pass:186 dwarn:0 dfail:0 fail:0 skip:23 fi-skl-i7-6700k total:209 pass:183 dwarn:0 dfail:0 fail:1 skip:25 fi-snb-i7-2600 total:209 pass:170 dwarn:0 dfail:0 fail:0 skip:39 ro-bdw-i5-5250u total:209 pass:171 dwarn:0 dfail:0 fail:0 skip:38 ro-bdw-i7-5557U total:209 pass:196 dwarn:0 dfail:0 fail:0 skip:13 ro-bdw-i7-5600u total:209 pass:179 dwarn:0 dfail:0 fail:1 skip:29 ro-bsw-n3050 total:209 pass:166 dwarn:0 dfail:0 fail:3 skip:40 ro-byt-n2820 total:209 pass:168 dwarn:0 dfail:0 fail:3 skip:38 ro-hsw-i3-4010u total:209 pass:186 dwarn:0 dfail:0 fail:0 skip:23 ro-hsw-i7-4770r total:209 pass:185 dwarn:0 dfail:0 fail:0 skip:24 ro-ilk-i7-620lm total:209 pass:145 dwarn:0 dfail:0 fail:1 skip:63 ro-ilk1-i5-650 total:204 pass:146 dwarn:0 dfail:0 fail:1 skip:57 ro-ivb-i7-3770 total:209 pass:176 dwarn:0 dfail:0 fail:0 skip:33 ro-ivb2-i7-3770 total:209 pass:180 dwarn:0 dfail:0 fail:0 skip:29 ro-skl-i7-6700hq total:204 pass:181 dwarn:0 dfail:0 fail:0 skip:23 ro-snb-i7-2620M total:209 pass:170 dwarn:0 dfail:0 fail:1 skip:38 fi-bdw-i7-5557u failed to connect after reboot fi-bsw-n3050 failed to connect after reboot Results at /archive/results/CI_IGT_test/RO_Patchwork_953/ f1eaed1 drm-intel-nightly: 2016y-05m-20d-14h-35m-29s UTC integration manifest 376347a drm/i915/ilk: Wait one vblank before enabling audio ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC i-g-t 8/9] lib: Replace intel specific header includes with intel_drm_stubs.h.
From: Robert FossReplace intel specific header includes with intel_drm_stubs.h. The stubbed functions will all call igt_require(false) and cause a skip. Signed-off-by: Robert Foss --- lib/drmtest.c | 2 +- lib/gpgpu_fill.c| 7 +++ lib/igt_aux.c | 2 +- lib/igt_aux.h | 3 ++- lib/igt_debugfs.c | 4 ++-- lib/igt_draw.h | 3 +-- lib/igt_fb.h| 3 ++- lib/igt_kms.c | 3 +-- lib/intel_batchbuffer.c | 4 lib/intel_batchbuffer.h | 3 +-- lib/intel_chipset.c | 2 +- lib/ioctl_wrappers.c| 1 - lib/ioctl_wrappers.h| 4 ++-- lib/media_fill_gen7.c | 3 +-- lib/media_fill_gen8.c | 4 +--- lib/media_fill_gen8lp.c | 6 ++ lib/media_fill_gen9.c | 4 +--- lib/media_spin.c| 2 -- lib/rendercopy_gen6.c | 5 ++--- lib/rendercopy_gen7.c | 4 +--- lib/rendercopy_gen8.c | 4 +--- lib/rendercopy_gen9.c | 5 + lib/rendercopy_i830.c | 5 + lib/rendercopy_i915.c | 9 +++-- 24 files changed, 31 insertions(+), 61 deletions(-) diff --git a/lib/drmtest.c b/lib/drmtest.c index 7d6b74a..f043607 100644 --- a/lib/drmtest.c +++ b/lib/drmtest.c @@ -48,13 +48,13 @@ #include #include "drmtest.h" -#include "i915_drm.h" #include "intel_chipset.h" #include "intel_io.h" #include "igt_gt.h" #include "igt_debugfs.h" #include "version.h" #include "config.h" +#include "intel_drm_stubs.h" #include "intel_reg.h" #include "ioctl_wrappers.h" diff --git a/lib/gpgpu_fill.c b/lib/gpgpu_fill.c index 4d98643..62b1161 100644 --- a/lib/gpgpu_fill.c +++ b/lib/gpgpu_fill.c @@ -25,15 +25,14 @@ * Dominik Zeromski */ -#include -#include -#include "intel_reg.h" #include "drmtest.h" -#include "intel_batchbuffer.h" #include "gen7_media.h" #include "gen8_media.h" #include "gpgpu_fill.h" +#include "intel_batchbuffer.h" +#include "intel_drm_stubs.h" +#include "intel_reg.h" /* shaders/gpgpu/gpgpu_fill.gxa */ static const uint32_t gen7_gpgpu_kernel[][4] = { diff --git a/lib/igt_aux.c b/lib/igt_aux.c index fe18365..772c902 100644 --- a/lib/igt_aux.c +++ b/lib/igt_aux.c @@ -52,12 +52,12 @@ #include #include "drmtest.h" -#include "i915_drm.h" #include "intel_chipset.h" #include "igt_aux.h" #include "igt_debugfs.h" #include "igt_gt.h" #include "config.h" +#include "intel_drm_stubs.h" #include "intel_reg.h" #include "ioctl_wrappers.h" #include "igt_kms.h" diff --git a/lib/igt_aux.h b/lib/igt_aux.h index f66de72..c66121b 100644 --- a/lib/igt_aux.h +++ b/lib/igt_aux.h @@ -28,10 +28,11 @@ #ifndef IGT_AUX_H #define IGT_AUX_H -#include #include #include +#include "intel_drm_stubs.h" + extern drm_intel_bo **trash_bos; extern int num_trash_bos; diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c index a32ed78..d9f371f 100644 --- a/lib/igt_debugfs.c +++ b/lib/igt_debugfs.c @@ -32,12 +32,12 @@ #include #include #include -#include #include "drmtest.h" +#include "intel_drm_stubs.h" #include "igt_aux.h" -#include "igt_kms.h" #include "igt_debugfs.h" +#include "igt_kms.h" /** * SECTION:igt_debugfs diff --git a/lib/igt_draw.h b/lib/igt_draw.h index b030131..c0e95ca 100644 --- a/lib/igt_draw.h +++ b/lib/igt_draw.h @@ -25,9 +25,8 @@ #ifndef __IGT_DRAW_H__ #define __IGT_DRAW_H__ -#include #include "igt_fb.h" - +#include "intel_drm_stubs.h" /** * igt_draw_method: * @IGT_DRAW_MMAP_CPU: draw using a CPU mmap. diff --git a/lib/igt_fb.h b/lib/igt_fb.h index ce2cc0f..82dbacb 100644 --- a/lib/igt_fb.h +++ b/lib/igt_fb.h @@ -38,10 +38,11 @@ typedef struct _cairo cairo_t; #include #include +#include #include #include -#include +#include "intel_drm_stubs.h" /* helpers to create nice-looking framebuffers */ struct igt_fb { diff --git a/lib/igt_kms.c b/lib/igt_kms.c index f85be1e..7afee53 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -41,12 +41,11 @@ #include #include -#include - #include "drmtest.h" #include "igt_kms.h" #include "igt_aux.h" #include "intel_chipset.h" +#include "intel_drm_stubs.h" #include "igt_debugfs.h" /* list of connectors that need resetting on exit */ diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c index 692521f..a28eb33 100644 --- a/lib/intel_batchbuffer.c +++ b/lib/intel_batchbuffer.c @@ -31,10 +31,8 @@ #include #include -#include "drm.h" #include "drmtest.h" #include "intel_batchbuffer.h" -#include "intel_bufmgr.h" #include "intel_chipset.h" #include "intel_reg.h" #include "rendercopy.h" @@ -43,8 +41,6 @@ #include "media_spin.h" #include "gpgpu_fill.h" -#include - /** * SECTION:intel_batchbuffer * @short_description: Batchbuffer and blitter support diff --git a/lib/intel_batchbuffer.h b/lib/intel_batchbuffer.h index 869747d..bd365db 100644 --- a/lib/intel_batchbuffer.h +++ b/lib/intel_batchbuffer.h @@ -2,10 +2,9 @@ #define INTEL_BATCHBUFFER_H #include -#include -#include #include
[Intel-gfx] [RFC i-g-t 9/9] tests: Replace intel specific header includes with intel_drm_stubs.h.
From: Robert FossReplace intel specific header includes with intel_drm_stubs.h. The stubbed functions will all call igt_require(false) and cause a skip. Additionally there a small amount of reformatting of the #includes was perfomed. "XXX" was fixed to where approptiate, spacing and alphabetical ordering was also enforced. Signed-off-by: Robert Foss --- tests/core_auth.c | 2 +- tests/core_get_client_auth.c | 2 +- tests/core_getclient.c | 3 ++- tests/drm_import_export.c | 7 +++ tests/drm_read.c | 5 +++-- tests/drm_vma_limiter.c| 6 +++--- tests/drm_vma_limiter_cached.c | 6 +++--- tests/drm_vma_limiter_cpu.c| 6 +++--- tests/drm_vma_limiter_gtt.c| 6 +++--- tests/drv_getparams_basic.c| 6 +++--- tests/gem_alive.c | 4 ++-- tests/gem_bad_address.c| 6 +++--- tests/gem_bad_batch.c | 6 +++--- tests/gem_bad_blit.c | 6 +++--- tests/gem_bad_length.c | 2 +- tests/gem_bad_reloc.c | 2 +- tests/gem_basic.c | 2 +- tests/gem_caching.c| 5 ++--- tests/gem_close_race.c | 2 +- tests/gem_concurrent_all.c | 5 ++--- tests/gem_cpu_reloc.c | 4 ++-- tests/gem_create.c | 8 tests/gem_ctx_bad_exec.c | 3 ++- tests/gem_ctx_basic.c | 8 +--- tests/gem_double_irq_loop.c| 7 --- tests/gem_evict_everything.c | 2 +- tests/gem_exec_bad_domains.c | 7 --- tests/gem_exec_big.c | 2 +- tests/gem_exec_blt.c | 2 +- tests/gem_exec_faulting_reloc.c| 2 +- tests/gem_exec_lut_handle.c| 2 +- tests/gem_exec_nop.c | 2 +- tests/gem_exec_params.c| 2 +- tests/gem_fence_thrash.c | 2 +- tests/gem_fence_upload.c | 7 --- tests/gem_flink_basic.c| 2 +- tests/gem_flink_race.c | 4 ++-- tests/gem_gpgpu_fill.c | 7 --- tests/gem_gtt_cpu_tlb.c| 2 +- tests/gem_gtt_speed.c | 2 +- tests/gem_hang.c | 6 +++--- tests/gem_hangcheck_forcewake.c| 7 --- tests/gem_largeobject.c| 2 +- tests/gem_lut_handle.c | 2 +- tests/gem_madvise.c| 3 +-- tests/gem_media_fill.c | 7 --- tests/gem_mmap.c | 2 +- tests/gem_mmap_gtt.c | 2 +- tests/gem_mmap_offset_exhaustion.c | 2 +- tests/gem_mmap_wc.c| 2 +- tests/gem_non_secure_batch.c | 5 +++-- tests/gem_partial_pwrite_pread.c | 1 - tests/gem_persistent_relocs.c | 1 - tests/gem_pin.c| 1 - tests/gem_pipe_control_store_loop.c| 7 --- tests/gem_ppgtt.c | 6 +++--- tests/gem_pread.c | 2 +- tests/gem_pread_after_blit.c | 1 - tests/gem_pwrite.c | 2 +- tests/gem_pwrite_pread.c | 7 --- tests/gem_pwrite_snooped.c | 2 +- tests/gem_read_read_speed.c| 5 ++--- tests/gem_readwrite.c | 2 +- tests/gem_reloc_overflow.c | 2 +- tests/gem_reloc_vs_gpu.c | 1 - tests/gem_render_copy.c| 5 ++--- tests/gem_render_copy_redux.c | 7 +++ tests/gem_render_linear_blits.c| 5 ++--- tests/gem_render_tiled_blits.c | 5 ++--- tests/gem_request_retire.c | 8 +++- tests/gem_seqno_wrap.c | 4 ++-- tests/gem_set_tiling_vs_blt.c | 7 --- tests/gem_set_tiling_vs_gtt.c | 2 +- tests/gem_set_tiling_vs_pwrite.c | 2 +- tests/gem_stolen.c | 14 ++ tests/gem_storedw_batches_loop.c | 7 --- tests/gem_storedw_loop.c | 2 +- tests/gem_streaming_writes.c | 2 +- tests/gem_stress.c | 6 ++ tests/gem_threaded_access_tiled.c | 4 ++-- tests/gem_tiled_blits.c| 1 - tests/gem_tiled_fence_blits.c | 5 ++--- tests/gem_tiled_partial_pwrite_pread.c | 1 - tests/gem_tiled_pread_basic.c | 2 +- tests/gem_tiled_pread_pwrite.c | 1 - tests/gem_tiled_swapping.c | 1 - tests/gem_tiled_wb.c | 2 +- tests/gem_tiled_wc.c | 2 +- tests/gem_tiling_max_stride.c | 2 +- tests/gem_unfence_active_buffers.c | 7 --- tests/gem_unref_active_buffers.c | 7 --- tests/gem_userptr_blits.c
[Intel-gfx] [RFC i-g-t 7/9] lib/intel_drm_stubs: Add stubs for functionality from libdrm_intel.
From: Robert FossThis patch provides stubs for functionality otherwise provided by libdrm_intel. The stubbed functions all fail with a call to igt_require(false). Defines and enums have been copied from libdrm_intel. Due to the stubbed tests failing with an igt_require() call, these stubs are not well suited for non-tests, since tools/benchmarks/etc 'skipping' execution is unhelpful. Signed-off-by: Robert Foss --- lib/Makefile.sources | 2 + lib/intel_drm_stubs.c | 258 + lib/intel_drm_stubs.h | 999 ++ 3 files changed, 1259 insertions(+) create mode 100644 lib/intel_drm_stubs.c create mode 100644 lib/intel_drm_stubs.h diff --git a/lib/Makefile.sources b/lib/Makefile.sources index 1316fd2..c0f9f6d 100644 --- a/lib/Makefile.sources +++ b/lib/Makefile.sources @@ -21,6 +21,8 @@ libintel_tools_la_SOURCES = \ intel_batchbuffer.c \ intel_batchbuffer.h \ intel_chipset.h \ + intel_drm_stubs.c \ + intel_drm_stubs.h \ intel_os.c \ intel_io.h \ intel_mmio.c\ diff --git a/lib/intel_drm_stubs.c b/lib/intel_drm_stubs.c new file mode 100644 index 000..d46a9b3 --- /dev/null +++ b/lib/intel_drm_stubs.c @@ -0,0 +1,258 @@ +#ifndef HAVE_INTEL + +#include "intel_drm_stubs.h" + +drm_intel_bufmgr *drm_intel_bufmgr_gem_init(int fd, int batch_size) +{ + igt_require(false); + return (drm_intel_bufmgr *) NULL; +} + +void drm_intel_bo_unreference(drm_intel_bo *bo) +{ + igt_require(false); +} + +drm_intel_bo *drm_intel_bo_alloc(drm_intel_bufmgr *bufmgr, const char *name, +unsigned long size, unsigned int alignment) +{ + igt_require(false); + return (drm_intel_bo *) NULL; +} + +int drm_intel_bo_subdata(drm_intel_bo *bo, unsigned long offset, +unsigned long size, const void *data) +{ + igt_require(false); + return 0; +} + +int drm_intel_gem_bo_context_exec(drm_intel_bo *bo, drm_intel_context *ctx, + int used, unsigned int flags) +{ + igt_require(false); + return 0; +} + +int drm_intel_bo_emit_reloc(drm_intel_bo *bo, uint32_t offset, + drm_intel_bo *target_bo, uint32_t target_offset, + uint32_t read_domains, uint32_t write_domain) +{ + igt_require(false); + return 0; +} + +int drm_intel_bo_emit_reloc_fence(drm_intel_bo *bo, uint32_t offset, + drm_intel_bo *target_bo, + uint32_t target_offset, + uint32_t read_domains, uint32_t write_domain) +{ + igt_require(false); + return 0; +} + +int drm_intel_bo_get_tiling(drm_intel_bo *bo, uint32_t * tiling_mode, + uint32_t * swizzle_mode) +{ + igt_require(false); + return 0; +} + +int drm_intel_bo_mrb_exec(drm_intel_bo *bo, int used, + struct drm_clip_rect *cliprects, int num_cliprects, int DR4, + unsigned int flags) +{ + igt_require(false); + return 0; +} + +void drm_intel_bufmgr_gem_set_aub_annotations(drm_intel_bo *bo, +drm_intel_aub_annotation *annotations, +unsigned count) +{ + igt_require(false); +} + +void drm_intel_bufmgr_gem_enable_reuse(drm_intel_bufmgr *bufmgr) +{ + igt_require(false); +} + +int drm_intel_bo_exec(drm_intel_bo *bo, int used, + struct drm_clip_rect *cliprects, int num_cliprects, int DR4) +{ + igt_require(false); + return 0; +} + +void drm_intel_bufmgr_destroy(drm_intel_bufmgr *bufmgr) +{ + igt_require(false); +} + +void drm_intel_bo_wait_rendering(drm_intel_bo *bo) +{ + igt_require(false); +} + +int drm_intel_bo_get_subdata(drm_intel_bo *bo, unsigned long offset, +unsigned long size, void *data) +{ + igt_require(false); + return 0; +} + +int drm_intel_bo_map(drm_intel_bo *bo, int write_enable) +{ + igt_require(false); + return 0; +} + +int drm_intel_gem_bo_map_gtt(drm_intel_bo *bo) +{ + igt_require(false); + return 0; +} + +void drm_intel_bufmgr_gem_enable_fenced_relocs(drm_intel_bufmgr *bufmgr) +{ + igt_require(false); +} + +int drm_intel_bo_unmap(drm_intel_bo *bo) +{ + igt_require(false); + return 0; +} + +int drm_intel_bo_flink(drm_intel_bo *bo, uint32_t * name) +{ + igt_require(false); + return 0; +} + +drm_intel_bo *drm_intel_bo_gem_create_from_name(drm_intel_bufmgr *bufmgr, + const char *name, + unsigned int handle) +{ + igt_require(false); + return (drm_intel_bo
[Intel-gfx] [RFC i-g-t 5/9] tests/gem_ppgtt: Switched to new aliases of intel specific functions.
From: Robert FossSwitched from drm_XXX aliases drm_intel_XXX aliases for symbols where that switch is possible. Signed-off-by: Robert Foss --- tests/gem_ppgtt.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/gem_ppgtt.c b/tests/gem_ppgtt.c index 1a620ad..eb15ca5 100644 --- a/tests/gem_ppgtt.c +++ b/tests/gem_ppgtt.c @@ -74,11 +74,11 @@ static void scratch_buf_init(struct igt_buf *buf, static void scratch_buf_fini(struct igt_buf *buf) { - dri_bo_unreference(buf->bo); + drm_intel_bo_unreference(buf->bo); memset(buf, 0, sizeof(*buf)); } -static void fork_rcs_copy(int target, dri_bo **dst, int count, unsigned flags) +static void fork_rcs_copy(int target, drm_intel_bo **dst, int count, unsigned flags) #define CREATE_CONTEXT 0x1 { igt_render_copyfunc_t render_copy; @@ -143,7 +143,7 @@ static void fork_rcs_copy(int target, dri_bo **dst, int count, unsigned flags) } } -static void fork_bcs_copy(int target, dri_bo **dst, int count) +static void fork_bcs_copy(int target, drm_intel_bo **dst, int count) { int devid; @@ -167,7 +167,7 @@ static void fork_bcs_copy(int target, dri_bo **dst, int count) igt_assert(batch); for (int i = 0; i <= target; i++) { - dri_bo *src[2]; + drm_intel_bo *src[2]; src[0] = create_bo(dst[child]->bufmgr, ~0); @@ -177,13 +177,13 @@ static void fork_bcs_copy(int target, dri_bo **dst, int count) intel_copy_bo(batch, src[0], src[1], SIZE); intel_copy_bo(batch, dst[child], src[0], SIZE); - dri_bo_unreference(src[1]); - dri_bo_unreference(src[0]); + drm_intel_bo_unreference(src[1]); + drm_intel_bo_unreference(src[0]); } } } -static void surfaces_check(dri_bo **bo, int count, uint32_t expected) +static void surfaces_check(drm_intel_bo **bo, int count, uint32_t expected) { for (int child = 0; child < count; child++) { uint32_t *ptr; @@ -322,7 +322,7 @@ int main(int argc, char **argv) igt_subtest_init(argc, argv); igt_subtest("blt-vs-render-ctx0") { - dri_bo *bcs[1], *rcs[N_CHILD]; + drm_intel_bo *bcs[1], *rcs[N_CHILD]; fork_bcs_copy(0x4000, bcs, 1); fork_rcs_copy(0x8000 / N_CHILD, rcs, N_CHILD, 0); @@ -334,7 +334,7 @@ int main(int argc, char **argv) } igt_subtest("blt-vs-render-ctxN") { - dri_bo *bcs[1], *rcs[N_CHILD]; + drm_intel_bo *bcs[1], *rcs[N_CHILD]; fork_rcs_copy(0x8000 / N_CHILD, rcs, N_CHILD, CREATE_CONTEXT); fork_bcs_copy(0x4000, bcs, 1); -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC i-g-t 0/9] Remove compile time depencencies on libdrm_intel.
From: Robert FossHey, I've been looking at the possibilty of removing the compile time depency on libdrm_intel. There are two technical solutions to this problem as far as I can see; stubs and conditional compilation. I'd like to compare the two approaches to provide an overview. Conditional compilation: + Programs that will not work on a given platform are not built. + Faster compilation (especially helpful on slow platforms like the RPi2). - Combinatorial complexity of different environments (what to build if we have libpciaccess and libdrm_vc4 but no libintel_drm for example), that that quickly will get worse with more configurations. - Currently some sources like igt_aux/igt_kms partially depend on libdrm_intel, to avoid #ifdef hell, configuraiton specific functionality should have to be extracted into new files. Stubs: + Relatively straight forward to implement, as can be seen from these patches. + Can easily be extended for any depency. - Duplicating defines/structs/functions is a maintenance burden. - Some built binaries will always skip, and are essentially useless. - Little compilation speedup. Robert Foss (9): configure.ac: Test for libdrm_intel and build for it if present. benchmarks/Makefile: Don't build benchmarks that depend on libdrm_intel. tools/Makefile: Don't build tools that depend on libdrm_intel. demos/Makefile: Don't build tools that depend on libdrm_intel. tests/gem_ppgtt: Switched to new aliases of intel specific functions. tests/gem_render_tiled_blits: Switched to new aliases of intel specific functions. lib/intel_drm_stubs: Add stubs for functionality from libdrm_intel. lib: Replace intel specific header includes with intel_drm_stubs.h. tests: Replace intel specific header includes with intel_drm_stubs.h. benchmarks/Makefile.sources| 15 +- configure.ac | 14 +- demos/Makefile.am | 5 +- demos/Makefile.sources | 7 + lib/Makefile.sources | 2 + lib/drmtest.c | 2 +- lib/gpgpu_fill.c | 7 +- lib/igt_aux.c | 2 +- lib/igt_aux.h | 3 +- lib/igt_debugfs.c | 4 +- lib/igt_draw.h | 3 +- lib/igt_fb.h | 3 +- lib/igt_kms.c | 3 +- lib/intel_batchbuffer.c| 4 - lib/intel_batchbuffer.h| 3 +- lib/intel_chipset.c| 2 +- lib/intel_drm_stubs.c | 258 + lib/intel_drm_stubs.h | 999 + lib/ioctl_wrappers.c | 1 - lib/ioctl_wrappers.h | 4 +- lib/media_fill_gen7.c | 3 +- lib/media_fill_gen8.c | 4 +- lib/media_fill_gen8lp.c| 6 +- lib/media_fill_gen9.c | 4 +- lib/media_spin.c | 2 - lib/rendercopy_gen6.c | 5 +- lib/rendercopy_gen7.c | 4 +- lib/rendercopy_gen8.c | 4 +- lib/rendercopy_gen9.c | 5 +- lib/rendercopy_i830.c | 5 +- lib/rendercopy_i915.c | 9 +- tests/core_auth.c | 2 +- tests/core_get_client_auth.c | 2 +- tests/core_getclient.c | 3 +- tests/drm_import_export.c | 7 +- tests/drm_read.c | 5 +- tests/drm_vma_limiter.c| 6 +- tests/drm_vma_limiter_cached.c | 6 +- tests/drm_vma_limiter_cpu.c| 6 +- tests/drm_vma_limiter_gtt.c| 6 +- tests/drv_getparams_basic.c| 6 +- tests/gem_alive.c | 4 +- tests/gem_bad_address.c| 6 +- tests/gem_bad_batch.c | 6 +- tests/gem_bad_blit.c | 6 +- tests/gem_bad_length.c | 2 +- tests/gem_bad_reloc.c | 2 +- tests/gem_basic.c | 2 +- tests/gem_caching.c| 5 +- tests/gem_close_race.c | 2 +- tests/gem_concurrent_all.c | 5 +- tests/gem_cpu_reloc.c | 4 +- tests/gem_create.c | 8 +- tests/gem_ctx_bad_exec.c | 3 +- tests/gem_ctx_basic.c | 8 +- tests/gem_double_irq_loop.c| 7 +- tests/gem_evict_everything.c | 2 +- tests/gem_exec_bad_domains.c | 7 +- tests/gem_exec_big.c | 2 +- tests/gem_exec_blt.c | 2 +- tests/gem_exec_faulting_reloc.c| 2 +- tests/gem_exec_lut_handle.c| 2 +- tests/gem_exec_nop.c | 2 +- tests/gem_exec_params.c
[Intel-gfx] [RFC i-g-t 6/9] tests/gem_render_tiled_blits: Switched to new aliases of intel specific functions.
From: Robert FossSwitched from drm_XXX aliases drm_intel_XXX aliases for symbols where that switch is possible. Signed-off-by: Robert Foss --- tests/gem_render_tiled_blits.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/gem_render_tiled_blits.c b/tests/gem_render_tiled_blits.c index fb2f39d..9f0e588 100644 --- a/tests/gem_render_tiled_blits.c +++ b/tests/gem_render_tiled_blits.c @@ -72,7 +72,7 @@ check_bo(struct intel_batchbuffer *batch, struct igt_buf *buf, uint32_t val) render_copy(batch, NULL, buf, 0, 0, WIDTH, HEIGHT, , 0, 0); if (snoop) { - do_or_die(dri_bo_map(linear, 0)); + do_or_die(drm_intel_bo_map(linear, 0)); ptr = linear->virtual; } else { do_or_die(drm_intel_bo_get_subdata(linear, 0, sizeof(data), data)); @@ -86,7 +86,7 @@ check_bo(struct intel_batchbuffer *batch, struct igt_buf *buf, uint32_t val) val++; } if (ptr != data) - dri_bo_unmap(linear); + drm_intel_bo_unmap(linear); } static void run_test (int fd, int count) -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC i-g-t 4/9] demos/Makefile: Don't build tools that depend on libdrm_intel.
From: Robert FossUse the HAS_INTEL automake flag to avoid building tools that won't compile unless libdrm_intel is available in the build system. Signed-off-by: Robert Foss --- demos/Makefile.am | 5 ++--- demos/Makefile.sources | 7 +++ 2 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 demos/Makefile.sources diff --git a/demos/Makefile.am b/demos/Makefile.am index e6fbb3b..02409fe 100644 --- a/demos/Makefile.am +++ b/demos/Makefile.am @@ -1,6 +1,5 @@ -bin_PROGRAMS = \ - intel_sprite_on \ - $(NULL) + +include Makefile.sources AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) $(LIBUNWIND_CFLAGS) diff --git a/demos/Makefile.sources b/demos/Makefile.sources new file mode 100644 index 000..98da30d --- /dev/null +++ b/demos/Makefile.sources @@ -0,0 +1,7 @@ +bin_PROGRAMS = \ + $(NULL) + +if HAVE_INTEL + bin_PROGRAMS += \ + intel_sprite_on \ +endif -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC i-g-t 3/9] tools/Makefile: Don't build tools that depend on libdrm_intel.
From: Robert FossUse the HAS_INTEL automake flag to avoid building tools that won't compile unless libdrm_intel is available in the build system. Signed-off-by: Robert Foss --- tools/Makefile.sources | 50 +- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/tools/Makefile.sources b/tools/Makefile.sources index 5d5958d..c2dab8e 100644 --- a/tools/Makefile.sources +++ b/tools/Makefile.sources @@ -1,42 +1,54 @@ -noinst_PROGRAMS = \ - hsw_compute_wrpll \ - skl_compute_wrpll \ - skl_ddb_allocation \ +noinst_PROGRAMS = \ + hsw_compute_wrpll \ + skl_compute_wrpll \ + skl_ddb_allocation \ $(NULL) -bin_PROGRAMS = \ +bin_PROGRAMS = \ igt_stats \ - intel_audio_dump\ + intel_audio_dump\ intel_reg \ intel_backlight \ intel_bios_dumper \ intel_bios_reader \ intel_display_crc \ intel_display_poller\ - intel_dump_decode \ - intel_error_decode \ intel_forcewaked\ intel_gpu_frequency \ - intel_framebuffer_dump \ intel_firmware_decode \ - intel_gpu_time \ - intel_gpu_top \ - intel_gtt \ + intel_gpu_time \ + intel_gpu_top \ + intel_gtt \ intel_infoframes\ intel_l3_parity \ intel_lid \ intel_opregion_decode \ intel_panel_fitter \ - intel_perf_counters \ - intel_reg_checker \ + intel_reg_checker \ intel_residency \ - intel_stepping \ + intel_stepping \ intel_watermark dist_bin_SCRIPTS = intel_gpu_abrt -intel_dump_decode_SOURCES =\ - intel_dump_decode.c +if HAVE_INTEL + bin_PROGRAMS += \ + intel_dump_decode \ + intel_error_decode \ + intel_framebuffer_dump \ + intel_perf_counters \ + $(NULL) + + intel_dump_decode_SOURCES = \ + intel_dump_decode.c \ + $(NULL) + + intel_error_decode_SOURCES =\ + intel_error_decode.c\ + $(NULL) + + intel_error_decode_LDFLAGS = -lz +endif intel_reg_SOURCES =\ intel_reg.c \ @@ -44,10 +56,6 @@ intel_reg_SOURCES = \ intel_reg_spec.c\ intel_reg_spec.h -intel_error_decode_SOURCES = \ - intel_error_decode.c -intel_error_decode_LDFLAGS = -lz - intel_bios_reader_SOURCES =\ intel_bios_reader.c \ intel_bios.h -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC i-g-t 2/9] benchmarks/Makefile: Don't build benchmarks that depend on libdrm_intel.
From: Robert FossUse the HAS_INTEL automake flag to avoid building benchmarks that won't compile unless libdrm_intel is available in the build system. Signed-off-by: Robert Foss --- benchmarks/Makefile.sources | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/benchmarks/Makefile.sources b/benchmarks/Makefile.sources index 81607a5..26ee3ea 100644 --- a/benchmarks/Makefile.sources +++ b/benchmarks/Makefile.sources @@ -1,10 +1,6 @@ benchmarksdir=$(libexecdir)/intel-gpu-tools/benchmarks benchmarks_PROGRAMS = \ - intel_upload_blit_large \ - intel_upload_blit_large_gtt \ - intel_upload_blit_large_map \ - intel_upload_blit_small \ gem_blt \ gem_create \ gem_exec_ctx\ @@ -16,6 +12,15 @@ benchmarks_PROGRAMS =\ gem_prw \ gem_set_domain \ gem_syslatency \ - gem_userptr_benchmark \ kms_vblank \ $(NULL) + +if HAVE_INTEL + benchmarks_PROGRAMS += \ + intel_upload_blit_large \ + intel_upload_blit_large_gtt \ + intel_upload_blit_large_map \ + intel_upload_blit_small \ + gem_userptr_benchmark \ + $(NULL) +endif -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC i-g-t 1/9] configure.ac: Test for libdrm_intel and build for it if present.
From: Robert FossTest for libdrm_intel and build for it if present. Also expose the HAVE_INTEL #define to allow code to be conditionally compiled. Signed-off-by: Robert Foss --- configure.ac | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 0589782..b6fc168 100644 --- a/configure.ac +++ b/configure.ac @@ -100,7 +100,7 @@ if test "x$GCC" = "xyes"; then fi AC_SUBST(ASSEMBLER_WARN_CFLAGS) -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm]) +PKG_CHECK_MODULES(DRM, [libdrm]) PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10]) case "$target_cpu" in @@ -150,6 +150,18 @@ PKG_CHECK_MODULES(GLIB, glib-2.0) # - # Configuration options # - +AC_ARG_ENABLE(intel, AS_HELP_STRING([--disable-intel], + [Enable building of intel specific parts (default: auto)]), + [INTEL=$enableval], [INTEL=auto]) +if test "x$INTEL" = xauto; then + PKG_CHECK_EXISTS([libdrm_intel >= 2.4.64], [INTEL=yes], [INTEL=no]) +fi +if test "x$INTEL" = xyes; then + PKG_CHECK_MODULES(DRM_INTEL, [libdrm_intel >= 2.4.64]) + AC_DEFINE(HAVE_INTEL, 1, [Have intel support]) +fi +AM_CONDITIONAL(HAVE_INTEL, [test "x$INTEL" = xyes]) + # for dma-buf tests AC_ARG_ENABLE(nouveau, AS_HELP_STRING([--disable-nouveau], [Enable use of nouveau API for prime tests (default: auto)]), -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/ilk: Wait one vblank before enabling audio
We no longer call ilk_audio_codec_enable() while we have vblanks disabled. As such, we can finally fix this and stop the occasional pipe underruns I'm seeing on this Dell OptiPlex 990. Cc: sta...@vger.kernel.org Signed-off-by: Lyude--- drivers/gpu/drm/i915/intel_audio.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 7d281b4..0d685fe 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -423,12 +423,8 @@ static void ilk_audio_codec_enable(struct drm_connector *connector, if (WARN_ON(port == PORT_A)) return; - /* -* FIXME: We're supposed to wait for vblank here, but we have vblanks -* disabled during the mode set. The proper fix would be to push the -* rest of the setup into a vblank work item, queued here, but the -* infrastructure is not there yet. -*/ + /* Need to wait one vblank before enabling audio */ + intel_wait_for_vblank(connector->dev, pipe); if (HAS_PCH_IBX(connector->dev)) { hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID(pipe); -- 2.5.5 ___ 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/gem_{ctx, exec}_create: Adjust timeout values for BAT
On Fri, May 20, 2016 at 08:02:46PM +0300, Marius Vlad wrote: > Tune down from 20s to 2s. Add the old timeout values under extended tests. Does it fail with the new timeout? If not, increase it. This test should be a fail on current kernels. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ 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/gem_exec_gttfill: Fix require memory assertion and tune down the timeout test for BAT.
On Fri, May 20, 2016 at 08:02:16PM +0300, Marius Vlad wrote: > There's no need to multiply the number of batches with the number of > engines as intel_require_memory() already compares against the aperture > size (count * BATCH_SIZE). nengines was because I first planned to allocate an array for each engine, because I wanted each to run as independently as possible (since we overwrite the batch if it is active, we will stall). Then I realised the futility and overallocation since we only have the single address space and so allocated the single array and set each engine off on a random order to hopefully avoid them lockstepping. > This also removes the weird assertion messages where we need > bogus amounts of RAM. It won't. However you put it, we need 256 exabytes to run the test on bdw+. Unless we can constrain the per-context GTT (i.e. a constrction flag to limit the address space to 32bits) Or we just don't use the allow-48bit flag and pretend that is equivalent. > Also tune down the timeout from from 10s to 2s to speed up BAT. It's not going to make any difference on the slowest machines, but meh. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix NULL pointer deference when out of PLLs in IVB
On Fri, May 20, 2016 at 03:47:06PM +0300, Ander Conselvan de Oliveira wrote: > In commit f9476a6c6d0c ("drm/i915: Refactor platform specifics out of > intel_get_shared_dpll()"), the ibx_get_dpll() function lacked an error > check, that can lead to a NULL pointer dereference when trying to enable > pipe C. s/pipe C/three pipes/ > > BUG: unable to handle kernel NULL pointer dereference at 0068 > IP: [] intel_reference_shared_dpll+0x15/0x100 [i915] > PGD cec87067 PUD d30ce067 PMD 0 > Oops: [#1] PREEMPT SMP > Modules linked in: snd_hda_intel i915 drm_kms_helper drm intel_gtt > sch_fq_codel cfg80211 binfmt_misc i2c_algo_bit cfbfillrect syscopyarea > cfbimgblt sysfillrect sysimgblt fb_sys_fops cfbcopyarea intel_rapl iosf_mbi > x86_pkg_temp_thermal coretemp agpgart kvm_intel snd_hda_codec_hdmi kvm > iTCO_wdt snd_hda_codec_realtek snd_hda_codec_generic irqbypass aesni_intel > aes_x86_64 glue_helper lrw gf128mul ablk_helper cryptd psmouse pcspkr > snd_hda_codec i2c_i801 snd_hwdep snd_hda_core snd_pcm snd_timer lpc_ich > mfd_core snd soundcore wmi evdev tpm_tis tpm [last unloaded: drm] > CPU: 3 PID: 5810 Comm: kms_flip Tainted: G U W 4.6.0-test+ #3 > Hardware name: /DZ77BH-55K, BIOS > BHZ7710H.86A.0100.2013.0517.0942 05/17/2013 > task: 8800d3908040 ti: 8801166c8000 task.ti: 8801166c8000 > RIP: 0010:[] [] > intel_reference_shared_dpll+0x15/0x100 [i915] > RSP: 0018:8801166cba60 EFLAGS: 00010246 > RAX: RBX: RCX: 0002 > RDX: 0001 RSI: 8800d07f1bf8 RDI: > RBP: 8801166cba88 R08: 0002 R09: 8800d32e5698 > R10: 0001 R11: 8800cc89ac88 R12: 8800d07f1bf8 > R13: R14: R15: > FS: 7f4c3fc8d8c0() GS:88011bcc() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: 0068 CR3: d3b4c000 CR4: 001406e0 > Stack: > 8800d07f1bf8 8800d04c > 8801166cbaa8 a04823a7 8800d07f1bf8 > 8800d32e5698 8801166cbab8 a04840cf 8801166cbaf0 > Call Trace: > [] ibx_get_dpll+0x47/0xa0 [i915] > [] intel_get_shared_dpll+0x1f/0x50 [i915] > [] ironlake_crtc_compute_clock+0x280/0x430 [i915] > [] intel_crtc_atomic_check+0x240/0x320 [i915] > [] drm_atomic_helper_check_planes+0x14e/0x1d0 > [drm_kms_helper] > [] intel_atomic_check+0x5dc/0x1110 [i915] > [] drm_atomic_check_only+0x14a/0x660 [drm] > [] ? drm_atomic_set_crtc_for_connector+0x96/0x100 [drm] > [] drm_atomic_commit+0x17/0x60 [drm] > [] restore_fbdev_mode+0x237/0x260 [drm_kms_helper] > [] ? drm_modeset_lock_all_ctx+0x9a/0xb0 [drm] > [] drm_fb_helper_restore_fbdev_mode_unlocked+0x33/0x80 > [drm_kms_helper] > [] drm_fb_helper_set_par+0x2d/0x50 [drm_kms_helper] > [] drm_fb_helper_hotplug_event+0xaa/0xf0 [drm_kms_helper] > [] drm_fb_helper_restore_fbdev_mode_unlocked+0x56/0x80 > [drm_kms_helper] > [] intel_fbdev_restore_mode+0x22/0x80 [i915] > [] i915_driver_lastclose+0xe/0x20 [i915] > [] drm_lastclose+0x2e/0x130 [drm] > [] drm_release+0x2ac/0x4b0 [drm] > [] __fput+0xed/0x1f0 > [] fput+0xe/0x10 > [] task_work_run+0x76/0xb0 > [] do_exit+0x3ab/0xc60 > [] ? trace_hardirqs_on_caller+0x12f/0x1c0 > [] do_group_exit+0x4e/0xc0 > [] SyS_exit_group+0x14/0x20 > [] entry_SYSCALL_64_fastpath+0x18/0xa8 > Code: 14 80 48 8d 34 90 b8 01 00 00 00 d3 e0 09 04 b3 5b 41 5c 5d c3 90 0f 1f > 44 00 00 55 48 89 e5 41 57 41 56 49 89 fe 41 55 41 54 53 <44> 8b 67 68 48 89 > f3 48 8b be 08 02 00 00 4c 8b 2e e8 15 9d fd > RIP [] intel_reference_shared_dpll+0x15/0x100 [i915] > RSP > CR2: 0068 > > Cc: Ville Syrjälä> Reported-by: Ville Syrjälä > Fixes: f9476a6c6d0c ("drm/i915: Refactor platform specifics out of > intel_get_shared_dpll()") > Signed-off-by: Ander Conselvan de Oliveira > Reviewed-by: Ville Syrjälä Tested-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_dpll_mgr.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c > b/drivers/gpu/drm/i915/intel_dpll_mgr.c > index f988adb..1e3d091 100644 > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > @@ -366,6 +366,9 @@ ibx_get_dpll(struct intel_crtc *crtc, struct > intel_crtc_state *crtc_state, >DPLL_ID_PCH_PLL_B); > } > > + if (!pll) > + return NULL; > + > /* reference the pll */ > intel_reference_shared_dpll(pll, crtc_state); > > -- > 2.5.5 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org
[Intel-gfx] [PATCH i-g-t] tests/gem_{ctx, exec}_create: Adjust timeout values for BAT
Tune down from 20s to 2s. Add the old timeout values under extended tests. Signed-off-by: Marius Vlad--- tests/gem_ctx_create.c | 2 ++ tests/gem_exec_create.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/tests/gem_ctx_create.c b/tests/gem_ctx_create.c index 0bdd408..ef3bdda 100644 --- a/tests/gem_ctx_create.c +++ b/tests/gem_ctx_create.c @@ -181,6 +181,8 @@ igt_main } igt_subtest("basic-files") + files(fd, 2, 1); + igt_subtest("extended-files") files(fd, 20, 1); igt_subtest("forked-files") files(fd, 20, ncpus); diff --git a/tests/gem_exec_create.c b/tests/gem_exec_create.c index 33fc85c..f77c206 100644 --- a/tests/gem_exec_create.c +++ b/tests/gem_exec_create.c @@ -154,6 +154,8 @@ igt_main igt_fork_hang_detector(device); igt_subtest("basic") + all(device, 0, 2, 1); + igt_subtest("extended") all(device, 0, 20, 1); igt_subtest("forked") all(device, 0, 20, ncpus); -- 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] tests/gem_exec_gttfill: Fix require memory assertion and tune down the timeout test for BAT.
There's no need to multiply the number of batches with the number of engines as intel_require_memory() already compares against the aperture size (count * BATCH_SIZE). This also removes the weird assertion messages where we need bogus amounts of RAM. Also tune down the timeout from from 10s to 2s to speed up BAT. CC: Chris WilsonSigned-off-by: Marius Vlad --- tests/gem_exec_gttfill.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/gem_exec_gttfill.c b/tests/gem_exec_gttfill.c index 5921923..4778e4d 100644 --- a/tests/gem_exec_gttfill.c +++ b/tests/gem_exec_gttfill.c @@ -144,7 +144,7 @@ static void fillgtt(int fd, unsigned ring, int timeout) count = size / BATCH_SIZE + 1; igt_debug("Using %'d batches to fill %'llu aperture on %d engines\n", count, (long long)size, nengine); - intel_require_memory(count * nengine, BATCH_SIZE, CHECK_RAM); + intel_require_memory(count, BATCH_SIZE, CHECK_RAM); memset(, 0, sizeof(execbuf)); execbuf.buffer_count = 1; @@ -195,7 +195,7 @@ igt_main igt_fork_hang_detector(device); igt_subtest("basic") - fillgtt(device, 0, 10); + fillgtt(device, 0, 2); for (e = intel_execution_engines; e->name; e++) igt_subtest_f("%s", e->name) -- 2.5.0 ___ 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/drv_module_reload_basic: Don't use rmmod exit code when reloading the module.
On pe, 2016-05-20 at 17:23 +0100, Chris Wilson wrote: > On Fri, May 20, 2016 at 07:00:18PM +0300, Imre Deak wrote: > > On pe, 2016-05-20 at 18:20 +0300, Marius Vlad wrote: > > > Either we return $IGT_EXIT_FAILURE or remove it entirely (like in > > > this > > > patch). If rmmod returns non-zero (i.e., Module: i915 is still in > > > use), reload > > > will bail with $IGT_EXIT_SKIP, making the check with lsmod > > > useless. > > > Also use the return value in the fault-injection loop. > > > > > > Signed-off-by: Marius Vlad> > > --- > > > tests/drv_module_reload_basic | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/tests/drv_module_reload_basic > > > b/tests/drv_module_reload_basic > > > index 3bba796..3a8df33 100755 > > > --- a/tests/drv_module_reload_basic > > > +++ b/tests/drv_module_reload_basic > > > @@ -30,7 +30,7 @@ function reload() { > > > > > > #ignore errors in ips - gen5 only > > > rmmod intel_ips &> /dev/null > > > - rmmod i915 || return $IGT_EXIT_SKIP > > > + rmmod i915 > > > > Not sure what was the reason to bail out here, continuing seems > > like > > the correct thing to do. > > If we can't unload, we can perform the modprobe testing. The system > is > not in a state suitable for testing so skip or fail. If we are > certain > that the rmmod failure is a bug, fail, if it merely something like > the > system doesn't support module unloading, skip. > > Continuing on after failure to unload is not a good idea. I meant continuing here and depending on the lsmod check later to exit. > -Chris > ___ 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/drv_module_reload_basic: Don't use rmmod exit code when reloading the module.
On Fri, May 20, 2016 at 07:00:18PM +0300, Imre Deak wrote: > On pe, 2016-05-20 at 18:20 +0300, Marius Vlad wrote: > > Either we return $IGT_EXIT_FAILURE or remove it entirely (like in > > this > > patch). If rmmod returns non-zero (i.e., Module: i915 is still in > > use), reload > > will bail with $IGT_EXIT_SKIP, making the check with lsmod useless. > > Also use the return value in the fault-injection loop. > > > > Signed-off-by: Marius Vlad> > --- > > tests/drv_module_reload_basic | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/tests/drv_module_reload_basic > > b/tests/drv_module_reload_basic > > index 3bba796..3a8df33 100755 > > --- a/tests/drv_module_reload_basic > > +++ b/tests/drv_module_reload_basic > > @@ -30,7 +30,7 @@ function reload() { > > > > #ignore errors in ips - gen5 only > > rmmod intel_ips &> /dev/null > > - rmmod i915 || return $IGT_EXIT_SKIP > > + rmmod i915 > > Not sure what was the reason to bail out here, continuing seems like > the correct thing to do. If we can't unload, we can perform the modprobe testing. The system is not in a state suitable for testing so skip or fail. If we are certain that the rmmod failure is a bug, fail, if it merely something like the system doesn't support module unloading, skip. Continuing on after failure to unload is not a good idea. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ 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/drv_module_reload_basic: Don't use rmmod exit code when reloading the module.
On pe, 2016-05-20 at 18:20 +0300, Marius Vlad wrote: > Either we return $IGT_EXIT_FAILURE or remove it entirely (like in > this > patch). If rmmod returns non-zero (i.e., Module: i915 is still in > use), reload > will bail with $IGT_EXIT_SKIP, making the check with lsmod useless. > Also use the return value in the fault-injection loop. > > Signed-off-by: Marius Vlad> --- > tests/drv_module_reload_basic | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/drv_module_reload_basic > b/tests/drv_module_reload_basic > index 3bba796..3a8df33 100755 > --- a/tests/drv_module_reload_basic > +++ b/tests/drv_module_reload_basic > @@ -30,7 +30,7 @@ function reload() { > > #ignore errors in ips - gen5 only > rmmod intel_ips &> /dev/null > - rmmod i915 || return $IGT_EXIT_SKIP > + rmmod i915 Not sure what was the reason to bail out here, continuing seems like the correct thing to do. > #ignore errors in intel-gtt, often built-in > rmmod intel-gtt &> /dev/null > # drm may be used by other devices (nouveau, radeon, udl, > etc) > @@ -76,7 +76,7 @@ finish_load || exit $? > > # Repeat the module reload trying to to generate faults > for i in $(seq 1 4); do > - reload inject_load_failure=$i > + reload inject_load_failure=$i || exit $? The idea was to keep the system in a working state even in case of failure here, so I'd still attempt a normal reload before exiting with failure. --Imre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Extend Fake HDMI live status to Valley View
On Atom E38xx based boards from two different manufacturers we have encountered the HDMI display being disabled early during the boot process. Extending the Fake HDMI live status to include Valley View chipsets fixes this problem on both boards. Signed-off-by: James Stafford--- drivers/gpu/drm/i915/intel_hdmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 6b52c6a..358f9b2 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1507,7 +1507,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) * So consider live_status only for certain platforms, for * others, read EDID to determine presence of sink. */ -if (INTEL_INFO(dev_priv)->gen < 7 || IS_IVYBRIDGE(dev_priv)) +if (INTEL_INFO(dev_priv)->gen < 7 || IS_IVYBRIDGE(dev_priv) || IS_VALLEYVIEW(dev_priv)) live_status = true; } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Ro.CI.BAT: failure for series starting with [1/4] drm/i915: Introduce intel_release_shared_dpll()
== Series Details == Series: series starting with [1/4] drm/i915: Introduce intel_release_shared_dpll() URL : https://patchwork.freedesktop.org/series/7467/ State : failure == Summary == Series 7467v1 Series without cover letter http://patchwork.freedesktop.org/api/1.0/series/7467/revisions/1/mbox Test drv_module_reload_basic: pass -> DMESG-WARN (ro-ilk1-i5-650) Test gem_exec_flush: Subgroup basic-batch-kernel-default-cmd: pass -> FAIL (fi-byt-n2820) Test kms_force_connector_basic: Subgroup force-load-detect: pass -> DMESG-WARN (ro-ilk1-i5-650) Test kms_frontbuffer_tracking: Subgroup basic: fail -> PASS (ro-bdw-i7-5600u) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: pass -> INCOMPLETE (fi-hsw-i7-4770k) fi-bdw-i7-5557u total:217 pass:204 dwarn:0 dfail:0 fail:0 skip:13 fi-byt-n2820 total:216 pass:172 dwarn:0 dfail:0 fail:3 skip:41 fi-hsw-i7-4770k total:192 pass:171 dwarn:0 dfail:0 fail:0 skip:20 fi-hsw-i7-4770r total:217 pass:191 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-i7-6700k total:217 pass:189 dwarn:0 dfail:0 fail:0 skip:28 ro-bdw-i5-5250u total:209 pass:171 dwarn:0 dfail:0 fail:0 skip:38 ro-bdw-i7-5557U total:209 pass:196 dwarn:0 dfail:0 fail:0 skip:13 ro-bdw-i7-5600u total:209 pass:180 dwarn:0 dfail:0 fail:0 skip:29 ro-bsw-n3050 total:209 pass:167 dwarn:0 dfail:0 fail:2 skip:40 ro-byt-n2820 total:209 pass:168 dwarn:0 dfail:0 fail:3 skip:38 ro-hsw-i3-4010u total:209 pass:186 dwarn:0 dfail:0 fail:0 skip:23 ro-hsw-i7-4770r total:209 pass:185 dwarn:0 dfail:0 fail:0 skip:24 ro-ilk-i7-620lm total:209 pass:145 dwarn:0 dfail:0 fail:1 skip:63 ro-ilk1-i5-650 total:204 pass:144 dwarn:2 dfail:0 fail:1 skip:57 ro-ivb-i7-3770 total:209 pass:176 dwarn:0 dfail:0 fail:0 skip:33 ro-ivb2-i7-3770 total:209 pass:180 dwarn:0 dfail:0 fail:0 skip:29 ro-skl-i7-6700hq total:204 pass:182 dwarn:0 dfail:0 fail:0 skip:22 ro-snb-i7-2620M total:209 pass:170 dwarn:0 dfail:0 fail:1 skip:38 fi-bsw-n3050 failed to connect after reboot Results at /archive/results/CI_IGT_test/RO_Patchwork_952/ f1eaed1 drm-intel-nightly: 2016y-05m-20d-14h-35m-29s UTC integration manifest c1979b0 drm/i915: Update kerneldoc for intel_dpll_mgr.c 9898545 drm/i915: Rename intel_shared_dpll->mode_set() to prepare() ba5fc89 drm/i915: Rename intel_shared_dpll_commit() to _swap_state() 23c857c drm/i915: Introduce intel_release_shared_dpll() ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] tests/drv_module_reload_basic: Don't use rmmod exit code when reloading the module.
Either we return $IGT_EXIT_FAILURE or remove it entirely (like in this patch). If rmmod returns non-zero (i.e., Module: i915 is still in use), reload will bail with $IGT_EXIT_SKIP, making the check with lsmod useless. Also use the return value in the fault-injection loop. Signed-off-by: Marius Vlad--- tests/drv_module_reload_basic | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/drv_module_reload_basic b/tests/drv_module_reload_basic index 3bba796..3a8df33 100755 --- a/tests/drv_module_reload_basic +++ b/tests/drv_module_reload_basic @@ -30,7 +30,7 @@ function reload() { #ignore errors in ips - gen5 only rmmod intel_ips &> /dev/null - rmmod i915 || return $IGT_EXIT_SKIP + rmmod i915 #ignore errors in intel-gtt, often built-in rmmod intel-gtt &> /dev/null # drm may be used by other devices (nouveau, radeon, udl, etc) @@ -76,7 +76,7 @@ finish_load || exit $? # Repeat the module reload trying to to generate faults for i in $(seq 1 4); do - reload inject_load_failure=$i + reload inject_load_failure=$i || exit $? done reload || exit $? -- 2.5.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: Update kerneldoc for intel_dpll_mgr.c
The documentation for most of the non-static members and structs were missing. Fix that. Cc: Daniel VetterSigned-off-by: Ander Conselvan de Oliveira --- Documentation/DocBook/gpu.tmpl| 7 +++ drivers/gpu/drm/i915/intel_dpll_mgr.c | 88 +-- drivers/gpu/drm/i915/intel_dpll_mgr.h | 83 + 3 files changed, 163 insertions(+), 15 deletions(-) diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl index 7586bf75..b96875d 100644 --- a/Documentation/DocBook/gpu.tmpl +++ b/Documentation/DocBook/gpu.tmpl @@ -3355,6 +3355,13 @@ int num_ioctls; !Idrivers/gpu/drm/i915/intel_bios.c !Idrivers/gpu/drm/i915/intel_vbt_defs.h + + + Display PLLs +!Pdrivers/gpu/drm/i915/intel_dpll_mgr.c Display PLLs +!Idrivers/gpu/drm/i915/intel_dpll_mgr.c +!Idrivers/gpu/drm/i915/intel_dpll_mgr.h + diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c index 26c9955..bcfe7a9 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -23,6 +23,33 @@ #include "intel_drv.h" +/** + * DOC: Display PLLs + * + * Display PLLs used for driving outputs vary by platform. While some have + * per-pipe or per-encoder dedicated PLLs, others allow the use of any PLL + * from a pool. In the latter scenario, it is possible that multiple pipes + * share a PLL if their configurations match. + * + * This file provides an abstraction over display PLLs. The function + * intel_shared_dpll_init() initializes the PLLs for the given platform. The + * users of a PLL are tracked and that tracking is integrated with the atomic + * modest interface. During an atomic operation, a PLL can be requested for a + * given crtc and encoder configuration by calling intel_get_shared_dpll() and + * a previously used PLL can be released with intel_release_shared_dpll(). + * Changes to the users are first staged in the atomic state, and then made + * effective by calling intel_shared_dpll_swap_state() during the atomic + * commit phase. + */ + +/** + * intel_get_shared_dpll_by_id - get a DPLL given its id + * @dev_priv: i915 device instance + * @id: pll id + * + * Returns: + * A pointer the DPLL with @id + */ struct intel_shared_dpll * intel_get_shared_dpll_by_id(struct drm_i915_private *dev_priv, enum intel_dpll_id id) @@ -30,6 +57,14 @@ intel_get_shared_dpll_by_id(struct drm_i915_private *dev_priv, return _priv->shared_dplls[id]; } +/** + * intel_get_shared_dpll_id - get the id of a DPLL + * @dev_priv: i915 device instance + * @pll: the DPLL + * + * Returns: + * The id of @pll + */ enum intel_dpll_id intel_get_shared_dpll_id(struct drm_i915_private *dev_priv, struct intel_shared_dpll *pll) @@ -58,6 +93,13 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv, pll->name, onoff(state), onoff(cur_state)); } +/** + * intel_prepare_shared_dpll - call a dpll's prepare hook + * @crtc: crtc which has a shared dpll + * + * This calls the PLL's prepare hook if it has one and if the PLL is not + * already enabled. The prepare hook is platform specific. + */ void intel_prepare_shared_dpll(struct intel_crtc *crtc) { struct drm_device *dev = crtc->base.dev; @@ -80,12 +122,10 @@ void intel_prepare_shared_dpll(struct intel_crtc *crtc) } /** - * intel_enable_shared_dpll - enable PCH PLL - * @dev_priv: i915 private structure - * @pipe: pipe PLL to enable + * intel_enable_shared_dpll - enable a crtc's shared DPLL + * @crtc: crtc which has a shared DPLL * - * The PCH PLL needs to be enabled before the PCH transcoder, since it - * drives the transcoder clock. + * Enable the shared DPLL used by @crtc. */ void intel_enable_shared_dpll(struct intel_crtc *crtc) { @@ -126,6 +166,12 @@ out: mutex_unlock(_priv->dpll_lock); } +/** + * intel_disable_shared_dpll - disable a crtc's shared DPLL + * @crtc: crtc which has a shared DPLL + * + * Disable the shared DPLL used by @crtc. + */ void intel_disable_shared_dpll(struct intel_crtc *crtc) { struct drm_device *dev = crtc->base.dev; @@ -228,6 +274,16 @@ intel_reference_shared_dpll(struct intel_shared_dpll *pll, shared_dpll[pll->id].crtc_mask |= 1 << crtc->pipe; } +/** + * intel_shared_dpll_swap_state - make atomic DPLL configuration effective + * @state: atomic state + * + * This is the dpll version of drm_atomic_helper_swap_state() since the + * helper does not handle driver-specific global state. + * + * Note thar this doesn't actually swap states, the dpll configutation in + * @state is left unchanged. + */ void intel_shared_dpll_swap_state(struct drm_atomic_state *state) { struct drm_i915_private *dev_priv = to_i915(state->dev); @@ -1706,6 +1762,12 @@ static const struct intel_dpll_mgr bxt_pll_mgr = { .get_dpll =
[Intel-gfx] [PATCH 3/4] drm/i915: Rename intel_shared_dpll->mode_set() to prepare()
The hook is called from intel_prepare_shared_dpll(). The name doesn't make sense after all the changes to modeset code. So just call it prepare. Signed-off-by: Ander Conselvan de Oliveira--- drivers/gpu/drm/i915/intel_dpll_mgr.c | 8 drivers/gpu/drm/i915/intel_dpll_mgr.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c index 27bbc46..26c9955 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -74,7 +74,7 @@ void intel_prepare_shared_dpll(struct intel_crtc *crtc) WARN_ON(pll->on); assert_shared_dpll_disabled(dev_priv, pll); - pll->funcs.mode_set(dev_priv, pll); + pll->funcs.prepare(dev_priv, pll); } mutex_unlock(_priv->dpll_lock); } @@ -264,8 +264,8 @@ static bool ibx_pch_dpll_get_hw_state(struct drm_i915_private *dev_priv, return val & DPLL_VCO_ENABLE; } -static void ibx_pch_dpll_mode_set(struct drm_i915_private *dev_priv, - struct intel_shared_dpll *pll) +static void ibx_pch_dpll_prepare(struct drm_i915_private *dev_priv, +struct intel_shared_dpll *pll) { I915_WRITE(PCH_FP0(pll->id), pll->config.hw_state.fp0); I915_WRITE(PCH_FP1(pll->id), pll->config.hw_state.fp1); @@ -354,7 +354,7 @@ ibx_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, } static const struct intel_shared_dpll_funcs ibx_pch_dpll_funcs = { - .mode_set = ibx_pch_dpll_mode_set, + .prepare = ibx_pch_dpll_prepare, .enable = ibx_pch_dpll_enable, .disable = ibx_pch_dpll_disable, .get_hw_state = ibx_pch_dpll_get_hw_state, diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h index 929ff7d..087e6b1 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h @@ -101,7 +101,7 @@ struct intel_shared_dpll_config { struct intel_shared_dpll_funcs { /* The mode_set hook is optional and should be used together with the * intel_prepare_shared_dpll function. */ - void (*mode_set)(struct drm_i915_private *dev_priv, + void (*prepare)(struct drm_i915_private *dev_priv, struct intel_shared_dpll *pll); void (*enable)(struct drm_i915_private *dev_priv, struct intel_shared_dpll *pll); -- 2.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: Rename intel_shared_dpll_commit() to _swap_state()
The function intel_shared_dpll_commit() performs the equivalent of drm_atomic_helper_swap_state() for the shared dpll state, which id not handled by the helpers. So rename it for consistency. Signed-off-by: Ander Conselvan de Oliveira--- drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_dpll_mgr.c | 2 +- drivers/gpu/drm/i915/intel_dpll_mgr.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index fe9b00c..dbb982aa 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13006,7 +13006,7 @@ static int intel_atomic_commit(struct drm_device *dev, drm_atomic_helper_swap_state(dev, state); dev_priv->wm.distrust_bios_wm = false; dev_priv->wm.skl_results = intel_state->wm_results; - intel_shared_dpll_commit(state); + intel_shared_dpll_swap_state(state); if (intel_state->modeset) { memcpy(dev_priv->min_pixclk, intel_state->min_pixclk, diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c index a3293cf..27bbc46 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -228,7 +228,7 @@ intel_reference_shared_dpll(struct intel_shared_dpll *pll, shared_dpll[pll->id].crtc_mask |= 1 << crtc->pipe; } -void intel_shared_dpll_commit(struct drm_atomic_state *state) +void intel_shared_dpll_swap_state(struct drm_atomic_state *state) { struct drm_i915_private *dev_priv = to_i915(state->dev); struct intel_shared_dpll_config *shared_dpll; diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h index b93eed8..929ff7d 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h @@ -151,7 +151,7 @@ void intel_release_shared_dpll(struct intel_crtc *crtc, void intel_prepare_shared_dpll(struct intel_crtc *crtc); void intel_enable_shared_dpll(struct intel_crtc *crtc); void intel_disable_shared_dpll(struct intel_crtc *crtc); -void intel_shared_dpll_commit(struct drm_atomic_state *state); +void intel_shared_dpll_swap_state(struct drm_atomic_state *state); void intel_shared_dpll_init(struct drm_device *dev); -- 2.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915: Introduce intel_release_shared_dpll()
While the details of getting a shared dpll are wrapped by intel_get_shared_dpll(), the release was still hand rolled into the modeset code. Fix that by creating an entry point for releasing the pll and move that code there. Signed-off-by: Ander Conselvan de Oliveira--- drivers/gpu/drm/i915/intel_display.c | 13 ++ drivers/gpu/drm/i915/intel_dpll_mgr.c | 45 +-- drivers/gpu/drm/i915/intel_dpll_mgr.h | 10 ++-- 3 files changed, 26 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a500f08..fe9b00c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12419,7 +12419,6 @@ static void intel_modeset_clear_plls(struct drm_atomic_state *state) { struct drm_device *dev = state->dev; struct drm_i915_private *dev_priv = to_i915(dev); - struct intel_shared_dpll_config *shared_dpll = NULL; struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; int i; @@ -12429,21 +12428,13 @@ static void intel_modeset_clear_plls(struct drm_atomic_state *state) for_each_crtc_in_state(state, crtc, crtc_state, i) { struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct intel_shared_dpll *old_dpll = - to_intel_crtc_state(crtc->state)->shared_dpll; if (!needs_modeset(crtc_state)) continue; + intel_release_shared_dpll(intel_crtc, + to_intel_crtc_state(crtc_state)); to_intel_crtc_state(crtc_state)->shared_dpll = NULL; - - if (!old_dpll) - continue; - - if (!shared_dpll) - shared_dpll = intel_atomic_get_shared_dpll_state(state); - - intel_shared_dpll_config_put(shared_dpll, old_dpll, intel_crtc); } } diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c index 0aac3ec..a3293cf 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -41,28 +41,6 @@ intel_get_shared_dpll_id(struct drm_i915_private *dev_priv, return (enum intel_dpll_id) (pll - dev_priv->shared_dplls); } -void -intel_shared_dpll_config_get(struct intel_shared_dpll_config *config, -struct intel_shared_dpll *pll, -struct intel_crtc *crtc) -{ - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); - enum intel_dpll_id id = intel_get_shared_dpll_id(dev_priv, pll); - - config[id].crtc_mask |= 1 << crtc->pipe; -} - -void -intel_shared_dpll_config_put(struct intel_shared_dpll_config *config, -struct intel_shared_dpll *pll, -struct intel_crtc *crtc) -{ - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); - enum intel_dpll_id id = intel_get_shared_dpll_id(dev_priv, pll); - - config[id].crtc_mask &= ~(1 << crtc->pipe); -} - /* For ILK+ */ void assert_shared_dpll(struct drm_i915_private *dev_priv, struct intel_shared_dpll *pll, @@ -247,7 +225,7 @@ intel_reference_shared_dpll(struct intel_shared_dpll *pll, DRM_DEBUG_DRIVER("using %s for pipe %c\n", pll->name, pipe_name(crtc->pipe)); - intel_shared_dpll_config_get(shared_dpll, pll, crtc); + shared_dpll[pll->id].crtc_mask |= 1 << crtc->pipe; } void intel_shared_dpll_commit(struct drm_atomic_state *state) @@ -1784,3 +1762,24 @@ intel_get_shared_dpll(struct intel_crtc *crtc, return dpll_mgr->get_dpll(crtc, crtc_state, encoder); } + +/** + * intel_release_shared_dpll - releases a shared DPLL from a crtc atomic state + * @crtc: crtc + * @crtc_state: atomic stat for @crtc + * + */ +void intel_release_shared_dpll(struct intel_crtc *crtc, + struct intel_crtc_state *crtc_state) +{ + struct intel_shared_dpll_config *shared_dpll_config; + struct intel_shared_dpll *dpll = crtc_state->shared_dpll; + + if (!dpll) + return; + + shared_dpll_config = + intel_atomic_get_shared_dpll_state(crtc_state->base.state); + + shared_dpll_config[dpll->id].crtc_mask &= ~(1 << crtc->pipe); +} diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h index 89c5ada..b93eed8 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h @@ -138,14 +138,6 @@ intel_get_shared_dpll_by_id(struct drm_i915_private *dev_priv, enum intel_dpll_id intel_get_shared_dpll_id(struct drm_i915_private *dev_priv, struct intel_shared_dpll *pll); -void -intel_shared_dpll_config_get(struct intel_shared_dpll_config *config, -
Re: [Intel-gfx] [PATCH v6 3/7] drm/i915/guc: add enable_guc_loading parameter
Tested-by: Fiedorowicz, LukaszRun similar tests as for previous version of this patch. Results are the same and logs are cleaner. I'm happy with how it works. -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of Tvrtko Ursulin Sent: Friday, May 20, 2016 12:43 PM To: Intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] [PATCH v6 3/7] drm/i915/guc: add enable_guc_loading parameter From: Dave Gordon Split the function of "enable_guc_submission" into two separate options. The new one ("enable_guc_loading") controls only the *fetching and loading* of the GuC firmware image. The existing one is redefined to control only the *use* of the GuC for batch submission once the firmware is loaded. In addition, the degree of control has been refined from a simple bool to an integer key, allowing several options: -1 (default) whatever the platform default is 0 DISABLE don't load/use the GuC 1 BEST EFFORT try to load/use the GuC, fallback if not available 2 REQUIRE must load/use the GuC, else leave the GPU wedged The new platform default (as coded here) will be to attempt to load the GuC iff the device has a GuC that requires firmware, but not yet to use it for submission. A later patch will change to enable it if appropriate. v4: Changed some error-message levels, mostly ERROR->INFO, per review comments by Tvrtko Ursulin. v5: Dropped one more error message, disabled GuC submission on hypothetical firmware-free devices [Tvrtko Ursulin]. v6: Logging tidy by Tvrtko Ursulin: * Do not log falling back to execlists when wedging the GPU. * Do not log fw load errors when load was disabled by user. * Pass down some error code from fw load for log message to make more sense. Signed-off-by: Dave Gordon Cc: Tvrtko Ursulin Reviewed-by: Tvrtko Ursulin (v5) Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_gem.c| 5 +- drivers/gpu/drm/i915/i915_guc_submission.c | 4 +- drivers/gpu/drm/i915/i915_params.c | 14 +++- drivers/gpu/drm/i915/i915_params.h | 3 +- drivers/gpu/drm/i915/intel_guc_loader.c| 123 + 5 files changed, 89 insertions(+), 60 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 88dce5482f2f..1a3a07eca0d0 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4868,11 +4868,8 @@ i915_gem_init_hw(struct drm_device *dev) /* We can't enable contexts until all firmware is loaded */ if (HAS_GUC(dev)) { ret = intel_guc_setup(dev); - if (ret) { - DRM_ERROR("Failed to initialize GuC, error %d\n", ret); - ret = -EIO; + if (ret) goto out; - } } /* diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 169242a8adff..916cd6778cf3 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -969,7 +969,7 @@ int intel_guc_suspend(struct drm_device *dev) struct intel_context *ctx; u32 data[3]; - if (!i915.enable_guc_submission) + if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS) return 0; ctx = dev_priv->kernel_context; @@ -995,7 +995,7 @@ int intel_guc_resume(struct drm_device *dev) struct intel_context *ctx; u32 data[3]; - if (!i915.enable_guc_submission) + if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS) return 0; ctx = dev_priv->kernel_context; diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index cd74fb8e9387..21a323c01cdb 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -53,7 +53,8 @@ struct i915_params i915 __read_mostly = { .verbose_state_checks = 1, .nuclear_pageflip = 0, .edp_vswing = 0, - .enable_guc_submission = false, + .enable_guc_loading = -1, + .enable_guc_submission = 0, .guc_log_level = -1, .enable_dp_mst = true, .inject_load_failure = 0, @@ -193,8 +194,15 @@ MODULE_PARM_DESC(edp_vswing, "(0=use value from vbt [default], 1=low power swing(200mV)," "2=default swing(400mV))"); -module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, bool, 0400); -MODULE_PARM_DESC(enable_guc_submission, "Enable GuC submission (default:false)"); +module_param_named_unsafe(enable_guc_loading, i915.enable_guc_loading, +int, 0400); MODULE_PARM_DESC(enable_guc_loading, + "Enable GuC
Re: [Intel-gfx] ✗ Ro.CI.BAT: failure for vga_switcheroo: Add helper for deferred probing
On Thu, May 19, 2016 at 04:08:31PM -, Patchwork wrote: > == Series Details == > > Series: vga_switcheroo: Add helper for deferred probing > URL : https://patchwork.freedesktop.org/series/7409/ > State : failure > > == Summary == > > Series 7409v1 vga_switcheroo: Add helper for deferred probing > http://patchwork.freedesktop.org/api/1.0/series/7409/revisions/1/mbox > > Test drv_hangman: > Subgroup error-state-basic: > fail -> PASS (ro-ilk1-i5-650) > Test drv_module_reload_basic: > dmesg-warn -> PASS (fi-hsw-i7-4770r) > Test kms_flip: > Subgroup basic-flip-vs-wf_vblank: > fail -> PASS (ro-byt-n2820) > Test kms_pipe_crc_basic: > Subgroup hang-read-crc-pipe-b: > pass -> FAIL (fi-hsw-i7-4770r) [ 487.307292] [drm:pipe_crc_set_source] collecting CRCs for pipe B, pf [ 487.390551] [drm:pipe_crc_set_source] stopping CRCs for pipe B [ 487.423865] [drm:hsw_audio_codec_disable] Disable audio codec on pipe B [ 487.423882] [drm:intel_disable_pipe] disabling pipe B ... [ 498.853439] [drm:intel_get_hpd_pins] hotplug event received, stat 0x0020, dig 0x00101012, pins 0x0020 [ 498.853447] [drm:intel_hpd_irq_storm_detect] Received HPD interrupt on PIN 5 - cnt: 0 [ 498.853535] [drm:i915_hotplug_work_func] running encoder hotplug functions [ 498.853549] [drm:i915_hotplug_work_func] Connector HDMI-A-1 (pin 5) received hotplug event. [ 498.853563] [drm:intel_hdmi_detect] [CONNECTOR:37:HDMI-A-1] [ 498.941487] [drm:intel_hdmi_detect] HDMI live status down [ 498.941505] [drm:intel_hpd_irq_event] [CONNECTOR:37:HDMI-A-1] status updated from connected to disconnected [ 498.941844] [drm:pipe_crc_set_source] collecting CRCs for pipe B, pf [ 499.051160] [drm:intel_get_hpd_pins] hotplug event received, stat 0x0020, dig 0x00101012, pins 0x0020 [ 499.051169] [drm:intel_hpd_irq_storm_detect] Received HPD interrupt on PIN 5 - cnt: 1 [ 499.051246] [drm:i915_hotplug_work_func] running encoder hotplug functions [ 499.051259] [drm:i915_hotplug_work_func] Connector HDMI-A-1 (pin 5) received hotplug event. [ 499.051271] [drm:intel_hdmi_detect] [CONNECTOR:37:HDMI-A-1] [ 499.078751] [drm:do_gmbus_xfer] GMBUS [i915 gmbus dpb] NAK for addr: 0040 w(1) [ 499.078756] [drm:do_gmbus_xfer] GMBUS [i915 gmbus dpb] NAK on first message, retry [ 499.078975] [drm:do_gmbus_xfer] GMBUS [i915 gmbus dpb] NAK for addr: 0040 w(1) [ 499.078983] [drm:drm_detect_monitor_audio] Monitor has basic audio support [ 499.078987] [drm:intel_hpd_irq_event] [CONNECTOR:37:HDMI-A-1] status updated from disconnected to connected [ 500.827624] [drm:intel_print_rc6_info] Enabling RC6 states: RC6 on [ 500.827649] [drm:gen6_enable_rps] Overclocking supported. Max: 1300MHz, Overclock max: 1300MHz [ 503.942669] kms_pipe_crc_basic: exiting, ret=99 So we shut down the pipe, then the monitor got suddenly disconnected, we didn't even try to enabled the pipe, tried to grab some CRCs, and then the monitor re-appeared. I think the attempt to grab CRCs from a disabled pipe is a race in igt_kms. The same one that sometimes causes us to sometimes attempt a modeset with a too small an FB. So basically for_each_connected_output() sees the old connected state, but then later igt_display_commit() will refresh the state from the kernel and decided that there's actually nothing to do, and then we just go on thinking everything is fine. So we really need to remove the "refresh from kernel" part from igt_display_commit(), or at least only sample that information in the test once, at the very start. > > fi-bdw-i7-5557u total:219 pass:206 dwarn:0 dfail:0 fail:0 skip:13 > fi-bsw-n3050 total:218 pass:174 dwarn:0 dfail:0 fail:2 skip:42 > fi-byt-n2820 total:218 pass:175 dwarn:0 dfail:0 fail:2 skip:41 > fi-hsw-i7-4770k total:219 pass:198 dwarn:0 dfail:0 fail:0 skip:21 > fi-hsw-i7-4770r total:219 pass:192 dwarn:0 dfail:0 fail:1 skip:26 > fi-skl-i7-6700k total:219 pass:191 dwarn:0 dfail:0 fail:0 skip:28 > ro-bdw-i5-5250u total:217 pass:179 dwarn:0 dfail:0 fail:0 skip:38 > ro-bdw-i7-5557U total:217 pass:204 dwarn:0 dfail:0 fail:0 skip:13 > ro-bdw-i7-5600u total:217 pass:184 dwarn:0 dfail:0 fail:1 skip:32 > ro-byt-n2820 total:216 pass:172 dwarn:0 dfail:0 fail:3 skip:41 > ro-hsw-i3-4010u total:216 pass:191 dwarn:0 dfail:0 fail:0 skip:25 > ro-hsw-i7-4770r total:217 pass:191 dwarn:0 dfail:0 fail:0 skip:26 > ro-ilk-i7-620lm total:217 pass:149 dwarn:0 dfail:0 fail:1 skip:67 > ro-ilk1-i5-650 total:212 pass:150 dwarn:0 dfail:0 fail:1 skip:61 > ro-ivb-i7-3770 total:217 pass:181 dwarn:0 dfail:0 fail:0 skip:36 > ro-ivb2-i7-3770 total:217 pass:185 dwarn:0 dfail:0 fail:0 skip:32 > ro-skl-i7-6700hq total:212 pass:187 dwarn:0 dfail:0 fail:0 skip:25 >
[Intel-gfx] ✗ Ro.CI.BAT: failure for drm/i915: Fix NULL pointer deference when out of PLLs in IVB
== Series Details == Series: drm/i915: Fix NULL pointer deference when out of PLLs in IVB URL : https://patchwork.freedesktop.org/series/7458/ State : failure == Summary == Series 7458v1 drm/i915: Fix NULL pointer deference when out of PLLs in IVB http://patchwork.freedesktop.org/api/1.0/series/7458/revisions/1/mbox Test drv_hangman: Subgroup error-state-basic: pass -> INCOMPLETE (fi-snb-i7-2600) Test gem_exec_flush: Subgroup basic-batch-kernel-default-cmd: fail -> PASS (fi-byt-n2820) Test kms_flip: Subgroup basic-flip-vs-wf_vblank: fail -> PASS (ro-bdw-i7-5600u) Test kms_force_connector_basic: Subgroup force-connector-state: skip -> PASS (ro-ivb2-i7-3770) Subgroup force-edid: skip -> PASS (ro-ivb2-i7-3770) Subgroup prune-stale-modes: skip -> PASS (ro-ivb2-i7-3770) fi-bdw-i7-5557u total:217 pass:204 dwarn:0 dfail:0 fail:0 skip:13 fi-bsw-n3050 total:216 pass:172 dwarn:0 dfail:0 fail:2 skip:42 fi-byt-n2820 total:216 pass:173 dwarn:0 dfail:0 fail:2 skip:41 fi-hsw-i7-4770k total:217 pass:195 dwarn:0 dfail:0 fail:0 skip:22 fi-hsw-i7-4770r total:217 pass:191 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-i7-6700k total:217 pass:189 dwarn:0 dfail:0 fail:0 skip:28 fi-snb-i7-2600 total:36 pass:27 dwarn:0 dfail:0 fail:0 skip:8 ro-bdw-i5-5250u total:209 pass:171 dwarn:0 dfail:0 fail:0 skip:38 ro-bdw-i7-5557U total:209 pass:196 dwarn:0 dfail:0 fail:0 skip:13 ro-bdw-i7-5600u total:209 pass:179 dwarn:0 dfail:0 fail:1 skip:29 ro-bsw-n3050 total:209 pass:167 dwarn:0 dfail:0 fail:2 skip:40 ro-byt-n2820 total:209 pass:168 dwarn:0 dfail:0 fail:3 skip:38 ro-hsw-i3-4010u total:209 pass:186 dwarn:0 dfail:0 fail:0 skip:23 ro-hsw-i7-4770r total:209 pass:185 dwarn:0 dfail:0 fail:0 skip:24 ro-ilk1-i5-650 total:204 pass:146 dwarn:0 dfail:0 fail:1 skip:57 ro-ivb-i7-3770 total:209 pass:176 dwarn:0 dfail:0 fail:0 skip:33 ro-ivb2-i7-3770 total:209 pass:180 dwarn:0 dfail:0 fail:0 skip:29 ro-skl-i7-6700hq total:204 pass:182 dwarn:0 dfail:0 fail:0 skip:22 ro-snb-i7-2620M total:209 pass:170 dwarn:0 dfail:0 fail:1 skip:38 Results at /archive/results/CI_IGT_test/RO_Patchwork_951/ 019aa31 drm-intel-nightly: 2016y-05m-20d-12h-47m-05s UTC integration manifest 8aba6f6c drm/i915: Fix NULL pointer deference when out of PLLs in IVB ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Re-enable GGTT earlier during resume on pre-gen6 platforms
On Sat, May 07, 2016 at 09:18:24PM +0300, Ville Syrjälä wrote: > On Fri, May 06, 2016 at 09:22:49PM +0100, Chris Wilson wrote: > > On Fri, May 06, 2016 at 09:35:55PM +0300, ville.syrj...@linux.intel.com > > wrote: > > > @@ -730,9 +730,14 @@ int i915_suspend_switcheroo(struct drm_device *dev, > > > pm_message_t state) > > > static int i915_drm_resume(struct drm_device *dev) > > > { > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > + int ret; > > > > > > disable_rpm_wakeref_asserts(dev_priv); > > > > > > + ret = i915_ggtt_enable_hw(dev); > > > + if (ret) > > > + DRM_ERROR("failed to re-enable GGTT\n"); > > > > Would it not be fatal for resume as well? Failure means we can't use the > > GGTT, so all subsequent writes will be going into a random address. > > Yeah, I assume things would blow up. The question is however, what can > we do in this case? We'd basically have to shut the entire driver down. > I don't think we have a way to do that? panic() seems like the answer here. If we risk scribbling into random memory we should make sure that we just drop everything. Kind regards, David ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix NULL pointer deference when out of PLLs in IVB
In commit f9476a6c6d0c ("drm/i915: Refactor platform specifics out of intel_get_shared_dpll()"), the ibx_get_dpll() function lacked an error check, that can lead to a NULL pointer dereference when trying to enable pipe C. BUG: unable to handle kernel NULL pointer dereference at 0068 IP: [] intel_reference_shared_dpll+0x15/0x100 [i915] PGD cec87067 PUD d30ce067 PMD 0 Oops: [#1] PREEMPT SMP Modules linked in: snd_hda_intel i915 drm_kms_helper drm intel_gtt sch_fq_codel cfg80211 binfmt_misc i2c_algo_bit cfbfillrect syscopyarea cfbimgblt sysfillrect sysimgblt fb_sys_fops cfbcopyarea intel_rapl iosf_mbi x86_pkg_temp_thermal coretemp agpgart kvm_intel snd_hda_codec_hdmi kvm iTCO_wdt snd_hda_codec_realtek snd_hda_codec_generic irqbypass aesni_intel aes_x86_64 glue_helper lrw gf128mul ablk_helper cryptd psmouse pcspkr snd_hda_codec i2c_i801 snd_hwdep snd_hda_core snd_pcm snd_timer lpc_ich mfd_core snd soundcore wmi evdev tpm_tis tpm [last unloaded: drm] CPU: 3 PID: 5810 Comm: kms_flip Tainted: G U W 4.6.0-test+ #3 Hardware name: /DZ77BH-55K, BIOS BHZ7710H.86A.0100.2013.0517.0942 05/17/2013 task: 8800d3908040 ti: 8801166c8000 task.ti: 8801166c8000 RIP: 0010:[] [] intel_reference_shared_dpll+0x15/0x100 [i915] RSP: 0018:8801166cba60 EFLAGS: 00010246 RAX: RBX: RCX: 0002 RDX: 0001 RSI: 8800d07f1bf8 RDI: RBP: 8801166cba88 R08: 0002 R09: 8800d32e5698 R10: 0001 R11: 8800cc89ac88 R12: 8800d07f1bf8 R13: R14: R15: FS: 7f4c3fc8d8c0() GS:88011bcc() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 0068 CR3: d3b4c000 CR4: 001406e0 Stack: 8800d07f1bf8 8800d04c 8801166cbaa8 a04823a7 8800d07f1bf8 8800d32e5698 8801166cbab8 a04840cf 8801166cbaf0 Call Trace: [] ibx_get_dpll+0x47/0xa0 [i915] [] intel_get_shared_dpll+0x1f/0x50 [i915] [] ironlake_crtc_compute_clock+0x280/0x430 [i915] [] intel_crtc_atomic_check+0x240/0x320 [i915] [] drm_atomic_helper_check_planes+0x14e/0x1d0 [drm_kms_helper] [] intel_atomic_check+0x5dc/0x1110 [i915] [] drm_atomic_check_only+0x14a/0x660 [drm] [] ? drm_atomic_set_crtc_for_connector+0x96/0x100 [drm] [] drm_atomic_commit+0x17/0x60 [drm] [] restore_fbdev_mode+0x237/0x260 [drm_kms_helper] [] ? drm_modeset_lock_all_ctx+0x9a/0xb0 [drm] [] drm_fb_helper_restore_fbdev_mode_unlocked+0x33/0x80 [drm_kms_helper] [] drm_fb_helper_set_par+0x2d/0x50 [drm_kms_helper] [] drm_fb_helper_hotplug_event+0xaa/0xf0 [drm_kms_helper] [] drm_fb_helper_restore_fbdev_mode_unlocked+0x56/0x80 [drm_kms_helper] [] intel_fbdev_restore_mode+0x22/0x80 [i915] [] i915_driver_lastclose+0xe/0x20 [i915] [] drm_lastclose+0x2e/0x130 [drm] [] drm_release+0x2ac/0x4b0 [drm] [] __fput+0xed/0x1f0 [] fput+0xe/0x10 [] task_work_run+0x76/0xb0 [] do_exit+0x3ab/0xc60 [] ? trace_hardirqs_on_caller+0x12f/0x1c0 [] do_group_exit+0x4e/0xc0 [] SyS_exit_group+0x14/0x20 [] entry_SYSCALL_64_fastpath+0x18/0xa8 Code: 14 80 48 8d 34 90 b8 01 00 00 00 d3 e0 09 04 b3 5b 41 5c 5d c3 90 0f 1f 44 00 00 55 48 89 e5 41 57 41 56 49 89 fe 41 55 41 54 53 <44> 8b 67 68 48 89 f3 48 8b be 08 02 00 00 4c 8b 2e e8 15 9d fd RIP [] intel_reference_shared_dpll+0x15/0x100 [i915] RSP CR2: 0068 Cc: Ville SyrjäläReported-by: Ville Syrjälä Fixes: f9476a6c6d0c ("drm/i915: Refactor platform specifics out of intel_get_shared_dpll()") Signed-off-by: Ander Conselvan de Oliveira --- drivers/gpu/drm/i915/intel_dpll_mgr.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c index f988adb..1e3d091 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -366,6 +366,9 @@ ibx_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, DPLL_ID_PCH_PLL_B); } + if (!pll) + return NULL; + /* reference the pll */ intel_reference_shared_dpll(pll, crtc_state); -- 2.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/9] drm/i915: gvt: Introduce the basic architecture of GVT-g
On Fri, May 20, 2016 at 12:18:10PM +, Wang, Zhi A wrote: > Thanks! See my replies below. > > > -Original Message- > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > > Sent: Friday, May 20, 2016 2:49 PM > > To: Wang, Zhi A> > Cc: intel-gfx@lists.freedesktop.org; tvrtko.ursu...@linux.intel.com; > > joonas.lahti...@linux.intel.com; Tian, Kevin ; Lv, > > Zhiyuan > > Subject: Re: [PATCH 3/9] drm/i915: gvt: Introduce the basic architecture of > > GVT-g > > > > On Tue, May 17, 2016 at 04:19:03AM -0400, Zhi Wang wrote: > > > +config DRM_I915_GVT > > > +bool "Intel GVT-g host driver" > > > +depends on DRM_I915 > > > +default n > > > +help > > > + Enabling GVT-g mediated graphics pass-through technique for > > Intel i915 > > > + based integrated graphics card. With GVT-g, it's possible to > > have one > > > + integrated i915 device shared by multiple VMs. Performance > > critical > > > + operations such as aperture accesses and ring buffer > > operations > > > + are passed-through to VM, with a minimal set of conflicting > > resources > > > + (e.g. display settings) mediated by GVT host driver. The > > > benefit > > of GVT > > > + is on both the performance, given that each VM could directly > > operate > > > + its aperture space and submit commands like running on native, > > and > > > + the feature completeness, given that a true GEN hardware is > > exposed. > > > > Just like that? No userspace support required? i.e. can it work with kvm or > > xen? > > Wants a comment on any possible danger (or why is it 'n' by default)? > > > Basically that text is only technical introduction. Or maybe we could discuss > what we want here: > a. Basic technical introduction. > b. Need Xen and KVM > c. Possible danger as it's only preliminary for now. > > i915 Kconfig introduction consists 3 parts: > a. Introduction > b. Userspace requirement > c. Limitiation > > Can I follow that? :) I'm never going to argue against giving good advice to the reader. Even if it is "for further details please see 01.org/blah" > > > +#ifndef __GVT_DEBUG_H__ > > > +#define __GVT_DEBUG_H__ > > > + > > > +#define gvt_info(fmt, args...) \ > > > + DRM_INFO("gvt: "fmt, ##args) > > > + > > > +#define gvt_err(fmt, args...) \ > > > + DRM_ERROR("gvt: "fmt, ##args) > > > > I think it is fair to say that neither of these will look nice in > > user-facing > > messages. > > > > Then how about [drm][gvt]? :) Just a comment to say to hesitate before using anything above DRM_DEBUG and ask yourself "is this the right way to provide this information to the user, is it as clear/useful as possible?" > > > +static struct intel_gvt_device_info broadwell_device_info = { > > > + .max_gtt_gm_sz = GB(4), /* 4GB */ > > > + .gtt_start_offset = MB(8), /* 8MB */ > > > + .max_gtt_size = MB(8), /* 8MB */ > > > + .gtt_entry_size = 8, > > > + .gtt_entry_size_shift = 3, > > > + .gmadr_bytes_in_cmd = 8, > > > + .mmio_size = MB(2), /* 2MB */ > > > + .mmio_bar = 0, /* BAR 0 */ > > > + .max_support_vgpu = 8, > > > > Too much superfluous detail upfront. It is hard to critique when you have no > > idea what it is for. At the moment, it looks like you are duplicating > > information > > we have elsewhere, and I'd rather not. (For starters, it makes GVT a bigger > > maintainance burden.) > > > OK. Then I think I should remove most them in the first path and add them in > the later patch. And do you want me to merge these HW descriptions into > i915_device_info? Some of them might not be used by i915 driver itself. > That's why I keep a dedicated data structure here. It's going to be on a case by case basis, but I would rather have hw descriptions in core. > > > +/** > > > + * intel_gvt_destroy_device - destroy a GVT device > > > + * @gvt_device: gvt device > > > + * > > > + * This function is called at the driver unloading stage, to destroy > > > +a > > > + * GVT device and free the related resources. > > > + * > > > + */ > > > +void intel_gvt_destroy_device(void *device) > > > > This should not be void *. You can forward declare struct intel_gvt such > > that the > > core has an opaque pointer. > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > b/drivers/gpu/drm/i915/i915_drv.h index 72f0b02..7d0b8d3 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -1703,6 +1703,10 @@ struct i915_workarounds { > > > u32 hw_whitelist_count[I915_NUM_ENGINES]; > > > }; > > > > > > +struct i915_gvt { > > > + void *gvt; > > > > As before. No void * here. > > > Let me think an approach to remove the "void *" here. How about we do like > this: > struct a{ > Struct b; > }; > > Let i915 take only struct b, then in gvt we do container_of() so that we > don't need to expose the declaration of struct a to i915? :)
Re: [Intel-gfx] [PATCH 6/9] drm/i915: Generate addressing mode bit from flag in context.
But the corresponding definition about desc.addressing_mode is in intel_lrc.c. And when we want to generate these bits in i915_gem_context.c we can't not reference them. Do you want me to move define GEN8_CTX_ADDRESSING_MODE_SHIFT 3 #define GEN8_CTX_ADDRESSING_MODE(ctx) \ (ctx->use_48bit_address_space_mode ? \ LEGACY_64B_CONTEXT :\ LEGACY_32B_CONTEXT) to intel_lrc.h? Thanks, Zhi. > -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Friday, May 20, 2016 3:04 PM > To: Wang, Zhi A> Cc: intel-gfx@lists.freedesktop.org; tvrtko.ursu...@linux.intel.com; > joonas.lahti...@linux.intel.com; Tian, Kevin ; Lv, > Zhiyuan > Subject: Re: [PATCH 6/9] drm/i915: Generate addressing mode bit from flag in > context. > > On Tue, May 17, 2016 at 04:19:06AM -0400, Zhi Wang wrote: > > Previously the addressing mode bit in context descriptor is generated > > from context PPGTT. As we allow context could be used without PPGTT, > > and we still need to know the addressing mode during context > > submission, a flag is introduced. > > > > And the addressing mode bit will be generated from this flag. > > > > v5: > > > > - Change USES_FULL_48BIT(dev) to USES_FULL_48BIT(dev_priv) (Tvrtko) > > > > Signed-off-by: Zhi Wang > > --- > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > drivers/gpu/drm/i915/i915_gem_context.c | 2 ++ > > drivers/gpu/drm/i915/intel_lrc.c| 9 + > > 3 files changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h index cc83f2d..91f69e5 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -884,6 +884,7 @@ struct intel_context { > > bool initialised; > > } engine[I915_NUM_ENGINES]; > > u32 ring_buffer_size; > > + bool use_48bit_addressing_mode; > > A whole bool? You could put the desc.addressing_mode in there! > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [CI 04/20] drm/i915: Remove the dedicated hangcheck workqueue
On Fri, May 20, 2016 at 01:07:29PM +0100, Tvrtko Ursulin wrote: > > On 19/05/16 14:13, Chris Wilson wrote: > >On Thu, May 19, 2016 at 01:50:51PM +0100, Tvrtko Ursulin wrote: > >> > >>On 19/05/16 12:32, Chris Wilson wrote: > >>>The queue only ever contains at most one item and has no special flags. > >>>It is just a very simple wrapper around the system-wq - a complication > >>>with no benefits. > >> > >>How much time do we take in the reset case - is it acceptable to do > >>that work from the system wq? > > > >Hangcheck is a handful of register reads and some pointer chasing per > >engine. (There is a seqno_barrier in there which may be reasonably > >expensive but not a cpu hog). The error capture is run from the > >hangcheck context - and that is no small task (especially if we ever > >apply the object compression patches), but for safety we need to call > >stop_machine() so it really doesn't matter at that point. > > I don't see a stop_machine? So until there is one, using the system > wq is a bit impolite in the error capture state, agreed? It's in the queue to fix the odd oops we get during capture. You'd be happy with system_long_wq in the meantime? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [CI 16/20] drm/i915: Only query timestamp when measuring elapsed time
On Thu, May 19, 2016 at 04:44:03PM +0100, Tvrtko Ursulin wrote: > > On 19/05/16 12:32, Chris Wilson wrote: > >Avoid the two calls to ktime_get_raw_ns() (at best it reads the TSC) as > >we only need to compute the elapsed time for a timed wait. > > > >v2: Eliminate the unused local variable reducing the function size by 64 > >bytes (using the storage space on the callers stack rather than adding > >to our stack frame) > > > >Signed-off-by: Chris Wilson> >--- > > drivers/gpu/drm/i915/i915_gem.c | 14 +- > > 1 file changed, 5 insertions(+), 9 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/i915_gem.c > >b/drivers/gpu/drm/i915/i915_gem.c > >index b48a3b46e86f..2c254cf49c15 100644 > >--- a/drivers/gpu/drm/i915/i915_gem.c > >+++ b/drivers/gpu/drm/i915/i915_gem.c > >@@ -1215,7 +1215,6 @@ int __i915_wait_request(struct drm_i915_gem_request > >*req, > > int state = interruptible ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE; > > struct intel_wait wait; > > unsigned long timeout_remain; > >-s64 before = 0; /* Only to silence a compiler warning. */ > > int ret = 0; > > > > might_sleep(); > >@@ -1234,12 +1233,9 @@ int __i915_wait_request(struct drm_i915_gem_request > >*req, > > if (*timeout == 0) > > return -ETIME; > > > >+/* Record current time in case interrupted, or wedged */ > > timeout_remain = nsecs_to_jiffies_timeout(*timeout); > >- > >-/* > >- * Record current time in case interrupted by signal, or wedged. > >- */ > >-before = ktime_get_raw_ns(); > >+*timeout += ktime_get_raw_ns(); > > } > > > > trace_i915_gem_request_wait_begin(req); > >@@ -1296,9 +1292,9 @@ complete: > > trace_i915_gem_request_wait_end(req); > > > > if (timeout) { > >-s64 tres = *timeout - (ktime_get_raw_ns() - before); > >- > >-*timeout = tres < 0 ? 0 : tres; > >+*timeout -= ktime_get_raw_ns(); > >+if (*timeout < 0) > >+*timeout = 0; > > > > /* > > * Apparently ktime isn't accurate enough and occasionally has a > > > > I think this is bad, better have a local than play games with > callers storage. It's smaller faster code :-) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/9] drm/i915: Make ring buffer size configurable
Thanks. Will do. :) > -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Friday, May 20, 2016 3:02 PM > To: Wang, Zhi A> Cc: intel-gfx@lists.freedesktop.org; tvrtko.ursu...@linux.intel.com; > joonas.lahti...@linux.intel.com; Tian, Kevin ; Lv, > Zhiyuan > Subject: Re: [PATCH 5/9] drm/i915: Make ring buffer size configurable > > On Tue, May 17, 2016 at 04:19:05AM -0400, Zhi Wang wrote: > > This patch introduces an option for configuring ring buffer size > > during context creation. If no ring buffer size is specified, the > > default size > > (4 * PAGE_SIZE) will be used. > > > > Signed-off-by: Zhi Wang > > --- > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > drivers/gpu/drm/i915/intel_lrc.c | 8 ++-- > > 2 files changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h index ea04352..cc83f2d 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -883,6 +883,7 @@ struct intel_context { > > uint32_t *lrc_reg_state; > > bool initialised; > > } engine[I915_NUM_ENGINES]; > > + u32 ring_buffer_size; > > > > struct list_head link; > > }; > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > > b/drivers/gpu/drm/i915/intel_lrc.c > > index db10c96..d52c806 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -2495,7 +2495,7 @@ static int execlists_context_deferred_alloc(struct > intel_context *ctx, > > struct intel_engine_cs *engine) { > > struct drm_i915_gem_object *ctx_obj; > > - uint32_t context_size; > > + uint32_t context_size, ring_buffer_size; > > struct intel_ringbuffer *ringbuf; > > int ret; > > > > @@ -2513,7 +2513,11 @@ static int execlists_context_deferred_alloc(struct > intel_context *ctx, > > return PTR_ERR(ctx_obj); > > } > > > > - ringbuf = intel_engine_create_ringbuffer(engine, 4 * PAGE_SIZE); > > + ring_buffer_size = ctx->ring_buffer_size; > > + if (!ring_buffer_size) > > + ring_buffer_size = 4 * PAGE_SIZE; > > Just don't let it be zero (during construction). > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [CI 06/20] drm/i915: Slaughter the thundering i915_wait_request herd
On Fri, May 20, 2016 at 01:04:13PM +0100, Tvrtko Ursulin wrote: > >+p = >waiters.rb_node; > >+while (*p) { > >+parent = *p; > >+if (wait->seqno == to_wait(parent)->seqno) { > >+/* We have multiple waiters on the same seqno, select > >+ * the highest priority task (that with the smallest > >+ * task->prio) to serve as the bottom-half for this > >+ * group. > >+ */ > >+if (wait->task->prio > to_wait(parent)->task->prio) { > >+p = >rb_right; > >+first = false; > >+} else > >+p = >rb_left; > >+} else if (i915_seqno_passed(wait->seqno, > >+ to_wait(parent)->seqno)) { > >+p = >rb_right; > >+if (i915_seqno_passed(seqno, to_wait(parent)->seqno)) > >+completed = parent; > > Hm don't you need the completed handling in the equal seqno case as well? i915_seqno_passed() returnts true if seqnoA == seqnoB > >+void intel_engine_remove_wait(struct intel_engine_cs *engine, > >+ struct intel_wait *wait) > >+{ > >+struct intel_breadcrumbs *b = >breadcrumbs; > >+ > >+/* Quick check to see if this waiter was already decoupled from > >+ * the tree by the bottom-half to avoid contention on the spinlock > >+ * by the herd. > >+ */ > >+if (RB_EMPTY_NODE(>node)) > >+return; > >+ > >+spin_lock(>lock); > >+ > >+if (b->first_waiter == wait->task) { > >+struct rb_node *next; > >+struct task_struct *task; > >+const int priority = wait->task->prio; > >+ > >+/* We are the current bottom-half. Find the next candidate, > >+ * the first waiter in the queue on the remaining oldest > >+ * request. As multiple seqnos may complete in the time it > >+ * takes us to wake up and find the next waiter, we have to > >+ * wake up that waiter for it to perform its own coherent > >+ * completion check. > >+ */ > >+next = rb_next(>node); > >+if (chain_wakeup(next, priority)) { > > Don't get this, next waiter my be a different seqno so how is the > priority check relevant? We only want to call try_to_wake_up() on processes at least as important as us. > Also, how can the next node be smaller priority anyway, if equal > seqno if has be equal or greater I thought? Next seqno can have a smaller priority (and be complete). chain_wakeup() just asks if the next waiter is as important as ourselves (the next waiter may be for a different seqno). > Then since add_waiter will wake up one extra waiter to handle the > missed irq case, here you may skip checking them based simply on > task priority which seems wrong. Correct. They will be handled by the next waiter in the qeueue, not us. Our job is to wake as many completed (+ the potentially completed bh) as possible without incurring undue delay for ourselves. All completed waiters will be woken in turn as the next bh to run will look at the list and wake up those at the the same priority as itself (+ the next potentially completed bh). > >+static inline bool intel_engine_wakeup(struct intel_engine_cs *engine) > >+{ > >+bool wakeup = false; > >+struct task_struct *task = READ_ONCE(engine->breadcrumbs.first_waiter); > >+/* Note that for this not to dangerously chase a dangling pointer, > >+ * the caller is responsible for ensure that the task remain valid for > >+ * wake_up_process() i.e. that the RCU grace period cannot expire. > >+ */ > > This gets called from hard irq context and I did not manage to > figure out what makes it safe in the remove waiter path? Why the > hard irq couldn't not sample NULL here? Because we only need RCU access to task, which is provided by the hard irq context. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/9] drm/i915: Introduce host graphics memory partition for GVT-g
> -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Friday, May 20, 2016 3:01 PM > To: Wang, Zhi A> Cc: intel-gfx@lists.freedesktop.org; tvrtko.ursu...@linux.intel.com; > joonas.lahti...@linux.intel.com; Tian, Kevin ; Lv, > Zhiyuan ; Niu, Bing > Subject: Re: [PATCH 4/9] drm/i915: Introduce host graphics memory partition > for GVT-g > > On Tue, May 17, 2016 at 04:19:04AM -0400, Zhi Wang wrote: > > From: Bing Niu > > > > This patch introduces host graphics memory partition when GVT-g is > > enabled. > > > > Under GVT-g, i915 host driver only owned limited graphics resources, > > others are managed by GVT-g resource allocator and kept for other vGPUs. > > Strong justification required for why the user is expected to get these right > via > module parameters. > Sure, will add more comments in both code and patches about that. > > +/* > > + * Under GVT-g, i915 host driver only owned limited graphics > > +resources, > > + * others are managed by GVT-g resource allocator and kept for other > vGPUs. > > + * > > + * For graphics memory space partition, a typical layout looks like: > > + * > > + * +---+---+--+---+ > > + * |* Host | *GVT-g Resource |* Host| *GVT-g Resource | > > + * | Owned | Allocator Managed | Owned| Allocator Managed | > > + * | | | | > | > > + * +---+---+--+---+---+ > > + * | | | | | | | | > | > > + * | i915 | vm 1 | vm 2 | vm 3 | i915 | vm 1 | vm 2 | vm 3 | > > + * | | | | | | | | > | > > + * +---+---+---+--+---+---+---+ > > + * | Aperture|Hidden > | > > + * +---+--+ > > + * | GGTT memory space > | > > + * +--+ > > + */ > > struct i915_gvt { > > void *gvt; > > + u64 low_gm_size; > > + u64 high_gm_size; > > }; > > > > struct i915_virtual_gpu { > > diff --git a/drivers/gpu/drm/i915/i915_vgpu.c > > b/drivers/gpu/drm/i915/i915_vgpu.c > > index 5312816..9382f04 100644 > > --- a/drivers/gpu/drm/i915/i915_vgpu.c > > +++ b/drivers/gpu/drm/i915/i915_vgpu.c > > @@ -189,13 +189,24 @@ int intel_vgt_balloon(struct drm_i915_private > *dev_priv) > > unsigned long unmappable_base, unmappable_size, unmappable_end; > > int ret; > > > > - if (!intel_vgpu_active(dev_priv)) > > + if (!intel_vgpu_active(dev_priv) && !intel_gvt_active(dev_priv)) > > return 0; > > > > - mappable_base = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.base)); > > - mappable_size = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.size)); > > - unmappable_base = > I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.base)); > > - unmappable_size = > I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.size)); > > + if (intel_gvt_active(dev_priv)) { > > + mappable_base = 0; > > + mappable_size = dev_priv->gvt.low_gm_size; > > + unmappable_base = dev_priv->ggtt.mappable_end; > > + unmappable_size = dev_priv->gvt.high_gm_size; > > + } else if (intel_vgpu_active(dev_priv)) { > > + mappable_base = I915_READ( > > + vgtif_reg(avail_rs.mappable_gmadr.base)); > > + mappable_size = I915_READ( > > + vgtif_reg(avail_rs.mappable_gmadr.size)); > > + unmappable_base = I915_READ( > > + vgtif_reg(avail_rs.nonmappable_gmadr.base)); > > + unmappable_size = I915_READ( > > + vgtif_reg(avail_rs.nonmappable_gmadr.size)); > > + } > else > return 0; > > and lose the early return. OK. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/9] drm/i915: gvt: Introduce the basic architecture of GVT-g
Thanks! See my replies below. > -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Friday, May 20, 2016 2:49 PM > To: Wang, Zhi A> Cc: intel-gfx@lists.freedesktop.org; tvrtko.ursu...@linux.intel.com; > joonas.lahti...@linux.intel.com; Tian, Kevin ; Lv, > Zhiyuan > Subject: Re: [PATCH 3/9] drm/i915: gvt: Introduce the basic architecture of > GVT-g > > On Tue, May 17, 2016 at 04:19:03AM -0400, Zhi Wang wrote: > > +config DRM_I915_GVT > > +bool "Intel GVT-g host driver" > > +depends on DRM_I915 > > +default n > > +help > > + Enabling GVT-g mediated graphics pass-through technique for > Intel i915 > > + based integrated graphics card. With GVT-g, it's possible to > have one > > + integrated i915 device shared by multiple VMs. Performance > critical > > + operations such as aperture accesses and ring buffer > operations > > + are passed-through to VM, with a minimal set of conflicting > resources > > + (e.g. display settings) mediated by GVT host driver. The benefit > of GVT > > + is on both the performance, given that each VM could directly > operate > > + its aperture space and submit commands like running on native, > and > > + the feature completeness, given that a true GEN hardware is > exposed. > > Just like that? No userspace support required? i.e. can it work with kvm or > xen? > Wants a comment on any possible danger (or why is it 'n' by default)? > Basically that text is only technical introduction. Or maybe we could discuss what we want here: a. Basic technical introduction. b. Need Xen and KVM c. Possible danger as it's only preliminary for now. i915 Kconfig introduction consists 3 parts: a. Introduction b. Userspace requirement c. Limitiation Can I follow that? :) > > +#ifndef __GVT_DEBUG_H__ > > +#define __GVT_DEBUG_H__ > > + > > +#define gvt_info(fmt, args...) \ > > + DRM_INFO("gvt: "fmt, ##args) > > + > > +#define gvt_err(fmt, args...) \ > > + DRM_ERROR("gvt: "fmt, ##args) > > I think it is fair to say that neither of these will look nice in user-facing > messages. > Then how about [drm][gvt]? :) > > + > > +#define gvt_dbg_core(fmt, args...) \ > > + DRM_DEBUG_DRIVER("gvt: core: "fmt, ##args) > > + > > +#endif > > diff --git a/drivers/gpu/drm/i915/gvt/gvt.c > > b/drivers/gpu/drm/i915/gvt/gvt.c new file mode 100644 index > > 000..aa40357 > > --- /dev/null > > +++ b/drivers/gpu/drm/i915/gvt/gvt.c > > @@ -0,0 +1,205 @@ > > +/* > > + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved. > > + * > > + * Permission is hereby granted, free of charge, to any person > > +obtaining a > > + * copy of this software and associated documentation files (the > > +"Software"), > > + * to deal in the Software without restriction, including without > > +limitation > > + * the rights to use, copy, modify, merge, publish, distribute, > > +sublicense, > > + * and/or sell copies of the Software, and to permit persons to whom > > +the > > + * Software is furnished to do so, subject to the following conditions: > > + * > > + * The above copyright notice and this permission notice (including > > +the next > > + * paragraph) shall be included in all copies or substantial portions > > +of the > > + * Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY > KIND, > > +EXPRESS OR > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > > +MERCHANTABILITY, > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO > EVENT > > +SHALL > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, > DAMAGES > > +OR OTHER > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR > OTHERWISE, > > +ARISING FROM, > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > OTHER > > +DEALINGS IN THE > > + * SOFTWARE. > > + */ > > + > > +#include > > +#include > > +#include > > + > > +#include "gvt.h" > > + > > +struct intel_gvt_host intel_gvt_host; > > + > > +static const char * const supported_hypervisors[] = { > > + [INTEL_GVT_HYPERVISOR_XEN] = "XEN", > > + [INTEL_GVT_HYPERVISOR_KVM] = "KVM", > > +}; > > + > > +#define MB(x) (x * 1024ULL * 1024ULL) #define GB(x) (x * MB(1024)) > > + > > +/* > > + * The layout of BAR0 in BDW: > > + * |< - MMIO 2MB ->|<- Reserved 6MB ->|<- MAX GTT 8MB->| > > + * > > + * GTT offset in BAR0 starts from 8MB to 16MB, and > > + * Whatever GTT size is configured in BIOS, > > + * the size of BAR0 is always 16MB. The actual configured > > + * GTT size can be found in GMCH_CTRL. > > + */ > > +static struct intel_gvt_device_info broadwell_device_info = { > > + .max_gtt_gm_sz = GB(4), /* 4GB */ > > + .gtt_start_offset = MB(8), /* 8MB */ > > + .max_gtt_size = MB(8), /* 8MB */ > > + .gtt_entry_size = 8, > > + .gtt_entry_size_shift = 3, > > + .gmadr_bytes_in_cmd = 8, > > + .mmio_size
[Intel-gfx] ✗ Ro.CI.BAT: warning for series starting with [CI,1/5] drm/i915/fbdev: Limit the global async-domain synchronization
== Series Details == Series: series starting with [CI,1/5] drm/i915/fbdev: Limit the global async-domain synchronization URL : https://patchwork.freedesktop.org/series/7451/ State : warning == Summary == Series 7451v1 Series without cover letter http://patchwork.freedesktop.org/api/1.0/series/7451/revisions/1/mbox Test drv_module_reload_basic: pass -> SKIP (ro-ivb-i7-3770) dmesg-warn -> PASS (ro-bdw-i7-5600u) pass -> SKIP (ro-hsw-i3-4010u) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-b: dmesg-warn -> PASS (ro-ivb-i7-3770) fi-byt-n2820 total:216 pass:173 dwarn:0 dfail:0 fail:2 skip:41 fi-hsw-i7-4770k total:217 pass:195 dwarn:0 dfail:0 fail:0 skip:22 fi-hsw-i7-4770r total:217 pass:191 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-i7-6700k total:217 pass:189 dwarn:0 dfail:0 fail:0 skip:28 ro-bdw-i5-5250u total:217 pass:179 dwarn:0 dfail:0 fail:0 skip:38 ro-bdw-i7-5557U total:217 pass:204 dwarn:0 dfail:0 fail:0 skip:13 ro-bdw-i7-5600u total:217 pass:185 dwarn:0 dfail:0 fail:1 skip:31 ro-bsw-n3050 total:217 pass:173 dwarn:0 dfail:0 fail:2 skip:42 ro-byt-n2820 total:216 pass:172 dwarn:0 dfail:0 fail:3 skip:41 ro-hsw-i3-4010u total:216 pass:190 dwarn:0 dfail:0 fail:0 skip:26 ro-hsw-i7-4770r total:217 pass:191 dwarn:0 dfail:0 fail:0 skip:26 ro-ilk-i7-620lm total:217 pass:149 dwarn:0 dfail:0 fail:1 skip:67 ro-ilk1-i5-650 total:212 pass:150 dwarn:0 dfail:0 fail:1 skip:61 ro-ivb-i7-3770 total:217 pass:180 dwarn:0 dfail:0 fail:0 skip:37 ro-ivb2-i7-3770 total:217 pass:185 dwarn:0 dfail:0 fail:0 skip:32 ro-skl-i7-6700hq total:212 pass:188 dwarn:0 dfail:0 fail:0 skip:24 ro-snb-i7-2620M total:217 pass:175 dwarn:0 dfail:0 fail:1 skip:41 fi-bdw-i7-5557u failed to connect after reboot fi-bsw-n3050 failed to connect after reboot Results at /archive/results/CI_IGT_test/RO_Patchwork_950/ 9d15199 drm-intel-nightly: 2016y-05m-20d-07h-54m-59s UTC integration manifest e94db93 drm/i915: Inline sg_next() for the optimised SGL iterator ef885d8 drm/i915: Introduce & use new lightweight SGL iterators 6817d50 drm/i915: optimise i915_gem_object_map() for small objects c189b80 drm/i915: refactor i915_gem_object_pin_map() bf17e58 drm/i915/fbdev: Limit the global async-domain synchronization ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [CI 04/20] drm/i915: Remove the dedicated hangcheck workqueue
On 19/05/16 14:13, Chris Wilson wrote: On Thu, May 19, 2016 at 01:50:51PM +0100, Tvrtko Ursulin wrote: On 19/05/16 12:32, Chris Wilson wrote: The queue only ever contains at most one item and has no special flags. It is just a very simple wrapper around the system-wq - a complication with no benefits. How much time do we take in the reset case - is it acceptable to do that work from the system wq? Hangcheck is a handful of register reads and some pointer chasing per engine. (There is a seqno_barrier in there which may be reasonably expensive but not a cpu hog). The error capture is run from the hangcheck context - and that is no small task (especially if we ever apply the object compression patches), but for safety we need to call stop_machine() so it really doesn't matter at that point. I don't see a stop_machine? So until there is one, using the system wq is a bit impolite in the error capture state, agreed? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 9/9] drm/i915: Introduce GVT context creation API
On Tue, May 17, 2016 at 04:19:09AM -0400, Zhi Wang wrote: > GVT workload scheduler needs special host LRC contexts, the so called > "shadow LRC context" to submit guest workload to host i915. During the > guest workload submission, GVT fills the shadow LRC context with the > content of guest LRC context: engine context is copied without changes, > ring context is mostly owned by host i915. > > The GVT-g workload scheduler flow: > > +---+ +---+ > | GVT Guest | | GVT Guest | > +-+-^---+ +-+-^---+ >| | | | >| | GVT-g | | GVT-g > vELSP write| | emulates vELSP write| | emulates >| | Execlist/CSB| | Execlist/CSB >| | Status | | Status >| | | | > +--v-+-v-+-+ > | GVT Virtual Execlist Submission| > +--+---+---+ >| | >| Per-VM/Ring Workoad Q | Per-VM/Ring Workload Q >+-+--+ ++ >+---v+^ +---v+ >|GVT Workload|... | |GVT Workload|... >++| ++ > | > | Pick Workload from Q > ++-+ > |GVT Workload Scheduler| > ++-+ > | * Shadow guest LRC context > +--v--+ * Shadow guest ring buffer > | GVT Context | * Scan/Patch guest RB instructions > +--+--+ > | > v > Host i915 GEM Submission > v5: > - Only compile this feature when CONFIG_DRM_I915_GVT is enabled. > - Rebase the code into new repo. > - Add a comment about the ring buffer size. > > v2: > > Mostly based on Daniel's idea. Call the refactored core logic of GEM > context creation service and LRC context creation service to create the GVT > context. > > Signed-off-by: Zhi Wang> --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_gem_context.c | 31 +++ > 2 files changed, 32 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index b8f1e9a..7e5a506 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3398,6 +3398,7 @@ i915_gem_context_get(struct drm_i915_file_private > *file_priv, u32 id); > void i915_gem_context_free(struct kref *ctx_ref); > struct drm_i915_gem_object * > i915_gem_alloc_context_obj(struct drm_device *dev, size_t size); > +struct intel_context *i915_gem_create_gvt_context(struct drm_device *dev); > static inline void i915_gem_context_reference(struct intel_context *ctx) > { > kref_get(>ref); > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c > b/drivers/gpu/drm/i915/i915_gem_context.c > index 057e2fe..a69bb86 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -354,6 +354,37 @@ err_destroy: > return ERR_PTR(ret); > } > > +#if IS_ENABLED(CONFIG_DRM_I915_GVT) > +/** > + * i915_gem_create_gvt_context - create a GVT GEM context > + * @dev: drm device * > + * > + * This function is used to create a GVT specific GEM context. > + * > + * Returns: > + * pointer to intel_context on success, NULL if failed > + * > + */ > +struct intel_context * > +i915_gem_create_gvt_context(struct drm_device *dev) i915_gem_context_create_gvt -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [CI 06/20] drm/i915: Slaughter the thundering i915_wait_request herd
On 19/05/16 12:32, Chris Wilson wrote: One particularly stressful scenario consists of many independent tasks all competing for GPU time and waiting upon the results (e.g. realtime transcoding of many, many streams). One bottleneck in particular is that each client waits on its own results, but every client is woken up after every batchbuffer - hence the thunder of hooves as then every client must do its heavyweight dance to read a coherent seqno to see if it is the lucky one. Ideally, we only want one client to wake up after the interrupt and check its request for completion. Since the requests must retire in order, we can select the first client on the oldest request to be woken. Once that client has completed his wait, we can then wake up the next client and so on. However, all clients then incur latency as every process in the chain may be delayed for scheduling - this may also then cause some priority inversion. To reduce the latency, when a client is added or removed from the list, we scan the tree for completed seqno and wake up all the completed waiters in parallel. Using igt/benchmarks/gem_latency, we can demonstrate this effect. The benchmark measures the number of GPU cycles between completion of a batch and the client waking up from a call to wait-ioctl. With many concurrent waiters, with each on a different request, we observe that the wakeup latency before the patch scales nearly linearly with the number of waiters (before external factors kick in making the scaling much worse). After applying the patch, we can see that only the single waiter for the request is being woken up, providing a constant wakeup latency for every operation. However, the situation is not quite as rosy for many waiters on the same request, though to the best of my knowledge this is much less likely in practice. Here, we can observe that the concurrent waiters incur extra latency from being woken up by the solitary bottom-half, rather than directly by the interrupt. This appears to be scheduler induced (having discounted adverse effects from having a rbtree walk/erase in the wakeup path), each additional wake_up_process() costs approximately 1us on big core. Another effect of performing the secondary wakeups from the first bottom-half is the incurred delay this imposes on high priority threads - rather than immediately returning to userspace and leaving the interrupt handler to wake the others. To offset the delay incurred with additional waiters on a request, we could use a hybrid scheme that did a quick read in the interrupt handler and dequeued all the completed waiters (incurring the overhead in the interrupt handler, not the best plan either as we then incur GPU submission latency) but we would still have to wake up the bottom-half every time to do the heavyweight slow read. Or we could only kick the waiters on the seqno with the same priority as the current task (i.e. in the realtime waiter scenario, only it is woken up immediately by the interrupt and simply queues the next waiter before returning to userspace, minimising its delay at the expense of the chain, and also reducing contention on its scheduler runqueue). This is effective at avoid long pauses in the interrupt handler and at avoiding the extra latency in realtime/high-priority waiters. v2: Convert from a kworker per engine into a dedicated kthread for the bottom-half. v3: Rename request members and tweak comments. v4: Use a per-engine spinlock in the breadcrumbs bottom-half. v5: Fix race in locklessly checking waiter status and kicking the task on adding a new waiter. v6: Fix deciding when to force the timer to hide missing interrupts. v7: Move the bottom-half from the kthread to the first client process. v8: Reword a few comments v9: Break the busy loop when the interrupt is unmasked or has fired. v10: Comments, unnecessary churn, better debugging from Tvrtko v11: Wake all completed waiters on removing the current bottom-half to reduce the latency of waking up a herd of clients all waiting on the same request. v12: Rearrange missed-interrupt fault injection so that it works with igt/drv_missed_irq_hang v13: Rename intel_breadcrumb and friends to intel_wait in preparation for signal handling. v14: RCU commentary, assert_spin_locked v15: Hide BUG_ON behind the compiler; report on gem_latency findings. v16: Sort seqno-groups by priority so that first-waiter has the highest task priority (and so avoid priority inversion). v17: Add waiters to post-mortem GPU hang state. A lot of time has passed since I last looked at this. :I A lot of complexity to recall. :) Testcase: igt/gem_concurrent_blit Testcase: igt/benchmarks/gem_latency Signed-off-by: Chris WilsonCc: "Rogozhkin, Dmitry V" Cc: "Gong, Zhipeng" Cc: Tvrtko Ursulin Cc: Dave Gordon Cc: "Goel, Akash" ---
Re: [Intel-gfx] [PATCH 8/9] drm/i915: Support context single submission
On Tue, May 17, 2016 at 04:19:08AM -0400, Zhi Wang wrote: > This patch introduces the support of context signle submission. As GVT > context may come from different guests, which requires different > configuration of render registers. It can't be combined in a dual ELSP > submission combo. > > We make this function as a context feature in context creation service. > Only GVT-g will create this kinds of GEM context currently. > > v5: > > - Only compile this feature when CONFIG_DRM_I915_GVT=y. (Tvrtko) > > Signed-off-by: Zhi Wang> --- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/intel_lrc.c | 15 +++ > 2 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 9688006..b8f1e9a 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -890,8 +890,8 @@ struct intel_context { > bool use_48bit_addressing_mode; > #if IS_ENABLED(CONFIG_DRM_I915_GVT) > bool enable_status_change_notification; > + bool single_submission; > #endif > - > struct list_head link; > }; > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > b/drivers/gpu/drm/i915/intel_lrc.c > index 9069836..8d1a9c2 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -455,6 +455,21 @@ static void execlists_context_unqueue(struct > intel_engine_cs *engine) > i915_gem_request_unreference(req0); > req0 = cursor; > } else { > +#if IS_ENABLED(CONFIG_DRM_I915_GVT) > + But no ifdefs in code. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/9] drm/i915: Generate addressing mode bit from flag in context.
On Tue, May 17, 2016 at 04:19:06AM -0400, Zhi Wang wrote: > Previously the addressing mode bit in context descriptor is generated from > context PPGTT. As we allow context could be used without PPGTT, and we > still need to know the addressing mode during context submission, a flag > is introduced. > > And the addressing mode bit will be generated from this flag. > > v5: > > - Change USES_FULL_48BIT(dev) to USES_FULL_48BIT(dev_priv) (Tvrtko) > > Signed-off-by: Zhi Wang> --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_gem_context.c | 2 ++ > drivers/gpu/drm/i915/intel_lrc.c| 9 + > 3 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index cc83f2d..91f69e5 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -884,6 +884,7 @@ struct intel_context { > bool initialised; > } engine[I915_NUM_ENGINES]; > u32 ring_buffer_size; > + bool use_48bit_addressing_mode; A whole bool? You could put the desc.addressing_mode in there! -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/9] drm/i915: Make ring buffer size configurable
On Tue, May 17, 2016 at 04:19:05AM -0400, Zhi Wang wrote: > This patch introduces an option for configuring ring buffer size during > context creation. If no ring buffer size is specified, the default size > (4 * PAGE_SIZE) will be used. > > Signed-off-by: Zhi Wang> --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_lrc.c | 8 ++-- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index ea04352..cc83f2d 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -883,6 +883,7 @@ struct intel_context { > uint32_t *lrc_reg_state; > bool initialised; > } engine[I915_NUM_ENGINES]; > + u32 ring_buffer_size; > > struct list_head link; > }; > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > b/drivers/gpu/drm/i915/intel_lrc.c > index db10c96..d52c806 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -2495,7 +2495,7 @@ static int execlists_context_deferred_alloc(struct > intel_context *ctx, > struct intel_engine_cs *engine) > { > struct drm_i915_gem_object *ctx_obj; > - uint32_t context_size; > + uint32_t context_size, ring_buffer_size; > struct intel_ringbuffer *ringbuf; > int ret; > > @@ -2513,7 +2513,11 @@ static int execlists_context_deferred_alloc(struct > intel_context *ctx, > return PTR_ERR(ctx_obj); > } > > - ringbuf = intel_engine_create_ringbuffer(engine, 4 * PAGE_SIZE); > + ring_buffer_size = ctx->ring_buffer_size; > + if (!ring_buffer_size) > + ring_buffer_size = 4 * PAGE_SIZE; Just don't let it be zero (during construction). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/9] drm/i915: Introduce host graphics memory partition for GVT-g
On Tue, May 17, 2016 at 04:19:04AM -0400, Zhi Wang wrote: > From: Bing Niu> > This patch introduces host graphics memory partition when GVT-g > is enabled. > > Under GVT-g, i915 host driver only owned limited graphics resources, > others are managed by GVT-g resource allocator and kept for other vGPUs. Strong justification required for why the user is expected to get these right via module parameters. > +/* > + * Under GVT-g, i915 host driver only owned limited graphics resources, > + * others are managed by GVT-g resource allocator and kept for other vGPUs. > + * > + * For graphics memory space partition, a typical layout looks like: > + * > + * +---+---+--+---+ > + * |* Host | *GVT-g Resource |* Host| *GVT-g Resource | > + * | Owned | Allocator Managed | Owned| Allocator Managed | > + * | | | | | > + * +---+---+--+---+---+ > + * | | | | | | | | | > + * | i915 | vm 1 | vm 2 | vm 3 | i915 | vm 1 | vm 2 | vm 3 | > + * | | | | | | | | | > + * +---+---+---+--+---+---+---+ > + * | Aperture|Hidden| > + * +---+--+ > + * | GGTT memory space | > + * +--+ > + */ > struct i915_gvt { > void *gvt; > + u64 low_gm_size; > + u64 high_gm_size; > }; > > struct i915_virtual_gpu { > diff --git a/drivers/gpu/drm/i915/i915_vgpu.c > b/drivers/gpu/drm/i915/i915_vgpu.c > index 5312816..9382f04 100644 > --- a/drivers/gpu/drm/i915/i915_vgpu.c > +++ b/drivers/gpu/drm/i915/i915_vgpu.c > @@ -189,13 +189,24 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv) > unsigned long unmappable_base, unmappable_size, unmappable_end; > int ret; > > - if (!intel_vgpu_active(dev_priv)) > + if (!intel_vgpu_active(dev_priv) && !intel_gvt_active(dev_priv)) > return 0; > > - mappable_base = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.base)); > - mappable_size = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.size)); > - unmappable_base = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.base)); > - unmappable_size = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.size)); > + if (intel_gvt_active(dev_priv)) { > + mappable_base = 0; > + mappable_size = dev_priv->gvt.low_gm_size; > + unmappable_base = dev_priv->ggtt.mappable_end; > + unmappable_size = dev_priv->gvt.high_gm_size; > + } else if (intel_vgpu_active(dev_priv)) { > + mappable_base = I915_READ( > + vgtif_reg(avail_rs.mappable_gmadr.base)); > + mappable_size = I915_READ( > + vgtif_reg(avail_rs.mappable_gmadr.size)); > + unmappable_base = I915_READ( > + vgtif_reg(avail_rs.nonmappable_gmadr.base)); > + unmappable_size = I915_READ( > + vgtif_reg(avail_rs.nonmappable_gmadr.size)); > + } else return 0; and lose the early return. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/9] drm/i915: gvt: Introduce the basic architecture of GVT-g
On Tue, May 17, 2016 at 04:19:03AM -0400, Zhi Wang wrote: > +config DRM_I915_GVT > +bool "Intel GVT-g host driver" > +depends on DRM_I915 > +default n > +help > + Enabling GVT-g mediated graphics pass-through technique for Intel > i915 > + based integrated graphics card. With GVT-g, it's possible to have > one > + integrated i915 device shared by multiple VMs. Performance critical > + operations such as aperture accesses and ring buffer operations > + are passed-through to VM, with a minimal set of conflicting > resources > + (e.g. display settings) mediated by GVT host driver. The benefit > of GVT > + is on both the performance, given that each VM could directly > operate > + its aperture space and submit commands like running on native, and > + the feature completeness, given that a true GEN hardware is > exposed. Just like that? No userspace support required? i.e. can it work with kvm or xen? Wants a comment on any possible danger (or why is it 'n' by default)? > +#ifndef __GVT_DEBUG_H__ > +#define __GVT_DEBUG_H__ > + > +#define gvt_info(fmt, args...) \ > + DRM_INFO("gvt: "fmt, ##args) > + > +#define gvt_err(fmt, args...) \ > + DRM_ERROR("gvt: "fmt, ##args) I think it is fair to say that neither of these will look nice in user-facing messages. > + > +#define gvt_dbg_core(fmt, args...) \ > + DRM_DEBUG_DRIVER("gvt: core: "fmt, ##args) > + > +#endif > diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c > new file mode 100644 > index 000..aa40357 > --- /dev/null > +++ b/drivers/gpu/drm/i915/gvt/gvt.c > @@ -0,0 +1,205 @@ > +/* > + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > FROM, > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > THE > + * SOFTWARE. > + */ > + > +#include > +#include > +#include > + > +#include "gvt.h" > + > +struct intel_gvt_host intel_gvt_host; > + > +static const char * const supported_hypervisors[] = { > + [INTEL_GVT_HYPERVISOR_XEN] = "XEN", > + [INTEL_GVT_HYPERVISOR_KVM] = "KVM", > +}; > + > +#define MB(x) (x * 1024ULL * 1024ULL) > +#define GB(x) (x * MB(1024)) > + > +/* > + * The layout of BAR0 in BDW: > + * |< - MMIO 2MB ->|<- Reserved 6MB ->|<- MAX GTT 8MB->| > + * > + * GTT offset in BAR0 starts from 8MB to 16MB, and > + * Whatever GTT size is configured in BIOS, > + * the size of BAR0 is always 16MB. The actual configured > + * GTT size can be found in GMCH_CTRL. > + */ > +static struct intel_gvt_device_info broadwell_device_info = { > + .max_gtt_gm_sz = GB(4), /* 4GB */ > + .gtt_start_offset = MB(8), /* 8MB */ > + .max_gtt_size = MB(8), /* 8MB */ > + .gtt_entry_size = 8, > + .gtt_entry_size_shift = 3, > + .gmadr_bytes_in_cmd = 8, > + .mmio_size = MB(2), /* 2MB */ > + .mmio_bar = 0, /* BAR 0 */ > + .max_support_vgpu = 8, Too much superfluous detail upfront. It is hard to critique when you have no idea what it is for. At the moment, it looks like you are duplicating information we have elsewhere, and I'd rather not. (For starters, it makes GVT a bigger maintainance burden.) > +/** > + * intel_gvt_destroy_device - destroy a GVT device > + * @gvt_device: gvt device > + * > + * This function is called at the driver unloading stage, to destroy a > + * GVT device and free the related resources. > + * > + */ > +void intel_gvt_destroy_device(void *device) This should not be void *. You can forward declare struct intel_gvt such that the core has an opaque pointer. > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 72f0b02..7d0b8d3 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1703,6 +1703,10 @@ struct i915_workarounds { > u32 hw_whitelist_count[I915_NUM_ENGINES]; > }; > > +struct i915_gvt {
Re: [Intel-gfx] [PATCH 7/9] drm/i915: Introduce execlist context status change notification
Oh. Thanks Chris! > -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Friday, May 20, 2016 2:17 PM > To: Wang, Zhi A> Cc: intel-gfx@lists.freedesktop.org; tvrtko.ursu...@linux.intel.com; > joonas.lahti...@linux.intel.com; Tian, Kevin ; Lv, > Zhiyuan > Subject: Re: [PATCH 7/9] drm/i915: Introduce execlist context status change > notification > > On Tue, May 17, 2016 at 04:19:07AM -0400, Zhi Wang wrote: > > This patch introduces an approach to track the execlist context status > > change. > > > > GVT-g uses GVT context as the "shadow context". The content inside GVT > > context will be copied back to guest after the context is idle. So > > GVT-g has to know the status of the execlist context. > > > > This function is configurable in the context creation service. > > Currently, Only GVT-g will create the "status-change-notification" > > enabled GEM context. > > > > v5: > > > > - Only compile this feature when CONFIG_DRM_I915_GVT is > > enabled.(Tvrtko) > > > > Signed-off-by: Zhi Wang > > --- > > drivers/gpu/drm/i915/i915_drv.h | 6 ++ > > drivers/gpu/drm/i915/intel_lrc.c | 30 ++ > > drivers/gpu/drm/i915/intel_lrc.h | 7 +++ > > 3 files changed, 43 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h index 91f69e5..9688006 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -882,9 +882,15 @@ struct intel_context { > > u64 lrc_desc; > > uint32_t *lrc_reg_state; > > bool initialised; > > +#if IS_ENABLED(CONFIG_DRM_I915_GVT) > > + struct atomic_notifier_head status_notifier_head; > > struct atomic_notifier_head status_notifier; > > > +#endif > > } engine[I915_NUM_ENGINES]; > > u32 ring_buffer_size; > > bool use_48bit_addressing_mode; > > +#if IS_ENABLED(CONFIG_DRM_I915_GVT) > > + bool enable_status_change_notification; > > unsigned status_notify; > > > +#endif > > > > struct list_head link; > > }; > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > > b/drivers/gpu/drm/i915/intel_lrc.c > > index d97623f..9069836 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -415,6 +415,20 @@ static void execlists_submit_requests(struct > drm_i915_gem_request *rq0, > > spin_unlock_irq(_priv->uncore.lock); > > } > > > > +#if IS_ENABLED(CONFIG_DRM_I915_GVT) > > Actually, I think there will be other use cases for this notifier. > So let's not clutter the code with #ifdefs. Remove the ones from the struct > and > do > OK. I guess the reason you want to turn a "bool" to "unsigned" is: if (status_notify & NOTFIY_CONTEXT_STATUS) notify context status change if (status_notify & NOTIFY_OTHER_THINGS) notify other things extend it in future. Or is there any other reason? Glad to know. :) > > +static inline void execlists_context_status_change( > > + struct drm_i915_gem_request *req, > > + unsigned long status) > > +{ > > if (!IS_ENABLED(CONFIG_DRM_I915_GVT)) > return; > > instead. The compiler should be fine with the dead-code elimination. > OK. Will do. > > > + if (!req->ctx->enable_status_change_notification) > > What locks are you holding here? > I'm holding engine->execlist_lock at this path. > if (!READ_ONCE(req->ctx->status_notify)) > return; > > > + return; > > + > > + atomic_notifier_call_chain( > > + >ctx->engine[req->engine->id].status_notifier_head, > > I think we have enough justification for req->ctx_engine; > Are you saying you're doing some refactors on req->ctx, or you want to get some local pointers in this function, so that the req->x could be much shorter? :) > > + status, req); > > +} > > +#endif > > + > > static void execlists_context_unqueue(struct intel_engine_cs *engine) > > { > > struct drm_i915_gem_request *req0 = NULL, *req1 = NULL; @@ -450,6 > > +464,13 @@ static void execlists_context_unqueue(struct intel_engine_cs > *engine) > > if (unlikely(!req0)) > > return; > > > > +#if IS_ENABLED(CONFIG_DRM_I915_GVT) > > + execlists_context_status_change(req0, CONTEXT_SCHEDULE_IN); > > + > > + if (req1) > > + execlists_context_status_change(req1, CONTEXT_SCHEDULE_IN); > > > +#endif > > + > > if (req0->elsp_submitted & engine->idle_lite_restore_wa) { > > /* > > * WaIdleLiteRestore: make sure we never cause a lite restore @@ > > -488,6 +509,10 @@ execlists_check_remove_request(struct intel_engine_cs > *engine, u32 ctx_id) > > if (--head_req->elsp_submitted > 0) > > return 0; > > > > +#if IS_ENABLED(CONFIG_DRM_I915_GVT) > > + execlists_context_status_change(head_req, CONTEXT_SCHEDULE_OUT); > > +#endif > > + > >
[Intel-gfx] ✗ Ro.CI.BAT: failure for drm/i915/fbdev: Limit the global async-domain synchronization (rev3)
== Series Details == Series: drm/i915/fbdev: Limit the global async-domain synchronization (rev3) URL : https://patchwork.freedesktop.org/series/7332/ State : failure == Summary == Series 7332v3 drm/i915/fbdev: Limit the global async-domain synchronization http://patchwork.freedesktop.org/api/1.0/series/7332/revisions/3/mbox Test drv_module_reload_basic: dmesg-warn -> PASS (ro-bdw-i7-5600u) Test kms_flip: Subgroup basic-flip-vs-wf_vblank: pass -> FAIL (ro-byt-n2820) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-b: dmesg-warn -> PASS (ro-ivb-i7-3770) pass -> INCOMPLETE (fi-hsw-i7-4770k) fi-bdw-i7-5557u total:217 pass:204 dwarn:0 dfail:0 fail:0 skip:13 fi-hsw-i7-4770k total:198 pass:177 dwarn:0 dfail:0 fail:0 skip:20 fi-hsw-i7-4770r total:217 pass:191 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-i7-6700k total:217 pass:189 dwarn:0 dfail:0 fail:0 skip:28 ro-bdw-i5-5250u total:217 pass:179 dwarn:0 dfail:0 fail:0 skip:38 ro-bdw-i7-5557U total:217 pass:204 dwarn:0 dfail:0 fail:0 skip:13 ro-bdw-i7-5600u total:217 pass:185 dwarn:0 dfail:0 fail:1 skip:31 ro-bsw-n3050 total:217 pass:172 dwarn:0 dfail:0 fail:3 skip:42 ro-byt-n2820 total:216 pass:171 dwarn:0 dfail:0 fail:4 skip:41 ro-hsw-i3-4010u total:216 pass:191 dwarn:0 dfail:0 fail:0 skip:25 ro-hsw-i7-4770r total:217 pass:191 dwarn:0 dfail:0 fail:0 skip:26 ro-ilk-i7-620lm total:217 pass:149 dwarn:0 dfail:0 fail:1 skip:67 ro-ilk1-i5-650 total:212 pass:150 dwarn:0 dfail:0 fail:1 skip:61 ro-ivb-i7-3770 total:217 pass:181 dwarn:0 dfail:0 fail:0 skip:36 ro-ivb2-i7-3770 total:217 pass:185 dwarn:0 dfail:0 fail:0 skip:32 ro-skl-i7-6700hq total:212 pass:188 dwarn:0 dfail:0 fail:0 skip:24 ro-snb-i7-2620M total:217 pass:175 dwarn:0 dfail:0 fail:1 skip:41 fi-bsw-n3050 failed to connect after reboot fi-byt-n2820 failed to connect after reboot fi-snb-i7-2600 failed to connect after reboot Results at /archive/results/CI_IGT_test/RO_Patchwork_949/ 9d15199 drm-intel-nightly: 2016y-05m-20d-07h-54m-59s UTC integration manifest 1d7f750 drm/i915/fbdev: Limit the global async-domain synchronization ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 3/7] drm/i915/guc: add enable_guc_loading parameter
Tested-by: Fiedorowicz, LukaszI've done some manual testing on patches mentioned below. Results looks good. I did never encountered any kernel panic or hangs. GPU was wedged when scenario expected it. My testing scenario would look like this: - boot with different parameters combination - run gem_exec_nop - check dmesg and guc debugfs files When GuC was required (option 2) GPU was wedged as expected. When GuC wasn't required in successfully fall back to legacy submission. I've checked scenarios with and without valid GuC firmware present. -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of Tvrtko Ursulin Sent: Tuesday, May 17, 2016 11:09 AM To: Gordon, David S ; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v5 3/7] drm/i915/guc: add enable_guc_loading parameter On 16/05/16 20:12, Dave Gordon wrote: > Split the function of "enable_guc_submission" into two separate > options. The new one ("enable_guc_loading") controls only the > *fetching and loading* of the GuC firmware image. The existing one is > redefined to control only the *use* of the GuC for batch submission > once the firmware is loaded. > > In addition, the degree of control has been refined from a simple bool > to an integer key, allowing several options: > -1 (default) whatever the platform default is > 0 DISABLE don't load/use the GuC > 1 BEST EFFORT try to load/use the GuC, fallback if not available > 2 REQUIRE must load/use the GuC, else leave the GPU wedged > > The new platform default (as coded here) will be to attempt to load > the GuC iff the device has a GuC that requires firmware, but not yet > to use it for submission. A later patch will change to enable it if > appropriate. > > v4: > Changed some error-message levels, mostly ERROR->INFO, per > review comments by Tvrtko Ursulin. > > v5: > Dropped one more error message, disabled GuC submission on > hypothetical firmware-free devices [Tvrtko Ursulin]. Ok, hope it works. :) Reviewed-by: Tvrtko Ursulin Has someone at least done some manual testing on the various combination of new option values? Please have them splat a Tested-by just because the logic is not straight forward for me. Regards, Tvrtko > > Signed-off-by: Dave Gordon > Cc: Tvrtko Ursulin > --- > drivers/gpu/drm/i915/i915_gem.c| 5 +- > drivers/gpu/drm/i915/i915_guc_submission.c | 4 +- > drivers/gpu/drm/i915/i915_params.c | 14 +++- > drivers/gpu/drm/i915/i915_params.h | 3 +- > drivers/gpu/drm/i915/intel_guc_loader.c| 108 > - > 5 files changed, 77 insertions(+), 57 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > b/drivers/gpu/drm/i915/i915_gem.c index 2cc8c24..1e63744 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4868,11 +4868,8 @@ int i915_gem_init_engines(struct drm_device *dev) > /* We can't enable contexts until all firmware is loaded */ > if (HAS_GUC(dev)) { > ret = intel_guc_setup(dev); > - if (ret) { > - DRM_ERROR("Failed to initialize GuC, error %d\n", ret); > - ret = -EIO; > + if (ret) > goto out; > - } > } > > /* > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c > b/drivers/gpu/drm/i915/i915_guc_submission.c > index 169242a..916cd67 100644 > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > @@ -969,7 +969,7 @@ int intel_guc_suspend(struct drm_device *dev) > struct intel_context *ctx; > u32 data[3]; > > - if (!i915.enable_guc_submission) > + if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS) > return 0; > > ctx = dev_priv->kernel_context; > @@ -995,7 +995,7 @@ int intel_guc_resume(struct drm_device *dev) > struct intel_context *ctx; > u32 data[3]; > > - if (!i915.enable_guc_submission) > + if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS) > return 0; > > ctx = dev_priv->kernel_context; > diff --git a/drivers/gpu/drm/i915/i915_params.c > b/drivers/gpu/drm/i915/i915_params.c > index 383c076..6a5578c 100644 > --- a/drivers/gpu/drm/i915/i915_params.c > +++ b/drivers/gpu/drm/i915/i915_params.c > @@ -54,7 +54,8 @@ struct i915_params i915 __read_mostly = { > .verbose_state_checks = 1, > .nuclear_pageflip = 0, > .edp_vswing = 0, > - .enable_guc_submission = false, > + .enable_guc_loading = -1, > + .enable_guc_submission = 0, > .guc_log_level = -1, > .enable_dp_mst = true, > .inject_load_failure = 0, > @@ -198,8 +199,15 @@ struct i915_params i915 __read_mostly =
Re: [Intel-gfx] [PATCH 7/9] drm/i915: Introduce execlist context status change notification
On Tue, May 17, 2016 at 04:19:07AM -0400, Zhi Wang wrote: > This patch introduces an approach to track the execlist context status > change. > > GVT-g uses GVT context as the "shadow context". The content inside GVT > context will be copied back to guest after the context is idle. So GVT-g > has to know the status of the execlist context. > > This function is configurable in the context creation service. Currently, > Only GVT-g will create the "status-change-notification" enabled GEM > context. > > v5: > > - Only compile this feature when CONFIG_DRM_I915_GVT is enabled.(Tvrtko) > > Signed-off-by: Zhi Wang> --- > drivers/gpu/drm/i915/i915_drv.h | 6 ++ > drivers/gpu/drm/i915/intel_lrc.c | 30 ++ > drivers/gpu/drm/i915/intel_lrc.h | 7 +++ > 3 files changed, 43 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 91f69e5..9688006 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -882,9 +882,15 @@ struct intel_context { > u64 lrc_desc; > uint32_t *lrc_reg_state; > bool initialised; > +#if IS_ENABLED(CONFIG_DRM_I915_GVT) > + struct atomic_notifier_head status_notifier_head; struct atomic_notifier_head status_notifier; > +#endif > } engine[I915_NUM_ENGINES]; > u32 ring_buffer_size; > bool use_48bit_addressing_mode; > +#if IS_ENABLED(CONFIG_DRM_I915_GVT) > + bool enable_status_change_notification; unsigned status_notify; > +#endif > > struct list_head link; > }; > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > b/drivers/gpu/drm/i915/intel_lrc.c > index d97623f..9069836 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -415,6 +415,20 @@ static void execlists_submit_requests(struct > drm_i915_gem_request *rq0, > spin_unlock_irq(_priv->uncore.lock); > } > > +#if IS_ENABLED(CONFIG_DRM_I915_GVT) Actually, I think there will be other use cases for this notifier. So let's not clutter the code with #ifdefs. Remove the ones from the struct and do > +static inline void execlists_context_status_change( > + struct drm_i915_gem_request *req, > + unsigned long status) > +{ if (!IS_ENABLED(CONFIG_DRM_I915_GVT)) return; instead. The compiler should be fine with the dead-code elimination. > + if (!req->ctx->enable_status_change_notification) What locks are you holding here? if (!READ_ONCE(req->ctx->status_notify)) return; > + return; > + > + atomic_notifier_call_chain( > + >ctx->engine[req->engine->id].status_notifier_head, I think we have enough justification for req->ctx_engine; > + status, req); > +} > +#endif > + > static void execlists_context_unqueue(struct intel_engine_cs *engine) > { > struct drm_i915_gem_request *req0 = NULL, *req1 = NULL; > @@ -450,6 +464,13 @@ static void execlists_context_unqueue(struct > intel_engine_cs *engine) > if (unlikely(!req0)) > return; > > +#if IS_ENABLED(CONFIG_DRM_I915_GVT) > + execlists_context_status_change(req0, CONTEXT_SCHEDULE_IN); > + > + if (req1) > + execlists_context_status_change(req1, CONTEXT_SCHEDULE_IN); > +#endif > + > if (req0->elsp_submitted & engine->idle_lite_restore_wa) { > /* >* WaIdleLiteRestore: make sure we never cause a lite restore > @@ -488,6 +509,10 @@ execlists_check_remove_request(struct intel_engine_cs > *engine, u32 ctx_id) > if (--head_req->elsp_submitted > 0) > return 0; > > +#if IS_ENABLED(CONFIG_DRM_I915_GVT) > + execlists_context_status_change(head_req, CONTEXT_SCHEDULE_OUT); > +#endif > + > list_del(_req->execlist_link); > i915_gem_request_unreference(head_req); > > @@ -2534,6 +2559,11 @@ static int execlists_context_deferred_alloc(struct > intel_context *ctx, > ctx->engine[engine->id].state = ctx_obj; > ctx->engine[engine->id].initialised = engine->init_context == NULL; > > +#if IS_ENABLED(CONFIG_DRM_I915_GVT) > + if (ctx->enable_status_change_notification) Always do the init. That allows us to start switching the notify on/off at runtime. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Ro.CI.BAT: failure for Enable GuC submission (rev3)
== Series Details == Series: Enable GuC submission (rev3) URL : https://patchwork.freedesktop.org/series/7153/ State : failure == Summary == Series 7153v3 Enable GuC submission http://patchwork.freedesktop.org/api/1.0/series/7153/revisions/3/mbox Test drv_module_reload_basic: dmesg-warn -> PASS (ro-bdw-i7-5600u) Test kms_flip: Subgroup basic-flip-vs-wf_vblank: pass -> FAIL (ro-byt-n2820) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-b: dmesg-warn -> PASS (ro-ivb-i7-3770) fi-bdw-i7-5557u total:217 pass:204 dwarn:0 dfail:0 fail:0 skip:13 fi-bsw-n3050 total:216 pass:172 dwarn:0 dfail:0 fail:2 skip:42 fi-byt-n2820 total:216 pass:173 dwarn:0 dfail:0 fail:2 skip:41 fi-hsw-i7-4770r total:217 pass:191 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-i7-6700k total:217 pass:189 dwarn:0 dfail:0 fail:0 skip:28 ro-bdw-i5-5250u total:217 pass:179 dwarn:0 dfail:0 fail:0 skip:38 ro-bdw-i7-5557U total:217 pass:204 dwarn:0 dfail:0 fail:0 skip:13 ro-bdw-i7-5600u total:217 pass:185 dwarn:0 dfail:0 fail:1 skip:31 ro-bsw-n3050 total:217 pass:172 dwarn:0 dfail:0 fail:3 skip:42 ro-byt-n2820 total:216 pass:171 dwarn:0 dfail:0 fail:4 skip:41 ro-hsw-i3-4010u total:216 pass:191 dwarn:0 dfail:0 fail:0 skip:25 ro-hsw-i7-4770r total:217 pass:191 dwarn:0 dfail:0 fail:0 skip:26 ro-ilk-i7-620lm total:217 pass:148 dwarn:0 dfail:0 fail:2 skip:67 ro-ilk1-i5-650 total:212 pass:150 dwarn:0 dfail:0 fail:1 skip:61 ro-ivb-i7-3770 total:217 pass:181 dwarn:0 dfail:0 fail:0 skip:36 ro-ivb2-i7-3770 total:217 pass:185 dwarn:0 dfail:0 fail:0 skip:32 ro-skl-i7-6700hq total:212 pass:188 dwarn:0 dfail:0 fail:0 skip:24 ro-snb-i7-2620M total:217 pass:175 dwarn:0 dfail:0 fail:1 skip:41 fi-snb-i7-2600 failed to connect after reboot Results at /archive/results/CI_IGT_test/RO_Patchwork_948/ 9d15199 drm-intel-nightly: 2016y-05m-20d-07h-54m-59s UTC integration manifest c1bdfec drm/i915/guc: change default to using GuC submission if possible 429ed03 drm/i915/guc: rework guc_add_workqueue_item() 7e031c7 drm/i915/guc: don't spinwait if the GuC's workqueue is full 274cb6c drm/i915/guc: pass request (not client) to i915_guc_{wq_check_space, submit}() 25aa845 drm/i915/guc: add enable_guc_loading parameter b9dc454 drm/i915/guc: distinguish HAS_GUC() from HAS_GUC_UCODE/HAS_GUC_SCHED 7bdfe0d drm/i915/guc: rename loader entry points ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/9] drm/i915: gvt: Introduce the basic architecture of GVT-g
Thanks Tvrtko.:) See my replies below. > -Original Message- > From: Tvrtko Ursulin [mailto:tvrtko.ursu...@linux.intel.com] > Sent: Wednesday, May 18, 2016 2:23 PM > To: Wang, Zhi A; intel-gfx@lists.freedesktop.org; > joonas.lahti...@linux.intel.com; ch...@chris-wilson.co.uk; Tian, Kevin > ; Lv, Zhiyuan > Subject: Re: [PATCH 3/9] drm/i915: gvt: Introduce the basic architecture of > GVT-g > > > On 17/05/16 09:19, Zhi Wang wrote: > > This patch introduces the very basic framework of GVT-g device model, > > includes basic prototypes, definitions, initialization. > > > > v5: > > Take Tvrtko's comments: > > - Fix the misspelled words in Kconfg > > - Let functions take drm_i915_private * instead of struct drm_device * > > - Remove redundant prints/local varible initialization > > > > v3: > > Take Joonas' comments: > > - Change file name i915_gvt.* to intel_gvt.* > > - Move GVT kernel parameter into intel_gvt.c > > - Remove redundant debug macros > > - Change error handling style > > - Add introductions for some stub functions > > - Introduce drm/i915_gvt.h. > > > > Take Kevin's comments: > > - Move GVT-g host/guest check into intel_vgt_balloon in i915_gem_gtt.c > > > > v2: > > - Introduce i915_gvt.c. > > It's necessary to introduce the stubs between i915 driver and GVT-g > > host, as GVT-g components is configurable in kernel config. When > > disabled, the stubs here do nothing. > > > > Take Joonas' comments: > > - Replace boolean return value with int. > > - Replace customized info/warn/debug macros with DRM macros. > > - Document all non-static functions like i915. > > - Remove empty and unused functions. > > - Replace magic number with marcos. > > - Set GVT-g in kernel config to "n" by default. > > > > Signed-off-by: Zhi Wang > > --- > > drivers/gpu/drm/i915/Kconfig | 15 +++ > > drivers/gpu/drm/i915/Makefile| 5 + > > drivers/gpu/drm/i915/gvt/Makefile| 5 + > > drivers/gpu/drm/i915/gvt/debug.h | 36 ++ > > drivers/gpu/drm/i915/gvt/gvt.c | 205 > +++ > > drivers/gpu/drm/i915/gvt/gvt.h | 84 ++ > > drivers/gpu/drm/i915/gvt/hypercall.h | 38 +++ > > drivers/gpu/drm/i915/gvt/mpt.h | 49 + > > drivers/gpu/drm/i915/i915_dma.c | 17 ++- > > drivers/gpu/drm/i915/i915_drv.h | 12 ++ > > drivers/gpu/drm/i915/intel_gvt.c | 98 + > > drivers/gpu/drm/i915/intel_gvt.h | 49 + > > include/drm/i915_gvt.h | 31 ++ > > 13 files changed, 640 insertions(+), 4 deletions(-) > > create mode 100644 drivers/gpu/drm/i915/gvt/Makefile > > create mode 100644 drivers/gpu/drm/i915/gvt/debug.h > > create mode 100644 drivers/gpu/drm/i915/gvt/gvt.c > > create mode 100644 drivers/gpu/drm/i915/gvt/gvt.h > > create mode 100644 drivers/gpu/drm/i915/gvt/hypercall.h > > create mode 100644 drivers/gpu/drm/i915/gvt/mpt.h > > create mode 100644 drivers/gpu/drm/i915/intel_gvt.c > > create mode 100644 drivers/gpu/drm/i915/intel_gvt.h > > create mode 100644 include/drm/i915_gvt.h > > > > diff --git a/drivers/gpu/drm/i915/Kconfig > > b/drivers/gpu/drm/i915/Kconfig index 29a32b1..feb56ee 100644 > > --- a/drivers/gpu/drm/i915/Kconfig > > +++ b/drivers/gpu/drm/i915/Kconfig > > @@ -57,6 +57,21 @@ config DRM_I915_USERPTR > > > > If in doubt, say "Y". > > > > +config DRM_I915_GVT > > +bool "Intel GVT-g host driver" > > +depends on DRM_I915 > > +default n > > +help > > + Enabling GVT-g mediated graphics pass-through technique for > Intel i915 > > + based integrated graphics card. With GVT-g, it's possible to > have one > > + integrated i915 device shared by multiple VMs. Performance > critical > > + operations such as aperture accesses and ring buffer > operations > > + are passed-through to VM, with a minimal set of conflicting > > +resources > > Maybe I am confused but to me this is not clear. If we have multiple VMs > sharing the GPU where performance critical operations are passed-through to > the *VM* ? Aren't they passed-through to the GPU/host/hypervisor or > something rather than the VM? > This section is talking about aperture partition. For aperture partition, first hypervisor will allocate a aperture region and notify VM the begin/end of this region, then VM would know in the whole "guest" aperture bar, only a portion could be used. Then hypervisor directly map the guest aperture region to the allocated host aperture portion through the EPT. When guest accesses this aperture, it won't be trapped, just directly reached the related host aperture region. > > + (e.g. display settings) mediated by GVT host driver. The benefit > of GVT > > + is on both the performance, given that each VM could directly > operate > > + its aperture
[Intel-gfx] [CI 3/5] drm/i915: optimise i915_gem_object_map() for small objects
From: Dave GordonWe're using this function for ringbuffers and other "small" objects, so it's worth avoiding an extra malloc()/free() cycle if the page array is small enough to put on the stack. Here we've chosen an arbitrary cutoff of 32 (4k) pages, which is big enough for a ringbuffer (4 pages) or a context image (currently up to 22 pages). v5: change name of local array [Chris Wilson] Signed-off-by: Dave Gordon Reviewed-by: Tvrtko Ursulin Reviewed-by: Chris Wilson Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ac5fbf2d3e9a..5797fa23b00f 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2404,7 +2404,8 @@ static void *i915_gem_object_map(const struct drm_i915_gem_object *obj) unsigned long n_pages = obj->base.size >> PAGE_SHIFT; struct sg_table *sgt = obj->pages; struct sg_page_iter sg_iter; - struct page **pages; + struct page *stack_pages[32]; + struct page **pages = stack_pages; unsigned long i = 0; void *addr; @@ -2412,9 +2413,12 @@ static void *i915_gem_object_map(const struct drm_i915_gem_object *obj) if (n_pages == 1) return kmap(sg_page(sgt->sgl)); - pages = drm_malloc_gfp(n_pages, sizeof(*pages), GFP_TEMPORARY); - if (pages == NULL) - return NULL; + if (n_pages > ARRAY_SIZE(stack_pages)) { + /* Too big for stack -- allocate temporary array instead */ + pages = drm_malloc_gfp(n_pages, sizeof(*pages), GFP_TEMPORARY); + if (pages == NULL) + return NULL; + } for_each_sg_page(sgt->sgl, _iter, sgt->nents, 0) pages[i++] = sg_page_iter_page(_iter); @@ -2424,7 +2428,8 @@ static void *i915_gem_object_map(const struct drm_i915_gem_object *obj) addr = vmap(pages, n_pages, 0, PAGE_KERNEL); - drm_free_large(pages); + if (pages != stack_pages) + drm_free_large(pages); return addr; } -- 2.8.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [CI 2/5] drm/i915: refactor i915_gem_object_pin_map()
From: Dave GordonThe recently-added i915_gem_object_pin_map() can be further optimised for "small" objects. To facilitate this, and simplify the error paths before adding the new code, this patch pulls out the "mapping" part of the operation (involving local allocations which must be undone before return) into its own subfunction. The next patch will then insert the new optimisation into the middle of the now-separated subfunction. This reorganisation will probably not affect the generated code, as the compiler will most likely inline it anyway, but it makes the logical structure a bit clearer and easier to modify. v2: Restructure loop-over-pages & error check [Chris Wilson] v3: Add page count to debug messages [Chris Wilson] Convert WARN_ON() to GEM_BUG_ON() v4: Drop the DEBUG messages [Tvrtko Ursulin] Signed-off-by: Dave Gordon Reviewed-by: Tvrtko Ursulin Reviewed-by: Chris Wilson Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 54 + 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 6a31445a0471..ac5fbf2d3e9a 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2398,6 +2398,38 @@ i915_gem_object_get_pages(struct drm_i915_gem_object *obj) return 0; } +/* The 'mapping' part of i915_gem_object_pin_map() below */ +static void *i915_gem_object_map(const struct drm_i915_gem_object *obj) +{ + unsigned long n_pages = obj->base.size >> PAGE_SHIFT; + struct sg_table *sgt = obj->pages; + struct sg_page_iter sg_iter; + struct page **pages; + unsigned long i = 0; + void *addr; + + /* A single page can always be kmapped */ + if (n_pages == 1) + return kmap(sg_page(sgt->sgl)); + + pages = drm_malloc_gfp(n_pages, sizeof(*pages), GFP_TEMPORARY); + if (pages == NULL) + return NULL; + + for_each_sg_page(sgt->sgl, _iter, sgt->nents, 0) + pages[i++] = sg_page_iter_page(_iter); + + /* Check that we have the expected number of pages */ + GEM_BUG_ON(i != n_pages); + + addr = vmap(pages, n_pages, 0, PAGE_KERNEL); + + drm_free_large(pages); + + return addr; +} + +/* get, pin, and map the pages of the object into kernel space */ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj) { int ret; @@ -2411,27 +2443,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj) i915_gem_object_pin_pages(obj); if (obj->mapping == NULL) { - struct page **pages; - - pages = NULL; - if (obj->base.size == PAGE_SIZE) - obj->mapping = kmap(sg_page(obj->pages->sgl)); - else - pages = drm_malloc_gfp(obj->base.size >> PAGE_SHIFT, - sizeof(*pages), - GFP_TEMPORARY); - if (pages != NULL) { - struct sg_page_iter sg_iter; - int n; - - n = 0; - for_each_sg_page(obj->pages->sgl, _iter, -obj->pages->nents, 0) - pages[n++] = sg_page_iter_page(_iter); - - obj->mapping = vmap(pages, n, 0, PAGE_KERNEL); - drm_free_large(pages); - } + obj->mapping = i915_gem_object_map(obj); if (obj->mapping == NULL) { i915_gem_object_unpin_pages(obj); return ERR_PTR(-ENOMEM); -- 2.8.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [CI 5/5] drm/i915: Inline sg_next() for the optimised SGL iterator
From: Dave GordonAvoiding the out-of-line call to sg_next() reduces the kernel execution overhead by 10% in some workloads (for example the Unreal Engine 4 demo Atlantis on 2GiB GTTs) which are dominated by the cost of inserting PTEs due to texture thrashing. We can demonstrate this in a microbenchmark that forces us to rebind the object on every execbuf, where we can measure a 25% improvement, in the time required to execute an execbuf requiring a texture to be rebound, for inlining the sg_next() for large texture sizes. Benchmark: igt/benchmarks/gem_exec_fault Benchmark: igt/benchmarks/gem_exec_trace/Atlantis Signed-off-by: Dave Gordon Cc: Chris Wilson Reviewed-by: Chris Wilson Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_drv.h | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 63ff5fa2b2bd..e2abd9eb352b 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2278,6 +2278,25 @@ static __always_inline struct sgt_iter { } /** + * __sg_next - return the next scatterlist entry in a list + * @sg:The current sg entry + * + * Description: + * If the entry is the last, return NULL; otherwise, step to the next + * element in the array (@sg@+1). If that's a chain pointer, follow it; + * otherwise just return the pointer to the current element. + **/ +static inline struct scatterlist *__sg_next(struct scatterlist *sg) +{ +#ifdef CONFIG_DEBUG_SG + BUG_ON(sg->sg_magic != SG_MAGIC); +#endif + return sg_is_last(sg) ? NULL : + likely(!sg_is_chain(++sg)) ? sg : + sg_chain_ptr(sg); +} + +/** * for_each_sgt_dma - iterate over the DMA addresses of the given sg_table * @__dmap:DMA address (output) * @__iter:'struct sgt_iter' (iterator state, internal) @@ -2287,7 +2306,7 @@ static __always_inline struct sgt_iter { for ((__iter) = __sgt_iter((__sgt)->sgl, true); \ ((__dmap) = (__iter).dma + (__iter).curr); \ (((__iter).curr += PAGE_SIZE) < (__iter).max) || \ -((__iter) = __sgt_iter(sg_next((__iter).sgp), true), 0)) +((__iter) = __sgt_iter(__sg_next((__iter).sgp), true), 0)) /** * for_each_sgt_page - iterate over the pages of the given sg_table @@ -2300,7 +2319,7 @@ static __always_inline struct sgt_iter { ((__pp) = (__iter).pfn == 0 ? NULL : \ pfn_to_page((__iter).pfn + ((__iter).curr >> PAGE_SHIFT))); \ (((__iter).curr += PAGE_SIZE) < (__iter).max) || \ -((__iter) = __sgt_iter(sg_next((__iter).sgp), false), 0)) +((__iter) = __sgt_iter(__sg_next((__iter).sgp), false), 0)) /** * Request queue structure. -- 2.8.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 3/7] drm/i915/guc: add enable_guc_loading parameter
From: Dave GordonSplit the function of "enable_guc_submission" into two separate options. The new one ("enable_guc_loading") controls only the *fetching and loading* of the GuC firmware image. The existing one is redefined to control only the *use* of the GuC for batch submission once the firmware is loaded. In addition, the degree of control has been refined from a simple bool to an integer key, allowing several options: -1 (default) whatever the platform default is 0 DISABLE don't load/use the GuC 1 BEST EFFORT try to load/use the GuC, fallback if not available 2 REQUIRE must load/use the GuC, else leave the GPU wedged The new platform default (as coded here) will be to attempt to load the GuC iff the device has a GuC that requires firmware, but not yet to use it for submission. A later patch will change to enable it if appropriate. v4: Changed some error-message levels, mostly ERROR->INFO, per review comments by Tvrtko Ursulin. v5: Dropped one more error message, disabled GuC submission on hypothetical firmware-free devices [Tvrtko Ursulin]. v6: Logging tidy by Tvrtko Ursulin: * Do not log falling back to execlists when wedging the GPU. * Do not log fw load errors when load was disabled by user. * Pass down some error code from fw load for log message to make more sense. Signed-off-by: Dave Gordon Cc: Tvrtko Ursulin Reviewed-by: Tvrtko Ursulin (v5) Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_gem.c| 5 +- drivers/gpu/drm/i915/i915_guc_submission.c | 4 +- drivers/gpu/drm/i915/i915_params.c | 14 +++- drivers/gpu/drm/i915/i915_params.h | 3 +- drivers/gpu/drm/i915/intel_guc_loader.c| 123 + 5 files changed, 89 insertions(+), 60 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 88dce5482f2f..1a3a07eca0d0 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4868,11 +4868,8 @@ i915_gem_init_hw(struct drm_device *dev) /* We can't enable contexts until all firmware is loaded */ if (HAS_GUC(dev)) { ret = intel_guc_setup(dev); - if (ret) { - DRM_ERROR("Failed to initialize GuC, error %d\n", ret); - ret = -EIO; + if (ret) goto out; - } } /* diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 169242a8adff..916cd6778cf3 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -969,7 +969,7 @@ int intel_guc_suspend(struct drm_device *dev) struct intel_context *ctx; u32 data[3]; - if (!i915.enable_guc_submission) + if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS) return 0; ctx = dev_priv->kernel_context; @@ -995,7 +995,7 @@ int intel_guc_resume(struct drm_device *dev) struct intel_context *ctx; u32 data[3]; - if (!i915.enable_guc_submission) + if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS) return 0; ctx = dev_priv->kernel_context; diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index cd74fb8e9387..21a323c01cdb 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -53,7 +53,8 @@ struct i915_params i915 __read_mostly = { .verbose_state_checks = 1, .nuclear_pageflip = 0, .edp_vswing = 0, - .enable_guc_submission = false, + .enable_guc_loading = -1, + .enable_guc_submission = 0, .guc_log_level = -1, .enable_dp_mst = true, .inject_load_failure = 0, @@ -193,8 +194,15 @@ MODULE_PARM_DESC(edp_vswing, "(0=use value from vbt [default], 1=low power swing(200mV)," "2=default swing(400mV))"); -module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, bool, 0400); -MODULE_PARM_DESC(enable_guc_submission, "Enable GuC submission (default:false)"); +module_param_named_unsafe(enable_guc_loading, i915.enable_guc_loading, int, 0400); +MODULE_PARM_DESC(enable_guc_loading, + "Enable GuC firmware loading " + "(-1=auto [default], 0=never, 1=if available, 2=required)"); + +module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, int, 0400); +MODULE_PARM_DESC(enable_guc_submission, + "Enable GuC submission " + "(-1=auto, 0=never [default], 1=if available, 2=required)"); module_param_named(guc_log_level, i915.guc_log_level, int, 0400); MODULE_PARM_DESC(guc_log_level, diff --git
[Intel-gfx] [PATCH v3] drm/i915/fbdev: Limit the global async-domain synchronization
During cleanup we have to synchronise with the async task we are using to initialise and register our fbdev. Currently, we are using a full synchronisation on the global domain, but we can restrict this to just synchronising up to our task if we remember our cookie. v2: async_synchronize_cookie() takes an exclusive upper bound, to synchronize with our task we have to pass in the next cookie. v3: Drop premature disregarding of the active cookie (we need to wait until the task is complete before continuing in the teardown). Signed-off-by: Chris WilsonCc: Lukas Wunner --- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_fbdev.c | 29 - 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 0741b2d3aa65..68751aee52cc 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -159,6 +159,7 @@ struct intel_framebuffer { struct intel_fbdev { struct drm_fb_helper helper; struct intel_framebuffer *fb; + async_cookie_t cookie; int preferred_bpp; }; diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 99e27530e264..bfb2fd291e4e 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -538,8 +538,7 @@ static const struct drm_fb_helper_funcs intel_fb_helper_funcs = { .fb_probe = intelfb_create, }; -static void intel_fbdev_destroy(struct drm_device *dev, - struct intel_fbdev *ifbdev) +static void intel_fbdev_destroy(struct intel_fbdev *ifbdev) { /* We rely on the object-free to release the VMA pinning for * the info->screen_base mmaping. Leaking the VMA is simpler than @@ -554,12 +553,14 @@ static void intel_fbdev_destroy(struct drm_device *dev, if (ifbdev->fb) { drm_framebuffer_unregister_private(>fb->base); - mutex_lock(>struct_mutex); + mutex_lock(>helper.dev->struct_mutex); intel_unpin_fb_obj(>fb->base, BIT(DRM_ROTATE_0)); - mutex_unlock(>struct_mutex); + mutex_unlock(>helper.dev->struct_mutex); drm_framebuffer_remove(>fb->base); } + + kfree(ifbdev); } /* @@ -736,32 +737,34 @@ int intel_fbdev_init(struct drm_device *dev) static void intel_fbdev_initial_config(void *data, async_cookie_t cookie) { - struct drm_i915_private *dev_priv = data; - struct intel_fbdev *ifbdev = dev_priv->fbdev; + struct intel_fbdev *ifbdev = data; /* Due to peculiar init order wrt to hpd handling this is separate. */ if (drm_fb_helper_initial_config(>helper, ifbdev->preferred_bpp)) - intel_fbdev_fini(dev_priv->dev); + intel_fbdev_fini(ifbdev->helper.dev); } void intel_fbdev_initial_config_async(struct drm_device *dev) { - async_schedule(intel_fbdev_initial_config, to_i915(dev)); + struct intel_fbdev *ifbdev = to_i915(dev)->fbdev; + + ifbdev->cookie = async_schedule(intel_fbdev_initial_config, ifbdev); } void intel_fbdev_fini(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - if (!dev_priv->fbdev) + struct intel_fbdev *ifbdev = dev_priv->fbdev; + + if (!ifbdev) return; flush_work(_priv->fbdev_suspend_work); + if (ifbdev->cookie && !current_is_async()) + async_synchronize_cookie(ifbdev->cookie + 1); - if (!current_is_async()) - async_synchronize_full(); - intel_fbdev_destroy(dev, dev_priv->fbdev); - kfree(dev_priv->fbdev); + intel_fbdev_destroy(ifbdev); dev_priv->fbdev = NULL; } -- 2.8.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/9] drm/i915/gvt: Fold vGPU active check into inner functions
Thanks! :) > -Original Message- > From: Tvrtko Ursulin [mailto:tvrtko.ursu...@linux.intel.com] > Sent: Wednesday, May 18, 2016 1:55 PM > To: Wang, Zhi A; intel-gfx@lists.freedesktop.org; > joonas.lahti...@linux.intel.com; ch...@chris-wilson.co.uk; Tian, Kevin > ; Lv, Zhiyuan > Subject: Re: [PATCH 2/9] drm/i915/gvt: Fold vGPU active check into inner > functions > > > On 17/05/16 09:19, Zhi Wang wrote: > > v5: > > - Let functions take struct drm_i915_private *. (Tvrtko) > > > > - Fold vGPU related active check into the inner functions. (Kevin) > > > > Signed-off-by: Zhi Wang > > --- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 11 --- > > drivers/gpu/drm/i915/i915_vgpu.c| 13 + > > drivers/gpu/drm/i915/i915_vgpu.h| 4 ++-- > > 3 files changed, 15 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > > b/drivers/gpu/drm/i915/i915_gem_gtt.c > > index 7eab619..820b59e 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > @@ -2737,11 +2737,9 @@ static int i915_gem_setup_global_gtt(struct > drm_device *dev, > > i915_address_space_init(>base, dev_priv); > > ggtt->base.total += PAGE_SIZE; > > > > - if (intel_vgpu_active(dev_priv)) { > > - ret = intel_vgt_balloon(dev); > > - if (ret) > > - return ret; > > - } > > + ret = intel_vgt_balloon(dev_priv); > > + if (ret) > > + return ret; > > > > if (!HAS_LLC(dev)) > > ggtt->base.mm.color_adjust = i915_gtt_color_adjust; @@ > -2841,8 > > +2839,7 @@ void i915_ggtt_cleanup_hw(struct drm_device *dev) > > i915_gem_cleanup_stolen(dev); > > > > if (drm_mm_initialized(>base.mm)) { > > - if (intel_vgpu_active(dev_priv)) > > - intel_vgt_deballoon(); > > + intel_vgt_deballoon(dev_priv); > > > > drm_mm_takedown(>base.mm); > > list_del(>base.global_link); > > diff --git a/drivers/gpu/drm/i915/i915_vgpu.c > > b/drivers/gpu/drm/i915/i915_vgpu.c > > index d5a7a5e..5312816 100644 > > --- a/drivers/gpu/drm/i915/i915_vgpu.c > > +++ b/drivers/gpu/drm/i915/i915_vgpu.c > > @@ -101,10 +101,13 @@ static struct _balloon_info_ bl_info; > >* This function is called to deallocate the ballooned-out graphic memory, > when > >* driver is unloaded or when ballooning fails. > >*/ > > -void intel_vgt_deballoon(void) > > +void intel_vgt_deballoon(struct drm_i915_private *dev_priv) > > { > > int i; > > > > + if (!intel_vgpu_active(dev_priv)) > > + return; > > + > > DRM_DEBUG("VGT deballoon.\n"); > > > > for (i = 0; i < 4; i++) { > > @@ -177,9 +180,8 @@ static int vgt_balloon_space(struct drm_mm *mm, > >* Returns: > >* zero on success, non-zero if configuration invalid or ballooning failed > >*/ > > -int intel_vgt_balloon(struct drm_device *dev) > > +int intel_vgt_balloon(struct drm_i915_private *dev_priv) > > { > > - struct drm_i915_private *dev_priv = to_i915(dev); > > struct i915_ggtt *ggtt = _priv->ggtt; > > unsigned long ggtt_end = ggtt->base.start + ggtt->base.total; > > > > @@ -187,6 +189,9 @@ int intel_vgt_balloon(struct drm_device *dev) > > unsigned long unmappable_base, unmappable_size, > unmappable_end; > > int ret; > > > > + if (!intel_vgpu_active(dev_priv)) > > + return 0; > > + > > mappable_base = > I915_READ(vgtif_reg(avail_rs.mappable_gmadr.base)); > > mappable_size = > I915_READ(vgtif_reg(avail_rs.mappable_gmadr.size)); > > unmappable_base = > > I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.base)); > > @@ -258,6 +263,6 @@ int intel_vgt_balloon(struct drm_device *dev) > > > > err: > > DRM_ERROR("VGT balloon fail\n"); > > - intel_vgt_deballoon(); > > + intel_vgt_deballoon(dev_priv); > > return ret; > > } > > diff --git a/drivers/gpu/drm/i915/i915_vgpu.h > > b/drivers/gpu/drm/i915/i915_vgpu.h > > index 07e67d5..f8917c6 100644 > > --- a/drivers/gpu/drm/i915/i915_vgpu.h > > +++ b/drivers/gpu/drm/i915/i915_vgpu.h > > @@ -27,7 +27,7 @@ > > #include "i915_pvinfo.h" > > > > extern void i915_check_vgpu(struct drm_i915_private *dev_priv); > > -extern int intel_vgt_balloon(struct drm_device *dev); -extern void > > intel_vgt_deballoon(void); > > +extern int intel_vgt_balloon(struct drm_i915_private *dev_priv); > > +extern void intel_vgt_deballoon(struct drm_i915_private *dev_priv); > > > > #endif /* _I915_VGPU_H_ */ > > > > Looks clean to me. > > Reviewed-by: Tvrtko Ursulin > > Regards, > > Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 3/4] Introduce & use new lightweight SGL iterators
On Fri, May 20, 2016 at 09:15:06AM +0100, Chris Wilson wrote: > On Fri, May 20, 2016 at 01:31:29AM +0100, Dave Gordon wrote: > > The existing for_each_sg_page() iterator is somewhat heavyweight, and is > > limiting i915 driver performance in a few benchmarks. So here we > > introduce somewhat lighter weight iterators, primarily for use with GEM > > objects or other case where we need only deal with whole aligned pages. > > > > Unlike the old iterator, the new iterators use an internal state > > structure which is not intended to be accessed by the caller; instead > > each takes as a parameter an output variable which is set before each > > iteration. This makes them particularly simple to use :) > > > > One of the new iterators provides the caller with the DMA address of > > each page in turn; the other provides the 'struct page' pointer required > > by many memory management operations. > > > > Various uses of for_each_sg_page() are then converted to the new macros. > > > > Signed-off-by: Dave Gordon> > Cc: Chris Wilson > > --- > > drivers/gpu/drm/i915/i915_drv.h | 62 +-- > > drivers/gpu/drm/i915/i915_gem.c | 20 - > > drivers/gpu/drm/i915/i915_gem_fence.c | 14 +++--- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 76 > > - > > drivers/gpu/drm/i915/i915_gem_userptr.c | 7 ++- > > 5 files changed, 116 insertions(+), 63 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 6894d8e..01cde0f 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -2120,6 +2120,10 @@ struct drm_i915_gem_object_ops { > > #define INTEL_FRONTBUFFER_ALL_MASK(pipe) \ > > (0xff << (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe))) > > > > +void i915_gem_track_fb(struct drm_i915_gem_object *old, > > + struct drm_i915_gem_object *new, > > + unsigned frontbuffer_bits); > > + > > That's not a much better spot, since this is now before the object it > acts upon. One day we'll get our types separated from their actors. > > > struct drm_i915_gem_object { > > struct drm_gem_object base; > > > > @@ -2251,9 +2255,61 @@ struct drm_i915_gem_object { > > }; > > #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base) > > > > -void i915_gem_track_fb(struct drm_i915_gem_object *old, > > - struct drm_i915_gem_object *new, > > - unsigned frontbuffer_bits); > > +/* > > + * New optimised SGL iterator for GEM objects > > + */ > > +struct sgt_iter { > > + struct scatterlist *sgp; > > + union { > > + unsigned long pfn; > > + unsigned long dma; > > + } ix; > > An anonymous union here would be slightly more pleasant. And we should > be using the correct types. > > > + unsigned int curr; > > + unsigned int max; > > +}; > > + > > +/* Constructor for new iterator */ > > +static inline struct sgt_iter > > +__sgt_iter(struct scatterlist *sgl, bool dma) > > +{ > > + struct sgt_iter s = { .sgp = sgl }; > > + > > + if (s.sgp) { > > Not acceptable just to GPF out if passed in a NULL? It's a BUG for all > our use cases. And would save a conditional every chain step. Oh, it's how we stop after __sg_next(). Ok, played a bit and the only thing that sticks is /* - * New optimised SGL iterator for GEM objects + * Optimised SGL iterator for GEM objects */ struct sgt_iter { struct scatterlist *sgp; union { unsigned long pfn; - unsigned long dma; - } ix; + dma_addr_t dma; + }; unsigned int curr; unsigned int max; }; -/* Constructor for new iterator */ -static inline struct sgt_iter +static __always_inline struct sgt_iter Nothing is ever new past the commit that introduces it! Final diff (a few % in micro, consistently above the noise): diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 53ff4993f5b8..586f06646630 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2256,31 +2256,26 @@ struct drm_i915_gem_object { #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base) /* - * New optimised SGL iterator for GEM objects + * Optimised SGL iterator for GEM objects */ -struct sgt_iter { +static __always_inline struct sgt_iter { struct scatterlist *sgp; union { unsigned long pfn; - unsigned long dma; - } ix; + dma_addr_t dma; + }; unsigned int curr; unsigned int max; -}; - -/* Constructor for new iterator */ -static inline struct sgt_iter -__sgt_iter(struct scatterlist *sgl, bool dma) -{ +} __sgt_iter(struct scatterlist *sgl, bool dma) { struct sgt_iter s = { .sgp = sgl }; if (s.sgp) { s.max =
[Intel-gfx] ✗ Ro.CI.BAT: warning for Add USB typeC based DP support for BXT platform (rev7)
== Series Details == Series: Add USB typeC based DP support for BXT platform (rev7) URL : https://patchwork.freedesktop.org/series/1731/ State : warning == Summary == Series 1731v7 Add USB typeC based DP support for BXT platform http://patchwork.freedesktop.org/api/1.0/series/1731/revisions/7/mbox Test drv_module_reload_basic: pass -> DMESG-WARN (ro-hsw-i7-4770r) pass -> DMESG-WARN (fi-hsw-i7-4770r) pass -> DMESG-WARN (ro-hsw-i3-4010u) pass -> DMESG-WARN (fi-bdw-i7-5557u) Test gem_exec_suspend: Subgroup basic-s3: pass -> DMESG-WARN (ro-hsw-i3-4010u) pass -> DMESG-WARN (ro-bdw-i7-5600u) pass -> DMESG-WARN (fi-hsw-i7-4770r) pass -> DMESG-WARN (fi-skl-i7-6700k) pass -> DMESG-WARN (ro-hsw-i7-4770r) pass -> DMESG-WARN (fi-bdw-i7-5557u) Test kms_flip: Subgroup basic-flip-vs-dpms: pass -> SKIP (ro-bdw-i7-5557U) pass -> DMESG-WARN (ro-hsw-i3-4010u) pass -> DMESG-WARN (ro-bdw-i7-5600u) pass -> DMESG-WARN (fi-hsw-i7-4770r) pass -> DMESG-WARN (fi-skl-i7-6700k) pass -> DMESG-WARN (ro-hsw-i7-4770r) pass -> DMESG-WARN (fi-bdw-i7-5557u) Subgroup basic-flip-vs-modeset: pass -> SKIP (ro-bdw-i7-5557U) pass -> DMESG-WARN (ro-hsw-i3-4010u) pass -> DMESG-WARN (ro-bdw-i7-5600u) pass -> DMESG-WARN (fi-hsw-i7-4770r) pass -> DMESG-WARN (fi-skl-i7-6700k) pass -> DMESG-WARN (ro-hsw-i7-4770r) pass -> DMESG-WARN (fi-bdw-i7-5557u) Subgroup basic-flip-vs-wf_vblank: pass -> SKIP (ro-bdw-i7-5557U) pass -> DMESG-WARN (ro-hsw-i3-4010u) pass -> DMESG-WARN (ro-bdw-i7-5600u) pass -> DMESG-WARN (fi-hsw-i7-4770r) pass -> DMESG-WARN (fi-skl-i7-6700k) pass -> DMESG-WARN (ro-hsw-i7-4770r) pass -> DMESG-WARN (fi-bdw-i7-5557u) Subgroup basic-plain-flip: pass -> SKIP (ro-bdw-i7-5557U) pass -> DMESG-WARN (ro-hsw-i3-4010u) pass -> DMESG-WARN (ro-bdw-i7-5600u) pass -> DMESG-WARN (fi-hsw-i7-4770r) pass -> DMESG-WARN (fi-skl-i7-6700k) pass -> DMESG-WARN (ro-hsw-i7-4770r) pass -> DMESG-WARN (fi-bdw-i7-5557u) Test kms_frontbuffer_tracking: Subgroup basic: pass -> SKIP (ro-bdw-i7-5557U) pass -> DMESG-WARN (ro-hsw-i3-4010u) fail -> DMESG-FAIL (ro-bdw-i7-5600u) pass -> DMESG-WARN (fi-hsw-i7-4770r) pass -> DMESG-WARN (fi-skl-i7-6700k) pass -> DMESG-WARN (ro-hsw-i7-4770r) pass -> DMESG-WARN (fi-bdw-i7-5557u) Test kms_pipe_crc_basic: Subgroup hang-read-crc-pipe-a: pass -> SKIP (ro-bdw-i7-5557U) pass -> DMESG-WARN (ro-hsw-i3-4010u) pass -> DMESG-WARN (ro-bdw-i7-5600u) pass -> DMESG-WARN (fi-hsw-i7-4770r) pass -> DMESG-WARN (fi-skl-i7-6700k) pass -> DMESG-WARN (ro-hsw-i7-4770r) pass -> DMESG-WARN (fi-bdw-i7-5557u) Subgroup hang-read-crc-pipe-b: pass -> SKIP (ro-bdw-i7-5557U) pass -> DMESG-WARN (ro-hsw-i3-4010u) pass -> DMESG-WARN (ro-bdw-i7-5600u) pass -> DMESG-WARN (fi-hsw-i7-4770r) pass -> DMESG-WARN (fi-skl-i7-6700k) pass -> DMESG-WARN (ro-hsw-i7-4770r) pass -> DMESG-WARN (fi-bdw-i7-5557u) Subgroup hang-read-crc-pipe-c: pass -> SKIP (ro-bdw-i7-5557U) pass -> DMESG-WARN (ro-hsw-i3-4010u) pass -> DMESG-WARN (ro-bdw-i7-5600u) pass -> DMESG-WARN (fi-hsw-i7-4770r) pass -> DMESG-WARN (fi-skl-i7-6700k) pass -> DMESG-WARN (ro-hsw-i7-4770r) pass -> DMESG-WARN (fi-bdw-i7-5557u) Subgroup nonblocking-crc-pipe-a: pass -> SKIP (ro-bdw-i7-5557U) pass -> DMESG-WARN (ro-hsw-i3-4010u) pass -> DMESG-WARN (ro-bdw-i7-5600u) pass -> DMESG-WARN (fi-hsw-i7-4770r) pass -> DMESG-WARN (fi-skl-i7-6700k)
[Intel-gfx] ✗ Ro.CI.BAT: failure for series starting with [v4,1/4] drm/i915: refactor i915_gem_object_pin_map()
== Series Details == Series: series starting with [v4,1/4] drm/i915: refactor i915_gem_object_pin_map() URL : https://patchwork.freedesktop.org/series/7432/ State : failure == Summary == Series 7432v1 Series without cover letter http://patchwork.freedesktop.org/api/1.0/series/7432/revisions/1/mbox Test drv_module_reload_basic: dmesg-warn -> PASS (ro-bdw-i7-5600u) Test kms_flip: Subgroup basic-flip-vs-wf_vblank: pass -> FAIL (ro-bdw-i7-5600u) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-b: dmesg-warn -> PASS (ro-ivb-i7-3770) Test kms_sink_crc_basic: pass -> SKIP (ro-skl-i7-6700hq) fi-bdw-i7-5557u total:217 pass:204 dwarn:0 dfail:0 fail:0 skip:13 fi-bsw-n3050 total:216 pass:172 dwarn:0 dfail:0 fail:2 skip:42 fi-hsw-i7-4770k total:217 pass:195 dwarn:0 dfail:0 fail:0 skip:22 fi-hsw-i7-4770r total:217 pass:191 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-i7-6700k total:217 pass:189 dwarn:0 dfail:0 fail:0 skip:28 ro-bdw-i5-5250u total:217 pass:179 dwarn:0 dfail:0 fail:0 skip:38 ro-bdw-i7-5557U total:217 pass:204 dwarn:0 dfail:0 fail:0 skip:13 ro-bdw-i7-5600u total:217 pass:184 dwarn:0 dfail:0 fail:2 skip:31 ro-bsw-n3050 total:217 pass:173 dwarn:0 dfail:0 fail:2 skip:42 ro-byt-n2820 total:216 pass:172 dwarn:0 dfail:0 fail:3 skip:41 ro-hsw-i3-4010u total:216 pass:191 dwarn:0 dfail:0 fail:0 skip:25 ro-hsw-i7-4770r total:217 pass:191 dwarn:0 dfail:0 fail:0 skip:26 ro-ilk-i7-620lm total:217 pass:149 dwarn:0 dfail:0 fail:1 skip:67 ro-ilk1-i5-650 total:212 pass:150 dwarn:0 dfail:0 fail:1 skip:61 ro-ivb-i7-3770 total:217 pass:181 dwarn:0 dfail:0 fail:0 skip:36 ro-ivb2-i7-3770 total:217 pass:185 dwarn:0 dfail:0 fail:0 skip:32 ro-skl-i7-6700hq total:212 pass:187 dwarn:0 dfail:0 fail:0 skip:25 ro-snb-i7-2620M total:217 pass:175 dwarn:0 dfail:0 fail:1 skip:41 fi-byt-n2820 failed to connect after reboot Results at /archive/results/CI_IGT_test/RO_Patchwork_945/ 9d15199 drm-intel-nightly: 2016y-05m-20d-07h-54m-59s UTC integration manifest 7854a0c Try inlining sg_next() cd4e1f3 Introduce & use new lightweight SGL iterators bf7123c drm/i915: optimise i915_gem_object_map() for small objects afffbfb drm/i915: refactor i915_gem_object_pin_map() ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Ro.CI.BAT: warning for SKL watermark algorithm updates (rev2)
== Series Details == Series: SKL watermark algorithm updates (rev2) URL : https://patchwork.freedesktop.org/series/7262/ State : warning == Summary == Series 7262v2 SKL watermark algorithm updates http://patchwork.freedesktop.org/api/1.0/series/7262/revisions/2/mbox Test drv_module_reload_basic: dmesg-warn -> PASS (ro-bdw-i7-5600u) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-b: dmesg-warn -> PASS (ro-ivb-i7-3770) Subgroup suspend-read-crc-pipe-c: pass -> DMESG-WARN (ro-ivb-i7-3770) fi-bdw-i7-5557u total:217 pass:204 dwarn:0 dfail:0 fail:0 skip:13 fi-bsw-n3050 total:216 pass:172 dwarn:0 dfail:0 fail:2 skip:42 fi-byt-n2820 total:216 pass:173 dwarn:0 dfail:0 fail:2 skip:41 fi-hsw-i7-4770r total:217 pass:191 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-i7-6700k total:217 pass:189 dwarn:0 dfail:0 fail:0 skip:28 ro-bdw-i5-5250u total:217 pass:179 dwarn:0 dfail:0 fail:0 skip:38 ro-bdw-i7-5557U total:217 pass:204 dwarn:0 dfail:0 fail:0 skip:13 ro-bdw-i7-5600u total:217 pass:185 dwarn:0 dfail:0 fail:1 skip:31 ro-bsw-n3050 total:217 pass:173 dwarn:0 dfail:0 fail:2 skip:42 ro-byt-n2820 total:216 pass:172 dwarn:0 dfail:0 fail:3 skip:41 ro-hsw-i3-4010u total:216 pass:191 dwarn:0 dfail:0 fail:0 skip:25 ro-hsw-i7-4770r total:217 pass:191 dwarn:0 dfail:0 fail:0 skip:26 ro-ilk-i7-620lm total:217 pass:149 dwarn:0 dfail:0 fail:1 skip:67 ro-ilk1-i5-650 total:212 pass:150 dwarn:0 dfail:0 fail:1 skip:61 ro-ivb-i7-3770 total:217 pass:180 dwarn:1 dfail:0 fail:0 skip:36 ro-ivb2-i7-3770 total:217 pass:185 dwarn:0 dfail:0 fail:0 skip:32 ro-skl-i7-6700hq total:212 pass:188 dwarn:0 dfail:0 fail:0 skip:24 ro-snb-i7-2620M total:217 pass:175 dwarn:0 dfail:0 fail:1 skip:41 fi-hsw-i7-4770k failed to connect after reboot Results at /archive/results/CI_IGT_test/RO_Patchwork_944/ 9d15199 drm-intel-nightly: 2016y-05m-20d-07h-54m-59s UTC integration manifest 4a818c0 drm/i915/skl+: Use scaling amount for plane data rate calculation (v4) fa51434 drm/i915/skl+: calculate plane pixel rate (v4) c95c5fe drm/i915/skl+: calculate ddb minimum allocation (v4) c454d27 drm/i915: Don't try to calculate relative data rates during hw readout ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/fbdev: Limit the global async-domain synchronization
On Thu, May 19, 2016 at 10:14:35PM +0200, Lukas Wunner wrote: > Hi Chris, > > On Thu, May 19, 2016 at 09:28:10AM +0100, Chris Wilson wrote: > > During cleanup we have to synchronise with the async task we are using > > to initialise and register our fbdev. Currently, we are using a full > > synchronisation on the global domain, but we can restrict this to just > > synchronising up to our task if we remember our cookie. > > > > v2: async_synchronize_cookie() takes an exclusive upper bound, to > > synchronize with our task we have to pass in the next cookie. > > Oops, good catch, missed that in my own version of this patch: > https://lists.freedesktop.org/archives/intel-gfx/2016-March/091257.html > > > > > Signed-off-by: Chris Wilson> > --- > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > drivers/gpu/drm/i915/intel_fbdev.c | 31 ++- > > 2 files changed, 19 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > b/drivers/gpu/drm/i915/intel_drv.h > > index 3536292babe0..5bb045ba608e 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -159,6 +159,7 @@ struct intel_framebuffer { > > struct intel_fbdev { > > struct drm_fb_helper helper; > > struct intel_framebuffer *fb; > > + async_cookie_t cookie; > > int preferred_bpp; > > }; > > > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c > > b/drivers/gpu/drm/i915/intel_fbdev.c > > index 99e27530e264..2e9c3f38c023 100644 > > --- a/drivers/gpu/drm/i915/intel_fbdev.c > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > > @@ -538,8 +538,7 @@ static const struct drm_fb_helper_funcs > > intel_fb_helper_funcs = { > > .fb_probe = intelfb_create, > > }; > > > > -static void intel_fbdev_destroy(struct drm_device *dev, > > - struct intel_fbdev *ifbdev) > > +static void intel_fbdev_destroy(struct intel_fbdev *ifbdev) > > { > > /* We rely on the object-free to release the VMA pinning for > > * the info->screen_base mmaping. Leaking the VMA is simpler than > > @@ -554,12 +553,14 @@ static void intel_fbdev_destroy(struct drm_device > > *dev, > > if (ifbdev->fb) { > > drm_framebuffer_unregister_private(>fb->base); > > > > - mutex_lock(>struct_mutex); > > + mutex_lock(>helper.dev->struct_mutex); > > intel_unpin_fb_obj(>fb->base, BIT(DRM_ROTATE_0)); > > - mutex_unlock(>struct_mutex); > > + mutex_unlock(>helper.dev->struct_mutex); > > > > drm_framebuffer_remove(>fb->base); > > } > > + > > + kfree(ifbdev); > > } > > > > /* > > @@ -736,32 +737,36 @@ int intel_fbdev_init(struct drm_device *dev) > > > > static void intel_fbdev_initial_config(void *data, async_cookie_t cookie) > > { > > - struct drm_i915_private *dev_priv = data; > > - struct intel_fbdev *ifbdev = dev_priv->fbdev; > > + struct intel_fbdev *ifbdev = data; > > + > > + ifbdev->cookie = 0; > > Hm, why are you setting this to 0 here? IIUC the effect is that > async_synchronize_cookie() will wait until intel_fbdev_initial_config() > has been *entered*, but isn't the desired effect that it has *finished*? True, it's also an unserialised write. Premature optimisation to not block the teardown waiting for the out-of-order completion of earlier tasks. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCHv6 5/5] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT
To support USB type C alternate DP mode, the display driver needs to know the number of lanes required by the DP panel as well as number of lanes that can be supported by the type-C cable. Sometimes, the type-C cable may limit the bandwidth even if Panel can support more lanes. To address these scenarios, the display driver will start link training with max lanes, and if that fails, the driver falls back to x2 lanes; and repeats this procedure for all bandwidth/lane configurations. * Since link training is done before modeset only the port (and not pipe/planes) and its associated PLLs are enabled. * On DP hotplug: Directly start link training on the DP encoder. * On Connected boot scenarios: When booted with an LFP and a DP, sometimes BIOS brings up DP. In these cases, we disable the crtc and then do upfront link training; and bring it back up. * All local changes made for upfront link training are reset to their previous values once it is done; so that the subsequent modeset is not aware of these changes. Changes since v5: * Moved retry logic in upfront to intel_dp.c so that it can be used for all platforms. Changes since v4: * Removed usage of crtc_state in upfront link training; Hence no need to find free crtc to do upfront now. * Re-enable crtc if it was disabled for upfront. * Use separate variables to track max lane count and link rate found by upfront, without modifying the original DPCD read from panel. Changes since v3: * Fixed a return value on BXT check * Reworked on top of bxt_ddi_pll_select split from Ander * Renamed from ddi_upfront to bxt_upfront since the upfront logic includes BXT specific functions for now. Changes since v2: * Rebased on top of latest dpll_mgr.c code and latest HPD related clean ups. * Corrected return values from upfront (Ander) * Corrected atomic locking for upfront in intel_dp.c (Ville) Changes since v1: * all pll related functions inside ddi.c Signed-off-by: Durgadoss R--- drivers/gpu/drm/i915/intel_ddi.c | 45 ++ drivers/gpu/drm/i915/intel_dp.c | 179 +-- drivers/gpu/drm/i915/intel_drv.h | 8 ++ 3 files changed, 226 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 7e6331a..8d224bf 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -2330,6 +2330,51 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port) return connector; } +bool intel_bxt_upfront_link_train(struct intel_dp *intel_dp, + int clock, uint8_t lane_count) +{ + struct intel_connector *connector = intel_dp->attached_connector; + struct intel_encoder *encoder = connector->encoder; + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); + struct intel_shared_dpll *pll; + struct intel_shared_dpll_config tmp_pll_config; + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); + enum intel_dpll_id dpll_id = (enum intel_dpll_id)dig_port->port; + + /* +* FIXME: Works only for BXT. +* Select the required PLL. This works for platforms where +* there is no shared DPLL. +*/ + pll = _priv->shared_dplls[dpll_id]; + if (WARN_ON(pll->active_mask)) { + DRM_ERROR("Shared DPLL already in use. active_mask:%x\n", pll->active_mask); + return false; + } + + tmp_pll_config = pll->config; + + if (!bxt_ddi_dp_set_dpll_hw_state(clock, >config.hw_state)) { + DRM_ERROR("Could not setup DPLL\n"); + pll->config = tmp_pll_config; + return false; + } + + /* Enable PLL followed by port */ + pll->funcs.enable(dev_priv, pll); + intel_ddi_pre_enable_dp(encoder, clock, lane_count, pll); + + DRM_DEBUG_KMS("Upfront link train %s: link_clock:%d lanes:%d\n", + intel_dp->train_set_valid ? "Passed" : "Failed", clock, lane_count); + + /* Disable port followed by PLL for next retry/clean up */ + intel_ddi_post_disable(encoder); + pll->funcs.disable(dev_priv, pll); + + pll->config = tmp_pll_config; + return intel_dp->train_set_valid; +} + void intel_ddi_init(struct drm_device *dev, enum port port) { struct drm_i915_private *dev_priv = dev->dev_private; diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 95ba5aa..6ecaa30 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -147,18 +147,28 @@ intel_dp_max_link_bw(struct intel_dp *intel_dp) max_link_bw = DP_LINK_BW_1_62; break; } - return max_link_bw; + /* +* Limit max link bw w.r.t to the max value found +* using Upfront link training also. +*/ + return min(max_link_bw, intel_dp->max_link_bw_upfront); } static u8
[Intel-gfx] [PATCH 1/5] drm/i915: Don't pass crtc_state to intel_dp_set_link_params()
From: Ander Conselvan de OliveiraDecouple intel_dp_set_link_params() from struct intel_crtc_state. This will be useful for implementing DP upfront link training. Reviewed-by: Durgadoss R Signed-off-by: Ander Conselvan de Oliveira --- drivers/gpu/drm/i915/intel_ddi.c| 3 ++- drivers/gpu/drm/i915/intel_dp.c | 9 + drivers/gpu/drm/i915/intel_dp_mst.c | 4 +++- drivers/gpu/drm/i915/intel_drv.h| 2 +- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index c454744..1cbdd66 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1619,7 +1619,8 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder) if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder); - intel_dp_set_link_params(intel_dp, crtc->config); + intel_dp_set_link_params(intel_dp, crtc->config->port_clock, +crtc->config->lane_count); intel_ddi_init_dp_buf_reg(intel_encoder); diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 3633002..95ba5aa 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1584,10 +1584,10 @@ found: } void intel_dp_set_link_params(struct intel_dp *intel_dp, - const struct intel_crtc_state *pipe_config) + int link_rate, uint8_t lane_count) { - intel_dp->link_rate = pipe_config->port_clock; - intel_dp->lane_count = pipe_config->lane_count; + intel_dp->link_rate = link_rate; + intel_dp->lane_count = lane_count; } static void intel_dp_prepare(struct intel_encoder *encoder) @@ -1599,7 +1599,8 @@ static void intel_dp_prepare(struct intel_encoder *encoder) struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc); const struct drm_display_mode *adjusted_mode = >config->base.adjusted_mode; - intel_dp_set_link_params(intel_dp, crtc->config); + intel_dp_set_link_params(intel_dp, crtc->config->port_clock, +crtc->config->lane_count); /* * There are four kinds of DP registers: diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index 7a34090..553a25e 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -175,7 +175,9 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder) intel_ddi_clk_select(_dig_port->base, intel_crtc->config); - intel_dp_set_link_params(intel_dp, intel_crtc->config); + intel_dp_set_link_params(intel_dp, +intel_crtc->config->port_clock, +intel_crtc->config->lane_count); intel_ddi_init_dp_buf_reg(_dig_port->base); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 0741b2d..91fee9f 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1320,7 +1320,7 @@ void intel_dp_init(struct drm_device *dev, i915_reg_t output_reg, enum port port bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port, struct intel_connector *intel_connector); void intel_dp_set_link_params(struct intel_dp *intel_dp, - const struct intel_crtc_state *pipe_config); + int link_rate, uint8_t lane_count); void intel_dp_start_link_train(struct intel_dp *intel_dp); void intel_dp_stop_link_train(struct intel_dp *intel_dp); void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCHv2 4/5] drm/i915: Split bxt_ddi_pll_select()
Split out of bxt_ddi_pll_select() the logic that calculates the pll dividers and dpll_hw_state into a new function that doesn't depend on crtc state. This will be used for enabling the port pll when doing upfront link training. v2: * Refactored code so that bxt_clk_div need not be exported (Durga) v1: * Rebased on top of intel_dpll_mgr.c (Durga) * Initial version from Ander on top of intel_ddi.c Signed-off-by: Ander Conselvan de OliveiraSigned-off-by: Durgadoss R --- drivers/gpu/drm/i915/intel_dpll_mgr.c | 165 +- drivers/gpu/drm/i915/intel_dpll_mgr.h | 3 + 2 files changed, 104 insertions(+), 64 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c index 70b3c12..be56816 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -1456,6 +1456,8 @@ struct bxt_clk_div { uint32_t m2_frac; bool m2_frac_en; uint32_t n; + + int vco; }; /* pre-calculated values for DP linkrates */ @@ -1469,57 +1471,60 @@ static const struct bxt_clk_div bxt_dp_clk_val[] = { {432000, 3, 1, 32, 1677722, 1, 1} }; -static struct intel_shared_dpll * -bxt_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, -struct intel_encoder *encoder) +static bool +bxt_ddi_hdmi_pll_dividers(struct intel_crtc *intel_crtc, + struct intel_crtc_state *crtc_state, int clock, + struct bxt_clk_div *clk_div) { - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); - struct intel_shared_dpll *pll; - enum intel_dpll_id i; - struct intel_digital_port *intel_dig_port; - struct bxt_clk_div clk_div = {0}; - int vco = 0; - uint32_t prop_coef, int_coef, gain_ctl, targ_cnt; - uint32_t lanestagger; - int clock = crtc_state->port_clock; + struct dpll best_clock; - if (encoder->type == INTEL_OUTPUT_HDMI) { - struct dpll best_clock; + /* Calculate HDMI div */ + /* +* FIXME: tie the following calculation into +* i9xx_crtc_compute_clock +*/ + if (!bxt_find_best_dpll(crtc_state, clock, _clock)) { + DRM_DEBUG_DRIVER("no PLL dividers found for clock %d pipe %c\n", +clock, pipe_name(intel_crtc->pipe)); + return false; + } - /* Calculate HDMI div */ - /* -* FIXME: tie the following calculation into -* i9xx_crtc_compute_clock -*/ - if (!bxt_find_best_dpll(crtc_state, clock, _clock)) { - DRM_DEBUG_DRIVER("no PLL dividers found for clock %d pipe %c\n", -clock, pipe_name(crtc->pipe)); - return NULL; - } + clk_div->p1 = best_clock.p1; + clk_div->p2 = best_clock.p2; + WARN_ON(best_clock.m1 != 2); + clk_div->n = best_clock.n; + clk_div->m2_int = best_clock.m2 >> 22; + clk_div->m2_frac = best_clock.m2 & ((1 << 22) - 1); + clk_div->m2_frac_en = clk_div->m2_frac != 0; - clk_div.p1 = best_clock.p1; - clk_div.p2 = best_clock.p2; - WARN_ON(best_clock.m1 != 2); - clk_div.n = best_clock.n; - clk_div.m2_int = best_clock.m2 >> 22; - clk_div.m2_frac = best_clock.m2 & ((1 << 22) - 1); - clk_div.m2_frac_en = clk_div.m2_frac != 0; + clk_div->vco = best_clock.vco; - vco = best_clock.vco; - } else if (encoder->type == INTEL_OUTPUT_DISPLAYPORT || - encoder->type == INTEL_OUTPUT_EDP) { - int i; + return true; +} - clk_div = bxt_dp_clk_val[0]; - for (i = 0; i < ARRAY_SIZE(bxt_dp_clk_val); ++i) { - if (bxt_dp_clk_val[i].clock == clock) { - clk_div = bxt_dp_clk_val[i]; - break; - } +static void bxt_ddi_dp_pll_dividers(int clock, struct bxt_clk_div *clk_div) +{ + int i; + + *clk_div = bxt_dp_clk_val[0]; + for (i = 0; i < ARRAY_SIZE(bxt_dp_clk_val); ++i) { + if (bxt_dp_clk_val[i].clock == clock) { + *clk_div = bxt_dp_clk_val[i]; + break; } - vco = clock * 10 / 2 * clk_div.p1 * clk_div.p2; } + clk_div->vco = clock * 10 / 2 * clk_div->p1 * clk_div->p2; +} + +static bool bxt_ddi_set_dpll_hw_state(int clock, + struct bxt_clk_div *clk_div, + struct intel_dpll_hw_state *dpll_hw_state) +{ + int vco = clk_div->vco; + uint32_t prop_coef, int_coef, gain_ctl, targ_cnt; + uint32_t lanestagger; +
[Intel-gfx] [PATCH 2/5] drm/i915: Remove ddi_pll_sel from intel_crtc_state
From: Ander Conselvan de OliveiraThe value of ddi_pll_sel is derived from the selection of shared dpll, so just calculate the final value when necessary. Reviewed-by: Durgadoss R Signed-off-by: Ander Conselvan de Oliveira --- drivers/gpu/drm/i915/intel_ddi.c | 33 ++- drivers/gpu/drm/i915/intel_display.c | 43 +++ drivers/gpu/drm/i915/intel_dp_mst.c | 3 ++- drivers/gpu/drm/i915/intel_dpll_mgr.c | 27 -- drivers/gpu/drm/i915/intel_drv.h | 2 +- 5 files changed, 38 insertions(+), 70 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 1cbdd66..65485ef 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1567,14 +1567,36 @@ uint32_t ddi_signal_levels(struct intel_dp *intel_dp) return DDI_BUF_TRANS_SELECT(level); } +static uint32_t hsw_pll_to_ddi_pll_sel(struct intel_shared_dpll *pll) +{ + switch (pll->id) { + case DPLL_ID_WRPLL1: + return PORT_CLK_SEL_WRPLL1; + case DPLL_ID_WRPLL2: + return PORT_CLK_SEL_WRPLL2; + case DPLL_ID_SPLL: + return PORT_CLK_SEL_SPLL; + case DPLL_ID_LCPLL_810: + return PORT_CLK_SEL_LCPLL_810; + case DPLL_ID_LCPLL_1350: + return PORT_CLK_SEL_LCPLL_1350; + case DPLL_ID_LCPLL_2700: + return PORT_CLK_SEL_LCPLL_2700; + default: + return PORT_CLK_SEL_NONE; + } +} + void intel_ddi_clk_select(struct intel_encoder *encoder, - const struct intel_crtc_state *pipe_config) + struct intel_shared_dpll *pll) { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); enum port port = intel_ddi_get_encoder_port(encoder); + if (WARN_ON(!pll)) + return; + if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) { - uint32_t dpll = pipe_config->ddi_pll_sel; uint32_t val; /* DDI -> PLL mapping */ @@ -1582,14 +1604,13 @@ void intel_ddi_clk_select(struct intel_encoder *encoder, val &= ~(DPLL_CTRL2_DDI_CLK_OFF(port) | DPLL_CTRL2_DDI_CLK_SEL_MASK(port)); - val |= (DPLL_CTRL2_DDI_CLK_SEL(dpll, port) | + val |= (DPLL_CTRL2_DDI_CLK_SEL(pll->id, port) | DPLL_CTRL2_DDI_SEL_OVERRIDE(port)); I915_WRITE(DPLL_CTRL2, val); } else if (INTEL_INFO(dev_priv)->gen < 9) { - WARN_ON(pipe_config->ddi_pll_sel == PORT_CLK_SEL_NONE); - I915_WRITE(PORT_CLK_SEL(port), pipe_config->ddi_pll_sel); + I915_WRITE(PORT_CLK_SEL(port), hsw_pll_to_ddi_pll_sel(pll)); } } @@ -1614,7 +1635,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder) intel_edp_panel_on(intel_dp); } - intel_ddi_clk_select(intel_encoder, crtc->config); + intel_ddi_clk_select(intel_encoder, crtc->config->shared_dpll); if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a500f08..0251841 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9669,15 +9669,12 @@ static void bxt_get_ddi_pll(struct drm_i915_private *dev_priv, switch (port) { case PORT_A: - pipe_config->ddi_pll_sel = SKL_DPLL0; id = DPLL_ID_SKL_DPLL0; break; case PORT_B: - pipe_config->ddi_pll_sel = SKL_DPLL1; id = DPLL_ID_SKL_DPLL1; break; case PORT_C: - pipe_config->ddi_pll_sel = SKL_DPLL2; id = DPLL_ID_SKL_DPLL2; break; default: @@ -9696,25 +9693,10 @@ static void skylake_get_ddi_pll(struct drm_i915_private *dev_priv, u32 temp; temp = I915_READ(DPLL_CTRL2) & DPLL_CTRL2_DDI_CLK_SEL_MASK(port); - pipe_config->ddi_pll_sel = temp >> (port * 3 + 1); + id = temp >> (port * 3 + 1); - switch (pipe_config->ddi_pll_sel) { - case SKL_DPLL0: - id = DPLL_ID_SKL_DPLL0; - break; - case SKL_DPLL1: - id = DPLL_ID_SKL_DPLL1; - break; - case SKL_DPLL2: - id = DPLL_ID_SKL_DPLL2; - break; - case SKL_DPLL3: - id = DPLL_ID_SKL_DPLL3; - break; - default: - MISSING_CASE(pipe_config->ddi_pll_sel); + if (WARN_ON(id < SKL_DPLL0 || id > SKL_DPLL3)) return; - } pipe_config->shared_dpll =
[Intel-gfx] [PATCHv6 0/5] Add USB typeC based DP support for BXT platform
This patch series adds upfront link training support to enable USB type C based DP on BXT platform. To support USB type C alternate DP mode, the display driver needs to know the number of lanes required by the DP panel as well as number of lanes that can be supported by the type-C cable. Sometimes, the type-C cable may limit the bandwidth even if Panel can support more lanes. The goal is to find out the number of lanes which can be supported using a particular cable so that we can cap 'max_available_lanes' to that number during modeset. Patch 1-3/5 : Refactor ddi_pll code so that upfront link training does not need crtc_state. Patch 4/5 : Split ddi_pll_select to be able to calculate pll config for DP during upfront link train. Patch 5/5 : Upfront implementation for BXT platform. Changes from v5: * Moved the retry logic from upfront code to intel_dp.c Changes from v4: * Don't use crtc_state in upfront link train * Use separate variables to track max lane count and link bw obtained through upfront link train Changes from v3: * Used an earlier patch from Ander that splits ddi_pll_select to be able to use it from upfront link train logic. Changes from v2: * Rebased on top of intel_dpll_mgr.c code. * Use drm_atomic_commit instead of atomic_helper_dpms, since the later uses crtc->acquire_ctx used also in legacy paths. * Using a drm_atomic_state helps in using the shared_dpll APIs as is, thus keeping the upfront code away from changing pll internals directly. * Fixed locking/retry/backoff logic in upfront according to atomic implementation. Changes from v1: * Using atomic_helper_dpms() to do DPMS off for upfront link training, instead of using load detect functions. * Made intel_get_shared_dpll handle non-atomic paths by duplicating the required shared_dpll_config and then working on top of it. This helps in upfront link train code not directly touch the 'pll/pll->config' internals. * Fixed the link_bw update logic in intel_dp_get_link_retry_params() for non-HBR2 panels. * As per comments on earlier version, merged few patches (that added new functions) with the upfront link train patch, to facilitate easy review. Link for v5:https://patchwork.freedesktop.org/patch/87327/ Link for v4:https://patchwork.freedesktop.org/patch/81656/ Link for v3:https://patchwork.freedesktop.org/patch/79792/ Link for v2:https://patchwork.freedesktop.org/patch/72207/ Link for v1:https://patchwork.freedesktop.org/patch/67784/ Link for RFCv2: https://patchwork.freedesktop.org/patch/61776/ Link for RFCv1: https://patchwork.freedesktop.org/patch/59589/ Ander Conselvan de Oliveira (3): drm/i915: Don't pass crtc_state to intel_dp_set_link_params() drm/i915: Remove ddi_pll_sel from intel_crtc_state drm/i915: Split intel_ddi_pre_enable() into DP and HDMI versions Durgadoss R (2): drm/i915: Split bxt_ddi_pll_select() drm/i915/dp: Enable Upfront link training for typeC DP support on BXT drivers/gpu/drm/i915/intel_ddi.c | 150 +++--- drivers/gpu/drm/i915/intel_display.c | 43 ++-- drivers/gpu/drm/i915/intel_dp.c | 188 +++-- drivers/gpu/drm/i915/intel_dp_mst.c | 7 +- drivers/gpu/drm/i915/intel_dpll_mgr.c | 192 ++ drivers/gpu/drm/i915/intel_dpll_mgr.h | 3 + drivers/gpu/drm/i915/intel_drv.h | 12 ++- 7 files changed, 420 insertions(+), 175 deletions(-) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/5] drm/i915: Split intel_ddi_pre_enable() into DP and HDMI versions
From: Ander Conselvan de OliveiraSplit intel_ddi_pre_enable() into encoder type specific versions that don't depend on crtc_state. The necessary parameters are passed as function arguments. This split will be necessary for implementing DP upfront link training. Reviewed-by: Durgadoss R Signed-off-by: Ander Conselvan de Oliveira --- drivers/gpu/drm/i915/intel_ddi.c | 75 +++- 1 file changed, 44 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 65485ef..7e6331a 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1614,48 +1614,61 @@ void intel_ddi_clk_select(struct intel_encoder *encoder, } } -static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder) +static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, + int link_rate, uint32_t lane_count, + struct intel_shared_dpll *pll) { - struct drm_encoder *encoder = _encoder->base; - struct drm_i915_private *dev_priv = to_i915(encoder->dev); - struct intel_crtc *crtc = to_intel_crtc(encoder->crtc); - enum port port = intel_ddi_get_encoder_port(intel_encoder); - int type = intel_encoder->type; + struct intel_dp *intel_dp = enc_to_intel_dp(>base); + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); + enum port port = intel_ddi_get_encoder_port(encoder); - if (type == INTEL_OUTPUT_HDMI) { - struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); + intel_prepare_ddi_buffer(encoder); - intel_dp_dual_mode_set_tmds_output(intel_hdmi, true); - } + if (encoder->type == INTEL_OUTPUT_EDP) + intel_edp_panel_on(intel_dp); - intel_prepare_ddi_buffer(intel_encoder); + intel_ddi_clk_select(encoder, pll); - if (type == INTEL_OUTPUT_EDP) { - struct intel_dp *intel_dp = enc_to_intel_dp(encoder); - intel_edp_panel_on(intel_dp); - } + intel_dp_set_link_params(intel_dp, link_rate, lane_count); + intel_ddi_init_dp_buf_reg(encoder); - intel_ddi_clk_select(intel_encoder, crtc->config->shared_dpll); + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); + intel_dp_start_link_train(intel_dp); + if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9) + intel_dp_stop_link_train(intel_dp); +} - if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) { - struct intel_dp *intel_dp = enc_to_intel_dp(encoder); +static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, + bool has_hdmi_sink, + struct drm_display_mode *adjusted_mode, + struct intel_shared_dpll *pll) +{ + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(>base); + struct drm_encoder *drm_encoder = >base; - intel_dp_set_link_params(intel_dp, crtc->config->port_clock, -crtc->config->lane_count); + intel_dp_dual_mode_set_tmds_output(intel_hdmi, true); + intel_prepare_ddi_buffer(encoder); + intel_ddi_clk_select(encoder, pll); + intel_hdmi->set_infoframes(drm_encoder, has_hdmi_sink, adjusted_mode); +} - intel_ddi_init_dp_buf_reg(intel_encoder); +static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder) +{ + struct drm_encoder *encoder = _encoder->base; + struct intel_crtc *crtc = to_intel_crtc(encoder->crtc); + int type = intel_encoder->type; - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); - intel_dp_start_link_train(intel_dp); - if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9) - intel_dp_stop_link_train(intel_dp); - } else if (type == INTEL_OUTPUT_HDMI) { - struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); + if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) + intel_ddi_pre_enable_dp(intel_encoder, + crtc->config->port_clock, + crtc->config->lane_count, + crtc->config->shared_dpll); - intel_hdmi->set_infoframes(encoder, - crtc->config->has_hdmi_sink, - >config->base.adjusted_mode); - } + if (type == INTEL_OUTPUT_HDMI) + intel_ddi_pre_enable_hdmi(intel_encoder, + crtc->config->has_hdmi_sink, + >config->base.adjusted_mode, +
Re: [Intel-gfx] [PATCH v4 3/4] Introduce & use new lightweight SGL iterators
On Fri, May 20, 2016 at 01:31:29AM +0100, Dave Gordon wrote: > The existing for_each_sg_page() iterator is somewhat heavyweight, and is > limiting i915 driver performance in a few benchmarks. So here we > introduce somewhat lighter weight iterators, primarily for use with GEM > objects or other case where we need only deal with whole aligned pages. > > Unlike the old iterator, the new iterators use an internal state > structure which is not intended to be accessed by the caller; instead > each takes as a parameter an output variable which is set before each > iteration. This makes them particularly simple to use :) > > One of the new iterators provides the caller with the DMA address of > each page in turn; the other provides the 'struct page' pointer required > by many memory management operations. > > Various uses of for_each_sg_page() are then converted to the new macros. > > Signed-off-by: Dave Gordon> Cc: Chris Wilson > --- > drivers/gpu/drm/i915/i915_drv.h | 62 +-- > drivers/gpu/drm/i915/i915_gem.c | 20 - > drivers/gpu/drm/i915/i915_gem_fence.c | 14 +++--- > drivers/gpu/drm/i915/i915_gem_gtt.c | 76 > - > drivers/gpu/drm/i915/i915_gem_userptr.c | 7 ++- > 5 files changed, 116 insertions(+), 63 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 6894d8e..01cde0f 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2120,6 +2120,10 @@ struct drm_i915_gem_object_ops { > #define INTEL_FRONTBUFFER_ALL_MASK(pipe) \ > (0xff << (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe))) > > +void i915_gem_track_fb(struct drm_i915_gem_object *old, > +struct drm_i915_gem_object *new, > +unsigned frontbuffer_bits); > + That's not a much better spot, since this is now before the object it acts upon. One day we'll get our types separated from their actors. > struct drm_i915_gem_object { > struct drm_gem_object base; > > @@ -2251,9 +2255,61 @@ struct drm_i915_gem_object { > }; > #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base) > > -void i915_gem_track_fb(struct drm_i915_gem_object *old, > -struct drm_i915_gem_object *new, > -unsigned frontbuffer_bits); > +/* > + * New optimised SGL iterator for GEM objects > + */ > +struct sgt_iter { > + struct scatterlist *sgp; > + union { > + unsigned long pfn; > + unsigned long dma; > + } ix; An anonymous union here would be slightly more pleasant. And we should be using the correct types. > + unsigned int curr; > + unsigned int max; > +}; > + > +/* Constructor for new iterator */ > +static inline struct sgt_iter > +__sgt_iter(struct scatterlist *sgl, bool dma) > +{ > + struct sgt_iter s = { .sgp = sgl }; > + > + if (s.sgp) { Not acceptable just to GPF out if passed in a NULL? It's a BUG for all our use cases. And would save a conditional every chain step. > + s.max = s.curr = s.sgp->offset; > + s.max += s.sgp->length; > + if (dma) > + s.ix.dma = sg_dma_address(s.sgp); > + else > + s.ix.pfn = page_to_pfn(sg_page(s.sgp)); > + } > + > + return s; > +} > + > +/** > + * for_each_sgt_dma - iterate over the DMA addresses of the given sg_table > + * @__dmap: DMA address (output) > + * @__iter: 'struct sgt_iter' (iterator state, internal) > + * @__sgt: sg_table to iterate over (input) > + */ > +#define for_each_sgt_dma(__dmap, __iter, __sgt) > \ > + for ((__iter) = __sgt_iter((__sgt)->sgl, true); \ > + ((__dmap) = (__iter).ix.dma + (__iter).curr); \ This prevents us using dma_addr_t 0, which is the error value iirc, so ok. > + (((__iter).curr += PAGE_SIZE) < (__iter).max) || \ > + ((__iter) = __sgt_iter(sg_next((__iter).sgp), true), 0)) > + > +/** > + * for_each_sgt_page - iterate over the pages of the given sg_table > + * @__pp:page pointer (output) > + * @__iter: 'struct sgt_iter' (iterator state, internal) > + * @__sgt: sg_table to iterate over (input) > + */ > +#define for_each_sgt_page(__pp, __iter, __sgt) > \ > + for ((__iter) = __sgt_iter((__sgt)->sgl, false);\ > + ((__pp) = (__iter).ix.pfn == 0 ? NULL :\ > +pfn_to_page((__iter).ix.pfn + ((__iter).curr >> > PAGE_SHIFT)));\ If we shifted in the __sgt_iter() we could do simpler instructions inside the loop. The compiler won't know that pfn_to_page() is always true, so maybe a pfn_to_page(),1 here will help? > + (((__iter).curr += PAGE_SIZE) < (__iter).max) || \ > + ((__iter) =
Re: [Intel-gfx] [PATCH] drm/i915/psr: Implement PSR2 w/a for gen9
On Thu, May 19, 2016 at 02:25:59PM +0530, Jindal, Sonika wrote: > Looks good to me. > > Reviewed-by: Sonika JindalOk, merged 2-7 except the aux handshake one - that needs the fixup from Ville's series and that one needs an ack from Dave for the helper patches still. -Daniel > > > > On 5/19/2016 12:44 PM, Daniel Vetter wrote: > >Found this while browsing Bspec. Looks like it applies to both skl and > >kbl. > > > >v2: Also for bxt (Art). > > > >Cc: Rodrigo Vivi > >Cc: Sonika Jindal > >Cc: Durgadoss R > >Cc: "Pandiyan, Dhinakaran" > >Cc: "Runyan, Arthur J" > >Signed-off-by: Daniel Vetter > >--- > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > drivers/gpu/drm/i915/intel_pm.c | 17 +++-- > > 2 files changed, 16 insertions(+), 2 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/i915_reg.h > >b/drivers/gpu/drm/i915/i915_reg.h > >index 0f99e67f2114..c51368744e9e 100644 > >--- a/drivers/gpu/drm/i915/i915_reg.h > >+++ b/drivers/gpu/drm/i915/i915_reg.h > >@@ -6043,6 +6043,7 @@ enum skl_disp_power_wells { > > #define CHICKEN_PAR1_1 _MMIO(0x42080) > > #define DPA_MASK_VBLANK_SRD (1 << 15) > > #define FORCE_ARB_IDLE_PLANES (1 << 14) > >+#define SKL_EDP_PSR_FIX_RDWRAP (1 << 3) > > #define _CHICKEN_PIPESL_1_A0x420b0 > > #define _CHICKEN_PIPESL_1_B0x420b4 > >diff --git a/drivers/gpu/drm/i915/intel_pm.c > >b/drivers/gpu/drm/i915/intel_pm.c > >index e0d5405a8b15..8d42261daf1f 100644 > >--- a/drivers/gpu/drm/i915/intel_pm.c > >+++ b/drivers/gpu/drm/i915/intel_pm.c > >@@ -58,6 +58,10 @@ static void bxt_init_clock_gating(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > >+/* See Bspec note for PSR2_CTL bit 31, Wa#828:bxt */ > >+I915_WRITE(CHICKEN_PAR1_1, > >+ I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP); > >+ > > /* WaDisableSDEUnitClockGating:bxt */ > > I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) | > >GEN8_SDEUNIT_CLOCK_GATE_DISABLE); > >@@ -6845,6 +6849,15 @@ static void gen8_set_l3sqc_credits(struct > >drm_i915_private *dev_priv, > > I915_WRITE(GEN7_MISCCPCTL, misccpctl); > > } > >+static void skylake_init_clock_gating(struct drm_device *dev) > >+{ > >+struct drm_i915_private *dev_priv = dev->dev_private; > >+ > >+/* See Bspec note for PSR2_CTL bit 31, Wa#828:skl,kbl */ > >+I915_WRITE(CHICKEN_PAR1_1, > >+ I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP); > >+} > >+ > > static void broadwell_init_clock_gating(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > >@@ -7307,9 +7320,9 @@ static void nop_init_clock_gating(struct drm_device > >*dev) > > void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv) > > { > > if (IS_SKYLAKE(dev_priv)) > >-dev_priv->display.init_clock_gating = nop_init_clock_gating; > >+dev_priv->display.init_clock_gating = skylake_init_clock_gating; > > else if (IS_KABYLAKE(dev_priv)) > >-dev_priv->display.init_clock_gating = nop_init_clock_gating; > >+dev_priv->display.init_clock_gating = skylake_init_clock_gating; > > else if (IS_BROXTON(dev_priv)) > > dev_priv->display.init_clock_gating = bxt_init_clock_gating; > > else if (IS_BROADWELL(dev_priv)) > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Ro.CI.BAT: warning for series starting with [1/7] drm/i915: Enable edp psr error interrupts on hsw (rev2)
On Thu, May 19, 2016 at 08:01:33AM -, Patchwork wrote: > == Series Details == > > Series: series starting with [1/7] drm/i915: Enable edp psr error interrupts > on hsw (rev2) > URL : https://patchwork.freedesktop.org/series/7357/ > State : warning > > == Summary == > > Series 7357v2 Series without cover letter > http://patchwork.freedesktop.org/api/1.0/series/7357/revisions/2/mbox > > Test drv_hangman: > Subgroup error-state-basic: > fail -> PASS (ro-ilk1-i5-650) > Test kms_pipe_crc_basic: > Subgroup hang-read-crc-pipe-a: > pass -> DMESG-WARN (ro-ivb2-i7-3770) This is a pretty good wtf since blanantly impossible. We have not bug for this, but I think I've seen it in other places. I'm creating a patch to make this code more resilient, hopefully that helps. Definitely unrelated to PSR since ivb just plain doesn't have PSR ... -Daniel > > ro-bdw-i5-5250u total:219 pass:181 dwarn:0 dfail:0 fail:0 skip:38 > ro-bdw-i7-5557U total:219 pass:206 dwarn:0 dfail:0 fail:0 skip:13 > ro-bdw-i7-5600u total:219 pass:187 dwarn:0 dfail:0 fail:0 skip:32 > ro-bsw-n3050 total:219 pass:174 dwarn:0 dfail:0 fail:3 skip:42 > ro-byt-n2820 total:218 pass:175 dwarn:0 dfail:0 fail:2 skip:41 > ro-hsw-i3-4010u total:218 pass:193 dwarn:0 dfail:0 fail:0 skip:25 > ro-hsw-i7-4770r total:219 pass:194 dwarn:0 dfail:0 fail:0 skip:25 > ro-ilk-i7-620lm total:219 pass:151 dwarn:0 dfail:0 fail:1 skip:67 > ro-ilk1-i5-650 total:214 pass:152 dwarn:0 dfail:0 fail:1 skip:61 > ro-ivb-i7-3770 total:219 pass:183 dwarn:0 dfail:0 fail:0 skip:36 > ro-ivb2-i7-3770 total:219 pass:186 dwarn:1 dfail:0 fail:0 skip:32 > ro-snb-i7-2620M total:219 pass:177 dwarn:0 dfail:0 fail:1 skip:41 > > Results at /archive/results/CI_IGT_test/RO_Patchwork_934/ > > a2499a0 drm-intel-nightly: 2016y-05m-18d-17h-17m-55s UTC integration manifest > 7f28bec drm/i915/psr: Implement PSR2 w/a for gen9 > 3ed1f4a drm/i915/psr: Use ->get_aux_send_ctl functions > 89aa4b8 drm/i915/psr: Order DP aux transactions correctly > f2227a1 drm/i915/psr: Skip aux handeshake if the vbt tells us to > 63132de drm/i915/psr: Make idle_frames sensible again > c77bcd8 drm/i915/psr: Try to program link training times correctly > 068aab2 drm/i915: Enable edp psr error interrupts on hsw > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 4/4] Try inlining sg_next()
On Fri, May 20, 2016 at 01:31:30AM +0100, Dave Gordon wrote: > Signed-off-by: Dave Gordon> Cc: Chris Wilson Much better. The effect of the inline gem:exec:fault:1MiB:+4.90% gem:exec:fault:1MiB:forked:+7.99% gem:exec:fault:16MiB: +22.94% gem:exec:fault:16MiB:forked: +19.96% gem:exec:fault:256MiB: +27.45% gem:exec:fault:256MiB:forked: +36.89% And it brings this series into parity with mine. Avoiding the out-of-line call to sg_next() reduces the kernel execution overhead by 10% in some workloads (for example the Unreal Engine 4 demo Atlantis on 2GiB GTTs) which are dominated by the cost of inserting PTEs due to texture thrashing. We can demonstrate this in a microbenchmark that forces us to rebind the object on every execbuf, where we can measure a 25% improvement, in the time required to execute an execbuf requiring a texture to be rebound, for inlining the sg_next() for large texture sizes. Benchmark: igt/benchmarks/gem_exec_fault Benchmark: igt/benchmarks/gem_exec_trace/Atlantis -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915/psr: Try to program link training times correctly
On Thu, May 19, 2016 at 04:20:02PM +0530, Jindal, Sonika wrote: > > > On 5/18/2016 10:17 PM, Daniel Vetter wrote: > >Oops. Hw default for programming these fields to 0 is "skip link > >training". Display won't take that too well usually. > But we were defaulting it to value 0, which means 500us for both TP1 and TP2 > or TP3 time. > I dont think it means skip link training. This is just to set the time for > the patterns. > Skip aux handshake can happen if bit 12 of SRD_CTL is set. > > Does this solution help in fixing the bug mentioned here? See the other thread, I misread and yes it does help. -Daniel > > > > >v2: Unbotch the math a bit. > > > >v3: Drop debug hunk. > > > >Tested-by: Lyude> >Cc: Lyude > >Cc: sta...@vger.kernel.org > >Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176 > >Cc: Rodrigo Vivi > >Cc: Sonika Jindal > >Cc: Durgadoss R > >Cc: "Pandiyan, Dhinakaran" > >Signed-off-by: Daniel Vetter > >--- > > drivers/gpu/drm/i915/intel_psr.c | 55 > > ++-- > > 1 file changed, 47 insertions(+), 8 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/intel_psr.c > >b/drivers/gpu/drm/i915/intel_psr.c > >index c3abae4bc596..a788d1e9589b 100644 > >--- a/drivers/gpu/drm/i915/intel_psr.c > >+++ b/drivers/gpu/drm/i915/intel_psr.c > >@@ -280,7 +280,10 @@ static void hsw_psr_enable_source(struct intel_dp > >*intel_dp) > > * with the 5 or 6 idle patterns. > > */ > > uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames); > >-uint32_t val = 0x0; > >+uint32_t val = EDP_PSR_ENABLE; > >+ > >+val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT; > >+val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT; > > if (IS_HASWELL(dev)) > > val |= EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES; > >@@ -288,14 +291,50 @@ static void hsw_psr_enable_source(struct intel_dp > >*intel_dp) > > if (dev_priv->psr.link_standby) > > val |= EDP_PSR_LINK_STANDBY; > >-I915_WRITE(EDP_PSR_CTL, val | > >- max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT | > >- idle_frames << EDP_PSR_IDLE_FRAME_SHIFT | > >- EDP_PSR_ENABLE); > >+if (dev_priv->vbt.psr.tp1_wakeup_time > 5) > >+val |= EDP_PSR_TP1_TIME_2500us; > >+else if (dev_priv->vbt.psr.tp1_wakeup_time > 1) > >+val |= EDP_PSR_TP1_TIME_500us; > >+else if (dev_priv->vbt.psr.tp1_wakeup_time > 0) > >+val |= EDP_PSR_TP1_TIME_100us; > >+else > >+val |= EDP_PSR_TP1_TIME_0us; > >+ > >+if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5) > >+val |= EDP_PSR_TP2_TP3_TIME_2500us; > >+else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1) > >+val |= EDP_PSR_TP2_TP3_TIME_500us; > >+else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0) > >+val |= EDP_PSR_TP2_TP3_TIME_100us; > >+else > >+val |= EDP_PSR_TP2_TP3_TIME_0us; > >+ > >+if (intel_dp_source_supports_hbr2(intel_dp) && > >+drm_dp_tps3_supported(intel_dp->dpcd)) > >+val |= EDP_PSR_TP1_TP3_SEL; > >+else > >+val |= EDP_PSR_TP1_TP2_SEL; > >+ > >+I915_WRITE(EDP_PSR_CTL, val); > >+ > >+if (!dev_priv->psr.psr2_support) > >+return; > >+ > >+/* FIXME: selective update is probably totally broken because it doesn't > >+ * mesh at all with our frontbuffer tracking. And the hw alone isn't > >+ * good enough. */ > >+val = EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE; > >+ > >+if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5) > >+val |= EDP_PSR2_TP2_TIME_2500; > >+else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1) > >+val |= EDP_PSR2_TP2_TIME_500; > >+else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0) > >+val |= EDP_PSR2_TP2_TIME_100; > >+else > >+val |= EDP_PSR2_TP2_TIME_50; > >-if (dev_priv->psr.psr2_support) > >-I915_WRITE(EDP_PSR2_CTL, EDP_PSR2_ENABLE | > >-EDP_SU_TRACK_ENABLE | EDP_PSR2_TP2_TIME_100); > >+I915_WRITE(EDP_PSR2_CTL, val); > > } > > static bool intel_psr_match_conditions(struct intel_dp *intel_dp) > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx