- 13 Nov, 2017 7 commits
-
-
Lionel Landwerlin authored
Gen8/9 aren't very different and we can merge some of this code. Signed-off-by:
Lionel Landwerlin <lionel.g.landwerlin@intel.com> Reviewed-by:
Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20171110190845.32574-4-lionel.g.landwerlin@intel.com
-
Lionel Landwerlin authored
We can enable GT3 as well as GT2. Signed-off-by:
-
Lionel Landwerlin authored
We were missing some registers and also can name one for which we only had the offset. Signed-off-by:
-
Maarten Lankhorst authored
Lock the bare minimum, instead of the entire world, and use interruptible locking because we can. Signed-off-by:
Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20171110113503.16253-6-maarten.lankhorst@linux.intel.comReviewed-by:
Daniel Vetter <daniel.vetter@ffwll.ch>
-
Maarten Lankhorst authored
Some parameters use CHECK_BOOL, but should really use CHECK_BOOL_INCOMPLETE. We cannot currently check whether the inherited infoframes and audio are set up correctly. Signed-off-by:
-
Maarten Lankhorst authored
Add PIPE_CONF_CHECK_BOOL for boolean options, which are printed with yesno. Signed-off-by:
-
Maarten Lankhorst authored
The flag just tells us IPS can be enabled, if the primary plane is not enabled it means IPS might not be. This never triggered in CI because we don't have a haswell ULT there, but can be reproduced easily with kms_atomic_transitions.plane-all-modeset-transition Signed-off-by:
-
- 12 Nov, 2017 2 commits
-
-
Oscar Mateo authored
GEN8_CONFIG0 (0xD00) is a protected by a lock (bit 31) which is set by the BIOS, so there is no way we can enable the three chicken bits mandated by the WA (the BIOS should be doing it instead). v2: Rebased v3: Standalone patch References: b033bb6d ("drm/i915/gen9: Enable must set chicken bits in config0 reg") Signed-off-by:
Oscar Mateo <oscar.mateo@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1510185589-9100-2-git-send-email-oscar.mateo@intel.comReviewed-by:
Mika Kuoppala <mika.kuoppala@linux.intel.com> Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk>
-
Chris Wilson authored
Now that we always execute a context switch upon module load, there is no need to queue a delayed task for doing so. The purpose of the delayed task is to enable GT powersaving, for which we need the HW state to be valid (i.e. having loaded a context and initialised basic state). We used to defer this operation as historically it was slow (due to slow register polling, fixed with commit 1758b90e ("drm/i915: Use a hybrid scheme for fast register waits")) but now we have a requirement to save the default HW state. v2: Load the kernel context (to provide the power context) upon resume. Signed-off-by:
Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20171112112738.1463-3-chris@chris-wilson.co.uk
-
- 11 Nov, 2017 1 commit
-
-
Maarten Lankhorst authored
We no longer use intel_crtc->wm.active for watermarks any more, which was incorrect. But this uncovered a bug in sanitize_watermarks(), which meant that we wrote the correct watermarks, but the next update would still use the wrong hw watermarks for calculating. This caused all further updates to fail with -EINVAL and the log would reveal an error like the one below: [ 10.043902] [drm:ilk_validate_wm_level.part.8 [i915]] Sprite WM0 too large 56 (max 0) [ 10.043960] [drm:ilk_validate_pipe_wm [i915]] LP0 watermark invalid [ 10.044030] [drm:intel_crtc_atomic_check [i915]] No valid intermediate pipe watermarks are possible Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com>
-
- 10 Nov, 2017 16 commits
-
-
Chris Wilson authored
Move all of the i915->mm initialisation to a private function that can be reused by the mock i915 device to save forgetting any more steps. For example, <7>[ 1542.046332] [IGT] drv_selftest: starting subtest mock_objects <4>[ 1542.123924] Setting dangerous option mock_selftests - tainting kernel <6>[ 1542.167941] i915: Performing mock selftests with st_random_seed=0x246f5ab5 st_timeout=1000 <4>[ 1542.178012] INFO: trying to register non-static key. <4>[ 1542.178027] the code is fine but needs lockdep annotation. <4>[ 1542.178032] turning off the locking correctness validator. <4>[ 1542.178041] CPU: 3 PID: 6008 Comm: kworker/3:7 Tainted: G U 4.14.0-rc8-CI-CI_DRM_3332+ #1 <4>[ 1542.178049] Hardware name: /NUC6CAYB, BIOS AYAPLCEL.86A.0040.2017.0619.1722 06/19/2017 <4>[ 1542.178144] Workqueue: events __i915_gem_free_work [i915] <4>[ 1542.178152] Call Trace: <4>[ 1542.178163] dump_stack+0x68/0x9f <4>[ 1542.178170] register_lock_class+0x3fd/0x580 <4>[ 1542.178177] ? unwind_next_frame+0x14/0x20 <4>[ 1542.178184] ? __save_stack_trace+0x73/0xd0 <4>[ 1542.178191] __lock_acquire+0xa4/0x1b00 <4>[ 1542.178254] ? __i915_gem_free_work+0x28/0xa0 [i915] <4>[ 1542.178261] ? __lock_acquire+0x4ab/0x1b00 <4>[ 1542.178268] lock_acquire+0xb0/0x200 <4>[ 1542.178273] ? lock_acquire+0xb0/0x200 <4>[ 1542.178336] ? __i915_gem_free_work+0x28/0xa0 [i915] <4>[ 1542.178344] _raw_spin_lock+0x32/0x50 <4>[ 1542.178405] ? __i915_gem_free_work+0x28/0xa0 [i915] <4>[ 1542.178468] __i915_gem_free_work+0x28/0xa0 [i915] <4>[ 1542.178476] process_one_work+0x221/0x650 <4>[ 1542.178483] worker_thread+0x4e/0x3c0 <4>[ 1542.178489] kthread+0x114/0x150 <4>[ 1542.178494] ? process_one_work+0x650/0x650 <4>[ 1542.178499] ? kthread_create_on_node+0x40/0x40 <4>[ 1542.178506] ret_from_fork+0x27/0x40 v2: Fish out i915->mm.object_stat_lock which was being inited over in i915_drv.c (Matthew) Signed-off-by:
-
Chris Wilson authored
Update the kerneldoc parameter name to match the real parameter name. Signed-off-by:
-
Chris Wilson authored
As we now record the default HW state and so only emit the "golden" renderstate once to prepare the HW, there is no advantage in keeping the renderstate batch around as it will never be used again. Signed-off-by:
-
Chris Wilson authored
Take a copy of the HW state after a reset upon module loading by executing a context switch from a blank context to the kernel context, thus saving the default hw state over the blank context image. We can then use the default hw state to initialise any future context, ensuring that each starts with the default view of hw state. v2: Unmap our default state from the GTT after stealing it from the context. This should stop us from accidentally overwriting it via the GTT (and frees up some precious GTT space). Testcase: igt/gem_ctx_isolation Signed-off-by:
-
Chris Wilson authored
In the next few patches, we will want to both copy out of the context image and write a valid image into a new context. To be completely safe, we should then couple in our domain tracking to ensure that we don't have any issues with stale data remaining in unwanted cachelines. Historically, we omitted the .write=true from the call to set-gtt-domain in i915_switch_context() in order to avoid a stall between every request as we would want to wait for the previous context write from the gpu. Since then, we limit the set-gtt-domain to only occur when we first bind the vma, so once in use we will never stall, and we are sure to flush the context following a load from swap. Equally we never applied the lessons learnt from ringbuffer submission to execlists; so time to apply the flush of the lrc after load as well. Signed-off-by:
-
Chris Wilson authored
intel_modeset_gem_init() now only sets up the legacy overlay, so let's remove the function and call the setup directly during driver load. This should help us find a better point in the initialisation sequence for it later. Signed-off-by:
-
Chris Wilson authored
Despite its name intel_init_clock_gating applies both display clock gating workarounds; GT mmio workarounds and the occasional GT power context workaround. Worse, sometimes it includes a context register workaround which we need to apply before we record the default HW state for all contexts. Signed-off-by:
-
Chris Wilson authored
GT powersaving is tightly coupled to the request infrastructure. To avoid complications with the order of initialisation in the next patch (where we want to send requests to hw during GEM init) move the powersaving initialisation into the purview of i915_gem_init(). Signed-off-by:
-
Chris Wilson authored
In the next few patches, we will have a hard requirement that we emit a context-switch to the perma-pinned i915->kernel_context (so that we can save the HW state using that context-switch). As the first context itself may be classed as a kernel context, we want to be explicit in our comparison. For an extra-layer of finesse, we can check the last unretired context on the engine; as well as the last retired context when idle. v2: verbose verbosity v3: Always force the switch, even when the engine is idle, and update the assert that this happens before suspend. Signed-off-by:
-
Tvrtko Ursulin authored
We want to be able to report back to userspace details about an engine's class, and in return for userspace to be able to request actions regarding certain classes of engines. To isolate the uABI from any variations between hw generations, we define an abstract class for the engines and internally map onto the hw. v2: Remove MAX from the uABI; keep it internal if we need it, but don't let userspace make the mistake of using it themselves. v3: s/OTHER/INVALID/ The use of OTHER is ill-defined, so remove it from the uABI as any future new type of engine can define a class to suit it. But keep a reserved value for an invalid class, so that we can always unambiguously express when something doesn't belong to the classification. Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by:
-
Chris Wilson authored
lockdep spotted that the mock tests were using the i915->mm.obj_lock without first initialiasing it: >[ 1303.217043] [IGT] drv_selftest: starting subtest mock_objects <4>[ 1303.240898] Setting dangerous option mock_selftests - tainting kernel <6>[ 1303.253665] i915: Performing mock selftests with st_random_seed=0xd87ea6c6 st_timeout=1000 <4>[ 1303.254812] INFO: trying to register non-static key. <4>[ 1303.254816] the code is fine but needs lockdep annotation. <4>[ 1303.254818] turning off the locking correctness validator. <4>[ 1303.254820] CPU: 4 PID: 13112 Comm: drv_selftest Tainted: G U W 4.14.0-rc8-CI-Patchwork_7058+ #1 <4>[ 1303.254823] Hardware name: MSI MS-7924/Z97M-G43(MS-7924), BIOS V1.12 02/15/2016 <4>[ 1303.254825] Call Trace: <4>[ 1303.254829] dump_stack+0x68/0x9f <4>[ 1303.254832] register_lock_class+0x3fd/0x580 <4>[ 1303.254835] ? ___slab_alloc.constprop.29+0x157/0x3d0 <4>[ 1303.254837] ? ___slab_alloc.constprop.29+0x157/0x3d0 <4>[ 1303.254840] ? sg_kmalloc+0x1e/0x50 <4>[ 1303.254842] ? debug_smp_processor_id+0x17/0x20 <4>[ 1303.254845] __lock_acquire+0xa4/0x1b00 <4>[ 1303.254884] ? __i915_gem_object_set_pages+0x116/0x1f0 [i915] <4>[ 1303.254887] ? __this_cpu_preempt_check+0x13/0x20 <4>[ 1303.254889] ? sg_kmalloc+0x1e/0x50 <4>[ 1303.254891] lock_acquire+0xb0/0x200 <4>[ 1303.254893] ? lock_acquire+0xb0/0x200 <4>[ 1303.254917] ? __i915_gem_object_set_pages+0x116/0x1f0 [i915] <4>[ 1303.254920] _raw_spin_lock+0x32/0x50 <4>[ 1303.254944] ? __i915_gem_object_set_pages+0x116/0x1f0 [i915] <4>[ 1303.254967] __i915_gem_object_set_pages+0x116/0x1f0 [i915] <4>[ 1303.254991] i915_gem_object_get_pages_phys+0x286/0x2b0 [i915] <4>[ 1303.255015] ____i915_gem_object_get_pages+0x20/0x60 [i915] <4>[ 1303.255039] i915_gem_object_attach_phys+0x137/0x1a0 [i915] <4>[ 1303.255063] igt_phys_object+0x45/0x120 [i915] <4>[ 1303.255094] __i915_subtests+0x40/0xd0 [i915] <4>[ 1303.255099] ? work_on_cpu_safe+0x60/0x60 <4>[ 1303.255131] i915_gem_object_mock_selftests+0x34/0x50 [i915] Signed-off-by:
Matthew Auld <matthew.william.auld@gmail.com> Reviewed-by:
-
Chris Wilson authored
So it appears that commit 5427f207 ("drm/i915: Bump wait-times for the final CS interrupt before parking") was a little over optimistic in its belief that it had successfully waited for all residual activity on the engines before parking. Numerous sightings in CI since then of <7>[ 52.542886] [IGT] core_auth: executing <3>[ 52.561013] [drm:intel_engines_park [i915]] *ERROR* vcs0 is not idle before parking <7>[ 52.561215] intel_engines_park vcs0 <7>[ 52.561229] intel_engines_park current seqno 98, last 98, hangcheck 0 [-247449 ms], inflight 0 <7>[ 52.561238] intel_engines_park Reset count: 0 <7>[ 52.561266] intel_engines_park Requests: <7>[ 52.561363] intel_engines_park RING_START: 0x00000000 [0x00000000] <7>[ 52.561377] intel_engines_park RING_HEAD: 0x00000000 [0x00000000] <7>[ 52.561390] intel_engines_park RING_TAIL: 0x00000000 [0x00000000] <7>[ 52.561406] intel_engines_park RING_CTL: 0x00000000 <7>[ 52.561422] intel_engines_park RING_MODE: 0x00000200 [idle] <7>[ 52.561442] intel_engines_park ACTHD: 0x00000000_00000000 <7>[ 52.561459] intel_engines_park BBADDR: 0x00000000_00000000 <7>[ 52.561474] intel_engines_park Execlist status: 0x00000301 00000000 <7>[ 52.561489] intel_engines_park Execlist CSB read 5 [5 cached], write 5 [5 from hws], interrupt posted? no <7>[ 52.561500] intel_engines_park ELSP[0] idle <7>[ 52.561510] intel_engines_park ELSP[1] idle <7>[ 52.561519] intel_engines_park HW active? 0x0 <7>[ 52.561608] intel_engines_park Idle? yes <7>[ 52.561617] intel_engines_park on Braswell, which indicates that the engine just needs that little bit longer after flushing the tasklet to settle. So give it a few more milliseconds before declaring an err and applying the emergency brake. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103479Signed-off-by:
-
Hans de Goede authored
intel_uncore_forcewake_reset() does forcewake puts and gets as such we need to make sure that no-one tries to access the PUNIT->PMIC bus (on systems where this bus is shared) while it runs, otherwise bad things happen. Normally this is taken care of by the i915_pmic_bus_access_notifier() which does an intel_uncore_forcewake_get(FORCEWAKE_ALL) when some other driver tries to access the PMIC bus, so that later forcewake gets are no-ops (for the duration of the bus access). But intel_uncore_forcewake_reset gets called in 3 cases: 1) Before registering the pmic_bus_access_notifier 2) After unregistering the pmic_bus_access_notifier 3) To reset forcewake state on a GPU reset In all 3 cases the i915_pmic_bus_access_notifier() protection is insufficient. This commit fixes this race by calling iosf_mbi_punit_acquire() before calling intel_uncore_forcewake_reset(). In the case where it is called directly after unregistering the pmic_bus_access_notifier, we need to hold the punit-lock over both calls to avoid a race where intel_uncore_fw_release_timer() may execute between the 2 calls. Reviewed-by:
Imre Deak <imre.deak@intel.com> Acked-by:
Ingo Molnar <mingo@kernel.org> Signed-off-by:
Hans de Goede <hdegoede@redhat.com> Link: https://patchwork.freedesktop.org/patch/msgid/20171019111620.26761-3-hdegoede@redhat.com
-
Hans de Goede authored
For race free unregistration drivers may need to acquire PMIC bus access through iosf_mbi_punit_acquire() and then (un)register the notifier without dropping the lock. This commit adds an unlocked variant of iosf_mbi_unregister_pmic_bus_access_notifier for this use case. Acked-by:
-
Chris Wilson authored
In order to allow the mock breadcrumbs tests to run without device irqs being enabled, move the intel_irqs_enabled() assert deeper to just before we commit to enabling the HW irq. v2: Add a FIXME explaining that placing the assertion so deep is not ideal, but a compromise for mock breadcrumbs. Signed-off-by:
-
Chris Wilson authored
Originally it was anticipated that timeouts would be a rare event, and so merit a warning that the test was incomplete. However, for igt we keep the timeout low, and hitting the timeout is intentional. It no longer necessitates a warning, but to be expected. Signed-off-by:
-
- 09 Nov, 2017 14 commits
-
-
Rodrigo Vivi authored
Signed-off-by:Rodrigo Vivi <rodrigo.vivi@intel.com>
-
Rodrigo Vivi authored
Signed-off-by: -
Chris Wilson authored
Whenever we want to unbind a vma, we must wait on all GPU activity to complete first. (This is what gives us the ability to do fine grained eviction and purging by only having to wait on the VMA that we need to unbind to proceed; though if pushed we can make it a rule that we are only allowed to unbind already idle VMA and move the burden of the work and organising the sleep onto the caller.) Currently, we might only sleep if the vma is still active on the GPU, but in principle i915_vma_unbind() always implies a sleep, so mark it up with a might_sleep(). Suggested-by:
-
Chris Wilson authored
vm_free_page() may call down into set_pages_array_wb() (which itself sleeps, on x86 at least) but only if on !llc and the caches overflow. Since this is unlikely, we only rarely trigger the error in practice, and so to make CI detection of this sleeping bug possible we want to mark the common vm_free_page() as a potential sleep. Suggested-by:
-
Chris Wilson authored
Trying to enable printk debugging for GEM is fraught with the issue of spam; interactions with HW are very frequent and often boring. However, one instance where they are not so boring is just before a BUG; here ftrace provides a facility to dump its ringbuffer on an oops. So for CI let's enable trace_printk() to capture the last exchanges with HW as a death rattle. For example, [ 79.234110] ------------[ cut here ]------------ [ 79.234137] kernel BUG at drivers/gpu/drm/i915/intel_lrc.c:907! [ 79.234145] invalid opcode: 0000 [#1] SMP [ 79.234153] Dumping ftrace buffer: [ 79.234158] --------------------------------- ... [ 79.314044] gem_conc-1059 1..s1 79203443us : intel_lrc_irq_handler: bcs0 out[0]: ctx=5.2, seqno=145 [ 79.314089] gem_conc-1059 1..s. 79220800us : intel_lrc_irq_handler: bcs0 csb[1/1]: status=0x00000018:0x00000005 [ 79.314133] gem_conc-1059 1..s. 79220803us : intel_lrc_irq_handler: bcs0 out[0]: ctx=5.1, seqno=145 [ 79.314177] gem_conc-1062 2..s1 79230458us : intel_lrc_irq_handler: bcs0 in[0]: ctx=8.1, seqno=146 [ 79.314220] gem_conc-1062 2..s1 79230515us : intel_lrc_irq_handler: bcs0 in[0]: ctx=8.2, seqno=147 [ 79.314265] gem_conc-1059 1..s1 79230951us : intel_lrc_irq_handler: bcs0 csb[2/3]: status=0x00000012:0x00000008 [ 79.314309] gem_conc-1059 1..s1 79230954us : intel_lrc_irq_handler: bcs0 out[0]: ctx=8.2, seqno=147 [ 79.314353] gem_conc-1059 1..s1 79230954us : intel_lrc_irq_handler: bcs0 csb[3/3]: status=0x00008002:0x00000008 [ 79.314396] gem_conc-1059 1..s1 79230955us : intel_lrc_irq_handler: bcs0 out[0]: ctx=8.1, seqno=147 [ 79.314402] --------------------------------- v2: Tweak the formatting to be more consistent between in/out. v3: do {} while (0) stub macro protection Suggested-by:Michał Winiarski <michal.winiarski@intel.com> Signed-off-by:
-
Ville Syrjälä authored
Eliminate a ton of pointless 'dev' variables in the DP code, and pass around 'dev_priv' instead of 'dev'. v2: Rebase Acked-by:
Jani Nikula <jani.nikula@intel.com> Reviewed-by:
Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Signed-off-by:
-
Ville Syrjälä authored
No need to pass 'dev' or 'dev_priv' when the function already takes 'intel_dp'. Also let's prefer passing 'dev_priv' instead of 'dev' when we have to pass one or the other. Signed-off-by:
-
Ville Syrjälä authored
Remove intel_digital_port->port and replace its users with intel_encoder->port. intel_encoder->port is a superset of intel_digital_port->port, and it works correctly even for MST encoders. v2: Eliminate a few dp_to_dig_port()->base.port cases too (DK) Performed with cocci: @@ @@ struct intel_digital_port { ... - enum port port; ... } @@ struct intel_digital_port *D; expression E; @@ - D->port = E; @@ struct intel_digital_port *D; @@ - D->port + D->base.port @ expression E; @@ ( - dp_to_dig_port(E)->port + dp_to_dig_port(E)->base.port | - enc_to_dig_port(E)->port + to_intel_encoder(E)->port ) @@ expression E; @@ - to_intel_encoder(&E->base) + E @@ struct intel_digital_port *D; identifier I, M; @@ I = &D->base <... ( - D->base.M + I->M | - &D->base + I ) ...> @@ identifier D; expression E; identifier M; @@ D = enc_to_dig_port(&E->base) <... ( - D->base.M + E->M | - &D->base + E ) ...> @@ identifier D, DP; expression E; identifier M; @@ DP = enc_to_intel_dp(&E->base) <... ( - dp_to_dig_port(DP)->base.M + E->M | - &dp_to_dig_port(DP)->base + E ) ...> @@ expression E; identifier M; @@ ( - enc_to_dig_port(&E->base)->base.M + E->M | - enc_to_dig_port(&E->base)->base + E | - enc_to_mst(&E->base)->primary->base.port + E->port ) @@ expression E; identifier D; @@ - struct intel_digital_port *D = E; ... when != D Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Reviewed-by: -
Ville Syrjälä authored
Replace dig_port->port with encoder->port in the BXT DPLL selection. We can do this because both the master encoder and the fake MST encoders have the same encoder->port value, whereas using dig_port->port only worked for the master encoder since the fake encoders were't derived from intel_digital_port. This eliminates the DP MST special case. Do this by hand because spatch is having problems with the control flow due to the dig_port assignment happening in two different branches. Signed-off-by:
-
Ville Syrjälä authored
Replace crtc->config usage with the passed down crtc state. Also take the opportunity for some s/pipe_config/crtc_state/ while at it. Signed-off-by:
-
Ville Syrjälä authored
Rather than digging through encoder->crtc and crtc->config in the DPIO PHY functions, pass down the correct crtc state from the caller. Signed-off-by:
-
Ville Syrjälä authored
Extract the current crtc from the crtc state rather than via the legacy encoder->crtc pointer whenever possible. Signed-off-by:
-
Ville Syrjälä authored
Extract the current crtc from the crtc state rather than via the legacy encoder->crtc pointer whenever possible. Signed-off-by:
-
Ville Syrjälä authored
Extract the current crtc from the crtc state rather than via the legacy encoder->crtc pointer whenever possible. Signed-off-by:
-
