OSDN Git Service
Maarten Lankhorst [Fri, 10 Nov 2017 11:34:53 +0000 (12:34 +0100)]
drm/i915: Update watermark state correctly in sanitize_watermarks
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: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes:
b6b178a77210 ("drm/i915: Calculate ironlake intermediate watermarks correctly, v2.")
Cc: stable@vger.kernel.org #v4.8+
Link: https://patchwork.freedesktop.org/patch/msgid/20171110113503.16253-1-maarten.lankhorst@linux.intel.com
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Chris Wilson [Fri, 10 Nov 2017 23:24:47 +0000 (23:24 +0000)]
drm/i915/selftests: Yet another forgotten mock_i915->mm initialiser
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 <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171110232447.21618-1-chris@chris-wilson.co.uk
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Chris Wilson [Thu, 9 Nov 2017 14:06:44 +0000 (14:06 +0000)]
drm/i915: Kerneldoc typo s/rps/rps_client/
Update the kerneldoc parameter name to match the real parameter name.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20171109140644.10805-1-chris@chris-wilson.co.uk
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Chris Wilson [Fri, 10 Nov 2017 14:26:34 +0000 (14:26 +0000)]
drm/i915: Stop caching the "golden" renderstate
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 <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171110142634.10551-8-chris@chris-wilson.co.uk
Chris Wilson [Fri, 10 Nov 2017 14:26:33 +0000 (14:26 +0000)]
drm/i915: Record the default hw state after reset upon load
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 <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171110142634.10551-7-chris@chris-wilson.co.uk
Chris Wilson [Fri, 10 Nov 2017 14:26:32 +0000 (14:26 +0000)]
drm/i915: Mark the context state as dirty/written
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 <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171110142634.10551-6-chris@chris-wilson.co.uk
Chris Wilson [Fri, 10 Nov 2017 14:26:31 +0000 (14:26 +0000)]
drm/i915: Inline intel_modeset_gem_init()
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 <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171110142634.10551-5-chris@chris-wilson.co.uk
Chris Wilson [Fri, 10 Nov 2017 14:26:30 +0000 (14:26 +0000)]
drm/i915: Move intel_init_clock_gating() to i915_gem_init()
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 <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171110142634.10551-4-chris@chris-wilson.co.uk
Chris Wilson [Fri, 10 Nov 2017 14:26:29 +0000 (14:26 +0000)]
drm/i915: Move GT powersaving init to i915_gem_init()
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 <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171110142634.10551-3-chris@chris-wilson.co.uk
Chris Wilson [Fri, 10 Nov 2017 14:26:28 +0000 (14:26 +0000)]
drm/i915: Force the switch to the i915->kernel_context
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: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> #v1
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171110142634.10551-2-chris@chris-wilson.co.uk
Tvrtko Ursulin [Fri, 10 Nov 2017 14:26:27 +0000 (14:26 +0000)]
drm/i915: Define an engine class enum for the uABI
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 <chris@chris-wilson.co.uk>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> #v2
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171110142634.10551-1-chris@chris-wilson.co.uk
Chris Wilson [Fri, 10 Nov 2017 15:19:19 +0000 (15:19 +0000)]
drm/i915/selftests: Initialise mock_i915->mm.obj_lock
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: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171110151919.18451-1-chris@chris-wilson.co.uk
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Chris Wilson [Fri, 10 Nov 2017 11:25:50 +0000 (11:25 +0000)]
drm/i915: Restore the wait for idle engine after flushing interrupts
So it appears that commit
5427f207852d ("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=103479
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171110112550.28909-1-chris@chris-wilson.co.uk
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Hans de Goede [Thu, 19 Oct 2017 11:16:20 +0000 (13:16 +0200)]
drm/i915: Acquire PUNIT->PMIC bus for intel_uncore_forcewake_reset()
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 [Thu, 19 Oct 2017 11:16:19 +0000 (13:16 +0200)]
x86/platform/intel/iosf_mbi: Add unlocked PMIC bus access notifier unregister
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: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171019111620.26761-2-hdegoede@redhat.com
Chris Wilson [Tue, 7 Nov 2017 10:20:03 +0000 (10:20 +0000)]
drm/i915: Move irqs enabled assertion deeper for mock breadcrumbs
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 <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171107102003.1802-1-chris@chris-wilson.co.uk
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Chris Wilson [Fri, 10 Nov 2017 10:11:10 +0000 (10:11 +0000)]
drm/i915/selftests: Reduce the volume of the timeout message
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: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171110101110.12042-1-chris@chris-wilson.co.uk
Reviwed-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Rodrigo Vivi [Thu, 9 Nov 2017 23:30:06 +0000 (15:30 -0800)]
drm/i915: Update DRIVER_DATE to
20171109
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Rodrigo Vivi [Thu, 9 Nov 2017 23:18:04 +0000 (15:18 -0800)]
drm/i915: Update DRIVER_DATE to
20171109
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Chris Wilson [Thu, 9 Nov 2017 21:34:50 +0000 (21:34 +0000)]
drm/i915: Mark up i915_vma_unbind() as a potential sleeper
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: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=103638
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171109213450.13875-2-chris@chris-wilson.co.uk
Chris Wilson [Thu, 9 Nov 2017 21:34:49 +0000 (21:34 +0000)]
drm/i915: Mark vm_free_page() as a potential sleeper agent
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: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=103638
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20171109213450.13875-1-chris@chris-wilson.co.uk
Chris Wilson [Thu, 9 Nov 2017 14:30:19 +0000 (14:30 +0000)]
drm/i915: Use trace_printk to provide a death rattle for GEM
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: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171109143019.16568-1-chris@chris-wilson.co.uk
Ville Syrjälä [Thu, 9 Nov 2017 15:27:58 +0000 (17:27 +0200)]
drm/i915: Clean up DP code local variables and calling conventions
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ä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171109152758.32257-1-ville.syrjala@linux.intel.com
Ville Syrjälä [Tue, 31 Oct 2017 20:51:22 +0000 (22:51 +0200)]
drm/i915: Clean up PPS code calling conventions
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ä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171031205123.13123-10-ville.syrjala@linux.intel.com
Acked-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Ville Syrjälä [Thu, 9 Nov 2017 15:24:34 +0000 (17:24 +0200)]
drm/i915: Nuke intel_digital_port->port
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: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171109152434.32074-1-ville.syrjala@linux.intel.com
Ville Syrjälä [Tue, 31 Oct 2017 20:51:20 +0000 (22:51 +0200)]
drm/i915: Replace dig_port->port with encoder port for BXT DPLL selection
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ä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171031205123.13123-8-ville.syrjala@linux.intel.com
Acked-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Ville Syrjälä [Tue, 31 Oct 2017 20:51:19 +0000 (22:51 +0200)]
drm/i915: Eliminate crtc->config usage from CRT code
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ä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171031205123.13123-7-ville.syrjala@linux.intel.com
Acked-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Ville Syrjälä [Tue, 31 Oct 2017 20:51:18 +0000 (22:51 +0200)]
drm/i915: Pass crtc state to DPIO PHY functions
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ä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171031205123.13123-6-ville.syrjala@linux.intel.com
Acked-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Ville Syrjälä [Tue, 31 Oct 2017 20:51:17 +0000 (22:51 +0200)]
drm/i915: Eliminate some encoder->crtc usage from TV code
Extract the current crtc from the crtc state rather than via
the legacy encoder->crtc pointer whenever possible.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171031205123.13123-5-ville.syrjala@linux.intel.com
Acked-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Ville Syrjälä [Tue, 31 Oct 2017 20:51:16 +0000 (22:51 +0200)]
drm/i915: Eliminate some encoder->crtc usage from SDVO code
Extract the current crtc from the crtc state rather than via
the legacy encoder->crtc pointer whenever possible.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171031205123.13123-4-ville.syrjala@linux.intel.com
Acked-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Ville Syrjälä [Tue, 31 Oct 2017 20:51:15 +0000 (22:51 +0200)]
drm/i915: Eliminate some encoder->crtc usage from DSI code
Extract the current crtc from the crtc state rather than via
the legacy encoder->crtc pointer whenever possible.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171031205123.13123-3-ville.syrjala@linux.intel.com
Acked-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Ville Syrjälä [Tue, 31 Oct 2017 20:51:14 +0000 (22:51 +0200)]
drm/i915: Eliminate some encoder->crtc usage from DP code
Extract the current crtc from the crtc state rather than via
the legacy encoder->crtc pointer whenever possible.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171031205123.13123-2-ville.syrjala@linux.intel.com
Acked-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Chris Wilson [Thu, 9 Nov 2017 08:55:40 +0000 (08:55 +0000)]
drm/i915: Reorder context-close to avoid calling i915_vma_close() under RCU
When we close the VMA, we unbind it from the ppgtt and tear down the
page directory pointing at it. That may trigger us to return WC pages
back to the system, requiring conversion back to WB which itself may
sleep. That makes i915_vma_close() unsuitable for use inside the RCU
read lock, which we need to hold to iterate the radixtree.
The fix is quite simple, we can close all the VMA as we close the ppgtt,
we only need to do that instead of closing them during destruction of
the LUT.
v2: Order between closing the LUT and the ppgtt is important; we use the
vma inside the LUT as a means of retrieving the object, and so we must
clear the LUT before freeing the VMA when closing the ppgtt.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103638
Fixes:
547da76b5777 ("drm/i915: Hold rcu_read_lock when iterating over the radixtree (vma idr)")
Fixes:
d1b48c1e7184 ("drm/i915: Replace execbuf vma ht with an idr")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171109085540.32264-1-chris@chris-wilson.co.uk
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Michal Wajdeczko [Fri, 3 Nov 2017 15:18:15 +0000 (15:18 +0000)]
drm/i915/guc: Simplify programming of GUC_SHIM_CONTROL
We can program GUC_SHIM_CONTROL register with all expected
bits without use of extra macro defined in fwif.h
v2: rebased without pre-prod code
v3: fixed typo
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171103151816.62048-4-michal.wajdeczko@intel.com
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Michal Wajdeczko [Fri, 3 Nov 2017 15:18:14 +0000 (15:18 +0000)]
drm/i915/guc: Drop legacy workarounds from guc_prepare_xfer
We don't keep the workarounds for pre-production hardware
(see intel_detect_preproduction_hw) thus we can drop some
extra steps during firmware upload needed only for unsupported
platforms.
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171103151816.62048-3-michal.wajdeczko@intel.com
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Michal Wajdeczko [Fri, 3 Nov 2017 15:18:13 +0000 (15:18 +0000)]
drm/i915/guc: Wait for ucode DMA transfer completion
We silently assumed that DMA transfer will be completed
within assumed timeout and thus we were waiting only at
last step for GuC to become ready. Add intermediate wait
to catch unexpected delays in DMA transfer.
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171103151816.62048-2-michal.wajdeczko@intel.com
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Michal Wajdeczko [Fri, 3 Nov 2017 15:18:12 +0000 (15:18 +0000)]
drm/i915/guc: Split GuC firmware xfer function into clear steps
Transfer of GuC firmware requires few steps that currently
are implemented in two large functions. Split this code into
smaller functions to make these steps small and clear.
Also be prepared for potential DMA xfer step failure.
v2: rename function steps (Sagar)
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171103151816.62048-1-michal.wajdeczko@intel.com
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Rafael Antognolli [Fri, 3 Nov 2017 18:30:27 +0000 (11:30 -0700)]
drm/i915: Implement ReadHitWriteOnlyDisable.
The workaround for this is described as:
"if RenderSurfaceState.Num_Multisamples > 1, disable RCC clock gating if
RenderSurfaceState.Num_Multisamples == 1, set 0x7010[14] = 1"
Further documentation in the internal bug referenced by the bspec
suggest that any of the above suggestions should suffice to fix the
issue. We are going with disabling RCC clock gating.
Unfortunately, what we are doing doesn't match the name of the
workaround, but at least it matches its description.
This change improves CNL stability by avoiding some of the hangs seen in
the platform.
v2: Only disable RCC clock gating.
Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171103183027.5051-1-rafael.antognolli@intel.com
Ville Syrjälä [Wed, 8 Nov 2017 13:35:55 +0000 (15:35 +0200)]
drm/i915: Move init_clock_gating() back to where it was
Apparently setting up a bunch of GT registers before we've properly
initialized the rest of the GT hardware leads to these setting being
lost. So looks like I broke HSW with commit
b7048ea12fbb ("drm/i915:
Do .init_clock_gating() earlier to avoid it clobbering watermarks")
by doing init_clock_gating() too early. This should actually affect
other platforms as well, but apparently not to such a great degree.
What I was ultimately after in that commit was to move the
ilk_init_lp_watermarks() call earlier. So let's undo the damage and
move init_clock_gating() back to where it was, and call
ilk_init_lp_watermarks() just before the watermark state readout.
This highlights how fragile and messed up our init order really is.
I wonder why we even initialize the display before gem. The opposite
order would make much more sense to me...
v2: Keep WaRsPkgCStateDisplayPMReq:hsw early as it really must
be done before all planes might get disabled.
Cc: stable@vger.kernel.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mark Janes <mark.a.janes@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reported-by: Mark Janes <mark.a.janes@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103549
Fixes:
b7048ea12fbb ("drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks")
References: https://lists.freedesktop.org/archives/intel-gfx/2017-November/145432.html
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171108133555.14091-1-ville.syrjala@linux.intel.com
Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Chris Wilson [Tue, 7 Nov 2017 22:06:56 +0000 (22:06 +0000)]
drm/i915: Prune the reservation shared fence array
The shared fence array is not autopruning and may continue to grow as an
object is shared between new timelines. Take the opportunity when we
think the object is idle (we have to confirm that any external fence is
also signaled) to decouple all the fences.
We apply a similar trick after waiting on an object, see commit
e54ca9774777 ("drm/i915: Remove completed fences after a wait")
v2: No longer need to handle the batch pool as a special case.
v3: Need to trylock from within i915_vma_retire as this may be called
form the shrinker - and we may later try to allocate underneath the
reservation lock, so a deadlock is possible.
References: https://bugs.freedesktop.org/show_bug.cgi?id=102936
Fixes:
d07f0e59b2c7 ("drm/i915: Move GEM activity tracking into a common struct reservation_object")
Fixes:
80b204bce8f2 ("drm/i915: Enable multiple timelines")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171107220656.5020-1-chris@chris-wilson.co.uk
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Chris Wilson [Wed, 8 Nov 2017 09:44:00 +0000 (09:44 +0000)]
drm/i915: Idle the GPU before shinking everything
The handling of contexts are peculiar. Instead of tieing their vma to
activity, we pin the context. This means that we cannot simply unbind
the context object itself at will (which would normally cause us to wait
for the vma to be idle), but must manually idle the GPU and retire
requests first.
A consequence of this peculiarity is when doing a last desperate attempt
to recover memory. If the memory is tied up inside active context
objects, we will fail to recover any memory simply by trying to unbind
the objects without first doing a wait-for-idle.
A side-effect of removing the call to shrinker_lock_uninterruptible()
from i915_gem_shrinker_oom() was that we removed an unlocked
wait-for-idle, and so lost the "natural" shrinkage of context objects.
By replacing that with a locked wait from inside i915_gem_shrink(), we
not only replace it with the ability to recover all context objects, but
do so for all i915_gem_shrink_all() callers.
v2: Switching requires request allocation, which is not permitted from
inside the shrinker as it only uses ordinary allocations.
References: https://bugs.freedesktop.org/show_bug.cgi?id=102936
Fixes:
f2123818ffad ("drm/i915: Move dev_priv->mm.[un]bound_list to its own lock")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171108094400.1386-1-chris@chris-wilson.co.uk
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Chris Wilson [Tue, 7 Nov 2017 15:22:11 +0000 (15:22 +0000)]
drm/i915: Include intel_engine_is_idle() status in engine pretty-printer
Upon parking, if we discover an active engine we dump its state. Follow
that state with an indication of whether the engine was idle.
References: https://bugs.freedesktop.org/show_bug.cgi?id=103479
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171107152211.19930-1-chris@chris-wilson.co.uk
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Chris Wilson [Sun, 5 Nov 2017 13:49:05 +0000 (13:49 +0000)]
drm/i915: Read ilk FDI PLL frequency once during initialisation
During intel_atomic_check(), we do not take the intel_runtime_pm_get()
wakeref and so should do the atomic modeset precalculations without
referring to the HW. However, on Ironlake we see
<7>[ 23.487557] [drm:intel_atomic_check [i915]] [CONNECTOR:47:VGA-1] checking for sink bpp constrains
<7>[ 23.487615] [drm:intel_atomic_check [i915]] clamping display bpp (was 36) to default limit of 24
<4>[ 23.487621] RPM wakelock ref not held during HW access
<4>[ 23.487652] ------------[ cut here ]------------
<4>[ 23.487697] WARNING: CPU: 0 PID: 1343 at drivers/gpu/drm/i915/intel_drv.h:1813 gen5_read32+0x183/0x200 [i915]
<4>[ 23.487701] Modules linked in: snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic i915 intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul snd_hda_intel ghash_clmulni_intel snd_hda_codec snd_hwdep snd_hda_core snd_pcm lpc_ich e1000e mei_me ptp mei pps_core prime_numbers
<4>[ 23.487784] CPU: 0 PID: 1343 Comm: debugfs_test Tainted: G W 4.14.0-rc7-CI-Trybot_1378+ #1
<4>[ 23.487788] Hardware name: Hewlett-Packard HP Compaq 8100 Elite SFF PC/304Ah, BIOS 786H1 v01.13 07/14/2011
<4>[ 23.487793] task:
ffff8801f90aa6c0 task.stack:
ffffc900013ec000
<4>[ 23.487838] RIP: 0010:gen5_read32+0x183/0x200 [i915]
<4>[ 23.487842] RSP: 0018:
ffffc900013efb58 EFLAGS:
00010292
<4>[ 23.487849] RAX:
000000000000002a RBX:
ffff880205c00000 RCX:
0000000000000006
<4>[ 23.487854] RDX:
000000000000140a RSI:
ffffffff81d0eb14 RDI:
ffffffff81cc26f6
<4>[ 23.487857] RBP:
ffffc900013efb80 R08:
ffff8801f90aaff8 R09:
0000000000000000
<4>[ 23.487861] R10:
0000000000000000 R11:
0000000000000000 R12:
0000000000000001
<4>[ 23.487865] R13:
0000000000046000 R14:
ffff88020ffaba78 R15:
ffff88020b109bf8
<4>[ 23.487870] FS:
00007f53b5e40a40(0000) GS:
ffff88021bc00000(0000) knlGS:
0000000000000000
<4>[ 23.487874] CS: 0010 DS: 0000 ES: 0000 CR0:
0000000080050033
<4>[ 23.487878] CR2:
000055e41900c0e8 CR3:
00000001fa0d6005 CR4:
00000000000206f0
<4>[ 23.487882] Call Trace:
<4>[ 23.487931] intel_atomic_check+0x745/0x1290 [i915]
<4>[ 23.487948] drm_atomic_check_only+0x459/0x560
<4>[ 23.487956] ? drm_atomic_set_crtc_for_connector+0xc9/0x100
<4>[ 23.488025] drm_atomic_commit+0x18/0x50
<4>[ 23.488035] restore_fbdev_mode_atomic+0x190/0x1f0
<4>[ 23.488059] restore_fbdev_mode+0x32/0x120
<4>[ 23.488072] drm_fb_helper_restore_fbdev_mode_unlocked+0x50/0xa0
<4>[ 23.488139] intel_fbdev_restore_mode+0x34/0x90 [i915]
<4>[ 23.488194] i915_driver_lastclose+0xe/0x10 [i915]
<4>[ 23.488208] drm_lastclose+0x39/0xf0
<4>[ 23.488219] drm_release+0x30c/0x3c0
<4>[ 23.488236] __fput+0xb9/0x200
<4>[ 23.488252] ____fput+0xe/0x10
<4>[ 23.488264] task_work_run+0x89/0xc0
<4>[ 23.488278] exit_to_usermode_loop+0x83/0x90
<4>[ 23.488290] syscall_return_slowpath+0xd0/0x110
<4>[ 23.488304] entry_SYSCALL_64_fastpath+0xaf/0xb1
<4>[ 23.488312] RIP: 0033:0x7f53b4317560
<4>[ 23.488320] RSP: 002b:
00007ffca7e70748 EFLAGS:
00000246 ORIG_RAX:
0000000000000003
<4>[ 23.488333] RAX:
0000000000000000 RBX:
0000000000000001 RCX:
00007f53b4317560
<4>[ 23.488340] RDX:
0000000000000005 RSI:
00007ffca7e70640 RDI:
0000000000000004
<4>[ 23.488347] RBP:
000055e417783900 R08:
000055e418f9e290 R09:
0000000000000000
<4>[ 23.488356] R10:
0000000000000000 R11:
0000000000000246 R12:
0000000000000001
<4>[ 23.488363] R13:
00007f53b4302c40 R14:
0000000000000000 R15:
0000000000000000
<4>[ 23.488384] Code: b5 f2 f2 e0 0f ff e9 c5 fe ff ff 80 3d 0e 4b 10 00 00 0f 85 c6 fe ff ff 48 c7 c7 30 73 29 a0 c6 05 fa 4a 10 00 01 e8 8e f2 f2 e0 <0f> ff e9 ac fe ff ff e8 51 9d f3 e0 85 c0 0f 85 01 ff ff ff 48
<4>[ 23.488780] ---[ end trace
6bc72ce7f1596190 ]---
<7>[ 23.488844] [drm:intel_atomic_check [i915]] checking fdi config on pipe A, lanes 1
<7>[ 23.488911] [drm:intel_atomic_check [i915]] hw max bpp: 36, pipe bpp: 24, dithering: 0
due to intel_fdi_link_freq() poking at FDI_PLL_BIOS_0. Avoid this by
recording the fdi pll frequency during device initiailisation.
v2: Also extract the static FDI PLL frequencies for Sandybridge and
Ivybridge.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171107214713.18704-1-chris@chris-wilson.co.uk
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Chris Wilson [Tue, 7 Nov 2017 11:56:53 +0000 (11:56 +0000)]
drm/i915/selftests: Take rpm wakeref around partial tiling tests
Since the partial tiling tests are poking into the GGTT to watch the
fence registers in operation, it itself needs the device rpm wakeref in
order for the GGTT to remain accessible.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171107115653.10716-1-chris@chris-wilson.co.uk
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Chris Wilson [Tue, 7 Nov 2017 11:40:51 +0000 (11:40 +0000)]
drm/i915/selftests: Take rpm wakeref around GGTT lowlevel tests
The vma routines are responsible for acquiring the device rpm wakeref
before they poke the HW. However, some of the selftests bypass the
higher level vma routines in order to poke directly at the lowlevel GGTT
functions; these are then responsible for managing rpm themselves.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171107114051.10583-1-chris@chris-wilson.co.uk
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Chris Wilson [Tue, 7 Nov 2017 11:05:59 +0000 (11:05 +0000)]
drm/i915/selftests: Skip mixed page exhaustion if only small pages available
If we only have 4k pages, we can't mix together different combinations
of hugepages to see if the world burns. However, as the loops did
nothing, we never set err to 0 and reported ENODEV aborting the test.
Teach the test to skip instead.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171107110559.6098-1-chris@chris-wilson.co.uk
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
Chris Wilson [Tue, 7 Nov 2017 14:03:38 +0000 (14:03 +0000)]
drm/i915: Prevent unbounded wm results in g4x_compute_wm()
Smatch warns of
drivers/gpu/drm/i915/intel_pm.c:1161 g4x_compute_wm() warn: signedness bug returning '(-
33554430)'
which is a result of it believing that wm may be INT_MAX following
g4x_tlb_miss_wa(). Just declaring g4x_tlb_miss_wa() as returning an
unsigned integer is not sufficient, we need to tell smatch that wm itself
is unsigned for it to not worry. So mark up the locals we expect to be
non-negative, and so silence smatch.
v2: Mark up vlv_compute_wm_level() as unsigned similarly.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> #v1
Link: https://patchwork.freedesktop.org/patch/msgid/20171107140338.13748-1-chris@chris-wilson.co.uk
Chris Wilson [Tue, 7 Nov 2017 13:53:24 +0000 (13:53 +0000)]
drm/i915: Simplify onion for bxt_ddi_phy_init()
Older compilers (gcc-4.9) are not as able to track uninitialised
variables as well as more recent compilers. In particular,
drivers/gpu/drm/i915/intel_dpio_phy.c: In function ‘bxt_ddi_phy_init’:
drivers/gpu/drm/i915/intel_dpio_phy.c:482:25: warning: ‘was_enabled’ may be used uninitialized in this function [-Wmaybe-uninitialized]
In this case, we can rearrange code slightly to make the control flow
clearer to the reader, as well as the compiler. That is we only call
uninit using the same predicate as calling init
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171107135324.28300-1-chris@chris-wilson.co.uk
Reviewed-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
Chris Wilson [Tue, 7 Nov 2017 14:53:34 +0000 (14:53 +0000)]
drm/i915: Silence compiler for csr_load_work_fn()
gcc-4.7 is not very smart and can not tell that "si" is guarded by size
being 0. So it complains,
drivers/gpu/drm/i915/intel_csr.c: In function ‘csr_load_work_fn’:
drivers/gpu/drm/i915/intel_csr.c:204:3: warning: ‘si’ may be used uninitialized in this function [-Wmaybe-uninitialized]
drivers/gpu/drm/i915/intel_csr.c:190:30: note: ‘si’ was declared in
Give in and mark si as NULL.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171107145334.27154-1-chris@chris-wilson.co.uk
Reviewed-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Chris Wilson [Tue, 7 Nov 2017 15:40:55 +0000 (15:40 +0000)]
drm/i915: Silence smatch for cmdparser
drivers/gpu/drm/i915/i915_cmd_parser.c:808:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:811:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:814:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:808:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:811:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:814:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:808:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:811:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:814:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:808:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:811:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:814:23: error: not an lvalue
If we move the shift into each case not only do we kill the warning from
smatch, but we shrink the code slightly:
text data bss dec hex filename
1267906 20587 3168
1291661 13b58d before
1267890 20587 3168
1291645 13b57d after
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171107154055.19460-1-chris@chris-wilson.co.uk
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
Chris Wilson [Mon, 6 Nov 2017 21:11:28 +0000 (21:11 +0000)]
drm/i915: Deconstruct struct sgt_dma initialiser
gcc-4.4 complains about:
struct sgt_dma iter = {
.sg = vma->pages->sgl,
.dma = sg_dma_address(iter.sg),
.max = iter.dma + iter.sg->length,
};
drivers/gpu/drm/i915/i915_gem_gtt.c: In function ‘gen8_ppgtt_insert_4lvl’:
drivers/gpu/drm/i915/i915_gem_gtt.c:938: error: ‘iter.sg’ is used uninitialized in this function
drivers/gpu/drm/i915/i915_gem_gtt.c:939: error: ‘iter.dma’ is used uninitialized in this function
and worse generates invalid code that triggers a GPF:
BUG: unable to handle kernel NULL pointer dereference at
0000000000000010
IP: gen8_ppgtt_insert_4lvl+0x1b/0x1e0 [i915]
PGD 0
Oops: 0000 [#1] SMP
Modules linked in: snd_aloop nf_conntrack_ipv6 nf_defrag_ipv6 nf_log_ipv6 ip6table_filter ip6_tables ctr ccm xt_state nf_log_ipv4
nf_log_common xt_LOG xt_limit xt_recent xt_owner xt_addrtype iptable_filter ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat
nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack libcrc32c ip_tables dm_mod vhost_net macvtap macvlan vhost tun kvm_intel kvm
irqbypass uas usb_storage hid_multitouch btusb btrtl uvcvideo videobuf2_v4l2 videobuf2_core videodev media videobuf2_vmalloc videobuf2_memops
sg ppdev dell_wmi sparse_keymap mei_wdt sd_mod iTCO_wdt iTCO_vendor_support rtsx_pci_ms memstick rtsx_pci_sdmmc mmc_core dell_smm_hwmon hwmon
dell_laptop dell_smbios dcdbas joydev input_leds hci_uart btintel btqca btbcm bluetooth parport_pc parport i2c_hid
intel_lpss_acpi intel_lpss pcspkr wmi int3400_thermal acpi_thermal_rel dell_rbtn mei_me mei snd_hda_codec_hdmi snd_hda_codec_realtek
snd_hda_codec_generic ahci libahci acpi_pad xhci_pci xhci_hcd snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_seq snd_seq_device
snd_pcm snd_timer snd soundcore int3403_thermal arc4 e1000e ptp pps_core i2c_i801 iwlmvm mac80211 rtsx_pci iwlwifi cfg80211 rfkill
intel_pch_thermal processor_thermal_device int340x_thermal_zone intel_soc_dts_iosf i915 video fjes
CPU: 2 PID: 2408 Comm: X Not tainted 4.10.0-rc5+ #1
Hardware name: Dell Inc. Latitude E7470/0T6HHJ, BIOS 1.11.3 11/09/2016
task:
ffff880219fe4740 task.stack:
ffffc90005f98000
RIP: 0010:gen8_ppgtt_insert_4lvl+0x1b/0x1e0 [i915]
RSP: 0018:
ffffc90005f9b8c8 EFLAGS:
00010246
RAX:
0000000000000000 RBX:
ffff8802167d8000 RCX:
0000000000000001
RDX:
00000000ffff7000 RSI:
ffff880219f94140 RDI:
ffff880228444000
RBP:
ffffc90005f9b948 R08:
0000000000000000 R09:
0000000000000000
R10:
0000000000000000 R11:
0000000000000000 R12:
0000000000000080
R13:
0000000000000001 R14:
ffffc90005f9bcd7 R15:
ffff88020c9a83c0
FS:
00007fb53e1ee920(0000) GS:
ffff88024dd00000(0000) knlGS:
0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0:
0000000080050033
CR2:
0000000000000010 CR3:
000000022ef95000 CR4:
00000000003406e0
DR0:
0000000000000000 DR1:
0000000000000000 DR2:
0000000000000000
DR3:
0000000000000000 DR6:
00000000fffe0ff0 DR7:
0000000000000400
Call Trace:
ppgtt_bind_vma+0x40/0x50 [i915]
i915_vma_bind+0xcb/0x1c0 [i915]
__i915_vma_do_pin+0x6e/0xd0 [i915]
i915_gem_execbuffer_reserve_vma+0x162/0x1d0 [i915]
i915_gem_execbuffer_reserve+0x4fc/0x510 [i915]
? __kmalloc+0x134/0x250
? i915_gem_wait_for_error+0x25/0x100 [i915]
? i915_gem_wait_for_error+0x25/0x100 [i915]
i915_gem_do_execbuffer+0x2df/0xa00 [i915]
? drm_malloc_gfp.clone.0+0x42/0x80 [i915]
? path_put+0x22/0x30
? __check_object_size+0x62/0x1f0
? terminate_walk+0x44/0x90
i915_gem_execbuffer2+0x95/0x1e0 [i915]
drm_ioctl+0x243/0x490
? handle_pte_fault+0x1d7/0x220
? i915_gem_do_execbuffer+0xa00/0xa00 [i915]
? handle_mm_fault+0x10d/0x2a0
vfs_ioctl+0x18/0x30
do_vfs_ioctl+0x14b/0x3f0
SyS_ioctl+0x92/0xa0
entry_SYSCALL_64_fastpath+0x1a/0xa9
RIP: 0033:0x7fb53b4fcb77
RSP: 002b:
00007ffe0c572898 EFLAGS:
00003246 ORIG_RAX:
0000000000000010
RAX:
ffffffffffffffda RBX:
00007fb53e17c038 RCX:
00007fb53b4fcb77
RDX:
00007ffe0c572900 RSI:
0000000040406469 RDI:
000000000000000b
RBP:
00007fb5376d67e0 R08:
0000000000000000 R09:
0000000000000000
R10:
0000000000000028 R11:
0000000000003246 R12:
0000000000000000
R13:
0000000000000000 R14:
000055eecb314d00 R15:
000055eecb315460
Code: 0f 84 5d ff ff ff eb a2 0f 1f 84 00 00 00 00 00 55 48 89 e5 41 57 41 56 41 55 41 54 53 48 83 ec 58 0f 1f 44 00 00 31 c0 89 4d b0 <4c>
8b 60 10 44 8b 70 0c 48 89 d0 4c 8b 2e 48 c1 e8 27 25 ff 01
RIP: gen8_ppgtt_insert_4lvl+0x1b/0x1e0 [i915] RSP:
ffffc90005f9b8c8
CR2:
0000000000000010
Recent gccs, such as 4.9, 6.3 or 7.2, do not generate the warning nor do
they explode on use. If we manually create the struct using locals from
the stack, this should eliminate this issue, and does not alter code
generation with gcc-7.2.
Fixes:
894ccebee2b0 ("drm/i915: Micro-optimise gen8_ppgtt_insert_entries()")
Reported-by: Kelly French <kfrench@federalhill.net>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Kelly French <kfrench@federalhill.net>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171106211128.12538-1-chris@chris-wilson.co.uk
Tested-by: Kelly French <kfrench@federalhill.net>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Michal Wajdeczko [Thu, 26 Oct 2017 17:36:57 +0000 (17:36 +0000)]
drm/i915: Handle error-state modparams in dedicated functions
Capturing and cleanup and modparams in error state requires
some macro tricks. Move that code into separated functions
for easier maintenance.
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20171026173657.49648-3-michal.wajdeczko@intel.com
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Michal Wajdeczko [Thu, 26 Oct 2017 17:36:56 +0000 (17:36 +0000)]
drm/i915: Make GuC log part of the uC error state
We keep details of GuC and HuC in separate error state struct.
Make GuC log part of it to group all related data together.
Since we are printing uC details at the end, with this change
GuC log will be moved there too.
v2: comment on new placement of the log (Chris)
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171026173657.49648-2-michal.wajdeczko@intel.com
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Michal Wajdeczko [Thu, 26 Oct 2017 17:36:55 +0000 (17:36 +0000)]
drm/i915: Add Guc/HuC firmware details to error state
Include GuC and HuC firmware details in captured error state
to provide additional debug information. To reuse existing
uc firmware pretty printer, introduce new drm-printer variant
that works with our i915_error_state_buf output. Also update
uc firmware pretty printer to accept const input.
v2: don't rely on current caps (Chris)
dump correct fw info (Michal)
v3: simplify capture of custom paths (Chris)
v4: improve 'why' comment (Joonas)
trim output if no fw path (Michal)
group code around uc error state (Michal)
v5: use error in cleanup_uc (Michal)
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171026173657.49648-1-michal.wajdeczko@intel.com
[ickle: allow printing uc_fw after allocation failure]
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Michal Wajdeczko [Mon, 6 Nov 2017 13:51:54 +0000 (13:51 +0000)]
drm/i915/guc: Assert ctch->vma is allocated
Silence smatch by demonstrating that ctch->vma is allocated
following a successful chch_init()
drivers/gpu/drm/i915/intel_guc_ct.c:204 ctch_open() error:
we previously assumed 'ctch->vma' could be null (see line 197)
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171106135154.52520-1-michal.wajdeczko@intel.com
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Chris Wilson [Mon, 6 Nov 2017 11:48:33 +0000 (11:48 +0000)]
drm/i915/guc: Assert guc->stage_desc_pool is allocated
Silence smatch by demonstrating that guc->stage_desc_pool is allocated
following a successful guc_stage_desc_pool_create(),
drivers/gpu/drm/i915/i915_guc_submission.c:1293 i915_guc_submission_init() error: we previously assumed 'guc->stage_desc_pool' could be null (see line 1261)
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171106114833.31199-1-chris@chris-wilson.co.uk
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Chris Wilson [Mon, 6 Nov 2017 11:15:08 +0000 (11:15 +0000)]
drm/i915: Lock llist_del_first() vs llist_del_all()
An oversight in commit
87701b4b5593 ("drm/i915: Only free the oldest
stale object before a fresh allocation") was that not only do we have to
serialise concurrent users of llist_del_first(), but we also have to
lock llist_del_first() vs llist_del_all().
From llist.h,
* This can be summarized as follows:
*
* | add | del_first | del_all
* add | - | - | -
* del_first | | L | L
* del_all | | | -
*
* Where, a particular row's operation can happen concurrently with a column's
* operation, with "-" being no lock needed, while "L" being lock is needed.
This should hopefully explain:
<4>[ 89.287106] general protection fault: 0000 [#1] PREEMPT SMP
<4>[ 89.287126] Modules linked in: snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic x86_pkg_temp_thermal intel_powerclamp coretemp i915 crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core r8169 mii mei_me mei snd_pcm prime_numbers i2c_hid pinctrl_geminilake pinctrl_intel
<4>[ 89.287226] CPU: 2 PID: 23 Comm: ksoftirqd/2 Tainted: G U 4.14.0-rc8-CI-CI_DRM_3315+ #1
<4>[ 89.287247] Hardware name: Intel Corp. Geminilake/GLK RVP2 LP4SD (07), BIOS GELKRVPA.X64.0062.B30.
1708222146 08/22/2017
<4>[ 89.287270] task:
ffff88017ab34ec0 task.stack:
ffffc90000128000
<4>[ 89.287290] RIP: 0010:llist_add_batch+0x4/0x20
<4>[ 89.287301] RSP: 0018:
ffffc9000012bdb8 EFLAGS:
00010296
<4>[ 89.287314] RAX:
ffffffff811017ad RBX:
6e468801a1560000 RCX:
ef3e53fceecdeb81
<4>[ 89.287330] RDX:
6e468801a1566130 RSI:
ffff880103d73d98 RDI:
ffff880103d73d98
<4>[ 89.287346] RBP:
ffffc9000012bdb8 R08:
ffff88017ab35780 R09:
0000000000000000
<4>[ 89.287361] R10:
ffffc9000012bd68 R11:
00000000abb18c3d R12:
ffffffffa01369e0
<4>[ 89.287377] R13:
ffff88017fd1b8f8 R14:
ffff88017ab34ec0 R15:
000000000000000a
<4>[ 89.287393] FS:
0000000000000000(0000) GS:
ffff88017fd00000(0000) knlGS:
0000000000000000
<4>[ 89.287411] CS: 0010 DS: 0000 ES: 0000 CR0:
0000000080050033
<4>[ 89.287424] CR2:
00007ff0c0755018 CR3:
000000016df9b000 CR4:
00000000003406e0
<4>[ 89.287440] Call Trace:
<4>[ 89.287511] __i915_gem_free_object_rcu+0x20/0x40 [i915]
<4>[ 89.287527] rcu_process_callbacks+0x27a/0x730
<4>[ 89.287544] __do_softirq+0xc0/0x4ae
<4>[ 89.287559] ? smpboot_thread_fn+0x2d/0x280
<4>[ 89.287571] run_ksoftirqd+0x1f/0x70
<4>[ 89.287582] smpboot_thread_fn+0x18a/0x280
<4>[ 89.287595] kthread+0x114/0x150
<4>[ 89.287605] ? sort_range+0x30/0x30
<4>[ 89.287615] ? kthread_create_on_node+0x40/0x40
<4>[ 89.287628] ret_from_fork+0x27/0x40
<4>[ 89.287641] Code: 0d 48 83 ea 01 4c 89 c1 48 83 fa ff 74 12 48 23 0c d7 74 ed 48 c1 e2 06 48 0f bd c9 48 8d 04 0a 5d c3 90 90 90 90 90 55 48 89 e5 <48> 8b 0a 48 89 0e 48 89 c8 f0 48 0f b1 3a 48 39 c1 75 ed 48 85
<1>[ 89.287774] RIP: llist_add_batch+0x4/0x20 RSP:
ffffc9000012bdb8
<4>[ 89.287826] ---[ end trace
e775d15174d8ae02 ]---
(Lockless lists are only easy (and lockless) when only using
llist_add/llist_del_all!)
Fixes:
87701b4b5593 ("drm/i915: Only free the oldest stale object before
a fresh allocation")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171106111508.11941-1-chris@chris-wilson.co.uk
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Chris Wilson [Wed, 25 Oct 2017 15:32:07 +0000 (16:32 +0100)]
drm/i915/selftests: Hide dangerous tests
Some tests are designed to exercise the limits of the HW and may trigger
unintended side-effects making the machine unusable. This should not be
executed by default, but are still useful for early platform validation.
References: https://bugs.freedesktop.org/show_bug.cgi?id=103453
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171025153207.9589-1-chris@chris-wilson.co.uk
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Chris Wilson [Sun, 5 Nov 2017 12:45:50 +0000 (12:45 +0000)]
drm/i915: Assert vma->flags are updated correctly during binding
As we bind, and unbind on error, we want to be sure that the vma->flags
are updated to reflect the binding state so that on the next invocation
all is well.
v2: Take two.
v3: Take three; vma-misplaced is checking map-and-fenceable so keep it
last!
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171105124550.32715-1-chris@chris-wilson.co.uk
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
Chris Wilson [Thu, 2 Nov 2017 13:14:30 +0000 (13:14 +0000)]
drm/i915: Set up mocs tables before restarting the engines
After a reset, we may immediately begin executing requests on restarting
the engines. Ergo this has to be last step with all re-initialisation
completed beforehand. The mocs setup was after we started executing the
requests; do it earlier!
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20171102131430.22328-1-chris@chris-wilson.co.uk
Tvrtko Ursulin [Fri, 3 Nov 2017 09:05:38 +0000 (09:05 +0000)]
drm/i915: Warn in debug builds of incorrect usages of ptr_pack_bits
GEM_BUG_ON if the packed bits do not fit into the specified width.
v2: Avoid using the macro argument twice.
v3: Drop unnecessary braces. (Joonas)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171103090538.14474-1-tvrtko.ursulin@linux.intel.com
Tvrtko Ursulin [Tue, 31 Oct 2017 10:23:25 +0000 (10:23 +0000)]
drm/i915: Reject unknown syncobj flags
We have to reject unknown flags for uAPI considerations, and also
because the curent implementation limits their i915 storage space
to two bits.
v2: (Chris Wilson)
* Fix fail in ABI check.
* Added unknown flags and BUILD_BUG_ON.
v3:
* Use ARCH_KMALLOC_MINALIGN instead of alignof. (Chris Wilson)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Fixes:
cf6e7bac6357 ("drm/i915: Add support for drm syncobjs")
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20171031102326.9738-1-tvrtko.ursulin@linux.intel.com
Lionel Landwerlin [Thu, 2 Nov 2017 12:18:27 +0000 (12:18 +0000)]
drm/i915: ensure oa config uuid is null terminated
Because dev_priv is 0-ed it's not currently an issue, but since we
have dev_priv->perf.oa.test_config.uuid size at uuid + 1, we could
just copy the null character.
v2: Use strlcpy instead of strncpy (Chris)
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20171102121827.436-1-lionel.g.landwerlin@intel.com
Chris Wilson [Wed, 1 Nov 2017 20:21:49 +0000 (20:21 +0000)]
drm/i915: Flush the irq and tasklets before asserting engine is idle
Before we assert that the engine is idle, make sure we flush any
residual tasklet. After that point, if the engine is not idle, more work
may be queued despite us trying to park the engine and go to sleep.
References: https://bugs.freedesktop.org/show_bug.cgi?id=103479
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171101202149.32493-1-chris@chris-wilson.co.uk
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Mika Kuoppala [Thu, 2 Nov 2017 09:48:36 +0000 (11:48 +0200)]
drm/i915: Use fallback forcewake if primary ack missing
There is a possibility on gen9 hardware to miss the forcewake ack
message. The recommended workaround is to use another free
bit and toggle it until original bit is successfully acknowledged.
Some future gen9 revs might or might not fix the underlying issue but
using fallback forcewake bit dance can be considered as harmless:
without the ack timeout we never reach the fallback bit forcewake.
Thus as of now we adopt a blanket approach for all gen9 and leave
the bypassing the fallback bit approach for future patches if
corresponding hw revisions do appear.
Commit
83e3337204b2 ("drm/i915: Increase maximum polling time to 50ms
for forcewake request/clear ack") did increase the forcewake timeout.
If the issue was a delayed ack, future work could include finding
a suitable timeout value both for primary ack and reserve toggle
to reduce the worst case latency.
v2: use bit 15, naming, comment (Chris), only wait fallback ack
v3: fix return on fallback, backoff after fallback write (Chris)
v4: udelay on first pass, grammar (Chris)
v4: s/reserve/fallback
References: HSDES #
1604254524
References: https://bugs.freedesktop.org/show_bug.cgi?id=102051
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20171102094836.2506-1-mika.kuoppala@linux.intel.com
Michel Thierry [Tue, 31 Oct 2017 22:53:09 +0000 (15:53 -0700)]
drm/i915/guc: Add support for reset engine using GuC commands
This patch adds per engine reset and recovery (TDR) support when GuC is
used to submit workloads to GPU.
In the case of i915 directly submission to ELSP, driver manages hang
detection, recovery and resubmission. With GuC submission these tasks
are shared between driver and GuC. i915 is still responsible for detecting
a hang, and when it does it only requests GuC to reset that Engine. GuC
internally manages acquiring forcewake and idling the engine before
resetting it.
Once the reset is successful, i915 takes over again and handles the
resubmission. The scheduler in i915 knows which requests are pending so
after resetting a engine, pending workloads/requests are resubmitted
again.
v2: s/i915_guc_request_engine_reset/i915_guc_reset_engine/ to match the
non-guc function names.
v3: Removed debug message about engine restarting from which request,
since the new baseline do it regardless of submission mode. (Chris)
v4: Rebase.
v5: Do not pass unnecessary reporting flags to the fw (Jeff);
tasklet_schedule(&execlists->irq_tasklet) handles the resubmit; rebase.
v6: Rename the existing reset engine function and share a similar
interface between guc and non-guc paths (Chris).
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20171031225309.10888-1-michel.thierry@intel.com
Reviewed-by: Jeff McGee <jeff.mcgee@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Michel Thierry [Mon, 30 Oct 2017 18:56:14 +0000 (11:56 -0700)]
drm/i915/guc: Rename the function that resets the GuC
intel_guc_reset sounds more like the microcontroller is the one performing
a reset, while in this case is the opposite. intel_reset_guc not only
makes it clearer, it follows the other intel_reset functions available.
v2: Print error message in English (Tvrtko).
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171030185616.32836-2-michel.thierry@intel.com
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Jeff McGee [Wed, 1 Nov 2017 22:16:30 +0000 (15:16 -0700)]
drm/i915/guc: Clear terminated attribute bit on GuC preemption context
If GuC firmware performs an engine reset while that engine had a
preemption pending, it will set the terminated attribute bit on our
preemption stage descriptor. GuC firmware retains all pending work
items for a high-priority GuC client, unlike the normal-priority GuC
client where work items are dropped. It wants to make sure the preempt-
to-idle work doesn't run when scheduling resumes, and uses this bit to
inform its scheduler and presumably us as well. Our job is to clear it
for the next preemption after reset, otherwise that and future
preemptions will never complete. We'll just clear it every time.
Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171101221630.25086-1-jeff.mcgee@intel.com
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Mika Kahola [Wed, 1 Nov 2017 10:08:50 +0000 (12:08 +0200)]
drm/i915/cnl: Symmetric scalers for each pipe
For Cannonlake the number of scalers for each pipe is 2. Let's increase
the number of scalers for pipe C.
v2: Use INTEL_GEN() instead of IS_CANNONLAKE()
Signed-off-by: Mika Kahola <mika.kahola@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1509530930-24960-1-git-send-email-mika.kahola@intel.com
Chris Wilson [Fri, 27 Oct 2017 11:06:17 +0000 (12:06 +0100)]
drm/i915: Give more details for the active-when-parking warning for the engines
If the we think the engine is still active when we attempt to park it,
we want more details -- so dump the engine state.
References: https://bugs.freedesktop.org/show_bug.cgi?id=103479
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171027110617.31745-4-chris@chris-wilson.co.uk
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Chris Wilson [Fri, 27 Oct 2017 11:06:16 +0000 (12:06 +0100)]
drm/i915: Move parking-while-active warning to intel_engines_park()
We will want to break this down to give detailed per-engine warnings as
to why we still think we are active as we attempt to park the engines.
For the first step, just move the warning verbatim from the idle-worker
to intel_engines_park().
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20171027110617.31745-3-chris@chris-wilson.co.uk
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Chris Wilson [Tue, 31 Oct 2017 12:22:35 +0000 (12:22 +0000)]
drm/i915: Check that the breadcrumb wasn't disarmed automatically before parking
We will disarm the breadcrumb interrupt if we see a user interrupt
whilst no one is waiting. This may race with the call to
intel_engine_disarm_breadcrumbs() triggering an assert that we aren't
trying to do the same job twice. Prevent this by checking that the irq
is still armed after flushing the interrupt (for the irq spinlock).
Fixes:
bcbd5c33a342 ("drm/i915/guc: Always enable the breadcrumbs irq")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171031122235.1395-1-chris@chris-wilson.co.uk
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Chris Wilson [Tue, 31 Oct 2017 10:36:07 +0000 (10:36 +0000)]
drm/i915: Check incoming alignment for unfenced buffers (on i915gm)
In case the object has changed tiling between calls to execbuf, we need
to check if the existing offset inside the GTT matches the new tiling
constraint. We even need to do this for "unfenced" tiled objects, where
the 3D commands use an implied fence and so the object still needs to
match the physical fence restrictions on alignment (only required for
gen2 and early gen3).
In commit
2889caa92321 ("drm/i915: Eliminate lots of iterations over
the execobjects array"), the idea was to remove the second guessing and
only set the NEEDS_MAP flag when required. However, the entire check
for an unusable offset for fencing was removed and not just the
secondary check. I.e.
/* avoid costly ping-pong once a batch bo ended up non-mappable */
if (entry->flags & __EXEC_OBJECT_NEEDS_MAP &&
!i915_vma_is_map_and_fenceable(vma))
return !only_mappable_for_reloc(entry->flags);
was entirely removed as the ping-pong between execbuf passes was fixed,
but its primary purpose in forcing unaligned unfenced access to be
rebound was forgotten.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103502
Fixes:
2889caa92321 ("drm/i915: Eliminate lots of iterations over the execobjects array")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171031103607.17836-1-chris@chris-wilson.co.uk
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Michel Thierry [Fri, 27 Oct 2017 22:32:07 +0000 (15:32 -0700)]
drm/i915/cnl: Remove unnecessary check in cnl_setup_private_ppat
There is no need check if PPGTT is disabled because that not possible
in CNL. Execlists and GuC submission modes rely on at least aliasing
PPGTT and even intel_sanitize_enable_ppgtt says: "We don't allow disabling
PPGTT for gen9+ as it's a requirement for execlists, the sole mechanism
available to submit work."
Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171027223207.7869-1-michel.thierry@intel.com
Ville Syrjälä [Mon, 30 Oct 2017 18:46:54 +0000 (20:46 +0200)]
drm/i915: Remove most encoder->type uses from the audio code
encoder->type isn't genreally safe around DDI ports, so let's
replace some uses in the audio code with the crtc state's
output_types instead.
Actually in these cases encoder->type would work since the DP
SST case is only relevant for VLV/CHV and encoder->type==DP
is a thing on those platforms. The DP MST cases would work
as well since MST encoder->type==DP_MST always. But I think
it's best to try and minimize the encoder->type use in general
to avoid showing a bad example to people.
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171030184654.17429-2-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Ville Syrjälä [Mon, 30 Oct 2017 18:46:53 +0000 (20:46 +0200)]
drm/i915: Pass around crtc and connector states for audio
Explicitly pass the crtc and connector states into the audio
code enable/disable hooks, and plumb them all the way down.
This gets rid of almost all crtc->config and encoder->crtc
uses. The one place where we still use them is
i915_audio_component_sync_audio_rate() since that gets called from
the audio driver and we don't have explicit states around then.
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171030184654.17429-1-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Chris Wilson [Mon, 30 Oct 2017 17:29:27 +0000 (17:29 +0000)]
drm/i915: Replace "cc-option -Wno-foo" with "cc-disable-warning foo"
To quote kbuild/makefiles.txt:
cc-disable-warning checks if gcc supports a given warning and returns
the commandline switch to disable it. This special function is needed,
because gcc 4.4 and later accept any unknown -Wno-* option and only
warn about it if there is another warning in the source file.
This is exactly what we were trying to achieve with cc-option -Wno-foo and
failed miserably.
Reported-by: kbuild-all@01.org
Fixes:
39bf4de89ff7 ("drm/i915: Add -Wall -Wextra to our build, set warnings to full")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171030172927.18158-1-chris@chris-wilson.co.uk
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Ville Syrjälä [Fri, 27 Oct 2017 19:31:28 +0000 (22:31 +0300)]
drm/i915: Use intel_ddi_get_config() for MST
Eliminate the partially duplicated DDI readout code from MST, and
instead just call intel_ddi_get_config(). As a nice bonus we get
more cross checking as intel_ddi_get_config() will populate
output_types based on the actual mode of the DDI port.
Additonally intel_ddi_get_config() must be changed to get the crtc
from the passed in crtc state rather than from the encoder->crtc link.
encoder->crtc really shouldn't be used anyway.
v2: Rebased on BXT MST latency_optim fix
Make intel_ddi_clock_get() static
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171027193128.14483-7-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Ville Syrjälä [Fri, 27 Oct 2017 19:31:27 +0000 (22:31 +0300)]
drm/i915: Pass a crtc state to ddi post_disable from MST code
Pass an old crtc state to intel_ddi_post_disable() from the MST code.
Note that this crtc state won't necessaitly match the one that was
passed to intel_ddi_pre_enable() if the first stream to be enabled isn't
the last stream to be disabled. But this is fine since the states should
be identical in every important way. This does mean people frobbing
the DDI pre_enable/post_disable hooks have to pay attention in what
parts of the state they consult.
The alternative would be to inline the relevant code into the MST code.
That is actually what we used to do for pre_enable before
commit
e081c8463ac9 ("drm/i915: Remove duplicate DDI enabling logic
from MST path"). For post_disable we've always called the DDI hook.
v2: Pimp up the comments explaining the MST issues
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171027193128.14483-6-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Ville Syrjälä [Fri, 27 Oct 2017 19:31:26 +0000 (22:31 +0300)]
drm/i915: Eliminate pll->state usage from bxt_calc_pll_link()
We should be using the DPLL hw state we got from the current crtc state
to determine the corresponding port clock frequency rather than getting
it via the current state programmed into the DPLL.
v2: Rebase due to intel_dpll_id changes
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171027193128.14483-5-ville.syrjala@linux.intel.com
Ville Syrjälä [Fri, 27 Oct 2017 19:31:25 +0000 (22:31 +0300)]
drm/i915: Nuke intel_ddi_get_encoder_port()
encoder->port works for FDI, and it also works for MST (regardless of
whether we're dealing with the "fake" MST encoder, or mst->primary).
So let's eliminate intel_ddi_get_encoder_port().
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171027193128.14483-4-ville.syrjala@linux.intel.com
Ville Syrjälä [Fri, 27 Oct 2017 19:31:24 +0000 (22:31 +0300)]
drm/i915: Stop frobbing with DDI encoder->type
Currently the DDI encoder->type will change at runtime depending on
what kind of hotplugs we've processed. That's quite bad since we can't
really trust that that current value of encoder->type actually matches
the type of signal we're trying to drive through it.
Let's eliminate that problem by declaring that non-eDP DDI port will
always have the encoder type as INTEL_OUTPUT_DDI. This means the code
can no longer try to distinguish DP vs. HDMI based on encoder->type.
We'll leave eDP as INTEL_OUTPUT_EDP, since it'll never change and
there's a bunch of code that relies on that value to identify eDP
encoders.
We'll introduce a new encoder .compute_output_type() hook. This allows
us to compute the full output_types before any encoder .compute_config()
hooks get called, thus those hooks can rely on output_types being
correct, which is useful for cloning on oldr platforms. For now we'll
just look at the connector type and pick the correct mode based on that.
In the future the new hook could be used to implement dynamic switching
between LS and PCON modes for LSPCON.
v2: Fix BXT/GLK PPS explosion with DSI/MST encoders
v3: Avoid the PPS warn on pure HDMI/DVI DDI encoders by checking dp.output_reg
v4: Rebase
v5: Populate output_types in .get_config() rather than in the caller
v5: Split out populating output_types in .get_config() (Maarten)
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171027193128.14483-3-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Ville Syrjälä [Fri, 27 Oct 2017 19:31:23 +0000 (22:31 +0300)]
drm/i915: Populate output_types from .get_config()
Rather than having the caller of .get_config() set output_types based on
encoder->type, let's just have .get_config() itself populate
output_types. This way we are isolated from encoder->type, which won't
be useable for this purpose anyway soon (at least for DDI encoders).
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171027193128.14483-2-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Ville Syrjälä [Mon, 30 Oct 2017 14:57:02 +0000 (16:57 +0200)]
drm/i915: Parse max HDMI TMDS clock from VBT
Starting from version 204 VBT can specify the max TMDS clock we are
allowed to use with HDMI ports. Parse that information and take it
into account when filtering modes and computing a crtc state.
Also take the opportunity to sort the platform check if ladder
from new to old.
v2: Add defines for the values into intel_vbt_defs.h (Jani)
Don't fall back to 0 silently for unknown values (Jani)
Skip the debug print for the 0 case (Jani)
Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171030145702.23662-1-ville.syrjala@linux.intel.com
Ville Syrjälä [Fri, 27 Oct 2017 20:17:38 +0000 (23:17 +0300)]
drm/i915/vbt: Fix HDMI level shifter and max data rate bitfield sizes
The HDMI level shifter value should be 5 bits and the max data rate 3 bits.
Cc: Jani Nikula <jani.nikula@intel.com>
Reported-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171027201738.3640-1-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Ville Syrjälä [Thu, 26 Oct 2017 15:14:04 +0000 (18:14 +0300)]
drm/i915: Clean up the mess around hdmi_12bpc_possible()
Move the crtc state related 12bpc checks into hdmi_12bpc_possible()
since that one already examines other parts of the crtc state.
Note that we can drop the !force_dvi check since
crtc_state->has_hdmi_sink already accounts for that.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171026151405.30710-1-ville.syrjala@linux.intel.com
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Ville Syrjälä [Fri, 27 Oct 2017 09:45:23 +0000 (12:45 +0300)]
drm/i915: Improve DP downstream HPD handling
DP dongles may signal downstream HPD via short HPD pulses. Setting the
sink to DPMS off apparently kills the downstream HPD (at least on my
DP->VGA dongle), so skip the DPMS off for such dongles when we turn
off the port.
v2: Deal with DDI as well by moving the check into
intel_dp_sink_dpms() (Dhinakaran)
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Pablo <pablodebiase@nanalysis.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103472
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99114
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171027094523.9317-1-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Ville Syrjälä [Fri, 27 Oct 2017 13:43:48 +0000 (16:43 +0300)]
drm/i915: Fix BXT lane latency optimal setting with MST
Call the DDI .pre_pll_enable() hook from the MST code so that BXT gets
the correct lane latency optimal setting applied. And we obviously need
to compute the correct value, and read it out to keep the state checker
happy.
While at it drop the useless 'encoder' parameter to
bxt_ddi_phy_calc_lane_lat_optim_mask()
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171027134348.31190-1-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Ville Syrjälä [Thu, 19 Oct 2017 13:37:15 +0000 (16:37 +0300)]
drm/i915: Stop using encoder->type in intel_ddi_enable_transcoder_func()
intel_ddi_enable_transcoder_func() already has the crtc state so we can
use that instead of the untrustworthy encoder->type.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171019133721.11794-5-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Ville Syrjälä [Thu, 19 Oct 2017 13:37:14 +0000 (16:37 +0300)]
drm/i915: Start using output_types for DPLL selection
encoder->type is not realiable for DP/HDMI so let's switch the DPLL
selection over to using output_types.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171019133721.11794-4-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Ville Syrjälä [Thu, 19 Oct 2017 13:37:13 +0000 (16:37 +0300)]
drm/i915: Pass crtc state to intel_prepare_dp_ddi_buffers()
Eliminate intel_prepare_dp_ddi_buffers()'s reliance on the encoder->type
by passing in the crtc state.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171019133721.11794-3-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Ville Syrjälä [Thu, 19 Oct 2017 13:37:12 +0000 (16:37 +0300)]
drm/i915: Don't use encoder->type in intel_ddi_set_pipe_settings()
encoder->type isn't reliable for DP/HDMI so instead extract the correct
type from the crtc state in intel_ddi_set_pipe_settings().
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171019133721.11794-2-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Chris Wilson [Thu, 26 Oct 2017 13:00:32 +0000 (14:00 +0100)]
drm/i915: Hold rcu_read_lock when iterating over the radixtree (vma idr)
Kasan spotted
[IGT] gem_tiled_pread_pwrite: exiting, ret=0
==================================================================
BUG: KASAN: use-after-free in __i915_gem_object_reset_page_iter+0x15c/0x170 [i915]
Read of size 8 at addr
ffff8801359da310 by task kworker/3:2/182
CPU: 3 PID: 182 Comm: kworker/3:2 Tainted: G U 4.14.0-rc6-CI-Custom_3340+ #1
Hardware name: Intel Corp. Geminilake/GLK RVP1 DDR4 (05), BIOS GELKRVPA.X64.0062.B30.
1708222146 08/22/2017
Workqueue: events __i915_gem_free_work [i915]
Call Trace:
dump_stack+0x68/0xa0
print_address_description+0x78/0x290
? __i915_gem_object_reset_page_iter+0x15c/0x170 [i915]
kasan_report+0x23d/0x350
__asan_report_load8_noabort+0x19/0x20
__i915_gem_object_reset_page_iter+0x15c/0x170 [i915]
? i915_gem_object_truncate+0x100/0x100 [i915]
? lock_acquire+0x380/0x380
__i915_gem_object_put_pages+0x30d/0x530 [i915]
__i915_gem_free_objects+0x551/0xbd0 [i915]
? lock_acquire+0x13e/0x380
__i915_gem_free_work+0x4e/0x70 [i915]
process_one_work+0x6f6/0x1590
? pwq_dec_nr_in_flight+0x2b0/0x2b0
worker_thread+0xe6/0xe90
? pci_mmcfg_check_reserved+0x110/0x110
kthread+0x309/0x410
? process_one_work+0x1590/0x1590
? kthread_create_on_node+0xb0/0xb0
ret_from_fork+0x27/0x40
Allocated by task 1801:
save_stack_trace+0x1b/0x20
kasan_kmalloc+0xee/0x190
kasan_slab_alloc+0x12/0x20
kmem_cache_alloc+0xdc/0x2e0
radix_tree_node_alloc.constprop.12+0x48/0x330
__radix_tree_create+0x274/0x480
__radix_tree_insert+0xa2/0x610
i915_gem_object_get_sg+0x224/0x670 [i915]
i915_gem_object_get_page+0xb5/0x1c0 [i915]
i915_gem_pread_ioctl+0x822/0xf60 [i915]
drm_ioctl_kernel+0x13f/0x1c0
drm_ioctl+0x6cf/0x980
do_vfs_ioctl+0x184/0xf30
SyS_ioctl+0x41/0x70
entry_SYSCALL_64_fastpath+0x1c/0xb1
Freed by task 37:
save_stack_trace+0x1b/0x20
kasan_slab_free+0xaf/0x190
kmem_cache_free+0xbf/0x340
radix_tree_node_rcu_free+0x79/0x90
rcu_process_callbacks+0x46d/0xf40
__do_softirq+0x21c/0x8d3
The buggy address belongs to the object at
ffff8801359da0f0
which belongs to the cache radix_tree_node of size 576
The buggy address is located 544 bytes inside of
576-byte region [
ffff8801359da0f0,
ffff8801359da330)
The buggy address belongs to the page:
page:
ffffea0004d67600 count:1 mapcount:0 mapping: (null) index:0x0 compound_mapcount: 0
flags: 0x8000000000008100(slab|head)
raw:
8000000000008100 0000000000000000 0000000000000000 0000000100110011
raw:
ffffea0004b52920 ffffea0004b38020 ffff88015b416a80 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8801359da200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801359da280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>
ffff8801359da300: fb fb fb fb fb fb fc fc fc fc fc fc fc fc fc fc
^
ffff8801359da380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff8801359da400: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================
Disabling lock debugging due to kernel taint
which looks like the slab containing the radixtree iter was freed as we
traversed the tree, taking the rcu read lock across the loop should
prevent that (deferring all the frees until the end).
Reported-by: Tomi Sarvela <tomi.p.sarvela@intel.com>
Fixes:
d1b48c1e7184 ("drm/i915: Replace execbuf vma ht with an idr")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171026130032.10677-2-chris@chris-wilson.co.uk
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
Chris Wilson [Thu, 26 Oct 2017 13:00:31 +0000 (14:00 +0100)]
drm/i915: Hold rcu_read_lock when iterating over the radixtree (objects)
Kasan spotted
[IGT] gem_tiled_pread_pwrite: exiting, ret=0
==================================================================
BUG: KASAN: use-after-free in __i915_gem_object_reset_page_iter+0x15c/0x170 [i915]
Read of size 8 at addr
ffff8801359da310 by task kworker/3:2/182
CPU: 3 PID: 182 Comm: kworker/3:2 Tainted: G U 4.14.0-rc6-CI-Custom_3340+ #1
Hardware name: Intel Corp. Geminilake/GLK RVP1 DDR4 (05), BIOS GELKRVPA.X64.0062.B30.
1708222146 08/22/2017
Workqueue: events __i915_gem_free_work [i915]
Call Trace:
dump_stack+0x68/0xa0
print_address_description+0x78/0x290
? __i915_gem_object_reset_page_iter+0x15c/0x170 [i915]
kasan_report+0x23d/0x350
__asan_report_load8_noabort+0x19/0x20
__i915_gem_object_reset_page_iter+0x15c/0x170 [i915]
? i915_gem_object_truncate+0x100/0x100 [i915]
? lock_acquire+0x380/0x380
__i915_gem_object_put_pages+0x30d/0x530 [i915]
__i915_gem_free_objects+0x551/0xbd0 [i915]
? lock_acquire+0x13e/0x380
__i915_gem_free_work+0x4e/0x70 [i915]
process_one_work+0x6f6/0x1590
? pwq_dec_nr_in_flight+0x2b0/0x2b0
worker_thread+0xe6/0xe90
? pci_mmcfg_check_reserved+0x110/0x110
kthread+0x309/0x410
? process_one_work+0x1590/0x1590
? kthread_create_on_node+0xb0/0xb0
ret_from_fork+0x27/0x40
Allocated by task 1801:
save_stack_trace+0x1b/0x20
kasan_kmalloc+0xee/0x190
kasan_slab_alloc+0x12/0x20
kmem_cache_alloc+0xdc/0x2e0
radix_tree_node_alloc.constprop.12+0x48/0x330
__radix_tree_create+0x274/0x480
__radix_tree_insert+0xa2/0x610
i915_gem_object_get_sg+0x224/0x670 [i915]
i915_gem_object_get_page+0xb5/0x1c0 [i915]
i915_gem_pread_ioctl+0x822/0xf60 [i915]
drm_ioctl_kernel+0x13f/0x1c0
drm_ioctl+0x6cf/0x980
do_vfs_ioctl+0x184/0xf30
SyS_ioctl+0x41/0x70
entry_SYSCALL_64_fastpath+0x1c/0xb1
Freed by task 37:
save_stack_trace+0x1b/0x20
kasan_slab_free+0xaf/0x190
kmem_cache_free+0xbf/0x340
radix_tree_node_rcu_free+0x79/0x90
rcu_process_callbacks+0x46d/0xf40
__do_softirq+0x21c/0x8d3
The buggy address belongs to the object at
ffff8801359da0f0
which belongs to the cache radix_tree_node of size 576
The buggy address is located 544 bytes inside of
576-byte region [
ffff8801359da0f0,
ffff8801359da330)
The buggy address belongs to the page:
page:
ffffea0004d67600 count:1 mapcount:0 mapping: (null) index:0x0 compound_mapcount: 0
flags: 0x8000000000008100(slab|head)
raw:
8000000000008100 0000000000000000 0000000000000000 0000000100110011
raw:
ffffea0004b52920 ffffea0004b38020 ffff88015b416a80 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8801359da200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801359da280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>
ffff8801359da300: fb fb fb fb fb fb fc fc fc fc fc fc fc fc fc fc
^
ffff8801359da380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff8801359da400: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================
Disabling lock debugging due to kernel taint
which looks like the slab containing the radixtree iter was freed as we
traversed the tree, taking the rcu read lock across the loop should
prevent that (deferring all the frees until the end).
Reported-by: Tomi Sarvela <tomi.p.sarvela@intel.com>
Fixes:
96d776345277 ("drm/i915: Use a radixtree for random access to the object's backing storage")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171026130032.10677-1-chris@chris-wilson.co.uk
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
Chris Wilson [Fri, 27 Oct 2017 09:43:11 +0000 (10:43 +0100)]
drm/i915: Empty the ring before disabling
An interesting snippet from Sandybridge's prm:
"Although a Ring Buffer can be enabled in the non-empty state, it must
not be disabled unless it is empty. Attempting to disable a Ring Buffer
in the non-empty state is UNDEFINED."
Let's avoid the undefined behaviour as we disable the rings prior to
reset and resume.
v2: Tell HEAD to catch up to TAIL (empty ring) first, then reset both to
0 (supposedly while stopped).
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/20171027094311.30380-1-chris@chris-wilson.co.uk
Jani Nikula [Thu, 26 Oct 2017 14:29:32 +0000 (17:29 +0300)]
drm/i915/edp: clean up code and comments around eDP DPCD read
Some minor drive-by cleanups.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
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/20171026142932.17737-2-jani.nikula@intel.com
Jani Nikula [Thu, 26 Oct 2017 14:29:31 +0000 (17:29 +0300)]
drm/i915/edp: read edp display control registers unconditionally
Per my reading of the eDP spec, DP_DPCD_DISPLAY_CONTROL_CAPABLE bit in
DP_EDP_CONFIGURATION_CAP should be set if the eDP display control
registers starting at offset DP_EDP_DPCD_REV are "enabled". Currently we
check the bit before reading the registers, and DP_EDP_DPCD_REV is the
only way to detect eDP revision.
Turns out there are (likely buggy) displays that require eDP 1.4+
features, such as supported link rates and link rate select, but do not
have the bit set. Read the display control registers
unconditionally. They are supposed to read zero anyway if they are not
supported, so there should be no harm in this.
This fixes the referenced bug by enabling the eDP version check, and
thus reading of the supported link rates. The panel in question has 0 in
DP_MAX_LINK_RATE which is only supported in eDP 1.4+. Without the
supported link rates method we default to RBR which is insufficient for
the panel native mode. As a curiosity, the panel also has a bogus value
of 0x12 in DP_EDP_DPCD_REV, but that passes our check for >= DP_EDP_14
(which is 0x03).
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103400
Reported-and-tested-by: Nicolas P. <issun.artiste@gmail.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: stable@vger.kernel.org
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
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/20171026142932.17737-1-jani.nikula@intel.com
Maarten Lankhorst [Thu, 19 Oct 2017 15:13:41 +0000 (17:13 +0200)]
drm/i915: Calculate ironlake intermediate watermarks correctly, v2.
The watermarks it should calculate against are the old optimal watermarks.
The currently active crtc watermarks are pure fiction, and are invalid in
case of a nonblocking modeset, page flip enabling/disabling planes or any
other reason.
When the crtc is disabled or during a modeset the intermediate watermarks
don't need to be programmed separately, and could be directly assigned
to the optimal watermarks.
Changes since v1:
- Use intel_atomic_get_old_crtc_state. (ville)
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171019151341.4579-2-maarten.lankhorst@linux.intel.com
[mlankhorst: Add cc stable and bugzilla link, since previous patch doesn't fix issue by itself]
Cc: stable@vger.kernel.org #v4.8+
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102373
Maarten Lankhorst [Thu, 19 Oct 2017 15:13:40 +0000 (17:13 +0200)]
drm/i915: Do not rely on wm preservation for ILK watermarks
The original intent was to preserve watermarks as much as possible
in intel_pipe_wm.raw_wm, and put the validated ones in intel_pipe_wm.wm.
It seems this approach is insufficient and we don't always preserve
the raw watermarks, so just use the atomic iterator we're already using
to get a const pointer to all bound planes on the crtc.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102373
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: stable@vger.kernel.org #v4.8+
Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171019151341.4579-1-maarten.lankhorst@linux.intel.com
Manasi Navare [Thu, 26 Oct 2017 21:52:00 +0000 (14:52 -0700)]
drm/i915: Cancel the modeset retry work during modeset cleanup
During modeset cleanup on driver unload we may have a pending
hotplug work. This needs to be canceled early during the teardown
so that it does not fire after we have freed the connector.
We do this after drm_kms_helper_poll_fini(dev) since this might
trigger modeset retry work due to link retrain and before
intel_fbdev_fini() since this work requires the lock from fbdev.
If this is not done we may see something like:
DEBUG_LOCKS_WARN_ON(mutex_is_locked(lock))
------------[ cut here ]------------
WARNING: CPU: 4 PID: 5010 at kernel/locking/mutex-debug.c:103 mutex_destroy+0x4e/0x60
Modules linked in: i915(-) snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec snd_hwdep snd_hda_core snd_pcm vgem ax88179_178
+a usbnet mii x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel e1000e ptp pps_core prime_numbers i2c_hid
+[last unloaded: snd_hda_intel]
CPU: 4 PID: 5010 Comm: drv_module_relo Tainted: G U 4.14.0-rc3-CI-CI_DRM_3186+ #1
Hardware name: Intel Corporation CoffeeLake Client Platform/CoffeeLake S UDIMM RVP, BIOS CNLSFWX1.R00.X104.A03.
1709140524 09/14/2017
task:
ffff8803c827aa40 task.stack:
ffffc90000520000
RIP: 0010:mutex_destroy+0x4e/0x60
RSP: 0018:
ffffc90000523d58 EFLAGS:
00010292
RAX:
000000000000002a RBX:
ffff88044fbef648 RCX:
0000000000000000
RDX:
0000000080000001 RSI:
0000000000000001 RDI:
ffffffff810f0cf0
RBP:
ffffc90000523d60 R08:
0000000000000001 R09:
0000000000000001
R10:
000000000f21cb81 R11:
0000000000000000 R12:
ffff88044f71efc8
R13:
ffffffffa02b3d20 R14:
ffffffffa02b3d90 R15:
ffff880459b29308
FS:
00007f5df4d6e8c0(0000) GS:
ffff88045d300000(0000) knlGS:
0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0:
0000000080050033
CR2:
000055ec51f00a18 CR3:
0000000451782006 CR4:
00000000003606e0
DR0:
0000000000000000 DR1:
0000000000000000 DR2:
0000000000000000
DR3:
0000000000000000 DR6:
00000000fffe0ff0 DR7:
0000000000000400
Call Trace:
drm_fb_helper_fini+0xd9/0x130
intel_fbdev_destroy+0x12/0x60 [i915]
intel_fbdev_fini+0x28/0x30 [i915]
intel_modeset_cleanup+0x45/0xa0 [i915]
i915_driver_unload+0x92/0x180 [i915]
i915_pci_remove+0x19/0x30 [i915]
i915_driver_unload+0x92/0x180 [i915]
i915_pci_remove+0x19/0x30 [i915]
pci_device_remove+0x39/0xb0
device_release_driver_internal+0x15d/0x220
driver_detach+0x40/0x80
bus_remove_driver+0x58/0xd0
driver_unregister+0x2c/0x40
pci_unregister_driver+0x36/0xb0
i915_exit+0x1a/0x8b [i915]
SyS_delete_module+0x18c/0x1e0
entry_SYSCALL_64_fastpath+0x1c/0xb1
RIP: 0033:0x7f5df3286287
RSP: 002b:
00007fff8e107cc8 EFLAGS:
00000246 ORIG_RAX:
00000000000000b0
RAX:
ffffffffffffffda RBX:
ffffffff81493a03 RCX:
00007f5df3286287
RDX:
0000000000000001 RSI:
0000000000000800 RDI:
0000564c7be02e48
RBP:
ffffc90000523f88 R08:
0000000000000000 R09:
0000000000000080
R10:
00007f5df4d6e8c0 R11:
0000000000000246 R12:
0000000000000000
R13:
00007fff8e107eb0 R14:
0000000000000000 R15:
0000000000000000
Or a GPF like:
general protection fault: 0000 [#1] PREEMPT SMP
Modules linked in: i915(-) snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec snd_hwdep snd_hda_core snd_pcm vgem ax88179_178
+a usbnet mii x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel e1000e ptp pps_core prime_numbers i2c_hid
+[last unloaded: snd_hda_intel]
CPU: 0 PID: 82 Comm: kworker/0:1 Tainted: G U W 4.14.0-rc3-CI-CI_DRM_3186+ #1
Hardware name: Intel Corporation CoffeeLake Client Platform/CoffeeLake S UDIMM RVP, BIOS CNLSFWX1.R00.X104.A03.
1709140524 09/14/2017
Workqueue: events intel_dp_modeset_retry_work_fn [i915]
task:
ffff88045a5caa40 task.stack:
ffffc90000378000
RIP: 0010:drm_setup_crtcs+0x143/0xbf0
RSP: 0018:
ffffc9000037bd20 EFLAGS:
00010202
RAX:
6b6b6b6b6b6b6b6b RBX:
0000000000000002 RCX:
0000000000000001
RDX:
0000000000000001 RSI:
0000000000000780 RDI:
00000000ffffffff
RBP:
ffffc9000037bdb8 R08:
0000000000000001 R09:
0000000000000001
R10:
0000000000000780 R11:
0000000000000000 R12:
0000000000000002
R13:
ffff88044fbef4e8 R14:
0000000000000780 R15:
0000000000000438
FS:
0000000000000000(0000) GS:
ffff88045d200000(0000) knlGS:
0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0:
0000000080050033
CR2:
000055ec51ee5168 CR3:
000000044c89d003 CR4:
00000000003606f0
Call Trace:
drm_fb_helper_hotplug_event.part.18+0x7e/0xc0
drm_fb_helper_hotplug_event+0x1a/0x20
intel_fbdev_output_poll_changed+0x1a/0x20 [i915]
drm_kms_helper_hotplug_event+0x27/0x30
intel_dp_modeset_retry_work_fn+0x77/0x80 [i915]
process_one_work+0x233/0x660
worker_thread+0x206/0x3b0
kthread+0x152/0x190
? process_one_work+0x660/0x660
? kthread_create_on_node+0x40/0x40
ret_from_fork+0x27/0x40
Code: 06 00 00 45 8b 45 20 31 db 45 31 e4 45 85 c0 0f 8e 91 06 00 00 44 8b 75 94 44 8b 7d 90 49 8b 45 28 49 63 d4 44 89 f6 41 83 c4 01 <48> 8b 04 d0 44
+89 fa 48 8b 38 48 8b 87 a8 01 00 00 ff 50 20 01
RIP: drm_setup_crtcs+0x143/0xbf0 RSP:
ffffc9000037bd20
---[ end trace
08901ff1a77d30c7 ]---
v2:
* Rename it to intel_hpd_poll_fini() and call drm_kms_helper_fini() inside it
as the first step before cancel work (Chris Wilson)
* Add GPF trace in commit message and make the function static (Maarten Lankhorst)
Suggested-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Fixes:
9301397a63b3 ("drm/i915: Implement Link Rate fallback on Link training failure")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tony Cheng <tony.cheng@amd.com>
Cc: Harry Wentland <Harry.wentland@amd.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1509054720-25325-1-git-send-email-manasi.d.navare@intel.com
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>