- 16 Oct, 2017 26 commits
-
-
Chris Wilson authored
If we fail to recover the HW state upon resume (i.e. our attempt to clear the wedged bit and reset during i915_gem_sanitize() fails), then skip the HW restart inside i915_gem_init_hw(). We will ultimately do the HW restart when successfully unwedging and resetting the HW later, but attempting to restore a wedged device upon resume is risky as the HW is in an unknown state. v2: Suppress the error message when detecting the already wedged HW. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103240 Testcase: igt/gem_eio/in-flight-suspend Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20171015143725.27764-1-chris@chris-wilson.co.ukReviewed-by:
Mika Kuoppala <mika.kuoppala@linux.intel.com>
-
Chris Wilson authored
We free objects in bulk after they wait for their RCU grace period. Currently, we take struct_mutex and unbind all the objects. This can lead to a long lock duration during which time those objects have their pages unfreeable (i.e. the shrinker is prevented from reaping those pages). If we only process a single object under the struct_mutex and then free the pages, the number of objects locked away from the shrinker is minimal and we allow regular clients better access to struct_mutex if they need it. Signed-off-by:
Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20171013202621.7276-9-chris@chris-wilson.co.uk
-
Chris Wilson authored
Inspired by Tvrtko's critique of the reaping of the stale contexts before allocating a new one, also limit the freed object reaping to the oldest stale object before allocating a fresh object. Unlike contexts, objects may have radically different sizes of backing storage, but similar to contexts, while we want to prevent starvation due to excessive freed lists, we also do not want to delay fresh allocations for too long. Only freeing the oldest on the freed object list before each allocation is a reasonable compromise. v2: Only a single consumer of llist_del_first() is allowed (although multiple llist_add are still allowed in parallel). Unlike i915_gem_context, i915_gem_flush_free_objects() is itself not serialized and so we need to add our own spinlock. Otherwise KASAN eventually spots a use-after-free for the race on *first->next. Signed-off-by:
-
Chris Wilson authored
Prefer to defer activating our GEM shrinker until we have a few megabytes to free; or we have accumulated sufficient mempressure by deferring the reclaim to force a shrink. The intent is that because our objects may typically be large, we are too effective at shrinking and are not rewarded for freeing more pages than the batch. It will also defer the initial shrinking to hopefully put it at a lower priority than say the buffer cache (although it will balance out over a number of reclaims, with GEM being more bursty). v2: Give it a feedback system to try and tune the batch size towards an effective size for the available objects. v3: Start keeping track of shrinker stats in debugfs v4: Protect against finding no shrinkable objects (div-by-zero) Signed-off-by:
-
Chris Wilson authored
shrink_slab() allows us to report back the number of objects we successfully scanned (out of the target shrinkctl->nr_to_scan). As report the number of pages owned by each GEM object as a separate item to the shrinker, we cannot precisely control the number of shrinker objects we scan on each pass; and indeed may free more than requested. If we fail to tell the shrinker about the number of objects we process, it will continue to hold a grudge against us as any objects left unscanned are added to the next reclaim -- and so we will keep on "unfairly" shrinking our own slab in comparison to other slabs. v2: fixup the misplaced addition, we want to count everything we scan (to match the number we reported earlier) not just the objects we successfully validated and freed. References: 912d572d ("drm/i915: wire up shrinkctl->nr_scanned") Signed-off-by:
-
Chris Wilson authored
Remove the struct_mutex requirement around dev_priv->mm.bound_list and dev_priv->mm.unbound_list by giving it its own spinlock. This reduces one more requirement for struct_mutex and in the process gives us slightly more accurate unbound_list tracking, which should improve the shrinker - but the drawback is that we drop the retirement before counting so i915_gem_object_is_active() may be stale and lead us to underestimate the number of objects that may be shrunk (see commit bed50aea ("drm/i915/shrinker: Flush active on objects before counting")). v2: Crosslink the spinlock to the lists it protects, and btw this changes s/obj->global_link/obj->mm.link/ v3: Fix decoupling of old links in i915_gem_object_attach_phys() v3.1: Fix the fix, only unlink if it was linked v3.2: Use a local for to_i915(obj->base.dev)->mm.obj_lock Signed-off-by:
-
Chris Wilson authored
In the next patch, we want to reduce the lock coverage within the shrinker, and one of the dangerous walks we have is over obj->vma_list. We are only walking the obj->vma_list in order to check whether it has been permanently pinned by HW access, typically via use on the scanout. But we have a couple of other long term pins, the context objects for which we currently have to check the individual vma pin_count. If we instead mark these using obj->pin_display, we can forgo the dangerous and sometimes slow list iteration. v2: Rearrange code to try and avoid confusion from false associations due to arrangement of whitespace along with rebasing on obj->pin_global. Signed-off-by:
-
Chris Wilson authored
It has now lost its meaning (it shows more than just pin_display), I do not believe that we are using in preference to the complete listing from i915_gem_gtt, or the listing from i915_gem_framebuffer, or the listing of active display objects in i915_display_info. Signed-off-by:
-
Chris Wilson authored
In the next patch, we want to extend use of the global pin counter for semi-permanent pinning of context/ring objects. Given that we plan to extend the usage to encompass a disparate set of objects, we want a name that reflects both and should entail less confusion. Signed-off-by:
-
Chris Wilson authored
Since we occasionally stuff an error pointer into obj->mm.pages for a semi-permanent or even permanent failure, we have to be more careful and not just test against NULL when deciding if the object has a complete set of its concurrent pages. Signed-off-by:
-
Rodrigo Vivi authored
Apparently RC6 residency is lower than expected with EI mode for most of the cases on CNL A0, B0 and C0. This Wa doesn't solve our lower residency, but I believe it is better to have it since EI is not expected to work by HW engineers anyways. Cc: David Weinehall <david.weinehall@linux.intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Signed-off-by:
Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by:
David Weinehall <david.weinehall@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170822235828.18322-1-rodrigo.vivi@intel.com
-
Michal Wajdeczko authored
No functional change. Just place the function closer to related definitions. Signed-off-by:
Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> Reviewed-by:
-
Michal Wajdeczko authored
In case of GuC firmware loading failure we were reporting DRM_ERROR also for case when GuC loading was not strictly required. Signed-off-by:
-
Michal Wajdeczko authored
Firmware loading for GuC and HuC are similar. Move common code into generic function for maximum reuse. v2: change message levels (Chris) Signed-off-by:
-
Michal Wajdeczko authored
Some of the error messages from DMC load were too generic and may be confusing for the user. Lets explicitly add DMC words there. Also as homepage of DMC firmware is same as for the GuC and Huc, lets reuse URL definition. Signed-off-by:
-
Michal Wajdeczko authored
In case of firmware fetch failure we should help user find latest firmware. URL macro duplication from csr.c will be fixed in upcoming patch. Suggested-by:
-
Michal Wajdeczko authored
Time to remove less important info and make messages clear and consistent. v2: change some message levels (Chris) v3: restore DRM_WARN (Chris) Signed-off-by:
-
Michal Wajdeczko authored
GuC status message printed right after firmware upload may be too optimistic, as we may fail on subsequent steps. Move that message to the end of intel_uc_init_hw where we know the status for sure. v2: use dev_info (Chris) Signed-off-by:
-
Michal Wajdeczko authored
Both GuC and HuC firmwares will be moved into WOPCM so we can check ucode size early for both cases. Suggested-by:
-
Michal Wajdeczko authored
Functions should be defined in their use order. Signed-off-by:
-
Michal Wajdeczko authored
Remaining functions in intel_guc_loader.c were focused around GuC firmware. Rename them to match object-verb pattern and rename file itself. Suggested-by:
-
Michal Wajdeczko authored
After GuC code reorg some documentation was left in wrong place. Move it closer to corresponding definitions. v2: use consistent name for the GuC (Sagar) Signed-off-by:
Sagar Arun Kamble <sagar.a.kamble@intel.com> Signed-off-by:
-
Michal Wajdeczko authored
Existing code needs some style fixes. To avoid polluting pure move patch, do it now as separate step. Suggested-by:
-
Michal Wajdeczko authored
We want to keep ucode xfer functions separate from other initialization. Once separated, add explicit forcewake. v2: use BLITTER domain only and add comment (Daniele) Suggested-by:
Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Signed-off-by:
-
Michal Wajdeczko authored
Function intel_guc_wopcm_size didn't fit well in the old place. With this move upcoming cleanup will be simpler. Suggested-by:
-
Weinan Li authored
Let GVT-g VM read the CSB and CSB write pointer from virtual HWSP, not all the host support this feature, need to check the BIT(3) of caps in PVINFO. v3 : Remove unnecessary comments. v4 : Separate VM enable patch with GVT-g implementation patch due to code dependency. v5 : Use inline for GVT virtual HWSP caps check function. v6 : Comments refine. Signed-off-by:
Weinan Li <weinan.z.li@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by:
-
- 13 Oct, 2017 14 commits
-
-
Chris Wilson authored
The compiler warns: drivers/gpu/drm/i915/intel_ddi.c:118:35: warning: ‘bdw_ddi_translations_fdi’ defined but not used Lo and behold, if we look at intel_ddi_get_buf_trans_fdi(), it uses hsw_ddi_translations_fdi[] for both Haswell and *Broadwell* Fixes: 7d1c42e6 ("drm/i915: Refactor code to select the DDI buf translation table") Signed-off-by:
Jani Nikula <jani.nikula@linux.intel.com> Reviewed-by:
Ville Syrjälä <ville.syrjala@linux.intel.com>
-
Chris Wilson authored
Always try to stop the rings, even if the GPU reset itself has been disabled (via modparam i915.reset). This should at least stop the hw from spinning in the background consuming resources (e.g. power and memory bandwidth) letting the system rest-in-peace. References: https://bugs.freedesktop.org/show_bug.cgi?id=103260Signed-off-by:
Mika Kuoppala <mika.kuoppala@intel.com>
-
Chris Wilson authored
Before modifying the ring register (RING_START, HEAD, TAIL, CTL) we first make sure it is stopped (or else the hw may not resample the registers). However, we do not need to let the hw restart until after we have reprogrammed all the rings. This should help prevent situations where pending operations on the ring may resume (because we are trying to re-initialize following an unsuccessful GPU hang, i.e. from i915_gem_unset_wedged). Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103260Signed-off-by:
-
Chris Wilson authored
Since the removal of the stop_machine(), it is allowed and expected for the nop_submit_request() and nop_complete_submit_request() to run in parallel to the i915_gem_set_wedged() processing. As such we can no longer assert that i915_gem_set_wedged() has completed inside the stop_machine prior to the individual nop_submit_request execution. Fixes: af7a8ffa ("drm/i915: Use rcu instead of stop_machine in set_wedged") Signed-off-by:
Daniel Vetter <daniel.vetter@intel.com> Reviewed-by:
-
Ville Syrjälä authored
Untangle intel_enable_ddi() by splitting it into DP and HDMI specific variants. v2: Keep using intel_ddi_get_encoder_port() for now Signed-off-by:
Jani Nikula <jani.nikula@intel.com>
-
Ville Syrjälä authored
Rather that plumb the link parameters separately to intel_ddi_pre_enable_dp() let's just pass the entire crtc state. intel_ddi_pre_enable_hdmi() already took the crtc state, but for some reason intel_ddi_pre_enable() still wanted to extract has_infoframe from therein and pass it in separately. Let's not do that since it's pointless. v2: Rebase due to more code getting pulled into the DDI hooks Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by:
-
Ville Syrjälä authored
Untangle intel_disable_ddi() by splitting it into DP and HDMI specific variants. Signed-off-by:
-
Ville Syrjälä authored
intel_edp_panel_on() will itself do the is_edp() check, so the caller doesn't have to bother. Pre-DDI code doesn't bother, so let's follow the same approach for DDI. Reviewed-by:
-
Ville Syrjälä authored
To clean up the mess in intel_ddi_post_disable() split it into two clean variants for HDMI and DP. v2: Rebase due to MST DPMS changes Reviewed-by: Jani Nikula <jani.nikula@intel.com> #v1 Signed-off-by:
-
Ville Syrjälä authored
To untangle the mess that is intel_ddi_post_disable() move the the bits needed by FDI into intel_ddi_fdi_post_disable(). This way we can stop worrying about FDI in intel_ddi_post_disable(). Reviewed-by:
-
Ville Syrjälä authored
Extract the code to disable the DDI_BUF_CTL into small helper. This will allows us to detangle the encoder type mess in intel_ddi_post_disable(). v2: Keep using intel_ddi_get_encoder_port() for now Reviewed-by:
-
Ville Syrjälä authored
Pull the code to disable the port clock into a function. We already have the intel_ddi_clk_select() counterpart. v2: Keep using intel_ddi_get_encoder_port() for now (Chris) Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
Ville Syrjälä authored
To make it easier to debug things let's dump the output types bitmask in the crtc state dump. And to make life that much better, let's pretty print it as a a human reaadable string as well. v2: Have the caller pass in the buffer (Chris) #undef OUTPUT_TYPE (Jani) Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Jani Nikula <jani.nikula@linux.intel.com> Signed-off-by: -
Harsha Sharma authored
Replace instances of drm_framebuffer_reference/unreference() with *_get/put() suffixes and drm_dev_unref with *_put() suffix because get/put is shorter and consistent with the kernel use of *_get/put suffixes. Done with following coccinelle semantic patch @@ expression ex; @@ ( -drm_framebuffer_unreference(ex); +drm_framebuffer_put(ex); | -drm_dev_unref(ex); +drm_dev_put(ex); | -drm_framebuffer_reference(ex); +drm_framebuffer_get(ex); ) Signed-off-by:
Harsha Sharma <harshasharmaiitr@gmail.com> [danvet: Drop the drm_dev_put change for now, to make the patch apply with out a backmerge.] Signed-off-by:
Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20171009120643.11953-1-harshasharmaiitr@gmail.com
-
