Ville Syrjälä [Fri, 1 Sep 2017 17:12:52 +0000 (20:12 +0300)]
drm/i915: io unmap functions want __iomem
Don't cast away the __iomem from the io_mapping functions so that
sparse won't be so unhappy when we pass the pointer to the unmap
functions. Instead let's move the cast to where we actually use the
pointer.
Fixes the following sparse warnings:
i915_gem.c:1022:33: warning: incorrect type in argument 1 (different address spaces)
i915_gem.c:1022:33: expected void [noderef] <asn:2>*vaddr
i915_gem.c:1022:33: got void *[assigned] vaddr
i915_gem.c:1027:34: warning: incorrect type in argument 1 (different address spaces)
i915_gem.c:1027:34: expected void [noderef] <asn:2>*vaddr
i915_gem.c:1027:34: got void *[assigned] vaddr
i915_gem.c:1199:33: warning: incorrect type in argument 1 (different address spaces)
i915_gem.c:1199:33: expected void [noderef] <asn:2>*vaddr
i915_gem.c:1199:33: got void *[assigned] vaddr
i915_gem.c:1204:34: warning: incorrect type in argument 1 (different address spaces)
i915_gem.c:1204:34: expected void [noderef] <asn:2>*vaddr
i915_gem.c:1204:34: got void *[assigned] vaddr
Ville Syrjälä [Fri, 1 Sep 2017 19:54:56 +0000 (22:54 +0300)]
drm/i915: Wake up the device for the fbdev setup
Our fbdev setup requires the device to be awake for access
through the GTT. If one boots without connected displays and
later plugs one in, we won't have any runtime PM references when
the fbdev setup runs. Explicitly grab a runtime PM reference during
the fbdev setup to avoid the following spew:
Changbin Du [Mon, 4 Sep 2017 08:01:01 +0000 (16:01 +0800)]
drm/i915: Add interface to reserve fence registers for vGPU
In the past, vGPU alloc fence registers by walking through mm.fence_list
to find fence which pin_count = 0 and vma is empty. vGPU may not find
enough fence registers this way. Because a fence can be bind to vma even
though it is not in using. We have found such failure many times these
days.
An option to resolve this issue is that we can force-remove fence from
vma in this case.
This patch added two new api to the fence management code:
- i915_reserve_fence() will try to find a free fence from fence_list
and force-remove vma if need.
- i915_unreserve_fence() reclaim a reserved fence after vGPU has
finished.
With this change, the fence management is more clear to work with vGPU.
GVTg do not need remove fence from fence_list in private.
v3: (Chris)
- Add struct_mutex lock assertion.
- Only count for unpinned fence.
v2: (Chris)
- Rename the new api for symmetry.
- Add safeguard to ensure at least 1 fence remained for host display.
The header comment in include/trace/define_trace.h specifies that the
TRACE_INCLUDE_PATH needs to be relative to the define_trace.h header
rather than the trace file including it. Most instances get that wrong
and work around it by adding the $(src) directory to the include path.
While this works, it is preferable to refer to the correct path to the
trace file in the first place and avoid any workaround.
Ville Syrjälä [Fri, 1 Sep 2017 14:31:23 +0000 (17:31 +0300)]
drm/i915: Fix enum pipe vs. enum transcoder for the PCH transcoder
Use enum pipe for PCH transcoders also in the FIFO underrun code.
Fixes the following new sparse warnings:
intel_fifo_underrun.c:340:49: warning: mixing different enum types
intel_fifo_underrun.c:340:49: int enum pipe versus
intel_fifo_underrun.c:340:49: int enum transcoder
intel_fifo_underrun.c:344:49: warning: mixing different enum types
intel_fifo_underrun.c:344:49: int enum pipe versus
intel_fifo_underrun.c:344:49: int enum transcoder
intel_fifo_underrun.c:397:57: warning: mixing different enum types
intel_fifo_underrun.c:397:57: int enum pipe versus
intel_fifo_underrun.c:397:57: int enum transcoder
intel_fifo_underrun.c:398:17: warning: mixing different enum types
intel_fifo_underrun.c:398:17: int enum pipe versus
intel_fifo_underrun.c:398:17: int enum transcoder
Ville Syrjälä [Fri, 1 Sep 2017 14:31:22 +0000 (17:31 +0300)]
drm/i915: Make i2c lock ops static
Make gmbus_lock_ops and proxy_lock_ops static to appease sparse
intel_i2c.c:652:34: warning: symbol 'gmbus_lock_ops' was not declared. Should it be static?
intel_sdvo.c:2981:34: warning: symbol 'proxy_lock_ops' was not declared. Should it be static?
Ville Syrjälä [Fri, 1 Sep 2017 14:31:21 +0000 (17:31 +0300)]
drm/i915: Make i9xx_load_ycbcr_conversion_matrix() static
Make i9xx_load_ycbcr_conversion_matrix() static to appease sparse:
intel_color.c:110:6: warning: symbol 'i9xx_load_ycbcr_conversion_matrix' was not declared. Should it be static?
Ville Syrjälä [Wed, 23 Aug 2017 15:22:23 +0000 (18:22 +0300)]
drm/i915: Pass proper old/new states to intel_plane_atomic_check_with_state()
Eliminate plane->state and crtc->state usage from
intel_plane_atomic_check_with_state() and its callers. Instead pass the
proper states in or dig them up from the top level atomic state.
Note that intel_plane_atomic_check_with_state() itself isn't allowed to
use the top level atomic state as there is none when it gets called from
the legacy cursor short circuit path.
v2: Rename some variables for easier comprehension (Maarten)
Up to Coffeelake we could deduce this GT number from the device ID.
This doesn't seem to be the case anymore. This change reorders pciids
per GT and adds a gt field to intel_device_info. We set this field on
the following platforms :
Manasi Navare [Tue, 15 Aug 2017 18:59:51 +0000 (11:59 -0700)]
drm/i915/edp: Increase T12 panel delay to 900 ms to fix DP AUX CH timeouts
This patch fixes the DP AUX CH timeouts observed during CI runs causing
CI Failures on a specific PCI device. This issue was fixed previously
by adding a quirk but looks like we need to increase this delay even more
in order to get rid all the DP AUX CH timeouts.
Fixes: 5db5832ef5ff03a2 ("drm/i915/edp: Add a T12 panel delay quirk to fix
DP AUX CH timeouts")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101144 Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> Cc: Clinton Taylor <clinton.a.taylor@intel.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1502823591-25310-1-git-send-email-manasi.d.navare@intel.com
Ville Syrjälä [Mon, 10 Jul 2017 19:33:47 +0000 (22:33 +0300)]
drm/i915: Consolidate max_cdclk_freq check in intel_crtc_compute_min_cdclk()
Currently the .modeset_calc_cdclk() hooks check the final cdclk value
against the max allowed. That's not really sufficient since the low
level calc_cdclk() functions effectively clamp the minimum required
cdclk to the max supported by the platform. Hence if the minimum
required exceeds the platforms capabilities we'd keep going anyway
using the max cdclk frequency.
To fix that let's move the check earlier into
intel_crtc_compute_min_cdclk() and we'll check the minimum required
cdclk of the pipe against the maximum supported by the platform.
Ville Syrjälä [Wed, 30 Aug 2017 18:57:03 +0000 (21:57 +0300)]
drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock"
Make the min_pixclk thing less confusing by changing it to track
the minimum acceptable cdclk frequency instead. This means moving
the application of the guardbands to a slightly higher level from
the low level platform specific calc_cdclk() functions.
The immediate benefit is elimination of the confusing 2x factors
on GLK/CNL+ in the audio workarounds (which stems from the fact
that the pipes produce two pixels per clock).
v2: Keep cdclk higher on CNL to workaround missing DDI clock voltage handling
v3: Squash with the CNL cdclk limits patch (DK)
v4: s/intel_min_cdclk/intel_pixel_rate_to_cdclk/ (DK)
Rodrigo Vivi [Thu, 31 Aug 2017 14:53:56 +0000 (07:53 -0700)]
drm/i915/cnl: Fix DP max voltage
On clock recovery this function is called to find out
the max voltage swing level that we could go.
However gen 9 functions use the old buffer translation tables
to figure that out. That table is not valid for CNL
causing an invalid number of entries and an invalid selection
on the max voltage swing level.
v2: Let's use same approach that previous platforms.
v3: Actually use n_entries and avoid duplicated -1.
v4: Avoid cnl_max_level and use current style.
Rodrigo Vivi [Tue, 29 Aug 2017 23:22:28 +0000 (16:22 -0700)]
drm/i915/cnl: Move ddi buf trans related functions up.
No functional changes. But those functions will be needed
to get max level for HDMI and DP, so let's move those
up closer to other similar functions existent for previous
platforms.
Rodrigo Vivi [Tue, 29 Aug 2017 23:22:27 +0000 (16:22 -0700)]
drm/i915/cnl: Move voltage check into ddi buf trans functions.
Let's start converging CNL buf translations to same style
used on previous platforms. So first thing is to use the
standard signature so we don't need to propagate the voltage
check into other parts of the code, but only on the parts
that it is really useful.
Rodrigo Vivi [Tue, 29 Aug 2017 23:22:26 +0000 (16:22 -0700)]
drm/i915: Enable voltage swing before enabling DDI_BUF_CTL.
Sequences for DisplayPort asks us to
" Configure voltage swing and related IO settings.
Refer to DDI Buffer section."
before "Configure and enable DDI_BUF_CTL"
On BXT and CNL this means to execute the ddi vswing sequences.
At this point these sequences calls are getting duplicated for DP
because they are all called from DP link trainning sequences.
However this patch is not yet removing it before a futher discussion
since spec also allows that during link training without disabling
anything:
"
Notes
Changing voltage swing during link training:
Change the swing setting following the DDI Buffer section.
The port does not need to be disabled.
"
Rodrigo Vivi [Tue, 29 Aug 2017 23:22:25 +0000 (16:22 -0700)]
drm/i915: Align vswing sequences with old ddi buffer registers.
Vswing sequences on BXT and CNL are equivalent
to the ddi buffer registers setting on other platforms.
For some reason it got aligned with skl_ddi_set_iboost what
is semantically incorrect. This forced us to keep skipping
ddi buffer translation tables on the platforms that has
the vswing sequences.
v2: Don't mess with DP signal levels on this patch.
Cc: Vandana Kannan <vandana.kannan@intel.com> Cc: Imre Deak <imre.deak@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170829232230.23051-3-rodrigo.vivi@intel.com
Rodrigo Vivi [Tue, 29 Aug 2017 23:09:07 +0000 (16:09 -0700)]
drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.
Driver’s CPU access to GTT is via the GTTMMADR BAR.
The current HW implementation of that BAR is to only
support <= DW (and maybe QW) writes—not 16/32/64B writes
that could occur with WC and/or SSE/AVX moves.
GTTMMADR must be marked uncacheable (UC).
Accesses to GTTMMADR(GTT), must be 64 bits or less (ie. 1 GTT entry).
v2: Get clarification on the reasons and spec is getting
updated to reflect it now.
Ville Syrjälä [Thu, 24 Aug 2017 19:10:50 +0000 (22:10 +0300)]
drm/i915: Skip fence alignemnt check for the CCS plane
The CCS won't have the same stride as the main surface anyway so trying
to guard against the fence stride not matching the CCS stride is
not sensible. Just skip the fence vs. fb alignment check for the aux
plane.
Cc: Ben Widawsky <ben@bwidawsk.net> Cc: Jason Ekstrand <jason@jlekstrand.net> Cc: Daniel Stone <daniels@collabora.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170824191100.10949-3-ville.syrjala@linux.intel.com Reviewed-by: Ben Widawsky <ben@bwidawsk.net> Fixes: 97a64a810c90 ("drm/i915: Add render decompression support") Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Ville Syrjälä [Thu, 24 Aug 2017 19:10:49 +0000 (22:10 +0300)]
drm/i915: Treat fb->offsets[] as a raw byte offset instead of a linear offset
Userspace wants to treat fb->offsets[] as raw byte offsets into the gem
bo. Adjust the kernel code to match.
Cc: Ben Widawsky <ben@bwidawsk.net> Cc: Jason Ekstrand <jason@jlekstrand.net> Cc: Daniel Stone <daniels@collabora.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170824191100.10949-2-ville.syrjala@linux.intel.com Acked-by: Ben Widawsky <ben@bwidawsk.net> Fixes: 97a64a810c90 ("drm/i915: Add render decompression support") Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Chris Wilson [Tue, 29 Aug 2017 19:25:46 +0000 (20:25 +0100)]
drm/i915: Always wake the device to flush the GTT
Since we hold the device wakeref when writing through the GTT (otherwise
the writes would fail), we presumed that before the device sleeps those
writes would naturally be flushed and that we wouldn't need our mmio
read trick. However, that presumption seems false and a sleepy bxt seems
to require us to always manually flush the GTT writes prior to direct
access.
Chris Wilson [Sat, 26 Aug 2017 11:09:35 +0000 (12:09 +0100)]
drm/i915: Discard the request queue if we fail to sleep before suspend
If we fail to clear the outstanding request queue before suspending,
mark those requests as lost.
References: https://bugs.freedesktop.org/show_bug.cgi?id=102037 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170826110935.10237-3-chris@chris-wilson.co.uk Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Chris Wilson [Sat, 26 Aug 2017 11:09:34 +0000 (12:09 +0100)]
drm/i915: Clear wedged status upon resume
When we wake up from suspend, the device has been powered down and
should come back afresh. We should be able to safely remove the wedged
status from the previous session and start afresh.
Chris Wilson [Sat, 26 Aug 2017 11:09:33 +0000 (12:09 +0100)]
drm/i915: Always sanity check engine state upon idling
When we do a locked idle we know that afterwards all requests have been
completed and the engines have been cleared of tasks. For whatever
reason, this doesn't always happen and we may go into a suspend with
ELSP still full, and this causes an issue upon resume as we get very,
very confused.
If the engines refuse to idle, mark the device as wedged. In the process
we get rid of the maybe unused open-coded version of wait_for_engines
reported by Nick Desaulniers and Matthias Kaehlcke.
v2: Suppress the -EIO before suspend, but keep it for seqno wrap.
References: https://bugs.freedesktop.org/show_bug.cgi?id=101891
References: https://bugs.freedesktop.org/show_bug.cgi?id=102456 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Matthias Kaehlcke <mka@chromium.org> Link: https://patchwork.freedesktop.org/patch/msgid/20170826110935.10237-1-chris@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Chris Wilson [Sat, 26 Aug 2017 13:56:20 +0000 (14:56 +0100)]
drm/i915: Don't use GPU relocations prior to cmdparser stalls
If we are using the cmdparser, we will have to copy the batch and so
stall for the relocations. Rather than prolong that stall by adding more
relocation requests, just use CPU relocations and do the stall upfront.
Chris Wilson [Mon, 28 Aug 2017 10:46:31 +0000 (11:46 +0100)]
drm/i915: Recreate vmapping even when the object is pinned
Sometimes we know we are the only user of the bo, but since we take a
protective pin_pages early on, an attempt to change the vmap on the
object is denied because it is busy. i915_gem_object_pin_map() cannot
tell from our single pin_count if the operation is safe. Instead we must
pass that information down from the caller in the manner of
I915_MAP_OVERRIDE.
This issue has existed from the introduction of the mapping, but was
never noticed as the only place where this conflict might happen is for
cached kernel buffers (such as allocated by i915_gem_batch_pool_get()).
Until recently there was only a single user (the cmdparser) so no
conflicts ever occurred. However, we now use it to allocate batches for
different operations (using MAP_WC on !llc for writes) in addition to the
existing shadow batch (using MAP_WB for reads).
We could either keep both mappings cached, or use a different write
mechanism if we detect a MAP_WB already exists (i.e. clflush
afterwards), but as we haven't seen this issue in the wild (it requires
hitting the GPU reloc path in addition to the cmdparser) for simplicity
just allow the mappings to be recreated.
v2: Include the i915_MAP_OVERRIDE bit in the enum so the compiler knows
about all the valid values.
Fixes: a155bfac9fd5 ("drm/i915: Async GPU relocation processing")
Testcase: igt/gem_lut_handle # byt, completely by accident Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170828104631.8606-1-chris@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Marta Lofstedt [Mon, 28 Aug 2017 12:18:10 +0000 (15:18 +0300)]
drm/i915: Beef up of Beef up the IPS vs. CRC workaround
Commit c8a64630c1e8 ("drm/i915: Beef up the IPS vs. CRC
workaround") was supposed to solve below bug. However, the
patch I tested is not the same as the one that got merged.
With this addition the test pass.
Chris Wilson [Fri, 25 Aug 2017 15:02:15 +0000 (16:02 +0100)]
drm/i915: Quietly cancel FBC activation if CRTC is turned off before worker
Since we use a worker to enable FBC on the CRTC, it is possible for the
CRTC to be switched off before we run. In this case, the CRTC will not
allow us to wait upon a vblank, so remove the DRM_ERROR as this is very
much expected.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102410 Fixes: 330a63b958c7 ("drm/i915/fbc: wait for a vblank instead of 50ms when enabling") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170825150215.19236-1-chris@chris-wilson.co.uk Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Praveen Paneri [Thu, 10 Aug 2017 18:30:33 +0000 (00:00 +0530)]
drm/i915: Fix FBC cfb stride programming for non X-tiled FB
When FBC is enabled for linear, legacy Y-tiled and Yf-tiled
surfaces on gen9, the cfb stride must be programmed by SW as
cfb_stride = ceiling[(at least plane width in pixels)/
(32 * compression limit factor)] * 8
v2: Minor fix for a build error
v3: Fixed subject, register name and platform check (Ville)
v4: Added WA details in comment (Paulo)
v5:
- Read modified reg write to preserve other bit values (Paulo)
- Store modified stride value in reg_params (Paulo)
- Keep GLK out of the WA (Paulo)
v6:
- added additional field in reg_params for gen9_wa_cfb_stride (Paulo)
- Used appropriate bit mask while writing the register (Paulo)
v7 (from Paulo):
- Fix coding style and spacing issues.
- Mask the old values before writing.
- Bikeshed comments and unnecessary checks.
Jani Nikula [Thu, 24 Aug 2017 18:54:03 +0000 (21:54 +0300)]
drm/i915/bios: throw away high level child device union
All the child device config fields, including legacy, are now available
in the same struct, so use it for everything.
As this change touches plenty of code with "p_child", rename them to
"child" while at it. Also do some simple unification and constification
where not intrusive. This in the name of avoiding extra cleanup churn
for the same lines as here.
Jani Nikula [Thu, 24 Aug 2017 18:54:02 +0000 (21:54 +0300)]
drm/i915/bios: add legacy contents to common child device config
Add legacy contents to common child device config, in preparation for
using a single child device config. Use unions where BDB versions of the
config differ. Use the naming from old_child_dev_config for legacy
fields.
Add both some new and some old fields to child device config
parameters. Prepare for switching to just one child device config. Use
naming from struct old_child_dev_config for common fields.
Ville Syrjälä [Thu, 17 Aug 2017 14:55:09 +0000 (17:55 +0300)]
drm/i915: Beef up the IPS vs. CRC workaround
Oneshot disabling of IPS when CRC capturing is started is insufficient.
IPS may get re-enabled by any plane update, and hence tests that keep
CRC capturing on across plane updates will start to see inconsistent
results as soon as IPS kicks back in. Add a new knob into the crtc state
to make sure IPS stays disabled as long as CRC capturing is enabled.
Forcing a modeset is the easiest way to handle this since that's already
how we do the panel fitter workaround. It's a little heavy handed just
for IPS, but seeing as we might already do the panel fitter workaround
I think it's better to follow that. We migth want to optimize both cases
later if someone gets too upset by the extra delay from the modeset.
v2: Check the right thing when deciding whether to force a modeset
v3: Rebase, check HAS_IPS before forcing a modeset,
move ips_force_disable check into pipe_config_supports_ips()
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Marta Lofstedt <marta.lofstedt@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101664 Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Tested-by: Marta Lofsted <marta.lofstedt@intel.com> #v2 Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170817145509.15549-1-ville.syrjala@linux.intel.com
Chris Wilson [Tue, 22 Aug 2017 11:05:17 +0000 (12:05 +0100)]
drm/i915: Ignore duplicate VMA stored within the per-object handle LUT
By using drm_gem_flink/drm_gem_open on an object using the same fd, it
is possible for a client to create multiple handles pointing to the same
object (tied to the same contexts and VMA), as exemplified by
igt::gem_handle_to_libdrm_bo(). Since this duplication has been possible
since forever, we cannot assume that the handle:(fpriv, object) is
unique and so must handle the multiple users of a single VMA.
v2: Added commentary noise.
Testcase: igt/gem_close
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102355 Fixes: 1b5dde0153b2 ("drm/i915: Replace execbuf vma ht with an idr") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170822110517.22277-3-chris@chris-wilson.co.uk Tested-by: Marta Lofstedt <marta.lofstedt@intel.com> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
Chris Wilson [Tue, 22 Aug 2017 11:05:16 +0000 (12:05 +0100)]
drm/i915: Assert that the handle->vma lut is empty on object close
Make sure that we are not leaking an entry in the ctx->handles_lut by
asserting that the object was removed prior to being freed. This should
be enforced by all such handles being removed by i915_gem_close_object.
Chris Wilson [Tue, 22 Aug 2017 11:05:15 +0000 (12:05 +0100)]
drm/i915: Assert the context is not closed on object-close
During the context-close, we should be decoupling all the vma from the
object so that upon object-closing we shouldn't see any vma from the
already closed contexts. So include a check upon closing the object that
the context is still open.
v2: Eek, the fpriv check is required for shared objects. Double eek, BAT
passed?
To avoid a potential hang condition with TLB invalidation
we need to enable masked bit 5 of MMIO 0xE5F0 at boot.
Same workaround was in place for previous platforms,
but the register offset has changed for CNL.
But also BSpec doesn't mention the bit 15 as set on gen9
platforms and mark bit as reserved on CNL.
Chris Wilson [Tue, 22 Aug 2017 17:38:28 +0000 (18:38 +0100)]
drm/i915: Keep a small stash of preallocated WC pages
We use WC pages for coherent writes into the ppGTT on !llc
architectures. However, to create a WC page requires a stop_machine(),
i.e. is very slow. To compensate we currently keep a per-vm cache of
recently freed pages, but we still see the slow startup of new contexts.
We can amoritize that cost slightly by allocating WC pages in small
batches (PAGEVEC_SIZE == 14) and since creating a WC page implies a
stop_machine() there is no penalty for keeping that stash global.
Chris Wilson [Thu, 17 Aug 2017 12:37:06 +0000 (13:37 +0100)]
drm/i915: Boost GPU clocks if we miss the pageflip's vblank
If we miss the current vblank because the gpu was busy, that may cause a
jitter as the frame rate temporarily drops. We try to limit the impact
of this by then boosting the GPU clock to deliver the frame as quickly
as possible. Originally done in commit 808d1ecace30 ("drm/i915: Boost GPU
frequency if we detect outstanding pageflips") but was never forward
ported to atomic and finally dropped in commit 6d6647463981 ("drm/i915:
Rip out legacy page_flip completion/irq handling").
One of the most typical use-cases for this is a mostly idle desktop.
Rendering one frame of the desktop's frontbuffer can easily be
accomplished by the GPU running at low frequency, but often exceeds
the time budget of the desktop compositor. The result is that animations
such as opening the menu, doing a fullscreen switch, or even just trying
to move a window around are slow and jerky. We need to respond within a
frame to give the best impression of a smooth UX, as a compromise we
instead respond if that first frame misses its goal. The result should
be a near-imperceivable initial delay and a smooth animation even
starting from idle. The cost, as ever, is that we spend more power than
is strictly necessary as we overestimate the required GPU frequency and
then try to ramp down.
This of course is reactionary, too little, too late; nevertheless it is
surprisingly effective.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102199 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170817123706.6777-1-chris@chris-wilson.co.uk Tested-by: Lyude Paul <lyude@redhat.com> Reviewed-by: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
Ville Syrjälä [Fri, 18 Aug 2017 13:49:58 +0000 (16:49 +0300)]
drm/i915: Constify states passed to enable/disable/etc. encoder hooks
The enable/disable/etc. encoder hooks aren't supposed to alter the
state(s), so pass them as const. Unfortunately C lacks any kind of deep
const thingy, so this can't catch all abuses. But at least it acts as a
hint to the reader telling them not to mess about with the state(s).
v2: Update intel_tv_mode_find() and ironlake_edp_pll_on() as well
v3: Deal with intel_sdvo_connector_state
Ville Syrjälä [Fri, 18 Aug 2017 13:49:56 +0000 (16:49 +0300)]
drm/i915: Plumb crtc_state to PSR enable/disable
The PSR enable/disable need to know things about the crtc state, so
plumb it through. This will become even more important when we start
to reuse the generic infoframe code for the VSC DIP programming as the
infoframe code wants the crtc state as well.
Ville Syrjälä [Fri, 18 Aug 2017 13:49:55 +0000 (16:49 +0300)]
drm/i915: Init infoframe vfuncs for DP encoders as well
DP ports may want to use the video DIP for SDP transmission, so let's
initialize the vfuncs for DP encoders as well. The only exception is
port A eDP prior to HSW as that one doesn't have a video DIP instance.
Ville Syrjälä [Tue, 22 Aug 2017 14:09:14 +0000 (17:09 +0300)]
drm/i915: Disable infoframes when shutting down DDI HDMI
Disabling the video DIP when shutting the port down seems like a good
idea.
Bspec says:
"When disabling both the DIP port and DIP transmission,
first disable the port and then disable DIP."
and
"Restriction : GCP is only supported with HDMI when the bits per color is
not equal to 8. GCP must be enabled prior to enabling TRANS_DDI_FUNC_CTL
for HDMI with bits per color not equal to 8 and disabled after disabling
TRANS_DDI_FUNC_CTL"
So let's do it in the .post_disable() hook.
v2: Remove double "dpms off" caused by rebase fail
Michel Thierry [Fri, 18 Aug 2017 17:23:42 +0000 (10:23 -0700)]
drm/i915: Re-enable per-engine reset for Broxton
The corruption in CSB mmio reads we were seeing has been tracked down to
incorrectly touching forcewake of all domains, following an engine reset.
It is still a mistery why we only catched this in Broxton, since it
could happen in any platform.
With that fix already merged, commit 89853a6bd0e3 ("drm/i915: Stop
touching forcewake following a gen6+ engine reset"), lets try to enable
per-engine resets in Broxton one more time.
This reverts commit f188258bde0f ("drm/i915: Disable per-engine reset for
Broxton").
Jani Nikula [Fri, 18 Aug 2017 09:30:19 +0000 (12:30 +0300)]
drm/i915/dp: rename intel_dp_is_edp to intel_dp_is_port_edp
Emphasize that this is based on the port, not intel_dp. This is also in
line with the underlying intel_bios_is_port_edp() function. No
functional changes.
Rodrigo Vivi [Tue, 15 Aug 2017 23:16:51 +0000 (16:16 -0700)]
drm/i915/cnl: Apply large line width optimization
This bit enables hardware that will change the approximation used for distances
calculations for AA wide lines so that they are rendered more accurately.
The default value for this bit leaves the legacy behavior. There is no good
reason to not enable the new approximation except if comparing to previous GEN
rendered images.
v2: Rebase
v3: Fix author.
Rebased by Rodrigo who also added a comment as suggested by Oscar.
Since it is surrounded by Workarounds let's just add a comment to
make clear it is not an Wa.
Let's inherit workarounds from previous platforms that
according to wa_database and BSpec are still valid for
Cannonlake.
v2: Add missed workarounds.
v3: Rebase
v4: Remove bad chunk that was added to rc6 disable. (Ander)
Also remove A0 W/a that are not needed anymore.
v5: Rebase on top of CFL.
v6: Remove empty gen9_init_perctx_bb and gen9_init_indirectctx_bb
since they don't carry any gen10 related W/a. (by Oscar).
Also Remove A0 exclusive workaround.
v7: Remove more A0 exclusive workarounds. As pointed out by Oscar
many workarounds were changed to be A0 only so let's remove
them.
Chris Wilson [Mon, 7 Aug 2017 12:19:19 +0000 (13:19 +0100)]
drm/i915: Clear lost context-switch interrupts across reset
During a global reset, we disable the irq. As we disable the irq, the
hardware may be raising a GT interrupt that we then ignore, leaving it
pending in the GTIIR. After the reset, we then re-enable the irq,
triggering the pending interrupt. However, that interrupt was for the
stale state from before the reset, and the contents of the CSB buffer
are now invalid.
v2: Add a comment to make it clear that the double clear is purely my
paranoia.
Andy Shevchenko [Thu, 17 Aug 2017 10:55:41 +0000 (13:55 +0300)]
drm/i915/bxt: use NULL for GPIO connection ID
The commit ec18aede5b20
("drm/i915/bxt: add bxt dsi gpio element support")
enables GPIO support for Broxton based platforms.
While using that API we might get into troubles in the future, because
we can't rely on label name in the driver since vendor firmware might
provide any GPIO pin there, e.g. "reset", and even mark it in _DSD (in
which case the request will fail).
To avoid inconsistency and potential issues we have two options:
a) generate GPIO ACPI mapping table and supply it via
acpi_dev_add_driver_gpios(), or
b) just pass NULL as connection ID.
The b) approach is much simpler and would work since the driver relies
on GPIO indices only. Moreover, the _CRS fallback mechanism, when
requesting GPIO, has been made stricter, and supplying non-NULL
connection ID when neither _DSD, nor GPIO ACPI mapping is present, is
making request fail.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101921 Fixes: 19c3cebd37c7 ("gpio: acpi: Even more tighten up ACPI GPIO lookups") Cc: Mika Kahola <mika.kahola@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Tested-by: Mika Kahola <mika.kahola@intel.com> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170817105541.63914-1-andriy.shevchenko@linux.intel.com
Chris Wilson [Thu, 17 Aug 2017 14:47:19 +0000 (15:47 +0100)]
drm/i915: Mark the GT as busy before idling the previous request
In a synchronous setup, we may retire the last request before we
complete allocating the next request. As the last request is retired, we
queue a timer to mark the device as idle, and promptly have to execute
ad cancel that timer once we complete allocating the request and need to
keep the device awake. If we rearrange the mark_busy() to occur before
we retire the previous request, we can skip this ping-pong.
v2: Joonas pointed out that unreserve_seqno() was now doing more than
doing seqno handling and should be renamed to reflect its wider purpose.
That also highlighted the new asymmetry with reserve_seqno(), so fixup
that and rename both to [un]reserve_engine().
Chris Wilson [Wed, 16 Aug 2017 08:52:08 +0000 (09:52 +0100)]
drm/i915: Replace execbuf vma ht with an idr
This was the competing idea long ago, but it was only with the rewrite
of the idr as an radixtree and using the radixtree directly ourselves,
along with the realisation that we can store the vma directly in the
radixtree and only need a list for the reverse mapping, that made the
patch performant enough to displace using a hashtable. Though the vma ht
is fast and doesn't require any extra allocation (as we can embed the node
inside the vma), it does require a thread for resizing and serialization
and will have the occasional slow lookup. That is hairy enough to
investigate alternatives and favour them if equivalent in peak performance.
One advantage of allocating an indirection entry is that we can support a
single shared bo between many clients, something that was done on a
first-come first-serve basis for shared GGTT vma previously. To offset
the extra allocations, we create yet another kmem_cache for them.
Chris Wilson [Wed, 16 Aug 2017 08:52:07 +0000 (09:52 +0100)]
drm/i915: Simplify eb_lookup_vmas()
Since the introduction of being able to perform a lockless lookup of an
object (i915_gem_object_get_rcu() in 254199ca8f79 ("drm/i915: Move object
release to a freelist + worker") we no longer need to split the
object/vma lookup into 3 phases and so combine them into a much simpler
single loop.
Chris Wilson [Wed, 16 Aug 2017 08:52:06 +0000 (09:52 +0100)]
drm/i915: Convert execbuf to use struct-of-array packing for critical fields
When userspace is doing most of the work, avoiding relocs (using
NO_RELOC) and opting out of implicit synchronisation (using ASYNC), we
still spend a lot of time processing the arrays in execbuf, even though
we now should have nothing to do most of the time. One issue that
becomes readily apparent in profiling anv is that iterating over the
large execobj[] is unfriendly to the loop prefetchers of the CPU and it
much prefers iterating over a pair of arrays rather than one big array.
v2: Clear vma[] on construction to handle errors during vma lookup
Chris Wilson [Wed, 16 Aug 2017 08:52:05 +0000 (09:52 +0100)]
drm/i915: Check context status before looking up our obj/vma
Since we keep the context around across the slow lookup where we may
drop the struct_mutex, we should double check that the context is still
valid upon reacquisition.
Chris Wilson [Wed, 16 Aug 2017 08:52:04 +0000 (09:52 +0100)]
drm/i915: Don't use MI_STORE_DWORD_IMM on Sandybridge/vcs
MI_STORE_DWORD_IMM just doesn't work on the video decode engine under
Sandybridge, so refrain from using it. Then switch the selftests over to
using the now common test prior to using MI_STORE_DWORD_IMM.
Fixes: a155bfac9fd5 ("drm/i915: Async GPU relocation processing") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.13-rc1+ Link: https://patchwork.freedesktop.org/patch/msgid/20170816085210.4199-1-chris@chris-wilson.co.uk Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Chris Wilson [Thu, 17 Aug 2017 17:32:29 +0000 (18:32 +0100)]
drm/i915: Stop touching forcewake following a gen6+ engine reset
Forcewake is not affected by the engine reset on gen6+. Indeed the
reason why we added intel_uncore_forcewake_reset() to
gen6_reset_engines() was to keep the bookkeeping intact because the
reset did not touch the forcewake bit (yet we cancelled the forcewake
consumers)! This was done in commit f9f161e3f8b88:
Author: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Date: Fri Aug 23 16:52:30 2013 +0300
drm/i915: sanitize forcewake registers on reset
In reset we try to restore the forcewake state to
pre reset state, using forcewake_count. The reset
doesn't seem to clear the forcewake bits so we
get warn on forcewake ack register not clearing.
That futzing of the forcewake bookkeeping was dropped in commit 54dbe639e1c7 ("drm/i915: Consolidate forcewake resetting to a single
function"), but it did not make the realisation that the remaining
intel_uncore_forcewake_reset() was redundant.
The new danger with using intel_uncore_forcewake_reset() with per-engine
resets is that the driver and hw are still in an active state as we
perform the reset. We may be using the forcewake to read protected
registers elsewhere and those results may be clobbered by the concurrent
dropping of forcewake.
Reported-by: Michel Thierry <michel.thierry@intel.com> Fixes: 0f9eb0e20641 ("drm/i915: Modify error handler for per engine hang recovery") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Michel Thierry <michel.thierry@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170817173229.20324-1-chris@chris-wilson.co.uk Reviewed-by: Michel Thierry <michel.thierry@intel.com> Acked-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Daniel Vetter [Tue, 15 Aug 2017 16:01:01 +0000 (18:01 +0200)]
MAINTAINERS: drm/i915 has a new maintainer team
For a bunch of reasons[1] I've decided to step down as maintainer and
let some other folks enjoy the reputation and hang out in the
spotlight.
Jani is going to stick around with his expertise in kms and having
done the fixes flow for a long time now. Joonas will join and bring in
his knowledge on all things GEM. Rodrigo has been less visible because
he's been doing tons of work taking care of the internal branch, and
it'd be good to have more continuity between these two worlds also on
the maintainer side.
1: They all boil down to: This is going to happen sooner or later
anyway, we have a great team, with the process improvements over the
last few years things work rather well, now is as good as any time to
do this. With that change I'll have more time for other aspects of the
stack development than maintainership.
Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Dave Airlie <airlied@gmail.com> Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Acked-by: Jani Nikula <jani.nikula@linux.intel.com> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170815160101.1683-1-daniel.vetter@ffwll.ch
Anusha Srivatsa [Wed, 16 Aug 2017 23:45:14 +0000 (16:45 -0700)]
drm/i915: Split pin mapping into per platform functions
Cleanup the code. Map the pins in accordance to
individual platforms rather than according to ports.
Create separate functions for platforms.
v2:
- Add missing condition for CoffeeLake. Make platform
specific functions static. Add function
i915_ddc_pin_mapping().
v3:
- Rename functions to x_port_to_ddc_pin() which directly
indicates the purpose. Correct default return values on CNP
and BXT. Rename i915_port_to_ to g4x_port_to since that was
the first platform to run this. Correct code style. (Paulo)
Sugested-by Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Clinton Taylor <clinton.a.taylor@intel.com> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1502927114-24012-1-git-send-email-anusha.srivatsa@intel.com
Jani Nikula [Thu, 17 Aug 2017 11:52:09 +0000 (14:52 +0300)]
drm/i915/opregion: let user specify override VBT via firmware load
Sometimes it would be most enlightening to debug systems by replacing
the VBT to be used. For example, in the referenced bug the BIOS provides
different VBT depending on the boot mode (UEFI vs. legacy). It would be
interesting to try the failing boot mode with the VBT from the working
boot, and see if that makes a difference.
Add a module parameter to load the VBT using the firmware loader, not
unlike the EDID firmware mechanism.
As a starting point for experimenting, one can pick up the BIOS provided
VBT from /sys/kernel/debug/dri/0/i915_opregion/i915_vbt.
v2: clarify firmware load return value check (Bob)
projects / kernel.git / log