Ville Syrjälä [Wed, 27 Nov 2019 20:12:22 +0000 (22:12 +0200)]
drm/i915/fbc: Reallocate cfb if we need more of it
The code assumes we can omit the cfb allocation once fbc
has been enabled once. That's nonsense. Let's try to
reallocate it if we need to.
The code is still a mess, but maybe this is enough to get
fbc going in some cases where it initially underallocates
the cfb and there's no full modeset to fix it up.
Cc: Daniel Drake <drake@endlessm.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Jian-Hong Pan <jian-hong@endlessm.com> 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/20191127201222.16669-15-ville.syrjala@linux.intel.com Tested-by: Daniel Drake <drake@endlessm.com> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Ville Syrjälä [Thu, 28 Nov 2019 15:03:38 +0000 (17:03 +0200)]
drm/i915/fbc: Wait for vblank after FBC disable on glk+
On glk+ the hardware gets confused if we disable FBC while
it's recompressing and we perform a plane update during the
same frame. The result is that top of the screen gets corrupted.
We can avoid that by giving the hardware enough time to finish
the FBC disable before we touch the plane registers. Ie. we need
an extra vblank wait after FBC disable.
v2: Don't do the vblank wait if we never activated FBC in hw
Ville Syrjälä [Wed, 27 Nov 2019 20:12:19 +0000 (22:12 +0200)]
drm/i915/fbc: Start using flip nuke
The hardware automagically nukes the cfb on flip. We can use
that whenever the plane/crtc configuration doesn't change too
much. Let's hook that up.
We'll need this for glk+ since we need to introduce an extra
vblank wait after FBC disable. As we're currently disabling
FBC around all plane updates we'd slow them down by an extra
frame. Not a great user experience when your fps is always
capped at vrefres/2. With flip nuke we don't need the extra
vblank wait.
Ville Syrjälä [Wed, 27 Nov 2019 20:12:16 +0000 (22:12 +0200)]
drm/i915/fbc: Make fence_id optional for i965gm
i965gm no longer needs the fence for scanout so we should be
do what we do for ctg+ and only configure a fence for FBC
when we have one.
In theory this should do nothing atm on account of
intel_fbc_can_activate() requiring the fence, but since
we do this for g4x+ let's do it for i965gm as well. We
may want to relax the requirements at some point and allow
FBC without a fence.
Ville Syrjälä [Wed, 27 Nov 2019 20:12:12 +0000 (22:12 +0200)]
drm/i915/fbc: Remove the FBC_RT_BASE setup for ILK/SNB
We don't want to use the FBC hardware render tracking so let's not
enable it. To use the hw tracking properly we'd anyway need to
integrate this into the command submissing path as the register is
context saved, and if rendering happens via the ppgtt we'd have
to configure it with the ppgtt address instead of the ggtt address.
Easier to use software tracking instead.
Note that on pre-ilk we can't actually disable render tracking.
However we can't rely on it because it requires that DSPSURF to
match the render target address, and since we play tricks
with DSPSURF that may not be the case. Hence we shall rely on
software render tracking on all platforms.
Ville Syrjälä [Wed, 27 Nov 2019 20:12:09 +0000 (22:12 +0200)]
drm/i915/fbc: Disable fbc by default on all glk+
We're missing a workaround in the fbc code for all glk+ platforms
which can cause corruption around the top of the screen. So
enabling fbc by default is a bad idea. I'm not keen to backport
the w/a so let's start by disabling fbc by default on all glk+.
We'll lift the restriction once the w/a is in place.
Cc: stable@vger.kernel.org Cc: Daniel Drake <drake@endlessm.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Jian-Hong Pan <jian-hong@endlessm.com> 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/20191127201222.16669-2-ville.syrjala@linux.intel.com Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
drm/i915/perf: Allow non-privileged access when OA buffer is not sampled
SAMPLE_OA_REPORT enables sampling of OA reports from the OA buffer.
Since reports from OA buffer had system wide visibility, collecting
samples from the OA buffer was a privileged operation on previous
platforms. Prior to TGL, it was also necessary to sample the OA buffer
to normalize reports from MI REPORT PERF COUNT.
TGL has a dedicated OAR unit to sample perf reports for a specific
render context. This removes the necessity to sample OA buffer.
- If not sampling the OA buffer, allow non-privileged access. An earlier
patch allows the non-privilege access:
https://patchwork.freedesktop.org/patch/337716/?series=68582&rev=1
- Clear up the path for non-privileged access in this patch
Chris Wilson [Sun, 8 Dec 2019 16:12:52 +0000 (16:12 +0000)]
drm/i915: Change i915_vma_unbind() to report -EAGAIN on activity
If someone else acquires the i915_vma before we complete our wait and
unbind it, we currently error out with -EBUSY. Use -EAGAIN instead so
that if necessary the caller is prepared to try again.
Chris Wilson [Sun, 8 Dec 2019 16:12:51 +0000 (16:12 +0000)]
drm/i915/gem: Avoid rcu_barrier() from shrinker paths
As i915_gem_object_unbind() waits on an rcu_barrier() to flush vm
releases (and destruction of their bound vma), we have to be careful not
to invoke that barrier from beneath the shrinker:
<4> [430.222671] WARNING: possible circular locking dependency detected
<4> [430.222673] 5.4.0-rc8-CI-CI_DRM_7508+ #1 Tainted: G U
<4> [430.222675] ------------------------------------------------------
<4> [430.222677] gem_pwrite/2317 is trying to acquire lock:
<4> [430.222678] ffffffff82248218 (rcu_state.barrier_mutex){+.+.}, at: rcu_barrier+0x23/0x190
<4> [430.222685]
but task is already holding lock:
<4> [430.222687] ffffffff82263a40 (fs_reclaim){+.+.}, at: fs_reclaim_acquire.part.117+0x0/0x30
<4> [430.222691]
which lock already depends on the new lock.
Chris Wilson [Sat, 7 Dec 2019 22:26:44 +0000 (22:26 +0000)]
drm/i915/gem: Comment on inability to check args.pad for MMAP_OFFSET
Since we didn't check and insist that args.pad must be zero for MMAP_GTT
historically, we cannot insert a check now as old userspace may be
feeding in garbage. As such the lack of check is enshrined into the ABI,
so add a comment to remind us we cannot add the check later.
In the extreme case, we may wish to wait on an rcu-barrier to reap stale
vm to purge the last of the object bindings. However, we are not allowed
to use rcu_barrier() beneath the dma_resv (i.e. object) lock and do not
take lightly the prospect of unlocking a mutex deep in the bowels of the
routine. i915_gem_object_unbind() itself does not need the object lock,
and it turns out the callers do not need to the unbind as part of a
locked sequence around set-cache-level, so rearrange the code to avoid
taking the object lock in the callers.
<4> [186.816311] ======================================================
<4> [186.816313] WARNING: possible circular locking dependency detected
<4> [186.816316] 5.4.0-rc8-CI-CI_DRM_7486+ #1 Tainted: G U
<4> [186.816318] ------------------------------------------------------
<4> [186.816320] perf_pmu/1321 is trying to acquire lock:
<4> [186.816322] ffff88849487c4d8 (&mm->mmap_sem#2){++++}, at: __might_fault+0x39/0x90
<4> [186.816331]
but task is already holding lock:
<4> [186.816333] ffffe8ffffa05008 (&cpuctx_mutex){+.+.}, at: perf_event_ctx_lock_nested+0xa9/0x1b0
<4> [186.816339]
which lock already depends on the new lock.
Chris Wilson [Fri, 6 Dec 2019 10:55:26 +0000 (10:55 +0000)]
drm/i915/gem: Pin gen6_ppgtt prior to constructing the request
All pinning must be done prior to i915_request_create, to avoid
timeline->mutex inversions.
Here we slightly abuse the context_barrier_task stages to utilise the
'skip' decision as an opportunity to acquire the pin on the new ppgtt.
Consider it s/skip/prepare/. At the moment, we only have on user of
context_barrier_task, so it might be worth breaking it down for the
specific task of set-vm and refactor it later if we find a second
purpose.
<4> [402.377487] WARNING: possible circular locking dependency detected
<4> [402.377493] 5.4.0-rc8-CI-CI_DRM_7491+ #1 Tainted: G U
<4> [402.377497] ------------------------------------------------------
<4> [402.377502] gem_exec_parall/2506 is trying to acquire lock:
<4> [402.377507] ffff888403cdac70 (&kernel#2){+.+.}, at: i915_request_create+0x16/0x1c0 [i915]
<4> [402.377593]
but task is already holding lock:
<4> [402.377597] ffff88835efad550 (&ppgtt->pin_mutex){+.+.}, at: gen6_ppgtt_pin+0x4d/0x110 [i915]
<4> [402.377660]
which lock already depends on the new lock.
Commit 9c722e17c1b9 ("drm/i915: Disable pipes in reverse order")
reverted the order that pipes gets disabled because of TGL
master/slave relationship between transcoders in MST mode.
But as stated in a comment in skl_commit_modeset_enables() the
enabling order is not always crescent, possibly causing previously
selected slave transcoder being enabled before master so another
approach will be needed to select a transcoder to master in MST mode.
It will be similar to the approach taken in port sync.
But instead of implement something like
intel_trans_port_sync_modeset_disables() to MST lets simply it and
iterate over all pipes 2 times, the first one disabling any slave and
then disabling everything else.
The MST bits will be added in another patch.
v2:
Not using crtc->active as it is deprecated
v3:
Removing is_trans_port_sync_mode() check, just check for
is_trans_port_sync_master() is enough
v4:
Adding and using is_trans_port_sync_slave(), otherwise non-port sync
pipes will be disabled in the first loop, what is not wrong but is
not what patch description promises
Cc: Lucas De Marchi <lucas.demarchi@intel.com> Cc: Manasi Navare <manasi.d.navare@intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (v2) Signed-off-by: José Roberto de Souza <jose.souza@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191205210350.96795-3-jose.souza@intel.com
drm/i915/display/tgl: Fix the order of the step to turn transcoder clock off
For TGL the step to turn off the transcoder clock was moved to after
the complete shutdown of DDI. Only the MST slave transcoders should
disable the clock before that.
v2:
- Adding last_mst_stream to intel_mst_post_disable_dp, make code more
easy to read and is similar to first_mst_stream in
intel_mst_pre_enable_dp()(Ville's idea)
- Calling intel_ddi_disable_pipe_clock() for GEN12+ right
intel_disable_ddi_buf() as stated in BSpec(Ville)
drm/i915/display: Do not check for the ddb allocations of turned off pipes
It should not care about DDB allocations of pipes going through
a fullmodeset, as at this point those pipes are disabled.
The comment in the code also points to that but that was not what
was being executed.
Chris Wilson [Fri, 6 Dec 2019 16:04:28 +0000 (16:04 +0000)]
drm/i915: Propagate errors on awaiting already signaled dma-fences
If we see an already signaled dma-fence that we want to await on, we skip
adding to the i915_sw_fence. However, we should pay attention to whether
there was an error on that fence and if so propagate it for our future
request.
Chris Wilson [Fri, 6 Dec 2019 16:04:27 +0000 (16:04 +0000)]
drm/i915: Propagate errors on awaiting already signaled fences
If we see an already signaled fence that we want to await on, we skip
adding to the i915_sw_fence. However, we should pay attention to whether
there was an error on that fence and if so propagate it for our future
request.
Chris Wilson [Fri, 6 Dec 2019 16:04:26 +0000 (16:04 +0000)]
drm/i915: Check for error before calling cmpxchg()
Only do the locked compare of the existing fence->error if we actually
need to set an error. As we tend to call i915_sw_fence_set_error_once()
unconditionally, it saves on typing to put the common has-error check
into the inline.
Tvrtko Ursulin [Fri, 29 Nov 2019 10:54:36 +0000 (10:54 +0000)]
drm/i915/pmu: Report frequency as zero while GPU is sleeping
We used to report the minimum possible frequency as both requested and
active while GPU was in sleep state. This was a consequence of sampling
the value from the "current frequency" field in our software tracking.
This was strictly speaking wrong, but given that until recently the
current frequency in sleeping state used to be equal to minimum, it did
not stand out sufficiently to be noticed as such.
After some recent changes have made the current frequency be reported
as last active before GPU went to sleep, meaning both requested and active
frequencies could end up being reported at their maximum values for the
duration of the GPU idle state, it became much more obvious that this does
not make sense.
To fix this we will now sample the frequency counters only when the GPU is
awake. As a consequence reported frequencies could be reported as below
the GPU reported minimum but that should be much less confusing that the
current situation.
v2:
* Split out early exit conditions for readability. (Chris)
Chris Wilson [Fri, 6 Dec 2019 10:55:23 +0000 (10:55 +0000)]
drm/i915/gem: Flush the pwrite through the chipset before signaling
Before we signal the fence to indicate completion, ensure the pwrite
through the indirect GGTT is coherent (as best as we know) in memory.
Any listeners to the fence may start immediately and sample from the
backing store prior to the writes being posted, thus seeing stale data.
Chris Wilson [Thu, 5 Dec 2019 21:58:42 +0000 (21:58 +0000)]
drm/i915/gt: Acquire a GT wakeref for the breadcrumb interrupt
Take a wakeref on the intel_gt specifically for the enabled breadcrumb
interrupt so that we can safely process the mmio. If the intel_gt is
already asleep by the time we try and setup the breadcrumb interrupt, by
a process of elimination we know the request must have been completed
and we can skip its enablement!
Chris Wilson [Thu, 5 Dec 2019 23:40:59 +0000 (23:40 +0000)]
drm/i915/gt: Trim gen6 ppgtt updates to PD cachelines
It appears now that we have the ring TLB invalidation in place, we need
only update the page directory cachelines that we have altered. A great
reduction from rewriting the whole 2MiB ppgtt on every update.
Chris Wilson [Thu, 5 Dec 2019 18:33:32 +0000 (18:33 +0000)]
drm/i915: Serialise i915_active_acquire() with __active_retire()
As __active_retire() does it's final atomic_dec() under the
ref->tree_lock spinlock, in order to prevent ourselves from reusing the
ref->cache and ref->tree as they are being destroyed, we need to
serialise with the retirement during i915_active_acquire().
Chris Wilson [Thu, 5 Dec 2019 14:59:34 +0000 (14:59 +0000)]
drm/i915/gt: Save irqstate around virtual_context_destroy
As virtual_context_destroy() may be called from a request signal, it may
be called from inside an irq-off section, and so we need to do a full
save/restore of the irq state rather than blindly re-enable irqs upon
unlocking.
<4> [110.024262] WARNING: inconsistent lock state
<4> [110.024277] 5.4.0-rc8-CI-CI_DRM_7489+ #1 Tainted: G U
<4> [110.024292] --------------------------------
<4> [110.024305] inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage.
<4> [110.024323] kworker/0:0/5 [HC0[0]:SC0[0]:HE1:SE1] takes:
<4> [110.024338] ffff88826a0c7a18 (&(&rq->lock)->rlock){?.-.}, at: i915_request_retire+0x221/0x930 [i915]
<4> [110.024592] {IN-HARDIRQ-W} state was registered at:
<4> [110.024612] lock_acquire+0xa7/0x1c0
<4> [110.024627] _raw_spin_lock_irqsave+0x33/0x50
<4> [110.024788] intel_engine_breadcrumbs_irq+0x38c/0x600 [i915]
<4> [110.024808] irq_work_run_list+0x49/0x70
<4> [110.024824] irq_work_run+0x26/0x50
<4> [110.024839] smp_irq_work_interrupt+0x44/0x1e0
<4> [110.024855] irq_work_interrupt+0xf/0x20
<4> [110.024871] __do_softirq+0xb7/0x47f
<4> [110.024885] irq_exit+0xba/0xc0
<4> [110.024898] do_IRQ+0x83/0x160
<4> [110.024910] ret_from_intr+0x0/0x1d
<4> [110.024922] irq event stamp: 172864
<4> [110.024938] hardirqs last enabled at (172863): [<ffffffff819ea214>] _raw_spin_unlock_irq+0x24/0x50
<4> [110.024963] hardirqs last disabled at (172864): [<ffffffff819e9fba>] _raw_spin_lock_irq+0xa/0x40
<4> [110.024988] softirqs last enabled at (172812): [<ffffffff81c00385>] __do_softirq+0x385/0x47f
<4> [110.025012] softirqs last disabled at (172797): [<ffffffff810b829a>] irq_exit+0xba/0xc0
<4> [110.025031]
other info that might help us debug this:
<4> [110.025049] Possible unsafe locking scenario:
Fixes: b1e3177bd1d8 ("drm/i915: Coordinate i915_active with its own mutex") Fixes: 6d06779e8672 ("drm/i915: Load balancing across a virtual engine") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191205145934.663183-1-chris@chris-wilson.co.uk
Chris Wilson [Thu, 5 Dec 2019 11:37:25 +0000 (11:37 +0000)]
drm/i915: Try hard to bind the context
It is not acceptable for context pinning to fail with -ENOSPC as we
should always be able to make space in the GGTT. The only reason we may
fail is that other "temporary" context pins are reserving their space
and we need to wait for an available slot.
Chris Wilson [Thu, 5 Dec 2019 11:37:24 +0000 (11:37 +0000)]
drm/i915: Ignore most failures during evict-vm
Removing all vma from the VM is best effort -- we only remove all those
ready to be removed, so forgive and VMA that becomes pinned. While
forgiving those that become pinned, also take a second look for any that
became unpinned as we waited.
Chris Wilson [Wed, 4 Dec 2019 23:26:16 +0000 (23:26 +0000)]
drm/i915: Remove vestigal i915_gem_context locals from cmdparser
The use GEM context itself was removed in commit cd30a5031704
("drm/i915/gem: Excise the per-batch whitelist from the context"), but
the locals were left in place as an oversight. Remove the parameters and
clean up.
Chris Wilson [Wed, 4 Dec 2019 12:35:56 +0000 (12:35 +0000)]
drm/i915/gem: Try to flush pending unbind events
If we cannot handle a vma within the unbind loop, try to flush the
pending events (i915_vma_parked, i915_vm_release) and try again. This
avoids a round trip to userspace that is not guaranteed to make forward
progress, as the events we wait upon require being idle.
Abdiel Janulgue [Wed, 4 Dec 2019 12:00:32 +0000 (12:00 +0000)]
drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
This is really just an alias of mmap_gtt. The 'mmap offset' nomenclature
comes from the value returned by this ioctl which is the offset into the
device fd which userpace uses with mmap(2).
mmap_gtt was our initial mmap_offset implementation, this extends
our CPU mmap support to allow additional fault handlers that depends on
the object's backing pages.
Note that we multiplex mmap_gtt and mmap_offset through the same ioctl,
and use the zero extending behaviour of drm to differentiate between
them, when we inspect the flags.
To support multiple mmap types on an object we need to support multiple
mmap_offsets for an object (each offset in the global device address
space corresponding to a unique instance of the object for a file + mmap
type). As we drop the simplified drm core idea of a single mmap_offset,
we need to provide replacement hooks for the dumb mmap interface as
well.
Ville Syrjälä [Wed, 27 Nov 2019 19:05:56 +0000 (21:05 +0200)]
drm/i915: Make intel_crtc_arm_fifo_underrun() functional on gen2
Assuming intel_crtc_arm_fifo_underrun() only gets called when
there's no pending plane updates we can utilize it on gen2 by
checking the active_planes bitmask so that we only re-enable
underrun reporting if some planes are active.
i915_fifo_underrun_reset_write() seems to have the necessary
hw_done/flip_done waits in place.
Let's just inline intel_pre_disable_primary_noatomic() into
intel_plane_disable_noatomic(). The CxSR disable we can do
regardless of which plane we're disabling, and while at it we can
make the gen2 underrun w/a accurate by consulting the active_planes
bitmask.
Ville Syrjälä [Wed, 27 Nov 2019 19:05:54 +0000 (21:05 +0200)]
drm/i915: Clean up the gen2 "no planes -> underrun" workaround
We have the active_planes bitmask now so use it to properly
determine when some planes are visible for the gen2 underrun
workaround.
This let's us almost eliminate intel_post_enable_primary().
The manual underrun checks we can simply move into
intel_atomic_commit_tail() since they loop over all the pipes
already. No point in repeating the checks multiple times when
there are multiple pipes in the commit.
Ville Syrjälä [Wed, 27 Nov 2019 19:05:53 +0000 (21:05 +0200)]
drm/i915: Clean up intel_{pre,post}_plane_update()
Change the calling convention to just pass the state+crtc and
switch to intel_ types throughout.
We'll also do a quick s/if (old_primary_state)/if (new_primary_state)/
so that we'll be able to eliminate old_primary_state later. This
is fine since we always have either both old and new state or neither.
Chris Wilson [Tue, 3 Dec 2019 15:50:32 +0000 (15:50 +0000)]
drm/i915/gem: Avoid parking the vma as we unbind
In order to avoid keeping a reference on the i915_vma (which is long
overdue!) we have to coordinate all the possible lifetimes and only use
the vma while we know it is alive. In this episode, we are reminded that
while idle, the closed vma are destroyed. So if the GT idles while we are
working with the vma, the vma itself becomes invalid.
First class i915_vma here we come, but in the meantime keep piling on
the straw.
drm/i915/dp: Power down sink before disable pipe/transcoder clock
Disabling pipe/transcoder clock before power down sink could cause
sink lost signal, causing it to trigger a hotplug to notify source
that link signal was lost.
drm/i915/display: Check the old state to find port sync slave
If the CRTC is going from enabled to disabled and it is a port sync
slave, it needs to check to the old state to be disabled before the
port sync master.
Cc: Manasi Navare <manasi.d.navare@intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191202222513.337777-1-jose.souza@intel.com
Let's move handling and reset for gen11 display IRQs to their own
functions, similar to how we deal with GT interrupts. This will make
the top-level functions a bit easier to read and potentially make things
easier to deal with in the future if new platforms wind up needing
different display handling logic.
Chris Wilson [Tue, 3 Dec 2019 12:41:55 +0000 (12:41 +0000)]
drm/i915/gt: Track the context validity explicitly
Rather than assume if and only if the engine->default_state is not set
that the context is invalid, instead track when we know the context has
valid state -- either because we have copied the default_state or we
have completed a context switch to save the HW state.
Chris Wilson [Tue, 3 Dec 2019 15:26:31 +0000 (15:26 +0000)]
drm/i915/execlists: Skip nested spinlock for validating pending
Only along the submission path can we guarantee that the locked request
is indeed from a foreign engine, and so the nesting of engine/rq is
permissible. On the submission tasklet (process_csb()), we may find
ourselves competing with the normal nesting of rq/engine, invalidating
our nesting. As we only use the spinlock for debug purposes, skip the
debug if we cannot acquire the spinlock for safe validation - catching
99% of the bugs is better than causing a hard lockup.
Chris Wilson [Tue, 3 Dec 2019 10:13:46 +0000 (10:13 +0000)]
drm/i915/gem: Take runtime-pm wakeref prior to unbinding
Some machines require ACPI for runtime resume, and ACPI is quite kmalloc
happy. We cannot handle kmalloc from inside the vm->mutex, as they are
used by the shrinker, and so we must ensure the global runtime-pm is
awake prior to unbinding to avoid the potential inversion.
<4> [57.121748] ======================================================
<4> [57.121750] WARNING: possible circular locking dependency detected
<4> [57.121753] 5.4.0-rc8-CI-CI_DRM_7466+ #1 Tainted: G U
<4> [57.121754] ------------------------------------------------------
<4> [57.121756] i915_pm_rpm/1105 is trying to acquire lock:
<4> [57.121758] ffffffff82263a40 (fs_reclaim){+.+.}, at: fs_reclaim_acquire.part.117+0x0/0x30
<4> [57.121766]
but task is already holding lock:
<4> [57.121768] ffff888475a593c0 (&vm->mutex){+.+.}, at: i915_vma_unbind+0x21/0x50 [i915]
<4> [57.121868]
which lock already depends on the new lock.
Chris Wilson [Mon, 2 Dec 2019 14:01:33 +0000 (14:01 +0000)]
drm/i915: Serialise i915_active_wait() with its retirement
As the i915_active.retire() may be running on another CPU as we detect
that the i915_active is idle, we may not wait for the retirement itself.
Wait for the remote callback by waiting for the retirement worker.
Similar to for i915_active.mutex, we require each class of i915_active
to have distinct lockdep chains as some, but by no means all,
i915_active are used within the shrinker and so have much more severe
usage constraints. By using a lockclass local to i915_active_init() all
i915_active workers have the same lock class, and we may generate false
positives when waiting for the i915_active. If we push the lockclass
into the caller, each class of i915_active will have distinct lockdep
chains.
Chris Wilson [Mon, 2 Dec 2019 17:43:10 +0000 (17:43 +0000)]
drm/i915/gem: Unbind all current vma on changing cache-level
Avoid dangerous race handling of destroyed vma by unbinding all vma
instead. Unfortunately, this stops us from trying to be clever and only
doing the minimal change required, so on first use of scanout we may
encounter an annoying stall as it transitions to a new cache level.
Lucas De Marchi [Tue, 26 Nov 2019 22:51:10 +0000 (14:51 -0800)]
drm/i915/bios: assume vbt is 4-byte aligned into oprom
The unaligned ioread32() will make us read byte by byte looking for the
vbt. We could just as well have done a ioread8() + a shift and avoid the
extra confusion on how we are looking for "$VBT".
However when using ACPI it's guaranteed the VBT is 4-byte aligned
per spec, so we can probably assume it here as well.
v2: do not try to simplify the loop by eliminating the auxiliary counter
(Jani and Ville)
Lucas De Marchi [Tue, 26 Nov 2019 22:51:09 +0000 (14:51 -0800)]
drm/i915/bios: fold pci rom map/unmap into copy function
We don't need to keep the pci rom mapped during the entire
intel_bios_init() anymore. Move it to the previous copy_vbt() function
and rename it to oprom_get_vbt() since now it's responsible to to all
operations related to get the vbt from the oprom.
v2: fix double __iomem attribute detected by sparse
v3: fix missing unmap on success (Ville)
Lucas De Marchi [Tue, 26 Nov 2019 22:51:08 +0000 (14:51 -0800)]
drm/i915/bios: do not discard address space
When we map the VBT through pci_map_rom() we may not be allowed
to simply discard the address space and go on reading the memory.
That doesn't work on my test system, but by dumping the rom via
sysfs I can can get the correct vbt. So change our find_vbt() to do
the same as done by pci_read_rom(), i.e. use memcpy_fromio().
v2: the just the minimal changes by not bothering with the unaligned io
reads: this can be done on top (from Ville and Jani)
v3: drop const in function return since now we are copying the vbt,
rather than just finding it
drm/i915/display: Suspend MST topology manager before destroy fbdev
MST topology needs to be suspended so we don't have any calls to
fbdev after it's finalized. MST will be destroyed later as part of
drm_mode_config_cleanup().
drm/i915/psr: Check if sink PSR capability changed
eDP specification states that sink can have its PSR capability
changed, I have never found any panel doing that but lets add that
for completeness.
For now it is not reading back the PSR capabilities and if possible
re-enabling PSR, this will be added if a panel is found using this
feature.
When this error happens sink link is not stable after the required
FW_EXIT_LATENCY period so it will miss the selective update.
As the other PSR errors, for now we are not trying to recover from
it.
eDP spec states that when sink enconters a problem that prevents it
to keep PSR running it should set PSR status to internal error and
set the reason why it happen to PSR_ERROR_STATUS but it is not how it
was implemented.
But also I don't want to change this behavior, who knows if there is
a panel out there that only set the PSR_ERROR_STATUS.
So here refactoring the code a bit to make more easy to read what was
state above as more checks will be added to this function.
v2:
returning a int instead of a bool in psr_get_status_and_error_status()
PSR2 HW only support a limited number of bits per pixel, if mode has
more than supported PSR2 should not be enabled.
BSpec: 50422
BSpec: 7713 Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191128014852.214135-1-jose.souza@intel.com
Lucas De Marchi [Wed, 27 Nov 2019 22:11:21 +0000 (14:11 -0800)]
drm/i915/dsb: fix cmd_buf being wrongly set
The "err" label is not really "err", but rather "out" since the return
path is shared between error condition and normal path. This broke when
commit 03cea61076f0 ("drm/i915/dsb: fix extra warning on error path
handling") added a "dsb->cmd_buf = NULL;" there, making DSB to stop
working since now all writes would pass-through via mmio.
Remove the set to NULL since it's actually not needed: we only set it if
all steps are successful. While at it, rename the label so this confusion
doesn't happen again.
Ville Syrjälä [Fri, 29 Nov 2019 18:54:34 +0000 (20:54 +0200)]
drm/i915: Stop using connector->encoder and encoder->crtc links in i915_display_info
Migrate away from the legacy encoder->crtc and connector->encoder links
in the debugfs display_info code. Other users still remain so can't kill
these off yet.
Ville Syrjälä [Fri, 29 Nov 2019 18:54:32 +0000 (20:54 +0200)]
drm/i915: Use the canonical [CRTC:%d:%s]/etc. format in i915_display_info
Use the canonical "[CRTC:%d:%s]" format for the obj id/name
in the debugfs display_info dump. Everyone should already be
familiar with the format since it's used in the debug logs
extensively.
Ville Syrjälä [Fri, 29 Nov 2019 18:54:30 +0000 (20:54 +0200)]
drm/i915: Dump the mode for the crtc just the once
No point in repeating the crtc mode for each cloned encoder.
Just print it once, and avoid using multiple lines for it.
And while at let's polish the fixed mode print to fit on
one line as well.
Matt Roper [Wed, 27 Nov 2019 22:13:14 +0000 (14:13 -0800)]
drm/i915: Program SHPD_FILTER_CNT on CNP+
The bspec tells us 'Program SHPD_FILTER_CNT with the "500 microseconds
adjusted" value before enabling hotplug detection' on CNP+. We haven't
been touching this register at all thus far, but we should probably
follow the bspec's guidance.
The register also exists on LPT and SPT, but there isn't any specific
guidance I can find on how we should be programming it there so let's
leave it be for now.
Matt Roper [Wed, 27 Nov 2019 22:13:13 +0000 (14:13 -0800)]
drm/i915/ehl: Make icp_digital_port_connected() use phy instead of port
When looking at SDEISR to determine the connection status of combo
outputs, we should use the phy index rather than the port index.
Although they're usually the same thing, EHL's DDI-D (port D) is
attached to PHY-A and SDEISR doesn't even have bits for a "D" output.
It's also possible that future platforms may map DDIs (the internal
display engine programming units) to PHYs (the output handling on the IO
side) in ways where port!=phy, so let's look at the PHY index by
default.
v2: Rename to intel_combo_phy_connected. (Lucas)
Fixes: 719d24002602 ("drm/i915/ehl: Enable DDI-D") Cc: José Roberto de Souza <jose.souza@intel.com> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> Reviewed-by: José Roberto de Souza <jose.souza@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191127221314.575575-2-matthew.d.roper@intel.com
Matt Roper [Wed, 27 Nov 2019 22:13:12 +0000 (14:13 -0800)]
drm/i915: Handle SDEISR according to PCH rather than platform
The South Display is part of the PCH so we should technically be basing
our port detection logic off the PCH in use rather than the platform
generation.
Ville Syrjälä [Thu, 28 Nov 2019 18:23:58 +0000 (20:23 +0200)]
drm/i915: Use the correct PCH transcoder for LPT/WPT in intel_sanitize_frame_start_delay()
LPT/WPT only have PCH transcoder A. Make sure we poke at its
chicken register instead of some non-existent register when
FDI is being driven by pipe B or C.
Chris Wilson [Sun, 1 Dec 2019 14:09:16 +0000 (14:09 +0000)]
drm/i915: Refactor gen6_flush_pd()
As the gen6 page directory is written on binding and after every update,
the code ended up duplicated. Refactor the code into a single routine to
share the locking and serialisation.
Chris Wilson [Sat, 30 Nov 2019 16:23:20 +0000 (16:23 +0000)]
drm/i915: Serialise access to GFX_FLSH_CNTL
Now that many threads may try to use the same mmio to flush the global
buffers after updating the PTE, serialise access to the mmio to prevent
concurrent access on gen7.