- 07 Nov, 2019 8 commits
-
-
Chris Wilson authored
The hidden aliasing-ppgtt's size is never revealed, as we only inspect the front GTT when engaged. However, we were "fixing" the hidden ppgtt to match, with the net result that we ended up leaking the unused portion on Braswell were we preallocated the entire set of top level PDP, see gen8_preallocate_top_level_pdp(). [ 26.025364] DMA-API: pci 0000:00:02.0: device driver has pending DMA allocations while released from device [count=2] [ 26.025364] One of leaked entries details: [device address=0x0000000230778000] [size=4096 bytes] [mapped with DMA_BIDIRECTIONAL] [mapped as single] [ 26.025683] WARNING: CPU: 0 PID: 415 at kernel/dma/debug.c:894 dma_debug_device_change+0x1a4/0x1f0 [ 26.025905] Modules linked in: i915(E-) intel_powerclamp(E) nls_ascii(E) nls_cp437(E) crct10dif_pclmul(E) crc32_pclmul(E) vfat(E) crc32c_intel(E) fat(E) ghash_clmulni_intel(E) prime_numbers(E) intel_gtt(E) i2c_algo_bit(E) efi_pstore(E) drm_kms_helper(E) syscopyarea(E) sysfillrect(E) sysimgblt(E) fb_sys_fops(E) evdev(E) drm(E) aesni_intel(E) glue_helper(E) crypto_simd(E) cryptd(E) intel_cstate(E) sg(E) efivars(E) pcspkr(E) video(E) button(E) efivarfs(E) ip_tables(E) x_tables(E) autofs4(E) sd_mod(E) lpc_ich(E) ahci(E) mfd_core(E) i2c_i801(E) libahci(E) i2c_designware_pci(E) i2c_designware_core(E) [ 26.026613] CPU: 0 PID: 415 Comm: rmmod Tainted: G E 5.4.0-rc6+ #25 [ 26.026837] Hardware name: /, BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015 [ 26.027080] RIP: 0010:dma_debug_device_change+0x1a4/0x1f0 [ 26.027319] Code: 89 54 24 08 e8 ad 60 62 00 48 8b 54 24 08 48 89 c6 41 57 4d 89 e9 49 89 d8 44 89 f1 41 54 48 c7 c7 e0 61 06 82 e8 c1 aa f5 ff <0f> 0b 5a 59 48 83 3c 24 00 0f 85 97 26 00 00 8b 05 77 47 92 01 85 [ 26.027600] RSP: 0018:ffff888228d2fcc8 EFLAGS: 00010282 [ 26.027831] RAX: 0000000000000000 RBX: 0000000230778000 RCX: 0000000000000000 [ 26.028053] RDX: 0000000000000001 RSI: 0000000000000008 RDI: ffffed10451a5f8f [ 26.028279] RBP: ffff88823480c0b0 R08: 0000000000000001 R09: ffffed1046e83eb1 [ 26.028500] R10: ffffed1046e83eb0 R11: ffff88823741f587 R12: ffffffff82067340 [ 26.028725] R13: 0000000000001000 R14: 0000000000000002 R15: ffffffff82067480 [ 26.028952] FS: 00007fdf3ed174c0(0000) GS:ffff888237400000(0000) knlGS:0000000000000000 [ 26.029185] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 26.029405] CR2: 000055e211109030 CR3: 0000000230139000 CR4: 00000000001006f0 [ 26.029622] Call Trace: [ 26.029846] notifier_call_chain+0x67/0xa0 [ 26.030076] blocking_notifier_call_chain+0x5a/0x80 [ 26.030305] device_release_driver_internal+0x20d/0x260 [ 26.030535] driver_detach+0x7b/0xe1 [ 26.030761] bus_remove_driver+0x8c/0x153 [ 26.030993] pci_unregister_driver+0x2d/0xf0 [ 26.032603] i915_exit+0x16/0x1c [i915] Reported-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Fixes: 1eda701e ("drm/i915/gtt: Recursive cleanup for gen8") References: c082afac ("drm/i915: Move aliasing_ppgtt underneath its i915_ggtt") Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Reviewed-by:
Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191106221223.7437-1-chris@chris-wilson.co.uk
-
Jani Nikula authored
The intel_dp_link_training.h include has no need or place in intel_display.h. Include it in intel_display.c instead. Cc: Manasi Navare <manasi.d.navare@intel.com> Fixes: eadf6f91 ("drm/i915/display/icl: Enable master-slaves in trans port sync") Reviewed-by:
Manasi Navare <manasi.d.navare@intel.com> Signed-off-by:
Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191029103947.7535-1-jani.nikula@intel.com
-
Daniel Vetter authored
So strictly speaking the existing annotation is also ok, because we have a chain of obj->mm.lock#I915_MM_GET_PAGES -> fs_reclaim -> obj->mm.lock (the shrinker cannot get at an object while we're in get_pages, hence this is safe). But it's confusing, so try to take the right subclass of the lock. This does a bit reduce our lockdep based checking, but then it's also less fragile, in case we ever change the nesting around. Signed-off-by:
Daniel Vetter <daniel.vetter@intel.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Will Deacon <will@kernel.org> Cc: linux-kernel@vger.kernel.org Signed-off-by:
Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by:
Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191104173720.2696-3-daniel.vetter@ffwll.ch
-
Daniel Vetter authored
Necessary to annotate functions where we might acquire a mutex_lock_nested() or similar. Needed by i915. Acked-by:
Peter Zijlstra (Intel) <peterz@infradead.org> Signed-off-by:
-
Daniel Vetter authored
The trouble with having a plain nesting flag for locks which do not naturally nest (unlike block devices and their partitions, which is the original motivation for nesting levels) is that lockdep will never spot a true deadlock if you screw up. This patch is an attempt at trying better, by highlighting a bit more of the actual nature of the nesting that's going on. Essentially we have two kinds of objects: - objects without pages allocated, which cannot be on any lru and are hence inaccessible to the shrinker. - objects which have pages allocated, which are on an lru, and which the shrinker can decide to throw out. For the former type of object, memory allocations while holding obj->mm.lock are permissible. For the latter they are not. And get/put_pages transitions between the two types of objects. This is still not entirely fool-proof since the rules might change. But as long as we run such a code ever at runtime lockdep should be able to observe the inconsistency and complain (like with any other lockdep class that we've split up in multiple classes). But there are a few clear benefits: - We can drop the nesting flag parameter from __i915_gem_object_put_pages, because that function by definition is never going allocate memory, and calling it on an object which doesn't have its pages allocated would be a bug. - We strictly catch more bugs, since there's not only one place in the entire tree which is annotated with the special class. All the other places that had explicit lockdep nesting annotations we're now going to leave up to lockdep again. - Specifically this catches stuff like calling get_pages from put_pages (which isn't really a good idea, if we can call get_pages so could the shrinker). I've seen patches do exactly that. Of course I fully expect CI will show me for the fool I am with this one here :-) v2: There can only be one (lockdep only has a cache for the first subclass, not for deeper ones, and we don't want to make these locks even slower). Still separate enums for better documentation. Real fix: don't forget about phys objs and pin_map(), and fix the shrinker to have the right annotations ... silly me. v3: Forgot usertptr too ... v4: Improve comment for pages_pin_count, drop the IMPORTANT comment and instead prime lockdep (Chris). v5: Appease checkpatch, no double empty lines (Chris) v6: More rebasing over selftest changes. Also somehow I forgot to push this patch :-/ Also format comments consistently while at it. v7: Fix typo in commit message (Joonas) Also drop the priming, with the lmem merge we now have allocations while holding the lmem lock, which wreaks the generic priming I've done in earlier patches. Should probably be resurrected when lmem is fixed. See commit 232a6eba Author: Matthew Auld <matthew.auld@intel.com> Date: Tue Oct 8 17:01:14 2019 +0100 drm/i915: introduce intel_memory_region I'm keeping the priming patch locally so it wont get lost. Cc: Matthew Auld <matthew.auld@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: "Tang, CQ" <cq.tang@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v5) Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> (v6) Signed-off-by:
-
Chris Wilson authored
Before we grab the engine wakeref, tidy up the previous heartbeat request. If we then abort because the engine powerwell is off, we ensure the request is freed as we know we will not have freed it when cancelling the work (as the work is running!). Fixes: 841e8672 ("drm/i915/gt: Only drop heartbeat.systole if the sole owner") References: 058179e7 ("drm/i915/gt: Replace hangcheck by heartbeats") Signed-off-by:
-
Lucas De Marchi authored
Prefer using intel_encoder and pass the base where needed rather than keeping both encoder and intel_encoder variables around. v2: actually add all changes to the patch Signed-off-by:
Lucas De Marchi <lucas.demarchi@intel.com> Reviewed-by:
-
James Ausmus authored
Another TGP ID has shown up, so let's add it to avoid South Display breakage on systems that have this ID. Cc: Lucas De Marchi <lucas.demarchi@intel.com> Cc: José Roberto de Souza <jose.souza@intel.com> Signed-off-by:
James Ausmus <james.ausmus@intel.com> Reviewed-by:
José Roberto de Souza <jose.souza@intel.com> Signed-off-by:
-
- 06 Nov, 2019 7 commits
-
-
Niranjana Vishwanathapura authored
We don't need rcu read side critical section to call pid_nr(), remove it. Signed-off-by:
Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> Reviewed-by:
-
Chris Wilson authored
When inside the lock, remember to unlock even if you want to leave early. Reported-by:
Dan Carpenter <dan.carpenter@oracle.com> Fixes: a4e7ccda ("drm/i915: Move context management under GEM") Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191106144155.25727-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
Mika spotted that only using cancel_delayed_work() could mean that we attempted to clear the heartbeat.systole while the worker was still running. Rectify the situation by only touching the systole from outside the worker if we suceeded in cancelling the worker before it could run. The worker is expected to clean up by itself upon idling. Reported-by:
-
Matthew Auld authored
We should not be unconditionally calling release_fake_lmem_bar. Signed-off-by:
Matthew Auld <matthew.auld@intel.com> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
-
Ville Syrjälä authored
The uapi vs. hw state split introduced a bug in intel_crtc_disable_noatomic() where it's now frobbing an already freed temp crtc state instead of adjusting the crtc state we are really left with. Fix that by making a cleaner separation beteen the two. This causes explosions on any machine that boots up with pipes already running but not hooked up to any encoder (typical behaviour for gen2-4 VBIOS). Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Fixes: 58d124ea ("drm/i915: Complete crtc hw/uapi split, v6.") Signed-off-by:
-
Chris Wilson authored
If the device does not have an aperture through which we can indirectly access and detile the buffers, simply reject the ioctl. Later we can extend the ioctl to support different modes, but as an extension the user must opt in and explicitly control the mmap type (viz MMAP_OFFSET_IOCTL). Signed-off-by:
Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Reviewed-by:
Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191105145305.14314-1-chris@chris-wilson.co.uk
-
Tvrtko Ursulin authored
It sounds like the hardware only needs the DSB object to be in global GTT and not in the mappable region. Signed-off-by:
Animesh Manna <animesh.manna@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191017155810.21654-1-tvrtko.ursulin@linux.intel.com
-
- 05 Nov, 2019 5 commits
-
-
Jani Nikula authored
Use intel_dsc_ prefix. No functional changes. Cc: Manasi Navare <manasi.d.navare@intel.com> Reviewed-by:
-
Jani Nikula authored
Split out the DP specific parts, making it easier to add DSI specific configuration. Also move the encoder specific parts towards the end, to allow overriding generic configuration if needed. This also improves clarity by making it clear the encoder independent configuration does not depend on the encoder specific parts. v2: Rebase Cc: Manasi Navare <manasi.d.navare@intel.com> Reviewed-by:
-
Jani Nikula authored
Use a simple pointer to the relevant element instead of duplicating the array subscription. No functional changes. Cc: Manasi Navare <manasi.d.navare@intel.com> Reviewed-by:
-
Jani Nikula authored
No need for them to be mutable. Cc: Manasi Navare <manasi.d.navare@intel.com> Reviewed-by:
-
Jani Nikula authored
Since CNP it's possible for rawclk to have two different values, 19.2 and 24 MHz. If the value indicated by SFUSE_STRAP register is different from the power on default for PCH_RAWCLK_FREQ, we'll end up having a mismatch between the rawclk hardware and software states after suspend/resume. On previous platforms this used to work by accident, because the power on defaults worked just fine. Update the rawclk also on resume. The natural place to do this would be intel_modeset_init_hw(), however VLV/CHV need it done before intel_power_domains_init_hw(). Thus put it there even if it feels slightly out of place. v2: Call intel_update_rawclck() in intel_power_domains_init_hw() for all platforms (Ville). Reported-by:Shawn Lee <shawn.c.lee@intel.com> Cc: Shawn Lee <shawn.c.lee@intel.com> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Reviewed-by:
-
- 04 Nov, 2019 15 commits
-
-
Lucas De Marchi authored
Just avoid the additional read in case DP_TP_CTL is enabled: read it once and save the value. Signed-off-by:
-
Lucas De Marchi authored
For MST on Tiger Lake there are different moments when we need to configure the transcoder clock select. For the first link this is in step 7.a of the spec, before training the link. For additional streams this should be done as part of step 8.b after programming receiver VC Payload ID. Bspec: 49190 Signed-off-by:
-
Lucas De Marchi authored
Wrap drm_atomic_get_old_connector_state so we can get the intel_digital_connector_state and make it easier to migrate to intel types. Signed-off-by:
-
José Roberto de Souza authored
The same macro as for_each_new_connector_in_state() but it uses intel/i915 types instead of the drm ones. Signed-off-by:
Mika Kahola <mika.kahola@intel.com> Reviewed-by:
-
Ville Syrjälä authored
Lots of redundant assignments inside intel_primary_plane_create(). Get rid of them. v2: Rebase due to fp16 landing Signed-off-by:
Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com> Reviewed-by:
Uma Shankar <uma.shankar@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191031165652.10868-8-ville.syrjala@linux.intel.com
-
Ville Syrjälä authored
Let's try to keep the pixel format arrays somewhat sorted: 1. RGB before YUV 2. smaller bpp before larger bpp 3. X before A 4. RGB before BGR Signed-off-by:
-
Ville Syrjälä authored
ICL+ again supports alpha blending with 10bpc pixel formats. Expose them. v2: Add all the stuff I missed earlier! Signed-off-by:
-
Ville Syrjälä authored
VLV/CHV sprite planes also support the C8 format. Let's expose that. Signed-off-by:
-
Ville Syrjälä authored
CHV pipe B sprites gained support for the 10bpc X/ARGB pixel formats. On VLV and CHV pipe A/C these are only supported by the primary plane. Add the require bits to expose the new formats. v2: Reorder the formats for consistency Signed-off-by:
-
Ville Syrjälä authored
Currently we expose VLV/CHV alpha blending only on the sprite planes, but the primary planes can do it as well. Let's flip it on. v2: Rebase due to fp16 landing Signed-off-by:
-
Ville Syrjälä authored
SNB-BDW support 10:10:10 formats on the sprite planes. Let's expose them. v2: Rebase due to fp16 landing Signed-off-by:
-
Chris Wilson authored
Sometimes Icelake forgets to reset the CSB pointers on a GPU reset, leading to it carry on updating the old tail of the buffer. <0>[ 618.138490] i915_sel-5636 3d..1 673425465us : trace_ports: vecs0: submit { 14de2:504, 0:0 } <0>[ 618.138490] i915_sel-5636 3.... 673425493us : intel_engine_reset: vecs0 flags=100 <0>[ 618.138490] i915_sel-5636 3.... 673425493us : execlists_reset_prepare: vecs0: depth<-0 <0>[ 618.138490] i915_sel-5636 3.... 673425493us : intel_engine_stop_cs: vecs0 <0>[ 618.138490] i915_sel-5636 3.... 673425523us : __intel_gt_reset: engine_mask=40 <0>[ 618.138490] i915_sel-5636 3.... 673425568us : execlists_reset: vecs0 <0>[ 618.138490] i915_sel-5636 3d..1 673425568us : process_csb: vecs0 cs-irq head=1, tail=2 <0>[ 618.138490] i915_sel-5636 3d..1 673425568us : process_csb: vecs0 csb[2]: status=0x00000001:0x40000000 <0>[ 618.138490] i915_sel-5636 3d..1 673425569us : trace_ports: vecs0: promote { 14de2:504*, 0:0 } <0>[ 618.138490] i915_sel-5636 3d..1 673425570us : __i915_request_reset: vecs0 rq=14de2:504, guilty? yes <0>[ 618.138490] i915_sel-5636 3d..1 673425571us : __execlists_reset: vecs0 replay {head:2de0, tail:2e48} <0>[ 618.138490] i915_sel-5636 3d..1 673425572us : __i915_request_unsubmit: vecs0 fence 14de2:504, current 503 <0>[ 618.138490] i915_sel-5636 3.... 673435544us : intel_engine_cancel_stop_cs: vecs0 <0>[ 618.138490] i915_sel-5636 3.... 673435544us : process_csb: vecs0 cs-irq head=11, tail=11 <0>[ 618.138490] i915_sel-5636 3d..1 673435545us : __i915_request_submit: vecs0 fence 14de2:504, current 503 <0>[ 618.138490] i915_sel-5636 3d..1 673435546us : __execlists_submission_tasklet: vecs0: queue_priority_hint:-2147483648, submit:yes <0>[ 618.138490] i915_sel-5636 3d..1 673435548us : trace_ports: vecs0: submit { 14de2:504*, 0:0 } <0>[ 618.138490] i915_sel-5636 3.... 673435549us : execlists_reset_finish: vecs0: depth->0 <0>[ 618.138490] ksoftirq-21 2..s. 673435592us : process_csb: vecs0 cs-irq head=11, tail=3 <0>[ 618.138490] ksoftirq-21 2..s. 673435593us : process_csb: vecs0 csb[0]: status=0x00000001:0x40000000 <0>[ 618.138490] ksoftirq-21 2..s. 673435594us : trace_ports: vecs0: promote { 14de2:504*, 0:0 } <0>[ 618.138490] ksoftirq-21 2..s. 673435596us : process_csb: vecs0 csb[1]: status=0x00000018:0x40000040 <0>[ 618.138490] ksoftirq-21 2..s. 673435597us : trace_ports: vecs0: completed { 14de2:504*, 0:0 } <0>[ 618.138490] ksoftirq-21 2..s. 673435612us : process_csb: process_csb:2188 GEM_BUG_ON(!i915_request_completed(*execlists->active) && !reset_in_progress(execlists)) After the reset, we do another clflush before checking the CSB to be sure we see whatever was left in the CSB prior to the reset. So it is unlikely to be an incoherent view of the CSB, and more likely that Icelake didn't reset its pointers. References: 582a6f90 ("drm/i915/execlists: Add a paranoid flush of the CSB pointers upon reset") Signed-off-by: -
Jani Nikula authored
Add another for_each style macro to the pile of custom looping macros. Reviewed-by:
-
Chris Wilson authored
Since the execlists_active() is no longer protected by the engine->active.lock, we need to protect the request pointer with RCU to prevent it being freed as we evaluate whether or not we need to preempt. Fixes: df403069 ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock") Fixes: 13ed13a4 ("drm/i915: Don't set queue_priority_hint if we don't kick the submission") Signed-off-by:
-
Chris Wilson authored
The counter is removed from the pm wakeref count, but it remains intact so that we can restore it upon resume. Ergo inside suspend, it may have a value. Signed-off-by:
Andi Shyti <andi.shyti@intel.com> Reviewed-by:
-
- 02 Nov, 2019 3 commits
-
-
Chris Wilson authored
Filter out warnings for the kernel context that is used to flush inactive contexts, as they do no not pose a risk. Signed-off-by:
-
Chris Wilson authored
Check that the context's ring register state still matches our expectations prior to execution. Signed-off-by:
-
Chris Wilson authored
Flushing the outer i915_active is not enough, as we need the barrier to be applied across all the active dma_fence callbacks. So we must serialise with each outstanding fence. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112096 References: f79520bb ("drm/i915/selftests: Synchronize checking active status with retirement") Signed-off-by:
-
- 01 Nov, 2019 2 commits
-
-
Chris Wilson authored
Avoid drivers/gpu/drm/i915/i915_perf.c:2442:85: warning: dubious: x | !y simply by inverting the predicate and reversing the ternary. v2: Move the long lines into their own function so there is no confusion on operator precedence. Signed-off-by:
Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191101192116.12647-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
Call suspend_prepare first so that we don't leave GuC so confused. Signed-off-by:
-
