Re: [Intel-gfx] [PATCH v2] drm/i915: Introduce an internal allocator for disposable private objects
On la, 2016-10-08 at 09:34 +0100, Chris Wilson wrote: > Quite a few of our objects used for internal hardware programming do not > benefit from being swappable or from being zero initialised. As such > they do not benefit from using a shmemfs backing storage and since they > are internal and never directly exposed to the user, we do not need to > worry about providing a filp. For these we can use an > drm_i915_gem_object wrapper around a sg_table of plain struct page. They > are not swap backed and not automatically pinned. If they are reaped > by the shrinker, the pages are released and the contents discarded. For > the internal use case, this is fine as for example, ringbuffers are > pinned from being written by a request to be read by the hardware. Once > they are idle, they can be discarded entirely. As such they are a good > match for execlist ringbuffers and a small variety of other internal > objects. > > In the first iteration, this is limited to the scratch batch buffers we > use (for command parsing and state initialisation). > > v2: Allocate physically contiguous pages, where possible. Does not hurt, but what does it gain us, exactly? > > Signed-off-by: Chris Wilson > Cc: Tvrtko Ursulin Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/9] drm/i915: Shrink cxsr_latency_table
On pe, 2016-10-07 at 14:34 +0100, Tvrtko Ursulin wrote: > @@ -807,14 +807,14 @@ struct intel_watermark_params { > > }; > > struct cxsr_latency { > - int is_desktop; > - int is_ddr3; > - unsigned long fsb_freq; > - unsigned long mem_freq; > - unsigned long display_sr; > - unsigned long display_hpll_disable; > - unsigned long cursor_sr; > - unsigned long cursor_hpll_disable; > + bool is_desktop : 1; > + bool is_ddr3 : 1; bitfields to the end of struct, with that; Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ 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: Shrink sdvo_cmd_names
On pe, 2016-10-07 at 14:34 +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Pack the struct _sdvo_cmd_name to save 736 bytes of .rodata. > > This is fine since the name pointers are used only for debug. > > Signed-off-by: Tvrtko Ursulin Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ 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: Shrink per-platform watermark configuration
On pe, 2016-10-07 at 14:34 +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Use types of more appropriate size in struct > intel_watermark_params to save 512 bytes of .rodata. > > Signed-off-by: Tvrtko Ursulin > Acked-by: Ville Syrjälä No changelog, so assuming equal to trybot one; Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ 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: Shrink TV modes const data
On pe, 2016-10-07 at 14:34 +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Make struct video_levels and struct tv_mode use data types > of sufficient width to save approximately one kilobyte in > the .rodata section. > > Signed-off-by: Tvrtko Ursulin Would it hurt us to make the struct packed? I can see why not to reorder the struct (although, it uses designated initializers, so it should not matter in that sense). Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for series starting with [01/42] drm/i915: Allow disabling error capture
== Series Details == Series: series starting with [01/42] drm/i915: Allow disabling error capture URL : https://patchwork.freedesktop.org/series/13436/ State : warning == Summary == Series 13436v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/13436/revisions/1/mbox/ Test drv_module_reload_basic: pass -> DMESG-WARN (fi-skl-6770hq) dmesg-warn -> PASS (fi-ilk-650) Test kms_pipe_crc_basic: Subgroup hang-read-crc-pipe-b: pass -> DMESG-WARN (fi-skl-6700k) Test vgem_basic: Subgroup unload: skip -> PASS (fi-byt-n2820) skip -> PASS (fi-bsw-n3050) pass -> SKIP (fi-skl-6770hq) skip -> PASS (fi-hsw-4770) fi-bdw-5557u total:248 pass:231 dwarn:0 dfail:0 fail:0 skip:17 fi-bsw-n3050 total:248 pass:205 dwarn:0 dfail:0 fail:0 skip:43 fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31 fi-byt-j1900 total:248 pass:214 dwarn:1 dfail:0 fail:1 skip:32 fi-byt-n2820 total:248 pass:211 dwarn:0 dfail:0 fail:1 skip:36 fi-hsw-4770 total:248 pass:225 dwarn:0 dfail:0 fail:0 skip:23 fi-hsw-4770r total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-ilk-650 total:248 pass:185 dwarn:0 dfail:0 fail:2 skip:61 fi-ivb-3520m total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27 fi-ivb-3770 total:248 pass:207 dwarn:0 dfail:0 fail:0 skip:41 fi-skl-6260u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-skl-6700hqtotal:248 pass:224 dwarn:1 dfail:0 fail:0 skip:23 fi-skl-6700k total:248 pass:221 dwarn:2 dfail:0 fail:0 skip:25 fi-skl-6770hqtotal:248 pass:229 dwarn:2 dfail:0 fail:1 skip:16 fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37 fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39 Results at /archive/results/CI_IGT_test/Patchwork_2647/ 81b22c4383bfe6290f7b9d821bf56768567c1718 drm-intel-nightly: 2016y-10m-07d-07h-29m-30s UTC integration manifest 33cf896 drm/i915: Support explicit fencing for execbuf 01f0787 drm/i915: Enable userspace to opt-out of implicit fencing 0815015 drm/i915: Enable multiple timelines 6409884 drm/i915: Defer setting of global seqno on request to submission 3f9e348 drm/i915: Reserve space in the global seqno during request allocation d9ec193 drm/i915: Create a unique name for the context b3bebb7 drm/i915: Move the global sync optimisation to the timeline 48703d4 drm/i915: Defer breadcrumb emission d673086 drm/i915: Record space required for breadcrumb emission 2493555 drm/i915: Rename ->emit_request to ->emit_breadcrumb 46618d0 drm/i915: Introduce a global_seqno for each request 454114b drm/i915: Wait first for submission, before waiting for request completion 271653c drm/i915: Queue the idling context switch after all other timelines a8990af drm/i915: Combine seqno + tracking into a global timeline struct 0c4f326 drm/i915: Restore nonblocking awaits for modesetting 609dbff drm: Add reference counting to drm_atomic_state a5cd0fa drm/i915: Move GEM activity tracking into a common struct reservation_object 6dfa747 drm/i915: Use lockless object free 51be5fa drm/i915: Treat a framebuffer reference as an active reference whilst shrinking 89cefa9 drm/i915: Move object release to a freelist + worker af3efa6 drm/i915: Acquire the backing storage outside of struct_mutex in set-domain bdaf19a drm/i915: Implement pwrite without struct-mutex 8856275 drm/i915: Implement pread without struct-mutex 3bc7efe drm/i915/dmabuf: Acquire the backing storage outside of struct_mutex 8824701 drm/i915: Move object backing storage manipulation to its own locking b4f1045 drm/i915: Pass around sg_table to get_pages/put_pages backend e9184f6 drm/i915: Refactor object page API 897bc5e drm/i915: Use radixtree to jump start intel_partial_pages() 2885c3f drm/i915: Use a radixtree for random access to the object's backing storage ba563be drm/i915: Markup GEM API with lockdep asserts 2c12286 drm/i915: Reuse the active golden render state batch 7a26c4d0 drm/i915: Introduce an internal allocator for disposable private objects 5f0cb1d drm/i915: Defer active reference until required 5dafc01 drm/i915: Remove unused i915_gem_active_wait() in favour of _unlocked() 21447b1 drm/i915: Rearrange i915_wait_request() accounting with callers 983cd49 drm/i915: Allow i915_sw_fence_await_sw_fence() to allocate 42b0316 drm/i915: Support asynchronous waits on struct fence from i915_gem_request 16d0a2a drm/i915: Compress GPU objects in error state 5b2d629 drm/i915: Consolidate error object printing d5c6209f drm/i915: Always use the GTT for error capture 6220554 drm/i915: Stop the machine whilst capturing the GPU crash dump a8d19e4 drm/i915: Allow disabling error capture
Re: [Intel-gfx] [PATCH 5/9] drm/i915: Use unsigned int for latencies
On pe, 2016-10-07 at 14:34 +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Signed-off-by: Tvrtko Ursulin Reviewed-by: Joonas Lahtinen And maybe Suggested-by? :) Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ 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: unsigned int is enough for crtc clock
On pe, 2016-10-07 at 14:34 +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Signed-off-by: Tvrtko Ursulin Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/9] drm/i915: Convert get_fifo_size return from int to unsigned int
On pe, 2016-10-07 at 14:34 +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Unsigned is a more appropriate data type in this case. > > Signed-off-by: Tvrtko Ursulin Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/9] drm/i915: Make intel_calculate_wm return unsigned int
On pe, 2016-10-07 at 14:34 +0100, Tvrtko Ursulin wrote: > @@ -595,18 +595,17 @@ static unsigned long intel_calculate_wm(unsigned int > clock_in_khz, > * clocks go from a few thousand to several hundred thousand. > * latency is usually a few thousand > */ > - entries_required = ((clock_in_khz / 1000) * cpp * latency_ns) / > - 1000; > + entries_required = ((clock_in_khz / 1000) * cpp * latency_ns) / 1000; Is the intermediary result always within int? > entries_required = DIV_ROUND_UP(entries_required, wm->cacheline_size); > > - DRM_DEBUG_KMS("FIFO entries required for mode: %ld\n", > entries_required); > + DRM_DEBUG_KMS("FIFO entries required for mode: %d\n", entries_required); > > wm_size = fifo_size - (entries_required + wm->guard_size); > > - DRM_DEBUG_KMS("FIFO watermark level: %ld\n", wm_size); > + DRM_DEBUG_KMS("FIFO watermark level: %d\n", wm_size); > > /* Don't promote wm_size to unsigned... */ > - if (wm_size > (long)wm->max_wm) > + if (wm_size > (int)wm->max_wm) > wm_size = wm->max_wm; > if (wm_size <= 0) > wm_size = wm->default_wm; This could be if (wm_size <= 0) wm_size = wm->default_wm; else if (wm_size > U16_MAX || (u16)wm_size > wm->max_wm) wm_size = wm->max_wm; or something? Other than that, types look better that they used to. Reviewed-by: Joonas Lahtinen The whole type landscape in watermark code seems bit sloppy to me. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ 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: Tidy watermark computation local types
On pe, 2016-10-07 at 18:16 +0300, Ville Syrjälä wrote: > On Fri, Oct 07, 2016 at 03:51:50PM +0100, Tvrtko Ursulin wrote: > > > > I tried to be careful, but it is of course possible I've missed > > something. Could you give me a more precise pointer on where exactly to > > look? In this particular patch? > > I don't have specifics in mind right now. But in general I've become a > bit wary of unsigned in my older days on account of integer promotions > and arithmetic conversions. It's far too easy to end up with an unsigned > value where signed was needed. Yes, if one is not paying attention to the maximum expected values. And looking back a few months, paying attention is what our watermark code needs badly. By quick look, some types are random and there're quite a few casts. Try "git grep max_wm" for example. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Introduce an internal allocator for disposable private objects
On 08/10/2016 09:34, Chris Wilson wrote: Quite a few of our objects used for internal hardware programming do not benefit from being swappable or from being zero initialised. As such they do not benefit from using a shmemfs backing storage and since they are internal and never directly exposed to the user, we do not need to worry about providing a filp. For these we can use an drm_i915_gem_object wrapper around a sg_table of plain struct page. They are not swap backed and not automatically pinned. If they are reaped by the shrinker, the pages are released and the contents discarded. For the internal use case, this is fine as for example, ringbuffers are pinned from being written by a request to be read by the hardware. Once they are idle, they can be discarded entirely. As such they are a good match for execlist ringbuffers and a small variety of other internal objects. In the first iteration, this is limited to the scratch batch buffers we use (for command parsing and state initialisation). v2: Allocate physically contiguous pages, where possible. Since the allocator will be used constantly at runtime, my recollection is that higher order allocations can easily become next to impossible, so I am wondering why? Also, on your last reply you did not write why you think this is interesting to try? Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/Makefile| 1 + drivers/gpu/drm/i915/i915_drv.h | 5 + drivers/gpu/drm/i915/i915_gem_batch_pool.c | 28 ++--- drivers/gpu/drm/i915/i915_gem_internal.c | 162 +++ drivers/gpu/drm/i915/i915_gem_render_state.c | 2 +- drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ++- 7 files changed, 190 insertions(+), 24 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_gem_internal.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index a998c2bce70a..b94a90f34d2d 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -35,6 +35,7 @@ i915-y += i915_cmd_parser.o \ i915_gem_execbuffer.o \ i915_gem_fence.o \ i915_gem_gtt.o \ + i915_gem_internal.o \ i915_gem.o \ i915_gem_render_state.o \ i915_gem_request.o \ diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index fee5cc92e2f2..bad97f1e5265 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3538,6 +3538,11 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, u32 gtt_offset, u32 size); +/* i915_gem_internal.c */ +struct drm_i915_gem_object * +i915_gem_object_create_internal(struct drm_device *dev, + unsigned int size); + /* i915_gem_shrinker.c */ unsigned long i915_gem_shrink(struct drm_i915_private *dev_priv, unsigned long target, diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.c b/drivers/gpu/drm/i915/i915_gem_batch_pool.c index cb25cad3318c..3934c9103cf2 100644 --- a/drivers/gpu/drm/i915/i915_gem_batch_pool.c +++ b/drivers/gpu/drm/i915/i915_gem_batch_pool.c @@ -97,9 +97,9 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool, size_t size) { struct drm_i915_gem_object *obj = NULL; - struct drm_i915_gem_object *tmp, *next; + struct drm_i915_gem_object *tmp; struct list_head *list; - int n; + int n, ret; lockdep_assert_held(&pool->engine->i915->drm.struct_mutex); @@ -112,19 +112,12 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool, n = ARRAY_SIZE(pool->cache_list) - 1; list = &pool->cache_list[n]; - list_for_each_entry_safe(tmp, next, list, batch_pool_link) { + list_for_each_entry(tmp, list, batch_pool_link) { /* The batches are strictly LRU ordered */ if (!i915_gem_active_is_idle(&tmp->last_read[pool->engine->id], &tmp->base.dev->struct_mutex)) break; - /* While we're looping, do some clean up */ - if (tmp->madv == __I915_MADV_PURGED) { - list_del(&tmp->batch_pool_link); - i915_gem_object_put(tmp); - continue; - } - if (tmp->base.size >= size) { obj = tmp; break; @@ -132,19 +125,16 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool, } if (obj == NULL) { - int ret; - - obj = i915_gem_object_create(&pool->engine->i915->drm, size); + obj = i915_gem_object_create_internal(&pool->engine->i915->drm, +
Re: [Intel-gfx] [PATCH v2] drm/i915: Introduce an internal allocator for disposable private objects
On Mon, Oct 10, 2016 at 09:11:49AM +0100, Tvrtko Ursulin wrote: > On 08/10/2016 09:34, Chris Wilson wrote: > >Quite a few of our objects used for internal hardware programming do not > >benefit from being swappable or from being zero initialised. As such > >they do not benefit from using a shmemfs backing storage and since they > >are internal and never directly exposed to the user, we do not need to > >worry about providing a filp. For these we can use an > >drm_i915_gem_object wrapper around a sg_table of plain struct page. They > >are not swap backed and not automatically pinned. If they are reaped > >by the shrinker, the pages are released and the contents discarded. For > >the internal use case, this is fine as for example, ringbuffers are > >pinned from being written by a request to be read by the hardware. Once > >they are idle, they can be discarded entirely. As such they are a good > >match for execlist ringbuffers and a small variety of other internal > >objects. > > > >In the first iteration, this is limited to the scratch batch buffers we > >use (for command parsing and state initialisation). > > > >v2: Allocate physically contiguous pages, where possible. > > Since the allocator will be used constantly at runtime, my > recollection is that higher order allocations can easily become next > to impossible, so I am wondering why? Also, on your last reply you > did not write why you think this is interesting to try? > > >Signed-off-by: Chris Wilson > >Cc: Tvrtko Ursulin > >--- > > drivers/gpu/drm/i915/Makefile| 1 + > > drivers/gpu/drm/i915/i915_drv.h | 5 + > > drivers/gpu/drm/i915/i915_gem_batch_pool.c | 28 ++--- > > drivers/gpu/drm/i915/i915_gem_internal.c | 162 > > +++ > > drivers/gpu/drm/i915/i915_gem_render_state.c | 2 +- > > drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- > > drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ++- > > 7 files changed, 190 insertions(+), 24 deletions(-) > > create mode 100644 drivers/gpu/drm/i915/i915_gem_internal.c > > > >diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > >index a998c2bce70a..b94a90f34d2d 100644 > >--- a/drivers/gpu/drm/i915/Makefile > >+++ b/drivers/gpu/drm/i915/Makefile > >@@ -35,6 +35,7 @@ i915-y += i915_cmd_parser.o \ > > i915_gem_execbuffer.o \ > > i915_gem_fence.o \ > > i915_gem_gtt.o \ > >+ i915_gem_internal.o \ > > i915_gem.o \ > > i915_gem_render_state.o \ > > i915_gem_request.o \ > >diff --git a/drivers/gpu/drm/i915/i915_drv.h > >b/drivers/gpu/drm/i915/i915_drv.h > >index fee5cc92e2f2..bad97f1e5265 100644 > >--- a/drivers/gpu/drm/i915/i915_drv.h > >+++ b/drivers/gpu/drm/i915/i915_drv.h > >@@ -3538,6 +3538,11 @@ i915_gem_object_create_stolen_for_preallocated(struct > >drm_device *dev, > >u32 gtt_offset, > >u32 size); > >+/* i915_gem_internal.c */ > >+struct drm_i915_gem_object * > >+i915_gem_object_create_internal(struct drm_device *dev, > >+unsigned int size); > >+ > > /* i915_gem_shrinker.c */ > > unsigned long i915_gem_shrink(struct drm_i915_private *dev_priv, > > unsigned long target, > >diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.c > >b/drivers/gpu/drm/i915/i915_gem_batch_pool.c > >index cb25cad3318c..3934c9103cf2 100644 > >--- a/drivers/gpu/drm/i915/i915_gem_batch_pool.c > >+++ b/drivers/gpu/drm/i915/i915_gem_batch_pool.c > >@@ -97,9 +97,9 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool, > > size_t size) > > { > > struct drm_i915_gem_object *obj = NULL; > >-struct drm_i915_gem_object *tmp, *next; > >+struct drm_i915_gem_object *tmp; > > struct list_head *list; > >-int n; > >+int n, ret; > > lockdep_assert_held(&pool->engine->i915->drm.struct_mutex); > >@@ -112,19 +112,12 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool > >*pool, > > n = ARRAY_SIZE(pool->cache_list) - 1; > > list = &pool->cache_list[n]; > >-list_for_each_entry_safe(tmp, next, list, batch_pool_link) { > >+list_for_each_entry(tmp, list, batch_pool_link) { > > /* The batches are strictly LRU ordered */ > > if (!i915_gem_active_is_idle(&tmp->last_read[pool->engine->id], > > &tmp->base.dev->struct_mutex)) > > break; > >-/* While we're looping, do some clean up */ > >-if (tmp->madv == __I915_MADV_PURGED) { > >-list_del(&tmp->batch_pool_link); > >-i915_gem_object_put(tmp); > >-continue; > >-} > >- > > if (tmp->base.size >= size) { > > obj = tmp; > > break; > >@@ -132,19 +125,16 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool > >*pool, > >
[Intel-gfx] ✗ Fi.CI.BAT: warning for .rodata diet 2 (non-disruptive version)
== Series Details == Series: .rodata diet 2 (non-disruptive version) URL : https://patchwork.freedesktop.org/series/13445/ State : warning == Summary == Series 13445v1 .rodata diet 2 (non-disruptive version) https://patchwork.freedesktop.org/api/1.0/series/13445/revisions/1/mbox/ Results at /archive/results/CI_IGT_test/Patchwork_2648/ 0f2a32f6bdc7590370d79a8ec5be27a8762f1d91 drm-intel-nightly: 2016y-10m-10d-05h-29m-53s UTC integration manifest cdb5478 drm/i915: Tidy watermark computation local types 4e9225d drm/i915: Make intel_calculate_wm return unsigned int 08ef44d drm/i915: Convert get_fifo_size return from int to unsigned int 359176c drm/i915: unsigned int is enough for crtc clock 2442f60 drm/i915: Use unsigned int for latencies cbb8eab drm/i915: Shrink TV modes const data 40148c9 drm/i915: Shrink per-platform watermark configuration b0fbaa3 drm/i915: Shrink sdvo_cmd_names 34dbd0b drm/i915: Shrink cxsr_latency_table ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Introduce an internal allocator for disposable private objects
On 10/10/2016 09:19, Chris Wilson wrote: On Mon, Oct 10, 2016 at 09:11:49AM +0100, Tvrtko Ursulin wrote: On 08/10/2016 09:34, Chris Wilson wrote: Quite a few of our objects used for internal hardware programming do not benefit from being swappable or from being zero initialised. As such they do not benefit from using a shmemfs backing storage and since they are internal and never directly exposed to the user, we do not need to worry about providing a filp. For these we can use an drm_i915_gem_object wrapper around a sg_table of plain struct page. They are not swap backed and not automatically pinned. If they are reaped by the shrinker, the pages are released and the contents discarded. For the internal use case, this is fine as for example, ringbuffers are pinned from being written by a request to be read by the hardware. Once they are idle, they can be discarded entirely. As such they are a good match for execlist ringbuffers and a small variety of other internal objects. In the first iteration, this is limited to the scratch batch buffers we use (for command parsing and state initialisation). v2: Allocate physically contiguous pages, where possible. Since the allocator will be used constantly at runtime, my recollection is that higher order allocations can easily become next to impossible, so I am wondering why? Also, on your last reply you did not write why you think this is interesting to try? Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/Makefile| 1 + drivers/gpu/drm/i915/i915_drv.h | 5 + drivers/gpu/drm/i915/i915_gem_batch_pool.c | 28 ++--- drivers/gpu/drm/i915/i915_gem_internal.c | 162 +++ drivers/gpu/drm/i915/i915_gem_render_state.c | 2 +- drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ++- 7 files changed, 190 insertions(+), 24 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_gem_internal.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index a998c2bce70a..b94a90f34d2d 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -35,6 +35,7 @@ i915-y += i915_cmd_parser.o \ i915_gem_execbuffer.o \ i915_gem_fence.o \ i915_gem_gtt.o \ + i915_gem_internal.o \ i915_gem.o \ i915_gem_render_state.o \ i915_gem_request.o \ diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index fee5cc92e2f2..bad97f1e5265 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3538,6 +3538,11 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, u32 gtt_offset, u32 size); +/* i915_gem_internal.c */ +struct drm_i915_gem_object * +i915_gem_object_create_internal(struct drm_device *dev, + unsigned int size); + /* i915_gem_shrinker.c */ unsigned long i915_gem_shrink(struct drm_i915_private *dev_priv, unsigned long target, diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.c b/drivers/gpu/drm/i915/i915_gem_batch_pool.c index cb25cad3318c..3934c9103cf2 100644 --- a/drivers/gpu/drm/i915/i915_gem_batch_pool.c +++ b/drivers/gpu/drm/i915/i915_gem_batch_pool.c @@ -97,9 +97,9 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool, size_t size) { struct drm_i915_gem_object *obj = NULL; - struct drm_i915_gem_object *tmp, *next; + struct drm_i915_gem_object *tmp; struct list_head *list; - int n; + int n, ret; lockdep_assert_held(&pool->engine->i915->drm.struct_mutex); @@ -112,19 +112,12 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool, n = ARRAY_SIZE(pool->cache_list) - 1; list = &pool->cache_list[n]; - list_for_each_entry_safe(tmp, next, list, batch_pool_link) { + list_for_each_entry(tmp, list, batch_pool_link) { /* The batches are strictly LRU ordered */ if (!i915_gem_active_is_idle(&tmp->last_read[pool->engine->id], &tmp->base.dev->struct_mutex)) break; - /* While we're looping, do some clean up */ - if (tmp->madv == __I915_MADV_PURGED) { - list_del(&tmp->batch_pool_link); - i915_gem_object_put(tmp); - continue; - } - if (tmp->base.size >= size) { obj = tmp; break; @@ -132,19 +125,16 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool, } if (obj == NULL) { - int ret; - - obj = i915_gem_object_create(&pool->engine->i915->drm, size); + obj = i
[Intel-gfx] ✗ Fi.CI.BAT: failure for .rodata diet 2 (non-disruptive version)
== Series Details == Series: .rodata diet 2 (non-disruptive version) URL : https://patchwork.freedesktop.org/series/13445/ State : failure == Summary == Series 13445v1 .rodata diet 2 (non-disruptive version) https://patchwork.freedesktop.org/api/1.0/series/13445/revisions/1/mbox/ Test gem_busy: Subgroup basic-hang-default: pass -> FAIL (fi-hsw-4770r) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: pass -> DMESG-WARN (fi-byt-j1900) Test vgem_basic: Subgroup unload: pass -> SKIP (fi-ilk-650) skip -> PASS (fi-ivb-3770) skip -> PASS (fi-bsw-n3050) pass -> SKIP (fi-hsw-4770) skip -> PASS (fi-skl-6700k) pass -> SKIP (fi-ivb-3520m) fi-bdw-5557u total:248 pass:231 dwarn:0 dfail:0 fail:0 skip:17 fi-bsw-n3050 total:248 pass:205 dwarn:0 dfail:0 fail:0 skip:43 fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31 fi-byt-j1900 total:248 pass:213 dwarn:2 dfail:0 fail:1 skip:32 fi-byt-n2820 total:248 pass:211 dwarn:0 dfail:0 fail:1 skip:36 fi-hsw-4770 total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-hsw-4770r total:248 pass:223 dwarn:0 dfail:0 fail:1 skip:24 fi-ilk-650 total:248 pass:184 dwarn:0 dfail:0 fail:2 skip:62 fi-ivb-3520m total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27 fi-ivb-3770 total:248 pass:208 dwarn:0 dfail:0 fail:0 skip:40 fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-6260u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-skl-6700hqtotal:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-skl-6700k total:248 pass:222 dwarn:1 dfail:0 fail:0 skip:25 fi-skl-6770hqtotal:248 pass:230 dwarn:1 dfail:0 fail:1 skip:16 fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37 fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39 Results at /archive/results/CI_IGT_test/Patchwork_2648/ 0f2a32f6bdc7590370d79a8ec5be27a8762f1d91 drm-intel-nightly: 2016y-10m-10d-05h-29m-53s UTC integration manifest cdb5478 drm/i915: Tidy watermark computation local types 4e9225d drm/i915: Make intel_calculate_wm return unsigned int 08ef44d drm/i915: Convert get_fifo_size return from int to unsigned int 359176c drm/i915: unsigned int is enough for crtc clock 2442f60 drm/i915: Use unsigned int for latencies cbb8eab drm/i915: Shrink TV modes const data 40148c9 drm/i915: Shrink per-platform watermark configuration b0fbaa3 drm/i915: Shrink sdvo_cmd_names 34dbd0b drm/i915: Shrink cxsr_latency_table ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/9] drm/i915: Make intel_calculate_wm return unsigned int
On 10/10/2016 09:02, Joonas Lahtinen wrote: On pe, 2016-10-07 at 14:34 +0100, Tvrtko Ursulin wrote: @@ -595,18 +595,17 @@ static unsigned long intel_calculate_wm(unsigned int clock_in_khz, * clocks go from a few thousand to several hundred thousand. * latency is usually a few thousand */ - entries_required = ((clock_in_khz / 1000) * cpp * latency_ns) / - 1000; + entries_required = ((clock_in_khz / 1000) * cpp * latency_ns) / 1000; Is the intermediary result always within int? I certainly hope so! Display clock in MHz * cpp * latency which is u16. So 2^31 / 2^16 / cpp=8 leaves 4GHz for the clock before it would overflow. entries_required = DIV_ROUND_UP(entries_required, wm->cacheline_size); - DRM_DEBUG_KMS("FIFO entries required for mode: %ld\n", entries_required); + DRM_DEBUG_KMS("FIFO entries required for mode: %d\n", entries_required); wm_size = fifo_size - (entries_required + wm->guard_size); - DRM_DEBUG_KMS("FIFO watermark level: %ld\n", wm_size); + DRM_DEBUG_KMS("FIFO watermark level: %d\n", wm_size); /* Don't promote wm_size to unsigned... */ - if (wm_size > (long)wm->max_wm) + if (wm_size > (int)wm->max_wm) wm_size = wm->max_wm; if (wm_size <= 0) wm_size = wm->default_wm; This could be if (wm_size <= 0) wm_size = wm->default_wm; else if (wm_size > U16_MAX || (u16)wm_size > wm->max_wm) wm_size = wm->max_wm; or something? Other than that, types look better that they used to. Reviewed-by: Joonas Lahtinen The whole type landscape in watermark code seems bit sloppy to me. Yes, but I would also like not to introduce regressions. So I am a bit reluctant to push forward with the second half of this series. :I Regards, Tvrtko ___ 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: Use unsigned int for latencies
On 10/10/2016 08:24, Joonas Lahtinen wrote: On pe, 2016-10-07 at 14:34 +0100, Tvrtko Ursulin wrote: From: Tvrtko Ursulin Signed-off-by: Tvrtko Ursulin Reviewed-by: Joonas Lahtinen And maybe Suggested-by? :) Definitely, yes, should have put it on all of the related ones. Will correct. Regards, Tvrtko ___ 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: Shrink TV modes const data
On 10/10/2016 07:49, Jani Nikula wrote: On Fri, 07 Oct 2016, Tvrtko Ursulin wrote: From: Tvrtko Ursulin Make struct video_levels and struct tv_mode use data types of sufficient width to save approximately one kilobyte in the .rodata section. Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/intel_tv.c | 50 - 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c index 3988c45f9e5f..fd4d59341897 100644 --- a/drivers/gpu/drm/i915/intel_tv.c +++ b/drivers/gpu/drm/i915/intel_tv.c @@ -86,7 +86,8 @@ struct intel_tv { }; struct video_levels { - int blank, black, burst; + u16 blank, black; + u8 burst; }; struct color_conversion { @@ -339,34 +340,43 @@ static const struct video_levels component_levels = { struct tv_mode { const char *name; - int clock; - int refresh; /* in millihertz (for precision) */ - u32 oversample; - int hsync_end, hblank_start, hblank_end, htotal; - bool progressive, trilevel_sync, component_only; - int vsync_start_f1, vsync_start_f2, vsync_len; - bool veq_ena; - int veq_start_f1, veq_start_f2, veq_len; - int vi_end_f1, vi_end_f2, nbr_end; - bool burst_ena; - int hburst_start, hburst_len; - int vburst_start_f1, vburst_end_f1; - int vburst_start_f2, vburst_end_f2; - int vburst_start_f3, vburst_end_f3; - int vburst_start_f4, vburst_end_f4; + + u32 clock; + u16 refresh; /* in millihertz (for precision) */ + u32 oversample; + u8 hsync_end; + u16 hblank_start, hblank_end, htotal; + bool progressive : 1, trilevel_sync : 1, component_only : 1; + u8 vsync_start_f1, vsync_start_f2, vsync_len; + bool veq_ena : 1; + u8 veq_start_f1, veq_start_f2, veq_len; + u8 vi_end_f1, vi_end_f2; + u16 nbr_end; + bool burst_ena : 1; + u8 hburst_start, hburst_len; + u8 vburst_start_f1; + u16 vburst_end_f1; + u8 vburst_start_f2; + u16 vburst_end_f2; + u8 vburst_start_f3; + u16 vburst_end_f3; + u8 vburst_start_f4; + u16 vburst_end_f4; Not convinced about the indentation change. I found it much more readable like that in this case. I still do, but thinking about it more, maybe it is just because my editor does no syntax highlighting for u32 types. I need to fix that. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Regression report] Weekly regression report WW41
On Mon, 10 Oct 2016, Jairo Miramontes wrote: > This week's new regressions Per the earlier discussion, please add the "regression" keyword to the bugs (as well as "bisected" or "bisect_pending" as appropriate). Please make this report's information available as bugzilla query links. The links are going to be something obnoxious like this, but at least we can get to the results directly: https://bugs.freedesktop.org/buglist.cgi?bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&bug_status=NEEDINFO&bug_status=PLEASETEST&component=DRM%2FIntel&keywords=regression%2C%20bisected%2C%20bisect_pending%2C%20&keywords_type=anywords&product=DRI&query_format=advanced I want that kind of queries with up-to-date information. You can then add additional queries for recent or this week's lists as you wish, using bugzilla queries. The important part is keeping *bugzilla* up to date, not emailed lists. Thanks, Jani. > +---+---+++ > | BugId | Summary | Created on | > Bisect | > +---+---+++ > | 98036 | [BYT] constant screen flicker and rendering e | 2016-10-03 | > No | > +---+---+++ > > > Previous Regressions > +---+---+++ > | BugId | Summary | Created on | > Bisect | > +---+---+++ > | 90112 | [BSW bisected] OglGSCloth/Lightsmark/CS/ Port | 2015-04-20 | > Yes| > | 94590 | [KBL] igt/kms_fbcon_fbt/psr-suspend regressio | 2016-03-17 | > No | > | 93263 | 945GM regression since 4.3| 2015-12-05 | > No | > | 92237 | [SNB]Horrible noise (audio) via DisplayPort [ | 2015-10-02 | > No | > | 97918 | [bdw regression 4.8] Severe graphics regressi | 2016-09-25 | > No | > | 93486 | [HP Compaq dc7800 Small Form Factor PC][REGRE | 2015-12-23 | > No | > | 92414 | [Intel-gfx] As of kernel 4.3-rc1 system will | 2015-10-10 | > Yes| > | 92050 | [regression]/bug introduced by commit [0e572f | 2015-09-19 | > No | > | 93393 | Regression for Skylake modesetting in kernel | 2015-12-16 | > No | > | 93802 | [IVB bisected] switching to tty1 causes fifo | 2016-01-20 | > Yes| > | 95197 | [i915] regression in 4.6-rc5: GPU HANG: ecode | 2016-04-28 | > No | > | 96800 | [regression] The drm-intel-nightly branch no | 2016-07-04 | > No | > | 95736 | [IVB bisected] *ERROR* uncleared fifo underru | 2016-05-24 | > Yes| > | 89629 | [i965 regression]igt/kms_rotation_crc/sprite- | 2015-03-18 | > No | > | 97878 | [SKL][REGRESSION][BISECTED] Dropped frames an | 2016-09-20 | > Yes| > | 96938 | [HSW modeset regression] i915/drm locks up wh | 2016-07-15 | > No | > | 87131 | [PNV regression] igt/gem_exec_lut_handle take | 2014-12-09 | > No | > | 97994 | [REGRESS] [BISECT] [i915] Monitor resolution | 2016-09-30 | > Yes| > | 87726 | [BDW Bisected] OglDrvCtx performance reduced | 2014-12-26 | > Yes| > | 91974 | [bisected] unrecoverable black screen after k | 2015-09-11 | > Yes| > | 96645 | [regression 4.7] [BISECT]Low package c-states | 2016-06-22 | > Yes| > | 94430 | [HSW+nvidia] regression: display becomes "dis | 2016-03-07 | > No | > | 97573 | [IVB/SNB/BYT/HSW/BDW] GuC boot kernel command | 2016-09-02 | > No | > | 93971 | video framerate performance regression with U | 2016-02-02 | > No | > | 89872 | [ HSW Bisected ] VGA was white screen when re | 2015-04-02 | > Yes| > | 96428 | [IVB bisected] [drm:intel_dp_aux_ch] *ERROR* | 2016-06-07 | > Yes| > | 91959 | [865g Linux regression] GPU hang and disabled | 2015-09-10 | > No | > | 95097 | [hdmi regression ilk] no signal to TV | 2016-04-24 | > No | > | 97867 | [HSW][Regression] 4.6.7 and beyond causes scr | 2016-09-19 | > No | > | 97450 | [SKL] [regression] Random display flickering | 2016-08-23 | > No | > | 87725 | [BDW Bisected] OglBatch7 performance reduced | 2014-12-26 | > Yes| > | 94337 | Linux 4.5 regression: FIFO underruns on Skyla | 2016-02-29 | > No | > | 97820 | [BDW BYT SKL HSW IVB]Regression] [GPU Hang] w | 2016-09-15 | > No | > | 97017 | [SKL GT3e guc bisected] ~10% performance drop | 2016-07-21 | > Yes| > | 96916 | Regression: screen flashes with PSR enabled | 2016-07-13 | > No | > | 97939 | [BYT] gem_fence_upload subtest thread-content | 2016-09-26 | > No | > | 90368 | [SNB BSW SKL BXT KBL IVB] [Regression] bisect | 2015-05-08 | > Yes| > | 94588 | [IVB/KBL/BSW/BXT/BDW] igt/gem_reloc_overflow | 2016-03-17 | > No | > | 96736 | kernel 4.6 regression: PSR causes screen to f | 2016-06-29 | > No | > | 90134 | [BSW Bisected]GFXBench3_gl_drive
Re: [Intel-gfx] [PATCH 2/2] drm/i915/gen9: fix watermarks when using the pipe scaler
On Fri, 07 Oct 2016, "Zanoni, Paulo R" wrote: > Em Sex, 2016-10-07 às 17:13 -0300, Paulo Zanoni escreveu: >> Luckily, the necessary adjustments for when we're using the scaler >> are >> exactly the same as the ones needed on ILK+, so just reuse the >> function we already have. > > Now that I sent it, I realized that I should just have inverted the > patch order so patch 1 could be Cc:stable to ease backporting... Seems to me patch 1/2 is also useful for backporting if patch 2/2 gets backported. cc: stable to both, with the dependency added to patch 2/2 when we have the commit id for 1/2 (done while applying). Documentation/stable_kernel_rules.txt: """ Additionally, some patches submitted via Option 1 may have additional patch prerequisites which can be cherry-picked. This can be specified in the following format in the sign-off area: Cc: # 3.3.x: a1f84a3: sched: Check for idle Cc: # 3.3.x: 1b9508f: sched: Rate-limit newidle Cc: # 3.3.x: fd21073: sched: Fix affinity logic Cc: # 3.3.x Signed-off-by: Ingo Molnar The tag sequence has the meaning of: git cherry-pick a1f84a3 git cherry-pick 1b9508f git cherry-pick fd21073 git cherry-pick """ BR, Jani. > >> >> Signed-off-by: Paulo Zanoni >> --- >> drivers/gpu/drm/i915/intel_pm.c | 10 ++ >> 1 file changed, 2 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c >> b/drivers/gpu/drm/i915/intel_pm.c >> index 3a6df2f..62d730d 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -3470,12 +3470,6 @@ skl_allocate_pipe_ddb(struct intel_crtc_state >> *cstate, >> return 0; >> } >> >> -static uint32_t skl_pipe_pixel_rate(const struct intel_crtc_state >> *config) >> -{ >> -/* TODO: Take into account the scalers once we support them >> */ >> -return config->base.adjusted_mode.crtc_clock; >> -} >> - >> /* >> * The max latency should be 257 (max the punit can code is 255 and >> we add 2us >> * for the read latency) and cpp should always be <= 8, so that >> @@ -3526,7 +3520,7 @@ static uint32_t >> skl_adjusted_plane_pixel_rate(const struct intel_crtc_state *cst >> * Adjusted plane pixel rate is just the pipe's adjusted >> pixel rate >> * with additional adjustments for plane-specific scaling. >> */ >> -adjusted_pixel_rate = skl_pipe_pixel_rate(cstate); >> +adjusted_pixel_rate = ilk_pipe_pixel_rate(cstate); >> downscale_amount = skl_plane_downscale_amount(pstate); >> >> pixel_rate = adjusted_pixel_rate * downscale_amount >> 16; >> @@ -3744,7 +3738,7 @@ skl_compute_linetime_wm(struct intel_crtc_state >> *cstate) >> if (!cstate->base.active) >> return 0; >> >> -pixel_rate = skl_pipe_pixel_rate(cstate); >> +pixel_rate = ilk_pipe_pixel_rate(cstate); >> >> if (WARN_ON(pixel_rate == 0)) >> return 0; > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915/gen9: fix watermarks when using the pipe scaler
On Fri, 07 Oct 2016, Paulo Zanoni wrote: > Luckily, the necessary adjustments for when we're using the scaler are > exactly the same as the ones needed on ILK+, so just reuse the > function we already have. > > v2: Invert the patch order so stable backports get easier. Replied to the other set first... this order is fine too, with or without cc: stable on the other one. BR, Jani. > > Cc: sta...@vger.kernel.org > Signed-off-by: Paulo Zanoni > --- > drivers/gpu/drm/i915/intel_pm.c | 12 +++- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index fe6c1c6..000b033 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3470,12 +3470,6 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, > return 0; > } > > -static uint32_t skl_pipe_pixel_rate(const struct intel_crtc_state *config) > -{ > - /* TODO: Take into account the scalers once we support them */ > - return config->base.adjusted_mode.crtc_clock; > -} > - > /* > * The max latency should be 257 (max the punit can code is 255 and we add > 2us > * for the read latency) and cpp should always be <= 8, so that > @@ -3526,7 +3520,7 @@ static uint32_t skl_adjusted_plane_pixel_rate(const > struct intel_crtc_state *cst >* Adjusted plane pixel rate is just the pipe's adjusted pixel rate >* with additional adjustments for plane-specific scaling. >*/ > - adjusted_pixel_rate = skl_pipe_pixel_rate(cstate); > + adjusted_pixel_rate = ilk_pipe_pixel_rate(cstate); > downscale_amount = skl_plane_downscale_amount(pstate); > > pixel_rate = adjusted_pixel_rate * downscale_amount >> 16; > @@ -3742,11 +3736,11 @@ skl_compute_linetime_wm(struct intel_crtc_state > *cstate) > if (!cstate->base.active) > return 0; > > - if (WARN_ON(skl_pipe_pixel_rate(cstate) == 0)) > + if (WARN_ON(ilk_pipe_pixel_rate(cstate) == 0)) > return 0; > > return DIV_ROUND_UP(8 * cstate->base.adjusted_mode.crtc_htotal * 1000, > - skl_pipe_pixel_rate(cstate)); > + ilk_pipe_pixel_rate(cstate)); > } > > static void skl_compute_transition_wm(struct intel_crtc_state *cstate, -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Updated drm-intel-testing
Hi all, New -testing cycle with cool stuff: - dsi/backlight fixes (Jani&Shawn) - a few reset improvements (Ben, Chris, Imre) - refactor port tracking for audio support (Dhinakaran) - a pile of gen9 wm fixes from Paulo - drop skl pre-production w/a (Jani) - refactor forcewake and shadow reg filters into tables, and unify the funcs/macros using them across platforms (Tvrtko) - fix DP detection to not require an edid (Ville) - register shadow VGA port (for testing), from Ville Happy testing! Cheers, Daniel -- 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 2/2] drm/i915: Assert we hold the CRTC powerwell for generating vblank interrupts
Op 07-10-16 om 21:49 schreef Chris Wilson: > To enable the vblank itself, we need to have an RPM wakeref for the mmio > access, and whilst generating the vblank interrupts we continue to > require the rpm wakeref. The assumption is that the RPM wakeref is held > by the display powerwell held by the active pipe. As this chain was not > obvious to me chasing the drm_wait_vblank ioctl, document it with a WARN > during *_vblank_enable(). > > Signed-off-by: Chris Wilson > Cc: Ville Syrjälä > Cc: Maarten Lankhorst Don't we prevent enabling the vblank irq through drm_crtc_vblank_on/off? I'd rather not have things look at crtc->state if possible, locking might not help you. ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Assert we hold the CRTC powerwell for generating vblank interrupts
On Mon, Oct 10, 2016 at 11:35:07AM +0200, Maarten Lankhorst wrote: > Op 07-10-16 om 21:49 schreef Chris Wilson: > > To enable the vblank itself, we need to have an RPM wakeref for the mmio > > access, and whilst generating the vblank interrupts we continue to > > require the rpm wakeref. The assumption is that the RPM wakeref is held > > by the display powerwell held by the active pipe. As this chain was not > > obvious to me chasing the drm_wait_vblank ioctl, document it with a WARN > > during *_vblank_enable(). > > > > Signed-off-by: Chris Wilson > > Cc: Ville Syrjälä > > Cc: Maarten Lankhorst > Don't we prevent enabling the vblank irq through drm_crtc_vblank_on/off? Should, but this is the boundary point from the midlayer, so sanitychecks ahoy. > I'd rather not have things look at crtc->state if possible, locking might not > help you. Ok, anything better? -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] ✗ Fi.CI.BAT: warning for drm/i915: Grab RPM wakeref around enabling vblank interrupts
== Series Details == Series: drm/i915: Grab RPM wakeref around enabling vblank interrupts URL : https://patchwork.freedesktop.org/series/13446/ State : warning == Summary == Series 13446v1 drm/i915: Grab RPM wakeref around enabling vblank interrupts https://patchwork.freedesktop.org/api/1.0/series/13446/revisions/1/mbox/ Test core_auth: Subgroup basic-auth: pass -> DMESG-WARN (fi-byt-j1900) pass -> DMESG-WARN (fi-bsw-n3050) pass -> DMESG-WARN (fi-skl-6700hq) pass -> DMESG-WARN (fi-byt-n2820) pass -> DMESG-WARN (fi-ivb-3520m) pass -> DMESG-WARN (fi-hsw-4770r) pass -> DMESG-WARN (fi-skl-6700k) pass -> DMESG-WARN (fi-hsw-4770) pass -> DMESG-WARN (fi-snb-2600) pass -> DMESG-WARN (fi-ilk-650) pass -> DMESG-WARN (fi-skl-6260u) pass -> DMESG-WARN (fi-snb-2520m) pass -> DMESG-WARN (fi-kbl-7200u) pass -> DMESG-WARN (fi-bxt-t5700) pass -> DMESG-WARN (fi-skl-6770hq) pass -> DMESG-WARN (fi-bdw-5557u) Test drv_getparams_basic: Subgroup basic-subslice-total: pass -> DMESG-WARN (fi-snb-2520m) pass -> DMESG-WARN (fi-byt-j1900) pass -> DMESG-WARN (fi-bxt-t5700) pass -> DMESG-WARN (fi-byt-n2820) pass -> DMESG-WARN (fi-bsw-n3050) Test drv_hangman: Subgroup error-state-basic: pass -> DMESG-WARN (fi-byt-j1900) pass -> DMESG-WARN (fi-bsw-n3050) pass -> DMESG-WARN (fi-skl-6700hq) pass -> DMESG-WARN (fi-byt-n2820) pass -> DMESG-WARN (fi-ivb-3520m) pass -> DMESG-WARN (fi-hsw-4770r) pass -> DMESG-WARN (fi-skl-6700k) pass -> DMESG-WARN (fi-hsw-4770) pass -> DMESG-WARN (fi-snb-2600) pass -> DMESG-WARN (fi-ilk-650) pass -> DMESG-WARN (fi-skl-6260u) pass -> DMESG-WARN (fi-snb-2520m) pass -> DMESG-WARN (fi-kbl-7200u) pass -> DMESG-WARN (fi-bxt-t5700) pass -> DMESG-WARN (fi-skl-6770hq) pass -> DMESG-WARN (fi-bdw-5557u) Test drv_module_reload_basic: pass -> DMESG-WARN (fi-byt-j1900) pass -> DMESG-WARN (fi-bsw-n3050) pass -> DMESG-WARN (fi-skl-6700hq) pass -> DMESG-WARN (fi-byt-n2820) pass -> DMESG-WARN (fi-ivb-3520m) pass -> DMESG-WARN (fi-hsw-4770r) pass -> DMESG-WARN (fi-kbl-7200u) pass -> DMESG-WARN (fi-hsw-4770) pass -> DMESG-WARN (fi-snb-2600) pass -> DMESG-WARN (fi-ilk-650) pass -> DMESG-WARN (fi-skl-6260u) pass -> DMESG-WARN (fi-snb-2520m) pass -> DMESG-WARN (fi-bxt-t5700) pass -> DMESG-WARN (fi-skl-6770hq) pass -> DMESG-WARN (fi-bdw-5557u) Test gem_basic: Subgroup create-fd-close: pass -> DMESG-WARN (fi-snb-2520m) pass -> DMESG-WARN (fi-byt-j1900) pass -> DMESG-WARN (fi-bxt-t5700) Test gem_busy: Subgroup basic-busy-default: pass -> DMESG-WARN (fi-bsw-n3050) pass -> DMESG-WARN (fi-byt-n2820) Subgroup basic-hang-default: pass -> DMESG-WARN (fi-byt-j1900) pass -> DMESG-WARN (fi-bsw-n3050) pass -> DMESG-WARN (fi-skl-6700hq) pass -> DMESG-WARN (fi-byt-n2820) pass -> DMESG-WARN (fi-ivb-3520m) pass -> DMESG-WARN (fi-hsw-4770r) pass -> DMESG-WARN (fi-skl-6700k) pass -> DMESG-WARN (fi-hsw-4770) pass -> DMESG-WARN (fi-snb-2600) pass -> DMESG-WARN (fi-ilk-650) pass -> DMESG-WARN (fi-skl-6260u) pass -> DMESG-WARN (fi-snb-2520m) pass -> DMESG-WARN (fi-kbl-7200u) pass -> DMESG-WARN (fi-bxt-t5700) pass -> DMESG-WARN (fi-skl-6770hq) pass -> DMESG-WARN (fi-bdw-5557u) Test gem_close_race: Subgroup basic-threads: pass -> DMESG-WARN (fi-byt-j1900) pass -> DMESG-WARN (fi-bsw-n3050) pass -> DMESG-WARN (fi-skl-6700hq) pass -> DMESG
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Assert we hold the CRTC powerwell for generating vblank interrupts
Op 10-10-16 om 11:57 schreef Chris Wilson: > On Mon, Oct 10, 2016 at 11:35:07AM +0200, Maarten Lankhorst wrote: >> Op 07-10-16 om 21:49 schreef Chris Wilson: >>> To enable the vblank itself, we need to have an RPM wakeref for the mmio >>> access, and whilst generating the vblank interrupts we continue to >>> require the rpm wakeref. The assumption is that the RPM wakeref is held >>> by the display powerwell held by the active pipe. As this chain was not >>> obvious to me chasing the drm_wait_vblank ioctl, document it with a WARN >>> during *_vblank_enable(). >>> >>> Signed-off-by: Chris Wilson >>> Cc: Ville Syrjälä >>> Cc: Maarten Lankhorst >> Don't we prevent enabling the vblank irq through drm_crtc_vblank_on/off? > Should, but this is the boundary point from the midlayer, so > sanitychecks ahoy. > >> I'd rather not have things look at crtc->state if possible, locking might >> not help you. > Ok, anything better? > -Chris I would say either intel_display_is_enabled(POWER_DOMAIN_PIPE(pipe)) or assert_rpm_wakelock_held. ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4] tools/intel_guc_logger: Utility for capturing GuC firmware logs in a file
From: Akash Goel This patch provides a test utility which helps capture GuC firmware logs and then dump them to file. The logs are pulled from a debugfs file '/sys/kernel/debug/dri/guc_log' and by default stored into a file 'guc_log_dump.dat'. The name, including the location, of the output file can be changed through a command line argument. The utility goes into an infinite loop where it waits for the arrival of new logs and as soon as new set of logs are produced it captures them in its local buffer which is then flushed out to the file on disk. Any time when logging needs to be ended, User can stop this utility (CTRL+C). Before entering into a loop, it first discards whatever logs are present in the debugfs file. This way User can first launch this utility and then start a workload/activity for which GuC firmware logs are to be actually captured and keep running the utility for as long as its needed, like once the workload is over this utility can be forcefully stopped. If the logging wasn't enabled on GuC side by the Driver at boot time, utility will first enable the logging and later on when it is stopped (CTRL+C) it will also pause the logging on GuC side. v2: - Use combination of alarm system call & SIGALRM signal to run the utility for required duration. (Tvrtko) - Fix inconsistencies, do minor cleanup and refactoring. (Tvrtko) v3: - Fix discrepancy for the output file command line option and update the Usage/help string. v4: - Update the exit condition for flusher thread, now will exit only after the capture loop is over and not when the flag to stop logging is set. This handles a corner case, due to which the dump of last captured buffer was getting missed. - Add a newline character at the end of assert messages. - Avoid the assert for the case, which occurs very rarely, when there are no bytes read from the relay file. Cc: Tvrtko Ursulin Signed-off-by: Akash Goel Reviewed-by: Tvrtko Ursulin (v3) --- tools/Makefile.sources | 1 + tools/intel_guc_logger.c | 438 +++ 2 files changed, 439 insertions(+) create mode 100644 tools/intel_guc_logger.c diff --git a/tools/Makefile.sources b/tools/Makefile.sources index 2bb6c8e..be58871 100644 --- a/tools/Makefile.sources +++ b/tools/Makefile.sources @@ -19,6 +19,7 @@ tools_prog_lists =\ intel_gpu_time \ intel_gpu_top \ intel_gtt \ + intel_guc_logger\ intel_infoframes\ intel_l3_parity \ intel_lid \ diff --git a/tools/intel_guc_logger.c b/tools/intel_guc_logger.c new file mode 100644 index 000..159a54e --- /dev/null +++ b/tools/intel_guc_logger.c @@ -0,0 +1,438 @@ + +#define _GNU_SOURCE /* For using O_DIRECT */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "igt.h" + +#define MB(x) ((uint64_t)(x) * 1024 * 1024) +#ifndef PAGE_SIZE + #define PAGE_SIZE 4096 +#endif +/* Currently the size of GuC log buffer is 19 pages & so is the size of relay + * subbuffer. If the size changes in future, then this define also needs to be + * updated accordingly. + */ +#define SUBBUF_SIZE (19*PAGE_SIZE) +/* Need large buffering from logger side to hide the DISK IO latency, Driver + * can only store 8 snapshots of GuC log buffer in relay. + */ +#define NUM_SUBBUFS 100 + +#define RELAY_FILE_NAME "guc_log" +#define DEFAULT_OUTPUT_FILE_NAME "guc_log_dump.dat" +#define CONTROL_FILE_NAME "i915_guc_log_control" + +char *read_buffer; +char *out_filename; +int poll_timeout = 2; /* by default 2ms timeout */ +pthread_mutex_t mutex; +pthread_t flush_thread; +int verbosity_level = 3; /* by default capture logs at max verbosity */ +uint32_t produced, consumed; +uint64_t total_bytes_written; +int num_buffers = NUM_SUBBUFS; +int relay_fd, outfile_fd = -1; +uint32_t test_duration, max_filesize; +pthread_cond_t underflow_cond, overflow_cond; +bool stop_logging, discard_oldlogs, capturing_stopped; + +static void guc_log_control(bool enable_logging) +{ + int control_fd; + char data[19]; + uint64_t val; + int ret; + + control_fd = igt_debugfs_open(CONTROL_FILE_NAME, O_WRONLY); + igt_assert_f(control_fd >= 0, "couldn't open the guc log control file\n"); + + val = enable_logging ? ((verbosity_level << 4) | 0x1) : 0; + + ret = snprintf(data, sizeof(data), "0x%" PRIx64, val); + igt_assert(ret > 2 && ret < sizeof(data)); + + ret = write(control_fd, data, ret); + igt_assert_f(ret > 0, "couldn't write to the log control file\n"); + + close(control_fd); +} + +static void int_sig_handler(int sig) +{ + igt_info("received signal %d\n", sig); + + stop_logging = true; +} + +static void pull_leftover_data(void) +{ + unsigned int bytes_read = 0; + int re
[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Defer disabling output polling until after we cancel hpd work
== Series Details == Series: drm/i915: Defer disabling output polling until after we cancel hpd work URL : https://patchwork.freedesktop.org/series/13447/ State : warning == Summary == Series 13447v1 drm/i915: Defer disabling output polling until after we cancel hpd work https://patchwork.freedesktop.org/api/1.0/series/13447/revisions/1/mbox/ Test drv_module_reload_basic: pass -> SKIP (fi-skl-6770hq) Test kms_psr_sink_crc: Subgroup psr_basic: pass -> DMESG-WARN (fi-skl-6700hq) Test vgem_basic: Subgroup unload: pass -> SKIP (fi-skl-6700k) pass -> SKIP (fi-byt-n2820) pass -> SKIP (fi-ilk-650) pass -> SKIP (fi-bdw-5557u) fi-bdw-5557u total:248 pass:231 dwarn:0 dfail:0 fail:0 skip:17 fi-bsw-n3050 total:248 pass:204 dwarn:0 dfail:0 fail:0 skip:44 fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31 fi-byt-j1900 total:248 pass:214 dwarn:1 dfail:0 fail:1 skip:32 fi-byt-n2820 total:248 pass:210 dwarn:0 dfail:0 fail:1 skip:37 fi-hsw-4770 total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-hsw-4770r total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-ilk-650 total:248 pass:184 dwarn:0 dfail:0 fail:2 skip:62 fi-ivb-3520m total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27 fi-ivb-3770 total:248 pass:207 dwarn:0 dfail:0 fail:0 skip:41 fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-6260u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-skl-6700hqtotal:248 pass:223 dwarn:1 dfail:0 fail:0 skip:24 fi-skl-6700k total:248 pass:221 dwarn:1 dfail:0 fail:0 skip:26 fi-skl-6770hqtotal:248 pass:230 dwarn:1 dfail:0 fail:1 skip:16 fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37 fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39 Results at /archive/results/CI_IGT_test/Patchwork_2650/ 75a740e573eb343099eeb9d8e8728dcc98df435a drm-intel-nightly: 2016y-10m-10d-09h-50m-22s UTC integration manifest 9680828 drm/i915: Defer disabling output polling until after we cancel hpd work ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Assert we hold the CRTC powerwell for generating vblank interrupts
On Mon, Oct 10, 2016 at 12:38:13PM +0200, Maarten Lankhorst wrote: > Op 10-10-16 om 11:57 schreef Chris Wilson: > > On Mon, Oct 10, 2016 at 11:35:07AM +0200, Maarten Lankhorst wrote: > >> Op 07-10-16 om 21:49 schreef Chris Wilson: > >>> To enable the vblank itself, we need to have an RPM wakeref for the mmio > >>> access, and whilst generating the vblank interrupts we continue to > >>> require the rpm wakeref. The assumption is that the RPM wakeref is held > >>> by the display powerwell held by the active pipe. As this chain was not > >>> obvious to me chasing the drm_wait_vblank ioctl, document it with a WARN > >>> during *_vblank_enable(). > >>> > >>> Signed-off-by: Chris Wilson > >>> Cc: Ville Syrjälä > >>> Cc: Maarten Lankhorst > >> Don't we prevent enabling the vblank irq through drm_crtc_vblank_on/off? > > Should, but this is the boundary point from the midlayer, so > > sanitychecks ahoy. > > > >> I'd rather not have things look at crtc->state if possible, locking might > >> not help you. > > Ok, anything better? > > -Chris > > I would say either intel_display_is_enabled(POWER_DOMAIN_PIPE(pipe)) or > assert_rpm_wakelock_held. I was aiming for a higher level assert than assert rpm wakelock, so let's try intel_display_is_enabled... -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] [RFC] drm/i915: Introduce i915_alloc_sg_table
From: Tvrtko Ursulin There are currently two places in the code which build the sg table for the object backing store and in the future there will be one more. Consolidate that into a single helper which takes a caller defined context and callbacks. !!! Compile tested only. !!! Signed-off-by: Tvrtko Ursulin Cc: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 162 drivers/gpu/drm/i915/i915_gem_gtt.c | 84 + drivers/gpu/drm/i915/i915_gem_gtt.h | 14 +++ drivers/gpu/drm/i915/i915_gem_userptr.c | 54 --- 4 files changed, 198 insertions(+), 116 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index a89a88922448..79dcd912759f 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2208,19 +2208,69 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj) return 0; } +struct i915_gem_get_pages_gtt_ctx { + struct drm_i915_private *dev_priv; + gfp_t gfp; + struct address_space *mapping; + unsigned int page_count; + bool shrunk_all; +}; + +static struct page * +i915_gem_gtt_get_page(void *context, unsigned int page_num) +{ + struct i915_gem_get_pages_gtt_ctx *ctx = context; + struct page *page; + + if (!ctx->shrunk_all) + page = shmem_read_mapping_page_gfp(ctx->mapping, page_num, + ctx->gfp); + else + page = shmem_read_mapping_page(ctx->mapping, page_num); + + /* Check that the i965g/gm workaround works. */ + WARN_ON(page && !IS_ERR(page) && (ctx->gfp & __GFP_DMA32) && + (page_to_pfn(page) >= 0x0010UL)); + + return page; +} + +static bool +i915_gem_gtt_get_page_failed(void *context, unsigned int page_num, +unsigned int err_cnt, int err) +{ + struct i915_gem_get_pages_gtt_ctx *ctx = context; + + if (err_cnt > 1) + return true; + + if (err_cnt == 0) { + i915_gem_shrink(ctx->dev_priv, ctx->page_count, + I915_SHRINK_BOUND | I915_SHRINK_UNBOUND | + I915_SHRINK_PURGEABLE); + } else { + i915_gem_shrink_all(ctx->dev_priv); + ctx->shrunk_all = true; + } + + return false; +} + +static void +i915_gem_gtt_put_page(void *context, struct page *page) +{ + put_page(page); +} + static int i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) { struct drm_i915_private *dev_priv = to_i915(obj->base.dev); - int page_count, i; - struct address_space *mapping; + struct i915_gem_get_pages_gtt_ctx ctx; struct sg_table *st; - struct scatterlist *sg; struct sgt_iter sgt_iter; struct page *page; - unsigned long last_pfn = 0; /* suppress gcc warning */ int ret; - gfp_t gfp; /* Assert that the object is not currently in any GPU domain. As it * wasn't in the GTT, there shouldn't be any way it could have been in @@ -2229,78 +2279,44 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) BUG_ON(obj->base.read_domains & I915_GEM_GPU_DOMAINS); BUG_ON(obj->base.write_domain & I915_GEM_GPU_DOMAINS); - st = kmalloc(sizeof(*st), GFP_KERNEL); - if (st == NULL) - return -ENOMEM; - - page_count = obj->base.size / PAGE_SIZE; - if (sg_alloc_table(st, page_count, GFP_KERNEL)) { - kfree(st); - return -ENOMEM; - } + ctx.dev_priv = dev_priv; + ctx.page_count = obj->base.size / PAGE_SIZE; + ctx.shrunk_all = false; /* Get the list of pages out of our struct file. They'll be pinned * at this point until we release them. * * Fail silently without starting the shrinker */ - mapping = obj->base.filp->f_mapping; - gfp = mapping_gfp_constraint(mapping, ~(__GFP_IO | __GFP_RECLAIM)); - gfp |= __GFP_NORETRY | __GFP_NOWARN; - sg = st->sgl; - st->nents = 0; - for (i = 0; i < page_count; i++) { - page = shmem_read_mapping_page_gfp(mapping, i, gfp); - if (IS_ERR(page)) { - i915_gem_shrink(dev_priv, - page_count, - I915_SHRINK_BOUND | - I915_SHRINK_UNBOUND | - I915_SHRINK_PURGEABLE); - page = shmem_read_mapping_page_gfp(mapping, i, gfp); - } - if (IS_ERR(page)) { - /* We've tried hard to allocate the memory by reaping -* our own buffer, now let the real VM do its job and -* go down in flames if truly OOM. -*/
Re: [Intel-gfx] [PATCH 16/42] drm/i915: Refactor object page API
On 07/10/2016 10:46, Chris Wilson wrote: The plan is to make obtaining the backing storage for the object avoid struct_mutex (i.e. use its own locking). The first step is to update the API so that normal users only call pin/unpin whilst working on the backing storage. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_cmd_parser.c | 2 +- drivers/gpu/drm/i915/i915_debugfs.c | 17 +-- drivers/gpu/drm/i915/i915_drv.h | 89 drivers/gpu/drm/i915/i915_gem.c | 207 +-- drivers/gpu/drm/i915/i915_gem_batch_pool.c | 3 +- drivers/gpu/drm/i915/i915_gem_dmabuf.c | 14 +- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- drivers/gpu/drm/i915/i915_gem_fence.c| 4 +- drivers/gpu/drm/i915/i915_gem_gtt.c | 10 +- drivers/gpu/drm/i915/i915_gem_internal.c | 19 +-- drivers/gpu/drm/i915/i915_gem_render_state.c | 2 +- drivers/gpu/drm/i915/i915_gem_shrinker.c | 10 +- drivers/gpu/drm/i915/i915_gem_stolen.c | 24 ++-- drivers/gpu/drm/i915/i915_gem_tiling.c | 8 +- drivers/gpu/drm/i915/i915_gem_userptr.c | 30 ++-- drivers/gpu/drm/i915/i915_gpu_error.c| 4 +- drivers/gpu/drm/i915/intel_lrc.c | 6 +- 17 files changed, 234 insertions(+), 217 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c index 70980f82a15b..8d20020cb9f9 100644 --- a/drivers/gpu/drm/i915/i915_cmd_parser.c +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c @@ -1290,7 +1290,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine, } if (ret == 0 && needs_clflush_after) - drm_clflush_virt_range(shadow_batch_obj->mapping, batch_len); + drm_clflush_virt_range(shadow_batch_obj->mm.mapping, batch_len); i915_gem_object_unpin_map(shadow_batch_obj); return ret; diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index e4b5ba771bea..b807ddf65e04 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -112,7 +112,7 @@ static char get_global_flag(struct drm_i915_gem_object *obj) static char get_pin_mapped_flag(struct drm_i915_gem_object *obj) { - return obj->mapping ? 'M' : ' '; + return obj->mm.mapping ? 'M' : ' '; } static u64 i915_gem_obj_total_ggtt_size(struct drm_i915_gem_object *obj) @@ -158,8 +158,8 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj) i915_gem_active_get_seqno(&obj->last_write, &obj->base.dev->struct_mutex), i915_cache_level_str(dev_priv, obj->cache_level), - obj->dirty ? " dirty" : "", - obj->madv == I915_MADV_DONTNEED ? " purgeable" : ""); + obj->mm.dirty ? " dirty" : "", + obj->mm.madv == I915_MADV_DONTNEED ? " purgeable" : ""); if (obj->base.name) seq_printf(m, " (name: %d)", obj->base.name); list_for_each_entry(vma, &obj->vma_list, obj_link) { @@ -411,12 +411,12 @@ static int i915_gem_object_info(struct seq_file *m, void *data) size += obj->base.size; ++count; - if (obj->madv == I915_MADV_DONTNEED) { + if (obj->mm.madv == I915_MADV_DONTNEED) { purgeable_size += obj->base.size; ++purgeable_count; } - if (obj->mapping) { + if (obj->mm.mapping) { mapped_count++; mapped_size += obj->base.size; } @@ -433,12 +433,12 @@ static int i915_gem_object_info(struct seq_file *m, void *data) ++dpy_count; } - if (obj->madv == I915_MADV_DONTNEED) { + if (obj->mm.madv == I915_MADV_DONTNEED) { purgeable_size += obj->base.size; ++purgeable_count; } - if (obj->mapping) { + if (obj->mm.mapping) { mapped_count++; mapped_size += obj->base.size; } @@ -2018,7 +2018,7 @@ static void i915_dump_lrc_obj(struct seq_file *m, seq_printf(m, "\tBound in GGTT at 0x%08x\n", i915_ggtt_offset(vma)); - if (i915_gem_object_get_pages(vma->obj)) { + if (i915_gem_object_pin_pages(vma->obj)) { seq_puts(m, "\tFailed to get pages for context object\n\n"); Error message needs updating to match the function call change. return; } @@ -2037,6 +2037,7 @@ static void i915_dump_lrc_obj(struct seq_file *m, kunmap_atomic(reg_state); } + i915_gem_object_unpin_pages(vma->obj); seq_putc(m, '\n'); } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/dr
Re: [Intel-gfx] [RFC] drm/i915: Introduce i915_alloc_sg_table
On Mon, Oct 10, 2016 at 11:53:18AM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > There are currently two places in the code which build the > sg table for the object backing store and in the future there > will be one more. > > Consolidate that into a single helper which takes a caller > defined context and callbacks. Not getting warm fuzzy feelings about it. To surmise, what you really want is a common method of applying sg coalescing whilst iteratively building the sg list. -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] [PATCH] drm/i915: Assert we hold the CRTC powerwell for generating vblank interrupts
To enable the vblank itself, we need to have an RPM wakeref for the mmio access, and whilst generating the vblank interrupts we continue to require the rpm wakeref. The assumption is that the RPM wakeref is held by the display powerwell held by the active pipe. As this chain was not obvious to me chasing the drm_wait_vblank ioctl, document it with a WARN during *_vblank_enable(). v2: Check the display power well rather than digging inside the atomic CRTC state. Signed-off-by: Chris Wilson Cc: Ville Syrjälä Cc: Maarten Lankhorst --- drivers/gpu/drm/i915/i915_irq.c | 20 1 file changed, 20 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 1e43fe30da11..f0f17055dbb9 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2715,6 +2715,14 @@ void i915_handle_error(struct drm_i915_private *dev_priv, i915_reset_and_wakeup(dev_priv); } +static void assert_pipe_is_awake(struct drm_i915_private *dev_priv, +enum pipe pipe) +{ + WARN_ON(IS_ENABLED(CONFIG_DRM_I915_DEBUG) && + !intel_display_power_is_enabled(dev_priv, + POWER_DOMAIN_PIPE(pipe))); +} + /* Called from drm generic code, passed 'crtc' which * we use as a pipe index */ @@ -2723,6 +2731,9 @@ static int i8xx_enable_vblank(struct drm_device *dev, unsigned int pipe) struct drm_i915_private *dev_priv = to_i915(dev); unsigned long irqflags; + /* vblank IRQ requires the powerwell, held awake by the CRTC */ + assert_pipe_is_awake(dev_priv, pipe); + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); i915_enable_pipestat(dev_priv, pipe, PIPE_VBLANK_INTERRUPT_STATUS); spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); @@ -2735,6 +2746,9 @@ static int i965_enable_vblank(struct drm_device *dev, unsigned int pipe) struct drm_i915_private *dev_priv = to_i915(dev); unsigned long irqflags; + /* vblank IRQ requires the powerwell, held awake by the CRTC */ + assert_pipe_is_awake(dev_priv, pipe); + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); i915_enable_pipestat(dev_priv, pipe, PIPE_START_VBLANK_INTERRUPT_STATUS); @@ -2750,6 +2764,9 @@ static int ironlake_enable_vblank(struct drm_device *dev, unsigned int pipe) uint32_t bit = INTEL_GEN(dev) >= 7 ? DE_PIPE_VBLANK_IVB(pipe) : DE_PIPE_VBLANK(pipe); + /* vblank IRQ requires the powerwell, held awake by the CRTC */ + assert_pipe_is_awake(dev_priv, pipe); + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); ilk_enable_display_irq(dev_priv, bit); spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); @@ -2762,6 +2779,9 @@ static int gen8_enable_vblank(struct drm_device *dev, unsigned int pipe) struct drm_i915_private *dev_priv = to_i915(dev); unsigned long irqflags; + /* vblank IRQ requires the powerwell, held awake by the CRTC */ + assert_pipe_is_awake(dev_priv, pipe); + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK); spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); -- 2.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Assert we hold the CRTC powerwell for generating vblank interrupts
To enable the vblank itself, we need to have an RPM wakeref for the mmio access, and whilst generating the vblank interrupts we continue to require the rpm wakeref. The assumption is that the RPM wakeref is held by the display powerwell held by the active pipe. As this chain was not obvious to me chasing the drm_wait_vblank ioctl, document it with a WARN during *_vblank_enable(). v2: Check the display power well rather than digging inside the atomic CRTC state. v3: Keep the WARN_ON() tidy (avoid macro expansions that get recorded in their completeness in the user string). Signed-off-by: Chris Wilson Cc: Ville Syrjälä Cc: Maarten Lankhorst --- drivers/gpu/drm/i915/i915_irq.c | 24 1 file changed, 24 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 1e43fe30da11..8e2eb5adde33 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2715,6 +2715,18 @@ void i915_handle_error(struct drm_i915_private *dev_priv, i915_reset_and_wakeup(dev_priv); } +static void assert_pipe_is_awake(struct drm_i915_private *dev_priv, +enum pipe pipe) +{ + enum intel_display_power_domain pipe_domain; + + if (!IS_ENABLED(CONFIG_DRM_I915_DEBUG)) + return; + + pipe_domain = POWER_DOMAIN_PIPE(pipe); + WARN_ON(!intel_display_power_is_enabled(dev_priv, pipe_domain)); +} + /* Called from drm generic code, passed 'crtc' which * we use as a pipe index */ @@ -2723,6 +2735,9 @@ static int i8xx_enable_vblank(struct drm_device *dev, unsigned int pipe) struct drm_i915_private *dev_priv = to_i915(dev); unsigned long irqflags; + /* vblank IRQ requires the powerwell, held awake by the CRTC */ + assert_pipe_is_awake(dev_priv, pipe); + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); i915_enable_pipestat(dev_priv, pipe, PIPE_VBLANK_INTERRUPT_STATUS); spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); @@ -2735,6 +2750,9 @@ static int i965_enable_vblank(struct drm_device *dev, unsigned int pipe) struct drm_i915_private *dev_priv = to_i915(dev); unsigned long irqflags; + /* vblank IRQ requires the powerwell, held awake by the CRTC */ + assert_pipe_is_awake(dev_priv, pipe); + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); i915_enable_pipestat(dev_priv, pipe, PIPE_START_VBLANK_INTERRUPT_STATUS); @@ -2750,6 +2768,9 @@ static int ironlake_enable_vblank(struct drm_device *dev, unsigned int pipe) uint32_t bit = INTEL_GEN(dev) >= 7 ? DE_PIPE_VBLANK_IVB(pipe) : DE_PIPE_VBLANK(pipe); + /* vblank IRQ requires the powerwell, held awake by the CRTC */ + assert_pipe_is_awake(dev_priv, pipe); + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); ilk_enable_display_irq(dev_priv, bit); spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); @@ -2762,6 +2783,9 @@ static int gen8_enable_vblank(struct drm_device *dev, unsigned int pipe) struct drm_i915_private *dev_priv = to_i915(dev); unsigned long irqflags; + /* vblank IRQ requires the powerwell, held awake by the CRTC */ + assert_pipe_is_awake(dev_priv, pipe); + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK); spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); -- 2.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Assert we hold the CRTC powerwell for generating vblank interrupts
On Mon, Oct 10, 2016 at 12:34:54PM +0100, Chris Wilson wrote: > To enable the vblank itself, we need to have an RPM wakeref for the mmio > access, and whilst generating the vblank interrupts we continue to > require the rpm wakeref. The assumption is that the RPM wakeref is held > by the display powerwell held by the active pipe. As this chain was not > obvious to me chasing the drm_wait_vblank ioctl, document it with a WARN > during *_vblank_enable(). > > v2: Check the display power well rather than digging inside the atomic > CRTC state. > > Signed-off-by: Chris Wilson > Cc: Ville Syrjälä > Cc: Maarten Lankhorst > --- > drivers/gpu/drm/i915/i915_irq.c | 20 > 1 file changed, 20 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 1e43fe30da11..f0f17055dbb9 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2715,6 +2715,14 @@ void i915_handle_error(struct drm_i915_private > *dev_priv, > i915_reset_and_wakeup(dev_priv); > } > > +static void assert_pipe_is_awake(struct drm_i915_private *dev_priv, > + enum pipe pipe) > +{ > + WARN_ON(IS_ENABLED(CONFIG_DRM_I915_DEBUG) && > + !intel_display_power_is_enabled(dev_priv, > + POWER_DOMAIN_PIPE(pipe))); Uses a mutex. And having a power well enabled doesn't mean much. It doesn't guarantee that vblanks work. > +} > + > /* Called from drm generic code, passed 'crtc' which > * we use as a pipe index > */ > @@ -2723,6 +2731,9 @@ static int i8xx_enable_vblank(struct drm_device *dev, > unsigned int pipe) > struct drm_i915_private *dev_priv = to_i915(dev); > unsigned long irqflags; > > + /* vblank IRQ requires the powerwell, held awake by the CRTC */ > + assert_pipe_is_awake(dev_priv, pipe); > + > spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > i915_enable_pipestat(dev_priv, pipe, PIPE_VBLANK_INTERRUPT_STATUS); > spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > @@ -2735,6 +2746,9 @@ static int i965_enable_vblank(struct drm_device *dev, > unsigned int pipe) > struct drm_i915_private *dev_priv = to_i915(dev); > unsigned long irqflags; > > + /* vblank IRQ requires the powerwell, held awake by the CRTC */ > + assert_pipe_is_awake(dev_priv, pipe); > + > spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > i915_enable_pipestat(dev_priv, pipe, >PIPE_START_VBLANK_INTERRUPT_STATUS); > @@ -2750,6 +2764,9 @@ static int ironlake_enable_vblank(struct drm_device > *dev, unsigned int pipe) > uint32_t bit = INTEL_GEN(dev) >= 7 ? > DE_PIPE_VBLANK_IVB(pipe) : DE_PIPE_VBLANK(pipe); > > + /* vblank IRQ requires the powerwell, held awake by the CRTC */ > + assert_pipe_is_awake(dev_priv, pipe); > + > spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > ilk_enable_display_irq(dev_priv, bit); > spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > @@ -2762,6 +2779,9 @@ static int gen8_enable_vblank(struct drm_device *dev, > unsigned int pipe) > struct drm_i915_private *dev_priv = to_i915(dev); > unsigned long irqflags; > > + /* vblank IRQ requires the powerwell, held awake by the CRTC */ > + assert_pipe_is_awake(dev_priv, pipe); > + > spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK); > spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > -- > 2.9.3 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Assert we hold the CRTC powerwell for generating vblank interrupts
On Mon, Oct 10, 2016 at 02:42:01PM +0300, Ville Syrjälä wrote: > On Mon, Oct 10, 2016 at 12:34:54PM +0100, Chris Wilson wrote: > > To enable the vblank itself, we need to have an RPM wakeref for the mmio > > access, and whilst generating the vblank interrupts we continue to > > require the rpm wakeref. The assumption is that the RPM wakeref is held > > by the display powerwell held by the active pipe. As this chain was not > > obvious to me chasing the drm_wait_vblank ioctl, document it with a WARN > > during *_vblank_enable(). > > > > v2: Check the display power well rather than digging inside the atomic > > CRTC state. > > > > Signed-off-by: Chris Wilson > > Cc: Ville Syrjälä > > Cc: Maarten Lankhorst > > --- > > drivers/gpu/drm/i915/i915_irq.c | 20 > > 1 file changed, 20 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > b/drivers/gpu/drm/i915/i915_irq.c > > index 1e43fe30da11..f0f17055dbb9 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -2715,6 +2715,14 @@ void i915_handle_error(struct drm_i915_private > > *dev_priv, > > i915_reset_and_wakeup(dev_priv); > > } > > > > +static void assert_pipe_is_awake(struct drm_i915_private *dev_priv, > > +enum pipe pipe) > > +{ > > + WARN_ON(IS_ENABLED(CONFIG_DRM_I915_DEBUG) && > > + !intel_display_power_is_enabled(dev_priv, > > + POWER_DOMAIN_PIPE(pipe))); > > Uses a mutex. And having a power well enabled doesn't mean much. It > doesn't guarantee that vblanks work. Impasse. :| There should be no point in an explicit assert_rpm_wakeref here as the register access should catch an error there. Is there no safe way we can assert the current state of the CRTC is correct for enabling vblanks? -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] [PATCH 1/2] drm/i915: Remove self-harming shrink_all on get_pages_gtt fail
When we notice the system under memory pressure, we try to evict some driver pages before asking the VM to shrink all caches. As a final step in that process, we tried to evict everything, including active buffers. This is harming ourselves, and we can mix shrinking all caches as well as our residual buffers (after the first pass of trying to shrink just our own buffers). Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 38a183faf9a7..ca1a5a5c6f19 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2246,7 +2246,6 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) * our own buffer, now let the real VM do its job and * go down in flames if truly OOM. */ - i915_gem_shrink_all(dev_priv); page = shmem_read_mapping_page(mapping, i); if (IS_ERR(page)) { ret = PTR_ERR(page); -- 2.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Allow compaction upto SWIOTLB max segment size
commit 1625e7e549c5 ("drm/i915: make compact dma scatter lists creation work with SWIOTLB backend") took a heavy handed approach to undo the scatterlist compaction in the face of SWIOTLB. (The compaction hit a bug whereby we tried to pass a segment larger than SWIOTLB could handle.) We can be a little more intelligent and try compacting the scatterlist up to the maximum SWIOTLB segment size (when using SWIOTLB). Signed-off-by: Chris Wilson CC: Imre Deak CC: Daniel Vetter Cc: Konrad Rzeszutek Wilk --- drivers/gpu/drm/i915/i915_gem.c | 23 +++ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ca1a5a5c6f19..8b3474d215a5 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2201,6 +2201,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) struct sgt_iter sgt_iter; struct page *page; unsigned long last_pfn = 0; /* suppress gcc warning */ + unsigned long max_segment; int ret; gfp_t gfp; @@ -2211,6 +2212,12 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) GEM_BUG_ON(obj->base.read_domains & I915_GEM_GPU_DOMAINS); GEM_BUG_ON(obj->base.write_domain & I915_GEM_GPU_DOMAINS); + max_segment = obj->base.size; +#ifdef CONFIG_SWIOTLB + if (swiotlb_nr_tbl()) + max_segment = IO_TLB_SEGSIZE << PAGE_SHIFT; +#endif + st = kmalloc(sizeof(*st), GFP_KERNEL); if (st == NULL) return ERR_PTR(-ENOMEM); @@ -2252,15 +2259,9 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) goto err_pages; } } -#ifdef CONFIG_SWIOTLB - if (swiotlb_nr_tbl()) { - st->nents++; - sg_set_page(sg, page, PAGE_SIZE, 0); - sg = sg_next(sg); - continue; - } -#endif - if (!i || page_to_pfn(page) != last_pfn + 1) { + if (!i || + sg->length >= max_segment || + page_to_pfn(page) != last_pfn + 1) { if (i) sg = sg_next(sg); st->nents++; @@ -2273,9 +2274,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) /* Check that the i965g/gm workaround works. */ WARN_ON((gfp & __GFP_DMA32) && (last_pfn >= 0x0010UL)); } -#ifdef CONFIG_SWIOTLB - if (!swiotlb_nr_tbl()) -#endif + if (st->nents < st->orig_nents) sg_mark_end(sg); ret = i915_gem_gtt_prepare_pages(obj, st); -- 2.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] A couple of GTT page allocation patches
Just a couple of patches from watching talos constantly swap in/out. Doing shrink_all after a shrink in this situation was just scanning the same list as the shrink to no avail, so useless without mixing in the full slab shrinker. And since there is no way to actually disable SWIOTLB (weird kconfig), we should do our best in its presence. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 9/9] drm/i915: Address broxton phy registers based on phy and channel number
On pe, 2016-10-07 at 10:28 +0300, Ander Conselvan de Oliveira wrote: > The port registers related to the phys in broxton map to different > channels and specific phys. Make that mapping explicit. > > v2: Pass enum dpio_phy to macros instead of mmio base. (Imre) > > v3: Fix typo in macros. (Imre) > > Signed-off-by: Ander Conselvan de Oliveira > --- > drivers/gpu/drm/i915/i915_drv.h | 2 + > drivers/gpu/drm/i915/i915_reg.h | 141 > ++ > drivers/gpu/drm/i915/intel_dpio_phy.c | 80 +++ > drivers/gpu/drm/i915/intel_dpll_mgr.c | 84 +++- > 4 files changed, 185 insertions(+), 122 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 77f1374..c3fa29a 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3735,6 +3735,8 @@ u32 vlv_flisdsi_read(struct drm_i915_private *dev_priv, > u32 reg); > void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val); > > /* intel_dpio_phy.c */ > +void bxt_port_to_phy_channel(enum port port, > + u32 *phy, enum dpio_channel *ch); Type of phy doesn't match the function definition, should be enum dpio_phy. > void bxt_ddi_phy_set_signal_level(struct drm_i915_private *dev_priv, > enum port port, u32 margin, u32 scale, > u32 enable, u32 deemphasis); > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index d3802c6..8f4612c 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1187,7 +1187,19 @@ enum skl_disp_power_wells { > #define DPIO_UPAR_SHIFT30 > > /* BXT PHY registers */ > -#define _BXT_PHY(phy, a, b) _MMIO_PIPE((phy), (a), (b)) > +#define _BXT_PHY0_BASE 0x6C000 > +#define _BXT_PHY1_BASE 0x162000 > +#define BXT_PHY_BASE(phy)_PIPE((phy), _BXT_PHY0_BASE, \ > + _BXT_PHY1_BASE) > + > +#define _BXT_PHY(phy, reg) \ > + _MMIO(BXT_PHY_BASE(phy) - _BXT_PHY0_BASE + (reg)) > + > +#define _BXT_PHY_CH(phy, ch, reg_ch0, reg_ch1) \ > + (BXT_PHY_BASE(phy) + _PIPE((ch), (reg_ch0) - _BXT_PHY0_BASE,\ > + (reg_ch1) - _BXT_PHY0_BASE)) > +#define _MMIO_BXT_PHY_CH(phy, ch, reg_ch0, reg_ch1) \ > + _MMIO(_BXT_PHY_CH(phy, ch, reg_ch0, reg_ch1)) > > #define BXT_P_CR_GT_DISP_PWRON _MMIO(0x138090) > #define GT_DISPLAY_POWER_ON(phy) (1 << (phy)) > @@ -1204,8 +1216,8 @@ enum skl_disp_power_wells { > #define _PHY_CTL_FAMILY_EDP 0x64C80 > #define _PHY_CTL_FAMILY_DDI 0x64C90 > #define COMMON_RESET_DIS (1 << 31) > -#define BXT_PHY_CTL_FAMILY(phy) _BXT_PHY((phy), > _PHY_CTL_FAMILY_DDI, \ > - _PHY_CTL_FAMILY_EDP) > +#define BXT_PHY_CTL_FAMILY(phy) _MMIO_PIPE((phy), > _PHY_CTL_FAMILY_DDI, \ > + _PHY_CTL_FAMILY_EDP) > > /* BXT PHY PLL registers */ > #define _PORT_PLL_A 0x46074 > @@ -1225,18 +1237,18 @@ enum skl_disp_power_wells { > #define PORT_PLL_P2_SHIFT 8 > #define PORT_PLL_P2_MASK (0x1f << PORT_PLL_P2_SHIFT) > #define PORT_PLL_P2(x) ((x) << PORT_PLL_P2_SHIFT) > -#define BXT_PORT_PLL_EBB_0(port) _MMIO_PORT3(port, _PORT_PLL_EBB_0_A, \ > - _PORT_PLL_EBB_0_B, \ > - _PORT_PLL_EBB_0_C) > +#define BXT_PORT_PLL_EBB_0(phy, ch) _MMIO_BXT_PHY_CH(phy, ch, \ > + _PORT_PLL_EBB_0_B, \ > + _PORT_PLL_EBB_0_C) > > #define _PORT_PLL_EBB_4_A0x162038 > #define _PORT_PLL_EBB_4_B0x6C038 > #define _PORT_PLL_EBB_4_C0x6C344 > #define PORT_PLL_10BIT_CLK_ENABLE (1 << 13) > #define PORT_PLL_RECALIBRATE (1 << 14) > -#define BXT_PORT_PLL_EBB_4(port) _MMIO_PORT3(port, _PORT_PLL_EBB_4_A, \ > - _PORT_PLL_EBB_4_B, \ > - _PORT_PLL_EBB_4_C) > +#define BXT_PORT_PLL_EBB_4(phy, ch) _MMIO_BXT_PHY_CH(phy, ch, \ > + _PORT_PLL_EBB_4_B, \ > + _PORT_PLL_EBB_4_C) > > #define _PORT_PLL_0_A0x162100 > #define _PORT_PLL_0_B0x6C100 > @@ -1267,62 +1279,56 @@ enum skl_disp_power_wells { > #define PORT_PLL_DCO_AMP_DEFAULT15 > #define PORT_PLL_DCO_AMP_MASK 0x3c00 > #define PORT_PLL_DCO_AMP(x)
Re: [Intel-gfx] [PATCH] drm/i915: Assert we hold the CRTC powerwell for generating vblank interrupts
On Mon, Oct 10, 2016 at 12:46:32PM +0100, Chris Wilson wrote: > On Mon, Oct 10, 2016 at 02:42:01PM +0300, Ville Syrjälä wrote: > > On Mon, Oct 10, 2016 at 12:34:54PM +0100, Chris Wilson wrote: > > > To enable the vblank itself, we need to have an RPM wakeref for the mmio > > > access, and whilst generating the vblank interrupts we continue to > > > require the rpm wakeref. The assumption is that the RPM wakeref is held > > > by the display powerwell held by the active pipe. As this chain was not > > > obvious to me chasing the drm_wait_vblank ioctl, document it with a WARN > > > during *_vblank_enable(). > > > > > > v2: Check the display power well rather than digging inside the atomic > > > CRTC state. > > > > > > Signed-off-by: Chris Wilson > > > Cc: Ville Syrjälä > > > Cc: Maarten Lankhorst > > > --- > > > drivers/gpu/drm/i915/i915_irq.c | 20 > > > 1 file changed, 20 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > > b/drivers/gpu/drm/i915/i915_irq.c > > > index 1e43fe30da11..f0f17055dbb9 100644 > > > --- a/drivers/gpu/drm/i915/i915_irq.c > > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > > @@ -2715,6 +2715,14 @@ void i915_handle_error(struct drm_i915_private > > > *dev_priv, > > > i915_reset_and_wakeup(dev_priv); > > > } > > > > > > +static void assert_pipe_is_awake(struct drm_i915_private *dev_priv, > > > + enum pipe pipe) > > > +{ > > > + WARN_ON(IS_ENABLED(CONFIG_DRM_I915_DEBUG) && > > > + !intel_display_power_is_enabled(dev_priv, > > > + POWER_DOMAIN_PIPE(pipe))); > > > > Uses a mutex. And having a power well enabled doesn't mean much. It > > doesn't guarantee that vblanks work. > > Impasse. :| > > There should be no point in an explicit assert_rpm_wakeref here as the > register access should catch an error there. Is there no safe way we can > assert the current state of the CRTC is correct for enabling vblanks? crtc->active might be the closest thing, if we just ignore any locking. Though it looks like that has gone a bit mad these days, what with being set from the .crtc_enable() hooks but getting cleared outside the .crtc_disable() hooks. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for series starting with [v2,1/3] drm/i915: Remove unused "valid" parameter from pte_encode (rev2)
== Series Details == Series: series starting with [v2,1/3] drm/i915: Remove unused "valid" parameter from pte_encode (rev2) URL : https://patchwork.freedesktop.org/series/13444/ State : warning == Summary == Series 13444v2 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/13444/revisions/2/mbox/ Test drv_module_reload_basic: pass -> SKIP (fi-skl-6260u) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: pass -> DMESG-WARN (fi-byt-j1900) Test vgem_basic: Subgroup unload: pass -> SKIP (fi-skl-6770hq) pass -> SKIP (fi-skl-6260u) pass -> SKIP (fi-bxt-t5700) pass -> SKIP (fi-skl-6700hq) fi-bdw-5557u total:248 pass:231 dwarn:0 dfail:0 fail:0 skip:17 fi-bxt-t5700 total:248 pass:216 dwarn:0 dfail:0 fail:0 skip:32 fi-byt-j1900 total:248 pass:213 dwarn:2 dfail:0 fail:1 skip:32 fi-byt-n2820 total:248 pass:210 dwarn:0 dfail:0 fail:1 skip:37 fi-hsw-4770 total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-hsw-4770r total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-ivb-3520m total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27 fi-ivb-3770 total:248 pass:207 dwarn:0 dfail:0 fail:0 skip:41 fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-6260u total:248 pass:231 dwarn:0 dfail:0 fail:0 skip:17 fi-skl-6700hqtotal:248 pass:223 dwarn:1 dfail:0 fail:0 skip:24 fi-skl-6700k total:248 pass:221 dwarn:1 dfail:0 fail:0 skip:26 fi-skl-6770hqtotal:248 pass:230 dwarn:1 dfail:0 fail:1 skip:16 fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37 fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39 Results at /archive/results/CI_IGT_test/Patchwork_2651/ f35ed31aea66b3230c366fcba5f3456ae2cb956e drm-intel-nightly: 2016y-10m-10d-11h-28m-51s UTC integration manifest b3114dc drm/i915/gtt: Free unused lower-level page tables a7480f3 drm/i915/gtt: Split gen8_ppgtt_clear_pte_range c151f15 drm/i915: Remove unused "valid" parameter from pte_encode ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt: Free unused lower-level page tables
On Fri, Oct 07, 2016 at 08:39:22PM +0200, Michał Winiarski wrote: > Since "Dynamic page table allocations" were introduced, our page tables > can grow (being dynamically allocated) with address space range usage. > Unfortunately, their lifetime is bound to vm. This is not a huge problem > when we're not using softpin - drm_mm is creating an upper bound on used > range by causing addresses for our VMAs to eventually be reused. > > With softpin, long lived contexts can drain the system out of memory > even with a single "small" object. For example: > > bo = bo_alloc(size); > while(true) > offset += size; > exec(bo, offset); > > Will cause us to create new allocations until all memory in the system > is used for tracking GPU pages (even though almost all PTEs in this vm > are pointing to scratch). > > Let's free unused page tables in clear_range to prevent this - if no > entries are used, we can safely free it and return this information to > the caller (so that higher-level entry is pointing to scratch). > > v2: Document return value and free semantics (Joonas) > v3: No newlines in vars block (Joonas) > > Cc: Chris Wilson > Cc: Joonas Lahtinen > Cc: Michel Thierry > Cc: Mika Kuoppala > Signed-off-by: Michał Winiarski Aside from not liking the bitmaps at all (yay for the extra memory pressure for no purpose), Reviewed-by: Chris wilson (and for the earlier patches) > gen8_for_each_pde(pt, pd, start, length, pde) { > if (WARN_ON(!pd->page_table[pde])) > break; > > - gen8_ppgtt_clear_pt(vm, pt, start, length); > + reduce = gen8_ppgtt_clear_pt(vm, pt, start, length); > + > + if (reduce) { I would not have put a newline here as the if() is coupled to the clear_pte() (and I would have just used if (clear_pte())). -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] ✗ Fi.CI.BAT: warning for drm/i915: Allocate intel_engine_cs structure only for the enabled engines (rev3)
== Series Details == Series: drm/i915: Allocate intel_engine_cs structure only for the enabled engines (rev3) URL : https://patchwork.freedesktop.org/series/13435/ State : warning == Summary == Series 13435v3 drm/i915: Allocate intel_engine_cs structure only for the enabled engines https://patchwork.freedesktop.org/api/1.0/series/13435/revisions/3/mbox/ Test vgem_basic: Subgroup unload: pass -> SKIP (fi-skl-6260u) pass -> SKIP (fi-skl-6700hq) skip -> PASS (fi-skl-6700k) fi-bdw-5557u total:248 pass:231 dwarn:0 dfail:0 fail:0 skip:17 fi-bsw-n3050 total:248 pass:204 dwarn:0 dfail:0 fail:0 skip:44 fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31 fi-byt-j1900 total:248 pass:214 dwarn:1 dfail:0 fail:1 skip:32 fi-byt-n2820 total:248 pass:210 dwarn:0 dfail:0 fail:1 skip:37 fi-hsw-4770 total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-hsw-4770r total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-ilk-650 total:248 pass:185 dwarn:0 dfail:0 fail:2 skip:61 fi-ivb-3520m total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27 fi-ivb-3770 total:248 pass:207 dwarn:0 dfail:0 fail:0 skip:41 fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-6260u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-skl-6700hqtotal:248 pass:223 dwarn:1 dfail:0 fail:0 skip:24 fi-skl-6700k total:248 pass:222 dwarn:1 dfail:0 fail:0 skip:25 fi-skl-6770hqtotal:248 pass:231 dwarn:1 dfail:0 fail:1 skip:15 fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37 fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39 Results at /archive/results/CI_IGT_test/Patchwork_2652/ f35ed31aea66b3230c366fcba5f3456ae2cb956e drm-intel-nightly: 2016y-10m-10d-11h-28m-51s UTC integration manifest 401facf drm/i915: Allocate intel_engine_cs structure only for the enabled engines ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use fence_write() from rpm resume
On Fri, Oct 07, 2016 at 08:31:50PM +0100, Chris Wilson wrote: > During rpm resume we restore the fences, but we do not have the > protection of struct_mutex. This rules out updating the activity > tracking on the fences, and requires us to rely on the rpm as the > serialisation barrier instead. > > [ 350.298052] [drm:intel_runtime_resume [i915]] Resuming device > [ 350.308606] > [ 350.310520] === > [ 350.315560] [ INFO: suspicious RCU usage. ] > [ 350.320554] 4.8.0-rc8-bsw-rapl+ #3133 Tainted: G U W > [ 350.327208] --- > [ 350.331977] ../drivers/gpu/drm/i915/i915_gem_request.h:371 suspicious > rcu_dereference_protected() usage! > [ 350.342619] > [ 350.342619] other info that might help us debug this: > [ 350.342619] > [ 350.351593] > [ 350.351593] rcu_scheduler_active = 1, debug_locks = 0 > [ 350.358952] 3 locks held by Xorg/320: > [ 350.363077] #0: (&dev->mode_config.mutex){+.+.+.}, at: > [] drm_modeset_lock_all+0x3c/0xd0 [drm] > [ 350.375162] #1: (crtc_ww_class_acquire){+.+.+.}, at: > [] drm_modeset_lock_all+0x46/0xd0 [drm] > [ 350.387022] #2: (crtc_ww_class_mutex){+.+.+.}, at: [] > drm_modeset_lock+0x36/0x110 [drm] > [ 350.398236] > [ 350.398236] stack backtrace: > [ 350.403196] CPU: 1 PID: 320 Comm: Xorg Tainted: G U W > 4.8.0-rc8-bsw-rapl+ #3133 > [ 350.412457] Hardware name: Intel Corporation CHERRYVIEW C0 > PLATFORM/Braswell CRB, BIOS BRAS.X64.X088.R00.1510270350 10/27/2015 > [ 350.425212] 8801680a78c8 81332187 > 88016c5c5000 > [ 350.433611] 0001 8801680a78f8 810ca6da > 88016cc8b0f0 > [ 350.442012] 88016cc8 88016cc8 880177ad > 8801680a7948 > [ 350.450409] Call Trace: > [ 350.453165] [] dump_stack+0x67/0x90 > [ 350.458931] [] lockdep_rcu_suspicious+0xea/0x120 > [ 350.466002] [] fence_update+0xbd/0x670 [i915] > [ 350.472766] [] i915_gem_restore_fences+0x52/0x70 [i915] > [ 350.480496] [] vlv_resume_prepare+0x72/0x570 [i915] > [ 350.487839] [] intel_runtime_resume+0x102/0x210 [i915] > [ 350.495442] [] pci_pm_runtime_resume+0x7f/0xb0 > [ 350.502274] [] ? pci_restore_standard_config+0x40/0x40 > [ 350.509883] [] __rpm_callback+0x35/0x70 > [ 350.516037] [] ? pci_restore_standard_config+0x40/0x40 > [ 350.523646] [] rpm_callback+0x24/0x80 > [ 350.529604] [] ? pci_restore_standard_config+0x40/0x40 > [ 350.537212] [] rpm_resume+0x4ad/0x740 > [ 350.543161] [] __pm_runtime_resume+0x51/0x80 > [ 350.549824] [] intel_runtime_pm_get+0x28/0x90 [i915] > [ 350.557265] [] intel_display_power_get+0x23/0x50 [i915] > [ 350.565001] [] intel_atomic_commit_tail+0xdfd/0x10b0 > [i915] > [ 350.573106] [] ? > drm_atomic_helper_swap_state+0x159/0x300 [drm_kms_helper] > [ 350.582659] [] ? _raw_spin_unlock+0x31/0x50 > [ 350.589205] [] ? > drm_atomic_helper_swap_state+0x159/0x300 [drm_kms_helper] > [ 350.598787] [] intel_atomic_commit+0x3b5/0x500 [i915] > [ 350.606319] [] ? > drm_atomic_set_crtc_for_connector+0xcc/0x100 [drm] > [ 350.615209] [] drm_atomic_commit+0x49/0x50 [drm] > [ 350.622242] [] drm_atomic_helper_set_config+0x88/0xc0 > [drm_kms_helper] > [ 350.631419] [] drm_mode_set_config_internal+0x6c/0x120 > [drm] > [ 350.639623] [] drm_mode_setcrtc+0x22c/0x4d0 [drm] > [ 350.646760] [] drm_ioctl+0x209/0x460 [drm] > [ 350.653217] [] ? drm_mode_getcrtc+0x150/0x150 [drm] > [ 350.660536] [] ? __lock_is_held+0x4a/0x70 > [ 350.666885] [] do_vfs_ioctl+0x93/0x6b0 > [ 350.672939] [] ? __fget+0x113/0x200 > [ 350.678797] [] ? __fget+0x5/0x200 > [ 350.684361] [] SyS_ioctl+0x44/0x80 > [ 350.690030] [] do_syscall_64+0x5b/0x120 > [ 350.696184] [] entry_SYSCALL64_slow_path+0x25/0x25 > > Note we also have to remember the lesson from commit 4fc788f5ee3d > ("drm/i915: Flush delayed fence releases after reset") where we have to > flush any changes to the fence on restore. > > Fixes: 49ef5294cda2 ("drm/i915: Move fence tracking from object to vma") > Reported-by: Ville Syrjälä > Signed-off-by: Chris Wilson > Cc: Ville Syrjälä > Cc: Joonas Lahtinen > Cc: Mika Kuoppala > --- > drivers/gpu/drm/i915/i915_gem_fence.c | 15 +-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c > b/drivers/gpu/drm/i915/i915_gem_fence.c > index 834d649496f3..1d6d95cef7f5 100644 > --- a/drivers/gpu/drm/i915/i915_gem_fence.c > +++ b/drivers/gpu/drm/i915/i915_gem_fence.c > @@ -371,6 +371,12 @@ void i915_gem_restore_fences(struct drm_device *dev) > struct drm_i915_private *dev_priv = to_i915(dev); > int i; > > + /* Note that this may be called outside of struct_mutex, by > + * runtime suspend/resume. The barrier we require is enforced by > + * rpm itself - all access to fences/GTT are only within an rpm > + * wakeref, and to acquire that wakeref you must pass through here. > + */ > + >
[Intel-gfx] [PATCH] drm/i915: Fix silent conflict from backmerge of drm-next into dinf
Merging from drm-next pulled back in a few lines of dead code due to the code movement around i915_gem_reset(), fix that up. Signed-off-by: Chris Wilson Cc: Jani Nikula Cc: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 981a106901ae..1418c1c522cb 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2616,8 +2616,6 @@ static void i915_gem_reset_engine(struct intel_engine_cs *engine) list_for_each_entry_continue(request, &engine->request_list, link) if (request->ctx == incomplete_ctx) reset_request(request); - - engine->i915->gt.active_engines &= ~intel_engine_flag(engine); } void i915_gem_reset(struct drm_i915_private *dev_priv) @@ -2628,7 +2626,6 @@ void i915_gem_reset(struct drm_i915_private *dev_priv) for_each_engine(engine, dev_priv) i915_gem_reset_engine(engine); - mod_delayed_work(dev_priv->wq, &dev_priv->gt.idle_work, 0); i915_gem_restore_fences(&dev_priv->drm); -- 2.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Remove self-harming shrink_all on get_pages_gtt fail
On Mon, Oct 10, 2016 at 12:49:58PM +0100, Chris Wilson wrote: > When we notice the system under memory pressure, we try to evict some > driver pages before asking the VM to shrink all caches. As a final step > in that process, we tried to evict everything, including active buffers. > This is harming ourselves, and we can mix shrinking all caches as well > as our residual buffers (after the first pass of trying to shrink just > our own buffers). Reviewed-by: Michał Winiarski -Michał > Signed-off-by: Chris Wilson > --- > drivers/gpu/drm/i915/i915_gem.c | 1 - > 1 file changed, 1 deletion(-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Use fence_write() from rpm resume
== Series Details == Series: drm/i915: Use fence_write() from rpm resume URL : https://patchwork.freedesktop.org/series/13458/ State : warning == Summary == Series 13458v1 drm/i915: Use fence_write() from rpm resume https://patchwork.freedesktop.org/api/1.0/series/13458/revisions/1/mbox/ File CI_DRM_1698/fi-bdw-5557u/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0x9f in position 11: invalid start byte File Patchwork_2653/fi-bdw-5557u/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xfc in position 10: invalid start byte File CI_DRM_1698/fi-bsw-n3050/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xa8 in position 11: invalid start byte File Patchwork_2653/fi-bsw-n3050/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xf6 in position 11: invalid start byte File CI_DRM_1698/fi-bxt-t5700/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xed in position 10: invalid continuation byte File Patchwork_2653/fi-bxt-t5700/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xaf in position 12: invalid start byte File CI_DRM_1698/fi-byt-j1900/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xc0 in position 15: invalid start byte File Patchwork_2653/fi-byt-j1900/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0x9f in position 10: invalid start byte File CI_DRM_1698/fi-byt-n2820/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xbb in position 10: invalid start byte File Patchwork_2653/fi-byt-n2820/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0x81 in position 10: invalid start byte File CI_DRM_1698/fi-hsw-4770/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xa1 in position 10: invalid start byte File Patchwork_2653/fi-hsw-4770/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xef in position 11: invalid continuation byte File CI_DRM_1698/fi-hsw-4770r/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xd8 in position 10: invalid continuation byte File Patchwork_2653/fi-hsw-4770r/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xad in position 12: invalid start byte File Patchwork_2653/fi-ilk-650/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xf3 in position 11: invalid continuation byte File CI_DRM_1698/fi-ivb-3520m/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xc5 in position 12: invalid continuation byte File Patchwork_2653/fi-ivb-3520m/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xb0 in position 10: invalid start byte File CI_DRM_1698/fi-ivb-3770/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0x94 in position 11: invalid start byte File Patchwork_2653/fi-ivb-3770/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xbf in position 15: invalid start byte File CI_DRM_1698/fi-kbl-7200u/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xc7 in position 10: invalid continuation byte File Patchwork_2653/fi-kbl-7200u/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xa8 in position 10: invalid start byte File CI_DRM_1698/fi-skl-6260u/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xb6 in position 11: invalid start byte File Patchwork_2653/fi-skl-6260u/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xc6 in position 10: invalid continuation byte File CI_DRM_1698/fi-skl-6700hq/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0x89 in position 12: invalid start byte File Patchwork_2653/fi-skl-6700hq/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0x87 in position 11: invalid start byte File CI_DRM_1698/fi-skl-6700k/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xbe in position 15: invalid start byte File Patchwork_2653/fi-skl-6700k/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0x9e in position 10: invalid start byte File CI_DRM_1698/fi-skl-6770hq/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xfd in position 11: invalid start byte File Patchwork_2653/fi-skl-6770hq/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xde in position 11: invalid continuation byte File CI_DRM_1698/fi-snb-2520m/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0x94 in position 12: invalid start byte File Patchwork_2653/fi-snb-2520m/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xcf in position 15: invalid continuation byte File CI_DRM_1698/fi-snb-2600/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xe8 in position 11: invalid continuation byte File Patchwork_2653/fi-snb-2600/results1.json.bz2.old not loaded: 'utf-8' codec can't decode byte 0xb2 in position 10: invalid start byte Test kms_pipe_crc_basic: Subgroup read-crc-pipe-b:
Re: [Intel-gfx] [PATCH] drm/i915: Use fence_write() from rpm resume
On Mon, Oct 10, 2016 at 03:41:04PM +0300, Ville Syrjälä wrote: > On Fri, Oct 07, 2016 at 08:31:50PM +0100, Chris Wilson wrote: > > During rpm resume we restore the fences, but we do not have the > > protection of struct_mutex. This rules out updating the activity > > tracking on the fences, and requires us to rely on the rpm as the > > serialisation barrier instead. > > > > [ 350.298052] [drm:intel_runtime_resume [i915]] Resuming device > > [ 350.308606] > > [ 350.310520] === > > [ 350.315560] [ INFO: suspicious RCU usage. ] > > [ 350.320554] 4.8.0-rc8-bsw-rapl+ #3133 Tainted: G U W > > [ 350.327208] --- > > [ 350.331977] ../drivers/gpu/drm/i915/i915_gem_request.h:371 suspicious > > rcu_dereference_protected() usage! > > [ 350.342619] > > [ 350.342619] other info that might help us debug this: > > [ 350.342619] > > [ 350.351593] > > [ 350.351593] rcu_scheduler_active = 1, debug_locks = 0 > > [ 350.358952] 3 locks held by Xorg/320: > > [ 350.363077] #0: (&dev->mode_config.mutex){+.+.+.}, at: > > [] drm_modeset_lock_all+0x3c/0xd0 [drm] > > [ 350.375162] #1: (crtc_ww_class_acquire){+.+.+.}, at: > > [] drm_modeset_lock_all+0x46/0xd0 [drm] > > [ 350.387022] #2: (crtc_ww_class_mutex){+.+.+.}, at: > > [] drm_modeset_lock+0x36/0x110 [drm] > > [ 350.398236] > > [ 350.398236] stack backtrace: > > [ 350.403196] CPU: 1 PID: 320 Comm: Xorg Tainted: G U W > > 4.8.0-rc8-bsw-rapl+ #3133 > > [ 350.412457] Hardware name: Intel Corporation CHERRYVIEW C0 > > PLATFORM/Braswell CRB, BIOS BRAS.X64.X088.R00.1510270350 10/27/2015 > > [ 350.425212] 8801680a78c8 81332187 > > 88016c5c5000 > > [ 350.433611] 0001 8801680a78f8 810ca6da > > 88016cc8b0f0 > > [ 350.442012] 88016cc8 88016cc8 880177ad > > 8801680a7948 > > [ 350.450409] Call Trace: > > [ 350.453165] [] dump_stack+0x67/0x90 > > [ 350.458931] [] lockdep_rcu_suspicious+0xea/0x120 > > [ 350.466002] [] fence_update+0xbd/0x670 [i915] > > [ 350.472766] [] i915_gem_restore_fences+0x52/0x70 > > [i915] > > [ 350.480496] [] vlv_resume_prepare+0x72/0x570 [i915] > > [ 350.487839] [] intel_runtime_resume+0x102/0x210 [i915] > > [ 350.495442] [] pci_pm_runtime_resume+0x7f/0xb0 > > [ 350.502274] [] ? pci_restore_standard_config+0x40/0x40 > > [ 350.509883] [] __rpm_callback+0x35/0x70 > > [ 350.516037] [] ? pci_restore_standard_config+0x40/0x40 > > [ 350.523646] [] rpm_callback+0x24/0x80 > > [ 350.529604] [] ? pci_restore_standard_config+0x40/0x40 > > [ 350.537212] [] rpm_resume+0x4ad/0x740 > > [ 350.543161] [] __pm_runtime_resume+0x51/0x80 > > [ 350.549824] [] intel_runtime_pm_get+0x28/0x90 [i915] > > [ 350.557265] [] intel_display_power_get+0x23/0x50 > > [i915] > > [ 350.565001] [] intel_atomic_commit_tail+0xdfd/0x10b0 > > [i915] > > [ 350.573106] [] ? > > drm_atomic_helper_swap_state+0x159/0x300 [drm_kms_helper] > > [ 350.582659] [] ? _raw_spin_unlock+0x31/0x50 > > [ 350.589205] [] ? > > drm_atomic_helper_swap_state+0x159/0x300 [drm_kms_helper] > > [ 350.598787] [] intel_atomic_commit+0x3b5/0x500 [i915] > > [ 350.606319] [] ? > > drm_atomic_set_crtc_for_connector+0xcc/0x100 [drm] > > [ 350.615209] [] drm_atomic_commit+0x49/0x50 [drm] > > [ 350.622242] [] drm_atomic_helper_set_config+0x88/0xc0 > > [drm_kms_helper] > > [ 350.631419] [] > > drm_mode_set_config_internal+0x6c/0x120 [drm] > > [ 350.639623] [] drm_mode_setcrtc+0x22c/0x4d0 [drm] > > [ 350.646760] [] drm_ioctl+0x209/0x460 [drm] > > [ 350.653217] [] ? drm_mode_getcrtc+0x150/0x150 [drm] > > [ 350.660536] [] ? __lock_is_held+0x4a/0x70 > > [ 350.666885] [] do_vfs_ioctl+0x93/0x6b0 > > [ 350.672939] [] ? __fget+0x113/0x200 > > [ 350.678797] [] ? __fget+0x5/0x200 > > [ 350.684361] [] SyS_ioctl+0x44/0x80 > > [ 350.690030] [] do_syscall_64+0x5b/0x120 > > [ 350.696184] [] entry_SYSCALL64_slow_path+0x25/0x25 > > > > Note we also have to remember the lesson from commit 4fc788f5ee3d > > ("drm/i915: Flush delayed fence releases after reset") where we have to > > flush any changes to the fence on restore. > > > > Fixes: 49ef5294cda2 ("drm/i915: Move fence tracking from object to vma") > > Reported-by: Ville Syrjälä > > Signed-off-by: Chris Wilson > > Cc: Ville Syrjälä > > Cc: Joonas Lahtinen > > Cc: Mika Kuoppala > > --- > > drivers/gpu/drm/i915/i915_gem_fence.c | 15 +-- > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c > > b/drivers/gpu/drm/i915/i915_gem_fence.c > > index 834d649496f3..1d6d95cef7f5 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_fence.c > > +++ b/drivers/gpu/drm/i915/i915_gem_fence.c > > @@ -371,6 +371,12 @@ void i915_gem_restore_fences(struct drm_device *dev) > > struct drm_i915_private *dev_priv = to_i915(dev); > > int i; > > > > + /* Note that this may be c
Re: [Intel-gfx] [PATCH] drm/i915: Fix silent conflict from backmerge of drm-next into dinf
On Mon, 10 Oct 2016, Chris Wilson wrote: > Merging from drm-next pulled back in a few lines of dead code due to the > code movement around i915_gem_reset(), fix that up. Actually the problem was introduced in commit ca09fb9f60b5f3ab2d57e761aaeea89a5147d784 Merge: 9f4ef05bcdcf 08895a8b6b06 Author: Dave Airlie Date: Wed Sep 28 12:08:49 2016 +1000 Merge tag 'v4.8-rc8' into drm-next and it's present in drm-next, and the conflict wasn't silent. If this is the right fix (Daniel ack?) I'll queue it with other drm-next fixes that I have. BR, Jani. > > Signed-off-by: Chris Wilson > Cc: Jani Nikula > Cc: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_gem.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 981a106901ae..1418c1c522cb 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2616,8 +2616,6 @@ static void i915_gem_reset_engine(struct > intel_engine_cs *engine) > list_for_each_entry_continue(request, &engine->request_list, link) > if (request->ctx == incomplete_ctx) > reset_request(request); > - > - engine->i915->gt.active_engines &= ~intel_engine_flag(engine); > } > > void i915_gem_reset(struct drm_i915_private *dev_priv) > @@ -2628,7 +2626,6 @@ void i915_gem_reset(struct drm_i915_private *dev_priv) > > for_each_engine(engine, dev_priv) > i915_gem_reset_engine(engine); > - mod_delayed_work(dev_priv->wq, &dev_priv->gt.idle_work, 0); > > i915_gem_restore_fences(&dev_priv->drm); -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use fence_write() from rpm resume
On Mon, Oct 10, 2016 at 02:07:16PM +0100, Chris Wilson wrote: > On Mon, Oct 10, 2016 at 03:41:04PM +0300, Ville Syrjälä wrote: > > On Fri, Oct 07, 2016 at 08:31:50PM +0100, Chris Wilson wrote: > > > During rpm resume we restore the fences, but we do not have the > > > protection of struct_mutex. This rules out updating the activity > > > tracking on the fences, and requires us to rely on the rpm as the > > > serialisation barrier instead. > > > > > > [ 350.298052] [drm:intel_runtime_resume [i915]] Resuming device > > > [ 350.308606] > > > [ 350.310520] === > > > [ 350.315560] [ INFO: suspicious RCU usage. ] > > > [ 350.320554] 4.8.0-rc8-bsw-rapl+ #3133 Tainted: G U W > > > [ 350.327208] --- > > > [ 350.331977] ../drivers/gpu/drm/i915/i915_gem_request.h:371 suspicious > > > rcu_dereference_protected() usage! > > > [ 350.342619] > > > [ 350.342619] other info that might help us debug this: > > > [ 350.342619] > > > [ 350.351593] > > > [ 350.351593] rcu_scheduler_active = 1, debug_locks = 0 > > > [ 350.358952] 3 locks held by Xorg/320: > > > [ 350.363077] #0: (&dev->mode_config.mutex){+.+.+.}, at: > > > [] drm_modeset_lock_all+0x3c/0xd0 [drm] > > > [ 350.375162] #1: (crtc_ww_class_acquire){+.+.+.}, at: > > > [] drm_modeset_lock_all+0x46/0xd0 [drm] > > > [ 350.387022] #2: (crtc_ww_class_mutex){+.+.+.}, at: > > > [] drm_modeset_lock+0x36/0x110 [drm] > > > [ 350.398236] > > > [ 350.398236] stack backtrace: > > > [ 350.403196] CPU: 1 PID: 320 Comm: Xorg Tainted: G U W > > > 4.8.0-rc8-bsw-rapl+ #3133 > > > [ 350.412457] Hardware name: Intel Corporation CHERRYVIEW C0 > > > PLATFORM/Braswell CRB, BIOS BRAS.X64.X088.R00.1510270350 10/27/2015 > > > [ 350.425212] 8801680a78c8 81332187 > > > 88016c5c5000 > > > [ 350.433611] 0001 8801680a78f8 810ca6da > > > 88016cc8b0f0 > > > [ 350.442012] 88016cc8 88016cc8 880177ad > > > 8801680a7948 > > > [ 350.450409] Call Trace: > > > [ 350.453165] [] dump_stack+0x67/0x90 > > > [ 350.458931] [] lockdep_rcu_suspicious+0xea/0x120 > > > [ 350.466002] [] fence_update+0xbd/0x670 [i915] > > > [ 350.472766] [] i915_gem_restore_fences+0x52/0x70 > > > [i915] > > > [ 350.480496] [] vlv_resume_prepare+0x72/0x570 [i915] > > > [ 350.487839] [] intel_runtime_resume+0x102/0x210 > > > [i915] > > > [ 350.495442] [] pci_pm_runtime_resume+0x7f/0xb0 > > > [ 350.502274] [] ? > > > pci_restore_standard_config+0x40/0x40 > > > [ 350.509883] [] __rpm_callback+0x35/0x70 > > > [ 350.516037] [] ? > > > pci_restore_standard_config+0x40/0x40 > > > [ 350.523646] [] rpm_callback+0x24/0x80 > > > [ 350.529604] [] ? > > > pci_restore_standard_config+0x40/0x40 > > > [ 350.537212] [] rpm_resume+0x4ad/0x740 > > > [ 350.543161] [] __pm_runtime_resume+0x51/0x80 > > > [ 350.549824] [] intel_runtime_pm_get+0x28/0x90 [i915] > > > [ 350.557265] [] intel_display_power_get+0x23/0x50 > > > [i915] > > > [ 350.565001] [] > > > intel_atomic_commit_tail+0xdfd/0x10b0 [i915] > > > [ 350.573106] [] ? > > > drm_atomic_helper_swap_state+0x159/0x300 [drm_kms_helper] > > > [ 350.582659] [] ? _raw_spin_unlock+0x31/0x50 > > > [ 350.589205] [] ? > > > drm_atomic_helper_swap_state+0x159/0x300 [drm_kms_helper] > > > [ 350.598787] [] intel_atomic_commit+0x3b5/0x500 > > > [i915] > > > [ 350.606319] [] ? > > > drm_atomic_set_crtc_for_connector+0xcc/0x100 [drm] > > > [ 350.615209] [] drm_atomic_commit+0x49/0x50 [drm] > > > [ 350.622242] [] > > > drm_atomic_helper_set_config+0x88/0xc0 [drm_kms_helper] > > > [ 350.631419] [] > > > drm_mode_set_config_internal+0x6c/0x120 [drm] > > > [ 350.639623] [] drm_mode_setcrtc+0x22c/0x4d0 [drm] > > > [ 350.646760] [] drm_ioctl+0x209/0x460 [drm] > > > [ 350.653217] [] ? drm_mode_getcrtc+0x150/0x150 [drm] > > > [ 350.660536] [] ? __lock_is_held+0x4a/0x70 > > > [ 350.666885] [] do_vfs_ioctl+0x93/0x6b0 > > > [ 350.672939] [] ? __fget+0x113/0x200 > > > [ 350.678797] [] ? __fget+0x5/0x200 > > > [ 350.684361] [] SyS_ioctl+0x44/0x80 > > > [ 350.690030] [] do_syscall_64+0x5b/0x120 > > > [ 350.696184] [] entry_SYSCALL64_slow_path+0x25/0x25 > > > > > > Note we also have to remember the lesson from commit 4fc788f5ee3d > > > ("drm/i915: Flush delayed fence releases after reset") where we have to > > > flush any changes to the fence on restore. > > > > > > Fixes: 49ef5294cda2 ("drm/i915: Move fence tracking from object to vma") > > > Reported-by: Ville Syrjälä > > > Signed-off-by: Chris Wilson > > > Cc: Ville Syrjälä > > > Cc: Joonas Lahtinen > > > Cc: Mika Kuoppala > > > --- > > > drivers/gpu/drm/i915/i915_gem_fence.c | 15 +-- > > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c > > > b/drivers/gpu/drm/i915/i915_gem_fence.c > > > index 834d649496f3..1d6d95ce
Re: [Intel-gfx] [PATCH v2 2/3] drm/i915/gtt: Split gen8_ppgtt_clear_pte_range
Michał Winiarski writes: > Let's use more top-down approach, where each gen8_ppgtt_clear_* function > is responsible for clearing the struct passed as an argument and calling > relevant clear_range functions on lower-level tables. > Doing this rather than operating on PTE ranges makes the implementation > of shrinking page tables quite simple. > > v2: Drop min when calculating num_entries, no negation in 48b ppgtt > check, no newlines in vars block (Joonas) > > Cc: Chris Wilson > Cc: Joonas Lahtinen > Cc: Michel Thierry > Cc: Mika Kuoppala > Signed-off-by: Michał Winiarski Reviewed-by: Mika Kuoppala > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 107 > +++- > 1 file changed, 58 insertions(+), 49 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 08e2f35..adabf58 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -704,59 +704,78 @@ static int gen8_48b_mm_switch(struct i915_hw_ppgtt > *ppgtt, > return gen8_write_pdp(req, 0, px_dma(&ppgtt->pml4)); > } > > -static void gen8_ppgtt_clear_pte_range(struct i915_address_space *vm, > -struct i915_page_directory_pointer *pdp, > -uint64_t start, > -uint64_t length, > -gen8_pte_t scratch_pte) > +static void gen8_ppgtt_clear_pt(struct i915_address_space *vm, > + struct i915_page_table *pt, > + uint64_t start, > + uint64_t length) > { > struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm); > + > + unsigned int pte_start = gen8_pte_index(start); > + unsigned int num_entries = gen8_pte_count(start, length); > + uint64_t pte; > gen8_pte_t *pt_vaddr; > - unsigned pdpe = gen8_pdpe_index(start); > - unsigned pde = gen8_pde_index(start); > - unsigned pte = gen8_pte_index(start); > - unsigned num_entries = length >> PAGE_SHIFT; > - unsigned last_pte, i; > + gen8_pte_t scratch_pte = gen8_pte_encode(vm->scratch_page.daddr, > + I915_CACHE_LLC); > > - if (WARN_ON(!pdp)) > + if (WARN_ON(!px_page(pt))) > return; > > - while (num_entries) { > - struct i915_page_directory *pd; > - struct i915_page_table *pt; > + bitmap_clear(pt->used_ptes, pte_start, num_entries); > > - if (WARN_ON(!pdp->page_directory[pdpe])) > - break; > + pt_vaddr = kmap_px(pt); > + > + for (pte = pte_start; pte < num_entries; pte++) > + pt_vaddr[pte] = scratch_pte; > > - pd = pdp->page_directory[pdpe]; > + kunmap_px(ppgtt, pt_vaddr); > +} > + > +static void gen8_ppgtt_clear_pd(struct i915_address_space *vm, > + struct i915_page_directory *pd, > + uint64_t start, > + uint64_t length) > +{ > + struct i915_page_table *pt; > + uint64_t pde; > > + gen8_for_each_pde(pt, pd, start, length, pde) { > if (WARN_ON(!pd->page_table[pde])) > break; > > - pt = pd->page_table[pde]; > + gen8_ppgtt_clear_pt(vm, pt, start, length); > + } > +} > > - if (WARN_ON(!px_page(pt))) > - break; > +static void gen8_ppgtt_clear_pdp(struct i915_address_space *vm, > + struct i915_page_directory_pointer *pdp, > + uint64_t start, > + uint64_t length) > +{ > + struct i915_page_directory *pd; > + uint64_t pdpe; > > - last_pte = pte + num_entries; > - if (last_pte > GEN8_PTES) > - last_pte = GEN8_PTES; > + gen8_for_each_pdpe(pd, pdp, start, length, pdpe) { > + if (WARN_ON(!pdp->page_directory[pdpe])) > + break; > > - pt_vaddr = kmap_px(pt); > + gen8_ppgtt_clear_pd(vm, pd, start, length); > + } > +} > > - for (i = pte; i < last_pte; i++) { > - pt_vaddr[i] = scratch_pte; > - num_entries--; > - } > +static void gen8_ppgtt_clear_pml4(struct i915_address_space *vm, > + struct i915_pml4 *pml4, > + uint64_t start, > + uint64_t length) > +{ > + struct i915_page_directory_pointer *pdp; > + uint64_t pml4e; > > - kunmap_px(ppgtt, pt_vaddr); > + gen8_for_each_pml4e(pdp, pml4, start, length, pml4e) { > + if (WARN_ON(!pml4->pdps[pml4e])) > + break; > > - pte = 0; > - if (++pde == I915_PDES) { > - if (++pdpe ==
Re: [Intel-gfx] [PATCH RFC v2 0/5] Link Training Compliance support, link rate fallback sending Uevent
On Sat, 08 Oct 2016, Manasi Navare wrote: > This adds support in the kernel to handle link training compliance > requests and send a uevent to train at requested parameters. > In this patch series, if the link training fails in modeset, then a > hotplug uevent is added to a work queue and scheduled and failed link > parameters are stored in intel_dp structure. This uevent > gets executed after the modeset is completed and after locks are released. > In the modeset retry, modes are validated based on lower link rate from > the failed link rate value to prune the modes in intel_dp_mode_valid(). > The lower link rate is then used toc onfigure the pipe and link is retrained > at this lower link rate. > > I have tested this with DPR-120 and the compliance tests are passing. > The DP CTS spec is based on DP 1.2 spec and it only expects to reduce the link > rate and train at the max lane count without reducing the lane count. Lane > count > reduction is added in DP 1.3 so Unigraf has agreed to add it to their next SW > release. > Hence I have not implemented fallback for lane count. But I can add it if you > think > we should add DP 1.3 link training algorithm. > > Please let me know your feedback on this. Compliance app is not required for > link training > tests and so I have tested this fallback using kernel console mode on Source > DUT. The question is, can you pass the link training tests using a userspace that responds to hotplug uevents? I think you mentioned a timeout in the CTS. If you're using fbdev, I think the retry modeset may happen faster than with a regular userspace. BR, Jani. > > v2: > * Corect the order of patches for correct dependencies > > Manasi Navare (4): > drm/i915; Add a function to return index of link rate > drm/i915: Add support for DP link training compliance > drm/i915: Work queue for uevent > drm/i915: Link Rate fallback on Link training failure > > Navare, Manasi D (1): > drm/i915: Change the placement of some static functions in intel_dp.c > > drivers/gpu/drm/i915/i915_drv.h | 2 + > drivers/gpu/drm/i915/intel_ddi.c | 10 +- > drivers/gpu/drm/i915/intel_dp.c | 277 > ++ > drivers/gpu/drm/i915/intel_dp_link_training.c | 12 +- > drivers/gpu/drm/i915/intel_drv.h | 8 +- > 5 files changed, 217 insertions(+), 92 deletions(-) -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915/gen9: don't call skl_pipe_pixel_rate() twice on the same function
== Series Details == Series: series starting with [1/2] drm/i915/gen9: don't call skl_pipe_pixel_rate() twice on the same function URL : https://patchwork.freedesktop.org/series/13464/ State : warning == Summary == Series 13464v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/13464/revisions/1/mbox/ Test kms_psr_sink_crc: Subgroup psr_basic: dmesg-warn -> PASS (fi-skl-6700hq) Test vgem_basic: Subgroup unload: skip -> PASS (fi-ivb-3770) pass -> SKIP (fi-skl-6700hq) skip -> PASS (fi-skl-6700k) pass -> SKIP (fi-ilk-650) pass -> SKIP (fi-skl-6260u) skip -> PASS (fi-byt-n2820) fi-bdw-5557u total:248 pass:231 dwarn:0 dfail:0 fail:0 skip:17 fi-bsw-n3050 total:248 pass:204 dwarn:0 dfail:0 fail:0 skip:44 fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31 fi-byt-j1900 total:248 pass:214 dwarn:1 dfail:0 fail:1 skip:32 fi-byt-n2820 total:248 pass:211 dwarn:0 dfail:0 fail:1 skip:36 fi-hsw-4770 total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-hsw-4770r total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-ilk-650 total:248 pass:184 dwarn:0 dfail:0 fail:2 skip:62 fi-ivb-3520m total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27 fi-ivb-3770 total:248 pass:208 dwarn:0 dfail:0 fail:0 skip:40 fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-6260u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-skl-6700hqtotal:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-skl-6700k total:248 pass:222 dwarn:1 dfail:0 fail:0 skip:25 fi-skl-6770hqtotal:248 pass:231 dwarn:1 dfail:0 fail:1 skip:15 fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37 fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39 Results at /archive/results/CI_IGT_test/Patchwork_2654/ f35ed31aea66b3230c366fcba5f3456ae2cb956e drm-intel-nightly: 2016y-10m-10d-11h-28m-51s UTC integration manifest 0bb92db drm/i915/gen9: fix watermarks when using the pipe scaler b7a6ac9 drm/i915/gen9: don't call skl_pipe_pixel_rate() twice on the same function ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Allow compaction upto SWIOTLB max segment size
On 10/10/2016 12:49, Chris Wilson wrote: commit 1625e7e549c5 ("drm/i915: make compact dma scatter lists creation work with SWIOTLB backend") took a heavy handed approach to undo the scatterlist compaction in the face of SWIOTLB. (The compaction hit a bug whereby we tried to pass a segment larger than SWIOTLB could handle.) We can be a little more intelligent and try compacting the scatterlist up to the maximum SWIOTLB segment size (when using SWIOTLB). Signed-off-by: Chris Wilson CC: Imre Deak CC: Daniel Vetter Cc: Konrad Rzeszutek Wilk --- drivers/gpu/drm/i915/i915_gem.c | 23 +++ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ca1a5a5c6f19..8b3474d215a5 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2201,6 +2201,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) struct sgt_iter sgt_iter; struct page *page; unsigned long last_pfn = 0; /* suppress gcc warning */ + unsigned long max_segment; unsigned int would be enough. int ret; gfp_t gfp; @@ -2211,6 +2212,12 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) GEM_BUG_ON(obj->base.read_domains & I915_GEM_GPU_DOMAINS); GEM_BUG_ON(obj->base.write_domain & I915_GEM_GPU_DOMAINS); + max_segment = obj->base.size; +#ifdef CONFIG_SWIOTLB + if (swiotlb_nr_tbl()) + max_segment = IO_TLB_SEGSIZE << PAGE_SHIFT; +#endif + Do you want to use IS_ENABLED here? st = kmalloc(sizeof(*st), GFP_KERNEL); if (st == NULL) return ERR_PTR(-ENOMEM); @@ -2252,15 +2259,9 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) goto err_pages; } } -#ifdef CONFIG_SWIOTLB - if (swiotlb_nr_tbl()) { - st->nents++; - sg_set_page(sg, page, PAGE_SIZE, 0); - sg = sg_next(sg); - continue; - } -#endif - if (!i || page_to_pfn(page) != last_pfn + 1) { + if (!i || + sg->length >= max_segment || I think this can overflow by a page, should be "sg->length >= (max_segment - PAGE_SIZE)", or alternatively substract one page at the max_segment assignment. + page_to_pfn(page) != last_pfn + 1) { if (i) sg = sg_next(sg); st->nents++; @@ -2273,9 +2274,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) /* Check that the i965g/gm workaround works. */ WARN_ON((gfp & __GFP_DMA32) && (last_pfn >= 0x0010UL)); } -#ifdef CONFIG_SWIOTLB - if (!swiotlb_nr_tbl()) -#endif + if (st->nents < st->orig_nents) sg_mark_end(sg); I wondered a few times that we could just terminate the table unconditionally. ret = i915_gem_gtt_prepare_pages(obj, st); Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use fence_write() from rpm resume
On Mon, Oct 10, 2016 at 04:13:26PM +0300, Ville Syrjälä wrote: > On Mon, Oct 10, 2016 at 02:07:16PM +0100, Chris Wilson wrote: > > On Mon, Oct 10, 2016 at 03:41:04PM +0300, Ville Syrjälä wrote: > > > On Fri, Oct 07, 2016 at 08:31:50PM +0100, Chris Wilson wrote: > > > > During rpm resume we restore the fences, but we do not have the > > > > protection of struct_mutex. This rules out updating the activity > > > > tracking on the fences, and requires us to rely on the rpm as the > > > > serialisation barrier instead. > > > > > > > > [ 350.298052] [drm:intel_runtime_resume [i915]] Resuming device > > > > [ 350.308606] > > > > [ 350.310520] === > > > > [ 350.315560] [ INFO: suspicious RCU usage. ] > > > > [ 350.320554] 4.8.0-rc8-bsw-rapl+ #3133 Tainted: G U W > > > > [ 350.327208] --- > > > > [ 350.331977] ../drivers/gpu/drm/i915/i915_gem_request.h:371 > > > > suspicious rcu_dereference_protected() usage! > > > > [ 350.342619] > > > > [ 350.342619] other info that might help us debug this: > > > > [ 350.342619] > > > > [ 350.351593] > > > > [ 350.351593] rcu_scheduler_active = 1, debug_locks = 0 > > > > [ 350.358952] 3 locks held by Xorg/320: > > > > [ 350.363077] #0: (&dev->mode_config.mutex){+.+.+.}, at: > > > > [] drm_modeset_lock_all+0x3c/0xd0 [drm] > > > > [ 350.375162] #1: (crtc_ww_class_acquire){+.+.+.}, at: > > > > [] drm_modeset_lock_all+0x46/0xd0 [drm] > > > > [ 350.387022] #2: (crtc_ww_class_mutex){+.+.+.}, at: > > > > [] drm_modeset_lock+0x36/0x110 [drm] > > > > [ 350.398236] > > > > [ 350.398236] stack backtrace: > > > > [ 350.403196] CPU: 1 PID: 320 Comm: Xorg Tainted: G U W > > > > 4.8.0-rc8-bsw-rapl+ #3133 > > > > [ 350.412457] Hardware name: Intel Corporation CHERRYVIEW C0 > > > > PLATFORM/Braswell CRB, BIOS BRAS.X64.X088.R00.1510270350 10/27/2015 > > > > [ 350.425212] 8801680a78c8 81332187 > > > > 88016c5c5000 > > > > [ 350.433611] 0001 8801680a78f8 810ca6da > > > > 88016cc8b0f0 > > > > [ 350.442012] 88016cc8 88016cc8 880177ad > > > > 8801680a7948 > > > > [ 350.450409] Call Trace: > > > > [ 350.453165] [] dump_stack+0x67/0x90 > > > > [ 350.458931] [] lockdep_rcu_suspicious+0xea/0x120 > > > > [ 350.466002] [] fence_update+0xbd/0x670 [i915] > > > > [ 350.472766] [] i915_gem_restore_fences+0x52/0x70 > > > > [i915] > > > > [ 350.480496] [] vlv_resume_prepare+0x72/0x570 > > > > [i915] > > > > [ 350.487839] [] intel_runtime_resume+0x102/0x210 > > > > [i915] > > > > [ 350.495442] [] pci_pm_runtime_resume+0x7f/0xb0 > > > > [ 350.502274] [] ? > > > > pci_restore_standard_config+0x40/0x40 > > > > [ 350.509883] [] __rpm_callback+0x35/0x70 > > > > [ 350.516037] [] ? > > > > pci_restore_standard_config+0x40/0x40 > > > > [ 350.523646] [] rpm_callback+0x24/0x80 > > > > [ 350.529604] [] ? > > > > pci_restore_standard_config+0x40/0x40 > > > > [ 350.537212] [] rpm_resume+0x4ad/0x740 > > > > [ 350.543161] [] __pm_runtime_resume+0x51/0x80 > > > > [ 350.549824] [] intel_runtime_pm_get+0x28/0x90 > > > > [i915] > > > > [ 350.557265] [] intel_display_power_get+0x23/0x50 > > > > [i915] > > > > [ 350.565001] [] > > > > intel_atomic_commit_tail+0xdfd/0x10b0 [i915] > > > > [ 350.573106] [] ? > > > > drm_atomic_helper_swap_state+0x159/0x300 [drm_kms_helper] > > > > [ 350.582659] [] ? _raw_spin_unlock+0x31/0x50 > > > > [ 350.589205] [] ? > > > > drm_atomic_helper_swap_state+0x159/0x300 [drm_kms_helper] > > > > [ 350.598787] [] intel_atomic_commit+0x3b5/0x500 > > > > [i915] > > > > [ 350.606319] [] ? > > > > drm_atomic_set_crtc_for_connector+0xcc/0x100 [drm] > > > > [ 350.615209] [] drm_atomic_commit+0x49/0x50 [drm] > > > > [ 350.622242] [] > > > > drm_atomic_helper_set_config+0x88/0xc0 [drm_kms_helper] > > > > [ 350.631419] [] > > > > drm_mode_set_config_internal+0x6c/0x120 [drm] > > > > [ 350.639623] [] drm_mode_setcrtc+0x22c/0x4d0 [drm] > > > > [ 350.646760] [] drm_ioctl+0x209/0x460 [drm] > > > > [ 350.653217] [] ? drm_mode_getcrtc+0x150/0x150 > > > > [drm] > > > > [ 350.660536] [] ? __lock_is_held+0x4a/0x70 > > > > [ 350.666885] [] do_vfs_ioctl+0x93/0x6b0 > > > > [ 350.672939] [] ? __fget+0x113/0x200 > > > > [ 350.678797] [] ? __fget+0x5/0x200 > > > > [ 350.684361] [] SyS_ioctl+0x44/0x80 > > > > [ 350.690030] [] do_syscall_64+0x5b/0x120 > > > > [ 350.696184] [] entry_SYSCALL64_slow_path+0x25/0x25 > > > > > > > > Note we also have to remember the lesson from commit 4fc788f5ee3d > > > > ("drm/i915: Flush delayed fence releases after reset") where we have to > > > > flush any changes to the fence on restore. > > > > > > > > Fixes: 49ef5294cda2 ("drm/i915: Move fence tracking from object to vma") > > > > Reported-by: Ville Syrjälä > > > > Signed-off-by: Chris Wilson > > > > Cc: Ville Syrjälä > > > > Cc: Joonas Lahtinen > > > > Cc: Mi
[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Use fence_write() from rpm resume
== Series Details == Series: drm/i915: Use fence_write() from rpm resume URL : https://patchwork.freedesktop.org/series/13458/ State : warning == Summary == Series 13458v1 drm/i915: Use fence_write() from rpm resume https://patchwork.freedesktop.org/api/1.0/series/13458/revisions/1/mbox/ Test kms_pipe_crc_basic: Subgroup read-crc-pipe-b: pass -> DMESG-WARN (fi-ilk-650) Subgroup suspend-read-crc-pipe-b: dmesg-warn -> PASS (fi-byt-j1900) Test kms_psr_sink_crc: Subgroup psr_basic: dmesg-warn -> PASS (fi-skl-6700hq) Test vgem_basic: Subgroup unload: skip -> PASS (fi-byt-n2820) pass -> SKIP (fi-skl-6700hq) pass -> SKIP (fi-skl-6260u) pass -> SKIP (fi-ilk-650) pass -> SKIP (fi-bxt-t5700) fi-bdw-5557u total:248 pass:231 dwarn:0 dfail:0 fail:0 skip:17 fi-bsw-n3050 total:248 pass:204 dwarn:0 dfail:0 fail:0 skip:44 fi-bxt-t5700 total:248 pass:216 dwarn:0 dfail:0 fail:0 skip:32 fi-byt-j1900 total:248 pass:215 dwarn:0 dfail:0 fail:1 skip:32 fi-byt-n2820 total:248 pass:211 dwarn:0 dfail:0 fail:1 skip:36 fi-hsw-4770 total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-hsw-4770r total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-ilk-650 total:248 pass:183 dwarn:1 dfail:0 fail:2 skip:62 fi-ivb-3520m total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27 fi-ivb-3770 total:248 pass:207 dwarn:0 dfail:0 fail:0 skip:41 fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-6260u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-skl-6700hqtotal:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-skl-6700k total:248 pass:221 dwarn:1 dfail:0 fail:0 skip:26 fi-skl-6770hqtotal:248 pass:231 dwarn:1 dfail:0 fail:1 skip:15 fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37 fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39 Results at /archive/results/CI_IGT_test/Patchwork_2653/ f35ed31aea66b3230c366fcba5f3456ae2cb956e drm-intel-nightly: 2016y-10m-10d-11h-28m-51s UTC integration manifest caf5334 drm/i915: Use fence_write() from rpm resume ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/3] drm/i915/gtt: Split gen8_ppgtt_clear_pte_range
On pe, 2016-10-07 at 15:17 +0200, Michał Winiarski wrote: > Let's use more top-down approach, where each gen8_ppgtt_clear_* function > is responsible for clearing the struct passed as an argument and calling > relevant clear_range functions on lower-level tables. > Doing this rather than operating on PTE ranges makes the implementation > of shrinking page tables quite simple. > > v2: Drop min when calculating num_entries, no negation in 48b ppgtt > check, no newlines in vars block (Joonas) > > Cc: Chris Wilson > Cc: Joonas Lahtinen > Cc: Michel Thierry > Cc: Mika Kuoppala > Signed-off-by: Michał Winiarski Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/11] drm/etnaviv: Remove manual call to reservation_object_test_signaled_rcu before wait
Am Mittwoch, den 05.10.2016, 21:45 +0530 schrieb Sumit Semwal: > Hi Lucas, > > On 23 September 2016 at 18:25, Daniel Vetter wrote: > > On Mon, Aug 29, 2016 at 08:08:25AM +0100, Chris Wilson wrote: > >> Since fence_wait_timeout_reservation_object_wait_timeout_rcu() with a > >> timeout of 0 becomes reservation_object_test_signaled_rcu(), we do not > >> need to handle such conversion in the caller. The only challenge are > >> those callers that wish to differentiate the error code between the > >> nonblocking busy check and potentially blocking wait. > >> > >> Signed-off-by: Chris Wilson > >> Cc: Lucas Stach > >> Cc: Russell King > >> Cc: Christian Gmeiner > > > > Reviewed-by: Daniel Vetter > > > Could you please let me know if this is in your tree already, or would > you like me to take it via drm-misc (in which case, an Acked-by would > be fabulous!) > I haven't picked it up yet. If you are going to take the series through drm-misc feel free to add: Acked-by: Lucas Stach Regards, Lucas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt: Free unused lower-level page tables
On pe, 2016-10-07 at 20:39 +0200, Michał Winiarski wrote: > Since "Dynamic page table allocations" were introduced, our page tables > can grow (being dynamically allocated) with address space range usage. > Unfortunately, their lifetime is bound to vm. This is not a huge problem > when we're not using softpin - drm_mm is creating an upper bound on used > range by causing addresses for our VMAs to eventually be reused. > > With softpin, long lived contexts can drain the system out of memory > even with a single "small" object. For example: > > bo = bo_alloc(size); > while(true) > offset += size; > exec(bo, offset); > > Will cause us to create new allocations until all memory in the system > is used for tracking GPU pages (even though almost all PTEs in this vm > are pointing to scratch). > > Let's free unused page tables in clear_range to prevent this - if no > entries are used, we can safely free it and return this information to > the caller (so that higher-level entry is pointing to scratch). > > v2: Document return value and free semantics (Joonas) > v3: No newlines in vars block (Joonas) > > Cc: Chris Wilson > Cc: Joonas Lahtinen > Cc: Michel Thierry > Cc: Mika Kuoppala > Signed-off-by: Michał Winiarski Reviewed-by: Joonas Lahtinen Now that we're using Patchwork for CI, better always send the whole series so that testing catches it properly. Once the CI results are in and OK, can be merged. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Allow compaction upto SWIOTLB max segment size
On Mon, Oct 10, 2016 at 02:30:40PM +0100, Tvrtko Ursulin wrote: > > On 10/10/2016 12:49, Chris Wilson wrote: > >commit 1625e7e549c5 ("drm/i915: make compact dma scatter lists creation > >work with SWIOTLB backend") took a heavy handed approach to undo the > >scatterlist compaction in the face of SWIOTLB. (The compaction hit a bug > >whereby we tried to pass a segment larger than SWIOTLB could handle.) We > >can be a little more intelligent and try compacting the scatterlist up > >to the maximum SWIOTLB segment size (when using SWIOTLB). > > > >Signed-off-by: Chris Wilson > >CC: Imre Deak > >CC: Daniel Vetter > >Cc: Konrad Rzeszutek Wilk > >--- > > drivers/gpu/drm/i915/i915_gem.c | 23 +++ > > 1 file changed, 11 insertions(+), 12 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/i915_gem.c > >b/drivers/gpu/drm/i915/i915_gem.c > >index ca1a5a5c6f19..8b3474d215a5 100644 > >--- a/drivers/gpu/drm/i915/i915_gem.c > >+++ b/drivers/gpu/drm/i915/i915_gem.c > >@@ -2201,6 +2201,7 @@ i915_gem_object_get_pages_gtt(struct > >drm_i915_gem_object *obj) > > struct sgt_iter sgt_iter; > > struct page *page; > > unsigned long last_pfn = 0; /* suppress gcc warning */ > >+unsigned long max_segment; > > unsigned int would be enough. > Current maximum object size >> PAGE_SHIFT is 36 bits. We don't impose any other restriction that would limit a sg chunk. > > int ret; > > gfp_t gfp; > >@@ -2211,6 +2212,12 @@ i915_gem_object_get_pages_gtt(struct > >drm_i915_gem_object *obj) > > GEM_BUG_ON(obj->base.read_domains & I915_GEM_GPU_DOMAINS); > > GEM_BUG_ON(obj->base.write_domain & I915_GEM_GPU_DOMAINS); > >+max_segment = obj->base.size; > >+#ifdef CONFIG_SWIOTLB > >+if (swiotlb_nr_tbl()) > >+max_segment = IO_TLB_SEGSIZE << PAGE_SHIFT; > >+#endif > >+ > > Do you want to use IS_ENABLED here? The symbol swiotlb_nr_tbl() is absent unless SWIOTLB is enabled at compile time. So we need the cpp guard, or do you mean switch to #if IS_ENABLED(CONFIG_SWIOTLB) which we probably should indeed. > > st = kmalloc(sizeof(*st), GFP_KERNEL); > > if (st == NULL) > > return ERR_PTR(-ENOMEM); > >@@ -2252,15 +2259,9 @@ i915_gem_object_get_pages_gtt(struct > >drm_i915_gem_object *obj) > > goto err_pages; > > } > > } > >-#ifdef CONFIG_SWIOTLB > >-if (swiotlb_nr_tbl()) { > >-st->nents++; > >-sg_set_page(sg, page, PAGE_SIZE, 0); > >-sg = sg_next(sg); > >-continue; > >-} > >-#endif > >-if (!i || page_to_pfn(page) != last_pfn + 1) { > >+if (!i || > >+sg->length >= max_segment || > > I think this can overflow by a page, should be "sg->length >= > (max_segment - PAGE_SIZE)", or alternatively substract one page at > the max_segment assignment. We are looking at the previous sg, right? (and we only ever increment by PAGE_SIZE). So: when the previous sg reaches the maximum length, start a new sg element. Otherwise we extend the previous sg element by a PAGE, so on the else branch the maximum of sg->length after the increment is max_segment. > >+page_to_pfn(page) != last_pfn + 1) { > > if (i) > > sg = sg_next(sg); > > st->nents++; > >@@ -2273,9 +2274,7 @@ i915_gem_object_get_pages_gtt(struct > >drm_i915_gem_object *obj) > > /* Check that the i965g/gm workaround works. */ > > WARN_ON((gfp & __GFP_DMA32) && (last_pfn >= 0x0010UL)); > > } > >-#ifdef CONFIG_SWIOTLB > >-if (!swiotlb_nr_tbl()) > >-#endif > >+if (st->nents < st->orig_nents) > > sg_mark_end(sg); > > I wondered a few times that we could just terminate the table > unconditionally. The caveat being that if we do insert orig_nents, then sg at this point is NULL. Which is clearer: if (st->nents < st->orig_nents) sg_mark_end(sg); or if (sg) sg_mark_end(sg); /* coalesced sg table */ ? -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/2] drm/i915: Allow compaction upto SWIOTLB max segment size
On 10/10/2016 14:30, Tvrtko Ursulin wrote: On 10/10/2016 12:49, Chris Wilson wrote: commit 1625e7e549c5 ("drm/i915: make compact dma scatter lists creation work with SWIOTLB backend") took a heavy handed approach to undo the scatterlist compaction in the face of SWIOTLB. (The compaction hit a bug whereby we tried to pass a segment larger than SWIOTLB could handle.) We can be a little more intelligent and try compacting the scatterlist up to the maximum SWIOTLB segment size (when using SWIOTLB). Signed-off-by: Chris Wilson CC: Imre Deak CC: Daniel Vetter Cc: Konrad Rzeszutek Wilk --- drivers/gpu/drm/i915/i915_gem.c | 23 +++ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ca1a5a5c6f19..8b3474d215a5 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2201,6 +2201,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) struct sgt_iter sgt_iter; struct page *page; unsigned long last_pfn = 0;/* suppress gcc warning */ +unsigned long max_segment; unsigned int would be enough. int ret; gfp_t gfp; @@ -2211,6 +2212,12 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) GEM_BUG_ON(obj->base.read_domains & I915_GEM_GPU_DOMAINS); GEM_BUG_ON(obj->base.write_domain & I915_GEM_GPU_DOMAINS); +max_segment = obj->base.size; +#ifdef CONFIG_SWIOTLB +if (swiotlb_nr_tbl()) +max_segment = IO_TLB_SEGSIZE << PAGE_SHIFT; +#endif + Do you want to use IS_ENABLED here? st = kmalloc(sizeof(*st), GFP_KERNEL); if (st == NULL) return ERR_PTR(-ENOMEM); @@ -2252,15 +2259,9 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) goto err_pages; } } -#ifdef CONFIG_SWIOTLB -if (swiotlb_nr_tbl()) { -st->nents++; -sg_set_page(sg, page, PAGE_SIZE, 0); -sg = sg_next(sg); -continue; -} -#endif -if (!i || page_to_pfn(page) != last_pfn + 1) { +if (!i || +sg->length >= max_segment || I think this can overflow by a page, should be "sg->length >= (max_segment - PAGE_SIZE)", or alternatively substract one page at the max_segment assignment. Or not. :) Regards. Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915/gen9: fix watermarks when using the pipe scaler
== Series Details == Series: series starting with [1/2] drm/i915/gen9: fix watermarks when using the pipe scaler URL : https://patchwork.freedesktop.org/series/13465/ State : warning == Summary == Series 13465v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/13465/revisions/1/mbox/ Test kms_psr_sink_crc: Subgroup psr_basic: dmesg-warn -> PASS (fi-skl-6700hq) Test vgem_basic: Subgroup unload: pass -> SKIP (fi-ilk-650) pass -> SKIP (fi-skl-6700hq) skip -> PASS (fi-bdw-5557u) pass -> SKIP (fi-skl-6260u) skip -> PASS (fi-byt-n2820) skip -> PASS (fi-skl-6700k) fi-bdw-5557u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-bsw-n3050 total:248 pass:204 dwarn:0 dfail:0 fail:0 skip:44 fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31 fi-byt-j1900 total:248 pass:214 dwarn:1 dfail:0 fail:1 skip:32 fi-byt-n2820 total:248 pass:211 dwarn:0 dfail:0 fail:1 skip:36 fi-hsw-4770 total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-hsw-4770r total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-ilk-650 total:248 pass:184 dwarn:0 dfail:0 fail:2 skip:62 fi-ivb-3520m total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27 fi-ivb-3770 total:248 pass:207 dwarn:0 dfail:0 fail:0 skip:41 fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-6260u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-skl-6700hqtotal:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-skl-6700k total:248 pass:222 dwarn:1 dfail:0 fail:0 skip:25 fi-skl-6770hqtotal:248 pass:231 dwarn:1 dfail:0 fail:1 skip:15 fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37 fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39 Results at /archive/results/CI_IGT_test/Patchwork_2655/ f35ed31aea66b3230c366fcba5f3456ae2cb956e drm-intel-nightly: 2016y-10m-10d-11h-28m-51s UTC integration manifest 1cee613 drm/i915/gen9: don't call ilk_pipe_pixel_rate() twice on the same function 3fa3b0b drm/i915/gen9: fix watermarks when using the pipe scaler ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] tools/intel_guc_logger: Utility for capturing GuC firmware logs in a file
On 10/10/2016 11:59, akash.g...@intel.com wrote: From: Akash Goel This patch provides a test utility which helps capture GuC firmware logs and then dump them to file. The logs are pulled from a debugfs file '/sys/kernel/debug/dri/guc_log' and by default stored into a file 'guc_log_dump.dat'. The name, including the location, of the output file can be changed through a command line argument. The utility goes into an infinite loop where it waits for the arrival of new logs and as soon as new set of logs are produced it captures them in its local buffer which is then flushed out to the file on disk. Any time when logging needs to be ended, User can stop this utility (CTRL+C). Before entering into a loop, it first discards whatever logs are present in the debugfs file. This way User can first launch this utility and then start a workload/activity for which GuC firmware logs are to be actually captured and keep running the utility for as long as its needed, like once the workload is over this utility can be forcefully stopped. If the logging wasn't enabled on GuC side by the Driver at boot time, utility will first enable the logging and later on when it is stopped (CTRL+C) it will also pause the logging on GuC side. v2: - Use combination of alarm system call & SIGALRM signal to run the utility for required duration. (Tvrtko) - Fix inconsistencies, do minor cleanup and refactoring. (Tvrtko) v3: - Fix discrepancy for the output file command line option and update the Usage/help string. v4: - Update the exit condition for flusher thread, now will exit only after the capture loop is over and not when the flag to stop logging is set. This handles a corner case, due to which the dump of last captured buffer was getting missed. - Add a newline character at the end of assert messages. - Avoid the assert for the case, which occurs very rarely, when there are no bytes read from the relay file. Cc: Tvrtko Ursulin Signed-off-by: Akash Goel Reviewed-by: Tvrtko Ursulin (v3) --- tools/Makefile.sources | 1 + tools/intel_guc_logger.c | 438 +++ 2 files changed, 439 insertions(+) create mode 100644 tools/intel_guc_logger.c diff --git a/tools/Makefile.sources b/tools/Makefile.sources index 2bb6c8e..be58871 100644 --- a/tools/Makefile.sources +++ b/tools/Makefile.sources @@ -19,6 +19,7 @@ tools_prog_lists =\ intel_gpu_time \ intel_gpu_top \ intel_gtt \ + intel_guc_logger\ intel_infoframes\ intel_l3_parity \ intel_lid \ diff --git a/tools/intel_guc_logger.c b/tools/intel_guc_logger.c new file mode 100644 index 000..159a54e --- /dev/null +++ b/tools/intel_guc_logger.c @@ -0,0 +1,438 @@ + +#define _GNU_SOURCE /* For using O_DIRECT */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "igt.h" + +#define MB(x) ((uint64_t)(x) * 1024 * 1024) +#ifndef PAGE_SIZE + #define PAGE_SIZE 4096 +#endif +/* Currently the size of GuC log buffer is 19 pages & so is the size of relay + * subbuffer. If the size changes in future, then this define also needs to be + * updated accordingly. + */ +#define SUBBUF_SIZE (19*PAGE_SIZE) +/* Need large buffering from logger side to hide the DISK IO latency, Driver + * can only store 8 snapshots of GuC log buffer in relay. + */ +#define NUM_SUBBUFS 100 + +#define RELAY_FILE_NAME "guc_log" +#define DEFAULT_OUTPUT_FILE_NAME "guc_log_dump.dat" +#define CONTROL_FILE_NAME "i915_guc_log_control" + +char *read_buffer; +char *out_filename; +int poll_timeout = 2; /* by default 2ms timeout */ +pthread_mutex_t mutex; +pthread_t flush_thread; +int verbosity_level = 3; /* by default capture logs at max verbosity */ +uint32_t produced, consumed; +uint64_t total_bytes_written; +int num_buffers = NUM_SUBBUFS; +int relay_fd, outfile_fd = -1; +uint32_t test_duration, max_filesize; +pthread_cond_t underflow_cond, overflow_cond; +bool stop_logging, discard_oldlogs, capturing_stopped; + +static void guc_log_control(bool enable_logging) +{ + int control_fd; + char data[19]; + uint64_t val; + int ret; + + control_fd = igt_debugfs_open(CONTROL_FILE_NAME, O_WRONLY); + igt_assert_f(control_fd >= 0, "couldn't open the guc log control file\n"); + + val = enable_logging ? ((verbosity_level << 4) | 0x1) : 0; + + ret = snprintf(data, sizeof(data), "0x%" PRIx64, val); + igt_assert(ret > 2 && ret < sizeof(data)); + + ret = write(control_fd, data, ret); + igt_assert_f(ret > 0, "couldn't write to the log control file\n"); + + close(control_fd); +} + +static void int_sig_handler(int sig) +{ + igt_info("received signal %d\n", sig); + + stop_logging = true; +} + +static void pull_leftover_dat
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Allow compaction upto SWIOTLB max segment size
On 10/10/2016 14:43, Chris Wilson wrote: On Mon, Oct 10, 2016 at 02:30:40PM +0100, Tvrtko Ursulin wrote: On 10/10/2016 12:49, Chris Wilson wrote: commit 1625e7e549c5 ("drm/i915: make compact dma scatter lists creation work with SWIOTLB backend") took a heavy handed approach to undo the scatterlist compaction in the face of SWIOTLB. (The compaction hit a bug whereby we tried to pass a segment larger than SWIOTLB could handle.) We can be a little more intelligent and try compacting the scatterlist up to the maximum SWIOTLB segment size (when using SWIOTLB). Signed-off-by: Chris Wilson CC: Imre Deak CC: Daniel Vetter Cc: Konrad Rzeszutek Wilk --- drivers/gpu/drm/i915/i915_gem.c | 23 +++ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ca1a5a5c6f19..8b3474d215a5 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2201,6 +2201,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) struct sgt_iter sgt_iter; struct page *page; unsigned long last_pfn = 0; /* suppress gcc warning */ + unsigned long max_segment; unsigned int would be enough. Current maximum object size >> PAGE_SHIFT is 36 bits. We don't impose any other restriction that would limit a sg chunk. My bad, for some reason I thought it is used only under the CONFIG_SWIOTLB guard. int ret; gfp_t gfp; @@ -2211,6 +2212,12 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) GEM_BUG_ON(obj->base.read_domains & I915_GEM_GPU_DOMAINS); GEM_BUG_ON(obj->base.write_domain & I915_GEM_GPU_DOMAINS); + max_segment = obj->base.size; +#ifdef CONFIG_SWIOTLB + if (swiotlb_nr_tbl()) + max_segment = IO_TLB_SEGSIZE << PAGE_SHIFT; +#endif + Do you want to use IS_ENABLED here? The symbol swiotlb_nr_tbl() is absent unless SWIOTLB is enabled at compile time. So we need the cpp guard, or do you mean switch to #if IS_ENABLED(CONFIG_SWIOTLB) which we probably should indeed. Ok, doesn't matter then. st = kmalloc(sizeof(*st), GFP_KERNEL); if (st == NULL) return ERR_PTR(-ENOMEM); @@ -2252,15 +2259,9 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) goto err_pages; } } -#ifdef CONFIG_SWIOTLB - if (swiotlb_nr_tbl()) { - st->nents++; - sg_set_page(sg, page, PAGE_SIZE, 0); - sg = sg_next(sg); - continue; - } -#endif - if (!i || page_to_pfn(page) != last_pfn + 1) { + if (!i || + sg->length >= max_segment || I think this can overflow by a page, should be "sg->length >= (max_segment - PAGE_SIZE)", or alternatively substract one page at the max_segment assignment. We are looking at the previous sg, right? (and we only ever increment by PAGE_SIZE). So: when the previous sg reaches the maximum length, start a new sg element. Otherwise we extend the previous sg element by a PAGE, so on the else branch the maximum of sg->length after the increment is max_segment. Yes I've corrected myself shortly after posting. + page_to_pfn(page) != last_pfn + 1) { if (i) sg = sg_next(sg); st->nents++; @@ -2273,9 +2274,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) /* Check that the i965g/gm workaround works. */ WARN_ON((gfp & __GFP_DMA32) && (last_pfn >= 0x0010UL)); } -#ifdef CONFIG_SWIOTLB - if (!swiotlb_nr_tbl()) -#endif + if (st->nents < st->orig_nents) sg_mark_end(sg); I wondered a few times that we could just terminate the table unconditionally. The caveat being that if we do insert orig_nents, then sg at this point is NULL. Which is clearer: if (st->nents < st->orig_nents) sg_mark_end(sg); or if (sg) sg_mark_end(sg); /* coalesced sg table */ ? I missed that as well (that it can be NULL here). Sorry for the noise then. "if (sg)" looks somewhat better to me FWIW. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for Link Training Compliance support, link rate fallback sending Uevent (rev2)
== Series Details == Series: Link Training Compliance support, link rate fallback sending Uevent (rev2) URL : https://patchwork.freedesktop.org/series/13468/ State : warning == Summary == Series 13468v2 Link Training Compliance support, link rate fallback sending Uevent https://patchwork.freedesktop.org/api/1.0/series/13468/revisions/2/mbox/ Test drv_module_reload_basic: pass -> SKIP (fi-ivb-3520m) Test kms_pipe_crc_basic: Subgroup read-crc-pipe-a-frame-sequence: pass -> DMESG-WARN (fi-skl-6770hq) Test kms_psr_sink_crc: Subgroup psr_basic: dmesg-warn -> PASS (fi-skl-6700hq) Test vgem_basic: Subgroup unload: skip -> PASS (fi-byt-n2820) pass -> SKIP (fi-ilk-650) skip -> PASS (fi-bsw-n3050) pass -> SKIP (fi-skl-6700hq) skip -> PASS (fi-bdw-5557u) pass -> SKIP (fi-skl-6260u) fi-bdw-5557u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-bsw-n3050 total:248 pass:205 dwarn:0 dfail:0 fail:0 skip:43 fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31 fi-byt-j1900 total:248 pass:214 dwarn:1 dfail:0 fail:1 skip:32 fi-byt-n2820 total:248 pass:211 dwarn:0 dfail:0 fail:1 skip:36 fi-hsw-4770 total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-hsw-4770r total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-ilk-650 total:248 pass:184 dwarn:0 dfail:0 fail:2 skip:62 fi-ivb-3520m total:248 pass:220 dwarn:0 dfail:0 fail:0 skip:28 fi-ivb-3770 total:248 pass:207 dwarn:0 dfail:0 fail:0 skip:41 fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-6260u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-skl-6700hqtotal:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-skl-6700k total:248 pass:221 dwarn:1 dfail:0 fail:0 skip:26 fi-skl-6770hqtotal:248 pass:230 dwarn:2 dfail:0 fail:1 skip:15 fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37 fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39 Results at /archive/results/CI_IGT_test/Patchwork_2656/ f35ed31aea66b3230c366fcba5f3456ae2cb956e drm-intel-nightly: 2016y-10m-10d-11h-28m-51s UTC integration manifest ab8641d drm/i915: Link Rate fallback on Link training failure bf2b27e drm/i915: Work queue for uevent 1498096 drm/i915: Add support for DP link training compliance 49563cd drm/i915; Add a function to return index of link rate 9e268a4 drm/i915: Change the placement of some static functions in intel_dp.c ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] tools/intel_guc_logger: Utility for capturing GuC firmware logs in a file
On 10/10/2016 7:22 PM, Tvrtko Ursulin wrote: On 10/10/2016 11:59, akash.g...@intel.com wrote: From: Akash Goel This patch provides a test utility which helps capture GuC firmware logs and then dump them to file. The logs are pulled from a debugfs file '/sys/kernel/debug/dri/guc_log' and by default stored into a file 'guc_log_dump.dat'. The name, including the location, of the output file can be changed through a command line argument. The utility goes into an infinite loop where it waits for the arrival of new logs and as soon as new set of logs are produced it captures them in its local buffer which is then flushed out to the file on disk. Any time when logging needs to be ended, User can stop this utility (CTRL+C). Before entering into a loop, it first discards whatever logs are present in the debugfs file. This way User can first launch this utility and then start a workload/activity for which GuC firmware logs are to be actually captured and keep running the utility for as long as its needed, like once the workload is over this utility can be forcefully stopped. If the logging wasn't enabled on GuC side by the Driver at boot time, utility will first enable the logging and later on when it is stopped (CTRL+C) it will also pause the logging on GuC side. v2: - Use combination of alarm system call & SIGALRM signal to run the utility for required duration. (Tvrtko) - Fix inconsistencies, do minor cleanup and refactoring. (Tvrtko) v3: - Fix discrepancy for the output file command line option and update the Usage/help string. v4: - Update the exit condition for flusher thread, now will exit only after the capture loop is over and not when the flag to stop logging is set. This handles a corner case, due to which the dump of last captured buffer was getting missed. - Add a newline character at the end of assert messages. - Avoid the assert for the case, which occurs very rarely, when there are no bytes read from the relay file. Cc: Tvrtko Ursulin Signed-off-by: Akash Goel Reviewed-by: Tvrtko Ursulin (v3) --- tools/Makefile.sources | 1 + tools/intel_guc_logger.c | 438 +++ 2 files changed, 439 insertions(+) create mode 100644 tools/intel_guc_logger.c diff --git a/tools/Makefile.sources b/tools/Makefile.sources index 2bb6c8e..be58871 100644 --- a/tools/Makefile.sources +++ b/tools/Makefile.sources @@ -19,6 +19,7 @@ tools_prog_lists =\ intel_gpu_time\ intel_gpu_top\ intel_gtt\ +intel_guc_logger\ intel_infoframes\ intel_l3_parity\ intel_lid\ diff --git a/tools/intel_guc_logger.c b/tools/intel_guc_logger.c new file mode 100644 index 000..159a54e --- /dev/null +++ b/tools/intel_guc_logger.c @@ -0,0 +1,438 @@ + +#define _GNU_SOURCE /* For using O_DIRECT */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "igt.h" + +#define MB(x) ((uint64_t)(x) * 1024 * 1024) +#ifndef PAGE_SIZE + #define PAGE_SIZE 4096 +#endif +/* Currently the size of GuC log buffer is 19 pages & so is the size of relay + * subbuffer. If the size changes in future, then this define also needs to be + * updated accordingly. + */ +#define SUBBUF_SIZE (19*PAGE_SIZE) +/* Need large buffering from logger side to hide the DISK IO latency, Driver + * can only store 8 snapshots of GuC log buffer in relay. + */ +#define NUM_SUBBUFS 100 + +#define RELAY_FILE_NAME "guc_log" +#define DEFAULT_OUTPUT_FILE_NAME "guc_log_dump.dat" +#define CONTROL_FILE_NAME "i915_guc_log_control" + +char *read_buffer; +char *out_filename; +int poll_timeout = 2; /* by default 2ms timeout */ +pthread_mutex_t mutex; +pthread_t flush_thread; +int verbosity_level = 3; /* by default capture logs at max verbosity */ +uint32_t produced, consumed; +uint64_t total_bytes_written; +int num_buffers = NUM_SUBBUFS; +int relay_fd, outfile_fd = -1; +uint32_t test_duration, max_filesize; +pthread_cond_t underflow_cond, overflow_cond; +bool stop_logging, discard_oldlogs, capturing_stopped; + +static void guc_log_control(bool enable_logging) +{ +int control_fd; +char data[19]; +uint64_t val; +int ret; + +control_fd = igt_debugfs_open(CONTROL_FILE_NAME, O_WRONLY); +igt_assert_f(control_fd >= 0, "couldn't open the guc log control file\n"); + +val = enable_logging ? ((verbosity_level << 4) | 0x1) : 0; + +ret = snprintf(data, sizeof(data), "0x%" PRIx64, val); +igt_assert(ret > 2 && ret < sizeof(data)); + +ret = write(control_fd, data, ret); +igt_assert_f(ret > 0, "couldn't write to the log control file\n"); + +close(control_fd); +} + +static void int_sig_handler(int sig) +{ +igt_info("received signal %d\n", sig); + +stop_logging = true; +} + +static void pull_leftover_data(void) +{ +unsigned int bytes_rea
Re: [Intel-gfx] [PATCH] drm/i915: Fix silent conflict from backmerge of drm-next into dinf
On Mon, 10 Oct 2016, Jani Nikula wrote: > On Mon, 10 Oct 2016, Chris Wilson wrote: >> Merging from drm-next pulled back in a few lines of dead code due to the >> code movement around i915_gem_reset(), fix that up. > > Actually the problem was introduced in > > commit ca09fb9f60b5f3ab2d57e761aaeea89a5147d784 > Merge: 9f4ef05bcdcf 08895a8b6b06 > Author: Dave Airlie > Date: Wed Sep 28 12:08:49 2016 +1000 > > Merge tag 'v4.8-rc8' into drm-next > > and it's present in drm-next, and the conflict wasn't silent. > > If this is the right fix (Daniel ack?) I'll queue it with other drm-next > fixes that I have. And pushed to drm-intel-next-fixes *only*, with an updated commit message. BR, Jani. > > > BR, > Jani. > > >> >> Signed-off-by: Chris Wilson >> Cc: Jani Nikula >> Cc: Daniel Vetter >> --- >> drivers/gpu/drm/i915/i915_gem.c | 3 --- >> 1 file changed, 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c >> b/drivers/gpu/drm/i915/i915_gem.c >> index 981a106901ae..1418c1c522cb 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -2616,8 +2616,6 @@ static void i915_gem_reset_engine(struct >> intel_engine_cs *engine) >> list_for_each_entry_continue(request, &engine->request_list, link) >> if (request->ctx == incomplete_ctx) >> reset_request(request); >> - >> -engine->i915->gt.active_engines &= ~intel_engine_flag(engine); >> } >> >> void i915_gem_reset(struct drm_i915_private *dev_priv) >> @@ -2628,7 +2626,6 @@ void i915_gem_reset(struct drm_i915_private *dev_priv) >> >> for_each_engine(engine, dev_priv) >> i915_gem_reset_engine(engine); >> -mod_delayed_work(dev_priv->wq, &dev_priv->gt.idle_work, 0); >> >> i915_gem_restore_fences(&dev_priv->drm); -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH RESEND 5/9] drm/i915/audio: set proper N/MCTS on more platforms
From: Libin Yang This patch applies setting proper N/M, N/CTS on more platforms. Reviewed-by: Ville Syrjälä Signed-off-by: Libin Yang Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_audio.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index db7fd6d8333f..bce3ad2eb86d 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -688,11 +688,7 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port, struct i915_audio_component *acomp = dev_priv->audio_component; int err = 0; - /* HSW, BDW, SKL, KBL need this fix */ - if (!IS_SKYLAKE(dev_priv) && - !IS_KABYLAKE(dev_priv) && - !IS_BROADWELL(dev_priv) && - !IS_HASWELL(dev_priv)) + if (!HAS_DDI(dev_priv)) return 0; i915_audio_component_get_power(kdev); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH RESEND 3/9] drm/i915/audio: use the same code for updating audio config
It gets fragile to duplicate the code for updating HSW_AUD_CFG. The only change should be that the hdmi pixel clock is also updated in i915_audio_component_sync_audio_rate(), but it should not be any different. Cc: Libin Yang Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_audio.c | 29 +++-- 1 file changed, 3 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 5d0bd07afa51..4d62b3e8ac19 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -671,10 +671,8 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port, struct drm_i915_private *dev_priv = kdev_to_i915(kdev); struct intel_encoder *intel_encoder; struct intel_crtc *crtc; - struct drm_display_mode *mode; + struct drm_display_mode *adjusted_mode; struct i915_audio_component *acomp = dev_priv->audio_component; - u32 tmp; - int n; int err = 0; /* HSW, BDW, SKL, KBL need this fix */ @@ -700,33 +698,12 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port, crtc = to_intel_crtc(intel_encoder->base.crtc); pipe = crtc->pipe; - mode = &crtc->config->base.adjusted_mode; + adjusted_mode = &crtc->config->base.adjusted_mode; /* port must be valid now, otherwise the pipe will be invalid */ acomp->aud_sample_rate[port] = rate; - /* 2. check whether to set the N/CTS/M manually or not */ - if (!audio_rate_need_prog(crtc, mode)) { - tmp = I915_READ(HSW_AUD_CFG(pipe)); - tmp &= ~AUD_CONFIG_N_PROG_ENABLE; - I915_WRITE(HSW_AUD_CFG(pipe), tmp); - goto unlock; - } - - n = audio_config_get_n(mode, rate); - if (n == 0) { - DRM_DEBUG_KMS("Using automatic mode for N value on port %c\n", - port_name(port)); - tmp = I915_READ(HSW_AUD_CFG(pipe)); - tmp &= ~AUD_CONFIG_N_PROG_ENABLE; - I915_WRITE(HSW_AUD_CFG(pipe), tmp); - goto unlock; - } - - /* 3. set the N/CTS/M */ - tmp = I915_READ(HSW_AUD_CFG(pipe)); - tmp = audio_config_setup_n_reg(n, tmp); - I915_WRITE(HSW_AUD_CFG(pipe), tmp); + hsw_audio_config_update(crtc, port, adjusted_mode); unlock: mutex_unlock(&dev_priv->av_mutex); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH RESEND 1/9] drm/i915/audio: abstract audio config update
Prepare for using the same code for updating HSW_AUD_CFG register. No functional changes. Cc: Libin Yang Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_audio.c | 68 ++ 1 file changed, 40 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 9583f432e02e..0a54f7cdce37 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -245,6 +245,45 @@ static void g4x_audio_codec_enable(struct drm_connector *connector, I915_WRITE(G4X_AUD_CNTL_ST, tmp); } +static void hsw_audio_config_update(struct intel_crtc *intel_crtc, + enum port port, + const struct drm_display_mode *adjusted_mode) +{ + struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); + struct i915_audio_component *acomp = dev_priv->audio_component; + enum pipe pipe = intel_crtc->pipe; + int n, rate; + u32 tmp; + + tmp = I915_READ(HSW_AUD_CFG(pipe)); + tmp &= ~AUD_CONFIG_N_VALUE_INDEX; + tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; + if (intel_crtc_has_dp_encoder(intel_crtc->config)) + tmp |= AUD_CONFIG_N_VALUE_INDEX; + else + tmp |= audio_config_hdmi_pixel_clock(adjusted_mode); + + tmp &= ~AUD_CONFIG_N_PROG_ENABLE; + if (audio_rate_need_prog(intel_crtc, adjusted_mode)) { + if (!acomp) + rate = 0; + else if (port >= PORT_A && port <= PORT_E) + rate = acomp->aud_sample_rate[port]; + else { + DRM_ERROR("invalid port: %d\n", port); + rate = 0; + } + + n = audio_config_get_n(adjusted_mode, rate); + if (n != 0) + tmp = audio_config_setup_n_reg(n, tmp); + else + DRM_DEBUG_KMS("no suitable N value is found\n"); + } + + I915_WRITE(HSW_AUD_CFG(pipe), tmp); +} + static void hsw_audio_codec_disable(struct intel_encoder *encoder) { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); @@ -283,11 +322,9 @@ static void hsw_audio_codec_enable(struct drm_connector *connector, struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc); enum pipe pipe = intel_crtc->pipe; enum port port = intel_encoder->port; - struct i915_audio_component *acomp = dev_priv->audio_component; const uint8_t *eld = connector->eld; uint32_t tmp; int len, i; - int n, rate; DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n", pipe_name(pipe), drm_eld_size(eld)); @@ -323,32 +360,7 @@ static void hsw_audio_codec_enable(struct drm_connector *connector, I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp); /* Enable timestamps */ - tmp = I915_READ(HSW_AUD_CFG(pipe)); - tmp &= ~AUD_CONFIG_N_VALUE_INDEX; - tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; - if (intel_crtc_has_dp_encoder(intel_crtc->config)) - tmp |= AUD_CONFIG_N_VALUE_INDEX; - else - tmp |= audio_config_hdmi_pixel_clock(adjusted_mode); - - tmp &= ~AUD_CONFIG_N_PROG_ENABLE; - if (audio_rate_need_prog(intel_crtc, adjusted_mode)) { - if (!acomp) - rate = 0; - else if (port >= PORT_A && port <= PORT_E) - rate = acomp->aud_sample_rate[port]; - else { - DRM_ERROR("invalid port: %d\n", port); - rate = 0; - } - n = audio_config_get_n(adjusted_mode, rate); - if (n != 0) - tmp = audio_config_setup_n_reg(n, tmp); - else - DRM_DEBUG_KMS("no suitable N value is found\n"); - } - - I915_WRITE(HSW_AUD_CFG(pipe), tmp); + hsw_audio_config_update(intel_crtc, port, adjusted_mode); mutex_unlock(&dev_priv->av_mutex); } -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH RESEND 0/9] drm/i915/audio: audio cleanups, 4k fixes
Resend of [1] due to no review. Patches 1-8 should be easy stuff to review. *wink*. BR, Jani. [1] https://patchwork.freedesktop.org/series/12754/ Jani Nikula (7): drm/i915/audio: abstract audio config update drm/i915/audio: port is going to be just fine, simplify checks drm/i915/audio: use the same code for updating audio config drm/i915/audio: split dp and hdmi audio config update drm/i915/audio: add register macros for audio config N value drm/i915/audio: rename N value getter to emphasize it's for hdmi drm/i915: set proper N/M in modeset Libin Yang (2): drm/i915/audio: set proper N/MCTS on more platforms drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock drivers/gpu/drm/i915/i915_reg.h| 11 ++ drivers/gpu/drm/i915/intel_audio.c | 260 - 2 files changed, 179 insertions(+), 92 deletions(-) -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH RESEND 4/9] drm/i915/audio: split dp and hdmi audio config update
The code for dp and hdmi are already different, and they're about to diverge even more. Split them for clarity in future work. No functional changes. Cc: Libin Yang Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_audio.c | 55 +++--- 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 4d62b3e8ac19..db7fd6d8333f 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -148,18 +148,6 @@ static uint32_t audio_config_setup_n_reg(int n, uint32_t val) return tmp; } -/* check whether N/CTS/M need be set manually */ -static bool audio_rate_need_prog(struct intel_crtc *crtc, -const struct drm_display_mode *mode) -{ - if (((mode->clock == TMDS_297M) || -(mode->clock == TMDS_296M)) && - intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI)) - return true; - else - return false; -} - static bool intel_eld_uptodate(struct drm_connector *connector, i915_reg_t reg_eldv, uint32_t bits_eldv, i915_reg_t reg_elda, uint32_t bits_elda, @@ -245,9 +233,26 @@ static void g4x_audio_codec_enable(struct drm_connector *connector, I915_WRITE(G4X_AUD_CNTL_ST, tmp); } -static void hsw_audio_config_update(struct intel_crtc *intel_crtc, - enum port port, - const struct drm_display_mode *adjusted_mode) +static void +hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port port, + const struct drm_display_mode *adjusted_mode) +{ + struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); + enum pipe pipe = intel_crtc->pipe; + u32 tmp; + + tmp = I915_READ(HSW_AUD_CFG(pipe)); + tmp &= ~AUD_CONFIG_N_VALUE_INDEX; + tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; + tmp &= ~AUD_CONFIG_N_PROG_ENABLE; + tmp |= AUD_CONFIG_N_VALUE_INDEX; + + I915_WRITE(HSW_AUD_CFG(pipe), tmp); +} + +static void +hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, enum port port, +const struct drm_display_mode *adjusted_mode) { struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); struct i915_audio_component *acomp = dev_priv->audio_component; @@ -259,13 +264,11 @@ static void hsw_audio_config_update(struct intel_crtc *intel_crtc, tmp = I915_READ(HSW_AUD_CFG(pipe)); tmp &= ~AUD_CONFIG_N_VALUE_INDEX; tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; - if (intel_crtc_has_dp_encoder(intel_crtc->config)) - tmp |= AUD_CONFIG_N_VALUE_INDEX; - else - tmp |= audio_config_hdmi_pixel_clock(adjusted_mode); - tmp &= ~AUD_CONFIG_N_PROG_ENABLE; - if (audio_rate_need_prog(intel_crtc, adjusted_mode)) { + tmp |= audio_config_hdmi_pixel_clock(adjusted_mode); + + if (adjusted_mode->clock == TMDS_296M || + adjusted_mode->clock == TMDS_297M) { n = audio_config_get_n(adjusted_mode, rate); if (n != 0) tmp = audio_config_setup_n_reg(n, tmp); @@ -276,6 +279,16 @@ static void hsw_audio_config_update(struct intel_crtc *intel_crtc, I915_WRITE(HSW_AUD_CFG(pipe), tmp); } +static void +hsw_audio_config_update(struct intel_crtc *intel_crtc, enum port port, + const struct drm_display_mode *adjusted_mode) +{ + if (intel_crtc_has_dp_encoder(intel_crtc->config)) + hsw_dp_audio_config_update(intel_crtc, port, adjusted_mode); + else + hsw_hdmi_audio_config_update(intel_crtc, port, adjusted_mode); +} + static void hsw_audio_codec_disable(struct intel_encoder *encoder) { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH RESEND 7/9] drm/i915/audio: add register macros for audio config N value
Have generic macros in line with the rest of the register bit definition macros instead of a dedicated function in intel_audio.c, and use them. No functional changes. Cc: Libin Yang Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_reg.h| 4 drivers/gpu/drm/i915/intel_audio.c | 23 ++- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index acc767a52d8e..595d196f753f 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7332,6 +7332,10 @@ enum { #define AUD_CONFIG_UPPER_N_MASK (0xff << 20) #define AUD_CONFIG_LOWER_N_SHIFT 4 #define AUD_CONFIG_LOWER_N_MASK (0xfff << 4) +#define AUD_CONFIG_N_MASK(AUD_CONFIG_UPPER_N_MASK | AUD_CONFIG_LOWER_N_MASK) +#define AUD_CONFIG_N(n) \ + (n) >> 12) & 0xff) << AUD_CONFIG_UPPER_N_SHIFT) | \ +(((n) & 0xfff) << AUD_CONFIG_LOWER_N_SHIFT)) #define AUD_CONFIG_PIXEL_CLOCK_HDMI_SHIFT16 #define AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK (0xf << 16) #define AUD_CONFIG_PIXEL_CLOCK_HDMI_25175(0 << 16) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 6aa619a84439..d2c6227f72b8 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -135,20 +135,6 @@ static int audio_config_get_n(const struct drm_display_mode *adjusted_mode, return 0; } -static uint32_t audio_config_setup_n_reg(int n, uint32_t val) -{ - int n_low, n_up; - uint32_t tmp = val; - - n_low = n & 0xfff; - n_up = (n >> 12) & 0xff; - tmp &= ~(AUD_CONFIG_UPPER_N_MASK | AUD_CONFIG_LOWER_N_MASK); - tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) | - (n_low << AUD_CONFIG_LOWER_N_SHIFT) | - AUD_CONFIG_N_PROG_ENABLE); - return tmp; -} - static bool intel_eld_uptodate(struct drm_connector *connector, i915_reg_t reg_eldv, uint32_t bits_eldv, i915_reg_t reg_elda, uint32_t bits_elda, @@ -271,10 +257,13 @@ hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, enum port port, if (adjusted_mode->crtc_clock == TMDS_296M || adjusted_mode->crtc_clock == TMDS_297M) { n = audio_config_get_n(adjusted_mode, rate); - if (n != 0) - tmp = audio_config_setup_n_reg(n, tmp); - else + if (n != 0) { + tmp &= ~AUD_CONFIG_N_MASK; + tmp |= AUD_CONFIG_N(n); + tmp |= AUD_CONFIG_N_PROG_ENABLE; + } else { DRM_DEBUG_KMS("no suitable N value is found\n"); + } } I915_WRITE(HSW_AUD_CFG(pipe), tmp); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH RESEND 2/9] drm/i915/audio: port is going to be just fine, simplify checks
If it was wrong, we'd be screwed already. Cc: Libin Yang Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_audio.c | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 0a54f7cdce37..5d0bd07afa51 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -251,8 +251,9 @@ static void hsw_audio_config_update(struct intel_crtc *intel_crtc, { struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); struct i915_audio_component *acomp = dev_priv->audio_component; + int rate = acomp ? acomp->aud_sample_rate[port] : 0; enum pipe pipe = intel_crtc->pipe; - int n, rate; + int n; u32 tmp; tmp = I915_READ(HSW_AUD_CFG(pipe)); @@ -265,15 +266,6 @@ static void hsw_audio_config_update(struct intel_crtc *intel_crtc, tmp &= ~AUD_CONFIG_N_PROG_ENABLE; if (audio_rate_need_prog(intel_crtc, adjusted_mode)) { - if (!acomp) - rate = 0; - else if (port >= PORT_A && port <= PORT_E) - rate = acomp->aud_sample_rate[port]; - else { - DRM_ERROR("invalid port: %d\n", port); - rate = 0; - } - n = audio_config_get_n(adjusted_mode, rate); if (n != 0) tmp = audio_config_setup_n_reg(n, tmp); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH RESEND 6/9] drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock
From: Libin Yang HDMI audio should use crtc_clock to get the TMDS clock. This patch renames mode to adjusted_mode to unify the name. Reviewed-by: Ville Syrjälä Signed-off-by: Libin Yang Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_audio.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index bce3ad2eb86d..6aa619a84439 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -121,13 +121,14 @@ static u32 audio_config_hdmi_pixel_clock(const struct drm_display_mode *adjusted return hdmi_audio_clock[i].config; } -static int audio_config_get_n(const struct drm_display_mode *mode, int rate) +static int audio_config_get_n(const struct drm_display_mode *adjusted_mode, + int rate) { int i; for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) { if ((rate == aud_ncts[i].sample_rate) && - (mode->clock == aud_ncts[i].clock)) { + (adjusted_mode->crtc_clock == aud_ncts[i].clock)) { return aud_ncts[i].n; } } @@ -267,8 +268,8 @@ hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, enum port port, tmp &= ~AUD_CONFIG_N_PROG_ENABLE; tmp |= audio_config_hdmi_pixel_clock(adjusted_mode); - if (adjusted_mode->clock == TMDS_296M || - adjusted_mode->clock == TMDS_297M) { + if (adjusted_mode->crtc_clock == TMDS_296M || + adjusted_mode->crtc_clock == TMDS_297M) { n = audio_config_get_n(adjusted_mode, rate); if (n != 0) tmp = audio_config_setup_n_reg(n, tmp); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH RESEND 8/9] drm/i915/audio: rename N value getter to emphasize it's for hdmi
We'll be getting a function and a table for dp parameters soon enough, so rename the function and table for hdmi. No functional changes. Cc: Libin Yang Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_audio.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index d2c6227f72b8..81df29ca4947 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -81,7 +81,7 @@ static const struct { int clock; int n; int cts; -} aud_ncts[] = { +} hdmi_aud_ncts[] = { { 44100, TMDS_296M, 4459, 234375 }, { 44100, TMDS_297M, 4704, 247500 }, { 48000, TMDS_296M, 5824, 281250 }, @@ -121,15 +121,15 @@ static u32 audio_config_hdmi_pixel_clock(const struct drm_display_mode *adjusted return hdmi_audio_clock[i].config; } -static int audio_config_get_n(const struct drm_display_mode *adjusted_mode, - int rate) +static int audio_config_hdmi_get_n(const struct drm_display_mode *adjusted_mode, + int rate) { int i; - for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) { - if ((rate == aud_ncts[i].sample_rate) && - (adjusted_mode->crtc_clock == aud_ncts[i].clock)) { - return aud_ncts[i].n; + for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) { + if (rate == hdmi_aud_ncts[i].sample_rate && + adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) { + return hdmi_aud_ncts[i].n; } } return 0; @@ -256,7 +256,7 @@ hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, enum port port, if (adjusted_mode->crtc_clock == TMDS_296M || adjusted_mode->crtc_clock == TMDS_297M) { - n = audio_config_get_n(adjusted_mode, rate); + n = audio_config_hdmi_get_n(adjusted_mode, rate); if (n != 0) { tmp &= ~AUD_CONFIG_N_MASK; tmp |= AUD_CONFIG_N(n); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset
When modeset occurs and the LS_CLK is set to some special values in DP mode, the N/M need to be set manually if audio is playing. Otherwise the first several seconds may be silent in audio playback. The relationship of Maud and Naud is expressed in the following equation: Maud/Naud = 512 * fs / f_LS_Clk Please refer VESA DisplayPort Standard spec for details. Signed-off-by: Libin Yang Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_reg.h| 7 +++ drivers/gpu/drm/i915/intel_audio.c | 100 - 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 595d196f753f..8d9dbc7d5b32 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7359,6 +7359,13 @@ enum { #define _HSW_AUD_MISC_CTRL_B 0x65110 #define HSW_AUD_MISC_CTRL(pipe)_MMIO_PIPE(pipe, _HSW_AUD_MISC_CTRL_A, _HSW_AUD_MISC_CTRL_B) +#define _HSW_AUD_M_CTS_ENABLE_A0x65028 +#define _HSW_AUD_M_CTS_ENABLE_B0x65128 +#define HSW_AUD_M_CTS_ENABLE(pipe) _MMIO_PIPE(pipe, _HSW_AUD_M_CTS_ENABLE_A, _HSW_AUD_M_CTS_ENABLE_B) +#define AUD_M_CTS_M_VALUE_INDEX (1 << 21) +#define AUD_M_CTS_M_PROG_ENABLE (1 << 20) +#define AUD_CONFIG_M_MASK0xf + #define _HSW_AUD_DIP_ELD_CTRL_ST_A 0x650b4 #define _HSW_AUD_DIP_ELD_CTRL_ST_B 0x651b4 #define HSW_AUD_DIP_ELD_CTRL(pipe) _MMIO_PIPE(pipe, _HSW_AUD_DIP_ELD_CTRL_ST_A, _HSW_AUD_DIP_ELD_CTRL_ST_B) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 81df29ca4947..0bc2701b6c9c 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -57,6 +57,70 @@ * struct &i915_audio_component_audio_ops @audio_ops is called from i915 driver. */ +/* DP N/M table */ +#define LC_540M 54 +#define LC_270M 27 +#define LC_162M 162000 + +struct dp_aud_n_m { + int sample_rate; + int clock; + u16 n; + u16 m; +}; + +static const struct dp_aud_n_m dp_aud_n_m[] = { + { 192000, LC_540M, 5625, 1024 }, + { 176400, LC_540M, 9375, 1568 }, + { 96000, LC_540M, 5625, 512 }, + { 88200, LC_540M, 9375, 784 }, + { 48000, LC_540M, 5625, 256 }, + { 44100, LC_540M, 9375, 392 }, + { 32000, LC_540M, 16875, 512 }, + { 192000, LC_270M, 5625, 2048 }, + { 176400, LC_270M, 9375, 3136 }, + { 96000, LC_270M, 5625, 1024 }, + { 88200, LC_270M, 9375, 1568 }, + { 48000, LC_270M, 5625, 512 }, + { 44100, LC_270M, 9375, 784 }, + { 32000, LC_270M, 16875, 1024 }, + { 192000, LC_162M, 3375, 2048 }, + { 176400, LC_162M, 5625, 3136 }, + { 96000, LC_162M, 3375, 1024 }, + { 88200, LC_162M, 5625, 1568 }, + { 48000, LC_162M, 3375, 512 }, + { 44100, LC_162M, 5625, 784 }, + { 32000, LC_162M, 10125, 1024 }, +}; + +static const struct dp_aud_n_m * +audio_config_dp_get_n_m(struct intel_crtc *intel_crtc, int rate) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) { + if (rate == dp_aud_n_m[i].sample_rate && + intel_crtc->config->port_clock == dp_aud_n_m[i].clock) + return &dp_aud_n_m[i]; + } + + return NULL; +} + +static int audio_config_dp_get_m(struct intel_crtc *intel_crtc, int rate) +{ + const struct dp_aud_n_m *nm = audio_config_dp_get_n_m(intel_crtc, rate); + + return nm ? nm->m : 0; +} + +static int audio_config_dp_get_n(struct intel_crtc *intel_crtc, int rate) +{ + const struct dp_aud_n_m *nm = audio_config_dp_get_n_m(intel_crtc, rate); + + return nm ? nm->n : 0; +} + static const struct { int clock; u32 config; @@ -225,8 +289,10 @@ hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port port, const struct drm_display_mode *adjusted_mode) { struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); + struct i915_audio_component *acomp = dev_priv->audio_component; + int rate = acomp ? acomp->aud_sample_rate[port] : 0; enum pipe pipe = intel_crtc->pipe; - u32 tmp; + u32 tmp, n, m; tmp = I915_READ(HSW_AUD_CFG(pipe)); tmp &= ~AUD_CONFIG_N_VALUE_INDEX; @@ -234,7 +300,30 @@ hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port port, tmp &= ~AUD_CONFIG_N_PROG_ENABLE; tmp |= AUD_CONFIG_N_VALUE_INDEX; + if (intel_crtc->config->port_clock == LC_540M || + intel_crtc->config->port_clock == LC_270M || + intel_crtc->config->port_clock == LC_162M) { + n = audio_config_dp_get_n(intel_crtc, rate); + if (n != 0) { + tmp &= ~AUD_CONFIG_N_MASK; + tmp |= AUD_CONFIG_N(n); + tmp |= AUD_CONFIG_N_PROG_ENABLE; + } else { +
[Intel-gfx] [PATCH] drm/crtc: constify drm_crtc_index parameter
Signed-off-by: Jani Nikula --- I needed this for some stuff that turned out to be a dead end. But this seems to be the right thing to do anyway. --- include/drm/drm_crtc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 61932f55f788..0aa292526567 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1342,7 +1342,7 @@ extern void drm_crtc_cleanup(struct drm_crtc *crtc); * Given a registered CRTC, return the index of that CRTC within a DRM * device's list of CRTCs. */ -static inline unsigned int drm_crtc_index(struct drm_crtc *crtc) +static inline unsigned int drm_crtc_index(const struct drm_crtc *crtc) { return crtc->index; } -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/crtc: constify drm_crtc_index parameter
On Mon, Oct 10, 2016 at 06:26:10PM +0300, Jani Nikula wrote: > Signed-off-by: Jani Nikula > > --- > > I needed this for some stuff that turned out to be a dead end. But this > seems to be the right thing to do anyway. Applied to drm-misc. There's also the connector and plane versions of this, care to spin them too? -Daniel > --- > include/drm/drm_crtc.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 61932f55f788..0aa292526567 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -1342,7 +1342,7 @@ extern void drm_crtc_cleanup(struct drm_crtc *crtc); > * Given a registered CRTC, return the index of that CRTC within a DRM > * device's list of CRTCs. > */ > -static inline unsigned int drm_crtc_index(struct drm_crtc *crtc) > +static inline unsigned int drm_crtc_index(const struct drm_crtc *crtc) > { > return crtc->index; > } > -- > 2.1.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [01/42] drm/i915: Allow disabling error capture (rev2)
== Series Details == Series: series starting with [01/42] drm/i915: Allow disabling error capture (rev2) URL : https://patchwork.freedesktop.org/series/13436/ State : failure == Summary == CC arch/x86/kernel/cpu/capflags.o LD fs/btrfs/btrfs.o LD arch/x86/kernel/cpu/built-in.o LD arch/x86/kernel/built-in.o LD drivers/usb/core/usbcore.o LD fs/btrfs/built-in.o LD drivers/usb/core/built-in.o LD arch/x86/built-in.o LD fs/ext4/ext4.o LD fs/ext4/built-in.o LD drivers/usb/host/built-in.o LD fs/built-in.o LD drivers/usb/built-in.o LD net/ipv6/ipv6.o LD net/ipv6/built-in.o LD [M] drivers/net/ethernet/intel/igb/igb.o LD [M] drivers/net/ethernet/intel/e1000e/e1000e.o LD net/ipv4/built-in.o LD net/core/built-in.o LD net/built-in.o LD drivers/net/ethernet/built-in.o LD drivers/net/built-in.o scripts/Makefile.build:440: recipe for target 'drivers/gpu/drm/i915' failed make[3]: *** [drivers/gpu/drm/i915] Error 2 scripts/Makefile.build:440: recipe for target 'drivers/gpu/drm' failed make[2]: *** [drivers/gpu/drm] Error 2 scripts/Makefile.build:440: recipe for target 'drivers/gpu' failed make[1]: *** [drivers/gpu] Error 2 Makefile:968: recipe for target 'drivers' failed make: *** [drivers] Error 2 Full logs at /archive/deploy/logs/Patchwork_2658 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/crtc: constify drm_crtc_index parameter
On Mon, Oct 10, 2016 at 06:26:10PM +0300, Jani Nikula wrote: > Signed-off-by: Jani Nikula Plus equivalents in drm_plane.h, drm_encoder.h ... Reviewed-by: Chris Wilson -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/crtc: constify drm_crtc_index parameter
On Mon, Oct 10, 2016 at 06:26:10PM +0300, Jani Nikula wrote: > Signed-off-by: Jani Nikula > > --- > > I needed this for some stuff that turned out to be a dead end. But this > seems to be the right thing to do anyway. > --- > include/drm/drm_crtc.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 61932f55f788..0aa292526567 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -1342,7 +1342,7 @@ extern void drm_crtc_cleanup(struct drm_crtc *crtc); > * Given a registered CRTC, return the index of that CRTC within a DRM > * device's list of CRTCs. > */ > -static inline unsigned int drm_crtc_index(struct drm_crtc *crtc) > +static inline unsigned int drm_crtc_index(const struct drm_crtc *crtc) > { > return crtc->index; BTW speaking about the index stuff. It dawned on me recently that calling drm_crtc_cleanup() & co. is totally not safe except maybe during the final cleanup. If you would do something like: a = drm_crtc_init(); b = drm_crtc_init(); drm_crtc_cleanup(a); c = drm_crtc_init(); b and c would end up with the same index. Or if you would do just a = drm_crtc_init(); b = drm_crtc_init(); drm_crtc_cleanup(a); We'd end up with num_crtc==1, but b->index==1, so we'd actually access beyond the allocated arrays in a bunch of places. This would need to fixed somehow, or at least documented clearly that if you have to call any of the cleanup functions during init, you have to abort the entire thing. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/crtc: constify drm_crtc_index parameter
On Mon, Oct 10, 2016 at 6:04 PM, Ville Syrjälä wrote: > On Mon, Oct 10, 2016 at 06:26:10PM +0300, Jani Nikula wrote: >> Signed-off-by: Jani Nikula >> >> --- >> >> I needed this for some stuff that turned out to be a dead end. But this >> seems to be the right thing to do anyway. >> --- >> include/drm/drm_crtc.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h >> index 61932f55f788..0aa292526567 100644 >> --- a/include/drm/drm_crtc.h >> +++ b/include/drm/drm_crtc.h >> @@ -1342,7 +1342,7 @@ extern void drm_crtc_cleanup(struct drm_crtc *crtc); >> * Given a registered CRTC, return the index of that CRTC within a DRM >> * device's list of CRTCs. >> */ >> -static inline unsigned int drm_crtc_index(struct drm_crtc *crtc) >> +static inline unsigned int drm_crtc_index(const struct drm_crtc *crtc) >> { >> return crtc->index; > > BTW speaking about the index stuff. It dawned on me recently that calling > drm_crtc_cleanup() & co. is totally not safe except maybe during the > final cleanup. > > If you would do something like: > a = drm_crtc_init(); > b = drm_crtc_init(); > drm_crtc_cleanup(a); > c = drm_crtc_init(); > > b and c would end up with the same index. > > > Or if you would do just > a = drm_crtc_init(); > b = drm_crtc_init(); > drm_crtc_cleanup(a); > > We'd end up with num_crtc==1, but b->index==1, so we'd actually access > beyond the allocated arrays in a bunch of places. > > This would need to fixed somehow, or at least documented clearly that if > you have to call any of the cleanup functions during init, you have to > abort the entire thing. You can't hotplug CRTCs, like you can't hotplug anything else than connectors. And for connectors we have an ida to make sure you never end up with duplicated indizes. So I think we're safe. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Introduce i915_alloc_sg_table
== Series Details == Series: drm/i915: Introduce i915_alloc_sg_table URL : https://patchwork.freedesktop.org/series/13524/ State : success == Summary == Series 13524v1 drm/i915: Introduce i915_alloc_sg_table https://patchwork.freedesktop.org/api/1.0/series/13524/revisions/1/mbox/ Test vgem_basic: Subgroup unload: skip -> PASS (fi-bdw-5557u) fi-bdw-5557u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-bsw-n3050 total:248 pass:204 dwarn:0 dfail:0 fail:0 skip:44 fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31 fi-byt-j1900 total:248 pass:213 dwarn:2 dfail:0 fail:1 skip:32 fi-hsw-4770 total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-hsw-4770r total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-ivb-3520m total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27 fi-ivb-3770 total:248 pass:207 dwarn:0 dfail:0 fail:0 skip:41 fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-6260u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-skl-6700hqtotal:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-skl-6700k total:248 pass:221 dwarn:1 dfail:0 fail:0 skip:26 fi-skl-6770hqtotal:248 pass:231 dwarn:1 dfail:0 fail:1 skip:15 fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37 fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39 Results at /archive/results/CI_IGT_test/Patchwork_2659/ e37a15c8d775e79dddc8345a0f6afdcfe1f607d9 drm-intel-nightly: 2016y-10m-10d-14h-33m-29s UTC integration manifest 1db9a28 drm/i915: Introduce i915_alloc_sg_table ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/crtc: constify drm_crtc_index parameter
On Mon, Oct 10, 2016 at 06:10:42PM +0200, Daniel Vetter wrote: > On Mon, Oct 10, 2016 at 6:04 PM, Ville Syrjälä > wrote: > > On Mon, Oct 10, 2016 at 06:26:10PM +0300, Jani Nikula wrote: > >> Signed-off-by: Jani Nikula > >> > >> --- > >> > >> I needed this for some stuff that turned out to be a dead end. But this > >> seems to be the right thing to do anyway. > >> --- > >> include/drm/drm_crtc.h | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > >> index 61932f55f788..0aa292526567 100644 > >> --- a/include/drm/drm_crtc.h > >> +++ b/include/drm/drm_crtc.h > >> @@ -1342,7 +1342,7 @@ extern void drm_crtc_cleanup(struct drm_crtc *crtc); > >> * Given a registered CRTC, return the index of that CRTC within a DRM > >> * device's list of CRTCs. > >> */ > >> -static inline unsigned int drm_crtc_index(struct drm_crtc *crtc) > >> +static inline unsigned int drm_crtc_index(const struct drm_crtc *crtc) > >> { > >> return crtc->index; > > > > BTW speaking about the index stuff. It dawned on me recently that calling > > drm_crtc_cleanup() & co. is totally not safe except maybe during the > > final cleanup. > > > > If you would do something like: > > a = drm_crtc_init(); > > b = drm_crtc_init(); > > drm_crtc_cleanup(a); > > c = drm_crtc_init(); > > > > b and c would end up with the same index. > > > > > > Or if you would do just > > a = drm_crtc_init(); > > b = drm_crtc_init(); > > drm_crtc_cleanup(a); > > > > We'd end up with num_crtc==1, but b->index==1, so we'd actually access > > beyond the allocated arrays in a bunch of places. > > > > This would need to fixed somehow, or at least documented clearly that if > > you have to call any of the cleanup functions during init, you have to > > abort the entire thing. > > You can't hotplug CRTCs, like you can't hotplug anything else than > connectors. And for connectors we have an ida to make sure you never > end up with duplicated indizes. So I think we're safe. Except in i915 we don't abort the driver load if we would fail part way into the crtc init. So we'd just keep going with bogus indexes and whatnot. I think I wrote a patch to change that, but I'd have find it again to be sure. Other drivers may or may not have the same problem. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Merge duplicate gen4 and vlv/chv enable vblank callbacks (rev3)
== Series Details == Series: series starting with [1/2] drm/i915: Merge duplicate gen4 and vlv/chv enable vblank callbacks (rev3) URL : https://patchwork.freedesktop.org/series/13459/ State : warning == Summary == Series 13459v3 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/13459/revisions/3/mbox/ Test vgem_basic: Subgroup unload: pass -> SKIP (fi-ilk-650) pass -> SKIP (fi-byt-n2820) skip -> PASS (fi-skl-6700k) skip -> PASS (fi-hsw-4770) fi-bdw-5557u total:248 pass:231 dwarn:0 dfail:0 fail:0 skip:17 fi-bsw-n3050 total:248 pass:204 dwarn:0 dfail:0 fail:0 skip:44 fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31 fi-byt-j1900 total:248 pass:214 dwarn:1 dfail:0 fail:1 skip:32 fi-byt-n2820 total:248 pass:210 dwarn:0 dfail:0 fail:1 skip:37 fi-hsw-4770 total:248 pass:225 dwarn:0 dfail:0 fail:0 skip:23 fi-hsw-4770r total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-ilk-650 total:248 pass:184 dwarn:0 dfail:0 fail:2 skip:62 fi-ivb-3520m total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27 fi-ivb-3770 total:248 pass:207 dwarn:0 dfail:0 fail:0 skip:41 fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-6260u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-skl-6700hqtotal:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-skl-6700k total:248 pass:222 dwarn:1 dfail:0 fail:0 skip:25 fi-skl-6770hqtotal:248 pass:231 dwarn:1 dfail:0 fail:1 skip:15 fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37 fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39 Results at /archive/results/CI_IGT_test/Patchwork_2660/ e37a15c8d775e79dddc8345a0f6afdcfe1f607d9 drm-intel-nightly: 2016y-10m-10d-14h-33m-29s UTC integration manifest 5dfc3dd drm/i915: Assert we hold the CRTC powerwell for generating vblank interrupts 2c51949 drm/i915: Merge duplicate gen4 and vlv/chv enable vblank callbacks ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/bxt: Broxton decoupled MMIO
On Mon, 2016-09-26 at 21:23 +0100, Tvrtko Ursulin wrote: > On 26/09/2016 12:08, Paneri, Praveen wrote: > > > > > > On 9/23/2016 3:19 PM, Tvrtko Ursulin wrote: > > > > > > > > > Hi, > > > > > > On 19/09/2016 18:15, Praveen Paneri wrote: > > > > [snip] > > > > > > > > > > > > > > > > > + > > > > enum forcewake_domains > > > > intel_uncore_forcewake_for_reg(struct drm_i915_private > > > > *dev_priv, > > > > i915_reg_t reg, unsigned int op); > > > > @@ -2854,6 +2864,7 @@ struct drm_i915_cmd_table { > > > > #define GT_FREQUENCY_MULTIPLIER 50 > > > > #define GEN9_FREQ_SCALER 3 > > > > +#define HAS_DECOUPLED_MMIO(dev_priv) (IS_BROXTON(dev_priv) > > > > && > > > > IS_BXT_REVID(dev_priv, BXT_REVID_C0, REVID_FOREVER)) > > > There is a recent patch series from Carlos Santa which moved all > > > these > > > type of things to device info. So I think you have to make this > > > compliant with that new style. > > I looked into it. Could you suggest where can I add the check for > > BXT > > C0 revision? > > Will this be okay > > #define HAS_DECOUPLED_MMIO(dev) (INTEL_INFO(dev)- > > >has_decoupled_mmio > > && IS_BXT_REVID(to_i915(dev), BXT_REVID_C0, REVID_FOREVER)) > Good point. I suggest a quick chat with Carlos then to see what was > the > plan for situation like this one. > > [snip] This looks good to me, the main point was to do some clean-up for the legacy features already there but also to move to a single approach (device info) when adding new features. I don't see any other way to specifically check for that version of BXT and this MMIO feature, so that looks good too. Carlos > > > > > > > > > > > > > > > > > + > > > > +ctrl_reg_data |= reg; > > > > +ctrl_reg_data |= (operation << GEN9_DECOUPLED_OP_SHIFT); > > > > +ctrl_reg_data |= (pd_engine << GEN9_DECOUPLED_PD_SHIFT); > > > > +__raw_i915_write32(dev_priv, GEN9_DECOUPLED_REG0_DW1, > > > > ctrl_reg_data); > > > > + > > > > +ctrl_reg_data |= GEN9_DECOUPLED_DW1_GO; > > > > +__raw_i915_write32(dev_priv, GEN9_DECOUPLED_REG0_DW1, > > > > ctrl_reg_data); > > > > + > > > > +if (wait_for_atomic((__raw_i915_read32(dev_priv, > > > > +GEN9_DECOUPLED_REG0_DW1) & GEN9_DECOUPLED_DW1_GO) > > > > == 0, > > > > +FORCEWAKE_ACK_TIMEOUT_MS)) > > > > +DRM_ERROR("Decoupled MMIO wait timed out\n"); > > > > + > > > Is FORCEWAKE_ACK_TIMEOUT_MS correct/relevant for decoupled MMIO? > > > It is > > > potentially quite a long atomic wait. > > This is max wait time. We might not wait for that long but I can > > still > > check on it. > Cool, I do think we need to know and document this in the commit > and/or > code comments where a brief explanation of decoupled mmio will be. > > > > > > > > > > > > Would it be worth returning some fixed value in the case of a > > > timeout? > > > Might be better than random stuff? (applies in the 64-bit read > > > case) > > This is same as forcewake implementation. If we change it, what > > would > > be the appropriate fixed value to be returned? > Another good point. In that case I suppose it doesn't matter so can > leave it like it was. It can only theoretically affect 64-bit reads, > yes? > > > > > > > > > > > > > > > > > +if (operation == GEN9_DECOUPLED_OP_READ) > > > > +*ptr_data = __raw_i915_read32(dev_priv, > > > > +GEN9_DECOUPLED_REG0_DW0); > > > > +} > > > > + > > > > #define GEN2_READ_HEADER(x) \ > > > > u##x val = 0; \ > > > > assert_rpm_wakelock_held(dev_priv); > > > > @@ -892,6 +928,20 @@ static inline void > > > > __force_wake_auto(struct > > > > drm_i915_private *dev_priv, > > > > dev_priv->uncore.funcs.force_wake_get(dev_priv, > > > > fw_domains); > > > > } > > > > +static inline bool __is_forcewake_active(struct > > > > drm_i915_private > > > > *dev_priv, > > > > + enum forcewake_domains fw_domains) > > > > +{ > > > > +struct intel_uncore_forcewake_domain *domain; > > > > + > > > > +/* Ideally GCC would be constant-fold and eliminate this > > > > loop */ > > > > +for_each_fw_domain_masked(domain, fw_domains, dev_priv) { > > > > +if (domain->wake_count) > > > > +fw_domains &= ~domain->mask; > > > > +} > > > > + > > > > +return fw_domains ? 0 : 1; > > > > +} > > > > + > > > > #define __gen6_read(x) \ > > > > static u##x \ > > > > gen6_read##x(struct drm_i915_private *dev_priv, i915_reg_t > > > > reg, bool > > > > trace) { \ > > > > @@ -940,6 +990,37 @@ gen9_read##x(struct drm_i915_private > > > > *dev_priv, > > > > i915_reg_t reg, bool trace) { \ > > > > GEN6_READ_FOOTER; \ > > > > } > > > > +#define __gen9_decoupled_read(x) \ > > > > +static u##x \ > > > > +gen9_decoupled_read##x(struct drm_i915_private *dev_priv, > > > > i915_reg_t > > > > reg, bool trace) { \ > > > > +enum forcewake_domains fw_engine; \ > > > fw_engines > > > > > > > > > > > +GEN6_READ_HEADER(x); \ > > > > +
Re: [Intel-gfx] [PATCH] drm/crtc: constify drm_crtc_index parameter
On Mon, Oct 10, 2016 at 6:30 PM, Ville Syrjälä wrote: >> You can't hotplug CRTCs, like you can't hotplug anything else than >> connectors. And for connectors we have an ida to make sure you never >> end up with duplicated indizes. So I think we're safe. > > Except in i915 we don't abort the driver load if we would fail part > way into the crtc init. So we'd just keep going with bogus indexes > and whatnot. I think I wrote a patch to change that, but I'd have > find it again to be sure. Other drivers may or may not have the same > problem. Hm, we should at least make sure we don't leave any partially initialized modeset objects behind when the modeset part of the driver fails to load. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Remove self-harming shrink_all on get_pages_gtt fail
== Series Details == Series: series starting with [1/2] drm/i915: Remove self-harming shrink_all on get_pages_gtt fail URL : https://patchwork.freedesktop.org/series/13527/ State : warning == Summary == Series 13527v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/13527/revisions/1/mbox/ Test kms_cursor_legacy: Subgroup basic-flip-before-cursor-legacy: pass -> DMESG-WARN (fi-byt-n2820) Test kms_psr_sink_crc: Subgroup psr_basic: pass -> DMESG-WARN (fi-skl-6700hq) Test vgem_basic: Subgroup unload: skip -> PASS (fi-hsw-4770) pass -> SKIP (fi-ilk-650) skip -> PASS (fi-bdw-5557u) skip -> PASS (fi-snb-2600) fi-bdw-5557u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-bsw-n3050 total:248 pass:204 dwarn:0 dfail:0 fail:0 skip:44 fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31 fi-byt-j1900 total:248 pass:215 dwarn:0 dfail:0 fail:1 skip:32 fi-byt-n2820 total:248 pass:210 dwarn:1 dfail:0 fail:1 skip:36 fi-hsw-4770 total:248 pass:225 dwarn:0 dfail:0 fail:0 skip:23 fi-hsw-4770r total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-ilk-650 total:248 pass:184 dwarn:0 dfail:0 fail:2 skip:62 fi-ivb-3520m total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27 fi-ivb-3770 total:248 pass:207 dwarn:0 dfail:0 fail:0 skip:41 fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-6260u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-skl-6700hqtotal:248 pass:223 dwarn:1 dfail:0 fail:0 skip:24 fi-skl-6700k total:248 pass:221 dwarn:1 dfail:0 fail:0 skip:26 fi-skl-6770hqtotal:248 pass:231 dwarn:1 dfail:0 fail:1 skip:15 fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37 fi-snb-2600 total:248 pass:210 dwarn:0 dfail:0 fail:0 skip:38 Results at /archive/results/CI_IGT_test/Patchwork_2661/ e37a15c8d775e79dddc8345a0f6afdcfe1f607d9 drm-intel-nightly: 2016y-10m-10d-14h-33m-29s UTC integration manifest aa705d3 drm/i915: Remove self-harming shrink_all on get_pages_gtt fail ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915/audio: audio cleanups, 4k fixes (rev3)
== Series Details == Series: drm/i915/audio: audio cleanups, 4k fixes (rev3) URL : https://patchwork.freedesktop.org/series/12754/ State : warning == Summary == Series 12754v3 drm/i915/audio: audio cleanups, 4k fixes https://patchwork.freedesktop.org/api/1.0/series/12754/revisions/3/mbox/ Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-c: pass -> DMESG-WARN (fi-skl-6700k) Test kms_psr_sink_crc: Subgroup psr_basic: pass -> DMESG-WARN (fi-skl-6700hq) Test pm_rpm: Subgroup basic-pci-d3-state: pass -> DMESG-WARN (fi-skl-6770hq) Test vgem_basic: Subgroup unload: skip -> PASS (fi-bsw-n3050) skip -> PASS (fi-hsw-4770) pass -> SKIP (fi-ilk-650) fi-bdw-5557u total:248 pass:231 dwarn:0 dfail:0 fail:0 skip:17 fi-bsw-n3050 total:248 pass:205 dwarn:0 dfail:0 fail:0 skip:43 fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31 fi-byt-j1900 total:248 pass:214 dwarn:1 dfail:0 fail:1 skip:32 fi-hsw-4770 total:248 pass:225 dwarn:0 dfail:0 fail:0 skip:23 fi-hsw-4770r total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-ilk-650 total:248 pass:184 dwarn:0 dfail:0 fail:2 skip:62 fi-ivb-3520m total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27 fi-ivb-3770 total:248 pass:207 dwarn:0 dfail:0 fail:0 skip:41 fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-6260u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-skl-6700hqtotal:248 pass:223 dwarn:1 dfail:0 fail:0 skip:24 fi-skl-6700k total:248 pass:220 dwarn:2 dfail:0 fail:0 skip:26 fi-skl-6770hqtotal:248 pass:230 dwarn:2 dfail:0 fail:1 skip:15 fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37 fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39 Results at /archive/results/CI_IGT_test/Patchwork_2663/ e37a15c8d775e79dddc8345a0f6afdcfe1f607d9 drm-intel-nightly: 2016y-10m-10d-14h-33m-29s UTC integration manifest ceae7cb drm/i915: set proper N/M in modeset 831ac5b drm/i915/audio: rename N value getter to emphasize it's for hdmi a12ba34 drm/i915/audio: add register macros for audio config N value d887d42 drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock ba64534 drm/i915/audio: set proper N/MCTS on more platforms cc137fd drm/i915/audio: split dp and hdmi audio config update 293d738 drm/i915/audio: use the same code for updating audio config f04978a drm/i915/audio: port is going to be just fine, simplify checks 5c152ee drm/i915/audio: abstract audio config update ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/crtc: constify drm_crtc_index parameter
== Series Details == Series: drm/crtc: constify drm_crtc_index parameter URL : https://patchwork.freedesktop.org/series/13537/ State : warning == Summary == Series 13537v1 drm/crtc: constify drm_crtc_index parameter https://patchwork.freedesktop.org/api/1.0/series/13537/revisions/1/mbox/ Test kms_cursor_legacy: Subgroup basic-flip-before-cursor-varying-size: pass -> DMESG-WARN (fi-byt-n2820) Test kms_frontbuffer_tracking: Subgroup basic: pass -> DMESG-WARN (fi-byt-n2820) Test vgem_basic: Subgroup unload: pass -> SKIP (fi-skl-6770hq) fi-bdw-5557u total:248 pass:231 dwarn:0 dfail:0 fail:0 skip:17 fi-bsw-n3050 total:248 pass:204 dwarn:0 dfail:0 fail:0 skip:44 fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31 fi-byt-j1900 total:248 pass:214 dwarn:1 dfail:0 fail:1 skip:32 fi-byt-n2820 total:248 pass:209 dwarn:2 dfail:0 fail:1 skip:36 fi-hsw-4770 total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-hsw-4770r total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-ilk-650 total:248 pass:185 dwarn:0 dfail:0 fail:2 skip:61 fi-ivb-3520m total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27 fi-ivb-3770 total:248 pass:207 dwarn:0 dfail:0 fail:0 skip:41 fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-6260u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-skl-6700hqtotal:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-skl-6700k total:248 pass:221 dwarn:1 dfail:0 fail:0 skip:26 fi-skl-6770hqtotal:248 pass:230 dwarn:1 dfail:0 fail:1 skip:16 fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37 fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39 Results at /archive/results/CI_IGT_test/Patchwork_2664/ e37a15c8d775e79dddc8345a0f6afdcfe1f607d9 drm-intel-nightly: 2016y-10m-10d-14h-33m-29s UTC integration manifest 31cdd5e drm/crtc: constify drm_crtc_index parameter ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
Mahesh Kumar is already working on a proper implementation for the workaround, but while we still don't have it, let's just unconditionally apply the workaround for everybody and we hope we can close all those numerous bugzilla tickets. Also, I'm not sure how easy it will be to backport the final implementation to the stable Kernels, and this patch here is probably easier to backport. At the present moment I still don't have confirmation that this patch fixes any of the bugs listed below, but we should definitely try testing all of them again. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94337 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94884 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95010 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96226 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96828 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97450 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97830 Cc: sta...@vger.kernel.org Cc: Mahesh Kumar Cc: Lyude Cc: Dhinakaran Pandiyan Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_pm.c | 49 ++--- 1 file changed, 41 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 62d730d..159831d 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2879,6 +2879,21 @@ skl_wm_plane_id(const struct intel_plane *plane) } } +/* + * FIXME: We still don't have the proper code detect if we need to apply the WA, + * so assume we'll always need it in order to avoid underruns. + */ +static bool intel_needs_memory_bw_wa(struct intel_atomic_state *state) +{ + struct drm_i915_private *dev_priv = to_i915(state->base.dev); + + if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) || + IS_KABYLAKE(dev_priv)) + return true; + + return false; +} + static bool intel_has_sagv(struct drm_i915_private *dev_priv) { @@ -2999,9 +3014,10 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state) struct drm_device *dev = state->dev; struct drm_i915_private *dev_priv = to_i915(dev); struct intel_atomic_state *intel_state = to_intel_atomic_state(state); - struct drm_crtc *crtc; + struct intel_crtc *crtc; + struct intel_plane *plane; enum pipe pipe; - int level, plane; + int level, id, latency; if (!intel_has_sagv(dev_priv)) return false; @@ -3019,27 +3035,36 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state) /* Since we're now guaranteed to only have one active CRTC... */ pipe = ffs(intel_state->active_crtcs) - 1; - crtc = dev_priv->pipe_to_crtc_mapping[pipe]; + crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); - if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE) + if (crtc->base.state->mode.flags & DRM_MODE_FLAG_INTERLACE) return false; - for_each_plane(dev_priv, pipe, plane) { + for_each_intel_plane_on_crtc(dev, crtc, plane) { + id = skl_wm_plane_id(plane); + /* Skip this plane if it's not enabled */ - if (intel_state->wm_results.plane[pipe][plane][0] == 0) + if (intel_state->wm_results.plane[pipe][id][0] == 0) continue; /* Find the highest enabled wm level for this plane */ for (level = ilk_wm_max_level(dev); -intel_state->wm_results.plane[pipe][plane][level] == 0; --level) +intel_state->wm_results.plane[pipe][id][level] == 0; --level) { } + latency = dev_priv->wm.skl_latency[level]; + + if (intel_needs_memory_bw_wa(intel_state) && + plane->base.state->fb->modifier[0] == + I915_FORMAT_MOD_X_TILED) + latency += 15; + /* * If any of the planes on this pipe don't enable wm levels * that incur memory latencies higher then 30µs we can't enable * the SAGV */ - if (dev_priv->wm.skl_latency[level] < SKL_SAGV_BLOCK_TIME) + if (latency < SKL_SAGV_BLOCK_TIME) return false; } @@ -3549,12 +3574,18 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, uint32_t width = 0, height = 0; uint32_t plane_pixel_rate; uint32_t y_tile_minimum, y_min_scanlines; + struct intel_atomic_state *state = + to_intel_atomic_state(cstate->base.state); + bool apply_memory_bw_wa = intel_needs_memory_bw_wa(state); if (latency == 0 || !cstate->base.active || !intel_pstate->base.visible) { *enabled = false; retur
[Intel-gfx] [PATCH 2/2] drm/i915/gen9: look for adjusted_mode in the SAGV check for interlaced
We want to look at the mode that we're actually going to set. All the other display checks for interlaced flags also look at adjusted_mode. Cc: Lyude Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 159831d..4b7de7a 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3037,7 +3037,7 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state) pipe = ffs(intel_state->active_crtcs) - 1; crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); - if (crtc->base.state->mode.flags & DRM_MODE_FLAG_INTERLACE) + if (crtc->base.state->adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) return false; for_each_intel_plane_on_crtc(dev, crtc, plane) { -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
Was there a new workaround added recently? Or was this something I just missed when writing the original code for this On Mon, 2016-10-10 at 17:30 -0300, Paulo Zanoni wrote: > Mahesh Kumar is already working on a proper implementation for the > workaround, but while we still don't have it, let's just > unconditionally apply the workaround for everybody and we hope we can > close all those numerous bugzilla tickets. Also, I'm not sure how > easy > it will be to backport the final implementation to the stable > Kernels, > and this patch here is probably easier to backport. > > At the present moment I still don't have confirmation that this patch > fixes any of the bugs listed below, but we should definitely try > testing all of them again. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94337 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94884 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95010 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96226 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96828 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97450 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97830 > Cc: sta...@vger.kernel.org > Cc: Mahesh Kumar > Cc: Lyude > Cc: Dhinakaran Pandiyan > Signed-off-by: Paulo Zanoni > --- > drivers/gpu/drm/i915/intel_pm.c | 49 > ++--- > 1 file changed, 41 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c > index 62d730d..159831d 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2879,6 +2879,21 @@ skl_wm_plane_id(const struct intel_plane > *plane) > } > } > > +/* > + * FIXME: We still don't have the proper code detect if we need to > apply the WA, > + * so assume we'll always need it in order to avoid underruns. > + */ > +static bool intel_needs_memory_bw_wa(struct intel_atomic_state > *state) > +{ > + struct drm_i915_private *dev_priv = to_i915(state- > >base.dev); > + > + if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) || > + IS_KABYLAKE(dev_priv)) > + return true; > + > + return false; > +} > + > static bool > intel_has_sagv(struct drm_i915_private *dev_priv) > { > @@ -2999,9 +3014,10 @@ bool intel_can_enable_sagv(struct > drm_atomic_state *state) > struct drm_device *dev = state->dev; > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_atomic_state *intel_state = > to_intel_atomic_state(state); > - struct drm_crtc *crtc; > + struct intel_crtc *crtc; > + struct intel_plane *plane; > enum pipe pipe; > - int level, plane; > + int level, id, latency; > > if (!intel_has_sagv(dev_priv)) > return false; > @@ -3019,27 +3035,36 @@ bool intel_can_enable_sagv(struct > drm_atomic_state *state) > > /* Since we're now guaranteed to only have one active > CRTC... */ > pipe = ffs(intel_state->active_crtcs) - 1; > - crtc = dev_priv->pipe_to_crtc_mapping[pipe]; > + crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > > - if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE) > + if (crtc->base.state->mode.flags & DRM_MODE_FLAG_INTERLACE) > return false; > > - for_each_plane(dev_priv, pipe, plane) { > + for_each_intel_plane_on_crtc(dev, crtc, plane) { > + id = skl_wm_plane_id(plane); > + > /* Skip this plane if it's not enabled */ > - if (intel_state->wm_results.plane[pipe][plane][0] == > 0) > + if (intel_state->wm_results.plane[pipe][id][0] == 0) > continue; > > /* Find the highest enabled wm level for this plane > */ > for (level = ilk_wm_max_level(dev); > - intel_state- > >wm_results.plane[pipe][plane][level] == 0; --level) > + intel_state->wm_results.plane[pipe][id][level] > == 0; --level) > { } > > + latency = dev_priv->wm.skl_latency[level]; > + > + if (intel_needs_memory_bw_wa(intel_state) && > + plane->base.state->fb->modifier[0] == > + I915_FORMAT_MOD_X_TILED) > + latency += 15; > + > /* > * If any of the planes on this pipe don't enable wm > levels > * that incur memory latencies higher then 30µs we > can't enable > * the SAGV > */ > - if (dev_priv->wm.skl_latency[level] < > SKL_SAGV_BLOCK_TIME) > + if (latency < SKL_SAGV_BLOCK_TIME) > return false; > } > > @@ -3549,12 +3574,18 @@ static int skl_compute_plane_wm(const struct > drm_i915_private *dev_priv, > uint32_t width = 0, height = 0; > uint32_t plane_pixel_rate; > uint32_t y_tile_min
Re: [Intel-gfx] [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
Em Seg, 2016-10-10 às 16:34 -0400, Lyude Paul escreveu: > Was there a new workaround added recently? Or was this something I > just > missed when writing the original code for this It's listed on the public spec: https://01.org/sites/default/files/docum entation/intel-gfx-prm-osrc-skl-vol12-display.pdf Pages 210 - 211. > > On Mon, 2016-10-10 at 17:30 -0300, Paulo Zanoni wrote: > > > > Mahesh Kumar is already working on a proper implementation for the > > workaround, but while we still don't have it, let's just > > unconditionally apply the workaround for everybody and we hope we > > can > > close all those numerous bugzilla tickets. Also, I'm not sure how > > easy > > it will be to backport the final implementation to the stable > > Kernels, > > and this patch here is probably easier to backport. > > > > At the present moment I still don't have confirmation that this > > patch > > fixes any of the bugs listed below, but we should definitely try > > testing all of them again. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94337 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94884 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95010 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96226 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96828 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97450 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97830 > > Cc: sta...@vger.kernel.org > > Cc: Mahesh Kumar > > Cc: Lyude > > Cc: Dhinakaran Pandiyan > > Signed-off-by: Paulo Zanoni > > --- > > drivers/gpu/drm/i915/intel_pm.c | 49 > > ++--- > > 1 file changed, 41 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > b/drivers/gpu/drm/i915/intel_pm.c > > index 62d730d..159831d 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -2879,6 +2879,21 @@ skl_wm_plane_id(const struct intel_plane > > *plane) > > } > > } > > > > +/* > > + * FIXME: We still don't have the proper code detect if we need to > > apply the WA, > > + * so assume we'll always need it in order to avoid underruns. > > + */ > > +static bool intel_needs_memory_bw_wa(struct intel_atomic_state > > *state) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(state- > > > > > > base.dev); > > + > > + if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) || > > + IS_KABYLAKE(dev_priv)) > > + return true; > > + > > + return false; > > +} > > + > > static bool > > intel_has_sagv(struct drm_i915_private *dev_priv) > > { > > @@ -2999,9 +3014,10 @@ bool intel_can_enable_sagv(struct > > drm_atomic_state *state) > > struct drm_device *dev = state->dev; > > struct drm_i915_private *dev_priv = to_i915(dev); > > struct intel_atomic_state *intel_state = > > to_intel_atomic_state(state); > > - struct drm_crtc *crtc; > > + struct intel_crtc *crtc; > > + struct intel_plane *plane; > > enum pipe pipe; > > - int level, plane; > > + int level, id, latency; > > > > if (!intel_has_sagv(dev_priv)) > > return false; > > @@ -3019,27 +3035,36 @@ bool intel_can_enable_sagv(struct > > drm_atomic_state *state) > > > > /* Since we're now guaranteed to only have one active > > CRTC... */ > > pipe = ffs(intel_state->active_crtcs) - 1; > > - crtc = dev_priv->pipe_to_crtc_mapping[pipe]; > > + crtc = to_intel_crtc(dev_priv- > > >pipe_to_crtc_mapping[pipe]); > > > > - if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE) > > + if (crtc->base.state->mode.flags & > > DRM_MODE_FLAG_INTERLACE) > > return false; > > > > - for_each_plane(dev_priv, pipe, plane) { > > + for_each_intel_plane_on_crtc(dev, crtc, plane) { > > + id = skl_wm_plane_id(plane); > > + > > /* Skip this plane if it's not enabled */ > > - if (intel_state->wm_results.plane[pipe][plane][0] > > == > > 0) > > + if (intel_state->wm_results.plane[pipe][id][0] == > > 0) > > continue; > > > > /* Find the highest enabled wm level for this > > plane > > */ > > for (level = ilk_wm_max_level(dev); > > - intel_state- > > > > > > wm_results.plane[pipe][plane][level] == 0; --level) > > + intel_state- > > >wm_results.plane[pipe][id][level] > > == 0; --level) > > { } > > > > + latency = dev_priv->wm.skl_latency[level]; > > + > > + if (intel_needs_memory_bw_wa(intel_state) && > > + plane->base.state->fb->modifier[0] == > > + I915_FORMAT_MOD_X_TILED) > > + latency += 15; > > + > > /* > > * If any of the planes on this pipe don't enable > > wm > > levels > > * that incur memory latencies higher then 30µs we > > can't enable > >
Re: [Intel-gfx] [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
I feel like the PRMs should be a little less confusing so it doesn't take four of us reading it to actually get the full spec into code… Anyway, while this is going to be running on more then just skylake I feel like intel_needs_memory_bw_wa() is a little confusing since I would expect eventually we're going to have to apply other unrelated memory bandwidth workarounds on other platforms. How about skl_needs_memory_bw_wa() or gen9_needs_memory_bw_wa()? On Mon, 2016-10-10 at 20:46 +, Zanoni, Paulo R wrote: > Em Seg, 2016-10-10 às 16:34 -0400, Lyude Paul escreveu: > > > > Was there a new workaround added recently? Or was this something I > > just > > missed when writing the original code for this > > It's listed on the public spec: > https://01.org/sites/default/files/docum > entation/intel-gfx-prm-osrc-skl-vol12-display.pdf > > Pages 210 - 211. > > > > > > > On Mon, 2016-10-10 at 17:30 -0300, Paulo Zanoni wrote: > > > > > > > > > Mahesh Kumar is already working on a proper implementation for > > > the > > > workaround, but while we still don't have it, let's just > > > unconditionally apply the workaround for everybody and we hope we > > > can > > > close all those numerous bugzilla tickets. Also, I'm not sure how > > > easy > > > it will be to backport the final implementation to the stable > > > Kernels, > > > and this patch here is probably easier to backport. > > > > > > At the present moment I still don't have confirmation that this > > > patch > > > fixes any of the bugs listed below, but we should definitely try > > > testing all of them again. > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94337 > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605 > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94884 > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95010 > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96226 > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96828 > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97450 > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97830 > > > Cc: sta...@vger.kernel.org > > > Cc: Mahesh Kumar > > > Cc: Lyude > > > Cc: Dhinakaran Pandiyan > > > Signed-off-by: Paulo Zanoni > > > --- > > > drivers/gpu/drm/i915/intel_pm.c | 49 > > > ++--- > > > 1 file changed, 41 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > b/drivers/gpu/drm/i915/intel_pm.c > > > index 62d730d..159831d 100644 > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > @@ -2879,6 +2879,21 @@ skl_wm_plane_id(const struct intel_plane > > > *plane) > > > } > > > } > > > > > > +/* > > > + * FIXME: We still don't have the proper code detect if we need > > > to > > > apply the WA, > > > + * so assume we'll always need it in order to avoid underruns. > > > + */ > > > +static bool intel_needs_memory_bw_wa(struct intel_atomic_state > > > *state) > > > +{ > > > + struct drm_i915_private *dev_priv = to_i915(state- > > > > > > > > > > > > base.dev); > > > + > > > + if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) || > > > + IS_KABYLAKE(dev_priv)) > > > + return true; > > > + > > > + return false; > > > +} > > > + > > > static bool > > > intel_has_sagv(struct drm_i915_private *dev_priv) > > > { > > > @@ -2999,9 +3014,10 @@ bool intel_can_enable_sagv(struct > > > drm_atomic_state *state) > > > struct drm_device *dev = state->dev; > > > struct drm_i915_private *dev_priv = to_i915(dev); > > > struct intel_atomic_state *intel_state = > > > to_intel_atomic_state(state); > > > - struct drm_crtc *crtc; > > > + struct intel_crtc *crtc; > > > + struct intel_plane *plane; > > > enum pipe pipe; > > > - int level, plane; > > > + int level, id, latency; > > > > > > if (!intel_has_sagv(dev_priv)) > > > return false; > > > @@ -3019,27 +3035,36 @@ bool intel_can_enable_sagv(struct > > > drm_atomic_state *state) > > > > > > /* Since we're now guaranteed to only have one active > > > CRTC... */ > > > pipe = ffs(intel_state->active_crtcs) - 1; > > > - crtc = dev_priv->pipe_to_crtc_mapping[pipe]; > > > + crtc = to_intel_crtc(dev_priv- > > > > > > > > pipe_to_crtc_mapping[pipe]); > > > > > > - if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE) > > > + if (crtc->base.state->mode.flags & > > > DRM_MODE_FLAG_INTERLACE) > > > return false; > > > > > > - for_each_plane(dev_priv, pipe, plane) { > > > + for_each_intel_plane_on_crtc(dev, crtc, plane) { > > > + id = skl_wm_plane_id(plane); > > > + > > > /* Skip this plane if it's not enabled */ > > > - if (intel_state- > > > >wm_results.plane[pipe][plane][0] > > > == > > > 0) > > > + if (intel_state->wm_results.plane[pipe][id][0] > > > == > > > 0) > > > continue; > > > > > > /* Find the highest enabled wm level for this > >
Re: [Intel-gfx] [PATCH 2/2] drm/i915/gen9: look for adjusted_mode in the SAGV check for interlaced
Reviewed-by: Lyude On Mon, 2016-10-10 at 17:30 -0300, Paulo Zanoni wrote: > We want to look at the mode that we're actually going to set. All the > other display checks for interlaced flags also look at adjusted_mode. > > Cc: Lyude > Signed-off-by: Paulo Zanoni > --- > drivers/gpu/drm/i915/intel_pm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c > index 159831d..4b7de7a 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3037,7 +3037,7 @@ bool intel_can_enable_sagv(struct > drm_atomic_state *state) > pipe = ffs(intel_state->active_crtcs) - 1; > crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > > - if (crtc->base.state->mode.flags & DRM_MODE_FLAG_INTERLACE) > + if (crtc->base.state->adjusted_mode.flags & > DRM_MODE_FLAG_INTERLACE) > return false; > > for_each_intel_plane_on_crtc(dev, crtc, plane) { ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
== Series Details == Series: series starting with [1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA URL : https://patchwork.freedesktop.org/series/13548/ State : warning == Summary == Series 13548v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/13548/revisions/1/mbox/ Test drv_module_reload_basic: pass -> SKIP (fi-ivb-3520m) Test kms_busy: Subgroup basic-flip-default-a: pass -> DMESG-WARN (fi-ilk-650) Test vgem_basic: Subgroup unload: pass -> SKIP (fi-ilk-650) fi-bdw-5557u total:248 pass:231 dwarn:0 dfail:0 fail:0 skip:17 fi-bsw-n3050 total:248 pass:204 dwarn:0 dfail:0 fail:0 skip:44 fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31 fi-byt-j1900 total:248 pass:212 dwarn:0 dfail:0 fail:3 skip:33 fi-byt-n2820 total:248 pass:211 dwarn:0 dfail:0 fail:1 skip:36 fi-hsw-4770 total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-hsw-4770r total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-ilk-650 total:248 pass:183 dwarn:1 dfail:0 fail:2 skip:62 fi-ivb-3520m total:248 pass:220 dwarn:0 dfail:0 fail:0 skip:28 fi-ivb-3770 total:248 pass:207 dwarn:0 dfail:0 fail:0 skip:41 fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26 fi-skl-6260u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16 fi-skl-6700hqtotal:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24 fi-skl-6700k total:248 pass:221 dwarn:1 dfail:0 fail:0 skip:26 fi-skl-6770hqtotal:248 pass:231 dwarn:1 dfail:0 fail:1 skip:15 fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37 fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39 Results at /archive/results/CI_IGT_test/Patchwork_2665/ e37a15c8d775e79dddc8345a0f6afdcfe1f607d9 drm-intel-nightly: 2016y-10m-10d-14h-33m-29s UTC integration manifest 6f2eff9 drm/i915/gen9: look for adjusted_mode in the SAGV check for interlaced 01581cd drm/i915/gen9: unconditionally apply the memory bandwidth WA ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx