Age | Commit message (Collapse) | Author |
|
According to the new init phases scheme we should register the driver
with frameworks/userspace only one the device is setup fully. So move
the shrinker registration later accordingly.
Also fix the shrinker unregistration order wrt. the acpi unregistration
to fix the corresponding init order.
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1458128348-15730-10-git-send-email-imre.deak@intel.com
|
|
We require the device to be powered only before accessing it, so we can
move this call later.
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1458128348-15730-9-git-send-email-imre.deak@intel.com
|
|
The only steps requiring device access is the fence and swizzling
initialization, so split these out keeping them in their current place
and move the rest of init steps earlier.
v2-v3:
- unchanged
v4:
- move call to i915_gem_detect_bit_6_swizzle() to
i915_gem_load_init_fences() and preserve the original order of
the detection of HW fence capailities wrt. swizzling (Chris)
CC: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1458132843-21860-1-git-send-email-imre.deak@intel.com
|
|
This init step accesses the device, but doesn't have any device
specific side effect. It also sets up some platform specific
attributes that may be required early, so move it earlier.
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1458128348-15730-7-git-send-email-imre.deak@intel.com
|
|
Split out the part initing the clock gating hooks and move it earlier.
Add a new NOP hook for platforms without the need to apply clockgating
or workaround settings, so that the hook can be called unconditionally.
Also add a WARN for future platforms that forget to add a hook.
The rest of the hooks in intel_init_pm() should be inited in the same
way, but atm some of the hooks are set only conditionally, so before
doing this we need to make the setup unconditional and use instead some
flags.
v2:
- add a NOP hook and WARN if no hook is set for the platform (Chris)
- use the term hook instead of callback for these functions (Jani)
v3:
- remove the GEN4() check it's already covered by earlier platform
checks (Chris)
CC: Jani Nikula <jani.nikula@intel.com>
CC: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1458128348-15730-6-git-send-email-imre.deak@intel.com
|
|
All of this is SW only initialization so we can move them earlier. Move
the mutex init where the rest of the locks are inited. While at it also
convert dev to dev_priv.
v2:
- use the term hook instead of callback for these functions (Jani)
CC: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1458128348-15730-5-git-send-email-imre.deak@intel.com
|
|
Most of the IRQ init is setting up hooks so move that part earlier.
Leave the pm_qos_add_request() call in place.
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1458128348-15730-4-git-send-email-imre.deak@intel.com
|
|
These are all SW only init steps not accessing the device and they only
need the platform identification macros to work, which are already
available earlier, so move these init steps earlier.
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1458128348-15730-3-git-send-email-imre.deak@intel.com
|
|
MCHBAR is cleaned up in i915_mmio_cleanup(), so the separate call in
i915_driver_load() is incorrect.
CC: David Weinehall <david.weinehall@intel.com>
Fixes: ad5c3d3ffbb2 ("drm/i915: Move MCHBAR setup earlier during init")
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1458128348-15730-2-git-send-email-imre.deak@intel.com
|
|
This time using only sed and a few by hand.
v2: Rename also intel_ring_id and intel_ring_initialized.
v3: Fixed typo in intel_ring_initialized.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1458126040-33105-1-git-send-email-tvrtko.ursulin@linux.intel.com
|
|
below and a couple manual fixups.
@@
identifier I, J;
@@
struct I {
...
- struct intel_engine_cs *J;
+ struct intel_engine_cs *engine;
...
}
@@
identifier I, J;
@@
struct I {
...
- struct intel_engine_cs J;
+ struct intel_engine_cs engine;
...
}
@@
struct drm_i915_private *d;
@@
(
- d->ring
+ d->engine
)
@@
struct i915_execbuffer_params *p;
@@
(
- p->ring
+ p->engine
)
@@
struct intel_ringbuffer *r;
@@
(
- r->ring
+ r->engine
)
@@
struct drm_i915_gem_request *req;
@@
(
- req->ring
+ req->engine
)
v2: Script missed the tracepoint code - fixed up by hand.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
|
|
commit e5756c10d841ddb448293c849392f3d6b809561f
Author: Imre Deak <imre.deak@intel.com>
Date: Fri Aug 14 18:43:30 2015 +0300
drm/i915/bxt: don't allow cached GEM mappings on A stepping
Added an exception of disallowing snooping for Broxton A
stepping hardware but userptr was still enabling it regardless.
Move the check to HAS_SNOOP now that it is used from multiple
call sites and use it.
v2: Userptr cannot be supported when it cannot be coherent and
generalize the code better. (Chris Wilson)
v3: Make has_snoop true only when !has_llc. (Chris Wilson)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1456920631-34302-1-git-send-email-tvrtko.ursulin@linux.intel.com
|
|
In addition to calculating final watermarks, let's also pre-calculate a
set of intermediate watermark values at atomic check time. These
intermediate watermarks are a combination of the watermarks for the old
state and the new state; they should satisfy the requirements of both
states which means they can be programmed immediately when we commit the
atomic state (without waiting for a vblank). Once the vblank does
happen, we can then re-program watermarks to the more optimal final
value.
v2: Significant rebasing/rewriting.
v3:
- Move 'need_postvbl_update' flag to CRTC state (Daniel)
- Don't forget to check intermediate watermark values for validity
(Maarten)
- Don't due async watermark optimization; just do it at the end of the
atomic transaction, after waiting for vblanks. We do want it to be
async eventually, but adding that now will cause more trouble for
Maarten's in-progress work. (Maarten)
- Don't allocate space in crtc_state for intermediate watermarks on
platforms that don't need it (gen9+).
- Move WaCxSRDisabledForSpriteScaling:ivb into intel_begin_crtc_commit
now that ilk_update_wm is gone.
v4:
- Add a wm_mutex to cover updates to intel_crtc->active and the
need_postvbl_update flag. Since we don't have async yet it isn't
terribly important yet, but might as well add it now.
- Change interface to program watermarks. Platforms will now expose
.initial_watermarks() and .optimize_watermarks() functions to do
watermark programming. These should lock wm_mutex, copy the
appropriate state values into intel_crtc->active, and then call
the internal program watermarks function.
v5:
- Skip intermediate watermark calculation/check during initial hardware
readout since we don't trust the existing HW values (and don't have
valid values of our own yet).
- Don't try to call .optimize_watermarks() on platforms that don't have
atomic watermarks yet. (Maarten)
v6:
- Rebase
v7:
- Further rebase
v8:
- A few minor indentation and line length fixes
v9:
- Yet another rebase since Maarten's patches reworked a bunch of the
code (wm_pre, wm_post, etc.) that this was previously based on.
v10:
- Move wm_mutex to dev_priv to protect against racing commits against
disjoint CRTC sets. (Maarten)
- Drop unnecessary clearing of cstate->wm.need_postvbl_update (Maarten)
v11:
- Now that we've moved to atomic watermark updates, make sure we call
the proper function to program watermarks in
{ironlake,haswell}_crtc_enable(); the failure to do so on the
previous patch iteration led to us not actually programming the
watermarks before turning on the CRTC, which was the cause of the
underruns that the CI system was seeing.
- Fix inverted logic for determining when to optimize watermarks. We
were needlessly optimizing when the intermediate/optimal values were
the same (harmless), but not actually optimizing when they differed
(also harmless, but wasteful from a power/bandwidth perspective).
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1456276813-5689-1-git-send-email-matthew.d.roper@intel.com
|
|
This reverts commit 1b39a917a9e00378c02c50ad86632ed3d872bfad.
Chris retracted his reviewed-by (which I failed to notice) and somehow
it blows up (I did it again!) as reported by Mika with the below
backtrace on module reload:
[ 58.170374] IP: [<ffffffffa00e04d3>]
intel_logical_ring_cleanup+0x83/0x100 [i915]
...
[ 58.170469] Call Trace:
[ 58.170479] [<ffffffffa00d0ed4>] i915_gem_cleanup_engines+0x34/0x60
[i915]
[ 58.170493] [<ffffffffa0154520>] i915_driver_unload+0x140/0x220
[i915]
[ 58.170497] [<ffffffff8154a4f4>] drm_dev_unregister+0x24/0xa0
[ 58.170501] [<ffffffff8154aace>] drm_put_dev+0x1e/0x60
[ 58.170506] [<ffffffffa00912a0>] i915_pci_remove+0x10/0x20 [i915]
[ 58.170510] [<ffffffff814766e4>] pci_device_remove+0x34/0xb0
[ 58.170514] [<ffffffff8156e7d5>] __device_release_driver+0x95/0x140
[ 58.170518] [<ffffffff8156e97c>] driver_detach+0xbc/0xc0
[ 58.170521] [<ffffffff8156d883>] bus_remove_driver+0x53/0xd0
[ 58.170525] [<ffffffff8156f3a7>] driver_unregister+0x27/0x50
[ 58.170528] [<ffffffff81475725>] pci_unregister_driver+0x25/0x70
[ 58.170531] [<ffffffff8154c274>] drm_pci_exit+0x74/0x90
[ 58.170543] [<ffffffffa0154cb0>] i915_exit+0x20/0x1aa [i915]
[ 58.170548] [<ffffffff8111846f>] SyS_delete_module+0x18f/0x1f0
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dave Gordon <david.s.gordon@intel.com>
Cc: Nick Hoath <nicholas.hoath@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
|
|
Backmerge to get at the new encoder_mask support in atomic helpers.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
|
|
Swap the order of context & engine cleanup, so that contexts are cleaned
up first, and *then* engines. This is a more sensible order anyway, but
in particular has become necessary since the 'intel_ring_initialized()
must be simple and inline' patch, which now uses ring->dev as an
'initialised' flag, so it can now be NULL after engine teardown. This
in turn can cause a problem in the context code, which (used to) check
the ring->dev->struct_mutex -- causing a fault if ring->dev was NULL.
Also rename the cleanup function to reflect what it actually does
(cleanup engines, not a ringbuffer), and fix an annoying whitespace issue.
v2: Also make the fix in i915_load_modeset_init, not just in
i915_driver_unload (Chris Wilson)
v3: Had extra stuff in it.
v4: Reverted extra stuff (so we're back to v2).
Rebased and updated commentary above (Dave Gordon).
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
Signed-off-by: David Gordon <david.s.gordon@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1453504211-7982-2-git-send-email-david.s.gordon@intel.com
|
|
Some Gen7/8 production parts may have the Display Pipe C fused off.
In this case, the display hardware will prevent the enable bit in
PIPE_CONF register (for Pipe C) from being set to 1.
Fixed by adjusting pipe_count to reflect this.
v2: Rename HSW_PIPE_C_DISABLE to IVB_PIPE_C_DISABLE as it already exists
on ivybridge (Ville)
v3: Remove unnecessary MMIO read, correct the description (Damien)
v4: Be more specific in description (Patrick)
Signed-off-by: Gabriel Feceoru <gabriel.feceoru@intel.com>
Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1453462125-21519-1-git-send-email-gabriel.feceoru@intel.com
|
|
Now that the drm core unlinks/disarms events there's no need to do so
ourselves anymore. Nuke the code.
Acked-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453756616-28942-4-git-send-email-daniel.vetter@ffwll.ch
|
|
While not being able to enable MSI interrupts may be a normal
circumstance, for debugging it may still be a useful information, so
emit an info about this.
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: David Weinehall <david.weinehall@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454071949-24677-3-git-send-email-imre.deak@intel.com
|
|
On SKL and KBL we can have pipe A/B/C disabled by fuse settings. The
pipes must be fused in descending order (e.g. C, B+C, A+B+C). We simply
decrease info->num_pipes if we find a valid fused out config.
v2: Don't store the pipe disabled mask in device info (Damien)
v3: Don't check FUSE_STRAP register for pipe c disabled
Cc: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
[Jani: fixed some checkpatch indentation complaints]
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453300280-10661-1-git-send-email-patrik.jakobsson@linux.intel.com
|
|
The only device specific dependency of the stolen memory setup is the
MMIO mapping and the stolen memory size. Both are already available in
i915_gtt_init(), so move the stolen initialization to there. The
clean-up code for i915_gtt_init() is in i915_global_gtt_cleanup(), so
move the stolen memory clean-up code there too.
This will be needed by an upcoming patch that needs the details of the
memory we reserve, but the change is also part of our generic goal to
move the initialization of resources with no or little dependencies on
other device specific resources towards the beginning of the init
sequence.
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: David Weinehall <david.weinehall@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453209992-25995-8-git-send-email-imre.deak@intel.com
|
|
Move the MCHBAR setup right after the MMIO setup, since the two things
are logically related and the MCHBAR setup code doesn't depend on any
other device specific resource. We'll also need MCHBAR to be ready
earlier in an upcoming patch, so this is also a preparation for that.
Factor out the init/clean-up code to separate functions to make things
clearer in the i915_driver_load()/unload() functions.
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: David Weinehall <david.weinehall@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453209992-25995-7-git-send-email-imre.deak@intel.com
|
|
Workqueue initalization doesn't depend on any other device specific
resource, so move it close to the beginning, so we don't need to
consider them when thinking about dependencies for other resources.
Also factor out things to separate init/cleanup functions to make
i915_driver_load()/unload() clearer, atm it's somewhat difficult to
follow there in what order resources are inited/cleaned-up.
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: David Weinehall <david.weinehall@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453209992-25995-6-git-send-email-imre.deak@intel.com
|
|
Factor out common clean-up code for the GEM load time init function.
Also rename i915_gem_load() to i915_gem_load_init() to have a better
match with its new clean-up function.
No functional change.
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: David Weinehall <david.weinehall@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453209992-25995-5-git-send-email-imre.deak@intel.com
|
|
Factor out the common GEM shrinker clean-up code and call the shrinker
init function from the same function from where the corresponding
shrinker clean-up function is called. Also add sanity checking to the
shrinker and OOM registration calls.
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: David Weinehall <david.weinehall@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453209992-25995-4-git-send-email-imre.deak@intel.com
|
|
Clarify the name of the label on the error path, making it clear what's
being cleaned up. The kmem_cache_destroy() calls are NOPs on the
corresponding error path.
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: David Weinehall <david.weinehall@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453209992-25995-3-git-send-email-imre.deak@intel.com
|
|
commit ebae38d061df3deffa7c17b030ea14a5216ee55f
Author: Animesh Manna <animesh.manna@intel.com>
Date: Wed Oct 28 23:58:55 2015 +0200
drm/i915/gen9: csr_init after runtime pm enable
moved the DMC/CSR initialization later during driver loading, but didn't
move the cleanup earlier correspondingly during unloading. Fix this up.
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: David Weinehall <david.weinehall@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453209992-25995-2-git-send-email-imre.deak@intel.com
|
|
This reverts commit 1803c035efb88afb9d3e7feb279ac29a83216382.
It seems to blow up on module unload due to a use-after free hitting a
BUG_ON with CONFIG_DEBUG_SG. Quoting from Tvrtko's mail:
"I've decoded the instructions and it pointed to SG_MAGIC checking:
488b8098010000 mov 0x198(%rax),%rax
ba21436587 mov $0x87654321,%edx
488b00 mov (%rax),%rax *** CRASH
"Grep showed 0x87654321 is SG_MAGIC, so likely candidate for this code
pattern is:
static inline struct page *sg_page(struct scatterlist *sg)
{
BUG_ON(sg->sg_magic != SG_MAGIC);
BUG_ON(sg_is_chain(sg));
return (struct page *)((sg)->page_link & ~0x3);
}
"Which would mean the offender is in intel_logical_ring_cleanup is most
likely:
...
if (ring->status_page.obj) {
kunmap(sg_page(ring->status_page.obj->pages->sgl));
ring->status_page.obj = NULL;
}
...
"I think that the i915_gem_context_fini will do a final unref on
dev_priv->kernel_context and then the ring buff has a copy which is
left dangling because:
lrc_setup_hardware_status_page(ring,
dev_priv->kernel_context->engine[ring->id].state);
and:
ring->status_page.obj = default_ctx_obj;
"Where default_ctx_obj == dev_priv->kernel_context->engine[ring->id].state
So indeed looks like the unload ordering is the trigger. In fact it
is almost the same fragility wrt/ kernel_context hidden dependency I
expressed my worry about in an e-mail yesterday or so. It only shows
if CONFIG_DEBUG_SG is set, otherwise it accesses freed memory and
probably just survives."
This causes serious trouble in our CI system since it took out all
gen8+ machines. Not yet clear why this wasn't caught in pre-merge
testing.
Backtrace from CI, for posterity:
[ 163.737836] general protection fault: 0000 [#1] PREEMPT SMP
[ 163.737849] Modules linked in: ax88179_178a usbnet mii snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic i915(-) x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_hda_codec snd_hwdep snd_hda_core snd_pcm mei_me mei i2c_hid e1000e ptp pps_core [last unloaded: snd_hda_intel]
[ 163.737902] CPU: 0 PID: 5812 Comm: rmmod Tainted: G U W 4.5.0-rc1-gfxbench+ #1
[ 163.737911] Hardware name: System manufacturer System Product Name/Z170M-PLUS, BIOS 0505 11/16/2015
[ 163.737920] task: ffff8800bb99cf80 ti: ffff88022ff2c000 task.ti: ffff88022ff2c000
[ 163.737928] RIP: 0010:[<ffffffffa018f723>] [<ffffffffa018f723>] intel_logical_ring_cleanup+0x83/0x100 [i915]
[ 163.737969] RSP: 0018:ffff88022ff2fd30 EFLAGS: 00010282
[ 163.737975] RAX: 6b6b6b6b6b6b6b6b RBX: ffff8800bb2f31b8 RCX: 0000000000000002
[ 163.737982] RDX: 0000000087654321 RSI: 000000000000000d RDI: ffff8800bb2f31f0
[ 163.737989] RBP: ffff88022ff2fd40 R08: 0000000000000000 R09: 0000000000000001
[ 163.737996] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8800bb2f0000
[ 163.738003] R13: ffff8800bb2f8fc8 R14: ffff8800bb285668 R15: 000055af1ae55210
[ 163.738010] FS: 00007f187014b700(0000) GS:ffff88023bc00000(0000) knlGS:0000000000000000
[ 163.738021] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 163.738030] CR2: 0000558f84e4cbc8 CR3: 000000022cd55000 CR4: 00000000003406f0
[ 163.738039] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 163.738048] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 163.738057] Stack:
[ 163.738062] ffff8800bb2f31b8 ffff8800bb2f0000 ffff88022ff2fd70 ffffffffa0180414
[ 163.738079] ffff8800bb2f0000 ffff8800bb285668 ffff8800bb2856c8 ffffffffa0242460
[ 163.738094] ffff88022ff2fd98 ffffffffa0202d30 ffff8800bb285668 ffff8800bb285668
[ 163.738109] Call Trace:
[ 163.738140] [<ffffffffa0180414>] i915_gem_cleanup_engines+0x34/0x60 [i915]
[ 163.738185] [<ffffffffa0202d30>] i915_driver_unload+0x150/0x270 [i915]
[ 163.738198] [<ffffffff815100f4>] drm_dev_unregister+0x24/0xa0
[ 163.738208] [<ffffffff815106ce>] drm_put_dev+0x1e/0x60
[ 163.738225] [<ffffffffa01412a0>] i915_pci_remove+0x10/0x20 [i915]
[ 163.738237] [<ffffffff8143d9b4>] pci_device_remove+0x34/0xb0
[ 163.738249] [<ffffffff81533d15>] __device_release_driver+0x95/0x140
[ 163.738259] [<ffffffff81533eb6>] driver_detach+0xb6/0xc0
[ 163.738268] [<ffffffff81532de3>] bus_remove_driver+0x53/0xd0
[ 163.738278] [<ffffffff815348d7>] driver_unregister+0x27/0x50
[ 163.738289] [<ffffffff8143ca15>] pci_unregister_driver+0x25/0x70
[ 163.738299] [<ffffffff81511de4>] drm_pci_exit+0x74/0x90
[ 163.738337] [<ffffffffa02034a9>] i915_exit+0x20/0x1a5 [i915]
[ 163.738349] [<ffffffff8110400f>] SyS_delete_module+0x18f/0x1f0
[ 163.738361] [<ffffffff817b8a9b>] entry_SYSCALL_64_fastpath+0x16/0x73
[ 163.738370] Code: ff d0 48 89 df e8 de a1 fd ff 48 8d 7b 38 e8 25 ab fd ff 48 8b 83 90 00 00 00 48 85 c0 74 25 48 8b 80 98 01 00 00 ba 21 43 65 87 <48> 8b 00 48 39 10 75 3c f6 40 08 01 75 38 48 c7 83 90 00 00 00
[ 163.738459] RIP [<ffffffffa018f723>] intel_logical_ring_cleanup+0x83/0x100 [i915]
[ 163.738498] RSP <ffff88022ff2fd30>
[ 163.738507] ---[ end trace 68f69ce4740fa44f ]---
Cc: Nick Hoath <nicholas.hoath@intel.com>
Cc: Dave Gordon <david.s.gordon@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Tested-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
|
|
Swap the order of context & engine cleanup, so that contexts are cleaned
up first, and *then* engines. This is a more sensible order anyway, but
in particular has become necessary since the 'intel_ring_initialized()
must be simple and inline' patch, which now uses ring->dev as an
'initialised' flag, so it can now be NULL after engine teardown. This
in turn can cause a problem in the context code, which (used to) check
the ring->dev->struct_mutex -- causing a fault if ring->dev was NULL.
Also rename the cleanup function to reflect what it actually does
(cleanup engines, not a ringbuffer), and fix an annoying whitespace issue.
v2: Also make the fix in i915_load_modeset_init, not just in
i915_driver_unload (Chris Wilson)
v3: Had extra stuff in it.
v4: Reverted extra stuff (so we're back to v2).
Rebased and updated commentary above (Dave Gordon).
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v2)
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1453405067-32890-3-git-send-email-david.s.gordon@intel.com
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
|
|
This reverts commit 396e33ae204f52abebec9e578f44c749305500f4.
This commit was triggering some FIFO underrun warnings on ILK-IVB
platforms (but surprisingly not on HSW/BDW that share more or less the
same codepaths). These underruns were caught by the continuous
integration (CI) system and could be reproduced consistently when
running the basic acceptance tests (BAT) on the affected platforms.
Note that this revert will cause a visible regression for some
end-users; the "flicker when mouse moves between monitors in X" issue
that was reported before this patch was merged will now return. However
regressions that are visible to CI have higher priority since they
prevent proper testing of future patches on those platforms. Hopefully
we'll be able to figure out the cause of the underruns quickly and
remerge an improved version of this patch to fix the regression.
Cc: Daniel Vetter <daniel@ffwll.ch>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93640
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1453232584-8543-1-git-send-email-matthew.d.roper@intel.com
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
|
|
Pull in Dave's drm-next pull request to have a clean base for 4.6.
Also, we need the various atomic state extensions Maarten recently
created.
Conflicts are just adjacent changes that all resolve to nothing in git
diff.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
|
|
git://anongit.freedesktop.org/drm-intel into drm-next
Since your main drm-next pull isn't out of the door yet I figured I might
as well flush out drm-misc instead of delaying for 4.6. It's really just
random stuff all over, biggest thing probably connector_mask tracking from
Maarten.
* tag 'topic/drm-misc-2016-01-17' of git://anongit.freedesktop.org/drm-intel: (24 commits)
drm/fb_cma_helper: Remove implicit call to disable_unused_functions
drm/sysfs: use kobj_to_dev()
drm/i915: Init power domains early in driver load
drm: Do not set connector->encoder in drivers
apple-gmux: Add initial documentation
drm: move MODULE_PARM_DESC to other file
drm/edid: index CEA/HDMI mode tables using the VIC
drm/atomic: Remove drm_atomic_connectors_for_crtc.
drm/i915: Update connector_mask during readout, v2.
drm: Remove opencoded drm_gem_object_release_handle()
drm: Do not set outparam on error during GEM handle allocation
drm/docs: more leftovers from the big vtable documentation pile
drm/atomic-helper: Reject legacy flips on a disabled pipe
drm/atomic: add connector mask to drm_crtc_state.
drm/tegra: Use __drm_atomic_helper_reset_connector for subclassing connector state, v2.
drm/atomic: Add __drm_atomic_helper_connector_reset, v2.
drm/i915: Set connector_state->connector using the helper.
drm: Use a normal idr allocation for the obj->name
drm: Only bump object-reference count when adding first handle
drm: Balance error path for GEM handle allocation
...
|
|
Since
commit ac9b8236551d1177fd07b56aef9b565d1864420d
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
Date: Fri Nov 27 18:55:26 2015 +0200
drm/i915: Introduce a gmbus power domain
gmbus also needs the power domain infrastructure right from the start,
since as soon as we register the i2c controllers someone can use them.
v2: Adjust cleanup paths too (Chris).
v3: Rebase onto -nightly (totally bogus tree I had lying around) and
also move dpio init head (Ville).
v4: Ville instead suggested to move gmbus setup later in the sequence,
since it's only needed by the modeset code.
v5: Move even close to the actual user, right next to the comment that
states where we really need gmbus (and interrupts!).
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Meelis Roos <mroos@linux.ee>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
Cc: stable@vger.kernel.org
References: http://www.spinics.net/lists/intel-gfx/msg83075.html
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1452682528-19437-1-git-send-email-daniel.vetter@ffwll.ch
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
|
|
In addition to calculating final watermarks, let's also pre-calculate a
set of intermediate watermark values at atomic check time. These
intermediate watermarks are a combination of the watermarks for the old
state and the new state; they should satisfy the requirements of both
states which means they can be programmed immediately when we commit the
atomic state (without waiting for a vblank). Once the vblank does
happen, we can then re-program watermarks to the more optimal final
value.
v2: Significant rebasing/rewriting.
v3:
- Move 'need_postvbl_update' flag to CRTC state (Daniel)
- Don't forget to check intermediate watermark values for validity
(Maarten)
- Don't due async watermark optimization; just do it at the end of the
atomic transaction, after waiting for vblanks. We do want it to be
async eventually, but adding that now will cause more trouble for
Maarten's in-progress work. (Maarten)
- Don't allocate space in crtc_state for intermediate watermarks on
platforms that don't need it (gen9+).
- Move WaCxSRDisabledForSpriteScaling:ivb into intel_begin_crtc_commit
now that ilk_update_wm is gone.
v4:
- Add a wm_mutex to cover updates to intel_crtc->active and the
need_postvbl_update flag. Since we don't have async yet it isn't
terribly important yet, but might as well add it now.
- Change interface to program watermarks. Platforms will now expose
.initial_watermarks() and .optimize_watermarks() functions to do
watermark programming. These should lock wm_mutex, copy the
appropriate state values into intel_crtc->active, and then call
the internal program watermarks function.
v5:
- Skip intermediate watermark calculation/check during initial hardware
readout since we don't trust the existing HW values (and don't have
valid values of our own yet).
- Don't try to call .optimize_watermarks() on platforms that don't have
atomic watermarks yet. (Maarten)
v6:
- Rebase
v7:
- Further rebase
v8:
- A few minor indentation and line length fixes
v9:
- Yet another rebase since Maarten's patches reworked a bunch of the
code (wm_pre, wm_post, etc.) that this was previously based on.
v10:
- Move wm_mutex to dev_priv to protect against racing commits against
disjoint CRTC sets. (Maarten)
- Drop unnecessary clearing of cstate->wm.need_postvbl_update (Maarten)
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1452108870-24204-1-git-send-email-matthew.d.roper@intel.com
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
|
|
Atm, we assert that the device is not suspended until the point when the
device is truly put to a suspended state. This is fine, but we can catch
more problems if we check that RPM refcount is non-zero. After that one
drops to zero we shouldn't access the device any more, even if the actual
device suspend may be delayed. Change assert_rpm_wakelock_held()
accordingly to check for a non-zero RPM refcount in addition to the
current device-not-suspended check.
For the new asserts to work we need to annotate every place explicitly in
the code where we expect that the device is powered. The places where we
only assume this, but may not hold an RPM reference:
- driver load
We assume the device to be powered until we enable RPM. Make this
explicit by taking an RPM reference around the load function.
- system and runtime sudpend/resume handlers
These handlers are called when the RPM reference becomes 0 and know the
exact point after which the device can get powered off. Disable the
RPM-reference-held check for their duration.
- the IRQ, hangcheck and RPS work handlers
These handlers are flushed in the system/runtime suspend handler
before the device is powered off, so it's guaranteed that they won't
run while the device is powered off even though they don't hold any
RPM reference. Disable the RPM-reference-held check for their duration.
In all these cases we still check that the device is not suspended.
These explicit annotations also have the positive side effect of
documenting our assumptions better.
This caught additional WARNs from the atomic modeset path, those should
be fixed separately.
v2:
- remove the redundant HAS_RUNTIME_PM check (moved to patch 1) (Ville)
v3:
- use a new dedicated RPM wakelock refcount to also catch cases where
our own RPM get/put functions were not called (Chris)
- assert also that the new RPM wakelock refcount is 0 in the RPM
suspend handler (Chris)
- change the assert error message to be more meaningful (Chris)
- prevent false assert errors and check that the RPM wakelock is 0 in
the RPM resume handler too
- prevent false assert errors in the hangcheck work too
- add a device not suspended assert check to the hangcheck work
v4:
- rename disable/enable_rpm_asserts to disable/enable_rpm_wakeref_asserts
and wakelock_count to wakeref_count
- disable the wakeref asserts in the IRQ handlers and RPS work too
- update/clarify commit message
v5:
- mark places we plan to change to use proper RPM refcounting with
separate DISABLE/ENABLE_RPM_WAKEREF_ASSERTS aliases (Chris)
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1450227139-13471-1-git-send-email-imre.deak@intel.com
|
|
dev_priv is the new black. Or something.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1450271061-32646-3-git-send-email-jani.nikula@intel.com
|
|
While at it, move the declaration to where everything else is declared.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/7d9d879603038889f0128cf7cbbd9f591edc11dd.1450089383.git.jani.nikula@intel.com
|
|
Do some further clean up based on the initial review of
drm/i915: Separate cherryview from valleyview.
In this case check for '(GEN7 || GEN8) && HAS_PCH_SPLIT' since we're
reading FUSE_STRAP and SFUSE_STRAP which live in the PCH.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Wayne Boyer <wayne.boyer@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1449514270-15171-3-git-send-email-wayne.boyer@intel.com
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
|
|
The cherryview device shares many characteristics with the valleyview
device. When support was added to the driver for cherryview, the
corresponding device info structure included .is_valleyview = 1.
This is not correct and leads to some confusion.
This patch changes .is_valleyview to .is_cherryview in the cherryview
device info structure and simplifies the IS_CHERRYVIEW macro.
Then where appropriate, instances of IS_VALLEYVIEW are replaced with
IS_VALLEYVIEW || IS_CHERRYVIEW or equivalent.
v2: Use IS_VALLEYVIEW || IS_CHERRYVIEW instead of defining a new macro.
Also add followup patches to fix issues discovered during the first
review. (Ville)
v3: Fix some style issues and one gen check. Remove CRT related changes
as CRT is not supported on CHV. (Imre, Ville)
v4: Make a few more optimizations. (Ville)
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Wayne Boyer <wayne.boyer@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1449692975-14803-1-git-send-email-wayne.boyer@intel.com
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
|
|
Userspace can pass in an offset that it presumes the object is located
at. The kernel will then do its utmost to fit the object into that
location. The assumption is that userspace is handling its own object
locations (for example along with full-ppgtt) and that the kernel will
rarely have to make space for the user's requests.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
v2: Fixed incorrect eviction found by Michal Winiarski - fix suggested by Chris
Wilson. Fixed incorrect error paths causing crash found by Michal Winiarski.
(Not published externally)
v3: Rebased because of trivial conflict in object_bind_to_vm. Fixed eviction
to allow eviction of soft-pinned objects when another soft-pinned object used
by a subsequent execbuffer overlaps reported by Michal Winiarski.
(Not published externally)
v4: Moved soft-pinned objects to the front of ordered_vmas so that they are
pinned first after an address conflict happens to avoid repeated conflicts in
rare cases (Suggested by Chris Wilson). Expanded comment on
drm_i915_gem_exec_object2.offset to cover this new API.
v5: Added I915_PARAM_HAS_EXEC_SOFTPIN parameter for detecting this capability
(Kristian). Added check for multiple pinnings on eviction (Akash). Made sure
buffers are not considered misplaced without the user specifying
EXEC_OBJECT_SUPPORTS_48B_ADDRESS. User must assume responsibility for any
addressing workarounds. Updated object2.offset field comment again to clarify
NO_RELOC case (Chris). checkpatch cleanup.
v6: Trivial rebase on latest drm-intel-nightly
v7: Catch attempts to pin above the max virtual address size and return
EINVAL (Tvrtko). Decouple EXEC_OBJECT_SUPPORTS_48B_ADDRESS and
EXEC_OBJECT_PINNED flags, user must pass both flags in any attempt to pin
something at an offset above 4GB (Chris, Daniel Vetter).
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Akash Goel <akash.goel@intel.com>
Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Zou Nanhai <nanhai.zou@intel.com>
Cc: Kristian Høgsberg <hoegsberg@gmail.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Acked-by: PDT
Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1449575707-20933-1-git-send-email-thomas.daniel@intel.com
|
|
Linux 4.4-rc2
Backmerge to get at
commit 1b0e3a049efe471c399674fd954500ce97438d30
Author: Imre Deak <imre.deak@intel.com>
Date: Thu Nov 5 23:04:11 2015 +0200
drm/i915/skl: disable display side power well support for now
so that we can proplery re-eanble skl power wells in -next.
Conflicts are just adjacent lines changed, except for intel_fbdev.c
where we need to interleave the changs. Nothing nefarious.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
|
|
Found by static analysis tool.
Signed-off-by: Namrta Salonie <namrta.salonie@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
|
|
We need to initialize the display core part early, before initializing
the rest of the display power state. This is also described in the bspec
termed "Display initialization sequence". Atm we run this sequence
during driver loading after power domain HW state initialization which
is too late and during runtime suspend/resume which is unneeded and can
interere with DMC functionality which handles HW resources toggled
by this init/uninit sequence automatically. The init sequence must be
run as the first step of HW power state initialization and during
system resume. The uninit sequence must be run during system suspend.
To address the above move the init sequence to the initial HW power
state setup and the uninit sequence to a new power domains suspend
function called during system suspend.
As part of the init sequence we also have to reprogram the DMC firmware
as it's lost across a system suspend/resume cycle.
After this change CD clock initialization during driver loading will
happen only later after other dependent HW/SW parts are initialized,
while during system resume it will get initialized as the last step of
the init sequence. This distinction can be removed by some refactoring
of platform independent parts. I left this refactoring out from this
series since I didn't want to change non-SKL parts. This is a TODO for
later.
v2:
- fix error path in i915_drm_suspend_late()
- don't try to re-program the DMC firmware if it failed to load
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1447774433-20834-1-git-send-email-imre.deak@intel.com
|
|
As all csr firmware related opertion are not using any
any data structures of drm framework level, so better to
use dev_priv instead of dev. it's a new style! :)
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Tested-by: Daniel Stone <daniels@collabora.com> # SKL
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1446069547-24760-9-git-send-email-imre.deak@intel.com
|
|
This removes two anti-patterns:
- Locking shouldn't be used to synchronize with async work (of any
form, whether callbacks, workers or other threads). This is what the
mutex_lock/unlock seems to have been for in intel_csr_load_program.
Instead ordering should be ensured with the generic
wait_for_completion()/complete(). Or more specific functions
provided by the core kernel like e.g.
flush_work()/cancel_work_sync() in the case of synchronizing with a
work item.
- Don't invent own completion like the following code did with the
(already removed) wait_for(csr_load_status_get()) pattern - it's
really hard to get these right when you want them to be _really_
correct (and be fast) in all cases. Furthermore it's easier to read
code using the well-known primitives than new ones using
non-standard names.
Before enabling/disabling DC6 check if the firmware is loaded
successfully. This is guaranteed during runtime s/r, since otherwise we
don't enable RPM, but not during system s/r.
Note that it's still unclear whether we need to enable/disable DC6
during system s/r, until that's clarified, keep the current behavior and
enable/disable DC6.
Also after this patch there is a race during system s/r where the
firmware may not be loaded yet, that's addressed in an upcoming patch.
v2-v3:
- unchanged
v4:
- rebased on latest drm-intel-nightly
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
[imre: added code and note about checking if the firmware loaded ok,
before enabling/disabling it]
Reviewed-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Tested-by: Daniel Stone <daniels@collabora.com> # SKL
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1447341037-2623-1-git-send-email-imre.deak@intel.com
|
|
Skl is fully dependent on dmc for going to low power state (dc5/dc6).
This requires a trigger from rpm. To ensure the dmc firmware
is available for runtime pm support rpm-reference-count is used
by not releasing the rpm reference if firmware loading is
not completed.
So moved the intel_csr_ucode_init call after runtime pm enable.
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
[imre: moved the call right after power domain init to avoid race with
the console modesetting]
Signed-off-by: Imre Deak <imre.deak@intel.com>
Tested-by: Daniel Stone <daniels@collabora.com> # SKL
Reviewed-by: A.Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1446069547-24760-2-git-send-email-imre.deak@intel.com
|
|
Reading the driver load/unload code leaves one confused as there's
an async_schedule() in the load, but not async_synchronize_full()
in sight. In fact it's hidden inside intel_fbdev.c. So let's move the
async_schedule() into intel_fbdev.c as well so that it's next to the
async_synchronize_full(), which should make the relationship easier
to see.
Plus this way we won't schedule a nop function call when fbdev is
disabled. And we were passing a pointer to a static inline
function to async_schedule(), which seems rather dubious to me.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1446815313-9490-4-git-send-email-ville.syrjala@linux.intel.com
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
|
|
We set up fbdev last during load, so doing the fbdev cleanup should be
first.
We weren't supposed to drop the init power during driver unload, but since
the fbdev teardown happened after intel_power_domains_fini() that could
have happened due in one of two ways. First it could have happened
during the modeset caused by normal fbdev cleanup. But in addition it
could have happened already via the intel_fbdev_initial_config() since
that is executed asynhronously, and the async_synchronize_full() was
done during fbdev cleanup, after intel_power_domains_fini(). All of
that got eliminated by
commit 292b990e86abc ("drm/i915: Update power domains on readout.")
since we now drop the init power synchronously during driver load.
So there is no real bug wrt. the init power anymore, but still it seems
better to do the fbdev cleanup first, before we've potentially cleaned
up something else important.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1446815313-9490-3-git-send-email-ville.syrjala@linux.intel.com
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
|
|
The GuC firmware load requires struct_mutex to create a GEM object,
but this collides badly with request_firmware. Move struct_mutex
locking down into the loader itself, so we don't hold it across the
entire load process, including request_firmware.
[ 20.451400] ======================================================
[ 20.451420] [ INFO: possible circular locking dependency detected ]
[ 20.451441] 4.3.0-rc5+ #1 Tainted: G W
[ 20.451457] -------------------------------------------------------
[ 20.451477] plymouthd/371 is trying to acquire lock:
[ 20.451494] (&dev->struct_mutex){+.+.+.}, at: [<ffffffffa0093c62>]
drm_gem_mmap+0x112/0x290 [drm]
[ 20.451538]
but task is already holding lock:
[ 20.451557] (&mm->mmap_sem){++++++}, at: [<ffffffff811fd9ac>]
vm_mmap_pgoff+0x8c/0xf0
[ 20.451591]
which lock already depends on the new lock.
[ 20.451617]
the existing dependency chain (in reverse order) is:
[ 20.451640]
-> #3 (&mm->mmap_sem){++++++}:
[ 20.451661] [<ffffffff8110644e>] lock_acquire+0xce/0x1c0
[ 20.451683] [<ffffffff8120ec9a>] __might_fault+0x7a/0xa0
[ 20.451705] [<ffffffff8127e34e>] filldir+0x9e/0x130
[ 20.451726] [<ffffffff81295b86>] dcache_readdir+0x186/0x230
[ 20.451748] [<ffffffff8127e117>] iterate_dir+0x97/0x130
[ 20.451769] [<ffffffff8127e66a>] SyS_getdents+0x9a/0x130
[ 20.451790] [<ffffffff8184f2f2>] entry_SYSCALL_64_fastpath+0x12/0x76
[ 20.451829]
-> #2 (&sb->s_type->i_mutex_key#2){+.+.+.}:
[ 20.451852] [<ffffffff8110644e>] lock_acquire+0xce/0x1c0
[ 20.451872] [<ffffffff8184b516>] mutex_lock_nested+0x86/0x400
[ 20.451893] [<ffffffff81277790>] walk_component+0x1d0/0x2a0
[ 20.451914] [<ffffffff812779f0>] link_path_walk+0x190/0x5a0
[ 20.451935] [<ffffffff8127803b>] path_openat+0xab/0x1260
[ 20.451955] [<ffffffff8127a651>] do_filp_open+0x91/0x100
[ 20.451975] [<ffffffff81267e67>] file_open_name+0xf7/0x150
[ 20.451995] [<ffffffff81267ef3>] filp_open+0x33/0x60
[ 20.452014] [<ffffffff8157e1e7>] _request_firmware+0x277/0x880
[ 20.452038] [<ffffffff8157e9e4>] request_firmware_work_func+0x34/0x80
[ 20.452060] [<ffffffff810c7020>] process_one_work+0x230/0x680
[ 20.452082] [<ffffffff810c74be>] worker_thread+0x4e/0x450
[ 20.452102] [<ffffffff810ce511>] kthread+0x101/0x120
[ 20.452121] [<ffffffff8184f66f>] ret_from_fork+0x3f/0x70
[ 20.452140]
-> #1 (umhelper_sem){++++.+}:
[ 20.452159] [<ffffffff8110644e>] lock_acquire+0xce/0x1c0
[ 20.452178] [<ffffffff8184c5c1>] down_read+0x51/0xa0
[ 20.452197] [<ffffffff810c203b>]
usermodehelper_read_trylock+0x5b/0x130
[ 20.452221] [<ffffffff8157e147>] _request_firmware+0x1d7/0x880
[ 20.452242] [<ffffffff8157e821>] request_firmware+0x31/0x50
[ 20.452262] [<ffffffffa01b54a4>]
intel_guc_ucode_init+0xf4/0x400 [i915]
[ 20.452305] [<ffffffffa0213913>] i915_driver_load+0xd63/0x16e0 [i915]
[ 20.452343] [<ffffffffa00987d9>] drm_dev_register+0xa9/0xc0 [drm]
[ 20.452369] [<ffffffffa009ae3d>] drm_get_pci_dev+0x8d/0x1e0 [drm]
[ 20.452396] [<ffffffffa01521e4>] i915_pci_probe+0x34/0x50 [i915]
[ 20.452421] [<ffffffff81464675>] local_pci_probe+0x45/0xa0
[ 20.452443] [<ffffffff81465a6d>] pci_device_probe+0xfd/0x140
[ 20.452464] [<ffffffff8156a2e4>] driver_probe_device+0x224/0x480
[ 20.452486] [<ffffffff8156a5c8>] __driver_attach+0x88/0x90
[ 20.452505] [<ffffffff81567cf3>] bus_for_each_dev+0x73/0xc0
[ 20.452526] [<ffffffff81569a7e>] driver_attach+0x1e/0x20
[ 20.452546] [<ffffffff815695ae>] bus_add_driver+0x1ee/0x280
[ 20.452566] [<ffffffff8156b100>] driver_register+0x60/0xe0
[ 20.453197] [<ffffffff81464050>] __pci_register_driver+0x60/0x70
[ 20.453845] [<ffffffffa009b070>] drm_pci_init+0xe0/0x110 [drm]
[ 20.454497] [<ffffffffa027f092>] 0xffffffffa027f092
[ 20.455156] [<ffffffff81002123>] do_one_initcall+0xb3/0x200
[ 20.455796] [<ffffffff811d8c01>] do_init_module+0x5f/0x1e7
[ 20.456434] [<ffffffff8114c4e6>] load_module+0x2126/0x27d0
[ 20.457071] [<ffffffff8114cdf9>] SyS_finit_module+0xb9/0xf0
[ 20.457738] [<ffffffff8184f2f2>] entry_SYSCALL_64_fastpath+0x12/0x76
[ 20.458370]
-> #0 (&dev->struct_mutex){+.+.+.}:
[ 20.459773] [<ffffffff8110584f>] __lock_acquire+0x191f/0x1ba0
[ 20.460451] [<ffffffff8110644e>] lock_acquire+0xce/0x1c0
[ 20.461074] [<ffffffffa0093c88>] drm_gem_mmap+0x138/0x290 [drm]
[ 20.461693] [<ffffffff8121a5ec>] mmap_region+0x3ec/0x670
[ 20.462298] [<ffffffff8121abb2>] do_mmap+0x342/0x420
[ 20.462901] [<ffffffff811fd9d2>] vm_mmap_pgoff+0xb2/0xf0
[ 20.463532] [<ffffffff81218f62>] SyS_mmap_pgoff+0x1f2/0x290
[ 20.464118] [<ffffffff8102187b>] SyS_mmap+0x1b/0x30
[ 20.464702] [<ffffffff8184f2f2>] entry_SYSCALL_64_fastpath+0x12/0x76
[ 20.465289]
other info that might help us debug this:
[ 20.467179] Chain exists of:
&dev->struct_mutex --> &sb->s_type->i_mutex_key#2 -->
&mm->mmap_sem
[ 20.468928] Possible unsafe locking scenario:
[ 20.470161] CPU0 CPU1
[ 20.470745] ---- ----
[ 20.471325] lock(&mm->mmap_sem);
[ 20.471902] lock(&sb->s_type->i_mutex_key#2);
[ 20.472538] lock(&mm->mmap_sem);
[ 20.473118] lock(&dev->struct_mutex);
[ 20.473704]
*** DEADLOCK ***
Signed-off-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
|
|
One branch of the if clause uses pr_info, the other pr_err; change
the 'false' branch to also use pr_info. This minor oversight has gone
unfixed since the initial vga_switcheroo implementation in 6a9ee8af.
Signed-off-by: Ioan-Adrian Ratiu <adi@adirat.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1446246960-22620-1-git-send-email-adi@adirat.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|