- 05 Jan, 2016 4 commits
-
-
Alex Dai authored
Split GuC work queue space checking from submission and move it to ring_alloc_request_extras. The reason is that failure in later i915_add_request() won't be handled. In the case timeout happens, driver can return early in order to handle the error. v1: Move wq_reserve_space to ring_reserve_space v2: Move wq_reserve_space to alloc_request_extras (Chris Wilson) v3: The work queue head pointer is cached by driver now. So we can quickly return if space is available. s/reserve/check/g (Dave Gordon) v4: Update cached wq head after ring doorbell; check wq space before ring doorbell in case unexpected error happens; call wq space check only when GuC submission is enabled. (Dave Gordon) Signed-off-by:Alex Dai <yu.dai@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1450295155-10050-1-git-send-email-yu.dai@intel.comReviewed-by:
Dave Gordon <david.s.gordon@intel.com> Signed-off-by:
Daniel Vetter <daniel.vetter@ffwll.ch>
-
Chris Wilson authored
If the system has no available swap pages, we cannot make forward progress in the shrinker by releasing active pages, only by releasing purgeable pages which are immediately reaped. Take total_swap_pages into account when counting up available objects to be shrunk and subsequently shrinking them. By doing so, we avoid unbinding objects that cannot be shrunk and so wasting CPU cycles flushing those objects from the GPU to the system and then immediately back again (as they will more than likely be reused shortly after). Based on a patch by Akash Goel. v2: frontswap registers extra swap pages available for the system, so it is already include in the count of available swap pages. v3: Use get_nr_swap_pages() to query the currently available amount of swap space. This should also stop us from shrinking the GPU buffers if we ever run out of swap space. Though at that point, we would expect the oom-notifier to be running and failing miserably... Reported-by:
Akash Goel <akash.goel@intel.com> Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Cc: linux-mm@kvack.org Cc: Akash Goel <akash.goel@intel.com> Cc: sourab.gupta@intel.com Link: http://patchwork.freedesktop.org/patch/msgid/1449244734-25733-2-git-send-email-chris@chris-wilson.co.ukAcked-by:
Johannes Weiner <hannes@cmpxchg.org> Signed-off-by:
-
Chris Wilson authored
Some modules, like i915.ko, use swappable objects and may try to swap them out under memory pressure (via the shrinker). Before doing so, they want to check using get_nr_swap_pages() to see if any swap space is available as otherwise they will waste time purging the object from the device without recovering any memory for the system. This requires the nr_swap_pages counter to be exported to the modules. Signed-off-by:
Andrew Morton <akpm@linux-foundation.org> Acked-by:
-
Michał Winiarski authored
According to PRM, some parts of HW require the addresses to be in a canonical form, where bits [63:48] == [47]. Let's convert addresses to canonical form prior to relocating and return converted offsets to userspace. We also need to make sure that userspace is using addresses in canonical form in case of softpin. v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville) v3: Rebase on top of softpin, fix a hole in relocate_entry, s/expect/require (Chris) v4: Handle softpin in validate_exec_list (Chris) v5: Convert back to canonical form at copy_to_user time (Chris) v6: Don't use struct exec_object2 in place of exec_object v7: Use sign_extend64 for converting to canonical form (Joonas), reject non-canonical and non-page-aligned offset for softpin (Chris) v8: Convert back to non-canonical form in a function, split the test for EXEC_OBJECT_PINNED (Chris) v9: s/canonial/canonical, drop accidental double newline (Chris) Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michel Thierry <michel.thierry@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by:Michał Winiarski <michal.winiarski@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1451409892-13708-1-git-send-email-michal.winiarski@intel.com Testcase: igt/gem_bad_reloc/negative-reloc-blt Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92699 Cc: drm-intel-fixes@lists.freedesktop.org Reviewed-by:
-
- 04 Jan, 2016 1 commit
-
-
Insu Yun authored
Since devm_kzalloc can be failed, it needs to be checked if not, NULL dereference could be happened. Signed-off-by:
Insu Yun <wuninsu@gmail.com> Link: http://patchwork.freedesktop.org/patch/msgid/1451491169-35068-1-git-send-email-wuninsu@gmail.comSigned-off-by:
Jani Nikula <jani.nikula@intel.com>
-
- 30 Dec, 2015 1 commit
-
-
Ben Widawsky authored
Signed-off-by:
Ben Widawsky <benjamin.widawsky@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1451427643-7266-1-git-send-email-benjamin.widawsky@intel.comSigned-off-by:
-
- 23 Dec, 2015 1 commit
-
-
Gary Wang authored
The total delay of HDMI hotplug detecting with 30ms is sometimes not enoughtfor HDMI live status up with specific HDMI monitors in BSW platform. After doing experiments for following monitors, it needs 80ms at least for those worst cases. Lenovo L246 1xwA (4 failed, necessary hot-plug delay: 58/40/60/40ms) Philips HH2AP (9 failed, necessary hot-plug delay: 80/50/50/60/46/40/58/58/39ms) BENQ ET-0035-N (6 failed, necessary hot-plug delay: 60/50/50/80/80/40ms) DELL U2713HM (2 failed, necessary hot-plug delay: 58/59ms) HP HP-LP2475w (5 failed, necessary hot-plug delay: 70/50/40/60/40ms) It looks like 70-80 ms is BSW platform needs in some bad cases of the monitors at this end (8 times delay at most). Keep less than 100ms for HDCP pulse HPD low (with at least 100ms) to respond a plug out. Reviewed-by:
Cooper Chiou <cooper.chiou@intel.com> Tested-by:
Gary Wang <gary.c.wang@intel.com> Cc: Gavin Hindman <gavin.hindman@intel.com> Cc: Sonika Jindal <sonika.jindal@intel.com> Cc: Shashank Sharma <shashank.sharma@intel.com> Cc: Shobhit Kumar <shobhit.kumar@intel.com> Signed-off-by:
Shobhit Kumar <shobhit.kumar@intel.com> Cc: drm-intel-fixes@lists.freedesktop.org Fixes: 237ed86c ("drm/i915: Check live status before reading edid") Signed-off-by:
-
- 22 Dec, 2015 8 commits
-
-
Maarten Lankhorst authored
When the crtc is configured but not active we currently clip to (0,0)x(0,0). This results in differences in calculations depending on dpms setting. When the crtc is enabled but not active run check_plane as if it were on, but afterwards set plane_state->visible = false for the checks. Signed-off-by:
Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Reviewed-by:
Mika Kahola <mika.kahola@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1447945645-32005-13-git-send-email-maarten.lankhorst@linux.intel.com
-
Maarten Lankhorst authored
On skylake when calculating plane visibility with the crtc in dpms off mode the real cdclk may be different from what it would be if the crtc was active. This may result in a WARN_ON(cdclk < crtc_clock) from skl_max_scale. The fix is to keep a atomic_cdclk that would be true if all crtc's were active. This is required to get the same calculations done correctly regardless of dpms mode. Signed-off-by:
-
Maarten Lankhorst authored
Parallel modesets are still not allowed, but this will allow updating a different crtc during a modeset if the clock is not changed. Additionally when all pipes are DPMS off the cdclk will be lowered to the minimum allowed. Changes since v1: - Add dev_priv->active_crtcs for tracking which crtcs are active. - Rename min_cdclk to min_pixclk and move to dev_priv. - Add a active_crtcs mask which is updated atomically. - Add intel_atomic_state->modeset which is set on modesets. - Commit new pixclk/active_crtcs right after state swap. Changes since v2: - Make the changes related to max_pixel_rate calculations more readable. Changes since v3: - Add cherryview and missing WARN_ON to readout. Signed-off-by:
-
Maarten Lankhorst authored
This fixes a warning when the crtc is turned off. In that case fb will be NULL, and crtc_clock will be 0. Because the crtc is no longer active this is not a bug, and shouldn't trigger the WARN_ON. Also remove handling a null crtc_state, with all transitional helpers gone this can no longer happen. Signed-off-by:
-
Jani Nikula authored
Add an overview and documentation for the VBT/BDB header structures. Acked-by:
-
Ville Syrjälä authored
Atomic changes broke check_digital_port_conflicts(). It needs to look at the global situation instead of just trying to find a conflict within the current atomic state. This bug made my HSW explode spectacularly after I had split the DDI encoders into separate DP and HDMI encoders. With the fix, things seem much more solid. I hope holding the connection_mutex is enough protection that we can actually walk the connectors even if they're not part of the current atomic state... v2: Regenerate the patch so that it actually applies (Jani) Cc: stable@vger.kernel.org Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Fixes: 5448a00d ("drm/i915: Don't use staged config in check_digital_port_conflicts()") Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by:
-
Joonas Lahtinen authored
Using __stringify(x) instead of #x adds support for macros as a parameter and compile-time concatenation reduces the runtime overhead. Slightly increases the .text size but should not matter. v2: - Define I915_STATE_WARN_ON though I915_STATE_WARN (Bikeshed inspiration by Chris) v3: - More specific commit message v4: - Do not directly pass arbitary string as format, instead guard with "%s" (Dave) Cc: Rob Clark <robdclark@gmail.com> Cc: Dave Gordon <david.s.gordon@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v3) Acked-by:
Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1450441647-23924-3-git-send-email-joonas.lahtinen@linux.intel.comSigned-off-by:
-
Joonas Lahtinen authored
Take advantage of WARN return value to simplify the flow. Cc: Rob Clark <robdclark@gmail.com> Reviewed-by:
-
- 21 Dec, 2015 7 commits
-
-
Lukas Wunner authored
Clean up after 0c82312f ("drm/i915: Pin the ifbdev for the info->system_base GGTT mmapping"): At each of the remaining "goto out" in intelfb_alloc(), fb can only be either an ERR_PTR or NULL, so the call to drm_framebuffer_unreference() is now obsolete. Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
Lukas Wunner <lukas@wunner.de> Link: http://patchwork.freedesktop.org/patch/msgid/56756c41.c306c20a.d0602.1830SMTPIN_ADDED_MISSING@mx.google.comReviewed-by:
-
Ville Syrjälä authored
Turns out CHV pipe C was glued on somewhat poorly, and there's something wrong with the cursor. If the cursor straddles the left screen edge, and is then moved away from the edge or disabled, the pipe will often underrun. If enough underruns are triggered quickly enough the pipe will fall over and die (it just scans out a solid color and reports a constant underrun). We need to turn the disp2d power well off and on again to recover the pipe. None of that is very nice for the user, so let's just refuse to place the cursor in the compromised position. The ddx appears to fall back to swcursor when the ioctl returns an error, so theoretically there's no loss of functionality for the user (discounting swcursor bugs). I suppose most cursors images actually have the hotspot not exactly at 0,0 so under typical conditions the fallback will in fact kick in as soon as the cursor touches the left edge of the screen. Any atomic compositor should anyway be prepared to fall back to GPU composition when things don't work out, so there should be no problem with those. Other things that I tried to solve this include flipping all display related clock gating knobs I could find, increasing the minimum gtt alignment all the way up to 512k. I also tried to see if there are more specific screen coordinates that hit the bug, but the findings were somewhat inconclusive. Sometimes the failures happen almost across the whole left edge, sometimes more at the very top and around the bottom half. I wasn't able to find any real pattern to these variations, so it seems our only choice is to just refuse to straddle the left screen edge at all. Cc: stable@vger.kernel.org Cc: Jason Plum <max@warheads.net> Testcase: igt/kms_chv_cursor_fail Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92826Signed-off-by:
-
Joonas Lahtinen authored
Move all the bool variables to the end as per the comment. Reviewed-by:
-
Joonas Lahtinen authored
Otherwise usage in the i915 debug macros yields problems due to i915_drv.h <-> i915_trace.h <-> intel_drv.h include loops. v2: - Document not-so-obvious need for linux/cache.h (Chris) Reviewed-by:
-
Ben Widawsky authored
It is unclear if this is even required on BXT. v2: Make sure to set the default value to false. Uncertain how my compiler doesn't complain with v1. Cc: Imre Deak <imre.deak@intel.com> Signed-off-by:
Imre Deak <imre.deak@intel.com> Signed-off-by:
-
Gary Wang authored
The total delay of HDMI hotplug detecting with 30ms have already been split into a resolution of 3 retries of 10ms each, for the worst cases. But it still suffered from only waiting 10ms at most in intel_hdmi_detect(). This patch corrects it by reading hotplug status with 4 times at most for 30ms delay. v2: - straight up to loop execution for more clear in code readability - mdelay will replace with msleep by Daniel's new patch drm/i915: mdelay(10) considered harmful - suggest to re-evaluate try times for being compatible to old HDMI monitor Reviewed-by:
-
Chris Wilson authored
There was a silent conflict between commit 0a878716 Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Thu Oct 15 14:23:01 2015 +0200 drm/i915: restore ggtt double-bind avoidance and commit 5bab6f60 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Fri Oct 23 18:43:32 2015 +0100 drm/i915: Serialise updates to GGTT with access through GGTT on Braswell thankfully caught by the extra WARN safegaurd in 0a878716. Since we now override the GGTT insert_pages callback when installing the aliasing ppgtt, we assert that the callback is the original ggtt routine. However, on Braswell we now use a different insertion routine to serialise access through the GGTT with updating the PTE and hence the conflict. To avoid the conflict, move the custom insertion routine for Braswell down a level. Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com Signed-off-by:
-
- 18 Dec, 2015 8 commits
-
-
Mika Kuoppala authored
commit 344df980 ("drm/i915/skl: Disable coarse power gating up until F0") failed to take into account that the same workaround is used in guc when forcewake is sampled. Wrap the condition check inside a macro and use it in both places to fix the guc side scope. Cc: Dave Gordon <david.s.gordon@intel.com> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> Reviewed-by:
Sagar Arun Kamble <sagar.a.kamble@intel.com> Signed-off-by:
Mika Kuoppala <mika.kuoppala@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1450286318-6854-1-git-send-email-mika.kuoppala@intel.com
-
Daniel Vetter authored
Signed-off-by: -
Mika Kuoppala authored
The workarounds for disabling hdc invalidation and also forcing context to be non coherent, are advised to be used up until rev D0. However as it was found that rev F0, without the WaForceEnableNonCoherent might system hang if the mesa tried to use coherent mode. As these two workarounds are about non coherent access, are grouped in scope and they point the same HSD, increase the scope of both to set default behaviour to non coherent access. References: HSD: gen9lp/2131413 References: http://lists.freedesktop.org/archives/mesa-dev/2015-November/101515.html Cc: Ben Widawsky <benjamin.widawsky@intel.com> Cc: Francisco Jerez <currojerez@riseup.net> Reviewed-by:
Francisco Jerez <currojerez@riseup.net> Signed-off-by:
-
Chris Wilson authored
Limit busywaiting only to the request currently being processed by the GPU. If the request is not currently being processed by the GPU, there is a very low likelihood of it being completed within the 2 microsecond spin timeout and so we will just be wasting CPU cycles. v2: Check for logical inversion when rebasing - we were incorrectly checking for this request being active, and instead busywaiting for when the GPU was not yet processing the request of interest. v3: Try another colour for the seqno names. v4: Another colour for the function names. v5: Remove the forced coherency when checking for the active request. On reflection and plenty of recent experimentation, the issue is not a cache coherency problem - but an irq/seqno ordering problem (timing issue). Here, we do not need the w/a to force ordering of the read with an interrupt. Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: "Rogozhkin, Dmitry V" <dmitry.v.rogozhkin@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: Eero Tamminen <eero.t.tamminen@intel.com> Cc: "Rantala, Valtteri" <valtteri.rantala@intel.com> Cc: stable@vger.kernel.org Signed-off-by:
-
Chris Wilson authored
When waiting for high frequency requests, the finite amount of time required to set up the irq and wait upon it limits the response rate. By busywaiting on the request completion for a short while we can service the high frequency waits as quick as possible. However, if it is a slow request, we want to sleep as quickly as possible. The tradeoff between waiting and sleeping is roughly the time it takes to sleep on a request, on the order of a microsecond. Based on measurements of synchronous workloads from across big core and little atom, I have set the limit for busywaiting as 10 microseconds. In most of the synchronous cases, we can reduce the limit down to as little as 2 miscroseconds, but that leaves quite a few test cases regressing by factors of 3 and more. The code currently uses the jiffie clock, but that is far too coarse (on the order of 10 milliseconds) and results in poor interactivity as the CPU ends up being hogged by slow requests. To get microsecond resolution we need to use a high resolution timer. The cheapest of which is polling local_clock(), but that is only valid on the same CPU. If we switch CPUs because the task was preempted, we can also use that as an indicator that the system is too busy to waste cycles on spinning and we should sleep instead. __i915_spin_request was introduced in commit 2def4ad9 [v4.2] Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Tue Apr 7 16:20:41 2015 +0100 drm/i915: Optimistically spin for the request completion v2: Drop full u64 for unsigned long - the timer is 32bit wraparound safe, so we can use native register sizes on smaller architectures. Mention the approximate microseconds units for elapsed time and add some extra comments describing the reason for busywaiting. v3: Raise the limit to 10us v4: Now 5us. Reported-by:
Jens Axboe <axboe@kernel.dk> Link: https://lkml.org/lkml/2015/11/12/621Reviewed-by:
-
Chris Wilson authored
The busywait in __i915_spin_request() does not respect pending signals and so may consume the entire timeslice for the task instead of returning to userspace to handle the signal. In the worst case this could cause a delay in signal processing of 20ms, which would be a noticeable jitter in cursor tracking. If a higher resolution signal was being used, for example to provide fairness of a server timeslices between clients, we could expect to detect some unfairness between clients (i.e. some windows not updating as fast as others). This issue was noticed when inspecting a report of poor interactivity resulting from excessively high __i915_spin_request usage. Fixes regression from commit 2def4ad9 [v4.2] Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Tue Apr 7 16:20:41 2015 +0100 drm/i915: Optimistically spin for the request completion v2: Try to assess the impact of the bug Signed-off-by:
-
Imre Deak authored
pm_runtime_{use,dont_use}_autosuspend() controls whether the device's sysfs power/autosuspend_delay_ms file is writeable or returns -EIO on access to user space. Since commit 25b181b4 Author: Imre Deak <imre.deak@intel.com> Date: Thu Dec 17 13:44:56 2015 +0200 drm/i915: get a permanent RPM reference on platforms w/o RPM support this sysfs file is writeable also on platforms without RPM support, but userspace (at least IGT) depends on this file being unchangable to determine whether the device supports runtime PM at all. So restore the old behavior. This gets rid of igt/pm_rpm failures on old platforms without RPM support, where the test should be skipped. Testcase: igt/pm_rpm/basic-rte Signed-off-by:David Weinehall <david.weinehall@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1450371873-878-1-git-send-email-imre.deak@intel.com
-
Jani Nikula authored
Use dev_priv rather than dev pointer where applicable. Remove plenty of unnecessary temp variables. No functional changes. Reviewed-by:
-
- 17 Dec, 2015 10 commits
-
-
Matt Roper authored
If we fail to reconstruct the BIOS fb (e.g., because the FB is too large), we'll be left with plane state that indicates the primary plane is visible yet has a NULL fb. This mismatch causes problems later on (e.g., for the watermark code). Since we've failed to reconstruct the BIOS FB, the best solution is to just disable the primary plane and pretend the BIOS never had it enabled. v2: Add intel_pre_disable_primary() call (Maarten) Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Ville Syrjälä <ville.syrjala@intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: drm-intel-fixes@lists.freedesktop.org Signed-off-by:
Matt Roper <matthew.d.roper@intel.com> Reviewed-by:
-
Chris Wilson authored
A long time ago (before 3.14) we relied on a permanent pinning of the ifbdev to lock the fb in place inside the GGTT. However, the introduction of stealing the BIOS framebuffer and reusing its address in the GGTT for the fbdev has muddied waters and we use an inherited fb. However, the inherited fb is only pinned whilst it is active and we no longer have an explicit pin for the info->system_base mmapping used by the fbdev. The result is that after some aperture pressure the fbdev may be evicted, but we continue to write the fbcon into the same GGTT address - overwriting anything else that may be put into that offset. The effect is most pronounced across suspend/resume as intel_fbdev_set_suspend() does a full clear over the whole scanout. v2: Only unpin the intel_fb is we allocate it. If we inherit the fb from the BIOS, we do not own the pinned vma (except for the reference we add in this patch for our access via info->screen_base). v3: Finish balancing the vma pinning for the normal !preallocated case. v4: Try to simplify the pinning even further. v5: Leak the VMA (cleaned up by object-free) to avoid complicated error paths. Signed-off-by:
-
Chris Wilson authored
As we mark the preallocated objects as bound, we should also flag them correctly as being map-and-fenceable (if appropriate!) so that later users do not get confused and try and rebind the pinned vma in order to get a map-and-fenceable binding. Signed-off-by:
Jesse Barnes <jbarnes@virtuousgeek.org> Signed-off-by:
-
Daniel Vetter authored
I missed this myself when reviewing commit 237ed86c Author: Sonika Jindal <sonika.jindal@intel.com> Date: Tue Sep 15 09:44:20 2015 +0530 drm/i915: Check live status before reading edid Long sleeps like this really shouldn't waste cpu cycles spinning. Cc: Sonika Jindal <sonika.jindal@intel.com> Cc: "Wang, Gary C" <gary.c.wang@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1449859455-32609-1-git-send-email-daniel.vetter@ffwll.chReviewed-by:
Sonika Jindal <sonika.jindal@intel.com> Signed-off-by:
-
Imre Deak authored
The device should be on for the whole duration of the update, so check for this. v2: - use the existing dev_priv directly everywhere (Ville) v3: - check also that we are in an RPM atomic section (Chris) - add the assert to i915_ggtt_insert_entries/clear_range too (Chris) Signed-off-by:
-
Imre Deak authored
In some cases we want to check whether we hold an RPM wakelock reference for the whole duration of a sequence. To achieve this add a new RPM atomic sequence counter that we increment any time the wakelock refcount drops to zero. Check whether the sequence number stays the same during the atomic section and that we hold the wakelock at the beginning of the section. Motivated by Chris. v2-v3: - unchanged v4: - swap the order of atomic_read() and assert_rpm_wakelock_held() in assert_rpm_atomic_begin() to avoid race Signed-off-by:
-
Imre Deak authored
With this change we have the corresponding wake lock checks in both the rpm get and put functions. v2-v3: - unchanged v4: - keep the corresponding check in the get helper (Chris) v5: - add a note to the commit message that with this change we have the checks both in the rpm get and put functions (Joonas) Signed-off-by:
-
Imre Deak authored
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 authored
Signed-off-by:
-
Imre Deak authored
As a preparation for follow-up patches add a new helper that checks whether we hold an RPM reference, since this is what we want most of the cases. Atm this helper will only check for the HW suspended state, a follow-up patch will do the actual change to check the refcount instead. One exception is the forcewake release timer function, where it's guaranteed that the HW is on even though the RPM refcount drops to zero. This guarantee is provided by flushing the timer in the runtime suspend handler. So leave the assert_device_not_suspended check in place there. Also rename assert_device_suspended for consistency and export these helpers as a preparation for the follow-up patches. No functional change. v3: - change the assert warning message to be more meaningful (Chris) Signed-off-by:
-
