- 21 Jul, 2016 3 commits
-
-
Dave Gordon authored
'ring' is an old deprecated term for a GPU engine, so we're trying to phase out all such terminology. eb_select_ring() not only has 'ring' (meaning engine) in its name, but it has an ugly calling convention whereby it returns an errno and stores a pointer-to-engine indirectly through an output parameter. As there is only one error it ever returns (-EINVAL), we can make it return the pointer directly, and have the caller pass back the error code -EINVAL if the pointer result is NULL. Thus we can replace - ret = eb_select_ring(dev_priv, file, args, &engine); - if (ret) - return ret; with + engine = eb_select_engine(dev_priv, file, args); + if (!engine) + return -EINVAL; for increased clarity and maybe save a few cycles too. Signed-off-by:
Dave Gordon <david.s.gordon@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1469034967-15840-4-git-send-email-david.s.gordon@intel.comReviewed-by:
Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
Dave Gordon authored
'ring' is an old deprecated term for a GPU engine. Chris Wilson wants to use the name for what is currently known as an intel_ringbuffer, but it will be dreadfully confusing if some rings are ringbuffers but other rings are still engines. So this patch changes the names of a bunch of parameters called 'ring' to either 'engine' or 'engine_id' according to what they actually are. Signed-off-by:
-
Dave Gordon authored
'ring' is an old deprecated term for a GPU engine. Here we make the terminology more consistent by renaming the 'ring' parameter of lots of macros that calculate addresses within the MMIO space of an engine. Signed-off-by:
-
- 20 Jul, 2016 19 commits
-
-
Mika Kuoppala authored
Add WaDisableGatherAtSetShaderCommonSlice for all gen9 as stated by bspec. The bspec told to put this workaround to the per ctx bb. Initial implementation and subsequent review were done based on bspec. Arun raised a suspicion that this would belong to indirect bb instead and he conducted more throughout investigation on the matter and indeed the documentation was wrong. v2: Move to indirect_ctx wa bb, as it is correct place (Arun) References: HSD#2135817 Cc: Arun Siluvery <arun.siluvery@linux.intel.com> Cc: Matthew Auld <matthew.auld@intel.com> Signed-off-by:
Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> (v1) Reviewed-by:
Arun Siluvery <arun.siluvery@linux.intel.com> Signed-off-by:
Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1469013973-24104-1-git-send-email-mika.kuoppala@intel.com
-
Mika Kuoppala authored
Add this workaround to prevent hang when in place compression is used. References: HSD#2135774 Cc: stable@vger.kernel.org Signed-off-by:
-
Chris Wilson authored
Rather than recomputing whether semaphores are enabled, we can do that computation once during early initialisation as the i915.semaphores module parameter is now read-only. s/i915_semaphores_is_enabled/i915.semaphores/ v2: Add the state to the debug dmesg as well Signed-off-by:
-
Chris Wilson authored
Just a different colour to better match virtual addresses elsewhere. s/ring->virtual_start/ring->vaddr/ Signed-off-by:
-
Chris Wilson authored
Ringbuffers are now being written to either through LLC or WC paths, so treating them as simply iomem is no longer adequate. However, for the older !llc hardware, the hardware is documentated as treating the TAIL register update as serialising, so we can relax the barriers when filling the rings (but even if it were not, it is still an uncached register write and so serialising anyway.). For simplicity, let's ignore the iomem annotation. v2: Remove iomem from ringbuffer->virtual_address v3: And for good measure add iomem elsewhere to keep sparse happy Signed-off-by:
-
Chris Wilson authored
Whilst this ultimately wraps kref_put_mutex(), our goal here is the lockless variant, so keep the _unlocked() suffix until we need it no more. s/drm_gem_object_unreference_unlocked/i915_gem_object_put_unlocked/ Signed-off-by:
-
Chris Wilson authored
Ultimately wraps kref_put(), so adopt its nomenclature for consistency with other subsystems. s/drm_gem_object_unreference/i915_gem_object_put/ Signed-off-by:
-
Chris Wilson authored
Ultimately wraps kref_get(), so adopt its nomenclature for consistency with other subsystems. s/drm_gem_object_reference/i915_gem_object_get/ Signed-off-by:
-
Chris Wilson authored
For symmetry with a forthcoming i915_gem_object_get() and i915_gem_object_put(). Signed-off-by:
-
Chris Wilson authored
As these are wrappers around kref_get/kref_put() it is preferable to follow the naming convention and use the same verb get/put in our wrapper names for manipulating a reference to the context. s/i915_gem_context_reference/i915_gem_context_get/ s/i915_gem_context_unreference/i915_gem_context_put/ Signed-off-by:
-
Chris Wilson authored
Now that we derive requests from struct fence, swap over to its nomenclature for references. It's shorter and more idiomatic across the kernel. s/i915_gem_request_reference/i915_gem_request_get/ s/i915_gem_request_unreference/i915_gem_request_put/ Signed-off-by:
Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/1469005202-9659-2-git-send-email-chris@chris-wilson.co.uk Link: http://patchwork.freedesktop.org/patch/msgid/1469017917-15134-1-git-send-email-chris@chris-wilson.co.uk
-
Chris Wilson authored
When transitioning to the GTT or CPU domain we wait on all rendering from i915 to complete (with the optimisation of allowing concurrent read access by both the GPU and client). We don't yet ensure all rendering from third parties (tracked by implicit fences on the dma-buf) is complete. Since implicitly tracked rendering by third parties will ignore our cache-domain tracking, we have to always wait upon rendering from third-parties when transitioning to direct access to the backing store. We still rely on clients notifying us of cache domain changes (i.e. they need to move to the GTT read or write domain after doing a CPU access before letting the third party render again). v2: This introduces a potential WARN_ON into i915_gem_object_free() as the current i915_vma_unbind() calls i915_gem_object_wait_rendering(). To hit this path we first need to render with the GPU, have a dma-buf attached with an unsignaled fence and then interrupt the wait. It does get fixed later in the series (when i915_vma_unbind() only waits on the active VMA and not all, including third-party, rendering. To offset that risk, use the __i915_vma_unbind_no_wait hack. Testcase: igt/prime_vgem/basic-fence-read Testcase: igt/prime_vgem/basic-fence-mmap Signed-off-by:
-
Chris Wilson authored
A foreign dma-buf does not share our cache domain tracking, and we rely on the producer ensuring cache coherency. Marking them as being in the CPU domain is incorrect. v2: Add commentary about the GTT domain. This is not the best place for it, but pending an actual overhaul of our domain tracking and explaining each one, this comment should help the next reader... Signed-off-by:
-
Chris Wilson authored
Since commit a6f766f3 ("drm/i915: Limit ring synchronisation (sw sempahores) RPS boosts") and commit bcafc4e3 ("drm/i915: Limit mmio flip RPS boosts") we have limited the waitboosting for semaphores and flips. Ideally we do not want to boost in either of these instances as no userspace consumer is waiting upon the results (though a userspace producer may be stalled trying to submit an execbuf - but in this case the producer is being throttled due to the engine being saturated with work). With the introduction of NO_WAITBOOST in the previous patch, we can finally disable these needless boosts. Signed-off-by:
-
Chris Wilson authored
We want to restrict waitboosting to known process contexts, where we can track which clients are receiving waitboosts and prevent excessive power wasting. For fence_wait() we do not have any client tracking and so that leaves it open to abuse. v2: Hide the IS_ERR_OR_NULL testing for special clients Signed-off-by:
-
Chris Wilson authored
dma-buf provides a generic fence class for interoperation between drivers. Internally we use the request structure as a fence, and so with only a little bit of interfacing we can rebase those requests on top of dma-buf fences. This will allow us, in the future, to pass those fences back to userspace or between drivers. v2: The fence_context needs to be globally unique, not just unique to this device. Signed-off-by:
-
Chris Wilson authored
Following a GPU reset upon hang, we retire all the requests and then mark them all as complete. If we mark them as complete first, we both keep the normal retirement order (completed first then retired) and provide a small optimisation for concurrent lookups. Signed-off-by:
-
Chris Wilson authored
In order to keep the memory allocated for requests reasonably tight, try to reuse the oldest request (so long as it is completed and has no external references) for the next allocation. v2: Throw in a comment to hopefully make sure no one mistakes the optimistic retirement of the oldest request for simply stealing it. Signed-off-by:
-
Chris Wilson authored
Migrate the request operations out of the main body of i915_gem.c and into their own C file for easier expansion. v2: Move __i915_add_request() across as well Signed-off-by:
-
- 19 Jul, 2016 10 commits
-
-
Rodrigo Vivi authored
This reverts commit 041824ee. We have latency issues that might impact the performance: #96606. and hangs and loading issues on resume after S4: #96526. This is also blocking a platform milestone so let's disable this for now while we make sure we don't have any more loading issue, or related basic hangs and it pass BAT for real in all platofmrs. In case BAT is wrong let's first fix BAT before re-enable it here. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96606 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96526 Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Dave Gordon <david.s.gordon@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Stable <stable@vger.kernel.org> Cc: Jani Nikula <jani.nikula@intel.com> Cc: Christophe Prigent <christophe.prigent@intel.com> Signed-off-by:
Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by:
-
Imre Deak authored
The purpose for each MOCS entry isn't well defined atm. Defining these is important to remove any uncertainty about the use of these entries for example in terms of performance and GPU/CPU coherency. Suggested by Ville. v4: - Rename I915_MOCS_AUTO to I915_MOCS_PTE. (Chris) CC: Rong R Yang <rong.r.yang@intel.com> CC: Yakui Zhao <yakui.zhao@intel.com> CC: Ville Syrjälä <ville.syrjala@linux.intel.com> CC: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
Imre Deak <imre.deak@intel.com> Reviewed-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1467383528-16142-1-git-send-email-imre.deak@intel.com
-
Imre Deak authored
Setting a write-back cache policy in the MOCS entry definition also implies snooping, which has a considerable overhead. This is unexpected for a few reasons: - From user-space's point of view since it didn't want a coherent surface (it didn't set the buffer as such via the set caching IOCTL). - There is a separate MOCS entry field for snooping (which we never set). - This MOCS table is about caching in (e)LLC and there is no (e)LLC on BXT. There is a separate table for L3 cache control. Considering the above the current behavior of snooping looks like an unintentional side-effect of the WB setting. Changing it to be LLC-UC gets rid of the snooping without any ill-effects. For a coherent surface the application would use a separate MOCS entry at index 1 and call the set caching IOCTL to setup the PTE entries for the corresponding buffer to be snooped. In the future we could also add a new MOCS entry for coherent surfaces. This resulted in 70% improvement in synthetic texturing benchmarks. Kudos to Valtteri Rantala, Eero Tamminen and Michael T Frederick and Ville who helped to narrow the source of problem to the kernel and to the snooping behaviour in particular. With a follow-up change to adjust the 3rd entry value igt/gem_mocs_settings is passing after this change. v2: - Rebase on v2 of patch 1/2. v3: - Set the entry as LLC uncached instead of PTE-passthrough. This way we also keep snooping disabled, but we also make the cacheability/ coherency setting indepent of the PTE which is managed by the kernel. (Chris) CC: Rong R Yang <rong.r.yang@intel.com> CC: Yakui Zhao <yakui.zhao@intel.com> CC: Valtteri Rantala <valtteri.rantala@intel.com> CC: Eero Tamminen <eero.t.tamminen@intel.com> CC: Michael T Frederick <michael.t.frederick@intel.com> CC: Ville Syrjälä <ville.syrjala@linux.intel.com> CC: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
Zhao Yakui <yakui.zhao@intel.com> Tested-by:
Rong R Yang <rong.r.yang@intel.com> Reviewed-by:
-
Imre Deak authored
Use named struct initializers for clarity. Also fix the target cache definition to reflect its role in GEN9 onwards. On GEN8 a TC value of 0 meant ELLC but on GEN9+ it means the TC and LRU controls are taken from the PTE. No functional change, igt/gem_mocs_settings still passing after this change. v2: (Chris) - Add back the hexa literals for the entries. Add note that igt/gem_mocs_settings still passes. CC: Rong R Yang <rong.r.yang@intel.com> CC: Yakui Zhao <yakui.zhao@intel.com> CC: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
Daniel Vetter authored
Not sure why so much slips through when 0day is catching these. Hopefully the much faster sphinx toolchain helps in unlazying people. Acked-by:
-
Daniel Vetter authored
Fairly minimal, there's still lots of functions without any docs, and which aren't static. But probably we want to first clean this up some more. - Drop the bogus const. Marking argument pointers themselves (instead of what they point at) as const provides roughly 0 value. And it's confusing, since the data the pointer points at _is_ being changed. - Remove kerneldoc for static functions. Keep comments where they seem valuable. - Indent and whitespace fixes. - Blockquote the bit field definitions of the descriptor for correct layouting. Acked-by:
-
Daniel Vetter authored
We need :: before, blank lines around and indentation with 4 _additional_ spaces to make it work. Also, don't use @param in code snippets, it results in confusion. Acked-by:
-
Chris Wilson authored
Even after adding individual page support for GTT mmaping, we can still fail to find any space within the mappable region, and drm_mm_insert_node() will then report ENOSPC. We have to then handle this error by using the shmem access to the pages. Fixes: b50a5371 ("drm/i915: Support for pread/pwrite ... objects") Testcase: igt/gem_concurrent_blit Signed-off-by:
-
Dave Gordon authored
Precursor for fix to secure batch execution. We will need to be able to retrieve the batch VMA (as well as the batch itself) from the eb list, so this patch extracts that part of eb_get_batch() into a separate function, and moves both parts to a more logical place in the file, near where the eb list is created. Also, it may not be obvious, but the current execbuffer2 ioctl interface requires that the buffer object containing the batch-to-be-executed be the LAST entry in the exec2_list[] array (I expected it to be the first!). To clarify this, we can replace the rather obscure construct "list_entry(eb->vmas.prev, ...)" in the old version of eb_get_batch() with the equivalent but more explicit "list_last_entry(&eb->vmas,...)" in the new eb_get_batch_vma() and of course add an explanatory comment. Signed-off-by:
-
Dave Gordon authored
Two different sets of flag bits are stored in the 'flags' member of a 'struct drm_i915_gem_exec_object2', and they're defined in two different source files, increasing the risk of an accidental clash. Some flags in this field are supplied by the user; these are defined in i915_drm.h, and they start from the LSB and work up. Other flags are defined in i915_gem_execbuffer, for internal use within that file only; they start from the MSB and work down. So here we add a compile-time check that the two sets of flags do not overlap, which would cause all sorts of confusion. Signed-off-by:
-
- 18 Jul, 2016 2 commits
-
-
Bob Paauwe authored
The i915 driver is now using atomic properties and atomic commit to handle the legacy set gamma IOCTL. However, if the driver is configured without atomic (nuclear_pageflip = false), it won't update the legacy properties for degamma_lut, gamma_lut and ctm leaving them out of sync with the atomic version of the properties. Until the driver is full atomic, make sure we update the non-atomic version of the properties. v2: Update the comment with a FIXME. (Daniel) v3: Update arguments of the gamma_set vfunc (Lionel) v4: Fixed vfunc prototype (Lionel) igt-testcase: kms_pipe_color / legacy-gamma-reset-pipeX Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: stable@vger.kernel.org #v4.7 Signed-off-by:
Bob Paauwe <bob.j.paauwe@intel.com> Signed-off-by:
Lionel Landwerlin <lionel.g.landwerlin@intel.com> Reviewed-by:
Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1468591142-2253-1-git-send-email-lionel.g.landwerlin@intel.com
-
Ville Syrjälä authored
eDP should be treated as connected even if doesn't have an EDID. In that case we'll use the timings from the VBT. That used to be the case until commit f21a2198 ("drm/i915: Splitting intel_dp_detect") broke things by considering even eDP disconnected if we fail to get an EDID for it. Fix things up again by treating eDP as always connected. Cc: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com> Cc: Nathan D Ciobanu <nathan.d.ciobanu@intel.com> Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> Cc: Larry Finger <larry.finger@lwfinger.net> Reported-by:
Larry Finger <larry.finger@lwfinger.net> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96675 Cc: drm-intel-fixes@lists.freedesktop.org Fixes: f21a2198 ("drm/i915: Splitting intel_dp_detect") Signed-off-by:
-
- 15 Jul, 2016 2 commits
-
-
Chris Wilson authored
Before suspend, and especially before building the hibernation image, we need to context image to be coherent in memory. To do this we require that we perform a context switch to a disposable context (i.e. the dev_priv->kernel_context) - when that switch is complete, all other context images will be complete. This leaves the kernel_context image as incomplete, but fortunately that is disposable and we can do a quick fixup of the logical state after resuming. v2: Share the nearly identical code to switch to the kernel context with eviction. v3: Explain why we need the switch and reset. Testcase: igt/gem_exec_suspend # bsw References: https://bugs.freedesktop.org/show_bug.cgi?id=96526Signed-off-by:
-
Chris Wilson authored
Currently execlists is exempt from emitting a request to switch each ring away from the current context over to the dev_priv->kernel_context (for whatever reason, just under execlists the GGTT is unlikely to be as fragmented, however the switch may help in some extreme cases). Extract the switcher and enable it for execlsts as well, as we need to do so in a later patch to force the context switch before suspend. (And since for that switch we explicitly require the disposable kernel context, rename the extracted function.) Signed-off-by:
-
- 14 Jul, 2016 4 commits
-
-
Chris Wilson authored
If the fbdev probing fails, and in our error path we fail to clear the dev_priv->fbdev, then we can try and use a dangling fbdev pointer, and in particular a NULL fb. This could also happen in pathological cases where we try to operate on the fbdev prior to it being probed. Reported-by:
-
Chris Wilson authored
Since the suspend_work can arm itself if the console_lock() is currently held elsewhere, simply calling flush_work() doesn't guarantee that the work is idle upon return. To do so requires using cancel_work_sync(). Signed-off-by:
-
Lionel Landwerlin authored
The i915 driver checks for color management properties changes as part of a plane update. Therefore a color management update must imply a plane update, otherwise we never update the transformation matrixes and degamma/gamma LUTs. v2: add comment about moving the commit of color management registers to an async worker v3: Commit color management register right after vblank v4: Move back color management commit condition together with planes commit v5: Trigger color management commit through the planes commit (Daniel) v6: Make plane change update more readable Fixes: 20a34e78 (drm/i915: Update color management during vblank evasion.) Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: drm-intel-fixes@lists.freedesktop.org Signed-off-by:Mario Kleiner <mario.kleiner.de@gmail.com> Signed-off-by:
-
Lyude authored
Unfortunately, there's two situations where we lose hpd right now: - Runtime suspend - When we've shut off all of the power wells on Valleyview/Cherryview While it would be nice if this didn't cause issues, this has the ability to get us in some awkward states where a user won't be able to get their display to turn on. For instance; if we boot a Valleyview system without any monitors connected, it won't need any of it's power wells and thus shut them off. Since this causes us to lose HPD, this means that unless the user knows how to ssh into their machine and do a manual reprobe for monitors, none of the monitors they connect after booting will actually work. Eventually we should come up with a better fix then having to enable polling for this, since this makes rpm a lot less useful, but for now the infrastructure in i915 just isn't there yet to get hpd in these situations. Changes since v1: - Add comment explaining the addition of the if (!mode_config->poll_running) in intel_hpd_init() - Remove unneeded if (!dev->mode_config.poll_enabled) in i915_hpd_poll_init_work() - Call to drm_helper_hpd_irq_event() after we disable polling - Add cancel_work_sync() call to intel_hpd_cancel_work() Changes since v2: - Apparently dev->mode_config.poll_running doesn't actually reflect whether or not a poll is currently in progress, and is actually used for dynamic module paramter enabling/disabling. So now we instead keep track of our own poll_running variable in dev_priv->hotplug - Clean i915_hpd_poll_init_work() a little bit Changes since v3: - Remove the now-redundant connector loop in intel_hpd_init(), just rely on intel_hpd_poll_enable() for setting connector->polled correctly on each connector - Get rid of poll_running - Don't assign enabled in i915_hpd_poll_init_work before we actually lock dev->mode_config.mutex - Wrap enabled assignment in i915_hpd_poll_init_work() in READ_ONCE() for doc purposes - Do the same for dev_priv->hotplug.poll_enabled with WRITE_ONCE in intel_hpd_poll_enable() - Add some comments about racing not mattering in intel_hpd_poll_enable Changes since v4: - Rename intel_hpd_poll_enable() to intel_hpd_poll_init() - Drop the bool argument from intel_hpd_poll_init() - Remove redundant calls to intel_hpd_poll_init() - Rename poll_enable_work to poll_init_work - Add some kerneldoc for intel_hpd_poll_init() - Cross-reference intel_hpd_poll_init() in intel_hpd_init() - Just copy the loop from intel_hpd_init() in intel_hpd_poll_init() Changes since v5: - Minor kerneldoc nitpicks Cc: stable@vger.kernel.org Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by:
Lyude <cpaul@redhat.com> Signed-off-by:
-
