dri-devel March 2014

dri-devel@lists.freedesktop.org

144 participants
926 discussions

[PATCH 0/5] Convert last few uses of __FUNCTION__ to __func__
by Joe Perches 25 Mar '14

25 Mar '14

Outside of staging, there aren't any more uses of __FUNCTION__ now... Joe Perches (5): powerpc: Convert last uses of __FUNCTION__ to __func__ x86: Convert last uses of __FUNCTION__ to __func__ block: Convert last uses of __FUNCTION__ to __func__ i915: Convert last uses of __FUNCTION__ to __func__ slab: Convert last uses of __FUNCTION__ to __func__ arch/powerpc/platforms/pseries/nvram.c | 11 +++++------ arch/x86/kernel/hpet.c | 2 +- arch/x86/kernel/rtc.c | 4 ++-- arch/x86/platform/intel-mid/intel_mid_vrtc.c | 2 +- drivers/block/drbd/drbd_int.h | 8 ++++---- drivers/block/xen-blkfront.c | 4 ++-- drivers/gpu/drm/i915/dvo_ns2501.c | 15 ++++++--------- mm/slab.h | 2 +- 8 files changed, 22 insertions(+), 26 deletions(-) -- 1.8.1.2.459.gbcd45b4.dirty

2 2

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Don't set mode_config's cursor size
by Daniel Vetter 25 Mar '14

25 Mar '14

On Tue, Mar 25, 2014 at 04:57:04PM +0000, Damien Lespiau wrote: > On Tue, Mar 25, 2014 at 04:38:24PM +0000, Chris Wilson wrote: > > For the record, > > > > 16:30 < agd5f> ickle, our GPUs don't have selectable cursor sizes > > 16:31 < agd5f> so on the newer ones, xf86-video-modesetting, etc. would > > allocate a 64x64 cursor and it would look squashed and funky since the > > hw expects 128x128 > > > > Which means I was confused when I thought part of the reasoning was > > indeed HiDPI support. (I'm still seem to remember that was part of the > > argument for large cursors anyway.) > > > > > Are you saying the Intel DDX currently derives a different meaning to > > > the intented behaviour? in which case it can still be changed to not do > > > that? > > > > I still disagree though. This provides all the information I need to > > support variable sized cursors and we can use large cursors today. > > I'd love the game to be about defining clear semantics more than "by > interpreting that value this way, I got what I always wanted" :) > > We can resolve that today with MAX_CURSOR_WIDTH, MAX_CURSOR_HEIGHT caps > as well (if you're alluding at the fact that drm_planes may still be a > few decades away). > > We'll still need to expose the full list of supported cursor sizes for > compositors at some point or another, my preferred way would be with a > property in the exposed cursor drm_plane. For the record I'll restate my comment here about the larger problem: Atm we have no way for userspace to figure out per-plane limits on sizes/strides, we only expose a lists of supported pixel formats. Imo exposing cursor limits is just part of this larger issue, so if we can get the current stuff working with some legalese, then I'm ok with that. Solving the larger issue is much more work, and it's better to have a few more odd cases working than not. For planes I guess we could have a few limits: min/max width height in pixels min/max stride alignment of stride And a pile of flags - needs pot stride/width/height - needs square size That should be enough to cover most single-plane things. For multiplanar I guess we might either just need 2nd/3rd set of those or some additional stuff expressed in flags (e.g. subsampled strides must be half of the Y stride). Or we'll throw our hands in the air for multi-planar for now ;-) Or we simply do this per-pixel format with one for each framebuffer plane, i.e. struct drm_get_plane_fb_limits { uint32_t plane_id; /* in */ uint32_t fourcc; /* in */ struct drm_plane_limits limits[MAX_FOURCC_PLANES]; /* the stuff above for all possible planes of a fourcc code */ } Saner drivers could then return the same thing for all fourccs codes in their backend. Just something to ponder. Adding dri-devel, maybe we can get this discussion started. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch

2 3

[PATCH] drm/ttm: remove fence_lock
by Maarten Lankhorst 25 Mar '14

25 Mar '14

