- 23 Jul, 2014 40 commits
-
-
Sonika Jindal authored
Signed-off-by:
Sonika Jindal <sonika.jindal@intel.com> Signed-off-by:
Daniel Vetter <daniel.vetter@ffwll.ch>
-
Rodrigo Vivi authored
semaphore _sync_seqno, _seqno and _mbox are smaller than number of rings. This optimization is to remove the ring itself from the list and the logic to do that is at intel_ring_sync_index as below: /* * rcs -> 0 = vcs, 1 = bcs, 2 = vecs, 3 = vcs2; * vcs -> 0 = bcs, 1 = vecs, 2 = vcs2, 3 = rcs; * bcs -> 0 = vecs, 1 = vcs2. 2 = rcs, 3 = vcs; * vecs -> 0 = vcs2, 1 = rcs, 2 = vcs, 3 = bcs; * vcs2 -> 0 = rcs, 1 = vcs, 2 = bcs, 3 = vecs; */ v2: Skip when from == to (Damien). v3: avoid computing idx when from == to (Damien). use ring == to instead of ring->id == to->id (Damien). use continue instead of return (Rodrigo). v4: avoid all unecessary computation (Damien). reduce idx to loop scope (Damien). Cc: Damien Lespiau <damien.lespiau@intel.com> Cc: Ben Widawsky <benjamin.widawsky@intel.com> Signed-off-by:Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by:
Damien Lespiau <damien.lespiau@intel.com> Reviewed-by:
Ben Widawsky <ben@bwidawsk.net> Signed-off-by:
-
Armin Reese authored
When using an IOMMU, GEM objects are mapped by their DMA address as the physical address is unknown. This depends on the underlying IOMMU driver to map and unmap the physical pages properly as defined in intel_iommu.c. The current code will tell the IOMMU to unmap the GEM BO's pages on the destruction of the first VMA that "maps" that BO. This is clearly wrong as there may be other VMAs "mapping" that BO (using flink). The scanout is one such example. The patch fixes this issue by only unmapping the DMA maps when there are no more VMAs mapping that object. This is equivalent to when an object is considered unbound as can be seen by the code. On the first VMA that again because bound, we will remap. An alternate solution would be to move the dma mapping to object creation and destrubtion. I am not sure if this is considered an unfriendly thing to do. Some notes to backporters trying to backport full PPGTT: The bug can never be hit without enabling the IOMMU. The existing code will also do the right thing when the object is shared via dmabuf. The failure should be demonstrable with flink. In cases when not using intel_iommu_strict it is likely (likely, as defined by: off the top of my head) on current workloads to *not* hit this bug since we often teardown all VMAs for an object shared across multiple VMs. We also finish access to that object before the first dma_unmapping. intel_iommu_strict with flinked buffers is likely to hit this issue. Signed-off-by:
Armin Reese <armin.c.reese@intel.com> [danvet: Add the excellent commit message provided by Ben.] Reviewed-by:
-
Paulo Zanoni authored
Since we merged runtime PM support for DPMS, it is possible that these assertions will be called when the power wells are disabled but a mode is "set", resulting in "failed assertion" and "device suspended while reading register" WARNs. To reproduce the bug: disable all screens using mode unset, do a modeset on one screen, disable it using DPMS, then try to do a mode unset on it again to see the WARNs. v2: The first version of this patch changed the assertions to also check the power domains. Daniel suggested that it would be better to just remove the assertions: "The modeset state checker will already notice when we've failed to turn off the pipe. And we check cursors and plane state in the enable sequence, too. Since we use these asserts a lot to lock down the precise modeset sequence I actually prefer if they're a bit dumb and don't check the power wells." Testcase: igt/rpm_rpm/dpms-mode-unset-lpsp Testcase: igt/rpm_rpm/dpms-mode-unset-non-lpsp Signed-off-by:
Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by:
-
Ben Widawsky authored
The comment [which was mine] is wrong. The context object can never be bound in a PPGTT because it is only capable of living in the Global GTT. So, remove the comment, and reorder the unref. What's nice about the latter is it keeps the context object alive past the PPGTT. This makes the destroy ordering symmetric with the creation ordering. Create: 1. Create context 2. Create PPGTT Destroy: 1. Destroy PPGTT 2. Destroy context As far as I know, this does not fix a bug. The code previously kept the context data structure, only the object was gone. As the code was, nothing tried to use the object after this point. NOTE: If in the future we have cases where the PPGTT can/should outlive the context (which doesn't occur today, but the code permits it), this ordering does not matter. Even if this occurs, as it stands now, we do not expect that to be the normal case, and having this order makes debugging a bit easier if we're tracking object lifetimes for the context vs ppgtt Signed-off-by:
-
Ben Widawsky authored
The bound list is global (all objects which back the VMAs are stored here). Recently the BUG() in the offset lookup was demoted to a WARN, but the fault actually lies in the caller, here. This bug has existed since the initial introduction of PPGTT (however, it was fixed in unmerged patches to fix up the error state). Note: The reason for the BUG_ON to WARN_ON demotion was _not_ to duct-tape over this bug here but another but triggerable without ppgtt. See the commit for details: commit f25748ea Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Tue Jun 17 22:34:38 2014 +0200 drm/i915: Don't BUG_ON in i915_gem_obj_offset A WARN_ON is perfectly fine. The BUG in here seems to be the cause behind hard-hangs when I cat the i915_gem_pageflip debugfs file (which calls this from an irq spinlock). But only while running a full igt run after a while. I still need to root cause the underlying issue. I'll also start reject patches which add new BUG_ON but don't come with a really good justification for it. The general rule really should be to just WARN and hope the driver survives for long enough. v2: Make the WARN a bit more useful per Chris' suggestion. Reviewed-by:
Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
Deepak S authored
This was fumbled while trying to use the cached min/min/rpe values in the vlv debugfs code. This is a regression from commit 03af2045 Author: Ville Syrjälä <ville.syrjala@linux.intel.com> Date: Sat Jun 28 02:03:53 2014 +0300 drm/i915: Use the cached min/min/rpe values in the vlv debugfs code Signed-off-by:Deepak S <deepak.s@linux.intel.com> Signed-off-by:
-
Paulo Zanoni authored
By the time I wrote this patch, it allowed me to catch some problems. But due to patch reordering - in order to prevent fake "regression" reports - this patch may be merged after the fixes of the problems identified by this patch. Reviewed-by:
-
Paulo Zanoni authored
The current code only runs when we do an I915_WRITE operation. It checks if the unclaimed register flag is set before we do the operation, and then it checks it again after we do the operation. This double check allows us to find out if the I915_WRITE operation in question is the bad one, or if some previous code is the bad one. When it finds a problem, our code uses DRM_ERROR to signal it. The good thing about the current code is that it detects the problem, so at least we can know we did something wrong. The problem is that even though we find the problem, we don't really have much information to actually debug it. So whenever I see one of these DRM_ERROR messages on my systems, the first thing I do is apply a patch to change the DRM_ERROR to a WARN and also check for unclaimed registers on I915_READ operations. This local patch makes things even slower, but it usually helps a lot in finding the bad code. The first point here is that since the current code is only useful to detect whether we have a problem or not, but it is not really good to find the cause of the problem, I don't think we should be checking both before and after every I915_WRITE operation: just doing the check once should be enough for us to quickly detect problems. With this change, the code that runs by default for every single user will only do 1 read operation for every single I915_WRITE, instead of 2. This patch does this change. The second point is that the local patch I have should be upstream, but since it makes things slower it should be disabled by default. So I added the i915.mmio_debug option to enable it. So after this patch, this is what will happen: - By default, we will try to detect unclaimed registers once after every I915_WRITE operation. Previously we tried twice for every I915_WRITE. - When we find an unclaimed register we will still print a DRM_ERROR message, but we will now tell the user to try again with i915.mmio_debug=1. - When we use i915.mmio_debug=1 we will try to find unclaimed registers both before and after every I915_READ and I915_WRITE operation, and we will print stack traces in case we find them. This should really help locating the exact point of the bad code (or at least finding out that i915.ko is not the problem). This commit also opens space for really-slow register debugging operations on other platforms. In theory we can now add lots and lots of debug code behind i915.mmio_debug, enable this option on our tests, and catch more problems. v2: - Remove not-so-useful comments (Daniel) - Fix the param definition macros (Rodrigo) Reviewed-by:Rodrigo Vivi <rodrigo.vivi@gmail.com> Signed-off-by:
-
Jesse Barnes authored
To avoid more spew with the new warnings. Signed-off-by:
Jesse Barnes <jbarnes@virtuousgeek.org> Signed-off-by:
-
Jesse Barnes authored
After this point, we'll modify it with the runtime routines. Signed-off-by:
-
Jesse Barnes authored
Before we've installed the handler, we can set this and avoid confusing init code that then thinks IRQs are enabled and spews complaints everywhere. Signed-off-by:
-
Jesse Barnes authored
Now that we use the runtime IRQ enable/disable functions in our suspend path, we can simply check the pm._irqs_disabled flag everywhere. So rename it to catch the users, and add an inline for it to make the checks clear everywhere. Signed-off-by:
-
Jesse Barnes authored
This was always the case on our suspend path, but it was recently exposed by the change to use our runtime IRQ disable routine rather than the full DRM IRQ disable. Keep the warning on the enable side, as that really would indicate a bug. Signed-off-by:
-
Paulo Zanoni authored
Move it from hsw_power_well_post_enable() (intel_pm.c) to i915_irq.c so we can reuse the nice IRQ macros we have there. The main difference is that now we're going to check if the IIR register is non-zero when we try to re-enable the interrupts. Signed-off-by:
-
Paulo Zanoni authored
So don't write it, otherwise we will trigger unclaimed register errors. Testcase: igt/pm_rpm/rte Signed-off-by:
-
Paulo Zanoni authored
If we enable unclaimed register reporting on Gen 8, we will discover that the IRQ registers for pipes B and C are also on the power well, so writes to them when the power well is disabled result in unclaimed register errors. Also, hsw_power_well_post_enable() already takes care of re-enabling them once the power well is enabled. Testcase: igt/pm_rpm/rte Signed-off-by:
-
Daniel Vetter authored
Traditionally we use genX_ for GT/render stuff and the codenames for display stuff. But the gt and pm interrupt handling functions on gen5/6+ stuck out as exceptions, so convert them. Looking at the diff this nicely realigns our ducks since almost all the callers are already platform-specific functions following the genX_ pattern. Spotted while reviewing some internal rps patches. No function change in this patch. Acked-by:
-
Damien Lespiau authored
Signed-off-by:
-
Chris Wilson authored
During the range invalidate, we walk the list of buffers associated with the mmu_notifer and find the ones that overlap the range. An optimisation is made to speed up the iteration by assuming the previous iter is still valid whilst the tree is unmodified. This exposes a bug when a range invalidate is triggered after we have just created the mmu_notifier, but before attaching any buffers. In that case, we presume we have an unmodified list and start walking from the last iter which is NULL. Oops. The easiest fix is then to initialise the serial of the tree to 1. Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Testecase: igt/gem_userptr_blts/stress-mm Signed-off-by:
-
Chris Wilson authored
Whilst waiting to obtain our locks for the last resort shrinking before an oom, we check whether or not a fatal signal was pending. If there was, we do not need to keep waiting as the oom will be aborted. Signed-off-by:
-
Damien Lespiau authored
In the future, we'll need the height of the fb to fetch from memory for WM computation. Signed-off-by:
-
Damien Lespiau authored
No need to list all the platforms explicitly. The prefix is a bit inconsistent since we usually pick gen8_ for GT related functions. But this anti-pattern is already established with snb, so material for a different patch. Cc: Damien Lespiau <damien.lespiau@intel.com> Reviewed-by:
-
Vandana Kannan authored
Updated drm documentation to include desscription of aspect ratio property. v2: Updated aspect ratio specific documentation on top of the HTML table created. Signed-off-by:
Vandana Kannan <vandana.kannan@intel.com> Cc: Sagar Kamble <sagar.a.kamble@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Reviewed-by:
Sagar Kamble <sagar.a.kamble@intel.com> Acked-by:
Dave Airlie <airlied@gmail.com> Signed-off-by:
-
Vandana Kannan authored
Create and attach the drm property to set aspect ratio. If there is no user specified value, then PAR_NONE/Automatic option is set by default. User can select aspect ratio 4:3 or 16:9. The aspect ratio selected by user would come into effect with a mode set. v2: Modified switch case to include aspect ratio enum changes v3: Modified the patch according the change in the earlier patch to return errno in case property creation fails. With this change, property will be attached only if creation is successful Signed-off-by:
-
Vandana Kannan authored
In case user has specified an input for aspect ratio through the property, then the user space value for PAR would take preference over the value from CEA mode list. v2: Thierry's review comments. - Modified the comment "Populate..." as per review comments v3: Thierry's review comments. - Modified the comment to block comment format. Signed-off-by:
Thierry Reding <treding@nvidia.com> Acked-by:
-
Vandana Kannan authored
Added a property to enable user space to set aspect ratio. This patch contains declaration of the property and code to create the property. v2: Thierry's review comments. - Made aspect ratio enum generic instead of HDMI/CEA specfic - Removed usage of temporary aspect_ratio variable v3: Thierry's review comments. - Fixed indentation v4: Thierry's review comments. - Return ENOMEM when property creation fails Signed-off-by:
-
Deepak S authored
Drop WaGsvBringDownFreq on CHV. When in RC6 requesting the min freq should be fine to bring the voltage down. Signed-off-by:
-
Paulo Zanoni authored
We already call intel_display_power_get, which will get a power domain, and every power domain should get a runtime PM reference, which will wake up the machine. v2: - Also touch intel_crt_detect() (Ville). Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> [danvet: Fixup commit message as spotted by Ville.] Signed-off-by:
-
Deepak S authored
We might be leaving the GPU Frequency (and thus vnn) high during the suspend. Force gt to move to lowest freq while suspending. v2: Fixed typo in commit message (Deepak) v3: Force gt to lowest freq in suspend_gt_powersave (Daniel) v4: Add GPU min freq set _after_ we've cancelled the rps works (Daniel) Signed-off-by:
-
Chris Wilson authored
Signed-off-by:
Jani Nikula <jani.nikula@intel.com> Signed-off-by:
-
Rodrigo Vivi authored
Panel Self Refresh is an eDP power saving feature specified by VESA's eDP v1.3, that allows some panel componets to shutdown while you still see static images on the screen. Besides being supported on the platform it must be supported by the eDP panel itself. Now that we have the propper frontbuffer tracking support and correct locks on place we can enabled this feature by default. Signed-off-by:
-
Daniel Vetter authored
We only need to check for this in psr_enable, everything else is already protect by the dev_priv->psr.enabled checks. Those need the psr locking, but these functions are called infrequent enough that the locking overhead is negligible. Suggested by Chris Wilson. Reviewed-by:
-
Daniel Vetter authored
Add busy_frontbuffer_bits and locking. Reviewed-by:
-
Daniel Vetter authored
I've tried to split this up, but all the changes are so tightly related that I didn't find a good way to do this without breaking bisecting. Essentially this completely changes how psr is glued into the overall driver, and there's not much you can do to soften such a paradigm change. - Use frontbuffer tracking bits stuff to separate disable and re-enable. - Don't re-check everything in the psr work. We have now accurate tracking for everything, so no need to check for sprites or tiling really. Allows us to ditch tons of locks. - That in turn allows us to properly cancel the work in the disable function - no more deadlocks. - Add a check for HSW sprites and force a flush. Apparently the hardware doesn't forward the flushing when updating the sprite base address. We can do the same trick everywhere else we have such issues, e.g. on baytrail with ... everything. - Don't re-enable psr with a delay in psr_exit. It really must be turned off forever if we detect a gtt write. At least with the current frontbuffer render tracking. Userspace can do a busy ioctl call or no-op pageflip to re-enable psr. - Drop redundant checks for crtc and crtc->active - now that they're only called from enable this is guaranteed. - Fix up the hsw port check. eDP can also happen on port D, but the issue is exactly that it doesn't work there. So an || check is wrong. - We still schedule the psr work with a delay. The frontbuffer flushing interface mandates that we upload the next full frame, so need to wait a bit. Once we have single-shot frame uploads we can do better here. v2: Don't enable psr initially, rely upon the fb flush of the initial plane setup for that. Gives us more unified code flow and makes the crtc enable sequence less a special case. v3: s/psr_exit/psr_invalidate/ for consistency v4: Fixup whitespace. v5: Correctly bail out of psr_invalidate/flush when dev_priv->psr.enabled is NULL. Spotted by Rodrigo. v6: - Only schedule work when there's work to do. Fixes WARNINGs reported by Rodrigo. - Comments Chris requested to clarify the code. v7: Fix conflict on rebase (Rodrigo) Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
-
Daniel Vetter authored
It's not really optional to have locking ... The ugly part is how much locking the psr work needs since it has to recheck everything. Which is way too much. But we need to ditch the psr work in it's current form anyway and implement proper frontbuffer tracking. The other nasty bit that had to go was the delayed work cancle in psr_exit. Which means a bunch of races just became a bit more likely, but mea culpa. v2: Fixup HAS_PSR checks, resulting in uninitialized mutex issues. Reviewed-by:
-
Daniel Vetter authored
We need to make sure that no one else is using this in the enable function and also that the work item hasn't raced with the disabled function. v2: Improve bisectability by moving one hunk to an earlier patch. v3: added missing dev_priv declaration (Rodrigo) Reviewed-by:
-
Daniel Vetter authored
Make sure we track the sw side (psr.active) correctly and WARN everywhere it might get out of sync with the hw. v2: Fixup WARN_ON logic inversion, reported by Rodrigo. Reviewed-by:
-
Daniel Vetter authored
It's disabled already except when we've raced. Reviewed-by:
-
Daniel Vetter authored
Trying to fish that one out through looping is a bit a locking nightmare. So just set it and use it in the work struct. v2: - Don't Oops in psr_work, spotted by Rodrigo. - Fix compile warning. Reviewed-by:
-
