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.
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.
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().
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.
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.
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
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.
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.
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.
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>
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.
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.
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;
...
}
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.
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>
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>
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>
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>
"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.
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 [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
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.
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.
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.
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.
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
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:
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>
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:
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: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.
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>
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)
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.
(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.
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!
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!
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.
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.
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).
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.
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.
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().
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."
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.
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.
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
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
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.
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().
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)
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).
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)
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()
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>
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).
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
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
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.
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.
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>
Chris Wilson [Tue, 24 Oct 2017 18:15:47 +0000 (19:15 +0100)]
drm/i915: Add -Wall -Wextra to our build, set warnings to full
Recently W=1 on gcc-7.2 (-Wunused-const-variable) caught a regression
that had been lurking for 6 months, so lets try enabling the full set of
warnings for CI builds. This means more patches will be rejected early
that contain trivial and sometimes not so trivial bugs. However, our
code does not yet compile cleanly with W=1, so we have to apply a filter
to the set of warnings until we can eliminate the mistakes. It also
means that developers will have to be running the full gamut of gcc to
ensure that as warnings come and go with gcc updates, we have the CI
build prepared.
v2: Use fine-grained -Wno overrides. Inside the makefile, we can
specify CFLAGS on a per-object level, which allows us to limit the scope
of any particular warning override.
v3: Place per-file overrides after the main enabling block.
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> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Acked-by: Tomi Sarvela <tomi.p.sarvela@intel.com> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20171024181547.27889-1-chris@chris-wilson.co.uk Acked-by: Jani Nikula <jani.nikula@intel.com>
Chris Wilson [Thu, 26 Oct 2017 11:50:48 +0000 (12:50 +0100)]
drm/i915: Include RING_MODE when dumping the engine state
Knowing the RING_MODE flags is useful for checking the state of the
engine, such as whether the CS is idle after trying to stop the engines
before reset.
Michał Winiarski [Thu, 26 Oct 2017 13:35:58 +0000 (15:35 +0200)]
drm/i915/guc: Preemption! With GuC
Pretty similar to what we have on execlists.
We're reusing most of the GEM code, however, due to GuC quirks we need a
couple of extra bits.
Preemption is implemented as GuC action, and actions can be pretty slow.
Because of that, we're using a mutex to serialize them. Since we're
requesting preemption from the tasklet, the task of creating a workitem
and wrapping it in GuC action is delegated to a worker.
To distinguish that preemption has finished, we're using additional
piece of HWSP, and since we're not getting context switch interrupts,
we're also adding a user interrupt.
The fact that our special preempt context has completed unfortunately
doesn't mean that we're ready to submit new work. We also need to wait
for GuC to finish its own processing.
v2: Don't compile out the wait for GuC, handle workqueue flush on reset,
no need for ordered workqueue, put on a reviewer hat when looking at my own
patches (Chris)
Move struct work around in intel_guc, move user interruput outside of
conditional (Michał)
Keep ring around rather than chase though intel_context
v3: Extract WA for flushing ggtt writes to a helper (Chris)
Keep work_struct in intel_guc rather than engine (Michał)
Use ordered workqueue for inject_preempt worker to avoid GuC quirks.
v4: Drop now unused INTEL_GUC_PREEMPT_OPTION_IMMEDIATE (Daniele)
Drop stray newlines, use container_of for intel_guc in worker,
check for presence of workqueue when flushing it, rather than
enable_guc_submission modparam, reorder preempt postprocessing (Chris)
v5: Make wq NULL after destroying it
v6: Swap struct guc_preempt_work members (Michał)
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Jeff McGee <jeff.mcgee@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Oscar Mateo <oscar.mateo@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20171026133558.19580-1-michal.winiarski@intel.com
Michał Winiarski [Wed, 25 Oct 2017 20:00:18 +0000 (22:00 +0200)]
drm/i915: Rename helpers used for unwinding, use macro for can_preempt
We would also like to make use of execlist_cancel_port_requests and
unwind_incomplete_requests in GuC preemption backend.
Let's rename the functions to use the correct prefixes, so that we can
simply add the declarations in the following patch.
Similar thing for applies for can_preempt, except we're introducing
HAS_LOGICAL_RING_PREEMPTION macro instad, converting other users that
were previously touching device info directly.
v2: s/intel_engine/execlists and pass execlists to unwind (Chris)
v3: use locked version for exporting, drop const qual (Chris)
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20171025200020.16636-11-michal.winiarski@intel.com
Michał Winiarski [Wed, 25 Oct 2017 20:00:17 +0000 (22:00 +0200)]
drm/i915/guc: Keep request->priority for its lifetime
We also want to support preemption with GuC submission backend.
In order to do that, we need to remember the priority, like we do on
execlists path.
v2: Remove completed prio == INT_MAX optimization
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Jeff McGee <jeff.mcgee@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20171025200020.16636-10-michal.winiarski@intel.com
projects / kernel.git / log