With the nouveau calls fixed there were only 2 places left that used fence_lock without a reservation, those are fixed in this patch: ttm_bo_cleanup_refs_or_queue is fixed by simply doing things the other way around. ttm_bo_cleanup_refs is fixed by taking a reservation first, then a pointer to the fence object and backs off from the reservation if waiting has to be performed. Signed-off-by: Maarten Lankhorst <maarten.lankhorst(a)canonical.com> --- This patch should be applied only after all the previous patches I sent are applied, since it depends on sync_obj_arg removal (kinda, probably fails to apply otherwise), uses ttm_bo_is_reserved, and depends on nouveau wait behavior being fixed. drivers/gpu/drm/nouveau/nouveau_bo.c | 4 - drivers/gpu/drm/nouveau/nouveau_gem.c | 15 +--- drivers/gpu/drm/radeon/radeon_object.c | 2 - drivers/gpu/drm/ttm/ttm_bo.c | 124 ++++++++++++-------------------- drivers/gpu/drm/ttm/ttm_bo_util.c | 3 - drivers/gpu/drm/ttm/ttm_bo_vm.c | 5 - drivers/gpu/drm/ttm/ttm_execbuf_util.c | 2 - drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 3 - include/drm/ttm/ttm_bo_api.h | 12 +-- include/drm/ttm/ttm_bo_driver.h | 3 - 10 files changed, 52 insertions(+), 121 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index cee7448..9749c45 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -1449,10 +1449,8 @@ nouveau_bo_fence(struct nouveau_bo *nvbo, struct nouveau_fence *fence) if (likely(fence)) nouveau_fence_ref(fence); - spin_lock(&nvbo->bo.bdev->fence_lock); old_fence = nvbo->bo.sync_obj; nvbo->bo.sync_obj = fence; - spin_unlock(&nvbo->bo.bdev->fence_lock); nouveau_fence_unref(&old_fence); } @@ -1552,9 +1550,7 @@ nouveau_bo_vma_del(struct nouveau_bo *nvbo, struct nouveau_vma *vma) if (vma->node) { if (nvbo->bo.mem.mem_type != TTM_PL_SYSTEM) { ttm_bo_reserve(&nvbo->bo, false, false, false, 0); - spin_lock(&nvbo->bo.bdev->fence_lock); ttm_bo_wait(&nvbo->bo, false, false, false); - spin_unlock(&nvbo->bo.bdev->fence_lock); ttm_bo_unreserve(&nvbo->bo); nouveau_vm_unmap(vma); } diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c index 6d8391d..eaa700a 100644 --- a/drivers/gpu/drm/nouveau/nouveau_gem.c +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c @@ -391,18 +391,11 @@ retry: static int validate_sync(struct nouveau_channel *chan, struct nouveau_bo *nvbo) { - struct nouveau_fence *fence = NULL; + struct nouveau_fence *fence = nvbo->bo.sync_obj; int ret = 0; - spin_lock(&nvbo->bo.bdev->fence_lock); - if (nvbo->bo.sync_obj) - fence = nouveau_fence_ref(nvbo->bo.sync_obj); - spin_unlock(&nvbo->bo.bdev->fence_lock); - - if (fence) { + if (fence) ret = nouveau_fence_sync(fence, chan); - nouveau_fence_unref(&fence); - } return ret; } @@ -614,9 +607,7 @@ nouveau_gem_pushbuf_reloc_apply(struct drm_device *dev, data |= r->vor; } - spin_lock(&nvbo->bo.bdev->fence_lock); ret = ttm_bo_wait(&nvbo->bo, false, false, false); - spin_unlock(&nvbo->bo.bdev->fence_lock); if (ret) { NV_ERROR(drm, "reloc wait_idle failed: %d\n", ret); break; @@ -848,11 +839,9 @@ nouveau_gem_ioctl_cpu_prep(struct drm_device *dev, void *data, ret = ttm_bo_reserve(&nvbo->bo, true, false, false, 0); if (!ret) { - spin_lock(&nvbo->bo.bdev->fence_lock); ret = ttm_bo_wait(&nvbo->bo, true, true, true); if (!no_wait && ret) fence = nouveau_fence_ref(nvbo->bo.sync_obj); - spin_unlock(&nvbo->bo.bdev->fence_lock); ttm_bo_unreserve(&nvbo->bo); } diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 808c444..df430e4 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -613,12 +613,10 @@ int radeon_bo_wait(struct radeon_bo *bo, u32 *mem_type, bool no_wait) r = ttm_bo_reserve(&bo->tbo, true, no_wait, false, 0); if (unlikely(r != 0)) return r; - spin_lock(&bo->tbo.bdev->fence_lock); if (mem_type) *mem_type = bo->tbo.mem.mem_type; if (bo->tbo.sync_obj) r = ttm_bo_wait(&bo->tbo, true, true, no_wait); - spin_unlock(&bo->tbo.bdev->fence_lock); ttm_bo_unreserve(&bo->tbo); return r; } diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index f6d7026..69fc29b 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -498,28 +498,23 @@ static void ttm_bo_cleanup_refs_or_queue(struct ttm_buffer_object *bo) { struct ttm_bo_device *bdev = bo->bdev; struct ttm_bo_global *glob = bo->glob; - struct ttm_bo_driver *driver; + struct ttm_bo_driver *driver = bdev->driver; void *sync_obj = NULL; int put_count; int ret; - spin_lock(&bdev->fence_lock); - (void) ttm_bo_wait(bo, false, false, true); - if (!bo->sync_obj) { - - spin_lock(&glob->lru_lock); - - /** - * Lock inversion between bo:reserve and bdev::fence_lock here, - * but that's OK, since we're only trylocking. - */ - - ret = ttm_bo_reserve_locked(bo, false, true, false, 0); + spin_lock(&glob->lru_lock); + ret = ttm_bo_reserve_locked(bo, false, true, false, 0); + if (!ret) { + ret = ttm_bo_wait(bo, false, false, true); - if (unlikely(ret == -EBUSY)) + if (unlikely(ret == -EBUSY)) { + sync_obj = driver->sync_obj_ref(bo->sync_obj); + atomic_set(&bo->reserved, 0); + wake_up_all(&bo->event_queue); goto queue; + } - spin_unlock(&bdev->fence_lock); put_count = ttm_bo_del_from_lru(bo); spin_unlock(&glob->lru_lock); @@ -528,18 +523,11 @@ static void ttm_bo_cleanup_refs_or_queue(struct ttm_buffer_object *bo) ttm_bo_list_ref_sub(bo, put_count, true); return; - } else { - spin_lock(&glob->lru_lock); } queue: - driver = bdev->driver; - if (bo->sync_obj) - sync_obj = driver->sync_obj_ref(bo->sync_obj); - kref_get(&bo->list_kref); list_add_tail(&bo->ddestroy, &bdev->ddestroy); spin_unlock(&glob->lru_lock); - spin_unlock(&bdev->fence_lock); if (sync_obj) { driver->sync_obj_flush(sync_obj); @@ -565,25 +553,15 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo, bool no_wait_gpu) { struct ttm_bo_device *bdev = bo->bdev; + struct ttm_bo_driver *driver = bdev->driver; struct ttm_bo_global *glob = bo->glob; int put_count; int ret = 0; + void *sync_obj; retry: - spin_lock(&bdev->fence_lock); - ret = ttm_bo_wait(bo, false, interruptible, no_wait_gpu); - spin_unlock(&bdev->fence_lock); - - if (unlikely(ret != 0)) - return ret; - spin_lock(&glob->lru_lock); - if (unlikely(list_empty(&bo->ddestroy))) { - spin_unlock(&glob->lru_lock); - return 0; - } - ret = ttm_bo_reserve_locked(bo, interruptible, no_wait_reserve, false, 0); @@ -592,18 +570,37 @@ retry: return ret; } - /** - * We can re-check for sync object without taking - * the bo::lock since setting the sync object requires - * also bo::reserved. A busy object at this point may - * be caused by another thread recently starting an accelerated - * eviction. - */ + if (unlikely(list_empty(&bo->ddestroy))) { + atomic_set(&bo->reserved, 0); + wake_up_all(&bo->event_queue); + spin_unlock(&glob->lru_lock); + return 0; + } + ret = ttm_bo_wait(bo, false, false, true); + + if (ret) { + if (no_wait_gpu) { + atomic_set(&bo->reserved, 0); + wake_up_all(&bo->event_queue); + spin_unlock(&glob->lru_lock); + return ret; + } - if (unlikely(bo->sync_obj)) { + /** + * Take a reference to the fence and unreserve, if the wait + * was succesful and no new sync_obj was attached, + * ttm_bo_wait in retry will return ret = 0, and end the loop. + */ + + sync_obj = driver->sync_obj_ref(&bo->sync_obj); atomic_set(&bo->reserved, 0); wake_up_all(&bo->event_queue); spin_unlock(&glob->lru_lock); + + ret = driver->sync_obj_wait(bo->sync_obj, false, interruptible); + driver->sync_obj_unref(&sync_obj); + if (ret) + return ret; goto retry; } @@ -735,9 +732,7 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, bool interruptible, struct ttm_placement placement; int ret = 0; - spin_lock(&bdev->fence_lock); ret = ttm_bo_wait(bo, false, interruptible, no_wait_gpu); - spin_unlock(&bdev->fence_lock); if (unlikely(ret != 0)) { if (ret != -ERESTARTSYS) { @@ -1054,7 +1049,6 @@ int ttm_bo_move_buffer(struct ttm_buffer_object *bo, { int ret = 0; struct ttm_mem_reg mem; - struct ttm_bo_device *bdev = bo->bdev; BUG_ON(!ttm_bo_is_reserved(bo)); @@ -1063,9 +1057,7 @@ int ttm_bo_move_buffer(struct ttm_buffer_object *bo, * Have the driver move function wait for idle when necessary, * instead of doing it here. */ - spin_lock(&bdev->fence_lock); ret = ttm_bo_wait(bo, false, interruptible, no_wait_gpu); - spin_unlock(&bdev->fence_lock); if (ret) return ret; mem.num_pages = bo->num_pages; @@ -1554,7 +1546,6 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev, bdev->glob = glob; bdev->need_dma32 = need_dma32; bdev->val_seq = 0; - spin_lock_init(&bdev->fence_lock); mutex_lock(&glob->device_list_mutex); list_add_tail(&bdev->device_list, &glob->device_list); mutex_unlock(&glob->device_list_mutex); @@ -1690,52 +1681,33 @@ int ttm_bo_wait(struct ttm_buffer_object *bo, bool lazy, bool interruptible, bool no_wait) { struct ttm_bo_driver *driver = bo->bdev->driver; - struct ttm_bo_device *bdev = bo->bdev; - void *sync_obj; int ret = 0; + WARN_ON_ONCE(!ttm_bo_is_reserved(bo)); + if (likely(bo->sync_obj == NULL)) return 0; - while (bo->sync_obj) { - + if (bo->sync_obj) { if (driver->sync_obj_signaled(bo->sync_obj)) { void *tmp_obj = bo->sync_obj; bo->sync_obj = NULL; clear_bit(TTM_BO_PRIV_FLAG_MOVING, &bo->priv_flags); - spin_unlock(&bdev->fence_lock); driver->sync_obj_unref(&tmp_obj); - spin_lock(&bdev->fence_lock); - continue; + return 0; } if (no_wait) return -EBUSY; - sync_obj = driver->sync_obj_ref(bo->sync_obj); - spin_unlock(&bdev->fence_lock); - ret = driver->sync_obj_wait(sync_obj, + ret = driver->sync_obj_wait(bo->sync_obj, lazy, interruptible); if (unlikely(ret != 0)) { - driver->sync_obj_unref(&sync_obj); - spin_lock(&bdev->fence_lock); return ret; } - spin_lock(&bdev->fence_lock); - if (likely(bo->sync_obj == sync_obj)) { - void *tmp_obj = bo->sync_obj; - bo->sync_obj = NULL; - clear_bit(TTM_BO_PRIV_FLAG_MOVING, - &bo->priv_flags); - spin_unlock(&bdev->fence_lock); - driver->sync_obj_unref(&sync_obj); - driver->sync_obj_unref(&tmp_obj); - spin_lock(&bdev->fence_lock); - } else { - spin_unlock(&bdev->fence_lock); - driver->sync_obj_unref(&sync_obj); - spin_lock(&bdev->fence_lock); - } + driver->sync_obj_unref(&bo->sync_obj); + clear_bit(TTM_BO_PRIV_FLAG_MOVING, + &bo->priv_flags); } return 0; } @@ -1799,9 +1771,7 @@ static int ttm_bo_swapout(struct ttm_mem_shrink *shrink) * Wait for GPU, then move to system cached. */ - spin_lock(&bo->bdev->fence_lock); ret = ttm_bo_wait(bo, false, false, false); - spin_unlock(&bo->bdev->fence_lock); if (unlikely(ret != 0)) goto out; diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index d80d1e8..84d6e01 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -622,7 +622,6 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo, struct ttm_buffer_object *ghost_obj; void *tmp_obj = NULL; - spin_lock(&bdev->fence_lock); if (bo->sync_obj) { tmp_obj = bo->sync_obj; bo->sync_obj = NULL; @@ -630,7 +629,6 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo, bo->sync_obj = driver->sync_obj_ref(sync_obj); if (evict) { ret = ttm_bo_wait(bo, false, false, false); - spin_unlock(&bdev->fence_lock); if (tmp_obj) driver->sync_obj_unref(&tmp_obj); if (ret) @@ -653,7 +651,6 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo, */ set_bit(TTM_BO_PRIV_FLAG_MOVING, &bo->priv_flags); - spin_unlock(&bdev->fence_lock); if (tmp_obj) driver->sync_obj_unref(&tmp_obj); diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index a877813..55718c2 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -122,17 +122,14 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf) * move. */ - spin_lock(&bdev->fence_lock); if (test_bit(TTM_BO_PRIV_FLAG_MOVING, &bo->priv_flags)) { ret = ttm_bo_wait(bo, false, true, false); - spin_unlock(&bdev->fence_lock); if (unlikely(ret != 0)) { retval = (ret != -ERESTARTSYS) ? VM_FAULT_SIGBUS : VM_FAULT_NOPAGE; goto out_unlock; } - } else - spin_unlock(&bdev->fence_lock); + } ret = ttm_mem_io_lock(man, true); if (unlikely(ret != 0)) { diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c index 721886e..51b5e97 100644 --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c @@ -207,7 +207,6 @@ void ttm_eu_fence_buffer_objects(struct list_head *list, void *sync_obj) driver = bdev->driver; glob = bo->glob; - spin_lock(&bdev->fence_lock); spin_lock(&glob->lru_lock); list_for_each_entry(entry, list, head) { @@ -218,7 +217,6 @@ void ttm_eu_fence_buffer_objects(struct list_head *list, void *sync_obj) entry->reserved = false; } spin_unlock(&glob->lru_lock); - spin_unlock(&bdev->fence_lock); list_for_each_entry(entry, list, head) { if (entry->old_sync_obj) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index ed3c1e7..e038c9a 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -251,13 +251,10 @@ static void vmw_dummy_query_bo_prepare(struct vmw_private *dev_priv) volatile SVGA3dQueryResult *result; bool dummy; int ret; - struct ttm_bo_device *bdev = &dev_priv->bdev; struct ttm_buffer_object *bo = dev_priv->dummy_query_bo; ttm_bo_reserve(bo, false, false, false, 0); - spin_lock(&bdev->fence_lock); ret = ttm_bo_wait(bo, false, false, false); - spin_unlock(&bdev->fence_lock); if (unlikely(ret != 0)) (void) vmw_fallback_wait(dev_priv, false, true, 0, false, 10*HZ); diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 816d9b1..6c69224 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -222,6 +222,8 @@ struct ttm_buffer_object { struct file *persistent_swap_storage; struct ttm_tt *ttm; bool evicted; + void *sync_obj; + unsigned long priv_flags; /** * Members protected by the bdev::lru_lock. @@ -242,16 +244,6 @@ struct ttm_buffer_object { atomic_t reserved; /** - * Members protected by struct buffer_object_device::fence_lock - * In addition, setting sync_obj to anything else - * than NULL requires bo::reserved to be held. This allows for - * checking NULL while reserved but not holding the mentioned lock. - */ - - void *sync_obj; - unsigned long priv_flags; - - /** * Members protected by the bdev::vm_lock */ diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index 0c8c3b5..aac101b 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -515,8 +515,6 @@ struct ttm_bo_global { * * @driver: Pointer to a struct ttm_bo_driver struct setup by the driver. * @man: An array of mem_type_managers. - * @fence_lock: Protects the synchronizing members on *all* bos belonging - * to this device. * @addr_space_mm: Range manager for the device address space. * lru_lock: Spinlock that protects the buffer+device lru lists and * ddestroy lists. @@ -539,7 +537,6 @@ struct ttm_bo_device { struct ttm_bo_driver *driver; rwlock_t vm_lock; struct ttm_mem_type_manager man[TTM_NUM_MEM_TYPES]; - spinlock_t fence_lock; /* * Protected by the vm lock. */

4 14

[DRM] drm_get_connector_name internal static string buffer causes a race
by Dmitry Malkin 25 Mar '14

25 Mar '14

Hello guys, I've been playing with reloading intel gfx driver (i915) in a cycle, for a while, and at some point I've found a non-deterministic kernel crash with a highly-variable iteration dependency -- 2 to 200 driver reload iterations. The apparent race is over the shared internal string buffer in drm_get_connector_name(). It is mostly harmless, due to its results being mostly used for log output, but in at least one case -- drm_sysfs_connector_add() -- this leads to a more critical error. Race scenario: - drm_sysfs_connector_add() - drm_get_connector_name() vs. - anything that generates log messages involving DRM connectors - drm_get_connector_name() (and many other from drm_crtc.c) shares with caller const char* to internal static char buffer. If something call it from other thread, while main thread strore and use returned pointer it may overwrite connector name. Here are we go: registering HDMI connecter (drm_sysfs_connector_add store and use pointer from drm_get_connector_name) and the same time got VGA connector name down through the stack. (the second thread is upowerd who watch continuously sysfs) Mar 24 14:23:04 haswell01 kernel: [ 417.570043] ------------[ cut here ]------------ Mar 24 14:23:04 haswell01 kernel: [ 417.570045] WARNING: CPU: 0 PID: 12700 at /build/buildd/linux-3.13.0/lib/kobject.c:223 kobject_add_internal+0x224/0x330() Mar 24 14:23:04 haswell01 kernel: [ 417.570046] kobject_add_internal failed for card0-VGA-1 with -EEXIST, don't try to register things with the same name in the same directory. Mar 24 14:23:04 haswell01 kernel: [ 417.570047] Modules linked in: i915(+) video drm_kms_helper drm i2c_algo_bit snd_hda_codec_realtek snd_hda_codec_hdmi bnep rfcomm bluetooth x86_pkg_temp_thermal intel_p owerclamp coretemp kvm_intel snd_hda_codec kvm snd_hwdep snd_pcm hid_generic snd_page_alloc crct10dif_pclmul snd_seq_midi crc32_pclmul ghash_clmulni_intel snd_seq_midi_event usbhid snd_rawmidi hid aesni_in tel aes_x86_64 lrw gf128mul ppdev glue_helper ablk_helper cryptd snd_seq snd_seq_device snd_timer snd mei_me psmouse lpc_ich soundcore mei mac_hid parport_pc serio_raw nls_iso8859_1 lp parport e1000e ahci ptp libahci pps_core [last unloaded: video] Mar 24 14:23:04 haswell01 kernel: [ 417.570068] CPU: 0 PID: 12700 Comm: modprobe Tainted: G W 3.13.0-19-generic #39-Ubuntu Mar 24 14:23:04 haswell01 kernel: [ 417.570069] Hardware name: /DQ87PG, BIOS PGQ8710H.86A.0144.2014.0113.1604 01/13/2014 Mar 24 14:23:04 haswell01 kernel: [ 417.570069] 0000000000000009 ffff8804051295f8 ffffffff81711075 ffff880405129640 Mar 24 14:23:04 haswell01 kernel: [ 417.570071] ffff880405129630 ffffffff810662cd ffff88040776a410 00000000ffffffef Mar 24 14:23:04 haswell01 kernel: [ 417.570074] 0000000000000000 ffff8804048dcc10 ffff880407769000 ffff880405129690 Mar 24 14:23:04 haswell01 kernel: [ 417.570076] Call Trace: Mar 24 14:23:04 haswell01 kernel: [ 417.570078] [<ffffffff81711075>] dump_stack+0x45/0x56 Mar 24 14:23:04 haswell01 kernel: [ 417.570080] [<ffffffff810662cd>] warn_slowpath_common+0x7d/0xa0 Mar 24 14:23:04 haswell01 kernel: [ 417.570081] [<ffffffff8106633c>] warn_slowpath_fmt+0x4c/0x50 Mar 24 14:23:04 haswell01 kernel: [ 417.570082] [<ffffffff81230083>] ? sysfs_create_dir_ns+0x73/0xc0 Mar 24 14:23:04 haswell01 kernel: [ 417.570084] [<ffffffff8135b9a4>] kobject_add_internal+0x224/0x330 Mar 24 14:23:04 haswell01 kernel: [ 417.570086] [<ffffffff8135bed5>] kobject_add+0x65/0xb0 Mar 24 14:23:04 haswell01 kernel: [ 417.570088] [<ffffffff814874f5>] device_add+0x125/0x640 Mar 24 14:23:04 haswell01 kernel: [ 417.570090] [<ffffffff81487c20>] device_create_groups_vargs+0xe0/0x110 Mar 24 14:23:04 haswell01 kernel: [ 417.570092] [<ffffffff81487cb1>] device_create+0x41/0x50 Mar 24 14:23:04 haswell01 kernel: [ 417.570097] [<ffffffffa0137fc9>] drm_sysfs_connector_add+0x69/0x230 [drm] Mar 24 14:23:04 haswell01 kernel: [ 417.570110] [<ffffffffa0549ca1>] intel_hdmi_init_connector+0x111/0x260 [i915] Mar 24 14:23:04 haswell01 kernel: [ 417.570119] [<ffffffffa0541670>] intel_ddi_init+0x270/0x2a0 [i915] Mar 24 14:23:04 haswell01 kernel: [ 417.570130] [<ffffffffa0533176>] intel_setup_outputs+0x4c6/0x750 [i915] Mar 24 14:23:04 haswell01 kernel: [ 417.570139] [<ffffffffa05370f7>] intel_modeset_init+0x607/0x8f0 [i915] Mar 24 14:23:04 haswell01 kernel: [ 417.570147] [<ffffffffa04f90a4>] i915_driver_load+0xbb4/0xe70 [i915] Mar 24 14:23:04 haswell01 kernel: [ 417.570153] [<ffffffffa0134cd2>] drm_dev_register+0xa2/0x1e0 [drm] Mar 24 14:23:04 haswell01 kernel: [ 417.570158] [<ffffffffa0136bc2>] drm_get_pci_dev+0x92/0x140 [drm] Mar 24 14:23:04 haswell01 kernel: [ 417.570166] [<ffffffffa04f567c>] i915_pci_probe+0x3c/0x90 [i915] Mar 24 14:23:04 haswell01 kernel: [ 417.570168] [<ffffffff8139e0e5>] local_pci_probe+0x45/0xa0 Mar 24 14:23:04 haswell01 kernel: [ 417.570170] [<ffffffff8139f385>] ? pci_match_device+0xc5/0xd0 Mar 24 14:23:04 haswell01 kernel: [ 417.570172] [<ffffffff8139f4a9>] pci_device_probe+0xd9/0x130 Mar 24 14:23:04 haswell01 kernel: [ 417.570174] [<ffffffff8148a7c5>] driver_probe_device+0x125/0x3b0 Mar 24 14:23:04 haswell01 kernel: [ 417.570176] [<ffffffff8148ab23>] __driver_attach+0x93/0xa0 Mar 24 14:23:04 haswell01 kernel: [ 417.570178] [<ffffffff8148aa90>] ? __device_attach+0x40/0x40 Mar 24 14:23:04 haswell01 kernel: [ 417.570179] [<ffffffff81488733>] bus_for_each_dev+0x63/0xa0 Mar 24 14:23:04 haswell01 kernel: [ 417.570181] [<ffffffff8148a17e>] driver_attach+0x1e/0x20 Mar 24 14:23:04 haswell01 kernel: [ 417.570183] [<ffffffff81489d60>] bus_add_driver+0x180/0x250 Mar 24 14:23:04 haswell01 kernel: [ 417.570185] [<ffffffffa01be000>] ? 0xffffffffa01bdfff Mar 24 14:23:04 haswell01 kernel: [ 417.570187] [<ffffffff8148b1a4>] driver_register+0x64/0xf0 Mar 24 14:23:04 haswell01 kernel: [ 417.570189] [<ffffffffa01be000>] ? 0xffffffffa01bdfff Mar 24 14:23:04 haswell01 kernel: [ 417.570191] [<ffffffff8139da7c>] __pci_register_driver+0x4c/0x50 Mar 24 14:23:04 haswell01 kernel: [ 417.570196] [<ffffffffa0136d8a>] drm_pci_init+0x11a/0x130 [drm] Mar 24 14:23:04 haswell01 kernel: [ 417.570198] [<ffffffffa01be000>] ? 0xffffffffa01bdfff Mar 24 14:23:04 haswell01 kernel: [ 417.570205] [<ffffffffa01be066>] i915_init+0x66/0x68 [i915] Mar 24 14:23:04 haswell01 kernel: [ 417.570207] [<ffffffff8100214a>] do_one_initcall+0xfa/0x1b0 Mar 24 14:23:04 haswell01 kernel: [ 417.570208] [<ffffffff81058ae3>] ? set_memory_nx+0x43/0x50 Mar 24 14:23:04 haswell01 kernel: [ 417.570211] [<ffffffff810e091d>] load_module+0x12dd/0x1b40 Mar 24 14:23:04 haswell01 kernel: [ 417.570213] [<ffffffff810dc3a0>] ? store_uevent+0x40/0x40 Mar 24 14:23:04 haswell01 kernel: [ 417.570215] [<ffffffff810e12f6>] SyS_finit_module+0x86/0xb0 Mar 24 14:23:04 haswell01 kernel: [ 417.570217] [<ffffffff81721c7f>] tracesys+0xe1/0xe6 Mar 24 14:23:04 haswell01 kernel: [ 417.570219] ---[ end trace 8cd466c13137554f ]--- How to reproduce: load/reload i915 driver many times (in my case it happens after 2-200 attempts) and you will got a sysfs dup warning and then while unloading driver it will crash (because of malformed connectors list): Mar 24 14:23:16 haswell01 kernel: [ 429.326174] BUG: unable to handle kernel NULL pointer dereference at 000000000000002f Mar 24 14:23:16 haswell01 kernel: [ 429.326177] IP: [<ffffffff8122de46>] sysfs_remove_file_ns+0x6/0x20 Mar 24 14:23:16 haswell01 kernel: [ 429.326184] PGD 3f588d067 PUD 406361067 PMD 0 Mar 24 14:23:16 haswell01 kernel: [ 429.326187] Oops: 0000 [#1] SMP Mar 24 14:23:16 haswell01 kernel: [ 429.326189] Modules linked in: i915 video drm_kms_helper drm i2c_algo_bit snd_hda_codec_realtek snd_hda_codec_hdmi bnep rfcomm bluetooth x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel snd_hda_codec kvm snd_hwdep snd_pcm hid_generic snd_page_alloc crct10dif_pclmul snd_seq_midi crc32_pclmul ghash_clmulni_intel snd_seq_midi_event usbhid snd_rawmidi hid aesni_intel aes_x86_64 lrw gf128mul ppdev glue_helper ablk_helper cryptd snd_seq snd_seq_device snd_timer snd mei_me psmouse lpc_ich soundcore mei mac_hid parport_pc serio_raw nls_iso8859_1 lp parport e1000e ahci ptp libahci pps_core [last unloaded: video] Mar 24 14:23:16 haswell01 kernel: [ 429.326219] CPU: 0 PID: 13302 Comm: dd Tainted: G W 3.13.0-19-generic #39-Ubuntu Mar 24 14:23:16 haswell01 kernel: [ 429.326221] Hardware name: /DQ87PG, BIOS PGQ8710H.86A.0144.2014.0113.1604 01/13/2014 Mar 24 14:23:16 haswell01 kernel: [ 429.326222] task: ffff8803f5aaafe0 ti: ffff880404d20000 task.ti: ffff880404d20000 Mar 24 14:23:16 haswell01 kernel: [ 429.326224] RIP: 0010:[<ffffffff8122de46>] [<ffffffff8122de46>] sysfs_remove_file_ns+0x6/0x20 Mar 24 14:23:16 haswell01 kernel: [ 429.326227] RSP: 0018:ffff880404d21d20 EFLAGS: 00010246 Mar 24 14:23:16 haswell01 kernel: [ 429.326228] RAX: 0000000000000000 RBX: ffffffffa0160160 RCX: ffffffffa0159356 Mar 24 14:23:16 haswell01 kernel: [ 429.326230] RDX: 0000000000000000 RSI: ffffffffa0160140 RDI: ffffffffffffffff Mar 24 14:23:16 haswell01 kernel: [ 429.326231] RBP: ffff880404d21d30 R08: ffffffffa0161120 R09: 000000000000fffe Mar 24 14:23:16 haswell01 kernel: [ 429.326232] R10: 0000000000000000 R11: ffffea0010199f80 R12: ffff880407769000 Mar 24 14:23:16 haswell01 kernel: [ 429.326234] R13: ffff880035d46af8 R14: ffff880035d46820 R15: ffffffffffffffed Mar 24 14:23:16 haswell01 kernel: [ 429.326236] FS: 00007f472f4ec740(0000) GS:ffff88041ea00000(0000) knlGS:0000000000000000 Mar 24 14:23:16 haswell01 kernel: [ 429.326238] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 Mar 24 14:23:16 haswell01 kernel: [ 429.326239] CR2: 000000000000002f CR3: 00000004050d3000 CR4: 00000000001407f0 Mar 24 14:23:16 haswell01 kernel: [ 429.326241] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 Mar 24 14:23:16 haswell01 kernel: [ 429.326242] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Mar 24 14:23:16 haswell01 kernel: [ 429.326243] Stack: Mar 24 14:23:16 haswell01 kernel: [ 429.326244] ffff880404d21d30 ffffffff81485a59 ffff880404d21d50 ffffffffa0137eb7 Mar 24 14:23:16 haswell01 kernel: [ 429.326247] ffff880407769000 ffff880035d46800 ffff880404d21d80 ffffffffa05381d0 Mar 24 14:23:16 haswell01 kernel: [ 429.326250] ffff8803f24e4000 ffff880035d46800 ffff8804081b9000 000000000000000c Mar 24 14:23:16 haswell01 kernel: [ 429.326253] Call Trace: Mar 24 14:23:16 haswell01 kernel: [ 429.326257] [<ffffffff81485a59>] ? device_remove_file+0x19/0x20 Mar 24 14:23:16 haswell01 kernel: [ 429.326267] [<ffffffffa0137eb7>] drm_sysfs_connector_remove+0x57/0x90 [drm] Mar 24 14:23:16 haswell01 kernel: [ 429.326282] [<ffffffffa05381d0>] intel_modeset_cleanup+0xd0/0x100 [i915] Mar 24 14:23:16 haswell01 kernel: [ 429.326288] [<ffffffffa04f95f0>] i915_driver_unload+0x290/0x340 [i915] Mar 24 14:23:16 haswell01 kernel: [ 429.326294] [<ffffffffa01346fc>] drm_dev_unregister+0x2c/0xe0 [drm] Mar 24 14:23:16 haswell01 kernel: [ 429.326299] [<ffffffffa01347eb>] drm_put_dev+0x3b/0x70 [drm] Mar 24 14:23:16 haswell01 kernel: [ 429.326304] [<ffffffffa04f558d>] i915_pci_remove+0x1d/0x20 [i915] Mar 24 14:23:16 haswell01 kernel: [ 429.326307] [<ffffffff8139efbb>] pci_device_remove+0x3b/0xb0 Mar 24 14:23:16 haswell01 kernel: [ 429.326310] [<ffffffff8148a24f>] __device_release_driver+0x7f/0xf0 Mar 24 14:23:16 haswell01 kernel: [ 429.326313] [<ffffffff8148a2e3>] device_release_driver+0x23/0x30 Mar 24 14:23:16 haswell01 kernel: [ 429.326315] [<ffffffff8148905d>] unbind_store+0xbd/0xe0 Mar 24 14:23:16 haswell01 kernel: [ 429.326317] [<ffffffff81488484>] drv_attr_store+0x24/0x40 Mar 24 14:23:16 haswell01 kernel: [ 429.326320] [<ffffffff8122e698>] sysfs_write_file+0x128/0x1c0 Mar 24 14:23:16 haswell01 kernel: [ 429.326323] [<ffffffff811b88c4>] vfs_write+0xb4/0x1f0 Mar 24 14:23:16 haswell01 kernel: [ 429.326325] [<ffffffff811b92f9>] SyS_write+0x49/0xa0 Mar 24 14:23:16 haswell01 kernel: [ 429.326328] [<ffffffff81721c7f>] tracesys+0xe1/0xe6 Mar 24 14:23:16 haswell01 kernel: [ 429.326329] Code: 58 c7 81 e8 7d 98 4e 00 48 83 c4 50 89 d8 5b 41 5c 41 5d 5d c3 bb fe ff ff ff eb e0 66 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 <48> 8b 7f 30 48 8b 36 48 89 e5 e8 bb 22 00 00 5d c3 66 0f 1f 84 Mar 24 14:23:16 haswell01 kernel: [ 429.326351] RIP [<ffffffff8122de46>] sysfs_remove_file_ns+0x6/0x20 Mar 24 14:23:16 haswell01 kernel: [ 429.326353] RSP <ffff880404d21d20> Mar 24 14:23:16 haswell01 kernel: [ 429.326355] CR2: 000000000000002f Mar 24 14:23:16 haswell01 kernel: [ 429.326356] ---[ end trace 8cd466c131375550 ]---

3 6

[pull] radeon drm-next-3.15
by Christian König 25 Mar '14

25 Mar '14

Hi Dave, this is the third pull request for 3.15 radeon changes. Highlights this time: - More DP work from Alex, especially making use of the new DP aux helpers - Marek's 1D and linear tiling fixes for CIK The following changes since commit 63ac07cdee6e1f2bf748ac3f28662e3c01a72496: drm/bridge/ptn3460: fix modular build (2014-03-24 09:21:15 +1000) are available in the git repository at: git://people.freedesktop.org/~deathsimple/linux drm-next-3.15 for you to fetch changes up to 020ff5467603483a97042625d12696c9b39922cf: drm/radeon: set PIPE_CONFIG for 1D and linear tiling modes on CIK (2014-03-25 13:13:24 +0100) ---------------------------------------------------------------- Alex Deucher (7): drm/radeon/dp: use i2c_get_adapdata rather than casting drm/radeon/dp: move sink power control to a separate function drm/radeon/atom: rework encoder enable/disable sequence drm/radeon: clarify special handling in i2c over aux drm/dp: make aux retries less chatty drm/radeon: use the new drm helpers for dp aux drm/radeon: use drm_dp_dpcd_read_link_status() Marek Olšák (1): drm/radeon: set PIPE_CONFIG for 1D and linear tiling modes on CIK drivers/gpu/drm/drm_dp_helper.c | 4 +- drivers/gpu/drm/radeon/atombios_dp.c | 268 +++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------- drivers/gpu/drm/radeon/atombios_encoders.c | 82 ++++++++++++----------------- drivers/gpu/drm/radeon/cik.c | 27 ++++++++-- drivers/gpu/drm/radeon/radeon_connectors.c | 17 ++++-- drivers/gpu/drm/radeon/radeon_drv.c | 3 +- drivers/gpu/drm/radeon/radeon_mode.h | 4 ++ 7 files changed, 202 insertions(+), 203 deletions(-)

1 0

[PATCH] drm/bridge: PTN3460 needs DRM_KMS_HELPER
by Arnd Bergmann 25 Mar '14

25 Mar '14

The recently added PTN3460 device driver uses interfaces that are provided by the KMS helper infrastructure, so we should explicitly select that to avoid this linker error: ERROR: "drm_helper_probe_single_connector_modes" [drivers/gpu/drm/bridge/ptn3460.ko] undefined! ERROR: "drm_helper_connector_dpms" [drivers/gpu/drm/bridge/ptn3460.ko] undefined! We have to drop the I2C dependency to avoid a circular dependency chain, but that's ok because DRM already selects I2C. Signed-off-by: Arnd Bergmann <arnd(a)arndb.de> --- I found this today doing randconfig builds on linux-next. diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index f8db069..884923f 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -1,4 +1,5 @@ config DRM_PTN3460 tristate "PTN3460 DP/LVDS bridge" - depends on DRM && I2C + depends on DRM + select DRM_KMS_HELPER ---help---

2 1

[PATCH v4] drm/i2c: tda998x: Deprecate "nxp,tda998x" in favour of "nxp,tda9989"
by Jean-Francois Moine 25 Mar '14

25 Mar '14

The tda998x driver accepts only 3 chips from the TDA998x family. To avoid confusion with the other TDA998x chips, this patch changes the driver compatible string to "nxp,tda9989". As the previous compatible string is not used in any DT, no compatibility is offered. Signed-off-by: Jean-Francois Moine <moinejf(a)free.fr> --- v4: - the armada and tilcdc DRM drivers are impacted v3: - fix the I2C ID (the OF compatible is not used for such drivers) - define only one compatible (Sebastian Hesselbarth) - change the subject (Sebastian Hesselbarth) v2: - change the subject to drm/i2c This patch applies after drm/i2c: tda998x: Fix lack of required reg in DT documentation --- Documentation/devicetree/bindings/drm/i2c/tda998x.txt | 4 ++-- drivers/gpu/drm/armada/armada_drv.c | 2 +- drivers/gpu/drm/i2c/tda998x_drv.c | 4 ++-- drivers/gpu/drm/tilcdc/tilcdc_slave.c | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt index e9e4bce..9b41c7e 100644 --- a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt @@ -1,7 +1,7 @@ Device-Tree bindings for the NXP TDA998x HDMI transmitter Required properties; - - compatible: must be "nxp,tda998x" + - compatible: must be "nxp,tda9989" - reg: I2C address @@ -20,7 +20,7 @@ Optional properties: Example: tda998x: hdmi-encoder { - compatible = "nxp,tda998x"; + compatible = "nxp,tda9989"; reg = <0x70>; interrupt-parent = <&gpio0>; interrupts = <27 2>; /* falling edge */ diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c index acf3a36..d949279 100644 --- a/drivers/gpu/drm/armada/armada_drv.c +++ b/drivers/gpu/drm/armada/armada_drv.c @@ -45,7 +45,7 @@ static const struct armada_drm_slave_config tda19988_config = { .polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT, .interlace_allowed = true, .info = { - .type = "tda998x", + .type = "tda9989", .addr = 0x70, .platform_data = &params, }, diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 48af5ca..249ef84 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -1367,14 +1367,14 @@ fail: #ifdef CONFIG_OF static const struct of_device_id tda998x_dt_ids[] = { - { .compatible = "nxp,tda998x", }, + { .compatible = "nxp,tda9989", }, { } }; MODULE_DEVICE_TABLE(of, tda998x_dt_ids); #endif static struct i2c_device_id tda998x_ids[] = { - { "tda998x", 0 }, + { "tda9989", 0 }, { } }; MODULE_DEVICE_TABLE(i2c, tda998x_ids); diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c b/drivers/gpu/drm/tilcdc/tilcdc_slave.c index 595068b..eadfed7 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_slave.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_slave.c @@ -112,7 +112,7 @@ static const struct drm_encoder_helper_funcs slave_encoder_helper_funcs = { }; static const struct i2c_board_info info = { - I2C_BOARD_INFO("tda998x", 0x70) + I2C_BOARD_INFO("tda9989", 0x70) }; static struct drm_encoder *slave_encoder_create(struct drm_device *dev, -- 1.9.1

1 0

dma-buf and ->dev refcount fun
by Dave Airlie 25 Mar '14

25 Mar '14

So I've got a reproducable oops with udl sharing from i915, start X, connect UDL, randr it into position, rip out udl device, kill X, we get an oops when dma_unmap_sg in i915_gem_unmap_dma_buf gets called, attachment->dev is pointing to a freed structure, now the drm + udl driver points dev->dev at the USB interface device, however when the device is unplugged, the USB interface device disappears in a poof of smoke, and we just keep a fake shell of the drm device around to keep userspace happy. So I'm wondering if should be using a different device to create dma-buf objects or whether dma-buf objects need to be keeping a reference on the interface device, Dave. [ 147.162846] [drm:i915_gem_unmap_dma_buf] *ERROR* attachment is ffff88002a9f2ab8 ffff880030b1e238 ffff88002a90cf00 [ 147.164228] [drm:i915_gem_unmap_dma_buf] *ERROR* obj is ffff88002a90cf00 [ 147.165641] [drm:i915_gem_unmap_dma_buf] *ERROR* sg is ffff88002a9ef2d0 ffff88002da0ca40 33 [ 147.167089] [drm:i915_gem_unmap_dma_buf] *ERROR* ops is ffff880030829c08 6b6b6b6b6b6b6b6b [ 147.168988] general protection fault: 0000 [#1] SMP [ 147.169379] Modules linked in: udl syscopyarea sysfillrect sysimgblt drm_usb rfcomm fuse ip6table_filter ip6_tables iptable_mangle bnep arc4 iwldvm mac80211 btusb iwlwifi bluetooth snd_hda_codec_conexant snd_hda_codec_generic cfg80211 snd_hda_intel snd_hda_codec snd_hwdep snd_seq e1000e snd_seq_device sdhci_pci sdhci coretemp kvm_intel mmc_core snd_pcm kvm microcode 6lowpan_iphc i2c_i801 thinkpad_acpi snd_timer snd lpc_ich mfd_core rfkill soundcore ptp pps_core wmi firewire_ohci firewire_core crc_itu_t yenta_socket i915 i2c_algo_bit drm_kms_helper drm i2c_core video [ 147.169379] CPU: 0 PID: 481 Comm: Xorg Not tainted 3.14.0-rc7+ #28 [ 147.169379] Hardware name: LENOVO 406334M/406334M, BIOS 6FET79WW (3.09 ) 10/02/2009 [ 147.169379] task: ffff880070ba29d0 ti: ffff88005df74000 task.ti: ffff88005df74000 [ 147.169379] RIP: 0010:[<ffffffffa00ff27e>] [<ffffffffa00ff27e>] i915_gem_unmap_dma_buf+0x10e/0x1a0 [i915] [ 147.169379] RSP: 0018:ffff88005df75c58 EFLAGS: 00010246 [ 147.169379] RAX: 0000000000000000 RBX: ffff88002a9ef2d0 RCX: 00000000831a8319 [ 147.169379] RDX: 00000000000000a0 RSI: 0000000000000001 RDI: ffff88002da0cf40 [ 147.169379] RBP: ffff88005df75c90 R08: 0000000000000000 R09: 0000000000000000 [ 147.169379] R10: 0000000000000000 R11: 0000000000000000 R12: ffff880030829c08 [ 147.169379] R13: ffff88002a90cf00 R14: 0000000000000000 R15: 6b6b6b6b6b6b6b6b [ 147.169379] FS: 00007f5e18ef89c0(0000) GS:ffff880078c00000(0000) knlGS:0000000000000000 [ 147.169379] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b [ 147.169379] CR2: 000000000042fa60 CR3: 000000003f5e7000 CR4: 00000000000007f0 [ 147.169379] Stack: [ 147.169379] 000000212a9f2ab8 ffff88002da0ca40 ffff88002a9f2ab8 ffff88002a9ef2d0 [ 147.169379] 0000000000000000 ffff88003fd413b0 ffff88005df75d70 ffff88005df75cb8 [ 147.169379] ffffffff81487851 ffff88002a9f2ab8 ffff88004446e818 ffff88004446e7b0 [ 147.169379] Call Trace: [ 147.169379] [<ffffffff81487851>] dma_buf_unmap_attachment+0x51/0x80 [ 147.169379] [<ffffffffa00392b2>] drm_prime_gem_destroy+0x22/0x40 [drm] [ 147.169379] [<ffffffffa061e8f5>] udl_gem_free_object+0x35/0x60 [udl] [ 147.169379] [<ffffffffa002241a>] drm_gem_object_free+0x2a/0x30 [drm] [ 147.169379] [<ffffffffa0022a74>] drm_gem_object_handle_unreference_unlocked+0xe4/0x120 [drm] [ 147.169379] [<ffffffffa0022cfc>] drm_gem_object_release_handle+0x5c/0x80 [drm] [ 147.169379] [<ffffffff8136fcc4>] idr_for_each+0x104/0x1a0 [ 147.169379] [<ffffffffa0022ca0>] ? drm_gem_dumb_destroy+0x20/0x20 [drm] [ 147.169379] [<ffffffff8172b5fe>] ? mutex_unlock+0xe/0x10 [ 147.169379] [<ffffffffa0023750>] drm_gem_release+0x20/0x30 [drm] [ 147.169379] [<ffffffffa00222a3>] drm_release+0x533/0x5c0 [drm] [ 147.169379] [<ffffffff811eb2f5>] __fput+0xf5/0x2c0 [ 147.169379] [<ffffffff811eb50e>] ____fput+0xe/0x10 [ 147.169379] [<ffffffff81099874>] task_work_run+0xb4/0xe0 [ 147.169379] [<ffffffff81072d4c>] do_exit+0x2ec/0xcd0

4 5

Re: [Linaro-mm-sig] [PATCH] dma-buf: add meta data attachment
by Daniel Vetter 25 Mar '14

25 Mar '14

Hi all, Adding piles more people. For the first case of caching the iommu mapping there's different answers, depening upon the exact case: 1) You need to fix your userspace to not constantly re-establish the sharing. 2) We need to add streaming dma support for real to dma-bufs so that the mapping can be kept while we transfer ownership around. Thus far no one really needed this though since usually you don't actually do cpu access. 3) You need opportunistic caching of imported/exported buffer objects and their mappings. For this you need a) subsystem import/export support which guarantees you to hand out the same dma-buf/native object again upon re-export or re-import (drm has it) b) some opportunistic caching of buffer objects (pretty much are real gpu drm drivers have it). No need of any metadata scheme, and given how much fun I've had implemented this for drm I don't you can make your metadata scheme work in a sane or correct way wrt lifetimes. For caching the iommu mapping if the iommu is the same for multiple devices: 1) We need some way to figure out which iommu serves which devices. 2) The exporter needs to consult this and might just hand out the same sg mapping out again if it wants to. No need for importers to do fancy stuff, or attach any importer-visible metadata to dma-bufs. Of course duplicating this code all over the place is a but uncool, so I expect that eventually we'll have a generic exporter implementation, at least for non-swappable buffers. drm/gem is a bit special here ... In general I don't like the idea of arbitrary metadata at all, sounds prone to abuse with crazy lifetime/refcounting rules for the objects involved. Also I think for a lot of your examples (like debugging) it would be much better if we have a standardized piece of metadata so that all drivers/platforms can use the same tooling. And it feels like I'm writing such a mail every few months ... Cheers, Daniel On Mon, Mar 24, 2014 at 7:20 AM, Bin Wang <binw(a)marvell.com> wrote: > Hi Sumit, > > On 03/21/2014 07:26 PM, Sumit Semwal wrote: >> Hi Bin Wang, >> >> On 21 March 2014 10:34, Bin Wang <binw(a)marvell.com> wrote: >>> we found there'd be varied specific data connected to a dmabuf, like >>> the iommu mapping of buffer, the dma descriptors (that can be shared >>> between several components). Though these info might be able to be >>> generated every time before dma operations, for performance sake, >>> it's better to be kept before really invalid. >> Thanks for your patch! >> I think we'd need to have more specific details on what does 'meta >> data' mean here; more specific comments inline. >>> Change-Id: I89d43dc3fe1ee3da91c42074da5df71b968e6d3c >>> Signed-off-by: Bin Wang <binw(a)marvell.com> >>> --- >>> drivers/base/dma-buf.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++ >>> include/linux/dma-buf.h | 22 ++++++++++ >>> 2 files changed, 122 insertions(+), 0 deletions(-) >>> >>> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c >>> index 08fe897..5c82e60 100644 >>> --- a/drivers/base/dma-buf.c >>> +++ b/drivers/base/dma-buf.c >>> @@ -50,6 +50,7 @@ static int dma_buf_release(struct inode *inode, struct file *file) >>> >>> BUG_ON(dmabuf->vmapping_counter); >>> >>> + dma_buf_meta_release(dmabuf); >>> dmabuf->ops->release(dmabuf); >>> >>> mutex_lock(&db_list.lock); >>> @@ -138,6 +139,7 @@ struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops *ops, >>> >>> mutex_init(&dmabuf->lock); >>> INIT_LIST_HEAD(&dmabuf->attachments); >>> + INIT_LIST_HEAD(&dmabuf->metas); >>> >> I am not sure I understand the relationship of 'meta data' with >> 'dma-buf', or 'attachments' - do you mean to have a list of some >> meta-data that is unrelated to the attachments to the dma-buf? I think >> it'd help to explain whether meta-data is specific to each dma-buf, >> and there's a list of them, or this meta-data is related to each >> importer device attachment? >> If it's related to each importer, it should really be added as part of >> the dma_buf_attachment, and not separately. > The "meta-data" here can be any kind of data related to the dmabuf, , it's specific > for each dmabuf. > > For example, we have iommu for a VPU device, VPU driver has > to map the dmabuf to get an IOVA address before it can access the buffer. This > dmabuf would be reused many times during the video playback, for performance > concern we don't want this dmabuf be map/unmaped by iommu driver every time > of VPU HW data transfer, since the physical pages not changed and iommu IOVA > plenty. > > Another example, in our SoC, device A and device B use the same dma chain format, > and these drivers share dmabuf between each other, thus the "dma chain" > array doesn't need to be generated every timer for each device. > > So here, with the help from the "meta-data", only the "first time" we need to generate > the "meta-data" from scratch, and later we can just reuse it until the buffer freed. > > The "meta-data" is related to the importer, or importers. However, the "dma_buf_attachment" > is per "devices", holding device specific attributes, while "meta-data" is per "dmabuf', > holding buffer specific attributes, like the example above. >>> mutex_lock(&db_list.lock); >>> list_add(&dmabuf->list_node, &db_list.head); >>> @@ -570,6 +572,104 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr) >>> } >>> EXPORT_SYMBOL_GPL(dma_buf_vunmap); >>> >>> +/** >>> + * dma_buf_meta_attach - Attach additional meta data to the dmabuf >>> + * @dmabuf: [in] the dmabuf to attach to >>> + * @id: [in] the id of the meta data >>> + * @pdata: [in] the raw data to be attached >>> + * @release: [in] the callback to release the meta data >>> + */ >>> +int dma_buf_meta_attach(struct dma_buf *dmabuf, int id, void *pdata, >>> + int (*release)(void *)) >>> +{ >>> + struct dma_buf_meta *pmeta; >>> + >>> + pmeta = kmalloc(sizeof(struct dma_buf_meta), GFP_KERNEL); >>> + if (pmeta == NULL) >>> + return -ENOMEM; >>> + >>> + pmeta->id = id; >> What does 'id' mean here? Also there is no check on any duplicity on >> the 'id', so if any device adds more meta data with the same ID, you >> would always work only on the first one? It would also help if you >> could explain the relevance of id. > The "id" here means a kind of specific "meta-data", for example we use > "#define VPU_DMABUF_META_ID 0x10000" for the VPU iommu related > data. > > Each importer would try to "dma_buf_meta_fetch()" the "meta-data" first, if > not found, it would generate and attach it. If found, it would just reuse it. > > I agree with you that it's better to add the check of id in this function as well. >>> + pmeta->pdata = pdata; >>> + pmeta->release = release; >>> + >>> + mutex_lock(&dmabuf->lock); >>> + list_add(&pmeta->node, &dmabuf->metas); >>> + mutex_unlock(&dmabuf->lock); >>> + >>> + return 0; >>> +} >>> +EXPORT_SYMBOL_GPL(dma_buf_meta_attach); >>> + >>> +/** >>> + * dma_buf_meta_dettach - Dettach the meta data from dmabuf by id >>> + * @dmabuf: [in] the dmabuf including the meta data >>> + * @id: [in] the id of the meta data >>> + */ >>> +int dma_buf_meta_dettach(struct dma_buf *dmabuf, int id) >>> +{ >>> + struct dma_buf_meta *pmeta, *tmp; >>> + int ret = -ENOENT; >>> + >>> + mutex_lock(&dmabuf->lock); >>> + list_for_each_entry_safe(pmeta, tmp, &dmabuf->metas, node) { >>> + if (pmeta->id == id) { >>> + if (pmeta->release) >>> + pmeta->release(pmeta->pdata); >>> + list_del(&pmeta->node); >>> + kfree(pmeta); >>> + ret = 0; >>> + break; >>> + } >>> + } >>> + mutex_unlock(&dmabuf->lock); >>> + >>> + return ret; >>> +} >>> +EXPORT_SYMBOL_GPL(dma_buf_meta_dettach); >>> + >>> +/** >>> + * dma_buf_meta_fetch - Get the meta data from dmabuf by id >>> + * @dmabuf: [in] the dmabuf including the meta data >>> + * @id: [in] the id of the meta data >>> + */ >>> +void *dma_buf_meta_fetch(struct dma_buf *dmabuf, int id) >>> +{ >>> + struct dma_buf_meta *pmeta; >>> + void *pdata = NULL; >>> + >>> + mutex_lock(&dmabuf->lock); >>> + list_for_each_entry(pmeta, &dmabuf->metas, node) { >>> + if (pmeta->id == id) { >>> + pdata = pmeta->pdata; >>> + break; >>> + } >>> + } >>> + mutex_unlock(&dmabuf->lock); >>> + >>> + return pdata; >>> +} >>> +EXPORT_SYMBOL_GPL(dma_buf_meta_fetch); >>> + >>> +/** >>> + * dma_buf_meta_release - Release all the meta data attached to the dmabuf >>> + * @dmabuf: [in] the dmabuf including the meta data >>> + */ >>> +void dma_buf_meta_release(struct dma_buf *dmabuf) >>> +{ >>> + struct dma_buf_meta *pmeta, *tmp; >>> + >>> + mutex_lock(&dmabuf->lock); >>> + list_for_each_entry_safe(pmeta, tmp, &dmabuf->metas, node) { >>> + if (pmeta->release) >>> + pmeta->release(pmeta->pdata); >>> + list_del(&pmeta->node); >>> + kfree(pmeta); >>> + } >>> + mutex_unlock(&dmabuf->lock); >>> + >>> + return; >>> +} >>> + >>> #ifdef CONFIG_DEBUG_FS >>> static int dma_buf_describe(struct seq_file *s) >>> { >>> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h >>> index dfac5ed..369d032 100644 >>> --- a/include/linux/dma-buf.h >>> +++ b/include/linux/dma-buf.h >>> @@ -120,6 +120,7 @@ struct dma_buf { >>> size_t size; >>> struct file *file; >>> struct list_head attachments; >>> + struct list_head metas; >>> const struct dma_buf_ops *ops; >>> /* mutex to serialize list manipulation, attach/detach and vmap/unmap */ >>> struct mutex lock; >>> @@ -149,6 +150,20 @@ struct dma_buf_attachment { >>> }; >>> >> Like I said above, I don't think I completely understand the need of >> 'a list of metas' - a sample usage would be most helpful to realise >> the relevance here. > As the examples above, in the list of "metas", there can be, VPU iommu mapping info, > camera/display dma chain info, or other debugging/tracking info etc. >>> /** >>> + * struct dma_buf_meta - holds varied meta data attached to the buffer >>> + * @id: the identification of the meta data >>> + * @dmabuf: buffer for this attachment. >>> + * @node: list of dma_buf_meta. >>> + * @pdata: specific meta data. >>> + */ >>> +struct dma_buf_meta { >>> + int id; >>> + struct list_head node; >>> + int (*release)(void *pdata); >>> + void *pdata; >>> +}; >>> + >>> +/** >>> * get_dma_buf - convenience wrapper for get_file. >>> * @dmabuf: [in] pointer to dma_buf >>> * >>> @@ -194,6 +209,13 @@ int dma_buf_mmap(struct dma_buf *, struct vm_area_struct *, >>> unsigned long); >>> void *dma_buf_vmap(struct dma_buf *); >>> void dma_buf_vunmap(struct dma_buf *, void *vaddr); >>> + >>> +int dma_buf_meta_attach(struct dma_buf *dmabuf, int id, void *pdata, >>> + int (*release)(void *)); >>> +int dma_buf_meta_dettach(struct dma_buf *dmabuf, int id); >>> +void *dma_buf_meta_fetch(struct dma_buf *dmabuf, int id); >>> +void dma_buf_meta_release(struct dma_buf *dmabuf); >>> + >>> int dma_buf_debugfs_create_file(const char *name, >>> int (*write)(struct seq_file *)); >>> #endif /* __DMA_BUF_H__ */ >>> -- >>> 1.7.0.4 >>> >>> >>> _______________________________________________ >>> Linaro-mm-sig mailing list >>> Linaro-mm-sig(a)lists.linaro.org >>> http://lists.linaro.org/mailman/listinfo/linaro-mm-sig >> >> > The purpose of this extension mainly is: > 1. Try to reuse the first generated specific data in the buffer's life time, to avoid redundant map/unmap, or generate/free. > 2. Try to share the "reusable" data between devices, if the devices support compatible formats. > > Regards, > Bin Wang > _______________________________________________ > Linaro-mm-sig mailing list > Linaro-mm-sig(a)lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-mm-sig -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch

3 2

another interesting lock recursion!
by Dave Airlie 25 Mar '14

25 Mar '14

So with runtime pm on nouveau, if the card gets powered down, and then you access a connector via sysfs, drm_sysfs.c:status_show locks the connector and calls into the driver, the driver then does a runtime_get_sync, which causes resume to happen which causes modesetting to reset the mode, which tries to take all the locks, and it all deadlocks in a pile, Ideas to fix this, move the runtime_get_sync into the drm_sysfs code, or allow the drivers to provide their own status show functions, moving the runtime get into the sysfs code probably involves having conditionals for device that support runtime PM at all, otherwise who knows what might happen, Dave.

2 3

← Newer
1
...
67
68
69
70
71
72
73
...
93
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

dri-devel March 2014