https://github.com/torvalds/linux
Revision 9128b040eb774e04bc23777b005ace2b66ab2a85 authored by Daniel Vetter on 03 March 2015, 16:31:21 UTC, committed by Linus Torvalds on 03 March 2015, 17:04:33 UTC
This is a tricky story of the new atomic state handling and the legacy
code fighting over each another. The bug at hand is an underrun of the
framebuffer reference with subsequent hilarity caused by the load
detect code. Which is peculiar since the the exact same code works
fine as the implementation of the legacy setcrtc ioctl.

Let's look at the ingredients:

- Currently our code is a crazy mix of legacy modeset interfaces to
  set the parameters and half-baked atomic state tracking underneath.
  While this transition is going we're using the transitional plane
  helpers to update the atomic side (drm_plane_helper_disable/update
  and friends), i.e. plane->state->fb. Since the state structure owns
  the fb those functions take care of that themselves.

  The legacy state (specifically crtc->primary->fb) is still managed
  by the old code (and mostly by the drm core), with the fb reference
  counting done by callers (core drm for the ioctl or the i915 load
  detect code). The relevant commit is

  commit ea2c67bb4affa84080c616920f3899f123786e56
  Author: Matt Roper <matthew.d.roper@intel.com>
  Date:   Tue Dec 23 10:41:52 2014 -0800

      drm/i915: Move to atomic plane helpers (v9)

- drm_plane_helper_disable has special code to handle multiple calls
  in a row - it checks plane->crtc == NULL and bails out. This is to
  match the proper atomic implementation which needs the crtc to get
  at the implied locking context atomic updates always need. See

  commit acf24a395c5a9290189b080383564437101d411c
  Author: Daniel Vetter <daniel.vetter@ffwll.ch>
  Date:   Tue Jul 29 15:33:05 2014 +0200

      drm/plane-helper: transitional atomic plane helpers

- The universal plane code split out the implicit primary plane from
  the CRTC into it's own full-blown drm_plane object. As part of that
  the setcrtc ioctl (which updated both the crtc mode and primary
  plane) learned to set crtc->primary->crtc on modeset to make sure
  the plane->crtc assignments statate up to date in

  commit e13161af80c185ecd8dc4641d0f5df58f9e3e0af
  Author: Matt Roper <matthew.d.roper@intel.com>
  Date:   Tue Apr 1 15:22:38 2014 -0700

      drm: Add drm_crtc_init_with_planes() (v2)

  Unfortunately we've forgotten to update the load detect code. Which
  wasn't a problem since the load detect modeset is temporary and
  always undone before we drop the locks.

- Finally there is a organically grown history (i.e. don't ask) around
  who sets the legacy plane->fb for the various driver entry points.
  Originally updating that was the drivers duty, but for almost all
  places we've moved that (plus updating the refcounts) into the core.
  Again the exception is the load detect code.

Taking all together the following happens:
- The load detect code doesn't set crtc->primary->crtc. This is only
  really an issue on crtcs never before used or when userspace
  explicitly disabled the primary plane.

- The plane helper glue code short-circuits because of that and leaves
  a non-NULL fb behind in plane->state->fb and plane->fb. The state
  fb isn't a real problem (it's properly refcounted on its own), it's
  just the canary.

- Load detect code drops the reference for that fb, but doesn't set
  plane->fb = NULL. This is ok since it's still living in that old
  world where drivers had to clear the pointer but the core/callers
  handled the refcounting.

- On the next modeset the drm core notices plane->fb and takes care of
  refcounting it properly by doing another unref. This drops the
  refcount to zero, leaving state->plane now pointing at freed memory.

- intel_plane_duplicate_state still assume it owns a reference to that
  very state->fb and bad things start to happen.

