- 14 Feb, 2014 5 commits
-
-
Daniel Vetter authored
Anything more than just one bool parameter is just a pain to read, symbolic constants are much better. Split out from Chris' vma-binding rework patch. v2: Undo the behaviour change in object_pin that Chris spotted. v3: Split out misplaced hunk to handle set_cache_level errors, spotted by Jani. v4: Keep the current over-zealous binding logic in the execbuffer code working with a quick hack while the overall binding code gets shuffled around. v5: Reorder the PIN_ flags for more natural patch splitup. v6: Pull out the PIN_GLOBAL split-up again. Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ben Widawsky <benjamin.widawsky@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
This is the same what we do for DP connectors, so make things more consistent. Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Antti Koskipää <antti.koskipaa@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Antti Koskipää <antti.koskipaa@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
Atm we set the parent of the dp i2c device to be the correspondig connector device. During driver cleanup we first remove the connector device through intel_modeset_cleanup()->drm_sysfs_connector_remove() and only after that the i2c device through the encoder's destroy callback. This order is not supported by the device core and we'll get a warning, see the below bugzilla ticket. The proper order is to remove first any child device and only then the parent device. The first part of the fix changes the i2c device's parent to be the drm device. Its logical owner is not the connector anyway, but the encoder. Since the encoder doesn't have a device object, the next best choice is the drm device. This is the same what we do in the case of the sdvo i2c device and what the nouveau driver does. The second part creates a symlink in the connector's sysfs directory pointing to the i2c device. This is so, that we keep the current ABI, which also makes sense in case someone wants to look up the i2c device belonging to a specific connector. Reference: http://lists.freedesktop.org/archives/intel-gfx/2014-January/038782.html Reference: http://lists.freedesktop.org/archives/intel-gfx/2014-February/039427.html Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70523Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Antti Koskipää <antti.koskipaa@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
Since commit d9255d57 Author: Paulo Zanoni <paulo.r.zanoni@intel.com> Date: Thu Sep 26 20:05:59 2013 -0300 it became clear that we need to separate the unload sequence into two parts: 1. remove all interfaces through which new operations on some object (crtc, encoder, connector) can be started and make sure all pending operations are completed 2. do the actual tear down of the internal representation of the above objects The above commit achieved this separation for connectors by splitting out the sysfs removal part from the connector's destroy callback and doing this removal before calling drm_mode_config_cleanup() which does the actual tear-down of all the drm objects. Since we'll have to customize the interface removal part for different types of connectors in the upcoming patches, add a new unregister callback and move the interface removal part to it. No functional change. Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Antti Koskipää <antti.koskipaa@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
- 13 Feb, 2014 5 commits
-
-
Paulo Zanoni authored
Reproducible by runtime suspending a Haswell machine with eDP + HDMI outputs connected. [ 209.600086] [drm:i915_runtime_suspend], Suspending device [ 209.688435] BUG: unable to handle kernel NULL pointer dereference at 0000000000000060 [ 209.688500] IP: [<ffffffffa0109d4e>] i915_sink_crc+0x6e/0xf0 [i915] [ 209.688577] PGD 36aba067 PUD 35d7f067 PMD 0 [ 209.688613] Oops: 0000 [#1] SMP [ 209.688641] Modules linked in: fuse ip6table_filter ip6_tables ebtable_nat ebtables iTCO_wdt iTCO_vendor_support x86_pkg_temp_thermal coretemp microcode serio_raw e1000e pcspkr i2c_i801 ptp mei_me mei lpc_ich mfd_core pps_core dm_crypt i915 i2c_algo_bit crc32_pclmul drm_kms_helper crc32c_intel drm ghash_clmulni_intel video [ 209.688893] CPU: 1 PID: 1797 Comm: pm_pc8 Not tainted 3.13.0+ #118 [ 209.688937] Hardware name: Intel Corporation Shark Bay Client platform/WhiteTip Mountain 1, BIOS HSWLPTU1.86C.0133.R00.1309172123 09/17/2013 [ 209.689023] task: ffff88007fb4b690 ti: ffff88007d9d2000 task.ti: ffff88007d9d2000 [ 209.689074] RIP: 0010:[<ffffffffa0109d4e>] [<ffffffffa0109d4e>] i915_sink_crc+0x6e/0xf0 [i915] [ 209.689169] RSP: 0018:ffff88007d9d3e68 EFLAGS: 00010246 [ 209.689205] RAX: 0000000000000000 RBX: ffff880036a03478 RCX: ffff8800366c9770 [ 209.689252] RDX: ffff88014325cf38 RSI: ffff88007fb4bd08 RDI: ffff88007fb4b690 [ 209.689299] RBP: ffff88007d9d3e98 R08: 0000000000000000 R09: 0000000000000000 [ 209.689346] R10: 0000000000000001 R11: 0000000000000000 R12: ffff8800366c9148 [ 209.689393] R13: 00000000ffffffed R14: ffff88007d9d3f50 R15: ffff880036a03478 [ 209.689441] FS: 00007f5a74bc29c0(0000) GS:ffff88014f240000(0000) knlGS:0000000000000000 [ 209.689494] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 209.689533] CR2: 0000000000000060 CR3: 0000000079d7e000 CR4: 00000000001407e0 [ 209.689580] Stack: [ 209.689594] 0000000000001000 ffff880146083980 ffff880146083980 0000000000000000 [ 209.689649] ffff880146083980 0000000000000001 ffff88007d9d3f00 ffffffff811d0744 [ 209.689702] 0000000000000046 00007fff7949fe20 ffff880036a034b8 0000000000000080 [ 209.689756] Call Trace: [ 209.689778] [<ffffffff811d0744>] seq_read+0x164/0x3e0 [ 209.689816] [<ffffffff811ab165>] vfs_read+0x95/0x160 [ 209.689851] [<ffffffff811abc79>] SyS_read+0x49/0xa0 [ 209.689888] [<ffffffff810ef64c>] ? __audit_syscall_entry+0x9c/0xf0 [ 209.689933] [<ffffffff81659412>] system_call_fastpath+0x16/0x1b Testcase: igt/pm_pc8 (do a full run, it will fail at the debugfs-read subtest) Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> [danvet: Flip around NULL check for robustness.] Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Damien Lespiau authored
Coverity points out that, if we end up in the 'failed' label, that's precisely because we couldn't retrieve a fixed mode (ie fixed_mode is NULL) and then "if (fixed_mode)" is always false. Remove that dead code. Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Jesse Barnes authored
It can be corrected later and may be what was actually desired, but generally isn't, so if we find nothing is enabled, let the core DRM fb helper figure something out. v2: free the array too (Jesse) Note that this also undoes any changes in case we bail out due to hw cloning. Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Ben Widawsky authored
This will make the code more readable, and extensible which is needed for upcoming feature work. Eventually, we'll do the same for init. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Ville Syrjälä authored
We have a couple of switch cases to compute the port value for the VIDEO_DIP_CTL register. Replace them with a simple macro. We do lose a few BUG() calls, but many people may consider that an improvement. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
- 12 Feb, 2014 30 commits
-
-
Daniel Vetter authored
... past the check for DRIVER_MODESET. Avoids races with userspace opening a master and our sarea setup. Cc: Signed-off-by: Stéphane Marchesin <marcheu@chromium.org> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Daniel Vetter authored
Avoids surprises when userspace races open/closes against this. Cc: Stéphane Marchesin <marcheu@chromium.org> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Daniel Vetter authored
We assign the sarea_priv pointer only in the dma ioctl, which is disallowed when kernel modesetting is enabled. So this is dead code. Cc: Stéphane Marchesin <marcheu@chromium.org> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Jesse Barnes authored
The BIOS or boot loader will generally create an initial display configuration for us that includes some set of active pipes and displays. This routine tries to figure out which pipes and connectors are active and stuffs them into the crtcs and modes array given to us by the drm_fb_helper code. The overall sequence is: intel_fbdev_init - from driver load intel_fbdev_init_bios - initialize the intel_fbdev using BIOS data drm_fb_helper_init - build fb helper structs drm_fb_helper_single_add_all_connectors - more fb helper structs intel_fbdev_initial_config - apply the config drm_fb_helper_initial_config - call ->probe then register_framebuffer() drm_setup_crtcs - build crtc config for fbdev intel_fb_initial_config - find active connectors etc drm_fb_helper_single_fb_probe - set up fbdev intelfb_create - re-use or alloc fb, build out fbdev structs v2: use BIOS connector config unconditionally if possible (Daniel) check for crtc cloning and reject (Daniel) fix up comments (Daniel) v3: use command line args and preferred modes first (Ville) Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> [danvet: Re-add the WARN_ON for a missing encoder crtc - the state sanitizer should take care of this. And spell-ocd the comments.] Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Jesse Barnes authored
This allows drivers to use them in custom initial_config functions. Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> Acked-by: Dave Airlie <airlied@gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Daniel Vetter authored
Just a bit of polish which I hope will help me with massaging some internal patches to use Imre's reworked pipestat handling: - Don't check for underrun reporting or enable pipestat interrupts twice. - Frob the comments a bit. - Do the iir PIPE_EVENT to pipe mapping explicitly with a switch. We only have one place which does this, so better to make it explicit. v2: Ville noticed that I've broken the logic a bit with trying to avoid checking whether we're interested in a given pipe twice. push the PIPESTAT read down after we've computed the mask of interesting bits first to avoid that duplication properly. v3: Squash in fixups from Imre on irc. Cc: Imre Deak <imre.deak@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Imre Deak <imre.deak@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Daniel Vetter authored
We want to reuse this in the fbdev initial config code independently from any fastboot hacks. So allow a bit more flexibility. v2: Forgot to git add ... v3: make non-static (Jesse) Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Jesse Barnes authored
We want to do this early on before we try to fetch the plane config, which depends on some of the pipe config state. Note that the important part is that we do this before we initialize gem, since otherwise we can't properly pre-reserve the stolen memory for framebuffers inherited from the bios. v2: split back out from get_plane_config change (Daniel) update for recent locking & reset changes (Jesse) Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> [danvet: Explain a bit more why we need to move this.] Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Imre Deak authored
So far during driver unload we called drm_framebuffer_cleanup() for the fbdev fb, which only removes the fb from the drm fb list regardless of its reference count, but leaves the fb bound on an active crtc. Since the fb's backing storage was freed this could mean we scan some random memory content out afterwards. It's not a big issue since the fb is allocated from stolen memory and afaik there is no other user for that than i915. It's still cleaner to properly unbind the fb and disable the crtc, which is what drm_framebuffer_remove() does. Note that after commit 88891eb1e9eca0ba619518bed31580f91e9cf84d Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Mon Feb 10 18:00:38 2014 +0100 we call drm_framebuffer_cleanup() only after dropping the last reference on the fb, but that won't happen since we don't unbind the fb. This results in a drm core warn about a leaked fb. Signed-off-by: Imre Deak <imre.deak@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Ben Widawsky authored
Signed-off-by: Ben Widawsky <ben@bwidawsk.net> Reviewed-by: Deepak S <deepak.s@intel.com> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Ben Widawsky authored
Everything can be overridden by module parameters, so don't confuse the users that are using them. We have RC6 turned on for all platforms which support it, but Ironlake, so the need to explain the situation is no longer pressing. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> Reviewed-by: Deepak S <deepak.s@intel.com> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Ben Widawsky authored
It wasn't ever used by the caller anyway with the exception of what we show in sysfs. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> Reviewed-by: Deepak S <deepak.s@intel.com> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> [danvet: Apply Deepak's suggestion.] Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Ben Widawsky authored
At one time, we though all future platforms would have the deeper RC6 states. As it turned out, they killed it after Ivybridge, and began using other means to achieve the power savings (the stuff we need to get to PC7+). The enable function was left in a weird state of odd corner cases as a result. Since the future is now, and we also have some insight into what's currently the future, we have an opportunity to simplify, and future proof the function. NOTE: VLV will be addressed in a subsequent patch. This patch was trying not to change functionality. NOTE2: All callers sanitize the return value anyway, so this patch is simply to have the code make a bit more sense. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> Reviewed-by: Deepak S <deepak.s@intel.com> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Paulo Zanoni authored
... and QUIRK_PIPEA_FORCE is not present. I initially thought that case was impossible and just added a WARN on it, but then I was told this case is possible due to QUIRK_PIPEA_FORCE. So let's add a WARN that serves two purposes: - tell us in case we have done something wrong; - document the only case where we expect this. Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Paulo Zanoni authored
Add a nice comment explaining why we shouldn't wait for a vblank on all cases, wait based on the HW gen, and add a comment saying we should probably skip that wait on some of the previous HW gens. Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Paulo Zanoni authored
Now that we pass struct intel_crtc as an argument, we can check for DSI inside the function, removing one more of those confusing boolean arguments. Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Paulo Zanoni authored
Now that we pass struct intel_crtc as an argument, there's no need for it. Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Paulo Zanoni authored
We want to remove those 3 boolean arguments. This is the first step. The "pipe" passed as the argument is always intel_crtc->pipe. Also adjust the function documentation. Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Paulo Zanoni authored
When I forked haswell_crtc_enable I copied all the code from ironlake_crtc_enable. The last piece of the function contains a big comment with a call to intel_wait_for_vblank. After this fork, we rearranged the Haswell code so that it enables the planes as the very last step of the modeset sequence, so we're sure that we call intel_enable_primary_plane after the pipe is really running, so the vblank waiting functions work as expected. I really believe this is what fixes the problem described by the big comment, so let's give it a try and get rid of that intel_wait_for_vblank, saving around 16ms per modeset (and init/resume). We can always revert if needed :) Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Paulo Zanoni authored
Because on Haswell, the pipe is never running at this point, so we hit the 50ms timeout waiting for nothing. We already have two other places where we wait for vblanks on haswell_crtc_enable, so we're safe. This gets us rid of one instance of "vblank wait timed out" for each mode set, which means driver init and resume are also 50ms faster. Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Paulo Zanoni authored
Depending on the HW gen and the connector type, the pipe won't start running right after we call intel_enable_pipe, so that intel_wait_for_vblank call we currently have will just sit there for the full 50ms timeout. So this patch adds an argument that will allow us to avoid the vblank wait in case we want. Currently all the callers still request for the vblank wait, so the behavior should still be the same. We also added a POSTING_READ on the register: previously intel_wait_for_vblank was acting as a POSTING_READ, but now if wait_for_vblank is false we'll stkip it, so we need an explicit POSTING_READ. Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Vandana Kannan authored
Instead of modifying intel_panel in lvds_init_connector/dsi_init/ edp_init_connector, making changes to move intel_panel->downclock_mode initialization to intel_panel_init() v2: Jani's review comments incorporated Removed downclock_mode local variable in dsi_init and edp_init_connector Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com> Reviewed-by: Jani Nikula <jani.nikula@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Jesse Barnes authored
Can be expanded up on to include all sorts of things (HDMI infoframe data, more DP status, etc). Should be useful for bug reports to get a baseline on the display config and info. v2: use seq_putc (Rodrigo) describe mode field names (Rodrigo) Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Jesse Barnes authored
Just like we have for connector type etc. v2: drop static array (Chris) v3: add kdoc (Daniel) Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> Acked-by: Dave Airlie <airlied@gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Jesse Barnes authored
For use by get_plane_config. v2: cleanup tile_height bits (Chris) Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Daniel Vetter authored
In Jesse's patch to switch the fbdev framebuffer from an embedded struct to a pointer the kfree in case of an error was missed. Fix this up by using our own internal fb allocation helper directly instead of reinventing that wheel. We need a to_intel_framebuffer cast unfortunately since all the other callers of _create still look better whith using a drm_framebuffer as return pointer. v2: Add an unlocked __intel_framebuffer_create function since our dev->struct_mutex locking is too much a mess. With ppgtt we even need it to take a look at the global gtt offset of pinned objects, since the vma list might chance from underneath us. At least with the current global gtt lookup functions. Reported by Mika. Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Jesse Barnes <jbarnes@virtuousgeek.org> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Daniel Vetter authored
Now that it's a normally kmalloce buffer we can use the usual cleanup paths. The upside here is that if we get the refcounting wrong will be able to catch it, since the drm core will complain about leftover framebuffers and kref about underflows. v2: Kill intel_framebuffer_fini - no longer needed now that we refcount all fbs properly and only confusing. v3: We actually still need to call unregister_private to remove the fb from the idr and drop the idr reference - the final unref doesn't do that. So much for remembering my own fb liftime rules. Reported by Imre Deak. Cc: Jesse Barnes <jbarnes@virtuousgeek.org> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> (v2) Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Damien Lespiau authored
If we can't actually determine at run-time we have a fused-off display, provide at least an option to disable it. v2: Move the i915.disable_display test in a separate check (Daniel Vetter) Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Damien Lespiau authored
FUSE_STRAP has a bit to inform us that the display has been fused off. Use it to setup the definitive number of pipes at run-time. v2: actually tweak num_pipes, not num_planes v3: also tests SFUSE_STRAP bit 7 v4: rebase on top of drm-nightly use DRM_INFO() for the message telling display is fused off try to read the FUSE_LOCK bit to determine if PCH display is disabled v5: Don't read SFUSE_STRAP (register on the PCH) if num_pipes is already 0 from the initial device info struct (to prevent hangs) (Daniel Vetter) Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> (for v3) Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (for v3) Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Chris Wilson authored
Arjan van de Ven reported that on his test machine that he was seeing stalls of greater than 1 frame greatly impacting the user experience. He tracked this down to being the locked flush during a pagefault as being the culprit hogging the struct_mutex and so blocking any other user from proceeding. Stalling on a pagefault is bad behaviour on userspace's part, for one it means that they are ignoring the coherency rules on pointer access through the GTT, but fortunately we can apply the same trick as the set-to-domain ioctl to do a lightweight, nonblocking flush of outstanding rendering first. "Prior to the patch it looks like this (this one testrun does not show the 20ms+ I've seen occasionally) 4.99 ms 2.36 ms 31360 __wait_seqno i915_wait_seqno i915_gem_object_wait_rendering i915_gem_object_set_to_gtt_domain i915_gem_fault __do_fault handle_ +pte_fault handle_mm_fault __do_page_fault do_page_fault page_fault 4.99 ms 2.75 ms 107751 __wait_seqno i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret 4.99 ms 1.63 ms 1666 i915_mutex_lock_interruptible i915_gem_fault __do_fault handle_pte_fault handle_mm_fault __do_page_fault do_page_fault page_fa +ult 4.93 ms 2.45 ms 980 i915_mutex_lock_interruptible intel_crtc_page_flip drm_mode_page_flip_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_ +sysret 4.89 ms 2.20 ms 3283 i915_mutex_lock_interruptible i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret 4.34 ms 1.66 ms 1715 i915_mutex_lock_interruptible i915_gem_pwrite_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret 3.73 ms 3.73 ms 49 i915_mutex_lock_interruptible i915_gem_set_domain_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret 3.17 ms 0.33 ms 931 i915_mutex_lock_interruptible i915_gem_madvise_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret 2.97 ms 0.43 ms 1029 i915_mutex_lock_interruptible i915_gem_busy_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret 2.55 ms 0.51 ms 735 i915_gem_get_tiling drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret After the patch it looks like this: 4.99 ms 2.14 ms 22212 __wait_seqno i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret 4.86 ms 0.99 ms 14170 __wait_seqno i915_gem_object_wait_rendering__nonblocking i915_gem_fault __do_fault handle_pte_fault handle_mm_fault __do_page_ +fault do_page_fault page_fault 3.59 ms 1.31 ms 325 i915_gem_get_tiling drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret 3.37 ms 3.37 ms 65 i915_mutex_lock_interruptible i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret 2.58 ms 2.58 ms 65 i915_mutex_lock_interruptible i915_gem_do_execbuffer.isra.23 i915_gem_execbuffer2 drm_ioctl i915_compat_ioctl compat_sys_ioctl +ia32_sysret 2.19 ms 2.19 ms 65 i915_mutex_lock_interruptible intel_crtc_page_flip drm_mode_page_flip_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_ +sysret 2.18 ms 2.18 ms 65 i915_mutex_lock_interruptible i915_gem_busy_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret 1.66 ms 1.66 ms 65 i915_gem_set_tiling drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret It may not look like it, but this is quite a large difference, and I've been unable to reproduce > 5 msec delays at all, while before they do happen (just not in the trace above)." gem_gtt_hog on an old Pineview (GMA3150), before: 4969.119ms after: 4122.749ms Reported-by: Arjan van de Ven <arjan.van.de.ven@intel.com> Testcase: igt/gem_gtt_hog Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-