- 05 Dec, 2017 2 commits
-
-
Chris Wilson authored
From the shrinker paths, we want to relinquish the GPU and GGTT access to the object, releasing the backing storage back to the system for swapout. As a part of that process we would unpin the pages, marking them for access by the CPU (for the swapout/swapin). However, if that process was interrupted after unbind the vma, we missed a flush of the inflight GGTT writes before we made that GTT space available again for reuse, with the prospect that we would redirect them to another page. The bug dates back to the introduction of multiple GGTT vma, but the code itself dates to commit 02bef8f9 ("drm/i915: Unbind closed vma for i915_gem_object_unbind()"). Fixes: 02bef8f9 ("drm/i915: Unbind closed vma for i915_gem_object_unbind()") Fixes: c5ad54cf ("drm/i915: Use partial view in mmap fault handler") Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: stable@vger.kernel.org Reviewed-by:
Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20171204132513.7303-1-chris@chris-wilson.co.uk
-
Anusha Srivatsa authored
There is a new version of dmc available for skylake. Following additions from ver1.27 1. Fix for the issue where DC_STATE was getting enabled even when disabled by driver causing data corruption. Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by:
Anusha Srivatsa <anusha.srivatsa@intel.com> Reviewed-by:
Imre Deak <imre.deak@intel.com> Signed-off-by:
-
- 04 Dec, 2017 2 commits
-
-
Lionel Landwerlin authored
We apply this logic to Gen9 as well. We didn't notice this issue as most part we've encountered so far only use the crystal as source for their timestamp registers. Fixes: dab91783 ("drm/i915: expose command stream timestamp frequency to userspace") Reviewed-by:
Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by:
Lionel Landwerlin <lionel.g.landwerlin@intel.com> Signed-off-by:
-
Randy Dunlap authored
Fix documentation build errors after intel_guc_loader.c was renamed to intel_guc_fw.c. Error: Cannot open file ../drivers/gpu/drm/i915/intel_guc_loader.c WARNING: kernel-doc '../scripts/kernel-doc -rst -enable-lineno -function GuC-specific firmware loader ../drivers/gpu/drm/i915/intel_guc_loader.c' failed with return code 1 Error: Cannot open file ../drivers/gpu/drm/i915/intel_guc_loader.c Error: Cannot open file ../drivers/gpu/drm/i915/intel_guc_loader.c WARNING: kernel-doc '../scripts/kernel-doc -rst -enable-lineno -internal ../drivers/gpu/drm/i915/intel_guc_loader.c' failed with return code 2 Fixes: e8668bbc ("drm/i915/guc: Rename intel_guc_loader.c to intel_guc_fw.c") Signed-off-by:
Randy Dunlap <rdunlap@infradead.org> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Signed-off-by:
Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/1b214f53-47f5-bef3-f58e-8136de5678ed@infradead.org
-
- 02 Dec, 2017 1 commit
-
-
Rodrigo Vivi authored
Signed-off-by:Rodrigo Vivi <rodrigo.vivi@intel.com>
-
- 01 Dec, 2017 13 commits
-
-
James Ausmus authored
Without masking out the old value, we can end up pointing the DDI to a disabled PLL, which makes the system fall over. Mask out the previous value before setting the PLL to DDI mapping. This can be observed by running igt/testdisplay with both an eDP and HDMI/DP output active. v2: Add the Bugzilla link Fixes: 555e38d2 ("drm/i915/cnl: DDI - PLL mapping") Testcase: igt/testdisplay Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103997 Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Matt Atwood <matthew.s.atwood@intel.com> Signed-off-by:
James Ausmus <james.ausmus@intel.com> Reviewed-by:
-
Chris Wilson authored
It has been many years since the last confirmed sighting (and fix) of an RC6 related bug (usually a system hang). Remove the parameter to stop users from setting dangerous values, as they often set it during triage and end up disabling the entire runtime pm instead (the option is not a fine scalpel!). Furthermore, it allows users to set known dangerous values which were intended for testing and not for production use. For testing, we can always patch in the required setting without having to expose ourselves to random abuse. v2: Fixup NEEDS_WaRsDisableCoarsePowerGating fumble, and document the lack of ilk support better. v3: Clear intel_info->rc6p if we don't support rc6 itself. Signed-off-by:
-
Chris Wilson authored
As we declare the GPU wedged if the reset fails, such a failure is quite terminal. Before taking that drastic action, let's sleep first and try active, in the hope that the hardware has quietened down and is then able to reset. After a few such attempts, it is fair to say that the HW is truly wedged. v2: Always print the failure message now, we precheck whether resets are disabled. References: https://bugs.freedesktop.org/show_bug.cgi?id=104007Signed-off-by:
Mika Kuoppala <mika.kuoppala@linux.intel.com>
-
Ville Syrjälä authored
Reject interlaced modes on VLV/CHV DP outputs. This simply does not work correctly in the hardware. We do get some output, but it's quite corrupted. The available documentation fails to mention this fact. I contacted some hardware people who eventually managed to locate the relevant HSD for VLV, which was resolved by declaring interlaced DP output as not supported. The HSD was never cloned for CHV even though it inherited most of the hardware and thus has the same problems with interlaced DP output. Cc: Dennis Vshivkov <awesome.walrus+bugzilla@gmail.com> Reported-by:
Dennis Vshivkov <awesome.walrus+bugzilla@gmail.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103922Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20171129180847.30613-1-ville.syrjala@linux.intel.comAcked-by:
-
Ville Syrjälä authored
Get rid of the crtc->config usages from within intel_pipe_{enable,disable}() by passing in the appropriate crtc state. Signed-off-by: -
Ville Syrjälä authored
We should make sure the pipe has fully started when we enable it from the i830 "power well". Otherwise theoretically i830 could also hit problems with vblank timestamps jumping around (since we skip the wait during modeset on i830). Additionally moving planes between the pipes etc. might not work correctly until both pipes are actually up and running. v2: Less pointless duplication in the code (Chris) Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
-
Ville Syrjälä authored
Previously I was under the impression that the scanline counter reads 0 when the pipe is off. Turns out that's not correct, and instead the scanline counter simply stops when the pipe stops, and it retains it's last value until the pipe starts up again, at which point the scanline counter jumps to vblank start. These jumps can cause the timestamp to jump backwards by one frame. Since we use the timestamps to guesstimage also the frame counter value on gen2, that would cause the frame counter to also jump backwards, which leads to a massice difference from the previous value. The end result is that flips/vblank events don't appear to complete as they're stuck waiting for the frame counter to catch up to that massive difference. Fix the problem properly by actually making sure the scanline counter has started to move before we assume that it's safe to enable vblank processing. v2: Less pointless duplication in the code (Chris) Cc: stable@vger.kernel.org Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
-
Ville Syrjälä authored
i830_disable_pipe() gets called from the power well code, and thus we're already holding the power domain mutex. That means we can't call plane->get_hw_state() as it will also try to grab the same mutex and will thus deadlock. Replace the assert_plane() calls (which calls ->get_hw_state()) with just raw register reads in i830_disable_pipe(). As a bonus we can now get a warning if plane C is enabled even though we don't even expose it as a drm plane. v2: Do a separate WARN_ON() for each plane (Chris) Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
-
Ville Syrjälä authored
Transcoder EDP does not support audio. Let's not try to read the state of the audio enable bit HSW_AUD_PIN_ELD_CP_VLD based on the pipe when using transcoder EDP. While at it make the function static and flatten it. Signed-off-by:
-
Ville Syrjälä authored
No point in adding the "force audio" property to DP connectors that don't support audio (g4x or port A). Signed-off-by:
-
Ville Syrjälä authored
Apparently g4x doesn't support audio over DP. Bspec lists the bit as "Reserved for Audio Output Enable", and empirical evidence tells us that the bit won't stick. So stop trying to enable DP audio on g4x. Link: https://patchwork.freedesktop.org/patch/msgid/20171129164303.18793-1-ville.syrjala@linux.intel.comReviewed-by:
-
Chris Wilson authored
To execute a requests requires us to have first woken the device, using the rpm wakeref (as the request needs to write to hardware to setup the context/ppGTT and execute on the GPU). So call intel_runtime_pm_get() around queuing the request; the request itself will then carry a wakeref until completion. References: https://bugs.freedesktop.org/show_bug.cgi?id=103994Signed-off-by:
-
Chris Wilson authored
When capturing the bo, we allocate an error object with an array of min(vma->size, vma->node.size) pages, plus a bit for compression overhead. However, when creating the fake vma to describe the bo, only one of the sizes was filled in, resulting in a too small array. Through my and CI testing, this was sufficient for the mostly empty NULL context as it compressed well (or the out-of-bounds access simply didn't cause an issue). However, in real workloads on Cannonlake, we were overflowing that array and causing havoc with the random memory corruption. Reported-by:
Rafael Antognolli <rafael.antognolli@intel.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103964 Fixes: 4e90a6e2 ("drm/i915: Record default HW state in the GPU error state") Signed-off-by:
Rodrigo Vivi <rodrigo.vivi@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20171201001536.13941-1-chris@chris-wilson.co.ukReviewed-by:
-
- 30 Nov, 2017 6 commits
-
-
Chris Wilson authored
drivers/gpu/drm/i915/i915_irq.c|3096 col 29| error: comparison between signed and unsigned integer expressions [-Werror=sign-compare] Signed-off-by:
Michal Wajdeczko <michal.wajdeczko@intel.com>
-
Maarten Lankhorst authored
This comment predates atomic, and I think with the way we currently track IPS, it's safe to enable this for the case we switch too. Changes since v1: - Keep IPS enabled when switching planes. Changes since v2: - Enable IPS when at least one plane is enabled. (Ville) Changes since v3: - Actually do what was advertised in v3, sigh! (Ville, CI) Signed-off-by:
Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20171122183906.47767-1-maarten.lankhorst@linux.intel.comReviewed-by:
-
Maarten Lankhorst authored
ips_enabled was used as a variable of whether IPS can be enabled or not, but should be used to test whether IPS is actually enabled. Changes since v1: - Call needs_modeset on new crtc state. (Ville) - IPS can be enabled with sprite plane enabled too. (Ville) - Fix CDCLK vs IPS workaround. (Ville) Changes since v2: - Only re-enable fastset when inheriting mode. (Ville) - Put the conditions for enabling and disabling IPS in a helper. Changes since v3: - Keep the max_cdclk workaround working. (Ville) - Also check logical cdclk out of paranoia. - Remove planes check from IPS disable function for initial disable. - Remove assert_plane_enabled/disabled checks and use crtc_state->active_planes for hsw_enable_ips only, always allow calling hsw_disable_ips to disable it initially in hw. Signed-off-by:
-
Imre Deak authored
We store a SW state of the t11_t12 timing in 100usec units but have to program it in 100msec as required by HW. The rounding used during programming means there will be a mismatch between the SW and HW states of this value triggering a "PPS state mismatch" error. Avoid this by storing the already rounded-up value in the SW state. Note that we still calculate panel_power_cycle_delay with the finer 100usec granularity to avoid any needless waits using that version of the delay. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103903 Cc: joks <joks@linux.pl> Signed-off-by:
-
Chris Wilson authored
If the HW is already wedged, attempting to submit a request will generate an -EIO. If we tried this during suspend, we would abort whereas all we want to do is to go sleep and throw away the corrupt state. Fixes: 5ab57c70 ("drm/i915: Flush logical context image out to memory upon suspend") Testcase: igt/gem_eio/suspend Signed-off-by:
-
Valtteri Rantala authored
Testing the texture read performance shows that the same tuning for the SQ credits is needed on GLK as on BXT/APL. This has been also confirmed by Altug from the HW team. V4: Rebase + fix Signed-off-by:
Valtteri Rantala <valtteri.rantala@intel.com> Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> (v1) Reviewed-by:
-
- 29 Nov, 2017 6 commits
-
-
Sagar Arun Kamble authored
This patch makes v9.39 firmware as default firmware for KBL. Note: GuC logging control is changed with this firmware. GuC is expecting i915 to set control bit to enable "default logging" while using GuC action UK_LOG_ENABLE_LOGGING. However i915 is currently not doing this because it is version specific change and can be handled entirely in GuC. It will need to be fixed in future firmwares. This update includes (since v9.14): - DCC spec changes for BXT + DCT enabling - Bug Fix for power conservation feature SLPC_DCC - Scheduler 1-element submission during DCC cycles. - SB based Pre-ETM/ETM flow enabling for debug signed GuC/HuC - Moving GuC non_critical r/w data to lower SRAM 64KB - Media engine Reset fix. Correctly marking context for resubmission in Media Reset case. - ABT Disable bug fix. Disabled Evaluation mode on context change. - Async FW in Engine Schedule feature (not enabled from KMD) - GuC clean up to align developer build in line to production build. - Disable ARAT interrupt before programming ARAT delta. - Memory range check in Parse to avoid failure due to overflow. - GuC Msg Channel Hang WA - Stall GUC for mmio access when IDI is low during CPD flow. - Fix for submit queue over flow issue - Enabling IBC on KBL GT3 15W, GT4 45W - Disabling wrong device ID WA in production signed kernel - Enabling WA for MSGCH hang issue upto required KBL stepping - Clear forcewake in CSB when SQ is empty. - 3Tries of GuC2CSME wake request - During reset one parameter was not getting accounted - Disable DCC 1-elem mode submission - Move UkGuckmdInterface.h file from 2016 folders to common 2016 folder. - This is file location change.No functional change done as part of this check in. - Enabling Guc Log changes for ultra low logging for OCA - Enabling Dynamic Render Power Well Hysteresis Programming for Compute Worklaods - Enabling build failure check to catch critical section overflow. - Disable build.bat redundant prints. - Move few least used functions to non-critical section. - Rearrange GuC documentation folder structure. - Synchronize SLPC internal debug interface with other branches. - Fixing Issue with Default Guc Log changes for OCA using special Control Bit - Aggressive DCC implementation for supported platforms. v2: Rebase. Updated commit message. Signed-off-by:
Jeff McGee <jeff.mcgee@intel.com> Signed-off-by:
Sagar Arun Kamble <sagar.a.kamble@intel.com> Cc: Spotswood John A <john.a.spotswood@intel.com> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Anusha Srivatsa<anusha.srivatsa@intel.com > Signed-off-by:
-
Sagar Arun Kamble authored
This patch makes v9.29 firmware as default firmware for BXT. Note: GuC logging control is changed with this firmware. GuC is expecting i915 to set control bit to enable "default logging" while using GuC action UK_LOG_ENABLE_LOGGING. However i915 is currently not doing this because it is version specific change and can be handled entirely in GuC. It will need to be fixed in future firmwares. This update includes (since v8.7): - Added support to log media reset count for host to read it - BXT WA for fixing MTP hangs. WaDisableDOPRenderClkGatingAtSubmit - Sub-feature level control for power management features. - Minor clean-up for power management interface. - Unified power management interface and scheduler interface into 1 file using same version. - Bug Fix for multi context scheduler flag. - DCC spec changes for BXT + DCT enabling - Springboard based Pre-ETM/ETM flow enabling for debug signed GuC/HuC - Moving GuC non_critical r/w data to lower SRAM 64KB - Enabled IBC for BXT - Media engine Reset fix. Correctly marking context for resubmission in Media Reset case. - SLPC Dynamic RPe fix to resolve issues where incorrect frequency was set. - ABT Disable bug fix. Disabled Evaluation mode on context change. - GuC clean up to align developer build in line to production build. - Disable ARAT interrupt before programming ARAT delta. - Memory range check in Parse to avoid failure due to overflow. - Clear forcewake in CSB when SQ is empty. - SLPC IBC 1.6 for APL to ensure multiplier does not cap IA below Pe. - Move UkGuckmdInterface.h file from 2016 folders to common 2016 folder. - This is file location change. No functional change done as part of this check in. - 3 tries of wake request needed from GuC2CSME for ME to wake up. Request has come from ME spec - During reset one parameter was not getting accounted - Enabling Guc Log changes for ultra low logging for OCA - Disable build.bat redundant prints. - Move few least used functions to non-critical section. - Rearrange GuC documentation folder structure. - Fixing Issue with Default Guc Log changes for OCA using special Control Bit v2: Rebase. Updated commit message. Signed-off-by:
-
Sagar Arun Kamble authored
This patch makes v9.33 firmware as default firmware for SKL. Note: GuC logging control is changed with this firmware. GuC is expecting i915 to set control bit to enable "default logging" while using GuC action UK_LOG_ENABLE_LOGGING. However i915 is currently not doing this because it is version specific change and can be handled entirely in GuC. It will need to be fixed in future firmwares. This update includes (since v6.1): - HuC RSA Keys updated. - Adding per engine preemption support in GuC scheduler - Minor bug fixes. - Added support to log media reset count for host to read it - Sub-feature level control for power management features. - Minor clean-up for power management interface. - Unified power management interface and scheduler interface into 1 file using same version. - Bug Fix for multi context scheduler flag. - DCC spec changes for BXT + DCT enabling - SB based Pre-ETM/ETM flow enabling for debug signed GuC/HuC - Moving GuC non_critical r/w data to lower SRAM 64KB - Media engine Reset fix. Correctly marking context for resubmission in Media Reset case. - ABT Disable bug fix. Disabled Evaluation mode on context change. - Async FW in Engine Schedule feature (not enabled from KMD) - GuC clean up to align developer build in line to production build. - DCC consistency fix for SKL - Disable ARAT interrupt before programming ARAT delta. - Memory range check in Parse to avoid failure due to overflow. - Enabled WA for MSGCH hang issue - Clear forcewake in CSB when SQ is empty. - Move UkGuckmdInterface.h file from 2016 folders to common 2016 folder. - This is file location change.No functional change done as part of this check in. - Enable decoupled freq for SKL GT4 - 3 tries of wake request needed from GuC2CSME for ME to wake up. Request has come from ME spec - During reset one parameter was not getting accounted - Enabling Guc Log changes for ultra low logging for OCA - Enabling build failure check to catch critical section overflow. - Disable build.bat redundant prints. - Move few least used functions to non-critical section. - Rearrange GuC documentation folder structure. - Synchronize SLPC internal debug interface with other branches. - Fixing Issue with Default Guc Log changes for OCA using special Control Bit v2: Rebase. Updated commit message. Signed-off-by:
-
Tvrtko Ursulin authored
Sagar noticed the check can be consolidated between the engine stats implementation and the PMU. My first choice was a static inline helper but that got into include ordering mess quickly fast so I went with a macro instead. At some point we should perhaps looking into taking out the non-ringubffer bits from intel_ringbuffer.h into a new intel_engine.h or something. v2: Use engine->flags. (Chris Wilson) v3: Rebase and mark GuC as not yet supported. (Chris Wilson) v4: Move flag setting to intel_engines_reset_default_submission. (Chris Wilson) v5: Move flag setting to logical_ring_setup. v6: intel_engines_reset_default_submission is the wrong place to set the flag - it needs to be in execlists_set_default_submission. (Sagar) v7: Flag setting in logical_ring_setup is not required. (Chris) Signed-off-by:Tvrtko Ursulin <tvrtko.ursulin@intel.com> Suggested-by:
-
Tvrtko Ursulin authored
Will be adding a new per-engine flags shortly so it makes sense to consolidate. v2: Keep the original code flow in intel_engine_cleanup_cmd_parser. (Joonas Lahtinen) Signed-off-by: -
Joonas Lahtinen authored
We seem to be missing some W/A for 2M pages and are getting a hit on raw GPU read bandwidths (even 30%) even though the GPU write bandwidths improve (even 10%). For now, disable THP, which is our only practical source of 2M pages until we have a W/A for the issue. v2: - Be explicit that we talk about GPU bandwidths (Eero) - s/deny/never/ because that's why (Chris) Reported-by:
Matthew Auld <matthew.auld@intel.com> Tested-by:
-
- 28 Nov, 2017 2 commits
-
-
Tvrtko Ursulin authored
In commit 0426c046 ("drm/i915/pmu: Only allow running on a single CPU") I attempted to clarify the CPU hotplug logic in our PMU implementation, but missed that a more logical error to return, when attempting to initialize an event on a currently inactive CPU, is -EINVAL rather than -ENODEV. This is because i915 PMU explicitly disallows running counters on more than one CPU at a time, and is not reporting that the requested CPU does not exist, or is off-line. Signed-off-by:
-
Chris Wilson authored
Previously we would enable hotplug polling on the outputs immediately upon construction. This would allow a very early hotplug event to trigger before we had finishing setting up the driver to handle it. Instead, move the output polling to the last step of registration, after we have set up all handlers, including the fbdev configuration. v2: Symmetrically turnoff the hotplug helper in unregister after the fbdev is first synchronised then finalized. This stops a late hotplug event being processed after the interrupts are disabled. Signed-off-by:
-
- 27 Nov, 2017 7 commits
-
-
Chris Wilson authored
The kerneldoc markup for i915_gem_timelines_mark_idle() was incorrect, so take the opportunity to also convert it from the "mark_idle" to "park" naming scheme. drivers/gpu/drm/i915/i915_gem_timeline.c:120: warning: No description found for parameter 'i915' Signed-off-by:
-
Chris Wilson authored
Switch over from the non-descript dev_priv locals to i915. Signed-off-by:
-
Chris Wilson authored
Since the shrinker is registered and unregistered during i915_driver_register and i915_driver_unregister, respectively, rename the init/cleanup functions to match. Signed-off-by:
-
Chris Wilson authored
It may be of interest to both compare the active HW context against the default (aka NULL) context, to see what has been changed and if either are corrupt. v2: Rename the fake vma as fake. Signed-off-by:
-
Chris Wilson authored
Even though all rendering should have been flushed at the end of the previous requests, add an extra flush after switching to the kernel_context. As the switch to the kernel_context is used when idling the gpu (e.g. suspend), having an extra layer of paranoia to ensure everything is flushed to memory seems sensible. Signed-off-by:
-
Arnd Bergmann authored
Building i915 without backlight support results in a harmless warning for intel_panel_set_backlight: drivers/gpu/drm/i915/intel_panel.c:653:13: error: 'intel_panel_set_backlight' defined but not used [-Werror=unused-function] This moves it into the CONFIG_BACKLIGHT_CLASS_DEVICE section that its caller is in. Signed-off-by:
Arnd Bergmann <arnd@arndb.de> Signed-off-by:
-
Arnd Bergmann authored
The alternative intel_backlight_device_register() definition apparently never got used, but I have now run into a case of i915 being compiled without CONFIG_BACKLIGHT_CLASS_DEVICE, resulting in a number of identical warnings: drivers/gpu/drm/i915/intel_drv.h:1739:12: error: 'intel_backlight_device_register' defined but not used [-Werror=unused-function] This marks the function as 'inline', which was surely the original intention here. Fixes: 1ebaa0b9 ("drm/i915: Move backlight registration to connector registration") Signed-off-by:
-
- 26 Nov, 2017 1 commit
-
-
Chris Wilson authored
As both the hotplug event and fbdev configuration run asynchronously, it is possible for them to run concurrently. If configuration fails, we were freeing the fbdev causing a use-after-free in the hotplug event. <7>[ 3069.935211] [drm:intel_fb_initial_config [i915]] Not using firmware configuration <7>[ 3069.935225] [drm:drm_setup_crtcs] looking for cmdline mode on connector 77 <7>[ 3069.935229] [drm:drm_setup_crtcs] looking for preferred mode on connector 77 0 <7>[ 3069.935233] [drm:drm_setup_crtcs] found mode 3200x1800 <7>[ 3069.935236] [drm:drm_setup_crtcs] picking CRTCs for 8192x8192 config <7>[ 3069.935253] [drm:drm_setup_crtcs] desired mode 3200x1800 set on crtc 43 (0,0) <7>[ 3069.935323] [drm:intelfb_create [i915]] no BIOS fb, allocating a new one <4>[ 3069.967737] general protection fault: 0000 [#1] PREEMPT SMP <0>[ 3069.977453] --------------------------------- <4>[ 3069.977457] Modules linked in: i915(+) vgem snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_hda_codec snd_hwdep snd_hda_core snd_pcm r8169 mei_me mii prime_numbers mei i2c_hid pinctrl_geminilake pinctrl_intel [last unloaded: i915] <4>[ 3069.977492] CPU: 1 PID: 15414 Comm: kworker/1:0 Tainted: G U 4.14.0-CI-CI_DRM_3388+ #1 <4>[ 3069.977497] Hardware name: Intel Corp. Geminilake/GLK RVP1 DDR4 (05), BIOS GELKRVPA.X64.0062.B30.1708222146 08/22/2017 <4>[ 3069.977508] Workqueue: events output_poll_execute <4>[ 3069.977512] task: ffff880177734e40 task.stack: ffffc90001fe4000 <4>[ 3069.977519] RIP: 0010:__lock_acquire+0x109/0x1b60 <4>[ 3069.977523] RSP: 0018:ffffc90001fe7bb0 EFLAGS: 00010002 <4>[ 3069.977526] RAX: 6b6b6b6b6b6b6b6b RBX: 0000000000000282 RCX: 0000000000000000 <4>[ 3069.977530] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff880170d4efd0 <4>[ 3069.977534] RBP: ffffc90001fe7c70 R08: 0000000000000001 R09: 0000000000000000 <4>[ 3069.977538] R10: 0000000000000000 R11: ffffffff81899609 R12: ffff880170d4efd0 <4>[ 3069.977542] R13: ffff880177734e40 R14: 0000000000000001 R15: 0000000000000000 <4>[ 3069.977547] FS: 0000000000000000(0000) GS:ffff88017fc80000(0000) knlGS:0000000000000000 <4>[ 3069.977551] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 <4>[ 3069.977555] CR2: 00007f7e8b7bcf04 CR3: 0000000003e0f000 CR4: 00000000003406e0 <4>[ 3069.977559] Call Trace: <4>[ 3069.977565] ? mark_held_locks+0x64/0x90 <4>[ 3069.977571] ? _raw_spin_unlock_irq+0x24/0x50 <4>[ 3069.977575] ? _raw_spin_unlock_irq+0x24/0x50 <4>[ 3069.977579] ? trace_hardirqs_on_caller+0xde/0x1c0 <4>[ 3069.977583] ? _raw_spin_unlock_irq+0x2f/0x50 <4>[ 3069.977588] ? finish_task_switch+0xa5/0x210 <4>[ 3069.977592] ? lock_acquire+0xaf/0x200 <4>[ 3069.977596] lock_acquire+0xaf/0x200 <4>[ 3069.977600] ? __mutex_lock+0x5e9/0x9b0 <4>[ 3069.977604] _raw_spin_lock+0x2a/0x40 <4>[ 3069.977608] ? __mutex_lock+0x5e9/0x9b0 <4>[ 3069.977612] __mutex_lock+0x5e9/0x9b0 <4>[ 3069.977616] ? drm_fb_helper_hotplug_event.part.19+0x16/0xa0 <4>[ 3069.977621] ? drm_fb_helper_hotplug_event.part.19+0x16/0xa0 <4>[ 3069.977625] drm_fb_helper_hotplug_event.part.19+0x16/0xa0 <4>[ 3069.977630] output_poll_execute+0x8d/0x180 <4>[ 3069.977635] process_one_work+0x22e/0x660 <4>[ 3069.977640] worker_thread+0x48/0x3a0 <4>[ 3069.977644] ? _raw_spin_unlock_irqrestore+0x4c/0x60 <4>[ 3069.977649] kthread+0x102/0x140 <4>[ 3069.977653] ? process_one_work+0x660/0x660 <4>[ 3069.977657] ? kthread_create_on_node+0x40/0x40 <4>[ 3069.977662] ret_from_fork+0x27/0x40 <4>[ 3069.977666] Code: 8d 62 f8 c3 49 81 3c 24 e0 fa 3c 82 41 be 00 00 00 00 45 0f 45 f0 83 fe 01 77 86 89 f0 49 8b 44 c4 08 48 85 c0 0f 84 76 ff ff ff <f0> ff 80 38 01 00 00 8b 1d 62 f9 e8 01 45 8b 85 b8 08 00 00 85 <1>[ 3069.977707] RIP: __lock_acquire+0x109/0x1b60 RSP: ffffc90001fe7bb0 <4>[ 3069.977712] ---[ end trace 4ad012eb3af62df7 ]--- In order to keep the dev_priv->ifbdev alive after failure, we have to avoid the free and leave it empty until we unload the module (which is less than ideal, but a necessary evil for simplicity). Then we can use intel_fbdev_sync() to serialise the hotplug event with the configuration. The serialisation between the two was removed in commit 934458c2 ("Revert "drm/i915: Fix races on fbdev""), but the use after free is much older, commit 366e39b4 ("drm/i915: Tear down fbdev if initialization fails") Fixes: 366e39b4 ("drm/i915: Tear down fbdev if initialization fails") Fixes: 934458c2 ("Revert "drm/i915: Fix races on fbdev"") Signed-off-by:
Lukas Wunner <lukas@wunner.de> Link: https://patchwork.freedesktop.org/patch/msgid/20171125194155.355-1-chris@chris-wilson.co.uk
-