Fix this all by applying the same duct-tape as for the legacy setcrtc
ioctl code and set crtc->primary->crtc properly.

Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Paul Bolle <pebolle@tiscali.nl>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Paulo Zanoni <przanoni@gmail.com>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: Matt Roper <matthew.d.roper@intel.com>
Reported-and-tested-by: Linus Torvalds <torvalds@linux-foundation.org>
Reported-by: Paul Bolle <pebolle@tiscali.nl>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
1 parent 023a600
History
Tip revision: 9128b040eb774e04bc23777b005ace2b66ab2a85 authored by Daniel Vetter on 03 March 2015, 16:31:21 UTC
drm/i915: Fix modeset state confusion in the load detect code
Tip revision: 9128b04
File Mode Size
fonts
lz4
lzo
mpi
raid6
reed_solomon
xz
zlib_deflate
zlib_inflate
.gitignore -rw-r--r-- 70 bytes
Kconfig -rw-r--r-- 11.9 KB
Kconfig.debug -rw-r--r-- 58.6 KB
Kconfig.kasan -rw-r--r-- 1.6 KB
Kconfig.kgdb -rw-r--r-- 4.1 KB
Kconfig.kmemcheck -rw-r--r-- 2.9 KB
Makefile -rw-r--r-- 6.2 KB
argv_split.c -rw-r--r-- 2.1 KB
asn1_decoder.c -rw-r--r-- 12.4 KB
assoc_array.c -rw-r--r-- 52.6 KB
atomic64.c -rw-r--r-- 4.1 KB
atomic64_test.c -rw-r--r-- 3.4 KB
audit.c -rw-r--r-- 1.7 KB
average.c -rw-r--r-- 1.9 KB
bcd.c -rw-r--r-- 261 bytes
bch.c -rw-r--r-- 35.6 KB
bitmap.c -rw-r--r-- 33.0 KB
bitrev.c -rw-r--r-- 1.9 KB
bsearch.c -rw-r--r-- 1.6 KB
btree.c -rw-r--r-- 19.3 KB
bug.c -rw-r--r-- 4.9 KB
build_OID_registry -rwxr-xr-x 4.7 KB
bust_spinlocks.c -rw-r--r-- 660 bytes
check_signature.c -rw-r--r-- 599 bytes
checksum.c -rw-r--r-- 5.0 KB
clz_ctz.c -rw-r--r-- 1.3 KB
clz_tab.c -rw-r--r-- 855 bytes
cmdline.c -rw-r--r-- 4.1 KB
compat_audit.c -rw-r--r-- 796 bytes
cordic.c -rw-r--r-- 2.5 KB
cpu-notifier-error-inject.c -rw-r--r-- 1.2 KB
cpu_rmap.c -rw-r--r-- 7.8 KB
cpumask.c -rw-r--r-- 5.8 KB
crc-ccitt.c -rw-r--r-- 3.0 KB
crc-itu-t.c -rw-r--r-- 2.8 KB
crc-t10dif.c -rw-r--r-- 1.5 KB
crc16.c -rw-r--r-- 2.8 KB
crc32.c -rw-r--r-- 45.4 KB
crc32defs.h -rw-r--r-- 2.0 KB
crc7.c -rw-r--r-- 2.6 KB
crc8.c -rw-r--r-- 2.4 KB
ctype.c -rw-r--r-- 1.4 KB
debug_locks.c -rw-r--r-- 1.2 KB
debugobjects.c -rw-r--r-- 26.1 KB
dec_and_lock.c -rw-r--r-- 784 bytes
decompress.c -rw-r--r-- 1.6 KB
decompress_bunzip2.c -rw-r--r-- 23.4 KB
decompress_inflate.c -rw-r--r-- 3.8 KB
decompress_unlz4.c -rw-r--r-- 4.1 KB
decompress_unlzma.c -rw-r--r-- 15.8 KB
decompress_unlzo.c -rw-r--r-- 6.8 KB
decompress_unxz.c -rw-r--r-- 10.6 KB
devres.c -rw-r--r-- 9.4 KB
digsig.c -rw-r--r-- 5.5 KB
div64.c -rw-r--r-- 4.0 KB
dma-debug.c -rw-r--r-- 41.9 KB
dump_stack.c -rw-r--r-- 1.1 KB
dynamic_debug.c -rw-r--r-- 25.1 KB
dynamic_queue_limits.c -rw-r--r-- 4.3 KB
earlycpio.c -rw-r--r-- 3.9 KB
extable.c -rw-r--r-- 2.4 KB
fault-inject.c -rw-r--r-- 6.0 KB
fdt.c -rw-r--r-- 69 bytes
fdt_empty_tree.c -rw-r--r-- 80 bytes
fdt_ro.c -rw-r--r-- 72 bytes
fdt_rw.c -rw-r--r-- 72 bytes
fdt_strerror.c -rw-r--r-- 78 bytes
fdt_sw.c -rw-r--r-- 72 bytes
fdt_wip.c -rw-r--r-- 73 bytes
find_last_bit.c -rw-r--r-- 1.1 KB
find_next_bit.c -rw-r--r-- 6.4 KB
flex_array.c -rw-r--r-- 11.0 KB
flex_proportions.c -rw-r--r-- 6.9 KB
gcd.c -rw-r--r-- 313 bytes
gen_crc32table.c -rw-r--r-- 3.2 KB
genalloc.c -rw-r--r-- 17.8 KB
glob.c -rw-r--r-- 7.7 KB
halfmd4.c -rw-r--r-- 2.0 KB
hexdump.c -rw-r--r-- 8.2 KB
hweight.c -rw-r--r-- 1.9 KB
idr.c -rw-r--r-- 28.0 KB
inflate.c -rw-r--r-- 38.6 KB
int_sqrt.c -rw-r--r-- 652 bytes
interval_tree.c -rw-r--r-- 499 bytes
interval_tree_test.c -rw-r--r-- 2.3 KB
iomap.c -rw-r--r-- 6.5 KB
iomap_copy.c -rw-r--r-- 2.1 KB
iommu-helper.c -rw-r--r-- 1.0 KB
ioremap.c -rw-r--r-- 2.1 KB
irq_regs.c -rw-r--r-- 604 bytes
is_single_threaded.c -rw-r--r-- 1.3 KB
jedec_ddr_data.c -rw-r--r-- 3.0 KB
kasprintf.c -rw-r--r-- 717 bytes
kfifo.c -rw-r--r-- 12.7 KB
klist.c -rw-r--r-- 9.3 KB
kobject.c -rw-r--r-- 25.5 KB
kobject_uevent.c -rw-r--r-- 11.3 KB
kstrtox.c -rw-r--r-- 9.4 KB
kstrtox.h -rw-r--r-- 254 bytes
lcm.c -rw-r--r-- 271 bytes
libcrc32c.c -rw-r--r-- 2.0 KB
list_debug.c -rw-r--r-- 2.6 KB
list_sort.c -rw-r--r-- 6.8 KB
llist.c -rw-r--r-- 3.1 KB
locking-selftest-hardirq.h -rw-r--r-- 207 bytes
locking-selftest-mutex.h -rw-r--r-- 120 bytes
locking-selftest-rlock-hardirq.h -rw-r--r-- 74 bytes
locking-selftest-rlock-softirq.h -rw-r--r-- 74 bytes
locking-selftest-rlock.h -rw-r--r-- 158 bytes
locking-selftest-rsem.h -rw-r--r-- 163 bytes
locking-selftest-softirq.h -rw-r--r-- 207 bytes
locking-selftest-spin-hardirq.h -rw-r--r-- 73 bytes
locking-selftest-spin-softirq.h -rw-r--r-- 73 bytes
locking-selftest-spin.h -rw-r--r-- 118 bytes
locking-selftest-wlock-hardirq.h -rw-r--r-- 74 bytes
locking-selftest-wlock-softirq.h -rw-r--r-- 74 bytes
locking-selftest-wlock.h -rw-r--r-- 158 bytes
locking-selftest-wsem.h -rw-r--r-- 163 bytes
locking-selftest.c -rw-r--r-- 40.1 KB
lockref.c -rw-r--r-- 4.1 KB
lru_cache.c -rw-r--r-- 19.4 KB
md5.c -rw-r--r-- 3.7 KB
memory-notifier-error-inject.c -rw-r--r-- 1.1 KB
memweight.c -rw-r--r-- 999 bytes
net_utils.c -rw-r--r-- 604 bytes
nlattr.c -rw-r--r-- 12.5 KB
notifier-error-inject.c -rw-r--r-- 2.7 KB
notifier-error-inject.h -rw-r--r-- 614 bytes
of-reconfig-notifier-error-inject.c -rw-r--r-- 1.3 KB
oid_registry.c -rw-r--r-- 3.8 KB
parser.c -rw-r--r-- 7.1 KB
pci_iomap.c -rw-r--r-- 2.2 KB
percpu-refcount.c -rw-r--r-- 11.7 KB
percpu_counter.c -rw-r--r-- 5.5 KB
percpu_ida.c -rw-r--r-- 9.5 KB
percpu_test.c -rw-r--r-- 3.2 KB
plist.c -rw-r--r-- 5.9 KB
pm-notifier-error-inject.c -rw-r--r-- 1.1 KB
proportions.c -rw-r--r-- 9.4 KB
radix-tree.c -rw-r--r-- 39.3 KB
random32.c -rw-r--r-- 12.6 KB
ratelimit.c -rw-r--r-- 1.5 KB
rational.c -rw-r--r-- 1.5 KB
rbtree.c -rw-r--r-- 15.0 KB
rbtree_test.c -rw-r--r-- 5.5 KB
reciprocal_div.c -rw-r--r-- 492 bytes
rhashtable.c -rw-r--r-- 30.5 KB
scatterlist.c -rw-r--r-- 18.2 KB
seq_buf.c -rw-r--r-- 7.8 KB
sha1.c -rw-r--r-- 6.1 KB
show_mem.c -rw-r--r-- 1.3 KB
smp_processor_id.c -rw-r--r-- 1.3 KB
sort.c -rw-r--r-- 2.5 KB
stmp_device.c -rw-r--r-- 2.1 KB
string.c -rw-r--r-- 17.0 KB
string_helpers.c -rw-r--r-- 9.9 KB
strncpy_from_user.c -rw-r--r-- 2.9 KB
strnlen_user.c -rw-r--r-- 3.6 KB
swiotlb.c -rw-r--r-- 27.1 KB
syscall.c -rw-r--r-- 2.4 KB
test-hexdump.c -rw-r--r-- 4.0 KB
test-kstrtox.c -rw-r--r-- 17.4 KB
test-string_helpers.c -rw-r--r-- 8.0 KB
test_bpf.c -rw-r--r-- 56.9 KB
test_firmware.c -rw-r--r-- 2.8 KB
test_kasan.c -rw-r--r-- 5.1 KB
test_module.c -rw-r--r-- 753 bytes
test_rhashtable.c -rw-r--r-- 5.0 KB
test_user_copy.c -rw-r--r-- 3.1 KB
textsearch.c -rw-r--r-- 9.4 KB
timerqueue.c -rw-r--r-- 3.1 KB
ts_bm.c -rw-r--r-- 5.3 KB
ts_fsm.c -rw-r--r-- 10.6 KB
ts_kmp.c -rw-r--r-- 4.3 KB
ucs2_string.c -rw-r--r-- 1.2 KB
usercopy.c -rw-r--r-- 197 bytes
uuid.c -rw-r--r-- 1.3 KB
vsprintf.c -rw-r--r-- 62.2 KB

back to top