- 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:
Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/1468884477-30086-1-git-send-email-rodrigo.vivi@intel.com
-
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:
Chris Wilson <chris@chris-wilson.co.uk> Signed-off-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 <david.s.gordon@intel.com> Reviewed-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:
Mika Kuoppala <mika.kuoppala@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1468590980-6186-1-git-send-email-chris@chris-wilson.co.uk
-
- 14 Jul, 2016 24 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:
-
Lyude authored
One of the things preventing us from using polling is the fact that calling valleyview_crt_detect_hotplug() when there's a VGA cable connected results in sending another hotplug. With polling enabled when HPD is disabled, this results in a scenario like this: - We enable power wells and reset the ADPA - output_poll_exec does force probe on VGA, triggering a hpd - HPD handler waits for poll to unlock dev->mode_config.mutex - output_poll_exec shuts off the ADPA, unlocks dev->mode_config.mutex - HPD handler runs, resets ADPA and brings us back to the start This results in an endless irq storm getting sent from the ADPA whenever a VGA connector gets detected in the middle of polling. Somewhat based off of the "drm/i915: Disable CRT HPD around force trigger" patch Ville Syrjälä sent a while back Cc: stable@vger.kernel.org Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by:
-
Lyude authored
While VGA hotplugging worked(ish) before, it looks like that was mainly because we'd unintentionally enable it in valleyview_crt_detect_hotplug() when we did a force trigger. This doesn't work reliably enough because whenever the display powerwell on vlv gets disabled, the values set in VLV_ADPA get cleared and consequently VGA hotplugging gets disabled. This causes bugs such as one we found on an Intel NUC, where doing the following sequence of hotplugs: - Disconnect all monitors - Connect VGA - Disconnect VGA - Connect HDMI Would result in VGA hotplugging becoming disabled, due to the powerwells getting toggled in the process of connecting HDMI. Changes since v3: - Expose intel_crt_reset() through intel_drv.h and call that in vlv_display_power_well_init() instead of encoder->base.funcs->reset(&encoder->base); Changes since v2: - Use intel_encoder structs instead of drm_encoder structs Changes since v1: - Instead of handling the register writes ourself, we just reuse intel_crt_detect() - Instead of resetting the ADPA during display IRQ installation, we now reset them in vlv_display_power_well_init() Cc: stable@vger.kernel.org Acked-by: -
Lyude authored
This lets call intel_crt_reset() in contexts where IRQs are disabled and as such, can't hold the locks required to work with the connectors. Cc: stable@vger.kernel.org Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Acked-by:
-
Chris Wilson authored
This function is no longer used outside of intel_pm.c so we can stop exposing it and rename the __gen6_update_ring_freq() to take its place. Suggested-by:
-
Chris Wilson authored
Some hardware requires a valid render context before it can initiate rc6 power gating of the GPU; the default state of the GPU is not sufficient and may lead to undefined behaviour. The first execution of any batch will load the "golden render state", at which point it is safe to enable rc6. As we do not forcibly load the kernel context at resume, we have to hook into the batch submission to be sure that the render state is setup before enabling rc6. However, since we don't enable powersaving until that first batch, we queued a delayed task in order to guarantee that the batch is indeed submitted. v2: Rearrange intel_disable_gt_powersave() to match. v3: Apply user specified cur_freq (or idle_freq if not set). v4: Give in, and supply a delayed work to autoenable rc6 v5: Mika suggested a couple of better names for delayed_resume_work v6: Rebalance rpm_put around the autoenable task Signed-off-by:
-
Chris Wilson authored
Instead of flushing the outstanding enabling, remember the requested frequency to apply when the powersave work runs. Signed-off-by:
-
Chris Wilson authored
To allow the user finer control over waitboosting, allow them to set the frequency we request for the boost. This also them allows to effectively disable the boosting by setting the boost request to a low frequency. Signed-off-by:
-
Chris Wilson authored
Move the overclocking max frequency detection alongside the regular frequency detection, before we expose the undefined value to userspace. Signed-off-by:
-
Chris Wilson authored
As these RPS frequency values are part of our userspace interface, they must be established before that userspace interface is registered. Signed-off-by:
-
Chris Wilson authored
Select idle frequency during initialisation, then reset the last known frequency when re-enabling. This allows us to preserve the user selected frequency across resets. v2: Stop CHV from overriding the user's choice in cherryview_enable_rps() Signed-off-by:
-
Chris Wilson authored
Upon resetting the GPU, we force the engines to be idle by clearing their request lists. However, I neglected to clear the GT active status and so the next request following the reset was not marking the device as busy again. (We had to wait until any outstanding retire worker finally ran and cleared the active status.) Fixes: 67d97da3 ("drm/i915: Only start retire worker when idle") Testcase: igt/pm_rps/reset Signed-off-by:
Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
-
Matthew Auld authored
This should already be handled by drm_gem_object_release, which is called later on. Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
Matthew Auld <matthew.auld@intel.com> Reviewed-by:
-
Ville Syrjälä authored
Dell XPS 13 9350 apparently doesn't like it when we use the panel type from OpRegion. The OpRegion panel type (0) tells us to use use low vswing for eDP, whereas the VBT panel type (2) tells us to use normal vswing. The problem is that low vswing results in some display flickers. Since no one seems to know how this stuff is supposed to be handled, let's just ignore the OpRegion panel type on SKL for now. v2: Print the panel type correctly in the debug output Reported-by:
James Bottomley <James.Bottomley@HansenPartnership.com> Cc: James Bottomley <James.Bottomley@HansenPartnership.com> Cc: drm-intel-fixes@lists.freedesktop.org References: https://lists.freedesktop.org/archives/intel-gfx/2016-June/098826.html Fixes: a0562819 ("drm/i915: Get panel_type from OpRegion panel details") Signed-off-by:
-
Tvrtko Ursulin authored
Created two common helpers for engine setup and engine init phases respectively to help with code sharing. Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1468422221-12132-1-git-send-email-tvrtko.ursulin@linux.intel.com Reviewed-by: Chris Wilson <chris-wilson.co.uk>
-
Tvrtko Ursulin authored
Common code deserves to be put in a separate file from legacy and execlists implementation for clarity and ease of maintenance. Signed-off-by: -
Tvrtko Ursulin authored
Engine contains dev_priv so need to pass it in. Signed-off-by:
-
Tvrtko Ursulin authored
Use more of the shared engine setup data for legacy engine initialization. This time to simplify the irq initialization code. Signed-off-by: -
Tvrtko Ursulin authored
With the unified common engine setup done, and the execlist engine initialization loop clearly split into two phases, we can eliminate the separate legacy engine initialization code. v2: Fix cleanup path for legacy. v3: Rename constructors. (Chris Wilson) Signed-off-by:
-
Tvrtko Ursulin authored
Move the execlist engine setup to vfuncs so that the engine init loop is clearly split into the mode agnostic and specific steps. Signed-off-by:
-
Dave Gordon authored
intel_lrc.c has a table of "logical rings" (meaning engines), while intel_ringbuffer.c has separately open-coded initialisation for each engine. We can deduplicate this somewhat by using the same first-stage engine-setup function for both modes. So here we expose the function that transfers information from the static table of (all) known engines to the dev_priv->engine array of engines available on this device (adjusting the names along the way) and then embed calls to it in both the LRC and the legacy-mode setup. Signed-off-by:
-
- 13 Jul, 2016 1 commit
-
-
Ville Syrjälä authored
Prior to gen6 we didn't have per-ring IMR registers, which means that since commit 61ff75ac ("drm/i915: Simplify enabling user-interrupts with L3-remapping") we're now masking off all interrupts when init_render_ring() gets called. That's rather rude. Let's limit the ring IMR frobbing to machines that actually have the per-ring IMR registers. Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Fixes: 61ff75ac ("drm/i915: Simplify enabling user-interrupts with L3-remapping") Signed-off-by:
-
- 12 Jul, 2016 1 commit
-
-
Chris Wilson authored
Make sure we keep kbuilder happy in all of its random configs by providing argument names for compile-time stubs. In file included from drivers/gpu/drm/i915/intel_dp_mst.c:27:0: drivers/gpu/drm/i915/i915_drv.h: In function 'i915_debugfs_register': >> drivers/gpu/drm/i915/i915_drv.h:3612:48: error: parameter name omitted static inline int i915_debugfs_register(struct drm_i915_private *) {return 0;} ^~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_drv.h: In function 'i915_debugfs_unregister': drivers/gpu/drm/i915/i915_drv.h:3613:51: error: parameter name omitted static inline void i915_debugfs_unregister(struct drm_i915_private *) {} Reported-by: 0day Signed-off-by:
-
