- 10 Jul, 2013 4 commits
-
-
Chris Wilson authored
This reverts commit 25ff1195 and the follow on for Valleyview commit 2dc8aae0. commit 25ff1195 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Thu Apr 4 21:31:03 2013 +0100 drm/i915: Workaround incoherence between fences and LLC across multiple CPUs commit 2dc8aae0 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Wed May 22 17:08:06 2013 +0100 drm/i915: Workaround incoherence with fence updates on Valleyview Jon Bloomfield came up with a plausible explanation and cheap fix (drm/i915: Fix incoherence with fence updates on Sandybridge+) for the race condition, so lets run with it. This is a candidate for stable as the old workaround incurs a significant cost (calling wbinvd on all CPUs before performing the register write) for some workloads as noted by Carsten Emde. Link: http://lists.freedesktop.org/archives/intel-gfx/2013-June/028819.html References: https://www.osadl.org/?id=1543#c7602 References: https://bugs.freedesktop.org/show_bug.cgi?id=63825Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Jon Bloomfield <jon.bloomfield@intel.com> Cc: Carsten Emde <C.Emde@osadl.org> Cc: stable@vger.kernel.org Signed-off-by:
Daniel Vetter <daniel.vetter@ffwll.ch>
-
Chris Wilson authored
This hopefully fixes the root cause behind the workaround added in commit 25ff1195 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Thu Apr 4 21:31:03 2013 +0100 drm/i915: Workaround incoherence between fences and LLC across multiple CPUs Thanks to further investigation by Jon Bloomfield, he realised that the 64-bit register might be broken up by the hardware into two 32-bit writes (a problem we have encountered elsewhere). This non-atomicity would then cause an issue where a second thread would see an intermediate register state (new high dword, old low dword), and this register would randomly be used in preference to its own thread register. This would cause the second thread to read from and write into a fairly random tiled location. Breaking the operation into 3 explicit 32-bit updates (first disable the fence, poke the upper bits, then poke the lower bits and enable) ensures that, given proper serialisation between the 32-bit register write and the memory transfer, that the fence value is always consistent. Armed with this knowledge, we can explain how the previous workaround work. The key to the corruption is that a second thread sees an erroneous fence register that conflicts and overrides its own. By serialising the fence update across all CPUs, we have a small window where no GTT access is occurring and so hide the potential corruption. This also leads to the conclusion that the earlier workaround was incomplete. v2: Be overly paranoid about the order in which fence updates become visible to the GPU to make really sure that we turn the fence off before doing the update, and then only switch the fence on afterwards. Signed-off-by:
Jon Bloomfield <jon.bloomfield@intel.com> Signed-off-by:
-
Chris Wilson authored
Daniel noticed a problem where is we wrote to an object with ring A in the middle of a very long running batch, then executed a quick batch on ring B before a batch that reads from the same object, its obj->ring would now point to ring B, but its last_write_seqno would be still relative to ring A. This would allow for the user to read from the object before the GPU had completed the write, as set_domain would only check that ring B had passed the last_write_seqno. To fix this simply (and inelegantly), we bump the last_write_seqno when switching rings so that the last_write_seqno is always relative to the current obj->ring. This fixes igt/tests/gem_write_read_ring_switch. Signed-off-by:
-
Guenter Roeck authored
This patch partially reverts commit 36ec8f87 for IvyBridge CPUs. The original commit results in repeated 'Timed out waiting for forcewake old ack to clear' messages on a Supermicro C7H61 board (BIOS version 2.00 and 2.00b) with i7-3770K CPU. It ultimately results in a hangup if the system is highly loaded. Reverting the commit for IvyBridge CPUs fixes the issue. Issue a warning if the CPU is IvyBridge and mt forcewake is disabled, since this condition can result in secondary issues. v2: Only revert patch for Ivybridge CPUs Issue info message if mt forcewake is disabled on Ivybridge Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=60541 Cc: Jesse Barnes <jbarnes@virtuousgeek.org> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Signed-off-by:
Guenter Roeck <linux@roeck-us.net> Cc: stable@vger.kernel.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66139Signed-off-by:
-
- 09 Jul, 2013 4 commits
-
-
Imre Deak authored
DP 1.2 compatible displays may report a 5.4Gbps maximum bandwidth which the driver will treat as an invalid value and use 1.62Gbps instead. Fix this by capping to 2.7Gbps for sinks reporting a 5.4Gbps max bw. Also add a warning for reserved values. v2: - allow only bw values explicitly listed in the DP standard (Daniel, Chris) Signed-off-by:
Imre Deak <imre.deak@intel.com> Signed-off-by:
-
Daniel Vetter authored
It's not a good idea to also run the pipe_control cleanup. This regression has been introduced whith the original cs tlb w/a in commit b45305fc Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Mon Dec 17 16:21:27 2012 +0100 drm/i915: Implement workaround for broken CS tlb on i830/845 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=64610 Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: stable@vger.kernel.org Reviewed-by:
-
Xiong Zhang authored
obj->mm_list link to dev_priv->mm.inactive_list/active_list obj->global_list link to dev_priv->mm.unbound_list/bound_list This regression has been introduced in commit 93927ca5 Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Thu Jan 10 18:03:00 2013 +0100 drm/i915: Revert shrinker changes from "Track unbound pages" Cc: stable@vger.kernel.org Signed-off-by:
Xiong Zhang <xiong.y.zhang@intel.com> [danvet: Add regression notice.] Signed-off-by:
-
Paulo Zanoni authored
Now that the audio driver is using our power well API, everything should be working correctly, so let's give it a try. Signed-off-by:
Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by:
-
- 04 Jul, 2013 1 commit
-
-
Daniel Vetter authored
This fixes gpu reset on my gm45 - without this patch the bsd thing is forever stuck since the seqno updates never reach the status page. Tbh I have no idea how this ever worked without rewriting the hws registers after a gpu reset. To satisfy my OCD also give the functions a bit more consistent names: - Use status_page everywhere, also for the physical addressed one. - Use init for the allocation part and setup for the register setup part consistently. Long term I'd really like to share the hw init parts completely between gpu reset, resume and driver load, i.e. to call i915_gem_init_hw instead of the individual pieces we might need. v2: Add the missing paragraph to the commit message about what bug exactly this patch here fixes. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65495 Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
lu hua <huax.lu@intel.com> Signed-off-by:
-
- 02 Jul, 2013 1 commit
-
-
Daniel Vetter authored
Every other place properly checks whether we've managed to set up the stolen allocator at boot-up properly, with the exception of the cleanup code. Which results in an ugly *ERROR* Memory manager not clean. Delaying takedown at module unload time since the drm_mm isn't initialized at all. v2: While at it check whether the stolen drm_mm is initialized instead of the more obscure stolen_base == 0 check. v3: Fix up the logic. Also we need to keep the stolen_base check in i915_gem_object_create_stolen_for_preallocated since that can be called before stolen memory is fully set up. Spotted by Chris Wilson. v4: Readd the conversion in i915_gem_object_create_stolen_for_preallocated, the check is for the dev_priv->mm.gtt_space drm_mm, the stolen allocatot must already be initialized when calling that function (if we indeed have stolen memory). Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65953 Cc: Chris Wilson <chris@chris-wilson.co.uk> Tested-by: lu hua <huax.lu@intel.com> (v3) Reviewed-by:
-
- 01 Jul, 2013 30 commits
-
-
Chris Wilson authored
So it appears that I have encountered some bogosity when trying to call i915_error_printf() with many arguments from print_error_buffers(). The symptom is that the vsnprintf parser tries to interpret an integer arg as a character string, the resulting OOPS indicating stack corruption. Replacing the single call with its 13 format specifiers and arguments with multiple calls to i915_error_printf() worked fine. This patch goes one step further and introduced i915_error_puts() to pass the strings simply. It may not fix the root cause, but it does prevent my box from dying and I think helps make print_error_buffers() more friendly. Signed-off-by:
-
Chris Wilson authored
Harmonise the completion logic between the non-blocking and normal wait_rendering paths, and move that logic into a common function. In the process, we note that the last_write_seqno is by definition the earlier of the two read/write seqnos and so all successful waits will have passed the last_write_seqno. Therefore we can unconditionally clear the write seqno and its domains in the completion logic. v2: Add the missing ring parameter, because sometimes it is good to have things compile. Signed-off-by:
-
Chris Wilson authored
In the introduction of the non-blocking wait, I cut'n'pasted the wait completion code from normal locked path. Unfortunately, this neglected that the normal path returned early if the wait returned early. The result is that read-only waits may return whilst the GPU is still writing to the bo. Fixes regression from commit 3236f57a [v3.7] Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Fri Aug 24 09:35:09 2012 +0100 drm/i915: Use a non-blocking wait for set-to-domain ioctl Signed-off-by:
-
Xiong Zhang authored
On DevCPT, the control register for Transcoder DP Sync Polarity is TRANS_DP_CTL, not DP_CTL. Without this patch, Many call trace occur on CPT machine with DP monitor. The call trace is like: *ERROR* mismatch in adjusted_mode.flags(expected X,found X) v2: use intel-crtc to simple patch, suggested by Daniel. Signed-off-by:
-
Daniel Vetter authored
Our interrupt handler (in hardirq context) could race with the timer (in softirq context), hence we need to hold the spinlock around the call to ->hdp_irq_setup in intel_hpd_irq_handler, too. But as an optimization (and more so to clarify things) we don't need to do the irqsave/restore dance in the hardirq context. Note also that on ilk+ the race isn't just against the hotplug reenable timer, but also against the fifo underrun reporting. That one also modifies the SDEIMR register (again protected by the same dev_priv->irq_lock). To lock things down again sprinkle a assert_spin_locked. But exclude the functions touching SDEIMR for now, I want to extract them all into a new helper function (like we do already for pipestate, display interrupts and all the various gt interrupts). v2: Add the missing 't' Egbert spotted in a comment. v3: Actually fix the right misspelled comment (Paulo). Cc: Egbert Eich <eich@suse.de> Reviewed-by:
-
Daniel Vetter authored
The usual pattern for our sub-function irq_handlers is that they check for the no-irq case themselves. This results in more streamlined code in the upper irq handlers. v2: Rebase on top of the i965g/gm sdvo hpd fix. Cc: Egbert Eich <eich@suse.de> Reviewed-by:
Egbert Eich <eich@suse.de> Reviewed-by:
-
Daniel Vetter authored
Everywhere the same. Note that this patch leaves unnecessary braces behind, but the next patch will kill those all anyway (including the if itself) so I've figured I can keep the diff a bit smaller. v2: Rebase on top of the i965g/gm sdvo hpd fix. Cc: Egbert Eich <eich@suse.de> Reviewed-by:
-
Daniel Vetter authored
We already have a vfunc for this (and other parts of the hpd storm handling code already use it). v2: Rebase on top of the i965g/gm sdvo hpd fix. Cc: Egbert Eich <eich@suse.de> Reviewed-by:
-
Daniel Vetter authored
The combination of Paulo's fifo underrun detection code and Egbert's hpd storm handling code unfortunately made the hpd storm handling code racy. To avoid duplicating tricky interrupt locking code over all platforms start with a bit of refactoring. This patch is the very first step since in the end the irq storm handling code will handle all hotplug logic (and so also encapsulate the locking nicely). v2: Rebase on top of the i965g/gm sdvo hpd fix. Cc: Egbert Eich <eich@suse.de> Reviewed-by:
-
Daniel Vetter authored
By the time we write DEIER in the postinstall hook the interrupt handler could run any time. And it does modify DEIER to handle interrupts. Hence the DEIER read-modify-write cycle for enabling the PCU event source is racy. Close this races the same way we handle vblank interrupts: Unconditionally enable the interrupt in the IER register, but conditionally mask it in IMR. The later poses no such race since the interrupt handler does not touch DEIMR. Also update the comment, the clearing has already happened unconditionally above. v2: Actually shove the updated comment into the right train^W commit, as spotted by Paulo. Cc: Paulo Zanoni <przanoni@gmail.com> Reviewed-by:
-
Daniel Vetter authored
The haswell unclaimed register handling code forgot to take the spinlock. Since this is in the context of the non-rentrant interupt handler and we only have one interrupt handler it is sufficient to just grab the spinlock - we do not need to exclude any other interrupts from running on the same cpu. To prevent such gaffles in the future sprinkle assert_spin_locked over these functions. Unfornately this requires us to hold the spinlock in the ironlake postinstall hook where it is not strictly required: Currently that is run in single-threaded context and with userspace exlcuded from running concurrent ioctls. Add a comment explaining this. v2: ivb_can_enable_err_int also needs to be protected by the spinlock. To ensure this won't happen in the future again also sprinkle a spinlock assert in there. v3: Kill the 2nd call to ivb_can_enable_err_int I've accidentally left behind, spotted by Paulo. Cc: Paulo Zanoni <przanoni@gmail.com> Reviewed-by:
Paulo Zanoni <przanoni@gmail.com> Signed-off-by:
-
Ben Widawsky authored
With updates to the spec, we can actually see the context layout, and how many dwords are allocated. That table suggests we need 70720 bytes per HW context. Rounded up, this is 18 pages. Looking at what lives after the current 4 pages we use, I can't see too much important (mostly it's d3d related), but there are a couple of things which look scary. I am hopeful this can explain some of our odd HSW failures. v2: Make the context only 17 pages. The power context space isn't used ever, and execlists aren't used in our driver, making the actual total 66944 bytes. v3: Add a comment to the code. (Jesse & Paulo) Reported-by:
"Azad, Vinit" <vinit.azad@intel.com> Cc: stable@vger.kernel.org Reviewed-by:
Jesse Barnes <jbarnes@virtuousgeek.org> Signed-off-by:
Ben Widawsky <ben@bwidawsk.net> Signed-off-by:
-
Ville Syrjälä authored
We forgot to add VLV_DISPLAY_BASE to the VLV sprite registers, which caused the sprites to not work at all. Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by:
-
Ville Syrjälä authored
The PIPECONF color range bit doesn't appear to be effective, on HDMI outputs at least. The color range bit in the port register works though, so let's use it. I have not yet verified whether the PIPECONF bit works on DP outputs. This reverts commit 83a2af88. Signed-off-by:
-
Ville Syrjälä authored
LPF is short for "low pass filter". Signed-off-by:
-
Ville Syrjälä authored
The current PLL settings produce a rather unstable picture when I hook up a VLV to my HP ZR24w display via a VGA cable. According to VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_9, we should use the the same LPF coefficients for DAC as we do for HDMI and RBR DP. And indeed that seems to cure the shivers. v2: Add the name of the relevant document to the commit message Signed-off-by:
-
Ville Syrjälä authored
If the current GPU frquency is below RPe, and we're asked to increase it, just go directly to RPe. This should provide better performance faster than letting the frequency trickle up in response to the up threshold interrupts. For now just do it for VLV, since that matches quite closely how VLV used to operate when the rps delayed timer kept things at RPe always. Signed-off-by:
-
Ville Syrjälä authored
There's little point in increasing the GPU frequency from the delayed rps work on VLV. Now when the GPU is idle, the GPU frequency actually keeps dropping gradually until it hits the minimum, whereas previously it just ping-ponged constantly between RPe and RPe-1. Signed-off-by:
-
Ville Syrjälä authored
I can't find GEN6_RP_INTERRUPT_LIMITS (0xA014) anywhere in VLV docs. Reading it always returns zero from what I can tell, and eliminating it doesn't seem to make any difference to the behaviour of the system. Signed-off-by:
-
Ville Syrjälä authored
Eliminate the weird inverted logic from the rps new_delay comparison. Signed-off-by:
-
Ville Syrjälä authored
It seems that even though Punit reports the frequency change to have been completed, it still reports the old frequency in the status register for some time. So rather than polling for Punit to complete the frequency change after each request, poll before. This gets rid of the spurious "Punit overrode GPU freq" messages. This also lets us continue working while Punit is performing the actual frequency change. As a result, openarena demo088-test1 timedemo average fps is increased by ~5 fps, and the slowest frame duration is reduced by ~25%. The sysfs cur_freq file always reads the current frequency from Punit anyway, so having rps.cur_delay be slightly off at times doesn't matter. Signed-off-by:
-
Ville Syrjälä authored
Always print both the MHz value and raw register value for rps stuff. Also kill a somewhat pointless local 'rpe' variable and just use dev_priv->rps.rpe_delay. While at it clean up the caps in "GPU" and "Punit" debug messages. Signed-off-by:
-
Chris Wilson authored
Report back the user error of attempting to setup a CRTC with an invalid framebuffer pitch. This is trickier than it should be as on gen4, there is a restriction that tiled surfaces must have a stride less than 16k - which is less than the largest supported CRTC size. v2: Fix the limits for gen3 v3: Move check into intel_framebuffer_init() and fix VLV limits. (vsyrjala) v4: Use idiomatic '>=' for generation checks References: https://bugs.freedesktop.org/show_bug.cgi?id=65099Signed-off-by:
-
Ville Syrjälä authored
No need to apply WaForceL3Serialization:vlv twice. Signed-off-by:
Damien Lespiau <damien.lespiau@intel.com> Signed-off-by:
-
Daniel Vetter authored
There are legit cases, e.g. when userspace asks for something impossible. So tune it down to debug output like we do with all other userspace-triggerable warnings. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66111#c5Reviewed-by:
-
Daniel Vetter authored
Bspec seems to be full of lies, at least it disagress with reality: Two systems corrobated that SDVO hpd bits are the same as on gen3. v2: Update comment a bit. Cc: Arthur Ranyan <arthur.j.runyan@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Tested-by:
Alex Fiestas <afiestas@kde.org> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=58405 Cc: stable@vger.kernel.org Acked-by:
-
Damien Lespiau authored
Follow the trend and don't code conditions with platforms but with features. Signed-off-by:
-
Jani Nikula authored
drivers/gpu/drm/i915/i915_gem.c: In function ‘i915_gem_object_bind_to_gtt’: drivers/gpu/drm/i915/i915_gem.c:3002:3: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 5 has type ‘size_t’ [-Wformat] v2: Use %zu instead of %d. Two char patch, and 100% wrong. (Ville) Signed-off-by:
Jani Nikula <jani.nikula@intel.com> Reviewed-by:
-
Daniel Vetter authored
Nothing the user (nor we) really can do about this, but upsets a nice quiet boot. Note that this happens mostly on SDVs where OEMs obviously haven't had a chance yet to appropriately trim the output list. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65988Reviewed-by:
-
Konrad Rzeszutek Wilk authored
Git commit 90797e6d ("drm/i915: create compact dma scatter lists for gem objects") makes certain assumptions about the under laying DMA API that are not always correct. On a ThinkPad X230 with an Intel HD 4000 with Xen during the bootup I see: [drm:intel_pipe_set_base] *ERROR* pin & fence failed [drm:intel_crtc_set_config] *ERROR* failed to set mode on [CRTC:3], err = -28 Bit of debugging traced it down to dma_map_sg failing (in i915_gem_gtt_prepare_object) as some of the SG entries were huge (3MB). That unfortunately are sizes that the SWIOTLB is incapable of handling - the maximum it can handle is a an entry of 512KB of virtual contiguous memory for its bounce buffer. (See IO_TLB_SEGSIZE). Previous to the above mention git commit the SG entries were of 4KB, and the code introduced by above git commit squashed the CPU contiguous PFNs in one big virtual address provided to DMA API. This patch is a simple semi-revert - were we emulate the old behavior if we detect that SWIOTLB is online. If it is not online then we continue on with the new compact scatter gather mechanism. An alternative solution would be for the the '.get_pages' and the i915_gem_gtt_prepare_object to retry with smaller max gap of the amount of PFNs that can be combined together - but with this issue discovered during rc7 that might be too risky. Reported-and-Tested-by:
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> CC: Chris Wilson <chris@chris-wilson.co.uk> CC: Imre Deak <imre.deak@intel.com> CC: Daniel Vetter <daniel.vetter@ffwll.ch> CC: David Airlie <airlied@linux.ie> CC: <dri-devel@lists.freedesktop.org> Signed-off-by:
-
