- 22 Mar, 2019 5 commits
-
-
Chris Wilson authored
In preparation to making the ppGTT binding for a context explicit (to facilitate reusing the same ppGTT between different contexts), allow the user to create and destroy named ppGTT. v2: Replace global barrier for swapping over the ppgtt and tlbs with a local context barrier (Tvrtko) v3: serialise with struct_mutex; it's lazy but required dammit v4: Rewrite igt_ctx_shared_exec to be more different (aimed to be more similarly, turned out different!) v5: Fix up test unwind for aliasing-ppgtt (snb) v6: Tighten language for uapi struct drm_i915_gem_vm_control. v7: Patch the context image for runtime ppgtt switching! Testcase: igt/gem_vm_create Testcase: igt/gem_ctx_param/vm Testcase: igt/gem_ctx_clone/vm Testcase: igt/gem_ctx_shared Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190322092325.5883-2-chris@chris-wilson.co.uk
-
Chris Wilson authored
An idea for extending uABI inspired by Vulkan's extension chains. Instead of expanding the data struct for each ioctl every time we need to add a new feature, define an extension chain instead. As we add optional interfaces to control the ioctl, we define a new extension struct that can be linked into the ioctl data only when required by the user. The key advantage being able to ignore large control structs for optional interfaces/extensions, while being able to process them in a consistent manner. In comparison to other extensible ioctls, the key difference is the use of a linked chain of extension structs vs an array of tagged pointers. For example, struct drm_amdgpu_cs_chunk { __u32 chunk_id; __u32 length_dw; __u64 chunk_data; }; struct drm_amdgpu_cs_in { __u32 ctx_id; __u32 bo_list_handle; __u32 num_chunks; __u32 _pad; __u64 chunks; }; allows userspace to pass in array of pointers to extension structs, but must therefore keep constructing that array along side the command stream. In dynamic situations like that, a linked list is preferred and does not similar from extra cache line misses as the extension structs themselves must still be loaded separate to the chunks array. v2: Apply the tail call optimisation directly to nip the worry of stack overflow in the bud. v3: Defend against recursion. v4: Fixup local types to match new uabi Opens: - do we include the result as an out-field in each chain? struct i915_user_extension { __u64 next_extension; __u64 name; __s32 result; __u32 mbz; /* reserved for future use */ }; * Undecided, so provision some room for future expansion. Signed-off-by: -
Sujaritha Sundaresan authored
Adding a call to intel_uc_suspend in i915_gem_suspend, which is a common point for the suspend/resume and hibernate paths. This fixes an unbalanced call that causes issues with the CTB register/deregister. v2: Making the call unconditional (Daniele) Moving the call to after the GEM_BUG_ON (Chris) Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Signed-off-by:
Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> Reviewed-by:
-
Chris Wilson authored
Use the igt_live_test framework for detecting whether an unwanted hang occurred during test execution, and report failure if it does. Signed-off-by:
Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190321194031.20240-2-chris@chris-wilson.co.uk
-
Chris Wilson authored
32 is too many for the likes of kbl, and in order to insert that many requests into the ring requires us to declare the first few hung -- understandably a slow and unexpected process. Instead, measure the size of a singe requests and use that to estimate the upper bound on the chain length we can use for our test, remembering to flush the previous chain between tests for safety. Signed-off-by:
"Yokoyama, Caz" <caz.yokoyama@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190321194031.20240-1-chris@chris-wilson.co.uk
-
- 21 Mar, 2019 8 commits
-
-
Chris Wilson authored
If we are already in the desired write domain of a set-domain ioctl, then there is nothing for us to do and we can quickly return back to userspace, avoiding any lock contention. By recognising that the write_domain is always a subset of the read_domains, and excluding the no-op case of requiring 0 read_domains in the ioctl, we can infer if the current write_domain matches the target read_domains, there is nothing for us to do. Secondary aspect of this is that we undo the arbitrary fetching and potential flushing of all pages for a set-domain(.write=CPU) call on a fresh object -- which was introduced simply because we do the get-pages before taking the struct_mutex. References: 40e62d5d ("drm/i915: Acquire the backing storage outside of struct_mutex in set-domain") Signed-off-by:
Matthew Auld <matthew.william.auld@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190321161908.8007-2-chris@chris-wilson.co.uk
-
Chris Wilson authored
When we return pages to the system, we ensure that they are marked as being in the CPU domain since any external access is uncontrolled and we must assume the worst. This means that we need to always flush the pages on acquisition if we need to use them on the GPU, and from the beginning have used set-domain. Set-domain is overkill for the purpose as it is a general synchronisation barrier, but our intent is to only flush the pages being swapped in. If we move that flush into the pages acquisition phase, we know then that when we have obj->mm.pages, they are coherent with the GPU and need only maintain that status without resorting to heavy handed use of set-domain. The principle knock-on effect for userspace is through mmap-gtt pagefaulting. Our uAPI has always implied that the GTT mmap was async (especially as when any pagefault occurs is unpredicatable to userspace) and so userspace had to apply explicit domain control itself (set-domain). However, swapping is transparent to the kernel, and so on first fault we need to acquire the pages and make them coherent for access through the GTT. Our use of set-domain here leaks into the uABI that the first pagefault was synchronous. This is unintentional and baring a few igt should be unoticed, nevertheless we bump the uABI version for mmap-gtt to reflect the change in behaviour. Another implication of the change is that gem_create() is presumed to create an object that is coherent with the CPU and is in the CPU write domain, so a set-domain(CPU) following a gem_create() would be a minor operation that merely checked whether we could allocate all pages for the object. On applying this change, a set-domain(CPU) causes a clflush as we acquire the pages. This will have a small impact on mesa as we move the clflush here on !llc from execbuf time to create, but that should have minimal performance impact as the same clflush exists but is now done early and because of the clflush issue, userspace recycles bo and so should resist allocating fresh objects. Internally, the presumption that objects are created in the CPU write-domain and remain so through writes to obj->mm.mapping is more prevalent than I expected; but easy enough to catch and apply a manual flush. For the future, we should push the page flush from the central set_pages() into the callers so that we can more finely control when it is applied, but for now doing it one location is easier to validate, at the cost of sometimes flushing when there is no need. Signed-off-by:
-
Chris Wilson authored
The timeline->name is only used for convenience in pretty printing the i915_request.fence->ops->get_timeline_name() and it is just as convenient to pull it from the gem_context directly. The few instances of its use inside GEM_TRACE() has proven more of a nuisance than helpful, so not worth saving imo. Signed-off-by:
-
Chris Wilson authored
The user_handle need only be known by userspace for it to lookup the context via the idr; internally we have no use for it. Signed-off-by:
-
Chris Wilson authored
Define a mutex for the exclusive use of interacting with the per-file context-idr, that was previously guarded by struct_mutex. This allows us to reduce the coverage of struct_mutex, with a view to removing the last bits coordinating GEM context later. (In the short term, we avoid taking struct_mutex while using the extended constructor functions, preventing some nasty recursion.) v2: s/context_lock/context_idr_lock/ Signed-off-by:
-
Chris Wilson authored
In later patches, it became apparent that userspace can see a partially constructed GEM context and begin using it before it was ready, to much hilarity. Close this window of opportunity by lifting the registration of the context with userspace (the insertion of the context into the filp's idr) to the very end of the CONTEXT_CREATE ioctl. Signed-off-by:
-
Dan Carpenter authored
The mock_context() function returns NULL on error, it doesn't return error pointers. Fixes: 85fddf0b ("drm/i915: Introduce a context barrier callback") Signed-off-by:
Dan Carpenter <dan.carpenter@oracle.com> Reviewed-by:
Mika Kuoppala <mika.kuoppala@linux.intel.com> Signed-off-by:
-
Chris Wilson authored
gcc-4.8 and older dislike the use of __builtin_constant_p() within a constant expression context, and so we must use the magical __is_constexpr() instead. For example, with gcc-4.8.5: ../drivers/gpu/drm/i915/i915_reg.h:167:27: error: first argument to ‘__builtin_choose_expr’ not a constant ../include/linux/build_bug.h:16:45: error: bit-field ‘<anonymous>’ width not an integer constant Reported-by:
Randy Dunlap <rdunlap@infradead.org> Reported-by:
Uma Shankar <uma.shankar@intel.com> Fixes: baa09e7d ("drm/i915: use REG_FIELD_PREP() to define register bitfield values") Signed-off-by:
Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190320154021.5244-1-chris@chris-wilson.co.uk
-
- 20 Mar, 2019 27 commits
-
-
Daniele Ceraolo Spurio authored
This allows us to ditch i915 in some more places. v2: use local var in check_vgpu (Paulo) Signed-off-by:
Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by:
Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by:
-
Daniele Ceraolo Spurio authored
This will allow futher simplifications in the uncore handling. v2: move register access setup under uncore (Chris) Signed-off-by:
-
Daniele Ceraolo Spurio authored
Use a local variable where it makes sense. Signed-off-by:
-
Daniele Ceraolo Spurio authored
Remove unneeded usage of dev_priv from 1 extra function. Signed-off-by:
-
Daniele Ceraolo Spurio authored
Move the init, fini, prune, suspend, resume function to work on intel_uncore instead of dev_priv. Signed-off-by:
-
Daniele Ceraolo Spurio authored
Now that the internal code all works on intel_uncore, flip the external-facing interface. v2: fix GVT. Signed-off-by:
-
Daniele Ceraolo Spurio authored
Get/put functions used outside of uncore.c are updated in the next patch for a nicer split. v2: use dev_priv where we still have it (Paulo) Signed-off-by:
-
Daniele Ceraolo Spurio authored
Upper bits are reserved on gen6, so no issue if we write them. Note that we're already doing this in the non-MT case of IVB, which uses the same register. Signed-off-by:
-
Manasi Navare authored
This patch fixes the PORT_SYNC_MODE_MASTER_SELECT macro to correctly do the left shifting to set the port sync master select correctly. I have tested this fix on ICL. Fixes: 49edbd49 ("drm/i915/icl: Define TRANS_DDI_FUNC_CTL DSI registers") Cc: Madhav Chauhan <madhav.chauhan@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Cc: <stable@vger.kernel.org> # v5.0+ Signed-off-by:
Manasi Navare <manasi.d.navare@intel.com> Reviewed-by:
-
Andy Shevchenko authored
Switch to bitmap_zalloc() to show clearly what we are allocating. Besides that it returns pointer of bitmap type instead of opaque void *. Signed-off-by:
Andy Shevchenko <andriy.shevchenko@linux.intel.com> Reviewed-by:
-
Ville Syrjälä authored
skl_update_pipe_wm() is quite pointless now. Just inline it into skl_compute_wm(). v2: s/skl_build_pipe_wm/skl_update_pipe_wm/ in the commit message (Matt) Cc: Neel Desai <neel.desai@intel.com> Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190312205844.6339-10-ville.syrjala@linux.intel.comReviewed-by:
Matt Roper <matthew.d.roper@intel.com>
-
Ville Syrjälä authored
{skl,icl}_build_plane_wm() don't need to be passed the pipe_wm, so don't. And skl_build_pipe_wm() can easily dig it out itself. Cc: Neel Desai <neel.desai@intel.com> Signed-off-by: -
Ville Syrjälä authored
Clean up skl_allocate_pipe_ddb() a bit by moving the 'wm' variable to tighter scope. We'll also consitify it where appropriate. Also initialize plane_alloc/uv_plane_alloc when decrlaring them rather than later. v2: Update commit message (Matt) Cc: Neel Desai <neel.desai@intel.com> Signed-off-by:
-
Ville Syrjälä authored
Currently we disable all the watermarks above the selected max level for every plane. That would mean that the cursor's watermarks may also get modified when another plane causes the selected max watermark level to change. That is not so great as we would like to keep the cursor as indepenedent as possible to avoid having to throttle it in resposne to other plane activity. To avoid that let's keep the watermarks enabled even for levels above the max selected watermark level, iff the plane has enough ddb for that particular level. This way the cursor's enabled watermarks only depend on the cursor itself. This is safe because the hardware will never choose to use a watermark level unless all enabled planes have also enabled that level. Cc: Neel Desai <neel.desai@intel.com> Signed-off-by:
-
Ville Syrjälä authored
We use a fixed ddb allocation for the cursor. Now the calculation actually makes sure we have enough ddb space, but let's double check anyway. Cc: Neel Desai <neel.desai@intel.com> Signed-off-by:
-
Ville Syrjälä authored
Currently we just assume that 32 or 8 blocks of ddb is sufficient for the cursor. The 32 might be, but the 8 is certainly not. The minimum we need is at least what level 0 watermarks need, but that is a bit restrictive, so instead let's calculate what level 7 would need for a 256x256 cursor. We'll use that to determine the fixed ddb allocation for the cursor. This way the cursor will never be responsible for missing out on deeper power saving states. v2: Loop to make sure this works even if some wm levels are totally disabled (latency==0) Cc: Neel Desai <neel.desai@intel.com> Signed-off-by: -
Ville Syrjälä authored
Extract the meat of skl_compute_plane_wm_params() into a lower level helper that doesn't depend on the plane state. We'll reuse this for the cursor ddb allocation calculations. Cc: Neel Desai <neel.desai@intel.com> Signed-off-by:
-
Ville Syrjälä authored
skl_compute_plane_wm() doesn't actually need the plane state. While it would make logically sense to pass it, we shall need to reuse skl_compute_plane_wm() to compute the minimum ddb allocation for the cursor before the cursor may be enabled. Thus we can't rely on the plane state. The alternative would be to duplicate a lot of the wm calculations for the cursor ddb allocation case, which doens't appeal to me. Cc: Neel Desai <neel.desai@intel.com> Signed-off-by:
-
Ville Syrjälä authored
If the minimum required ddb space for all the planes equals the total ddb space available we are allowed to use the relevant watermark level. Cc: Neel Desai <neel.desai@intel.com> Signed-off-by:
-
Ville Syrjälä authored
To allow unsetting .is_mobile for the desktop variant of PNV fix up the cdclk code to select the mobile HPLLVCO register for both PNV variants. Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Signed-off-by:
-
Ville Syrjälä authored
We want to allow the desktop PNV to not have .is_mobile set. To that end let's add a small helper to determine if the platform has the ASLE interrupt (or equivalent). Supposdely both PNV variants have it. Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Signed-off-by:
-
Ville Syrjälä authored
Add a small helper to determine if we have the panel power sequencer or not. We'll make PNV an exceptional case so that we can unset .is_mobile for the desktop variant. Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Signed-off-by:
-
Ville Syrjälä authored
Make the code self-documenting by introducing i9xx_has_pfit(). Also make PNV an exceptional case so that we can unset .is_mobile for the desktop variant. v2: s/gen4/gen>=4/ (Tvrtko) Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Signed-off-by:
-
Ville Syrjälä authored
g33/i964g/g45 are the exceptional cases when it comes to the swizzle detection. Let's reorder the code to handle them first and let everything else be handled by the else branch. This allows us to unset .is_mobile for the desktop PNV variant (which supposedly must follow the "mobile" path here). Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Signed-off-by:
-
Daniele Ceraolo Spurio authored
Exercise acquiring and releasing forcewake around register reads. In order to read a register behind a GT powerwell, we need to instruct that powerwell to wake up using a forcewake. When we no longer require the GT powerwell, we tell the GT to release our forcewake. Inside the forcewake, the register read should work but outside it should just return garbage, 0 being the most common garbage. Thus we can detect when we are inside and outside of the forcewake with just a simple register read, and so can verify that the GT powerwell is released when we say so. v2: Picking the right forcewaked register to return 0 outside of forcewake is an art. Signed-off-by:
-
Chris Wilson authored
If a test fails, we quite often mark the device as wedged. Provide the stub functions so that we can wedge the mock device, and avoid exploding on test failures. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109981Signed-off-by:
-
Joonas Lahtinen authored
Signed-off-by:
-
